Skip to content

Commit

Permalink
feat: secret owners do not auto-peek, and can use refresh (#1067)
Browse files Browse the repository at this point in the history
Update the testing harness to reflect an upcoming Juju change
regarding secrets: the special-casing for secret-get is going to be
removed, so that owners have the same behaviour as everyone
else - they get the tracked revision of the secret, and can call peek
to get the latest content, or refresh to get the latest content and
update the tracking pointer.

Also extends the Harness tests for getting secrets.

Fixes #1062
  • Loading branch information
tonyandrewmeyer authored Nov 23, 2023
1 parent b410730 commit a7f34cb
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 8 deletions.
3 changes: 1 addition & 2 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1291,8 +1291,7 @@ def get_content(self, *, refresh: bool = False) -> Dict[str, str]:
Args:
refresh: If true, fetch the latest revision's content and tell
Juju to update to tracking that revision. The default is to
get the content of the currently-tracked revision. This
parameter is only meaningful for secret observers, not owners.
get the content of the currently-tracked revision.
"""
if refresh or self._content is None:
self._content = self._backend.secret_get(
Expand Down
7 changes: 1 addition & 6 deletions ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2445,12 +2445,7 @@ def secret_get(self, *,
secret = self._ensure_secret_id_or_label(id, label)

# Check that caller has permission to get this secret
if secret.owner_name in [self.app_name, self.unit_name]:
# Owner or peer is calling, get latest revision
peek = True
if refresh:
raise ValueError('Secret owner cannot use refresh=True')
else:
if secret.owner_name not in [self.app_name, self.unit_name]:
# Observer is calling: does secret have a grant on relation between
# this charm (the observer) and the secret owner's app?
owner_app = secret.owner_name.split('/')[0]
Expand Down
53 changes: 53 additions & 0 deletions test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -4712,6 +4712,59 @@ def test_add_model_secret_by_unit_instance(self):
self.assertEqual(secret.id, secret_id)
self.assertEqual(secret.get_content(), {'password': 'hunter4'})

def test_get_secret_as_owner(self):
harness = ops.testing.Harness(ops.CharmBase, meta='name: webapp')
self.addCleanup(harness.cleanup)
harness.begin()
# App secret.
secret_id = harness.charm.app.add_secret({'password': 'hunter5'}).id
secret = harness.model.get_secret(id=secret_id)
self.assertEqual(secret.id, secret_id)
self.assertEqual(secret.get_content(), {'password': 'hunter5'})
# Unit secret.
secret_id = harness.charm.unit.add_secret({'password': 'hunter6'}).id
secret = harness.model.get_secret(id=secret_id)
self.assertEqual(secret.id, secret_id)
self.assertEqual(secret.get_content(), {'password': 'hunter6'})

def test_get_secret_and_refresh(self):
harness = ops.testing.Harness(ops.CharmBase, meta='name: webapp')
self.addCleanup(harness.cleanup)
harness.begin()
harness.set_leader(True)
secret = harness.charm.app.add_secret({'password': 'hunter6'})
secret.set_content({"password": "hunter7"})
retrieved_secret = harness.model.get_secret(id=secret.id)
self.assertEqual(retrieved_secret.id, secret.id)
self.assertEqual(retrieved_secret.get_content(), {'password': 'hunter6'})
self.assertEqual(retrieved_secret.peek_content(), {'password': 'hunter7'})
self.assertEqual(retrieved_secret.get_content(refresh=True), {'password': 'hunter7'})
self.assertEqual(retrieved_secret.get_content(), {'password': 'hunter7'})

def test_get_secret_removed(self):
harness = ops.testing.Harness(ops.CharmBase, meta='name: webapp')
self.addCleanup(harness.cleanup)
harness.begin()
harness.set_leader(True)
secret = harness.charm.app.add_secret({'password': 'hunter8'})
secret.set_content({"password": "hunter9"})
secret.remove_revision(secret.get_info().revision)
with self.assertRaises(ops.SecretNotFoundError):
harness.model.get_secret(id=secret.id)

def test_get_secret_by_label(self):
harness = ops.testing.Harness(ops.CharmBase, meta='name: webapp')
self.addCleanup(harness.cleanup)
harness.begin()
secret_id = harness.charm.app.add_secret({'password': 'hunter9'}, label="my-pass").id
secret = harness.model.get_secret(label="my-pass")
self.assertEqual(secret.label, "my-pass")
self.assertEqual(secret.get_content(), {'password': 'hunter9'})
secret = harness.model.get_secret(id=secret_id, label="other-name")
self.assertEqual(secret.get_content(), {'password': 'hunter9'})
secret = harness.model.get_secret(label="other-name")
self.assertEqual(secret.get_content(), {'password': 'hunter9'})

def test_add_model_secret_invalid_content(self):
harness = ops.testing.Harness(ops.CharmBase, meta='name: webapp')
self.addCleanup(harness.cleanup)
Expand Down

0 comments on commit a7f34cb

Please sign in to comment.