Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support user secrets: "secret" config type and Harness support #1166

Closed
jameinel opened this issue Mar 26, 2024 · 12 comments
Closed

Support user secrets: "secret" config type and Harness support #1166

jameinel opened this issue Mar 26, 2024 · 12 comments
Assignees
Labels
24.04 bug Something isn't working

Comments

@jameinel
Copy link
Member

jameinel commented Mar 26, 2024

To handle user secrets, with Juju (3.4(?) you can now have a config with a schema type of "secret". Juju will then ensure that it can hold a secret URL.

However, if I try to create a charm that uses this, when running the test suite I get:

Traceback (most recent call last):
  File "/home/jameinel/dev/charms/ubuntu-lite/test/test_charm.py", line 22, in test_on_start
    harness = testing.Harness(charm.Ubuntu)
  File "/home/jameinel/dev/charms/ubuntu-lite/venv/lib/python3.10/site-packages/ops/testing.py", line 251, in __init__
    self._backend = _TestingModelBackend(self._unit_name, self._meta, config_)
  File "/home/jameinel/dev/charms/ubuntu-lite/venv/lib/python3.10/site-packages/ops/testing.py", line 2079, in __init__
    self._config = _TestingConfig(config)
  File "/home/jameinel/dev/charms/ubuntu-lite/venv/lib/python3.10/site-packages/ops/testing.py", line 1966, in __init__
    self._config_set(key, value)
  File "/home/jameinel/dev/charms/ubuntu-lite/venv/lib/python3.10/site-packages/ops/testing.py", line 1996, in _config_set
    raise RuntimeError(
RuntimeError: Incorrectly formatted `options.yaml`: `type` needs to be one of [string, boolean, int, float], not secret.

I tested this with:

Installing collected packages: websocket-client, ops
  Attempting uninstall: ops
    Found existing installation: ops 1.5.5
    Uninstalling ops-1.5.5:
      Successfully uninstalled ops-1.5.5
Successfully installed ops-2.11.0 websocket-client-1.7.0
@jameinel
Copy link
Member Author

So to trigger this behavior, I updated my charm's config to have:

+++ b/config.yaml
@@ -1 +1,5 @@
-options: {}
+options:
+  mysec:
+    type: secret
+    default: nil
+    description: "tell me your secrets"

It turns out that it only fails because I set the 'default:' key, which makes the Testing infrastructure call _config_set with the defaults, and then it validates the type.
However, in practice, you shouldn't set the config, but you will need to read it.

@jameinel
Copy link
Member Author

and certainly the test suite will need to be able to set it :)

@tonyandrewmeyer
Copy link
Contributor

This seems to be undocumented functionality (config.yaml, charmcraft.yaml, manage application, how to use secrets, release notes, secret reference). Is there documentation somewhere else?

@jameinel
Copy link
Member Author

jameinel commented Mar 26, 2024 via email

@tonyandrewmeyer
Copy link
Contributor

tonyandrewmeyer commented Mar 28, 2024

We're going to get John's PR into ops 2.12 as an immediate unblocker, but we feel that there's more work to be done here.

It's likely that we need a new Harness method (add_user_secret) and the permissions checking probably needs to be updated as well. There might also be other adjustments that we haven't figured out yet as well.

We're expecting that we'll be able to do the more complete fix in 22.04.

Also: the Juju team are working on updating the documentation. We should double-check that at least the charmcraft.yaml and config.yaml pages are updated, since those can be done trivially.

@IronCore864
Copy link
Contributor

I see the Jira ticket for this issue is assigned to me, so today I did some research today and tried juju add-secret.

It seems user secrets added by this command belong to the Model, as shown by juju secrets output as well as confirmed by this code here. It's not the same as secrets added by the secret-add hook tools, which belong to either the Applicaiton or a Unit. Did I get it right?

And it seems Harness can add secrets with owner set as the app or unit. I suppose we can add a method Harness.add_user_secret and set the owner as self.model.uuid, but we need to change a bunch of other methods like grant_secret, _ensure_secret_owner, secret_set, etc.

Did I get the whole picture?

Also it would be nice to merge Tony's charmcraft PR so that we can pack and test it.

@benhoyt
Copy link
Collaborator

benhoyt commented Apr 1, 2024

Yeah, that sounds right, but maybe we can briefly go over this with Ian at the Juju standup today.

Thinking about it now, Harness.add_model_secret is not a great name, because it sounds like the secret is tied to the model (when it's actually app or unit). But yeah, adding add_user_secret is probably the way to go.

@benhoyt
Copy link
Collaborator

benhoyt commented Apr 1, 2024

Per Juju recommendation, let's also consider renaming Harness.add_model_secret to add_charm_secret (with an alias for backwards-compatibility) and call the user-secret one add_user_secret. This fits the Juju team / docs terminology of "charm secret" vs "user secret".

Presumably add_user_secret would add the effective emulated grants automatically.

@benhoyt benhoyt changed the title config can take a type secret Support user secrets: "secret" config type and Harness support Apr 3, 2024
@DnPlas
Copy link

DnPlas commented Apr 4, 2024

Hi @benhoyt regarding tests, what would be the recommended way of testing user secrets (adding, granting access, etc.) in an integration test environment? I see that we have add_secret, but that models application secrets, AFAICT, so in charm world I don't think we could do something like ops_test.model.add_secret (when we are using the pytest-operator).

@tonyandrewmeyer
Copy link
Contributor

tonyandrewmeyer commented Apr 4, 2024

what would be the recommended way of testing user secrets (adding, granting access, etc.) in an integration test environment? I see that we have add_secret, but that models application secrets, AFAICT,

ops.testing (Harness) is for unit tests, not integration tests. At the moment, writing unit tests for user secrets with ops.testing is not well supported, but when this ticket is closed (probably next pulse) that will be fixed.

For integration tests, you should use pytest-operator and python-libjuju (which have the model.add_secret method that maps to juju add-secret as you said).

@DnPlas
Copy link

DnPlas commented Apr 4, 2024

what would be the recommended way of testing user secrets (adding, granting access, etc.) in an integration test environment? I see that we have add_secret, but that models application secrets, AFAICT,

ops.testing (Harness) is for unit tests, not integration tests. At the moment, writing unit tests for user secrets with ops.testing is not well supported, but when this ticket is closed (probably next pulse) that will be fixed.

For integration tests, you should use pytest-operator and python-libjuju (which have the model.add_secret method that maps to juju add-secret as you said).

Thanks for clearing this out, I didn't know python-libjuju already had this.

@benhoyt
Copy link
Collaborator

benhoyt commented May 17, 2024

This is now done with #1167 and #1176

@benhoyt benhoyt closed this as completed May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
24.04 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants