[Alex Huang (wenhug)]

Summary

Makes a Helm chart the single source for the unbounded-operator manifests,
per the discussion with Jordan Olshevski (@jveski) and Philip Lombardi (@plombardi89). The static kubectl apply
manifests (deploy/unbounded-operator/rendered/, embedded in
kubectl unbounded install, and the release unbounded-operator-<tag>.yaml)
are now rendered from the chart at build time, so there is one definition to
maintain.

The immediate consumer of the chart is AKS, which wants to ship the operator as
a managed cluster extension for flex nodes (Extension Manager only installs
Helm charts).

Changes

• deploy/unbounded-operator/chart: ServiceAccount, ConfigMap, ClusterRole/Binding,
  registry-auth-migration Role/Binding, Deployment, and an optional Namespace
  (namespace.create, on only for the static render via values-static.yaml).
  Values: imageRegistry, image.name/tag/digest/reference, apiServerEndpoint,
  reapLegacyResources. Config hash is the same canonical JSON as install.
• deploy/unbounded-operator/*.yaml.tmpl removed.
• Chart templates keep the original file names (00-namespace.yaml …
  04-deployment.yaml), so make unbounded-operator-manifests is just
  helm template --output-dir + copy, the same pattern as gantry-manifests.
• values.schema.json keeps image and namespace strict but lets the root
  take extra keys, because AKS cluster extensions inject platform values
  (Azure.Cluster.*, IdentityClientId, DataBoundary, scope, ...).
• apiServerEndpoint is passed with --set-file in the Makefile so commas and
  other --set syntax survive verbatim, as with the old renderer.
• Each install path refuses to take over the other: the chart fails if
  unbounded-operator exists and isn't owned by the release (e.g. from
  kubectl unbounded install), and kubectl unbounded install fails if the
  operator Deployment or ClusterRoleBinding is owned by a Helm release.
• namespace.create is pinned to false in the schema; only the static render
  sets it (with --skip-schema-validation, like gantry), and
  values-static.yaml is excluded from the packaged chart, so a Helm release
  never owns the namespace the operator deploys components into.
• Tests: existing render tests only switch their render call to the chart;
  TestStaticAndHelmRenders pins the five files/eight objects and checks a Helm
  install renders the same objects minus the Namespace;
  TestChartAcceptsExtensionManagerValues covers the values Extension Manager injects; namespace drift check
  follows the gantry Makefile-default approach; make test passes the pinned
  bin/helm, and the chart tests fail (not skip) without helm.
• make unbounded-operator-chart-lint / -package; CLI reference documents the
  Helm path.

Validation

• rendered/ is object-for-object identical to the previous .yaml.tmpl
  output for default and custom (namespace/registry/endpoint/reap) values. The 6
  objects that existed in v0.21.0 match the published v0.21.0 release YAML; the
  2 extra are the Role/RoleBinding added in feat(gantry): managed authentication using acr-credential-provider  #1037.
• AKS 1.36.5, --network-plugin none, real flex node attached:
  • helm install --set image.tag=v0.21.0: operator and all components at
    v0.21.0, both nodes Ready, 80/80 cross-node requests OK during the swap;
    helm upgrade of apiServerEndpoint rolls the operator.
  • Guard blocked install over an operator owned by another release.
• Clean AKS: kubectl unbounded install built from this branch applied all 8
  objects, operator Ready, 13 CRDs; the Helm guard then refused to install over
  it. kubectl apply of the branch-built release YAML also works.
• AKS Extension Manager (fresh AKS, this chart under an internal test
  extension type): create, config update (operator rolls) and delete all
  succeed; kubectl unbounded install refuses to touch the extension's
  operator; after delete the namespace and a workload placed in it are still
  there, only the operator, its RBAC and ConfigMap are removed.
• Earlier: an in-place v0.8.0 → v0.18.0 upgrade with a flex node attached.

Notes for reviewers

• Every job that renders operator manifests now needs helm ($(HELM) is
  downloaded on demand, as for gantry-manifests).
• helm template adds no labels today, which is why static and Helm output
  match. Adding Helm-specific labels to the chart would change the static
  manifests too.
• Not in this PR: publishing the chart in release.yaml (like gantry-chart)
  and cleanup of CRDs/Sites on uninstall.

🤖 Generated with Claude Code

[Philip Lombardi (plombardi89)]

Thanks for this. Making the chart the single source works well: the static and Helm renders match, the config hash is computed the same way as install's, and --set-file keeps commas in the endpoint intact.

Requesting changes for three problems where kubectl unbounded install, helm uninstall or the AKS extension can overwrite or delete a working install. Each inline comment has steps to reproduce. I reproduced the first two on kind with Helm v3.21.3 and a kubectl-unbounded built from this branch (v0.22.0-5-gc70727b3c). The third is a question about Extension Manager.

Blocking

1. kubectl unbounded install silently takes over a Helm-managed operator (inline on docs/content/reference/cli.md).
2. namespace.create=true on a Helm install leads to helm uninstall deleting the namespace and every component in it (inline on values.schema.json).
3. Question: does deleting the AKS extension delete the namespace? (inline on NOTES.txt).

Suggestion

• Fail instead of skip when helm is missing (inline on chart_test.go).

[Philip Lombardi (plombardi89)]

Blocking: kubectl unbounded install silently takes over a Helm-managed operator

The chart refuses to install over a CLI install, but not the other way around. install server-side applies every object with ForceOwnership and never checks for Helm ownership (cmd/kubectl-unbounded/app/install.go:161, :774). So this line overstates what's enforced.

I reproduced both cases below on kind with Helm v3.21.3 and kubectl-unbounded built from this branch (v0.22.0-5-gc70727b3c).

Same namespace: the operator version flips back and forth

1. helm install unbounded-operator ./deploy/unbounded-operator/chart -n unbounded-system --create-namespace --set image.tag=v0.21.0
2. kubectl unbounded install --wait=false succeeds with no warning.
3. The Deployment image is now unbounded-operator:v0.22.0-5-gc70727b3c, and kubectl-unbounded owns the fields. The operator deploys components at its own version (install.go:304-306), so on a real cluster the components move to that version too.
4. helm upgrade with the same values sets the image back to v0.21.0.
5. kubectl unbounded install again sets it back to v0.22.0-5-gc70727b3c. This repeats every time either tool runs, and an AKS extension update counts as a helm upgrade.

Different namespaces: the two operators take permissions from each other

1. helm install unbounded-operator ./deploy/unbounded-operator/chart -n flex-system --create-namespace --set image.tag=v0.21.0
2. kubectl unbounded install --wait=false (defaults to unbounded-system) succeeds.
3. There are now two operator Deployments, and the only subject of the cluster-wide unbounded-operator ClusterRoleBinding is unbounded-system/unbounded-operator.
4. kubectl auth can-i list nodes --as=system:serviceaccount:flex-system:unbounded-operator returns no. The Helm-managed operator has lost its permissions.
5. helm upgrade points the binding back at flex-system. Now the CLI-installed operator has no permissions.

Suggested fix: before applying, have install read the unbounded-operator Deployment in the target namespace and the unbounded-operator ClusterRoleBinding. If either has the meta.helm.sh/release-name annotation, fail with a message saying the operator is managed by Helm or the AKS extension. If you'd rather do this in a follow-up, please open an issue and reword this line so it doesn't read as if both directions are guarded.

[Alex Huang (wenhug)]

Good catch, the doc only held one way. install now checks the operator Deployment in the target namespace and the unbounded-operator ClusterRoleBinding before applying anything, and bails out if either has meta.helm.sh/release-name.

Ran both your cases on AKS with a CLI built from the branch:

• same namespace: install errors out, image stays at v0.21.0
• flex-system + default namespace: errors on the ClusterRoleBinding, binding still points at flex-system and that SA can still list nodes

Unit tests cover both, and a plain reinstall over a CLI install still works. Reworded the doc so it says what each side actually refuses.

[Philip Lombardi (plombardi89)]

Blocking: namespace.create=true on a Helm install leads to helm uninstall deleting the namespace

The schema accepts namespace.create: true for Helm installs, and values-static.yaml ships in the chart. Helm rejects that install, but it leaves behind a failed release whose manifest includes the Namespace. From there, either a retry or an uninstall deletes the namespace and everything in it. The operator deploys every component into this namespace, including unbounded-net (internal/operator/component/namespace.go:11-12).

I reproduced both paths below on kind with Helm v3.21.3.

Path 1: retry

1. helm install op ./deploy/unbounded-operator/chart -n ns-a --create-namespace --set namespace.create=true --set image.tag=v0.21.0
2. It fails with namespaces "ns-a" already exists. However, the ServiceAccount, RBAC, ConfigMap and Deployment are all created, and the release is recorded as failed.
3. To simulate a component deployed by the operator: kubectl -n ns-a create configmap component-sentinel --from-literal=x=1
4. Retry with helm upgrade --install and the same flags. This succeeds, and the namespace now has app.kubernetes.io/managed-by: Helm and the release annotations.
5. helm uninstall op -n ns-a puts the namespace into Terminating, and component-sentinel is deleted with it.

Path 2: give up and uninstall

1. Run steps 1-3 from path 1.
2. helm uninstall op -n ns-a also deletes the namespace, even though --create-namespace namespaces are normally left behind by helm uninstall.

Suggested fix: follow the gantry chart. Pin "create": {"const": false} in the schema (deploy/gantry/chart/values.schema.json:55-65) and render the static profile with --skip-schema-validation (Makefile:928). Alternatively, add helm.sh/resource-policy: keep to the Namespace template. A .helmignore that excludes values-static.yaml from the packaged chart would also help.

[Alex Huang (wenhug)]

Went with the gantry approach: create is const: false in the schema, the static render uses --skip-schema-validation, and .helmignore keeps values-static.yaml out of the packaged chart. --set namespace.create=true now fails validation before Helm records a release or creates anything, so there's nothing left behind to retry or uninstall. Also added it to TestChartRejectsInvalidValues.

[Philip Lombardi (plombardi89)]

Blocking (question): does deleting the AKS extension delete the namespace?

This note and cli.md say that uninstalling leaves the components in place. The operator deploys all of them, including the unbounded-net CNI, into the release namespace (internal/operator/component/namespace.go:11-12).

1. Create the AKS extension. Extension Manager installs the chart into its release namespace.
2. Run kubectl unbounded site init. The operator deploys unbounded-net, machina and the other components into that same namespace.
3. Delete the extension.
4. Extension Manager runs helm uninstall, which removes the operator, its RBAC and its ConfigMap. This part is expected.
5. Unknown: if Extension Manager then deletes the release namespace, Kubernetes deletes everything in it, including the CNI. I'd expect that to break networking for attached nodes.

In your create/update/delete test through Extension Manager, were the namespace and the components still there after the delete? If the namespace is deleted, please either make the extension keep it, or update this note and cli.md to say that removing the extension removes every component.

[Alex Huang (wenhug)]

No. In the Extension Manager test (create, config update, delete) the namespace was still Active after the delete. Only the operator Deployment, SA, ConfigMap and ClusterRole/Binding went away, and the 12 CRDs stayed. Same result with an earlier minimal chart in a different namespace.

Caveat: that run didn't have Sites, so nothing else was living in the namespace. With the namespace kept, I'd expect unbounded-net etc. to survive too, but I haven't re-run it with Sites. Happy to do that before merge if you want it confirmed. Updated NOTES and cli.md to say the namespace and the components in it are left in place.

[Philip Lombardi (plombardi89)]

Suggestion: fail instead of skip when helm is missing

Before this PR, the operator render tests had no outside dependencies and always ran. With t.Skip here, a missing helm turns all of them into skips:

1. Use a fresh checkout with no helm on PATH and no bin/helm (i.e. make has never been run).
2. Change the chart in a way that breaks a template, for example the RBAC rules.
3. Run go test ./deploy/unbounded-operator/....
4. Go prints ok for the package. All 9 helm-backed tests (RBAC, lease scope, config hash, registry, ...) are skipped, and the skips only show with -v.
5. The developer sees a pass and pushes, and only CI catches the breakage.

Suggest t.Fatal("helm not found; run make install-helm or set HELM"), which matches deploy/gantry/render_test.go:760-762 and internal/gantry/origin/configmap_yaml_test.go:54-56.

[Alex Huang (wenhug)]

Done, t.Fatal now, same as gantry.

[Alex Huang (wenhug)]

Thanks for digging into this, and for the repro steps. All four are fixed in d7efa13 (kept as a separate commit so it's easy to diff). Details inline.

Unrelated, but I hit it while testing: on a clean AKS cluster, kubectl unbounded install times out in waitForCRDs with --version v0.22.0, on main as well as this branch. Main now requires machineclasses and machineallocations (#1054), but the published v0.22.0 operator doesn't install them yet, so it should go away with the next operator release. Just mentioning it in case someone else runs into it.

[Alex Huang (wenhug)]

Follow-up after running the whole thing through Extension Manager on a fresh AKS cluster:

• The first install failed schema validation. Besides Azure.Cluster.*, Extension Manager also passes IdentityClientId, IdentityPrincipalId, IdentityTenantId, IdentityType, IsWorkloadIdentityEnabled, DataBoundary and scope at the root. 6fac94c lets the root take extra keys; image and namespace are still strict, and namespace.create is still pinned to false. The test now uses the exact keys EM sent.
• With that, create / config update / delete through EM all work. The config update with a comma in apiServerEndpoint came through intact and rolled the operator, and kubectl unbounded install refused to touch the extension's operator.
• On your NOTES question: this time I put a ConfigMap and a running Deployment in the namespace before deleting the extension. After the delete the namespace was still Active and both were still there; only the operator, its RBAC and ConfigMap went away, and the CRDs stayed.