From a7f34cb1cf628e2e635c3b7f28143e883e4642b9 Mon Sep 17 00:00:00 2001
From: Tony Meyer <tony.meyer@canonical.com>
Date: Fri, 24 Nov 2023 11:57:57 +1300
Subject: [PATCH] feat: secret owners do not auto-peek, and can use refresh
 (#1067)

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
---
 ops/model.py         |  3 +--
 ops/testing.py       |  7 +-----
 test/test_testing.py | 53 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/ops/model.py b/ops/model.py
index 33f7f12cc..e372dd6d1 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -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(
diff --git a/ops/testing.py b/ops/testing.py
index fe0f71ffd..ab288b7e1 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -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]
diff --git a/test/test_testing.py b/test/test_testing.py
index 3ebbd4c91..9dd2473a8 100644
--- a/test/test_testing.py
+++ b/test/test_testing.py
@@ -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)