Cannot call Secret.set_info and Secret.set_content in the same hook

[tonyandrewmeyer]

If Secret.set_info() and Secret.set_content() are both called in the same hook, then whichever is first is ignored.

I expect that from a Juju point of view, this is because secret_set has been called twice and so it assumes that the later call replaces the earlier one, rather than trying to merge the two together. This is particularly unexpected behaviour in ops because we have two different methods exposing parts of secret_set, so a charmer doesn't necessarily know that it's the same underlying Juju hook tool.

This can be reproduced with this small charm:

class SecretInfoAndContentSetCharm(ops.CharmBase):
    """Charm the application."""

    def __init__(self, framework: ops.Framework):
        super().__init__(framework)
        framework.observe(self.on.start, self._on_start)
        framework.observe(self.on.set_action, self._on_set)

    def _on_start(self, event: ops.StartEvent):
        self.unit.add_secret(label="my-secret", content={"foo": "bar"})
        self.unit.status = ops.ActiveStatus()

    def _on_set(self, event):
        secret = self.model.get_secret(label="my-secret")
        secret.set_content(event.params)
        secret.set_info(description="hello world")

If you call the action (juju run unit-name/0 set one=two) with the code like it is above, then the description will be set, but the content will not. If you swap the code so that the set_content and set_info calls are around the other way, then the action will set the content but not the description.

[tonyandrewmeyer]

This seems non-trivial to fix - ops would need to keep the data passed to both set_content and set_info in a cache (merging as each was called) and then do the secret_set call only on conclusion of the hook (note that it's not just the current event, as the calls might be in the Juju event but also in a deferred or ops Lifecycle event).

At minimum, we should document this restriction so that it's easier to figure out what's happening.

[benhoyt]

Thanks for the report. It does seem odd to me that the second secret-set call (without content args) wipes out the content, so maybe this is a Juju bug. I'll put it on the list to discuss with Ian next Monday.

[benhoyt]

We discussed this in daily sync today, and decided this is probably is worth fixing. You'd be likely to call set_info when a secret expires, and you'd want to call set_content then too. So this could really happen in practice.

Our idea is to keep a cache on the backend (dict of secret id to something), and whenever set_info or set_content is called, it updates the info, merging info and content. But setting content a second time would replace.

[tonyandrewmeyer]

Interestingly, the tls-certificates-interface lib does have a set_content() followed immediately by set_info() in relation-changed. It seems like that can't work at the moment - there is another path where the secret doesn't already exist and a new one is created - maybe that is the one that always gets exercised?

It would be good if whoever works on this ticket takes a closer look at that, before and after the change.

[saltiyazan]

Interestingly, the tls-certificates-interface lib does have a set_content() followed immediately by set_info() in relation-changed. It seems like that can't work at the moment - there is another path where the secret doesn't already exist and a new one is created - maybe that is the one that always gets exercised?

It would be good if whoever works on this ticket takes a closer look at that, before and after the change.

I looked into this, and it appears to be dead code that will no longer be executed due to some changes introduced to the library. I'll investigate this further to confirm.

[saltiyazan]

I came across this issue and I was about to open a bug, happy that you are aware if it.
Do you suggest a specific workaround until a fix is merged?

[tonyandrewmeyer]

I came across this issue and I was about to open a bug, happy that you are aware if it.

Thanks for letting us know - I'll see if we can get a fix in for the next release.

Do you suggest a specific workaround until a fix is merged?

Unfortunately, I think the only workaround would be to call the hook tool yourself - basically, add a method to your charm that does what set_info does but can also set the content, using the private attributes to get the model backend, and use that.