[DPE-2442] Add juju secrets support #242
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmitry-ratushnyy
force-pushed
the
dmitry.ratushnyy/DPE-2442_fix
branch
3 times, most recently
from
4ef75ad to
b604378
Compare
dmitry-ratushnyy
changed the title
delgod
requested changes
Sep 4, 2023
| """Helper function to get Juju secret.""" | ||
| peer_data = self._peer_data(scope) | ||
|
|
||
| if not peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL): |
There was a problem hiding this comment.
Peer data bag stores the Secret ID inside in SECRET_INTERNAL_LABEL field.
The current code cannot retrieve the Secret object if we don't have the Secret ID (line 1126 below).
I think this part is going to be revoked in two weeks.
src/charm.py
Outdated
| return self.secrets[scope][Config.Secrets.SECRET_LABEL].id | ||
|
|
||
| def _juju_secrets_get(self, scope: Scopes) -> Optional[bool]: | ||
| """Helper function to get Juju secret.""" |
There was a problem hiding this comment.
the function name can be better. in fact, it is just caching secret content into the internal dictionary.
juditnovak
requested changes
Sep 4, 2023
delgod
previously approved these changes
Sep 5, 2023
MiaAltieri
added a commit
that referenced
this pull request
Sep 6, 2023
## Problem After implementing backups on K8s the libraries in each charm are out of sync. ## Solution Update the library code and update the charm to support these new changes ## Other changes Testing versions and requirements updates to solve depency issues in tests ## Future PRs The backup library has had substantial changes and so the old test suite is no longer supported. In a future PR these will be added. Failing TLS tests are resolved in #242 ## do NOT review these files - `lib/charms/mongodb/v0/helpers.py` - copy and paste from k8s charm - `lib/charms/mongodb/v0/mongodb_backups.py` - copy and paste from VM charm - `tests/unit/test_mongodb_backups.py` - removed as is now out of date, will be rewritten in a follow up PR
dmitry-ratushnyy
force-pushed
the
dmitry.ratushnyy/DPE-2442_fix
branch
4 times, most recently
from
377ec57 to
e4200f1
Compare
Updated doc string Co-authored-by: Mia Altieri <[email protected]>
dmitry-ratushnyy
force-pushed
the
dmitry.ratushnyy/DPE-2442_fix
branch
from
688f0e5 to
ea883c5
Compare
juditnovak
reviewed
Sep 13, 2023
juditnovak
reviewed
Sep 13, 2023
juditnovak
reviewed
Sep 13, 2023
delgod
previously approved these changes
Sep 13, 2023
MiaAltieri
previously approved these changes
Sep 14, 2023
src/charm.py
Outdated
| return self.secrets[scope][Config.Secrets.SECRET_LABEL].id | ||
|
|
||
| def _juju_secrets_get(self, scope: Scopes) -> Optional[bool]: | ||
| """Helper function to get Juju secret.""" |
There was a problem hiding this comment.
I like Judits docstring here, I think it explains the complexities that are going on. Unless you feel strongly about removing it.
juditnovak
reviewed
Sep 14, 2023
juditnovak
requested changes
Sep 14, 2023
src/charm.py
Outdated
| return self.secrets[scope][Config.Secrets.SECRET_LABEL].id | ||
|
|
||
| def _juju_secrets_get(self, scope: Scopes) -> Optional[bool]: | ||
| """Helper function to get Juju secret.""" |
There was a problem hiding this comment.
I agree with myself and @MiaAltieri :-D
Could we pls use the more verbose (and actually correct 😅 ) docstring here?
I apologize for the original one, it's on me, I'll update it in k8s as well
Co-authored-by: Mia Altieri <[email protected]>
dmitry-ratushnyy
dismissed stale reviews from MiaAltieri and delgod
via
45456f1
juditnovak
approved these changes
Sep 14, 2023
juditnovak
previously approved these changes
Sep 14, 2023
There was a problem hiding this comment.
Approving under the condition to pls address outstanding issues in the following tickets:
https://warthogs.atlassian.net/browse/DPE-2609
https://warthogs.atlassian.net/browse/DPE-2610
https://warthogs.atlassian.net/browse/DPE-2611
https://warthogs.atlassian.net/browse/DPE-2612
MiaAltieri
approved these changes
Sep 14, 2023
juditnovak
approved these changes
Sep 14, 2023
delgod
approved these changes
Sep 15, 2023
|
|
||
| secret_cache[key] = Config.Secrets.SECRET_DELETED_LABEL | ||
| secret.set_content(secret_cache) | ||
| logging.debug(f"Secret {scope}:{key}") |
There was a problem hiding this comment.
Can we or improve the message here or remove it?
Suggested change
| logging.debug(f"Secret {scope}:{key}") | |
| logging.debug(f"Secret {scope}:{key} removed") |
MiaAltieri
added a commit
that referenced
this pull request
Sep 15, 2023
Add support of juju secrets to the charm [DPE-2441](https://warthogs.atlassian.net/browse/DPE-2441) [DPE-2442](https://warthogs.atlassian.net/browse/DPE-2442) [DPE-2443](https://warthogs.atlassian.net/browse/DPE-2443) [DPE-2441]: https://warthogs.atlassian.net/browse/DPE-2441?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DPE-2442]: https://warthogs.atlassian.net/browse/DPE-2442?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DPE-2443]: https://warthogs.atlassian.net/browse/DPE-2443?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Dmitry Ratushnyy <[email protected]> Co-authored-by: Mia Altieri <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Add support of juju secrets to the charm
Related issues
DPE-2441
DPE-2442
DPE-2443