remote: support source-referenced secrets in services.json (env:/file:) instead of inline-only values #33
Replies: 5 comments 1 reply
|
I think the main idea here is right — pointing each field at its own source ( 1. Allow both a short string and an object, not just the string prefixThe proposal puts the source inside the string ( If we use a small value type with a custom JSON parser, it can accept both styles, and the escape hatch is no longer needed: "client_secret": "env:NPQS_SECRET" // short form (your proposal)
"client_secret": { "env": "NPQS_SECRET" } // object form — no guessing
"client_secret": { "file": "/var/run/secrets/npqs/token" }
"value": "plain text still works" // plain string = literal (unchanged)
2. Read the value on every request, instead of caching it with a TTLThe proposal caches a resolved
A TTL cache only helps if you have very high traffic and the secret is on slow/network storage — which is not the case for ESO mounts. So reading every time is simpler, picks up new secrets faster, and is correct here. We can still keep your "fail loud" idea by checking all references once when the file loads (so a missing env var or unreadable file fails right away, not on the first request). How it could look in codeBoth changes come from one small shared type (say
This keeps all the parsing and reading logic in one place that is easy to test, keeps the list of sources ( |
|
Changing config values for containers is an antipattern AFAIK. If correct we should read at startup time. Off the value needs to be changed you need to restart the container. |
|
Thanks @sthanikan2000 — the per-field reference direction is solid, but I want to push back on both of the specific changes. Re: suggestion 1 — accept both a string and an object formI'd prefer to keep it to the prefixed-string form only.
So: prefixed string only, with Re: suggestion 2 — read on every requestI'd actually go the other way and resolve once at startup (and The key point is what these secrets actually are. Every field in scope — the API key That means there's no automated, mid-operation rotation to chase here. A change to one of these is a rare, deliberate, operator-driven event (policy or compromise). When that happens, restarting the pod is a perfectly acceptable way to pick up the new value — it's a planned action, not a hot path. Re-reading on every request adds machinery to handle a rotation pattern these secrets don't have. So: resolve at startup, validate loudly there, restart to rotate. Simpler, and it matches how the values actually behave. If we ever point a field at a genuinely Vault-rotated credential, we can revisit lazy re-reads then — but I don't want to design for it now. |
|
Agree with the TLDR — flat scheme-prefixed strings, resolved and validated once at startup, fail loud on a missing credential. One change: if we're shipping a single source scheme to start, make it |
|
Aligning with where this is landing — flat scheme-prefixed strings, resolved and validated once at startup, fail loud on a missing credential. Happy to drop both of my earlier suggestions:
On the one still-open question, +1 to |
Uh oh!
There was an error while loading. Please reload this page.
Problem
The
remotepackage loads service definitions fromservices.jsonviaManager.LoadServices(remote/manager.go). Today, authentication secrets are embedded as literal values inside that file —auth.options.value(API key),token(bearer), andclient_secret(OAuth2) are all plain strings.This forces an all-or-nothing posture: because the secrets live inline, the entire config file becomes sensitive. That has real downsides for our deployment model:
nsw-vault-backendClusterSecretStore, which generates native Kubernetes Secrets consumed throughsecretKeyRef/ mounted volumes. The current "whole file is the secret" model can't consume those native Secrets by reference — it only accepts literal values.In short: the config layer has no way to say "this field's value comes from somewhere else." Secrets and structure are fused.
Proposed Solution
Introduce a scheme-prefixed value reference for any secret-bearing field in
services.json. A field value may be a literal, or a reference whose prefix names its source:{ "auth": { "type": "oauth2", "options": { "token_url": "https://idp.example.gov/token", "client_id": "my-client", "client_secret": "env:NPQS_CLIENT_SECRET", "scopes": ["npqs:submit"] } } }{ "auth": { "type": "bearer", "options": { "token": "file:/var/run/secrets/npqs/token" } } }Supported sources initially:
env:NAME— read from environment variableNAME(populated by ESO-generated Secret viasecretKeyRef/envFrom).file:/path— read from a file (populated by an ESO-generated Secret mounted as a volume). Trailing newline trimmed.Key design points:
services.jsonfiles keep working unchanged — this is purely additive. An explicitliteral:prefix is available as an escape hatch for the rare value that genuinely begins with a scheme name.file:references resolve inside the authenticator'sApply()(with a short TTL cache) rather than at client-construction time, so when ESO refreshes a mounted Secret the new value is picked up without a pod restart.env:resolves at startup (its natural lifecycle).This lets non-sensitive config live in a ConfigMap (committable, auditable, GitOps-friendly) while only references point at ESO-generated native Secrets — aligning the SDK with the Vault → ESO → native Secret pipeline.
Scope / non-goals
env:andfile:only. A directvault:source is intentionally out of scope — ESO already lands secrets as native Kubernetes Secrets, and a runtime Vault dependency in the SDK would defeat that. The scheme is extensible, sovault:(or others) can be added later without a breaking change.Alternatives
${VAR}interpolation (os.ExpandEnvover the file at load). Simpler (one line), but env-only, resolves once at load (no rotation without restart), and blindly rewrites the entire file — any literal containing$is a footgun. Rejected in favor of the per-field, multi-source scheme.authSecretRef+ fixed/secrets/<ref>/<key>mount convention. Clean for bearer/api_key but doesn't map onto OAuth2's mix of secret and non-secret fields, and couples the config to a single hard-coded mount layout. The per-fieldsource:refform is more flexible.All reactions