Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let isSystemUsername check all system users #2489

Merged
merged 4 commits into from Dec 8, 2023
Merged

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Nov 29, 2023

When one defines a fes_user in the manifest and has also streams defined, the database password of this user gets overridden. The secret, however, is not touched. Therefore, removing the user from the manifest immediately fixes the issue.

I found that when initializing robot / manifest users the check for name collisions with already defined system users is only done for the configured SuperUsername and ReplicationUsername - not for e.g. pooler of stream user. I changed it now to a loop over all system users that are already initialized at this point (not in the unit test though). Because, if this check does not happen, somewhere done the line the database role picks up a new random password. fes_user will be part of c.pgUsers and override the existing sytem user with the same name in the list of newUsers to sync here and here. Interestingly, I find no ALTER ROLE statements in the Postgres logs (probably because we don't want to log passwords).

Btw: The same bug affects the connection pooler role as well.

@FxKu FxKu added the bug label Nov 29, 2023
@FxKu FxKu added this to the 2.0 milestone Nov 29, 2023
@jopadi
Copy link
Member

jopadi commented Dec 8, 2023

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Dec 8, 2023

👍

@FxKu FxKu merged commit 9ee14f2 into master Dec 8, 2023
9 checks passed
@FxKu FxKu deleted the fix-isSystemUsername branch December 8, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants