feat(app-service): materialize manifest secrets[] as kubernetes secrets - #3758
Draft
RonnyPfannschmidt wants to merge 2 commits into
Draft
feat(app-service): materialize manifest secrets[] as kubernetes secrets#3758RonnyPfannschmidt wants to merge 2 commits into
RonnyPfannschmidt wants to merge 2 commits into
Conversation
Implements the top-level `secrets:` block of OlaresManifest.yaml. Each
entry pulls one Olares-provided env var (a SystemEnv or UserEnv,
referenced exactly like envs[].valueFrom) into its own Opaque Secret in
the app namespace, named verbatim after the entry and holding the value
under the constant key `value`.
The chart then consumes it with a plain secretKeyRef:
env:
- name: SMTP_PASSWORD
valueFrom:
secretKeyRef:
name: smtp-password
key: value
This closes a gap in the existing envs[] path: values pulled via
olaresEnv are rendered into helm values and end up as literal env values
in the pod spec, visible to anyone who can read the Deployment or its
ReplicaSet. secrets[] keeps them in a Secret instead. It mirrors the
model middleware credentials already use (tapr renders secretKeyRef
against a provisioned Secret) rather than inventing a new one.
Deliberately no new CRD: the Kubernetes Secret is the storage. Values
are re-resolved and rewritten on every install and upgrade so a changed
SystemEnv/UserEnv propagates. Cleanup is inherited from namespace
deletion on uninstall.
ApplySecrets runs from the existing applyAppEnv step on both the install
and upgrade paths, ahead of the helm install, so the Secrets exist by
the time pods start. The V3 helpers embed these, so all paths are
covered.
This is a transition helper built from existing primitives, expected to
be superseded once Olares provides first-class secrets; the
single-value-per-Secret shape is marked TODO(structured-secrets).
REQUIRES the schema change in beclab/api (top-level Secrets field on
AppConfiguration). The go.mod pin bump is intentionally NOT part of this
commit because the target pseudo-version does not exist until that lands
first; see the PR description.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@RonnyPfannschmidt is attempting to deploy a commit to the Lucky's projects Team on Vercel. A member of the Team first needs to authorize it. |
Hooks secrets[] into the existing env change-propagation chain instead of
only materializing Secrets at install and upgrade time.
Previously a rotated SystemEnv/UserEnv reached apps consuming it via
envs[], but silently never reached apps consuming it as a secret: the
SystemEnv/UserEnv controllers discover affected apps by scanning
appEnv.Envs, and secret declarations were deliberately not persisted
anywhere ("the Secret is the storage"). An app kept serving a stale
credential until someone happened to reinstall it — the worst failure
mode for a secrets feature, and precisely the case rotation exists for.
The declarations are now recorded on the AppEnv CR alongside Envs, which
makes them discoverable, and the rest of the chain is reused rather than
reinvented: annotate -> sync -> NeedApply -> ApplyEnvOp -> helm upgrade.
That also inherits the operational guards already proven there (deferral
while an app is Stopped, the env batch lease, re-enqueue on resume).
Only the declaration is stored, never the resolved value; AppSecretVar has
no field able to hold one. Change detection compares against the existing
Secret, which already holds the previous value, so nothing sensitive (nor
a hash of it) needs persisting to detect a rotation. A no-op reconcile
rewrites identical data, reports no change, and cannot trigger a spurious
redeploy.
A changed secret only requests a redeploy when the declaration sets
applyOnChange. This matters more than for envs: secret values are injected
into pods at start time, so without the redeploy the Secret is refreshed
while running pods keep serving the old value.
isReferenced in the SystemEnv and UserEnv controllers was duplicated
verbatim; both now delegate to one appEnvReferences helper so the two
cannot drift on which reference kinds they recognize.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repo:
beclab/Olares· Branch:feat/manifest-secrets-block· Lands: SECONDImportant
Blocked on beclab/api#5 — "feat(manifest): add top-level
secrets[]block to OlaresManifest".This branch does not compile in CI until that merges and the one-line
go.modbump below isapplied. See Before merging.
What
Implements the top-level
secrets:block ofOlaresManifest.yaml:Each declaration becomes its own Opaque Secret in the app namespace, named verbatim, holding the
resolved value under the constant key
value. The chart references it with nothing templated:Why
The existing
envs[].valueFrompath resolves a SystemEnv/UserEnv and renders it into.Values.olaresEnv.<NAME>, which charts drop into the containerenv:as a literal value — soit is visible to anyone who can read the Deployment or its ReplicaSet.
secrets[]keeps such valuesin a Secret instead, letting authors write meaningfully safer charts.
This mirrors the model middleware credentials already use (
taprrenderssecretKeyRefagainst aprovisioned Secret) rather than inventing a new one.
Change propagation
Rotating a referenced SystemEnv/UserEnv reaches secret-backed apps through the same chain that
already propagates env changes — not a parallel mechanism:
Recording the declarations on the
AppEnvCR (the api PR) is what makes the affected appsdiscoverable;
appEnvReferences— shared by both env controllers so they cannot drift — now matchessecrets[]as well asenvs[]. This inherits the operational guards already proven on that path:deferral while an app is
Stopped, theenv-batch-locklease, and re-enqueue on resume.Design notes
AppEnvCR — only the declaration.AppSecretVarhas no field to hold a value. Change detection compares against the existing Secret, which already
holds the previous value, so nothing sensitive (nor a hash of it) is persisted for that purpose.
A test serializes the CR and fails if the value ever appears in it.
applyOnChangegates the redeploy. Secret values are injected into pods at start time, sowithout a redeploy the Secret is refreshed while running pods keep serving the old value. A changed
secret without
applyOnChangerefreshes the Secret silently and takes effect on the next restart.cannot trigger a helm upgrade.
envs[]: the owner's UserEnv overrides a same-named cluster SystemEnv.valueFrom.envName, emptyname, duplicate names) rather than silently producing an empty Secret.
helm_ops_uninstall.go:79), whichremoves the Secrets. No teardown code added.
TODO(structured-secrets)marks the single-value-per-Secret shape; multi-key secrets are deferreduntil the Olares UX can author them.
Changes
pkg/appcfg/types.goAppSecretVaralias + re-exportedAppSecretValueKeypkg/appcfg/application.goSecretsonApplicationConfig(round-trips through the JSONSpec.Config)pkg/utils/app/app.gosecrets[]on the AppEnv CR inApplyAppEnv; change-awareApplySecretsForreturning the names that changed; sharedvalidateAppSecretDeclscontrollers/appenv_controller.gosyncSecretValues(→NeedApplyon real change whenapplyOnChange); sharedappEnvReferenceshelpercontrollers/systemenv_controller.go,controllers/userenv_controller.goisReferenceddelegates toappEnvReferences, which now scansSecretspkg/utils/app/secrets_test.go,controllers/appenv_secrets_test.go.gitignore.deps/+go.work.sumfor the cross-repo workflow (see below)Tests
a missing struct tag would fail closed with no secret at all)
ApplySecretsForreports only real changes (create → change; identical re-apply → none; rotate oneof two → just that one) — the contract the redeploy decision rests on
AppEnvCR; the resolved value is not (serialize-and-scan)ApplicationConfigappEnvReferencesmatches via env, via secret, and mixed; ignores a declaration withoutvalueFromAppSecretVarhas no value-bearing fieldsecrets[]Before merging
framework/app-service/go.mod:cd framework/app-service && go get github.com/beclab/api@<merge-sha>go build ./framework/app-service/...andgo test ./framework/app-service/pkg/appcfg/... ./framework/app-service/pkg/utils/app/... ./framework/app-service/controllers/...gh pr readythis PR.The bump is intentionally not in this branch: the target pseudo-version does not exist yet, so
committing a placeholder would only produce a misleading diff.
Verification so far
Verified locally with a gitignored
go.workpointing at a localbeclab/apicheckout of the api PR:go build ./framework/app-service/...passes;vetclean on the touched packagesappcfg,utils/app,controllerssuites pass, including the new testsframework/oacbuilds and its full suite passes against the same schemaTwo failures in the wider app-service suite —
TestMatchVersionand a 600s hang inpkg/task— arepre-existing, confirmed by stashing this branch's changes and reproducing them at
main.Not included (follow-ups)
oacvalidation forsecrets[]. oac parses non-strictly, so a typo'd entry currently passeschart lintsilently and only surfaces at install. Natural rules: non-emptyname, validDNS-1123 subdomain,
valueFrom.envNamerequired, no duplicates. Own PR (also needs oac'sv0.0.17pin bumped); benefits
cliimmediately via its localreplace.docs/developer/develop/package/manifest.md+ thezhcounterpart),and a pointer from the existing
type: passwordenv example towardsecrets[].Note on the
.gitignorehunkAdds
.deps/andgo.work.sumso a local checkout ofgithub.com/beclab/apican be wired in via a(gitignored)
go.workwhile developing across both repos. Unrelated to the runtime change and easyto drop if you'd rather not carry it.
🤖 Generated with Claude Code