Harness not modelling permission denied for user secrets

[DnPlas]

#1176 introduced support for user secrets in Harness, but it looks like it is not modelling the actual Juju behaviour when a user secret hasn't been granted to a charm and it tries to get a secret with self.model.get_secret(id=<id-from-config>).

On an actual model, if the charm config gets updated with a secret ID, but the charm hasn't been granted access to the secret, ops.model.ModelError: ERROR permission denied will be raised, but it seems that when testing, harness makes the charm to raise a different exception ops.model.SecretNotFoundError: Secret 'secret:16320250-1b76-4552-94d9-10ff484f88cb' not granted access to 'istio-pilot when calling the exact same piece of code.

Steps to reproduce

1. Deploy any charm that can use user secrets
2. Create a user secret
3. Pass the secret ID through charm configuration juju config <charm-name> my-secret=secret:some-id
4. Observe the charm state and logs. You should get something like this:

  File "/var/lib/juju/agents/unit-istio-pilot-0/charm/venv/ops/model.py", line 3397, in secret_get
    result = self._run('secret-get', *args, return_output=True, use_json=True)
  File "/var/lib/juju/agents/unit-istio-pilot-0/charm/venv/ops/model.py", line 3051, in _run
    raise ModelError(e.stderr) from e
ops.model.ModelError: ERROR permission denied

5. Add a unit test for testing what happens when the secret hasn't been granted to the charm. For example:

def test_permission_denied(...):
  secret_content = {"tls-crt": "a-tls-crt", "tls-key": "a-tls-key"}
  secret_id = harness.add_user_secret(secret_content)
  harness.update_config({"tls-secret-id": secret_id})
  harness.begin()
  with pytest.raises(ModelError) as err:
    harness.charm.function_call_to_get_secret() # <--- this is a function that calls self.model.get_secret(id=<id from config>)

6. The above will not raise because it is expecting a ModelError but a SecretNotFoundError exception is raised instead:

 ops.model.SecretNotFoundError: Secret 'secret:16320250-1b76-4552-94d9-10ff484f88cb' not granted access to 'istio-pilot

[DnPlas]

Extra information:

According to get_secret docs, it only raises SecretNotFoundError if the secret does not exist, but _ModelBackend.secret_get, which is called by the model.get_secret() method, will raise a ModelError under other conditions, which can include the permission denied scenario that I am describing.

[benhoyt]

@IronCore864 @tonyandrewmeyer It makes sense to me to make the testing exception the same as the real one. So I think we just change the SecretNotFoundError here to ModelError. Sound reasonable?

[tonyandrewmeyer]

@IronCore864 @tonyandrewmeyer It makes sense to me to make the testing exception the same as the real one.

I do agree with this in general.

So I think we just change the SecretNotFoundError here to ModelError. Sound reasonable?

However, I don't think this is the right fix here. I think this is a Juju bug. All of the other permission errors (like trying to do a get_info() when only having view permission, or trying to get a charm secret that you haven't been granted permission to) raise SecretNotFoundError). Apart from the inconsistency, behaving differently when there is a secret with no access and when there is no secret feels leaky to me.

So, in my opinion, we should open a Juju bug instead.

[tonyandrewmeyer]

So, in my opinion, we should open a Juju bug instead.

Checked with the Juju team and they agree that it should be consistent in Juju, so I opened a Juju bug.

[tonyandrewmeyer]

According to get_secret docs, it only raises SecretNotFoundError if the secret does not exist

Opened #1231 to make this explicit in the docs.

[tonyandrewmeyer]

Actually, I got this wrong. Charm secrets do return "permission denied" as well as user ones, so there must be an ops issue here.

[tonyandrewmeyer]

Juju 3.4.2, Microk8s, I get this behaviour:

hook tool	issue	user secret	charm secret
secret-get	no permission	"permission denied"	"permission denied"
secret-info-get	no permission	"not found"	"not found"
secret-set	no permission	delayed permission denied	delayed permission denied
secret-remove	no permission	delayed permission denied	delayed permission denied
secret-grant	no permission	"not found"	"not found"
secret-revoke	no permission	delayed permission denied	delayed permission denied
secret-get	doesn't exist	"not found"	"not found"
secret-info-get	doesn't exist	"not found"	"not found"
secret-set	doesn't exist	delayed not found	delayed not found
secret-remove	doesn't exist	delayed not found	delayed not found
secret-grant	doesn't exist	"not found"	"not found"
secret-revoke	doesn't exist	delayed not found	delayed not found

For the "delayed" cases, the hook tool returns successfully, and then when the hook finishes (after ops is done) the charm goes into error state with something like this in the debug-log:

unit-dummycharm-1: 12:56:16 ERROR juju.worker.uniter.context cannot apply changes: removing secrets: permission denied
unit-dummycharm-1: 12:56:16 ERROR juju.worker.uniter.operation hook "config-changed" (via hook dispatching script: dispatch) failed: removing secrets: permission denied

Also discussing this with @barrettj12 who is working on the Juju ticket.