feat(Harness): support user secrets

[IronCore864]

Add support for user secrets in Harness. Changes:

• Add a new API Harness.add_user_secret which simulates juju add-secret.
• add_model_secret is deprecated, use add_charm_secret. For now, add_model_secret is an alias to add_charm_secret.

Use case example:

config.yaml:

options:
  mysec:
    type: secret
    description: "tell me your secrets"

charm.py:

class MyVMCharm(ops.CharmBase):
    def __init__(self, framework: ops.Framework):
        super().__init__(framework)
        framework.observe(self.on.config_changed, self._on_config_changed)
    def _on_config_changed(self, event: ops.StartEvent):
        mysec = self.config.get('mysec')
        if mysec:
            sec = self.model.get_secret(id=mysec, label="mysec")
            self.config_from_secret = sec.get_content()

test_charm.py:

def test_config_changed(harness):
    secret_content = {'password': 'foo'}
    secret_id = harness.add_user_secret(secret_content)
    # grant the user secret to the app
    harness.grant_secret(secret_id, 'webapp')
    harness.begin()
    harness.update_config({'mysec': secret_id})
    secret = harness.model.get_secret(id=secret_id).get_content()
    assert harness.charm.config_from_secret == secret.get_content()

[tonyandrewmeyer]

Looking good - a few little nits and a couple of queries.

Other questions/comments:

• The trigger_secret_expiration method should probably raise an error if it's used with a user secret. Is that already the case via permissions checking?
• Likewise for trigger_secret_rotation.
• It doesn't seem like we are testing the permission code changes. I think we'd want to have tests that verify that those work as expected.

[benhoyt]

I agree with Tony's comments. I'll review further later. For now, can you please add a PR description with a brief description of the new API and with a usage example?

[IronCore864]

Looking good - a few little nits and a couple of queries.

Other questions/comments:

• The trigger_secret_expiration method should probably raise an error if it's used with a user secret. Is that already the case via permissions checking?
• Likewise for trigger_secret_rotation.
• It doesn't seem like we are testing the permission code changes. I think we'd want to have tests that verify that those work as expected.

Changed trigger_secret_expiration and trigger_secret_rotation to raise an error on user secret, also added two unit tests.

For user secret permissions, there are some unit tests.

[benhoyt]

This looks good to me -- thanks. Just a few minor comments.

[tonyandrewmeyer]

Can we adjust _ensure_secret_owner so that it explicitly talks about user secrets? It could have a user_secret = secret.owner_name == self.model.uuid and an and not user_secret, although that isn't strictly necessary, so maybe just a comment explaining the user secret situation, with the comment that explains unit and app permissions?

I would also feel more comfortable if we had tests that explicitly made sure that user secrets fail ownership checks. This will prevent regressions in the future if we change how the permissions checks are done.

[IronCore864]

Can we adjust _ensure_secret_owner so that it explicitly talks about user secrets? It could have a user_secret = secret.owner_name == self.model.uuid and an and not user_secret, although that isn't strictly necessary, so maybe just a comment explaining the user secret situation, with the comment that explains unit and app permissions?

I would also feel more comfortable if we had tests that explicitly made sure that user secrets fail ownership checks. This will prevent regressions in the future if we change how the permissions checks are done.

Yes it makes sense. I have added a test to try to do Secret.get_info to validate the ownership.

[IronCore864]

Status update: John has started a discussion in the juju development channel, now the preferred names are "application secrets" and "charm secrets", it seems it's not decided yet, I'll wait for a final conclusion before merging this one.

[tonyandrewmeyer]

LGTM!