feat(plane-enterprise): extend discrete credential parts to silo, live and Plane AI - #279
Conversation
…e and Plane AI Phase 1 wired the Django services to consume a rotating credential as discrete parts. Silo, live and Plane AI still needed a composed DSN, so a deployment backed by a managed rotation secret had to maintain hand-built URLs for them alongside the secret itself. With the companion app release those services read parts too, and this wires them up. Also fixes the shared-key defects a cross-service audit turned up. Per-workload least privilege. plane.infraCredsEnv is split into postgresCredsEnv / rabbitmqCredsEnv / redisCredsEnv (+ the existing opensearchCredsEnv) and recomposed from them, so the eleven existing callers render byte-identically. Live then takes Redis and nothing else; silo takes Postgres, RabbitMQ and Redis but not OpenSearch. Plane AI gets its own plane.piInfraCredsEnv because it reads different variable names: PLANE_PI_POSTGRES_* for its own database and FOLLOWER_POSTGRES_* for its read path into the main Plane database, both from the same external_secrets.database Secret — one managed instance hosting two databases is the shape this chart provisions. It deliberately receives no RABBITMQ_*: Plane AI resolves an AMQP broker ahead of a Redis one, and Redis is its broker here, so emitting RabbitMQ parts would silently move its queue. An explicit env.pi_envs.follower_postgres_uri still wins, as the escape hatch for a follower with genuinely separate credentials. Composed DSNs are blanked in silo/live/pi-api Secrets whenever the matching external_secrets block is set, for the same reason as the API: each app prefers a URL when one is present, so a stale composed URL would shadow the rotated password. Audit fixes: RUNNER_HMAC_SECRET_KEY was delivered only to the runner, which has no code that reads it, while the API — the only service that verifies the signature — never received it, so runner HMAC auth could not succeed. It is now mounted on the seven api-family workloads that already mount silo-secrets, for the same reason: the verifying side lives there. It was also the one shared-key render bypassing plane.secretValue, so requireExplicitSecrets did not cover it, and runner.deployment was the only workload missing appKeysSecretRef. Both fixed. Making the runner actually sign its requests is a product decision and stays out of scope. AES_SALT was advertised as an app-keys member but had no values key and was never rendered — api, silo and pi worked only because all three share the same hardcoded default. env.silo_envs.aes_salt now renders into all three Secrets, and only when non-empty, so that default still applies to existing instances. app_keys_existingSecret combined with a legacy *_existingSecret carrying the same key diverges per service: app-keys is mounted first, so a duplicate wins on the workloads mounting that group and loses elsewhere, leaving two services disagreeing on a key that has to match. The chart cannot see inside an operator-supplied Secret, so NOTES.txt now warns when both are set and lists the keys that must not be duplicated. The public-default warning covered four of the six app keys; SILO_HMAC_SECRET_KEY — the API's inbound-auth key for the whole silo surface — and CURSOR_WEBHOOK_SECRET were missing. Both added. values.yaml now documents which keys the app-keys Secret must contain, SECRET_KEY foremost: omitting it gives each API pod a different random signing key, silently. Version gating. The "these services read connection URLs" and "redis needs parts support" warnings are gated on planeVersion (v3.2.0 and v3.1.0 respectively) rather than always firing. planeVersion is operator-supplied and need not be semver, so an unparsable value warns instead of failing the render — semverCompare would error. Verified additive over Phase 1: 28 containers, no env var lost anywhere; the only change on default values is RUNNER_HMAC_SECRET_KEY reaching the api family, which is the fix. helm lint clean and ten configurations render valid YAML, including runner with and without app-keys, the legacy groups alone, and the four version gates. Requires the companion app release for silo/live/pi parts support; the chart warns when planeVersion predates it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
…agent consumer Chart goes to 3.4.0, one minor above the rebased Phase 1 branch (master had taken 3.2.0 while both branches were open). The redis gate moves from "< 3.1.0" to "< 3.2.0": v3.1.0 shipped without the API's discrete REDIS_* support, so the old bound told operators a released version was new enough when it is not. Both gates now sit at 3.2.0 because both app PRs are expected in that release; they stay as separate variables so that if one lands later, only its own bound moves. The agent consumer, which landed on master after these branches were cut, also mounts runner-secrets — it is in the api family and mounts silo-secrets for the same reason: the API is the side that verifies the runner's HMAC. Verified additive over current master: 30 containers, no env var lost. The additions are SKIP_ENV_VAR, which matches the application's own default, and RUNNER_HMAC_SECRET_KEY reaching the api family, which is the audit fix. helm lint clean, seven configurations render valid YAML, requireExplicitSecrets still catches the runner key, and the gate was checked at v3.1.0 (warns), v3.2.0 (silent) and an unparsable "stable" (warns). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d replica (3.5.0) (#284) * feat(plane-enterprise): dedicated hooks for AI provider keys and silo connectors Both of these could previously only be externalized through a whole-Secret hook, which made the one thing people actually need impossible. AI/LLM provider keys lived in the Plane AI Secret alongside PLANE_PI_DATABASE_URL, FOLLOWER_POSTGRES_URI, AMQP_URL, CELERY_BROKER_URL and the signing keys. Adopting pi_api_env_existingSecret therefore meant owning all of those too — and they are per-environment, while a provider account is not. So "one provider key shared by every environment" had no expression. ai_providers_existingSecret carries nothing environment-specific, so one Secret can be mirrored into as many namespaces as share the account, while a customer can point the same values key at a per-environment Secret. The chart only takes a name; the granularity is the secret store's business. The suppression had to cover the empty-string branches, not just the populated ones. The chart rendered COHERE_API_KEY: "" and CUSTOM_LLM_API_KEY: "" whenever those providers were disabled, and envFrom resolves later-source-wins — so a shared Secret mounted earlier would have been silently overwritten with empty strings. There is now an assertion for exactly this, and it fails on the pre-change render. Silo connector credentials had the same shape of problem: the silo Secret also carries DATABASE_URL, AMQP_URL and REDIS_URL. silo_connectors_existingSecret takes the whole connector surface, client ids and app names included, so it has a single owner rather than a half-external state. It is mounted on all nine workloads that mount silo-secrets, not just silo: the Django auth adapter reads GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET from that Secret on the api family, so scoping it to silo alone would have dropped those variables there. There is an assertion for that too. Also adds hack/resolve-env.py and hack/assert-secrets.py. The first resolves what each container's environment actually becomes — envFrom in list order, then explicit env on top — because a chart diff cannot show whether a key that moved between Secrets is still reachable. The second carries the assertions this work turns on, including a no-DSN check: a connection string with an embedded password is a rotation dead-end, since nothing can track a rotated password baked into a string. Verified byte-identical resolved environments across all workloads when no new hook is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(plane-enterprise): read-replica credential mirror, and omit an unset AWS region services.postgres.read_replica.remote_url was the last password-bearing connection string in this chart with no discrete-parts alternative, so a managed rotation could never reach the replica. external_secrets.database.readReplica now supplies POSTGRES_READ_REPLICA_{USER,PASSWORD} from a mirrored Secret with the endpoint in values, and DATABASE_READ_REPLICA_URL is blanked in that mode so a stale composed URL cannot shadow the rotated credential. No application change was needed: Django takes the replica's parts straight into its DATABASES dict, so nothing composes a URL at all. secretName defaults to the primary's, since a replica normally accepts the same credential. The fail guard that demanded remote_url now accepts the mirror as a source too, and says what the alternatives are rather than naming only one of them — otherwise enabling a replica keylessly was impossible by construction. AWS_REGION and AWS_S3_ENDPOINT_URL are now omitted when unset rather than rendered empty. The keys next to them were already treated this way because an empty AWS_ACCESS_KEY_ID derails boto3's credential chain; the region is subtler. When a pod identity is attached and OpenSearch has no basic auth, the API signs OpenSearch requests with SigV4 and reads the region as `os.environ.get("AWS_REGION") or default` — an empty string is present, so the default never applies, and every signed request goes out with an empty region and is rejected. Absent is the only safe encoding of "unset". The companion app change makes the same value safe on older charts. Verified with hack/: the resolved environment of every container is byte-identical when no new hook is set; a fully externalized render (database, rabbitmq, redis, replica, external object storage) now contains zero connection strings with embedded passwords; and in the keyless case AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and AWS_REGION are absent from every container rather than empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(plane-enterprise): storage credential mirror and a cloud-identity marker (3.5.0) external_secrets.storage covers the object-storage credentials that a pod identity cannot replace: an S3-compatible backend with no workload identity, or GCS with a service-account key. That key is the highest-value blob in a GCS deployment — the whole service-account JSON, private key included — and it had no hook at all, so the only way to keep it out of values.yaml was to take over the entire doc-store Secret and own USE_MINIO, the bucket name and the size limit along with it. The mirror is wired through the three composite credential helpers rather than per-workload, since those already reach every workload that mounts the doc-store Secret. It is inert while the bundled MinIO is deployed, which supplies its own credentials. serviceAccount.cloudIdentity declares that an identity is attached out of band. It changes no rendered output, and it cannot be inferred: an EKS Pod Identity association is an API object keyed on cluster, namespace and service account, and nothing about it shows up in the pod spec. What it buys is three warnings for the interactions an attached identity creates — a static env.aws_access_key silently beating the identity in the SDK chain, the same for Plane AI's Bedrock keys, and the one that is genuinely surprising: a remote OpenSearch with no credentials will be authenticated with SigV4, because both the API and Plane AI read "AWS credentials are present" as "sign requests" — and an identity puts credentials in every container on the ServiceAccount. Correct for Amazon OpenSearch Service, wrong for any other. The warning names both fixes. Verified with hack/: the resolved environment of every container is byte-identical when no new hook is set; ten configurations render valid YAML including all hooks at once and the legacy groups; and the acceptance case — every hook in use, requireExplicitSecrets on, all signing keys blanked — renders with no credential pattern, no connection string carrying a password, and AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY / AWS_REGION absent from every container. What remains in the chart's own Secrets is empty URL placeholders and non-secret configuration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(plane-enterprise): expose the 3.3.0–3.5.0 secret hooks in questions.yml None of the credential mirrors, shared key groups or ServiceAccount fields were reachable from the Rancher UI, which is how most operators configure this chart — a hook nobody can find is a hook nobody uses. Three new groups. Credential Mirrors covers external_secrets.{database,rabbitmq,redis, storage,opensearch}, kept apart from the existing whole-Secret replacements because they answer a different question: a mirror is read as parts and never composed into a DSN, so a managed rotation lands with nothing to recompute. Shared Secrets covers the four key groups. Service Account covers create/name/annotations/cloudIdentity plus env.requireExplicitSecrets. The descriptions carry the operational traps rather than restating the field names: Amazon MQ needs rabbitmq_ssl and port 5671 because the parts path has no URL scheme to carry TLS; ElastiCache puts its auth token in the password key; the signing keys must never be shared or rotated; EKS Pod Identity needs no annotation because it binds by ServiceAccount name. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
90de23d
into
feat/externalize-secrets-eso-rotation
|
Absorbed into #278, which now carries the whole plane-enterprise side as one PR. Verified rather than assumed: the Closing so nobody reviews the same work twice. plane-ce stays separate in #285 — it is a different chart and touches no file in this one. |
Description
Phase 1 wired the Django services to consume a rotating credential as discrete parts. Silo, live and Plane AI still needed a composed DSN, so a deployment backed by a managed rotation secret had to maintain hand-built URLs for them alongside the secret itself. With the companion app release (makeplane/plane-ee#8764) those services read parts too, and this wires them up — plus fixes the shared-key defects a cross-service audit turned up.
Per-workload least privilege
plane.infraCredsEnvis split intopostgresCredsEnv/rabbitmqCredsEnv/redisCredsEnv(alongside the existingopensearchCredsEnv) and recomposed from them, so the eleven existing callers render byte-identically — verified by diff.Each workload then takes only what it uses:
PLANE_PI_POSTGRES_*,FOLLOWER_POSTGRES_*, Redis, OpenSearch — no RabbitMQPlane AI needs its own
plane.piInfraCredsEnvbecause it reads different variable names:PLANE_PI_POSTGRES_*for its own database andFOLLOWER_POSTGRES_*for its read path into the main Plane database, both from the sameexternal_secrets.databaseSecret — one managed instance hosting two databases is the shape this chart provisions. It deliberately gets noRABBITMQ_*: Plane AI resolves an AMQP broker ahead of a Redis one, and Redis is its broker here, so emitting RabbitMQ parts would quietly move its queue. An explicitenv.pi_envs.follower_postgres_uristill wins, as the escape hatch for a follower with genuinely separate credentials.Composed DSNs are blanked in the silo/live/pi-api Secrets whenever the matching
external_secretsblock is set, for the same reason as the API: each app prefers a URL when one is present, so a stale composed URL would shadow the rotated password.Audit fixes
RUNNER_HMAC_SECRET_KEYwas broken by construction. It was delivered only to the runner — which has no code that reads it — while the API, the only service that verifies the signature, never received it. Runner HMAC auth could not succeed. It is now mounted on the seven api-family workloads that already mount silo-secrets, for the same reason: the verifying side lives there. It was also the one shared-key render bypassingplane.secretValue, sorequireExplicitSecretsdid not cover it, andrunner.deploymentwas the only workload missingappKeysSecretRef. Both fixed. Making the runner actually sign its requests is a product decision and stays out of scope.AES_SALTwas advertised as an app-keys member but had no values key and was never rendered — api, silo and pi worked only because all three share the same hardcoded"aes-salt"default.env.silo_envs.aes_saltnow renders into all three Secrets, and only when non-empty, so that default still applies to existing instances.app_keys_existingSecretplus a legacy*_existingSecretcarrying the same key diverges per service. App-keys is mounted first, so a duplicate wins on the workloads mounting that group and loses everywhere else, leaving two services disagreeing on a key that has to match — which surfaces as auth failures or undecryptable data. The chart cannot see inside an operator-supplied Secret, so NOTES.txt now warns when both are set and lists the keys that must not be duplicated.The public-default warning covered four of six app keys.
SILO_HMAC_SECRET_KEY— the API's inbound-auth key for the entire silo surface — andCURSOR_WEBHOOK_SECRETwere missing. Both added.values.yamlnow documents which keys the app-keys Secret must contain,SECRET_KEYforemost: omitting it gives each API pod a different random signing key, silently.Version gating
The "these services read connection URLs" and "redis needs parts support" warnings are now gated on
planeVersion(v3.2.0 and v3.1.0 respectively) rather than always firing.planeVersionis operator-supplied and need not be semver, so an unparsable value warns instead of failing the render —semverComparewould error onstable.Type of Change
Test Scenarios
Additive over Phase 1, verified mechanically: resolving every
envFromreference to its actual keys across 28 containers with all services enabled, before and after — no env var lost anywhere. The only change on default values isRUNNER_HMAC_SECRET_KEYreaching the api family, which is the fix.Helper split is byte-identical for the eleven existing callers (
diffof the full render).Per-workload wiring asserted programmatically, not by eye: live has exactly 4 Redis vars and no Postgres/RabbitMQ/OpenSearch; pi has
PLANE_PI_POSTGRES_*+FOLLOWER_POSTGRES_*+ Redis + OpenSearch and no RabbitMQ; silo has Postgres + RabbitMQ + Redis and no OpenSearch.helm lintclean. Ten configurations render valid YAML: defaults, all four external blocks, redis-only, pi + database + redis, runner with and without app-keys, app-keys + a legacy group,aes_saltset, every service enabled, and the legacy groups alone.requireExplicitSecrets=truewith an empty runner key now correctly fails the render. The version gate was checked at v3.0.0, v3.1.0, v3.2.0 andstable.Not covered: no live cluster apply. Wants a staging run with ESO + Reloader to confirm a rotated DB secret restarts silo/pi and they reconnect through the parts path.
References
planeVersionpredates it.🤖 Generated with Claude Code