DRAFT: Changes based on make helm-generate

[Lytol]

I noticed some fields missing from the Site CRD (specifically, sessionTolerations in this instance) while trying to update an existing environment. Particularly, config/crd/bases is out-of-sync with dist/chart/templates/crd, and it appears that there is drift in general between the Helm chart CRDs and the underlying base CRDs. From what I can gather, we use make helm-generate to keep these in sync, but it isn't reflected in any of the developer workflows or CI.

After running make helm-generate, there is a very large diff (captured in this draft PR) -- and it also appears to want to setup additional GH action workflows, etc. Is make helm-generate supposed to be just a one-time thing? How should we be keeping the Helm CRDs and the base CRDs in sync otherwise?

[claude]

Claude finished @Lytol's task —— View job

---

PR Review

• Read review guidelines
• Get PR diff and analyze changes
• Analyze helm-generate output and CI workflow implications

---

Summary

This PR captures the output of make helm-generate which uses the kubebuilder edit --plugins=helm.kubebuilder.io/v1-alpha command to regenerate the Helm chart from kustomize manifests. The changes reveal significant drift between config/crd/bases/ and dist/chart/templates/crd/.

Key Observations

Positive Changes:

1. CRD schema updates are correct - The sessionTolerations field (and flightdeck.enabled) now appear in the Helm chart CRDs, matching the base CRDs in config/crd/bases/. This addresses the original issue.
2. Affinity field fix - The affinity field in Connects/Workbenches CRDs was incorrectly typed as an array. The regeneration correctly changes it to a single Affinity object matching the Kubernetes API.

Concerns:

1. .github/workflows/test-chart.yml - This appears to be auto-generated boilerplate from kubebuilder. This repo already has comprehensive CI in build.yml (lines 106-110) that runs helm-lint and helm-template. The new workflow is largely redundant and incomplete (commented-out sections). Suggestion: Remove this file and document the expected workflow in CI instead.

2. Hardcoded namespace in role.yaml (line 26):

   namespace: posit-team

   Previously used {{ .Values.watchNamespace }}. This change loses flexibility for deployments using different namespaces.

3. Removal of resourcePolicy.keep annotations - Many RBAC resources had conditional helm.sh/resource-policy: keep annotations that are now removed. This changes helm uninstall behavior.

4. Removed webhook conversion configuration - The CRDs no longer include webhook conversion strategy blocks. Verify this is intentional if webhooks may be enabled in the future.

5. RoleBinding namespace comment (role_binding.yaml:21-24) references pulumi patching:

   # NOTE: The namespace will be set via kustomize to `posit-team-system` but we need this
   # role binding to be in the `posit-team` "watch namespace". I haven't figured out how to
   # do this directly in the kustomize layer, but we have an opportunity to patch arbitrary
   # things in pulumi, so that's where this will get patched before apply. ~ @meatballhat

   This comment suggests the templating relies on external patching which may not happen for all deployment methods.

Recommendations

1. Don't merge as-is - This is a DRAFT PR appropriately capturing the drift. The changes need selective review.

2. Establish a CRD sync workflow:

   • Add make helm-generate to the mgenerate target or create a dedicated target
   • Add a CI check that fails if make helm-generate produces a diff (like the existing git diff --exit-code in build.yml:112-115)

3. Cherry-pick needed changes:

   • The CRD schema updates (sessionTolerations, flightdeck.enabled, affinity type fix) should be applied
   • The RBAC and workflow changes need more careful consideration

4. Remove test-chart.yml - It duplicates existing CI and has incomplete configuration.

---