fix: make the addon catalog installable — valid label values + tenant appset in the default path - #197
Merged
Merged
Conversation
The portal renders charts/tenant and commits one manifest per tenant to tenants/<cluster>/<tenant>.yaml in the tenants GitOps repo. The ApplicationSet that applies those commits is what turns a portal write into a Platform CR on a cluster; without it, portal-created tenants never reach the cluster at all. It sat under applicationsets/opt-in/, which app-of-apps sources without directory.recurse and therefore never applies. So on a cluster built by cluster-bootstrap with tenants_repo_url set, the read-only deploy key for the tenants repo was registered in ArgoCD and nothing was installed that reads it — a credential provisioned for a consumer that does not exist. Enabling the tenant path meant an out-of-band apply, which put a manual step in the middle of an otherwise declarative install. The reason it was withheld is real: it named an org. An appset that hardcodes git@github.com:<org>/... cannot be correct in a fork, and one whose git generator reaches for a private repo it cannot see fails to generate, goes ErrorOccurred, and drags app-of-apps to Degraded. That argument applies to naming an org, not to the appset. So the org is no longer named. Both halves come from the cluster Secret, the same way gitops/repo-url already lets every addon appset template its own source: annotation gitops/tenants-repo-url the repo the portal pushes to label portal/tenants-enabled opts the cluster in A cluster that has not opted in matches nothing in the clusters generator, so the child git generator never runs and the appset sits healthy with no children. Nothing reaches for a repo it cannot see, which is the condition that kept it out of the default path. The generators are now a matrix — clusters first, git second, because a matrix child that consumes parameters must come after the one that produces them. That ordering also buys the routing the single git generator could not express: the discovered path is tenants/<cluster_name> rather than a tenants/* glob, and the destination is the matched cluster rather than a fixed in-cluster address. A glob crossed with the clusters generator would apply every cluster's tenants to every cluster; naming the directory after the cluster means each one sees only its own, and the appset is correct on an ArgoCD with more than one cluster registered. cluster_name carries the full EKS cluster name (cluster-bootstrap wires it from the cluster component's output, which is <environment>-<name>), so it matches the directory the portal writes. goTemplateOptions keeps missingkey=error, so a cluster labelled without its annotation fails to render rather than silently generating nothing. The two are stamped under one condition, alongside the deploy key, so they cannot arrive apart. The fork-safety gate now scans this appset — it only reads the applied set, and opt-in/ is excluded by construction.
…accepts
A Kubernetes label value is `[A-Za-z0-9]` joined by `-`, `_` or `.`, up
to 63 characters. `/` is not in that set — it is legal in a label KEY's
prefix and in an AWS tag value, and nowhere else. Every ApplicationSet
template stamped the repository dimension as
platform.nanohype.dev/repository: nanohype/eks-gitops
so every Application the controller rendered was rejected at admission:
Application.argoproj.io "cert-manager" is invalid: metadata.labels:
Invalid value: "nanohype/eks-gitops": a valid label must be an empty
string or consist of alphanumeric characters, '-', '_' or '.'
The blast radius is the whole catalog, not the one Application the error
names. app-of-apps syncs the ApplicationSets; a generated Application
that fails admission fails the sync task; the sync retries five times
and reports Failed. So app-of-apps sits OutOfSync/Degraded with zero
children and not one addon installs — no cert-manager, no external
secrets, no Cilium policies, no operator. The cluster comes up healthy,
ArgoCD runs, and the catalog is empty. Observed on a live cluster build,
where it stalled a tenant deploy waiting on a cert-manager that could
never arrive.
The dimension survives intact, rendered per-surface: the AWS tag keeps
`nanohype/eks-gitops` (a tag value may contain `/`), and the k8s label
carries `nanohype.eks-gitops`. Only the k8s surface changes — the
Repository tag on the Karpenter EC2NodeClass is untouched.
scripts/check-label-values.py applies the API server's own grammar to
every label value in the tracked YAML, so the manifest and the object it
becomes cannot disagree. It exists because nothing upstream could see
this: an ApplicationSet's spec.template.metadata.labels is
`additionalProperties: {type: string}` in the CRD schema, so kubeconform
admits any string; yamllint reads syntax; helm and kustomize render text
without submitting it; and `kyverno apply` evaluates policy rules rather
than API-server field validation. A valid manifest passed every gate and
broke on contact with the API server.
The gate checks values, not keys, and skips annotations — an annotation
value is unconstrained, which is where a genuine org/name slug belongs.
Inside a Kyverno policy it reads only the policy's own metadata.labels:
`spec.rules[].validate.pattern.metadata.labels` is a matcher over other
resources, where `?*` means "any non-empty value" and is correct. A value
that is entirely a Go template is skipped as unresolvable here, but the
literal text around a template is still checked, so a slash typed beside
a placeholder is caught.
Wired into the appsets CI job and `task validate`.
CI Results
One or more checks failed — see the job logs. |
The sync-wave gate classifies an ApplicationSet by its addons/<category>/ path, falling back to an explicit map for the ones with no such path. portal-tenants has neither, so the gate could not place it and reported a conflict. It had never needed to: opt-in/ is outside the applied set the gate reads, so an appset parked there is invisible to it, and moving one into the default path is what makes it answerable. Wave 100 is not an arbitrary large number and does not belong in any existing band. The manifests this appset applies declare Platform, Tenant, BudgetPolicy and the agent-plane CRs, so they need the operator's CRDs (ai-platform, 21) installed AND the operator itself running to reconcile them — which transitively means every other category. It is also the one appset whose content comes from outside this catalog, so its contents are not knowable when the wave is chosen. A band of its own at the end states that, where a slot inside apps (50-60) would imply it races the argo-platform workloads. Documented in CLAUDE.md alongside the other bands, which is where the gate's table says it reads them from.
CI Results
All checks passed. |
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.
Two changes, both found by a live cluster build. The first is why no cluster in the fleet could install a single addon.
1.
fix:the repository label value (the severe one)A Kubernetes label value is
[A-Za-z0-9]joined by-,_or., max 63 chars./is legal in a label key's prefix and in an AWS tag value — never in a label value. All 30 ApplicationSet templates stamped:So every Application the controller rendered was rejected at admission:
The blast radius is the whole catalog, not the one Application named. app-of-apps syncs the ApplicationSets; a generated Application that fails admission fails the sync task; the sync retries 5× and reports
Failed. Observed live:Zero children. No cert-manager, no external-secrets, no Cilium policies, no operator. The cluster is healthy, ArgoCD is running, and nothing is deployed.
The dimension survives, rendered per surface — AWS tag keeps
nanohype/eks-gitops, k8s label carriesnanohype.eks-gitops. TheRepositorytag on the Karpenter EC2NodeClass is untouched.Why every gate missed it
An ApplicationSet's
spec.template.metadata.labelsisadditionalProperties: {type: string}in the CRD schema — it is not a label until the controller renders an Application. So kubeconform admits any string, yamllint reads syntax, helm/kustomize render text without submitting it, andkyverno applyevaluates policy rules rather than API-server field validation.scripts/check-label-values.pyapplies the API server's own grammar to every label value in tracked YAML. Verified it fails closed by reintroducing the slash — 1 finding, correct file and path.It checks values not keys, skips annotations (unconstrained, and where an
org/nameslug belongs), reads only a Kyverno policy's ownmetadata.labels(spec.rules[].validate.pattern.metadata.labelsis a matcher, where?*is correct), and skips pure Go templates while still checking literal text beside a placeholder.Wired into the
appsetsCI job andtask validate.2.
feat:tenant boundary appset in the default installportal-tenantssat underapplicationsets/opt-in/, which app-of-apps sources withoutdirectory.recurseand never applies. So a cluster built withtenants_repo_urlset had the read-only deploy key for the tenants repo registered in ArgoCD and nothing installed that reads it — a credential provisioned for a consumer that does not exist.The reason it was withheld was real, but it was about naming an org, not about the appset. So the org is no longer named — both halves come from the cluster Secret, exactly as
gitops/repo-urlalready works:gitops/tenants-repo-url(annotation)portal/tenants-enabled(label)A cluster that has not opted in matches nothing in the clusters generator, so the child git generator never runs and the appset sits healthy with no children — nothing reaches for a repo it cannot see, which is the condition
opt-in/exists to avoid.The generators become a matrix (clusters first, git second — a matrix child that consumes parameters must come after the one that produces them). That ordering also buys routing the single git generator could not express: the discovered path is
tenants/<cluster_name>rather than atenants/*glob, and the destination is the matched cluster rather than a fixed in-cluster address. A glob crossed with the clusters generator would apply every cluster's tenants to every cluster.cluster_namecarries the full EKS cluster name (cluster-bootstrap wires it from the cluster component's output,<environment>-<name>), matching the directory the portal writes.missingkey=erroris kept, so a cluster labelled without its annotation fails to render rather than silently generating nothing. cluster-bootstrap stamps both under one condition alongside the deploy key, so they cannot arrive apart.Verification
yamllint, kubeconform 37/37, sync-wave ordering, fork-safety (now scanning 28 applied appsets incl. this one), and the new label-value gate all pass locally.https://claude.ai/code/session_012iMnbboJuiUMSvu7n8oRhz