feat: expand the secret ID out to the full URI when only given the ID #1358
Conversation
| self.id = Secret._canonicalize_id(id) | ||
| if model_uuid is None: | ||
| warnings.warn( | ||
| '`model_uuid` should always be provided when creating a ' |
There was a problem hiding this comment.
In what cases can this happen? Only if people manually create a SecretInfo object? (Which they shouldn't be doing.)
There was a problem hiding this comment.
In what cases can this happen? Only if people manually create a
Secretobject? (Which they shouldn't be doing.)
Manually create a SecretInfo, yes. And yes, I don't know why they would, although we don't tell them not to (as we do with Secret, for example). This is probably overly cautious - I can't find any charm doing this (Scenario does, but we can fix that in advance of a release), but seemed better than just having None work and then in some future version (3.0?) suddenly not work.
Good question. I don't see it breaking anyone's charms, since it's already the case that the
Good call: 112 charms' tests passed with main, 110 with this branch. One of the failures was And this is a legitimate bug 😞. Harness isn't properly getting the secret because it can't handle the different types of URI like Juju can. I'll fix that and re-run.
Sorry, I always forget to mention I've done that. Juju 3.6b2, LXD (I don't think either of those should matter). With main: With this branch: |
…y forms can be used, like with Juju.
They pass now, so this doesn't break the unit tests of at least those 112 charms, which makes me fairly confident it won't break for anyone. |
Thanks! Guess which PR I worked on immediately prior to this 😆. |
Adjust the rules for getting the 'canonical' form of the Secret URI:
secret:, leave it as-is (unchanged)secret:prefix to end up with the formsecret:id(unchanged)secret://{model UUID}/{secret IDform (new)Also adjust the code that calls the canonicalization [sic] method so that the model UUID is always available in regular ops use. If someone is creating
SecretInfoobjects themselves then aNonevalue for the UUID is the default, for backwards compatibility (but a warning is issued in that case).Fixes #1312.