-
Notifications
You must be signed in to change notification settings - Fork 12
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
[DPE-3705, DPE-3542] Reintroduce fallback keys and fix TLS secrets initialization #427
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #427 +/- ##
==========================================
- Coverage 66.39% 66.25% -0.14%
==========================================
Files 17 17
Lines 3169 3180 +11
Branches 419 424 +5
==========================================
+ Hits 2104 2107 +3
- Misses 930 935 +5
- Partials 135 138 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks correct. Two things:
- Did you've tests with some older charm revision?
- The mandatory libpatch bump!
Updated libpatch in 9cddb26 I tried testing upgrades from previous stable versions:
|
it's available from releases page (e.g. https://github.com/canonical/mysql-operator/releases?q=292&expanded=true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (not sure if this is in scope): when upgrading to this version of the charm, is it safe to rollback to previous version?
self.peer_relation_unit.update_relation_data(peers.id, {key: value}) | ||
|
||
fallback_key_to_secret_key = {v: k for k, v in SECRET_KEY_FALLBACKS.items()} | ||
if key in fallback_key_to_secret_key: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: when would this be true on the new charm?
my naive thought is that the new charm would always use the new keys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would be true when an old charm (running on juju <= 3.1.4 when secrets were not introduced) would upgrade to the new charm. in this case, the value from the old fallback_key should be deleted, and new value should be stored in juju secrets with the new key
this functionality was present in the charm, but was accidentally removed with a refactor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the new charm code ever call set_secret
with key=old key name?
like, we're checking the key
passed as an argument to see if it's a fallback key—we're not (I think) checking the key in the databag/secret
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe that the old (pre backups) code called set_secret
with keys like (root_password) -> this is incompatible with juju secrets due to the underscore. in the this old code, set_secret
set password in peer databag
the logic is as follows:
- if the old/fallback key exists in either the databag (or secret - but cannot exist in secret), then delete it
- update the new key in either the secret or the databag using data_interfaces
Issue
cert
instead ofcertificate
,cauth
instead ofcertificate-authority
)Solution