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

Don't watch secrets #808

Merged
merged 1 commit into from
May 7, 2024
Merged

Don't watch secrets #808

merged 1 commit into from
May 7, 2024

Conversation

mkuratczyk
Copy link
Contributor

Watching the secrets can lead to high memory usage if there's a lot of them. It seems like we don't actually rely on this, so we can turn that off completely.

Fixes #783

Watching the secrets can lead to high memory usage if there's a lot of
them. It seems like we don't actually rely on this, so we can turn that
off completely.

Fixes #783
Copy link
Contributor

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will have the side effect that updating the Secret won't update the user credentials in RabbitMQ. I think it's a fair trade-off, because we are fixing a legitimate problem. As a workaround, folks can use time-based reconciliation.

@Zerpet Zerpet added this to the v1.14 milestone May 7, 2024
@mkuratczyk
Copy link
Contributor Author

I just tried that with the 1.13.0 and it didn't work anyway. What I did was:

  1. deploy the userPreDefinedCreds.yaml example
  2. check with rabbitmqctl authenticate_user
  3. change the secret and re-apply userPreDefinedCreds.yaml
  4. check again with rabbitmqctl authenticate_user - the old password works, the new one doesn't

That's in line with the comment: https://github.com/rabbitmq/messaging-topology-operator/blob/main/docs/examples/users/userPreDefinedCreds.yaml#L7

@mkuratczyk mkuratczyk merged commit 7100873 into main May 7, 2024
7 checks passed
@Zerpet Zerpet deleted the do-not-watch-secrets branch May 7, 2024 12:28
@Zerpet
Copy link
Contributor

Zerpet commented May 7, 2024

Sure, it won't work with pre-defined creds, but it should work with secrets created as part of the User creation. In any case, time-based reconciliation would be the workaround.

@mkuratczyk
Copy link
Contributor Author

This functionality still works for me:

  1. deploy docs/examples/users/user.yaml
  2. rabbitmqctl authenticate_user
  3. edit the user-sample-user-credentials
  4. try the new password

Seems like we didn't lose anything and we gained lower memory usage and higher security. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow memory leak
3 participants