feat(profile): add secrets handling to kubernetes and machine init profiles - #2814
Open
tonyandrewmeyer wants to merge 8 commits into
Open
feat(profile): add secrets handling to kubernetes and machine init profiles#2814tonyandrewmeyer wants to merge 8 commits into
tonyandrewmeyer wants to merge 8 commits into
Conversation
- charmcraft.yaml: api-token config option (type: secret) with operator workflow comments; documents add-secret / grant-secret / config steps - src/charm.py: resolve api-token user secret on config-changed, surface BlockedStatus on SecretNotFoundError; create app-managed workload-password secret (SecretRotate.MONTHLY, expire=90d) idempotently on pebble-ready (leader only); handle secret-rotate and secret-expired - tests/unit/test_charm.py: eight scenario tests covering pebble-ready (no-op non-leader, creates secret on leader, idempotent if already exists), config-changed (active on resolution, blocked if not found, no-op if unset), secret-rotate (new revision with fresh password), secret-expired (removes expired revision) - tests/integration/test_charm.py: test_user_secret (add-secret / grant / config / wait active), test_app_managed_secret_created (list-secrets confirms workload-password visible after deploy)
- charmcraft.yaml: api-token config option (type: secret) with operator workflow comments mirroring the kubernetes profile - src/charm.py: resolve api-token user secret on config-changed with BlockedStatus on SecretNotFoundError; create app-managed workload-password secret (SecretRotate.MONTHLY, expire=90d) on start (leader only); handle secret-rotate and secret-expired - tests/unit/test_charm.py: eight scenario tests covering start (creates secret on leader, idempotent if already exists), config-changed (active on resolution, blocked if not found, no-op if unset), secret-rotate (new revision), secret-expired (removes expired revision) - tests/integration/test_charm.py: test_user_secret (add-secret / grant / config / wait active), test_app_managed_secret_created (list-secrets confirms workload-password visible after deploy)
The secrets example observed secret-rotate and secret-expired only, which left two gaps. A new revision of the user-provided api-token secret never reached the workload, because only config-changed read it; secret-changed now re-reads it with refresh=True. And every rotation left its old revision behind, because nothing handled secret-remove. The api-token secret is now fetched with a label, so the secret-changed handler can tell it apart from the app-managed one, and config-changed and secret-changed share a single _configure_api_token method. Also fixes two bugs in the existing example. The secret-expired handler removed the expiring revision, but that is the latest revision and Juju charms must not remove it -- it now adds a new revision, and the cleanup happens on secret-remove. And the unit tests used a hardcoded secret ID that is not a valid Juju secret identifier (those are 'secret:' followed by exactly 20 characters), so every test touching the api-token config errored out on an inconsistent scenario.
The secrets integration tests shelled out to 'juju' with subprocess, which skipped the model that pytest-jubilant creates for the test -- those calls went to whatever model happened to be current. Jubilant has first-class support for all of this: secrets(), add_secret(), grant_secret() and update_secret(). Also covers secret-changed end to end: the test adds a new revision of the user secret and checks that the charm re-read it, rather than only checking that the charm resolved the secret once.
… test The charm fixture asserts that exactly one built charm exists before the test runs, so requesting it makes a test fail immediately with a clear message when the charm hasn't been packed. Two of the secrets tests never took it, and test_workload_version_is_set had it removed.
…mment The three juju commands were listed in a comment above the option and again in its description. The description is the copy operators actually see, in 'juju config' output and on Charmhub, so keep that one.
Contributor
Author
|
@dwilding would you reviewing this? |
This file contains hidden or 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
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.
Extends both the
kubernetesandmachineprofiles so thatcharmcraft initscaffolds a charm that handles Juju secrets from both sides, with unit and integration tests for it.The profile is a strong signal to human and agentic charmers for best practices. Secrets are a good candidate for that signal because they are easy to get half-right: reading an observed secret once on
config-changedlooks correct until the operator adds a new revision and nothing happens, and rotating an owned secret looks correct until revisions pile up because nothing removes them. Both failures are silent, so the profile handles the full event set rather than the two events that are enough for a demo.A charm also meets secrets in two roles that need different code — it observes secrets an operator grants it, and it owns secrets it generates for its workload. A charm author who has only seen one usually writes the other incorrectly.
Changes
Both profiles in
charmcraft.yaml.j2:Adds an
api-tokenconfig option oftype: secret, with a description giving the operator the three commands they need (juju add-secret,juju grant-secret,juju config).In both profiles, the observed secret:
config-changedandsecret-changedshare one_configure_api_token(), which fetches withmodel.get_secret(id=..., label=API_TOKEN_LABEL)and reads withget_content(refresh=True). Attaching the label is what lets thesecret-changedhandler recognise the secret, since the event carries the label rather than the config value. A secret that isn't granted, or has gone away, leaves the unit blocked.In both profiles, the app-managed secret:
The leader creates a workload password on
start(machine) orpebble-ready(kubernetes), with monthly rotation and a 90 day expiry; creation is looked up by label first, so it's idempotent.secret-rotateandsecret-expiredboth add a new revision — expiry doesn't remove the expiring revision, because that's the latest one and other units may still be reading it.secret-removedoes the cleanup, once Juju reports that no unit is tracking a revision.make lint && make test.I've added or updated any relevant documentation.In documents I changed, I added a meta description if one was missing.