Repository: canonical/operator

Issue:
secret id is not always canonicalized before calling secret_get
Secret canonicalizes the ID it receives upon initialization.
However, before the Secret object is initialized, the ID is used to call secret_get. This results in the ID being sometimes canonicalized, sometimes not, depending on the caller.

For example:
```
self.model.get_secret(id="foo").get_content()
# resulting call: secret-get --id foo

self.model.get_secret(id="foo").peek_content()
# resulting call: secret-get --id secret:foo --peek
```

This is not a problem per se, but it might mean headache in the future. 

Discussion:

Patch:
diff --git a/ops/model.py b/ops/model.py
index 141bd4f..2ab4ff8 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -276,6 +276,9 @@ class Model:
         """
         if not (id or label):
             raise TypeError('Must provide an id or label, or both')
+        if id is not None:
+            # Canonicalize to "secret:<id>" form for consistency in backend calls.
+            id = Secret._canonicalize_id(id)
         try:
             content = self._backend.secret_get(id=id, label=label)
             return Secret(self._backend, id=id, label=label, content=content)


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index 976fb77..35df7ea 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2841,7 +2841,18 @@ class TestSecrets(unittest.TestCase):
         self.assertEqual(secret.get_content(), {'foo': 'g'})
 
         self.assertEqual(fake_script_calls(self, clear=True),
-                         [['secret-get', '123', '--format=json']])
+                         [['secret-get', 'secret:123', '--format=json']])
+
+    def test_get_secret_label(self):
+        fake_script(self, 'secret-get', """echo '{"foo": "g"}'""")
+
+        secret = self.model.get_secret(label='lbl')
+        self.assertIsNone(secret.id)
+        self.assertEqual(secret.label, 'lbl')
+        self.assertEqual(secret.get_content(), {'foo': 'g'})
+
+        self.assertEqual(fake_script_calls(self, clear=True),
+                         [['secret-get', '--label', 'lbl', '--format=json']])
 
     def test_get_secret_id_and_label(self):
         fake_script(self, 'secret-get', """echo '{"foo": "h"}'""")
@@ -2852,7 +2863,7 @@ class TestSecrets(unittest.TestCase):
         self.assertEqual(secret.get_content(), {'foo': 'h'})
 
         self.assertEqual(fake_script_calls(self, clear=True),
-                         [['secret-get', '123', '--label', 'l', '--format=json']])
+                         [['secret-get', 'secret:123', '--label', 'l', '--format=json']])
 
     def test_get_secret_no_args(self):
         with self.assertRaises(TypeError):