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

Improve SecretsManager API for easier configuration #9485

Open
adutra opened this issue Sep 9, 2024 · 4 comments
Open

Improve SecretsManager API for easier configuration #9485

adutra opened this issue Sep 9, 2024 · 4 comments

Comments

@adutra
Copy link
Contributor

adutra commented Sep 9, 2024

Issue description

I wonder if it's possible to change the SecretsManager API a bit to make the secret configuration experience more user-friendly.

Here's an example of env vars for an ADLS account name + secret:

- name: "nessie.catalog.service.adls.file-systems.filesystem1.account"
  value: "urn:nessie-secret:quarkus:nessie-catalog-secrets.adls.file-systems.filesystem1.account"
  
- name: "nessie-catalog-secrets.adls.file-systems.filesystem1.account.name"
  valueFrom:
    secretKeyRef:
      name: "adls-account-secret"
      key: "accountName"

- name: "nessie-catalog-secrets.adls.file-systems.filesystem1.account.secret"
  valueFrom:
    secretKeyRef:
      name: "adls-account-secret"
      key: "accountKeyRef"

That's verbose, but I'm OK with that. But when we add caching + secret reloading on top of that (see #8707), I guess we'd need to get rid of mounting secrets that way, and instead we'd get something like this:

- name: "nessie.catalog.service.adls.file-systems.filesystem1.account"
  value: "urn:nessie-secret:k8s:adls-account-secret?name=accountName&secret=accountKeyRef"

While this is way more concise, it's also a bit more opaque to the user, since 2 options were grouped together. In fact the grouping is rather for technical reasons: it's done merely to facilitate secret reloading and avoid broken rotated credentials.

If instead we can do the reloading at SecretsManager level, maybe we can keep the user-facing env vars more intuitive, and still achieve caching + reloading:

- name: "nessie.catalog.service.adls.file-systems.filesystem1.account-name"
  value: "urn:nessie-secret:k8s:adls-account-secret:accountName"

- name: "nessie.catalog.service.adls.file-systems.filesystem1.account-key"
  value: "urn:nessie-secret:k8s:adls-account-secret:accountKeyRef"

In the above example, two keys come from the same secret adls-account-secret; the secret would be
fetched only once and cached. When the cache expires the secret is fetched again and all the keys
are updated together.

In this design each URI in the configuration points to exactly one value. The fact that some values might be declared in the same container (like a K8s Secret) becomes an implementation detail. This could also simplify the Secret hierarchy as the application doesn't need to know if a secret is a TokenSecret or a BasicCredentials etc.

@dimas-b
Copy link
Member

dimas-b commented Sep 9, 2024

This proposal SGMT overall.

Maybe nitpicking, but the URN rfc8141 provides means to define components specific to the URN resolution process and to the (resolved) resource. WDYT about a URN like the following?

urn:nessie-secret:adls-account-secret?+provider=k8s?=name=accountName&secret=accountKeyRef

@adutra
Copy link
Contributor Author

adutra commented Sep 9, 2024

WDYT about a URN like the following?

urn:nessie-secret:adls-account-secret?+provider=k8s?=name=accountName&secret=accountKeyRef

Indeed we could use an r-component for the provider – but going down that route, I'd use an f-component for the secret key:

urn:nessie-secret:adls-account-secret?+provider=k8s#accountKeyRef

The only thing that annoys me is that the original syntax is easier to understand for users, i.e. urn:nessie-secret:<provider>:<name> is nicer than urn:nessie-secret:<name>?+provider=<provider>.

But I like the r-component the idea :-)

@dimas-b
Copy link
Member

dimas-b commented Sep 9, 2024

How about delegating resolution to runtime? That is URN: urn:nessie-secret:<name>. The list of providers (perhaps with priority order) is configured separately.

In this case ?=provider=k8s becomes optional, but could be used to disambiguate lookup.

Things like accountKeyRef could also have default values (perhaps provider-specifc), with optional overrides in the URN 🤔

@snazy
Copy link
Member

snazy commented Sep 9, 2024

Most setups will have one - maybe two (Quarkus config + one secrets manager) sources. So too much flexibility doesn't add a lot of value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants