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: generate new ca cert on config changed #140

Merged
merged 2 commits into from
Apr 18, 2024
Merged

fix: generate new ca cert on config changed #140

merged 2 commits into from
Apr 18, 2024

Conversation

gruyaume
Copy link
Contributor

Description

There is a bug where on ca common name config changed, the self-signed certificates charm would generate a new CA, store it in a secret but we would read the secret in the same event handler and it would be the old secret.

Now, we return after changing the root CA certificate and updating the secret and we observe the "secret_changed" event. This allows for reading the new ca certificate instead of reading the old one.

Fixes #132

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have bumped the version of the library

@gruyaume gruyaume requested a review from a team as a code owner April 17, 2024 14:58
Copy link
Contributor

@saltiyazan saltiyazan left a comment

Choose a reason for hiding this comment

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

In general this looks good, I have only one concern to validate.
And the title of the PR is not the most accurate, we did generate the CA until now, this has not changed.

src/charm.py Show resolved Hide resolved
@gruyaume gruyaume requested review from saltiyazan and kayra1 April 17, 2024 16:54
tests/integration/test_integration.py Dismissed Show dismissed Hide dismissed
Copy link
Contributor

@saltiyazan saltiyazan left a comment

Choose a reason for hiding this comment

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

Looks good, I'd still use a more accurate title :), we did generate the CA before this PR too, we didn't use it as the secret was not updated.

@gruyaume gruyaume merged commit 97b9dfd into main Apr 18, 2024
18 checks passed
@gruyaume gruyaume deleted the dev-TLSENG-171 branch April 18, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CA is not being regenerated when configured common name changes
3 participants