Harden GitOps delivery: fix silently-broken chart values, enforce the ADRs in CI - #1
Merged
Merged
Conversation
The repository had no LICENSE. Adds MIT, matching the sibling lakehouse-iceberg-batch project. .gitignore excluded .sops.yaml, which is wrong: that file holds public recipients and creation rules and is meant to be committed. Ignoring it would have silently excluded the config while the *.agekey rule below it does the actual protecting. Broadens the private-key rule and drops the .sops.yaml entry. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
All 13 HelmReleases pinned a range ("6.x", "61.x", "1.15.x"). Flux resolves
these against the chart index on every reconcile, which means the contents
of the cluster can change with no commit — breaking the three properties
GitOps exists to provide: the repo is the source of truth, every change is
reviewed, every change is revertible.
Pinning alone would trade silent drift for neglect, so Renovate is added in
the same change. renovate.json gates the upgrades that are not routine:
stateful workloads (storage format implications), external-secrets (0.14
moves ExternalSecret from v1beta1 to v1 and would require rewriting every
CR here), and Strimzi (operator must move before Kafka.spec.version). No
automerge — a repo that automerges its own dependency PRs is demonstrating
a pipeline nobody reads.
Two failures surfaced immediately from pinning:
- The StackGres HelmRepository URL served no index.yaml at all. The
.../any/latest/ path returns 404; the published repo is
.../stackgres/helm/.
- stackgres-operator 1.13.x declares kubeVersion "1.18.0-0 - 1.31.x" and
cannot install on a current cluster. Pinned to 1.19.0.
Rationale in ADR-0005 (added later in this branch).
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
The PostgreSQL cluster was postgres-dev in dev and postgres-prd in prod; Kafka was kafka-dev and kafka-prd. This reads as good hygiene and is the opposite: service DNS derives from the resource name, so the address of the database differed per environment, and consumers declared once in base/ had to hardcode one of them. They hardcoded dev. Production Airflow and production Trino were both configured to connect to postgres-dev.databases.svc.cluster.local, because that string was in base/ and prod inherited it. The name carried no information the repo did not already have three times over — different cluster, different Flux Kustomization, different overlay directory. Resources are now named for what they are: `postgres`, `kafka`. Also moves the four ExternalSecrets into base/. Their shape is identical in every environment; the only thing that differs is the vault behind the azure-keyvault ClusterSecretStore, which each overlay already defines. Keeping per-overlay copies meant two files to hand-sync for no benefit. This is not a free change on a live cluster: renaming an SGCluster makes StackGres provision new PVCs, so it is a backup-and-restore rather than an apply. Paid at zero cost here because dev is rebuildable and prod has never run. Rationale in ADR-0006 (added later in this branch). Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
Helm does not error on unknown values. Every defect below rendered cleanly,
passed `kustomize build`, and would have committed without complaint.
Trino
- Worker scaling was patched at coordinator.replicas / worker.replicas in
the prod overlay. Neither key exists in the Trino chart — worker count is
server.workers. Git recorded a scaled prod cluster; Helm ran the default.
- The catalogs referenced ${ENV:TRINO_PG_USER} and friends, but nothing set
those variables and no Secret existed. All three catalogs fail to
initialise and the coordinator does not start. Adds a trino-credentials
ExternalSecret and wires it through envFrom.
- Drops the `hive` catalog. It used hive.metastore=file against
/tmp/data — node-local, unshared, lost on restart. That is not a working
object-store catalog in a multi-pod deployment. Iceberg-on-MinIO is the
sibling lakehouse project's scope.
Superset
- The secret was written to configOverrides.secret. configOverrides values
are Python source appended to superset_config.py, so this is a syntax
error at import and SUPERSET_SECRET_KEY was never set. Moves it to
extraSecretEnv.SUPERSET_SECRET_KEY.
Airflow
- postgresql.enabled defaults to true, so the chart deployed its own
database while metadataConnection pointed at StackGres. Two databases,
neither consistently used.
- metadataConnection had no password, falling back to the chart default
"postgres". Supplies it from Key Vault via the ExternalSecret.
- Adds dags.gitSync: the scheduler previously had no DAG source at all.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
Three gaps that all shared a root cause — the HelmReleases described what to install and nothing about how it should behave once installed. Remediation and drift detection No HelmRelease set install/upgrade remediation, so a failed upgrade left the release wedged until someone noticed. None set driftDetection, which is what makes Flux a control loop rather than a periodic apply — without it a kubectl edit against prod survives indefinitely. cert-manager, ESO, ingress-nginx, StackGres and kube-prometheus-stack get explicit caBundle ignore rules: their webhook CA is injected at runtime, so drift detection would otherwise report and revert a live value on every reconcile. Resource requests and limits ADR-0004 argues four node pools prevent noisy-neighbour effects. Pools only isolate across pools; within data-compute, Trino GC and the Airflow scheduler compete for the same node. Nothing declared requests or limits, so the scheduler had no basis for placement and the kernel nothing to arbitrate with. The ADR's own rationale was unmet. Persistence Prometheus and Loki both ran on emptyDir. Every metric and log line was lost on pod restart, which makes the observability stack unable to answer the one question it exists for. Adds storageSpec, singleBinary persistence, Grafana persistence, and explicit retention. Also disables the Loki gateway (an extra hop that only matters when the read/write paths are split) and gives the rules sidecar limits. Prod overlays gain matching resource, retention and volume-size patches. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
docs/CONTEXT.md draws a CDC path from StackGres through Debezium into Kafka.
No KafkaConnector existed anywhere in the repository. The Strimzi operator
was deployed, KafkaConnect was configured, and the pipeline did nothing —
the diagram documented an intention, not a system.
The arrow turns out to be three separate pieces of configuration, all of
which have to be right for one event to flow:
- wal_level=logical, via a new SGPostgresConfig. The default "replica"
level does not emit enough information for logical decoding, so the
connector would have run and produced nothing.
- The KafkaConnector CR itself, with a bounded table.include.list. An
unbounded replication slot pins WAL for every table in the database,
and an unconsumed slot will fill the disk.
- A Role/RoleBinding for the Connect service account. ${secrets:...}
placeholders are resolved by the Connect pod against the API server;
without it the connector fails config resolution with a 403 and lands
in FAILED rather than reporting an obvious permissions error.
KafkaTopics are declared rather than auto-created, so retention and
partitioning are reviewable instead of inheriting broker defaults. Connect's
internal topics are compacted — losing them means the cluster forgets
connector offsets and re-snapshots on restart.
KafkaConnect moves from the per-environment overlays into base. The only
thing that had made it environment-specific was the bootstrap address,
which is now identical everywhere (see the naming refactor). Prod patches
replica count and replication factors.
Adds an SGScript that provisions the airflow role and database, with the SQL
templated by ESO around the vaulted password — so one credential in Key
Vault serves both the role creation and the chart that logs in with it.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
docs/CONTEXT.md claimed data-platform would not reconcile until databases
and monitoring were healthy. It would not have waited. Only cluster-infra
declared healthChecks; the other three layers reported Ready as soon as
their manifests were *applied*, so data-platform started against a
StackGres operator and a Strimzi operator that were still coming up.
Every layer now sets wait: true with an explicit timeout, which waits on the
readiness of everything applied — including the HelmReleases — rather than
enumerating a handful of Deployments. retryInterval is set so a transient
failure retries faster than the 10m reconcile interval.
Adds postBuild substitution from a per-environment cluster-vars ConfigMap,
for values that are environment-specific but not secret. This fixes Fluent
Bit, which hardcoded cluster=dev in base — prod logs would have been
labelled as dev.
Substitution is enabled on cluster-infra and monitoring only. data-platform
deliberately does not use it: Trino catalogs contain ${ENV:...} and the
Debezium connector contains ${secrets:...}, both resolved at runtime by
those components, and both would be mangled by envsubst.
Adds a notification Provider and Alerts. Drift detection and remediation
retries are only useful if somebody learns when they fire; until now a
failed reconcile was visible only to whoever happened to run `flux get all`.
Drift is reported at info severity, so it needs its own Alert or the
error-only filter swallows it.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
The platform holds an OLTP database, an object store and a Kafka cluster, and had no network or workload isolation of any kind. Pod Security Admission All ten namespaces were bare. Each now enforces `baseline` and audits and warns at `restricted`, so the distance to tightening is visible rather than guessed at. monitoring is the one deliberate exception at `privileged` — Fluent Bit and node-exporter need hostPath and hostNetwork, which baseline forbids. NetworkPolicies Kubernetes' default is that any pod may reach any other pod cluster-wide. The allow rules are written one-per-arrow from the architecture diagram, which makes network-policies.yaml the executable form of that diagram: if a connection is not drawn in CONTEXT.md, it is not permitted. These live in the layer that owns each namespace rather than in cluster-infra, because a policy applied before its namespace exists fails the whole Kustomization. Egress is deliberately not denied. Doing so without a matching DNS allow rule breaks name resolution for every pod, which is the most common way a NetworkPolicy rollout causes an outage. cert-manager cert-manager was installed with no Issuer and ingress-nginx with no Ingress — the entire TLS path was deployed and unused. Adds production and staging ClusterIssuers; staging matters because Let's Encrypt rate-limits duplicate certificates and a misconfigured Ingress exhausts that quota before anyone notices. Workload Identity ADR-0003 claimed the client-id annotation was wired. It was not present anywhere. Adds the service account annotation and the azure.workload.identity/use pod label that actually triggers token injection, with the client ID substituted per environment. PodDisruptionBudgets are prod-only: in dev every component runs a single replica, where minAvailable would block node drains rather than protect anything. Strimzi and StackGres manage their own, so those are absent. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
The repository had no CI at all. Every decision its ADRs recorded was being
violated somewhere in the manifests at the time those ADRs were written —
not through carelessness, but because a written decision that nothing checks
is a decision that has already started decaying.
Four jobs:
kustomize build — all ten overlays render
kubeconform — resources match their API schemas, including Flux,
Strimzi, cert-manager and ESO CRDs from the community
catalog
conftest — policy/gitops.rego enforces the ADRs directly: no
floating chart versions, driftDetection and
remediation present, dependsOn implies wait+timeout,
PSA labels on every namespace, no Secret in git
helm render — every HelmRelease rendered through its real chart,
then asserted against the resulting objects
The last job is the one that matters. Helm ignores unknown values silently,
so a patch targeting a key the chart does not have renders perfectly, passes
every YAML and schema check, and does nothing. No amount of manifest
validation catches that; only rendering through the chart and inspecting the
resulting Deployment does.
Reverting the Trino patch to the form this branch found it in reproduces the
failure the check exists for:
[FAIL] trino worker replicas == 3 (rendered: 1)
Rendering is pinned to KUBE_VERSION 1.31.0, which is how stackgres-operator
1.13.x was caught declaring a kubeVersion ceiling below the target cluster.
assert-overlay.py also checks that Kafka topic replication never exceeds the
broker count, that every rendered workload container declares requests and
limits, and that no environment-specific service name crosses into the other
overlay. Charts whose upstream templates omit resources on their own Jobs
are listed as explicit exemptions with a stated reason rather than
loosening the check.
Makefile exposes the same pipeline locally as `make validate`.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
The repository had no README — no front door at all for a portfolio piece.
Adds one modelled on the sibling lakehouse-iceberg-batch project, leading
with what CI proves, what was wrong and how it was caught, and a named list
of gaps rather than implied completeness.
New decision records:
ADR-0005 exact chart pins, upgrades via Renovate
ADR-0006 resource names do not encode their environment
ADR-0007 enforce the ADRs in CI rather than trusting them
Corrections to existing documents, each of which asserted something the
manifests did not do:
ADR-0002 described dev as "a VPS" — singular — which is incompatible
with ADR-0004's four node pools. A node holds one value per
label key, so on one VPS nothing schedules. Now four nodes,
with the cost math, which stays inside the original envelope.
ADR-0002 also claimed dev was "tested against a real cluster". Nothing
in the repo supports that: gotk-components.yaml is still the
bootstrap placeholder. Replaced with a precise statement of
what is actually verified — static rendering, chart rendering,
schema and policy checks — and a note to upgrade the claim only
once a real bootstrap has run.
ADR-0003 claimed Workload Identity was "wired in the overlay
ClusterSecretStore manifests". It was not wired anywhere, and
the ClusterSecretStore is the wrong place for it. Documents
where the three pieces of the trust chain actually live.
ADR-0004 rationale rested on noisy-neighbour isolation that node pools
alone do not provide. Explains why requests and limits are the
part that makes the topology real.
CONTEXT the dependsOn ordering claim now explains why wait: true is
what makes "healthy" mean anything. Adds the three pieces the
CDC arrow depends on, and a scope boundary stating plainly that
Trino does not query MinIO here — that is the sibling project's
subject, not something this repo should imply it already does.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw
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.
Reviews this repository against the charts it actually deploys, fixes what was
broken, and adds the CI that stops it breaking again.
The structure was already good — base/overlay split, layered reconcile
ordering, ESO over encrypted secrets, ADRs that weigh real alternatives. The
problem was that almost nothing was verified, and several documents asserted
things the manifests did not do.
What was broken
All of these rendered cleanly and passed
kustomize build. That is the point:YAML validity was never the bar.
coordinator.replicas/worker.replicas— neither key exists in the chart${ENV:...}with noenvFromand no Secretbase/hardcodedpostgres-dev.databases.svcmetadataConnection, with no passwordconfigOverrides.secretconfigOverridesvalues are Python source — syntax error at import;SUPERSET_SECRET_KEYnever setHelmRepositoryURL served no indexstackgres-operator1.13.x declareskubeVersion: … - 1.31.xcluster=devin basedependsOnwithoutwait: true6.x,61.x)emptyDirKafkaConnectorThe last two of these were found by reading the docs against the manifests; the
StackGres
kubeVersionfailure was found by the new CI.What CI now proves
kustomize buildproves YAML is valid. It cannot prove the values meananything to the chart, because Helm ignores unknown values silently.
Four jobs:
kustomize buildon all ten overlays,kubeconformagainst thecommunity CRD catalog,
conftestagainstpolicy/gitops.rego, and — the onethat matters — every
HelmReleaserendered through its real chart withassertions against the resulting Kubernetes objects.
Reverting the Trino patch to the form this branch found it in reproduces the
failure the check exists for:
policy/gitops.regoencodes the ADRs directly, so a change that contradicts adocumented decision fails with a message naming it. All six rules were
negative-tested against a synthetic bad manifest.
Reading the commits
Ten commits, each building independently (bisectable):
chore— LICENSE,.gitignore(.sops.yamlwas wrongly ignored)chore(deps)— exact pins + Renovaterefactor(naming)— drop environment from resource names (ADR-0006)fix(charts)— value correctionsfeat(reliability)— remediation, drift detection, resources, persistencefeat(cdc)— the Debezium pipelinefeat(flux)— health-gated ordering,postBuild, notificationsfeat(security)— PSA, NetworkPolicies, PDBs, ACME issuersci— the validation pipelinedocs— README, ADR-0005..0007, correctionsCommits 2 and 5 both touch all 13
helm-release.yamlfiles — commit 2 changesonly the
version:line, commit 5 adds the reliability and resource config.Those concerns genuinely share the same files; read them together per chart.
Scope, stated plainly
Verification here is static, for both environments. Everything is
established without a cluster: overlays render, charts render, rendered
objects carry the intended replicas and limits, resources match their API
schemas. Nothing demonstrates that pods schedule, that the CDC connector
reaches
RUNNING, or that a query returns rows.clusters/*/flux-system/gotk-components.yamlis still the bootstrapplaceholder, so no
flux bootstrapresult is recorded for either environment.ADR-0002 previously claimed dev was "tested against a real cluster"; that claim
is removed rather than left standing. The README's Scope and known gaps
section lists the rest — stale-but-pinned charts, the Bitnami catalog risk,
no backup story for stateful workloads, alerting that covers delivery but not
the platform.
The highest-value next step is runtime verification: a
kindjob runningflux installagainst a reduced-footprint overlay, which is the Kubernetesanalogue of the sibling lakehouse project's
make smoke.🤖 Generated with Claude Code
https://claude.ai/code/session_01E5ykf4XNPrwfvB5UjGdyZw