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

fix creating secrets for rotation users #2863

Merged
merged 5 commits into from
Feb 14, 2025
Merged

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Feb 12, 2025

In #2657 a bug was introduced which produces additional secrets for rotation users which are redundant to the original secret. Before syncing secrets we usually initialize the user list to get the desired state.

On UPDATE events we only needed to sync secrets in case the user setup changed. initUsers and syncSecrets were called in one if block. The behavior was changed to have to if blocks where secrets can be synced on annotations diff while skipping user initializing. Therefore, the current pgUser list is used which contains the rotation users instead of the original ones.

We better return to the previous behavior incl. the annotation diff check, even though we would probably always init users then because UPDATEs are usually tiggered from deployments changing the deployment-time annotation.

Another quick hack would be to not generate a secret for rotation users (when Rotated flag is true), but since rotation users replace their original counterparts in the pgUsers list, we would then not sync the corresponding secrets.

In a first step I extended the e2e test without changing any other code (quick hack only as a comment) to reproduce the bug.

@FxKu FxKu added the bugfix label Feb 12, 2025
@FxKu FxKu added this to the 1.15.0 milestone Feb 12, 2025
@hughcapet
Copy link
Member

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Feb 14, 2025

👍

@FxKu FxKu merged commit 2a4be1c into master Feb 14, 2025
10 checks passed
@FxKu FxKu deleted the rotation-secret-count-fix branch February 14, 2025 08:44
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.

2 participants