Repository: canonical/operator

Issue:
secret.get_content() un-intuitively cached locally
"Screenshot" from a debugger session.
As you can see, I'm fetching contents of a secret, saving it to `dict` called `full_content`.
Then I update `full_content`. NOT the secret, but the dictionary holding the output of the `get_content()` call.
Yet, when I call `secret.get_content()` afterwards, I find that it's returning the UPDATED content of the `full_content` dictionary, instead of the real contents of the secret. AGAIN, note that the secret has NOT been updated at this point.

However, as a result, the secret will actually never be updated. The update command silently fails on line 630, and neither the contents, nor the revision of the secret gets updated.

[The value of the `content` variable was `{'endpoints': 'host1:port,host2:port,host3:port'}` from the start]

```
625             import pdb; pdb.set_trace()
626  ->         full_content = secret.get_content()
627             old_content = copy.deepcopy(full_content)
628             full_content.update(content)
629             if old_content != full_content:
630                 secret.set_content(full_content)
631  
(Pdb) n
> /var/lib/juju/agents/unit-kafka-0/charm/lib/charms/data_platform_libs/v0/data_interfaces.py(627)update_relation_secret()
-> old_content = copy.deepcopy(full_content)
(Pdb) full_content
{'endpoints': 'host1:port,host2:port'}
(Pdb) n
> /var/lib/juju/agents/unit-kafka-0/charm/lib/charms/data_platform_libs/v0/data_interfaces.py(628)update_relation_secret()
-> full_content.update(content)
(Pdb) n
> /var/lib/juju/agents/unit-kafka-0/charm/lib/charms/data_platform_libs/v0/data_interfaces.py(629)update_relation_secret()
-> if old_content != full_content:
(Pdb) secret.get_content()
{'endpoints': 'host1:port,host2:port,host3:port'}
```

This behavior is highly unintuitive, and should be addressed.


Discussion:

Patch:
diff --git a/ops/model.py b/ops/model.py
index 24f40f5..906b463 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -1181,6 +1181,9 @@ class Secret:
     def get_content(self, *, refresh: bool = False) -> Dict[str, str]:
         """Get the secret's content.
 
+        Returns:
+            A copy of the secret's content dictionary.
+
         Args:
             refresh: If true, fetch the latest revision's content and tell
                 Juju to update to tracking that revision. The default is to
@@ -1190,7 +1193,7 @@ class Secret:
         if refresh or self._content is None:
             self._content = self._backend.secret_get(
                 id=self.id, label=self.label, refresh=refresh)
-        return self._content
+        return self._content.copy()
 
     def peek_content(self) -> Dict[str, str]:
         """Get the content of the latest revision of this secret.


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index b310e0f..d74dd17 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -3104,6 +3104,18 @@ class TestSecretClass(unittest.TestCase):
         self.assertEqual(fake_script_calls(self, clear=True),
                          [['secret-get', 'secret:z', '--format=json']])
 
+    def test_get_content_copies_dict(self):
+        fake_script(self, 'secret-get', """echo '{"foo": "bar"}'""")
+
+        secret = self.make_secret(id='z')
+        content = secret.get_content()
+        self.assertEqual(content, {'foo': 'bar'})
+        content['new'] = 'value'
+        self.assertEqual(secret.get_content(), {'foo': 'bar'})
+
+        self.assertEqual(fake_script_calls(self, clear=True),
+                         [['secret-get', 'secret:z', '--format=json']])
+
     def test_peek_content(self):
         fake_script(self, 'secret-get', """echo '{"foo": "peeked"}'""")