feat(secrets): add secret_provider schema and Go AES-256-CBC crypto#1129
Draft
mleonidas wants to merge 15 commits into
Draft
feat(secrets): add secret_provider schema and Go AES-256-CBC crypto#1129mleonidas wants to merge 15 commits into
mleonidas wants to merge 15 commits into
Conversation
- packages/db: new `secret_provider` table (workspace-scoped, encrypted bytea config, unique by (workspace_id, name)) with `secret_provider_type` enum covering aws_secrets_manager, doppler, env. Drizzle migration 0195. - apps/workspace-engine/pkg/crypto: AES-256-CBC implementation matching the TypeScript @ctrlplane/secrets format (`<iv-hex>:<ciphertext-hex>`, PKCS#7 padding, 32-byte hex key). Lets Go decrypt configs written by the TS api using the same VARIABLES_AES_256_KEY. - Interop golden tests: 4 ciphertexts produced by @ctrlplane/secrets are decrypted byte-for-byte by the Go implementation, plus round-trip and malformed-input coverage.
…providers
- OpenAPI spec: new schemas (SecretProvider, SecretProviderType, per-type
configs for aws_secrets_manager/doppler/env, UpsertSecretProviderRequest)
and 4 routes under /v1/workspaces/{workspaceId}/secret-providers. SecretProvider
responses intentionally omit the encrypted config blob.
- apps/api: secret-providers.ts handler with Drizzle CRUD, a Zod discriminated
union that enforces type<->config correspondence beyond what OpenAPI oneOf
can express, and AES-256-CBC encrypt-on-write via @ctrlplane/secrets. Router
mounted in v1/workspaces/index.ts. Added @ctrlplane/secrets to api deps.
- e2e: secret-providers.spec.ts covers CRUD round trip, asserts the decrypted
config is never returned on get or list, plus 404 / idempotent-upsert /
validation-rejection cases.
Plaintext configs are never persisted; encryption uses the shared
VARIABLES_AES_256_KEY also consumed by the Go resolver
- pkg/db: secret_provider added to the embedded sqlc schema; new
secret_providers.sql with GetSecretProviderByName +
ListSecretProvidersByWorkspaceID queries; sqlc regenerated.
- pkg/secrets: package scaffold for resolving variable_value rows of
kind = secret_ref.
- types.go: SecretReference, Provider, ProviderConfig, ProviderFactory,
ProviderConfigStore interfaces.
- registry.go: type-keyed factory lookup, mirrors pkg/jobagents/registry.go.
- store.go: sqlc-backed PostgresStore that loads secret_provider rows and
decrypts the bytea config in-memory via a Decryptor (AES-256-CBC,
interoperable with @ctrlplane/secrets).
- cache.go: goroutine-safe TTL cache keyed by
(workspaceID, providerName, path, key) with InvalidateProvider hook for
LISTEN/NOTIFY consumers.
- resolver.go: glues store -> registry -> provider with optional cache,
rejects empty Provider/Key, propagates upstream errors.
- Unit tests cover cache hit/miss/expiry/invalidation, registry build and
factory errors, resolver happy path, cache reuse, post-invalidate refetch,
validation, and store/provider error propagation. -race clean
…rets Manager - pkg/secrets/env: process-env provider with mandatory allowedKeys allowlist. Resolve hard-fails on any key not in the workspace's configured list before touching os.LookupEnv; mitigates the multi-tenant escape RFC 0006 flags. Lookup function is injectable for testing. - pkg/secrets/doppler: HTTP client against the v3 /configs/config/secret endpoint with a Bearer service token. Path parses to <project>/<config>; Key is the secret name. Prefers value.computed and falls back to value.raw. Tested with httptest.Server. - pkg/secrets/awssm: aws-sdk-go-v2 secretsmanager-backed provider. Path is the secret name or ARN; empty Key returns the raw SecretString while a non-empty Key extracts a JSON field via gjson (supports dotted paths). Static creds via accessKeyId/secretAccessKey, partial creds rejected, default credential chain otherwise. Tests use an injected client interface and make no real AWS calls. - pkg/secrets/providers: RegisterAll(*Registry) and NewDefaultRegistry() give main.go and tests a single seam to wire every built-in provider. - go.mod: aws-sdk-go-v2 secretsmanager + config + credentials, tidwall/gjson.
- oapi: new SecretReferenceValue (secretProvider, secretKey, secretPath) added to the Value oneOf. (*Value).GetType returns "secret_ref" when the provider/key fields are populated; regenerated oapi types include AsSecretReferenceValue / FromSecretReferenceValue. - pkg/db/convert.go: the secret_ref kind path no longer errors. It now builds a SecretReferenceValue from the row's secret_provider/secret_key/ secret_path columns. Test updated from "unsupported" to a positive roundtrip. - variableresolver: new SecretResolver interface (satisfied by *secrets.Resolver). ResolveValue takes a secretResolver and workspaceID; the new "secret_ref" case joins secret_path with "/" for provider-native path encoding, calls the resolver, and returns the plaintext wrapped as a LiteralValue so it flows into release.Variables like any other value. Resolve and its three helpers thread the resolver + workspace through. - desiredrelease: reconciler holds *secrets.Resolver, threaded into variableresolver.Resolve. Reconcile, NewController, and New all accept a resolver; controller wires it into Process. deploymentplan does the same via PostgresVarResolver. - main.go: newSecretResolver assembles PostgresStore (AES-256-CBC decryptor from VARIABLES_AES_256_KEY) + default provider registry + value cache (SECRETS_CACHE_TTL, default 5m). When VARIABLES_AES_256_KEY is unset the resolver is nil and any secret_ref encountered surfaces a clear error at resolve time without crashing the engine. - config: VariablesAes256Key (VARIABLES_AES_256_KEY) and SecretsCacheTTL (SECRETS_CACHE_TTL) added to the env-driven Config.
- variableresolver.ResolveValue now returns (*LiteralValue, bool, error);
the bool is true only when the resolved value originated from a
secret_ref (literal, reference, and the dead "sensitive" path stay
false).
- The three priority helpers (resolveFromResource, resolveFromValues,
resolveFromVariableSets) propagate that flag. variableresolver.Resolve
aggregates the keys whose value came from a secret_ref and returns
them alongside the resolved map: (map[string]LiteralValue, []string,
error). A "resolved.sensitive" OTEL span attribute records the count.
- desiredrelease.reconciler captures the slice into sensitiveVars and
passes it to buildRelease, which now stores it on
release.EncryptedVariables (nil collapses to []string{} for stable JSON).
- deploymentplan.VarResolver interface gains the []string return.
PostgresVarResolver and the mock used in controller_test.go are
updated; deploymentplan.Controller now writes sensitiveKeys directly
onto the release it constructs instead of the hardcoded empty slice.
- variableresolver test call sites updated to discard the new sensitive
return where the test isn't exercising it.
Test infrastructure: - harness.FakeSecretResolver and harness.FailingSecretResolver satisfy variableresolver.SecretResolver. The fake stores canned (provider, path, key) -> value entries and records every Resolve call; the failing variant returns the injected error for every call. - WithSecretResolver(...) PipelineOption threads the resolver into desiredrelease.NewController through ScenarioState. - SecretRefValue(provider, key, path...) builds an oapi.Value carrying a SecretReferenceValue so scenarios can declare secret_ref entries with the same ergonomics as DefaultValue / WithVariableValue. - ProcessDesiredReleasesErr() returns the reconcile error instead of failing the test, so negative-path tests can assert on it. - AssertReleaseEncryptedVariables asserts the set of keys recorded on release.EncryptedVariables, order-independent. Test cases (test/controllers/secret_ref_test.go): - Happy path: resolved plaintext lands on release.Variables and the key is appended to release.EncryptedVariables. Resolver receives the expected provider / joined path / key. - Mixed literal + secret_ref: EncryptedVariables contains only the secret_ref-origin keys. - Empty SecretPath: the SecretReference.Path passed to the resolver is empty, matching providers like env. - Resolver outage: the reconciler propagates the upstream error and AssertNoRelease holds. Mirrors the Phase-5 design point that re-resolve-each-dispatch means an outage blocks the release rather than silently shipping a release without the variable. - No resolver configured: a clear "no SecretResolver configured" error surfaces; no release is produced. Production fixes uncovered while writing the tests: - variableresolver: priority-cascade helpers used to swallow ResolveValue errors. Fine for literal/reference (try the next candidate) but wrong for secret_ref - a Doppler / AWS outage was silently producing a release with the secret variable missing instead of blocking dispatch. Added an ErrSecretResolution sentinel that every secret_ref error path wraps, and the helpers now propagate any error matching it via a new third return value. Non-secret errors keep the previous fallthrough behavior. variableresolver.Resolve wraps the propagated error with the affected variable key. - desiredrelease.Controller / reconciler now hold variableresolver.SecretResolver (interface) instead of *secrets.Resolver (concrete). *secrets.Resolver still satisfies the interface so main.go is unchanged, but the harness can inject fakes without pulling pkg/db or pkg/crypto into the test binary.
* adds caching layer by provider/workspace on both value and provider
Job agent configs now interpolate {{ . }} expressions at dispatch time so
secrets resolved into dc.JobAgentVariables (and any other DispatchContext
field) can be referenced directly from agent config fields like apiKey,
token, or password. Without this, those fields were opaque strings and the
agent saw the literal "{{ .jobAgentVariables.argo_token }}" instead of the
resolved value.
- pkg/workspace/jobs/config_template.go: renderJobAgentConfig walks the
oapi.JobAgentConfig map, recurses into nested maps and arrays, and runs
every string value that contains "{{" through pkg/templatefuncs against
dispatchCtx.Map(). Strings without "{{" pass through unchanged; non-string
scalars (numbers, booleans, nil) are untouched. Errors include the dotted
path of the offending field for fast operator triage.
- pkg/workspace/jobs/factory.go: BuildDispatchContext calls
renderJobAgentConfig immediately after populateJobAgentVariables, so the
agent receives a JobAgentConfig with secrets already substituted. The
typed agent config readers (GetArgoCDJobAgentConfig etc.) read the
rendered values without any agent-side changes.
- Template render is a pure in-memory substitution: it does not call the
secret provider. Provider calls happen earlier, inside
populateJobAgentVariables -> variableresolver.ResolveForJobAgent ->
secrets.Resolver.Resolve. The render only reuses what's already in
dc.JobAgentVariables (and dc.Release.Variables, dc.Resource, etc.).
- Tests cover string passthrough, nested map / array recursion, scalar
passthrough, parse-error reporting with field path, empty-config short
circuit, missing top-level key error (security-safe: no silent empty),
and visible "<no value>" marker for a missing leaf key.
Lets variable_value rows pin a specific provider-side version of a secret. Useful for reproducibility (a release dispatched against version N keeps resolving the same value regardless of rotation) and for tying a release to a known-good upstream secret. - packages/db: variable_value gains a nullable secret_version text column. Drizzle migration 0196_third_reavers.sql. - workspace-engine schema mirror (pkg/db/queries/schema.sql) tracks the column; queries/variables.sql json_agg blocks now emit secretVersion; sqlc regenerated. VariableValueAggRow gets SecretVersion *string. - oapi: SecretReferenceValue gains an optional secretVersion field with a description of AWS / Doppler / env semantics. Regenerated. - pkg/db/convert.go: the secret_ref kind path copies secret_version onto the SecretReferenceValue when non-empty (nil stays nil so resolvers can treat it as "latest"). - pkg/secrets/types.go: SecretReference grows a Version string. - pkg/secrets/cache.go: cacheKey gains Version, so latest vs pinned-v1 vs pinned-v2 are distinct cache entries. Pinned entries are effectively immutable for the TTL window (upstream cannot change a past version); latest entries still TTL-gate to pick up rotations. - variableresolver/value.go: resolveSecretReference passes srv.SecretVersion through to the secrets.SecretReference. - pkg/secrets/awssm: when Version is set, route to VersionId (UUID form) or VersionStage (anything starting with AWS, plus the "stage:<name>" prefix for user-defined stages with the prefix stripped). Empty stays AWSCURRENT. - pkg/secrets/doppler: when Version is set, attach accept_secret_version=<v> as a query parameter; otherwise omit it. - pkg/secrets/env: ignored (process env has no version). Tests cover the UUID / AWSCURRENT / stage-prefix / empty paths for awssm, the query-param presence/absence for doppler, and cache key disambiguation across multiple pinned versions plus latest. Existing tests untouched.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mleonidas
changed the title
mleonidas
force-pushed
the
mleonidas/add_secrets_provider
branch
from
4f651d4 to
10c128a
Compare
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
1 participant
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.
No description provided.