Skip to content

Commit

Permalink
Add Secret.unique_identifier (just the XID part)
Browse files Browse the repository at this point in the history
We want to provide a way to allow charms to test two secrets for
equality / see if they refer to the same secret object.

It would be nice if Secret.id was a unique identifier, but it's really
a locator URI. Too late to change that, though.

We considered deprecating Secret.id entirely, and adding two new
properties, "uri" and "unique_identifier". However, given that this
locator is called ID throughout the rest of the codebase and by Juju,
I think that would introduce more confusion.

The name "unique_identifier" is long on purpose: it should be
relatively rare to use it, and we want to nudge people to using the
locator (.id) and labels instead.

See context at https://bugs.launchpad.net/juju/+bug/2028402
  • Loading branch information
benhoyt committed Aug 3, 2023
1 parent ca04872 commit 4f8fc88
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 1 deletion.
29 changes: 28 additions & 1 deletion ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1094,13 +1094,40 @@ def _validate_content(cls, content: Optional[Dict[str, str]]):

@property
def id(self) -> Optional[str]:
"""Unique identifier for this secret.
"""Locator ID (URI) for this secret.
This has an unfortunate name for historical reasons, as it's not
really a unique identifier, but the secret's locator URI, which may or
may not include the model UUID (for cross-model secrets).
Charms should treat this as an opaque string for looking up secrets
and sharing them via relation data. If you want a truly unique
identifier for checking equality, use :attr:`unique_identifier`.
This will be None if you obtained the secret using
:meth:`Model.get_secret` with a label but no ID.
"""
return self._id

@property
def unique_identifier(self) -> Optional[str]:
"""Unique identifier of this secret.
This is the secret's globally-unique identifier (currently a
20-character Xid, for example "9m4e2mr0ui3e8a215n4g"). It is useful
for determining if two secrets refer to the same secret object. Most
charms should use :attr:`id` (the secret's locator ID) instead.
This will be None if you obtained the secret using
:meth:`Model.get_secret` with a label but no ID.
"""
if self._id is None:
return None
if '/' in self._id:
return self._id.rsplit('/', 1)[-1]
else:
return self._id.removeprefix('secret:')

@property
def label(self) -> Optional[str]:
"""Label used to reference this secret locally.
Expand Down
31 changes: 31 additions & 0 deletions test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -2973,6 +2973,37 @@ def test_get_secret_other_error(self):
self.model.get_secret(id='123')
self.assertNotIsInstance(cm.exception, ops.SecretNotFoundError)

def test_secret_unique_identifier(self):
fake_script(self, 'secret-get', """echo '{"foo": "g"}'""")

secret = self.model.get_secret(label='lbl')
self.assertIsNone(secret.id)
self.assertIsNone(secret.unique_identifier)

secret = self.model.get_secret(id='123')
self.assertEqual(secret.id, 'secret:123')
self.assertEqual(secret.unique_identifier, '123')

secret = self.model.get_secret(id='secret:124')
self.assertEqual(secret.id, 'secret:124')
self.assertEqual(secret.unique_identifier, '124')

secret = self.model.get_secret(id='secret://125')
self.assertEqual(secret.id, 'secret://125')
self.assertEqual(secret.unique_identifier, '125')

secret = self.model.get_secret(id='secret://modeluuid/126')
self.assertEqual(secret.id, 'secret://modeluuid/126')
self.assertEqual(secret.unique_identifier, '126')

self.assertEqual(fake_script_calls(self, clear=True), [
['secret-get', '--label', 'lbl', '--format=json'],
['secret-get', 'secret:123', '--format=json'],
['secret-get', 'secret:124', '--format=json'],
['secret-get', 'secret://125', '--format=json'],
['secret-get', 'secret://modeluuid/126', '--format=json'],
])


class TestSecretInfo(unittest.TestCase):
def test_init(self):
Expand Down

0 comments on commit 4f8fc88

Please sign in to comment.