Merge pull request #82 from canonical/fix-secret-id-canonical

fix secret id canonicalization

pyproject.toml

@@ -8,7 +8,7 @@ build-backend = "setuptools.build_meta"
 [project]
 name = "ops-scenario"
 
-version = "5.6.1"
+version = "5.6.2"
 
 authors = [
     { name = "Pietro Pasotti", email = "pietro.pasotti@canonical.com" }

scenario/mocking.py

@@ -9,9 +9,9 @@
 from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Set, Tuple, Union
 
 from ops import JujuVersion, pebble
+from ops.model import ModelError, RelationNotFoundError
+from ops.model import Secret as Secret_Ops  # lol
 from ops.model import (
-    ModelError,
-    RelationNotFoundError,
     SecretInfo,
     SecretNotFoundError,
     SecretRotate,
@@ -138,13 +138,19 @@ def _get_relation_by_id(
             raise RelationNotFoundError()
 
     def _get_secret(self, id=None, label=None):
-        # cleanup id:
-        if id and id.startswith("secret:"):
-            id = id[7:]
+        canonicalize_id = Secret_Ops._canonicalize_id
 
         if id:
+            # in scenario, you can create Secret(id="foo"),
+            # but ops.Secret will prepend a "secret:" prefix to that ID.
+            # we allow getting secret by either version.
             try:
-                return next(filter(lambda s: s.id == id, self._state.secrets))
+                return next(
+                    filter(
+                        lambda s: canonicalize_id(s.id) == canonicalize_id(id),
+                        self._state.secrets,
+                    ),
+                )
             except StopIteration:
                 raise SecretNotFoundError()
         elif label:

scenario/state.py

@@ -126,6 +126,10 @@ def copy(self) -> "Self":
 @dataclasses.dataclass(frozen=True)
 class Secret(_DCBase):
     id: str
+    # CAUTION: ops-created Secrets (via .add_secret()) will have a canonicalized
+    #  secret id (`secret:` prefix)
+    #  but user-created ones will not. Using post-init to patch it in feels bad, but requiring the user to
+    #  add the prefix manually every time seems painful as well.
 
     # mapping from revision IDs to each revision's contents
     contents: Dict[int, "RawSecretRevisionContents"]

tests/test_e2e/test_secrets.py

@@ -5,6 +5,7 @@
 from ops.framework import Framework
 from ops.model import SecretNotFoundError, SecretRotate
 
+from scenario import Context
 from scenario.state import Relation, Secret, State
 from tests.helpers import trigger
 
@@ -278,3 +279,21 @@ def post_event(charm: CharmBase):
         meta={"name": "local", "requires": {"foo": {"interface": "bar"}}},
         post_event=post_event,
     )
+
+
+class GrantingCharm(CharmBase):
+    def __init__(self, *args):
+        super().__init__(*args)
+        self.framework.observe(self.on.start, self._on_start)
+
+    def _on_start(self, _):
+        secret = self.app.add_secret({"foo": "bar"})
+        secret.grant(self.model.relations["bar"][0])
+
+
+def test_grant_after_add():
+    context = Context(
+        GrantingCharm, meta={"name": "foo", "provides": {"bar": {"interface": "bar"}}}
+    )
+    state = State(relations=[Relation("bar")])
+    context.run("start", state)