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

[DPE-3184] Update secrets implementation #145

Merged
merged 3 commits into from
Feb 6, 2024
Merged

Conversation

dragomirp
Copy link
Contributor

@dragomirp dragomirp commented Feb 5, 2024

Update peer secrets implementation to be up to date with other PG charms

pyproject.toml Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missed the linter configs when ruff was updated

Comment on lines -240 to -253

def add_user(self, username: str, password: str):
"""Adds user to app databag."""
if not self.charm.unit.is_leader():
return

self.charm.set_secret(APP_SCOPE, username, password)

def remove_user(self, username: str):
"""Removes user from app databag."""
if not self.charm.unit.is_leader():
return

self.charm.set_secret(APP_SCOPE, username, None)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing arbitrary secret fields as with the k8s charm

@dragomirp dragomirp marked this pull request as ready for review February 5, 2024 19:14
Copy link
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

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

LGTM!

src/charm.py Outdated
else:
del self.peers.app_databag[key]
self.peer_relation_unit.delete_relation_data(peers.id, [secret_key])
Copy link
Contributor

Choose a reason for hiding this comment

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

I really should have introduced a

def peer_relation_data(scope: Scope) -> DataPeer:
    if scope == APP_SCOPE:
        return self.peer_relation_app
    elif scope == APP_UNIT:
        return self.peer_relation_unit     

function, to turn the above (repetitive) structure to

    self.peer_relation_data(scope).delete_relation_data(peers.id, [secret_key])

If you've got the capacity, don't U wanna do that? If not, of course feel free to ignore (it's cosmetics)

@dragomirp dragomirp requested a review from juditnovak February 6, 2024 11:22
Copy link
Contributor

@juditnovak juditnovak left a comment

Choose a reason for hiding this comment

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

Even more approved than one round earlier :-)

@dragomirp dragomirp merged commit c608b55 into main Feb 6, 2024
28 checks passed
@dragomirp dragomirp deleted the dpe-3184-secrets-update branch February 6, 2024 14:18
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.

4 participants