fix(hack): stop sync-helm-crds.sh reviving dead manual CRDs - #3842
Merged
tekton-robot merged 2 commits intoAug 3, 2026
Conversation
hack/sync-helm-crds.sh still regenerated the manual per-component CRD copies under config/base/, config/kubernetes/base/, and config/openshift/base/ (300-operator_v1alpha1_<component>_crd.yaml). Commit 1ae0906 ("Cleanup manual CRDs and use generated CRDs in kustomize") deleted those files and pointed every kustomization.yaml at config/base/generated-crds/*.yaml directly, but never updated this script to match. Running `make sync-helm-crds` therefore silently resurrected ~4,700 lines of unreferenced manifests on every use. Drop the write_config_crd step and the now-unused BASE_DIR/K8S_DIR/ OPENSHIFT_DIR variables. The script now only assembles the Helm chart CRD bundles from config/base/generated-crds/, which remains the sole source of truth consumed by kustomize. Signed-off-by: Jawed khelil <jkhelil@redhat.com> Assisted-by: Claude Sonnet 4.5 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
4 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3842 +/- ##
=======================================
Coverage 25.46% 25.46%
=======================================
Files 449 449
Lines 23494 23494
=======================================
Hits 5982 5982
Misses 16823 16823
Partials 689 689
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Hub.Options in tektonconfig_types.go was missing the +optional marker that every sibling component's Options field has (Pipeline, Trigger, Chain, Dashboard, Addon, ManualApprovalGate, Pruner, Scheduler, MulticlusterProxyAAE, SyncerService). As a result, `make generate-crds` (verified by running it against unmodified main, independent of this branch) produces a `required: [options]` entry under spec.hub in the generated TektonConfig CRD — the file kustomize applies directly to clusters. That would reject any TektonConfig CR that does not explicitly set spec.hub.options, a breaking regression for existing users on upgrade. Add +optional and omitempty to match every other component, and regenerate config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml and charts/tekton-operator/templates/kubernetes-crds.yaml via `make sync-helm-crds`. The regeneration also picks up an unrelated but already-correct doc fix: the networkPolicy field description now mentions TektonPipeline alongside TektonTrigger as components with NetworkPolicy reconciliation implemented. charts/tekton-operator/templates/openshift-crds.yaml is intentionally left untouched here: it has independently drifted to bundle only 3 of the 13 CRDs it should, and re-syncing it pulls in a large, unrelated diff that deserves its own review. Signed-off-by: Jawed khelil <jkhelil@redhat.com> Assisted-by: Claude Sonnet 4.5 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced Aug 2, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enarha The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
|
/lgtm |
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
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.
Changes
1.
hack/sync-helm-crds.shstill regenerated dead manual CRD copieshack/sync-helm-crds.shstill regenerated the manual per-component CRDcopies under
config/base/,config/kubernetes/base/, andconfig/openshift/base/(300-operator_v1alpha1_<component>_crd.yaml).Commit 1ae0906 ("Cleanup manual CRDs and use generated CRDs in
kustomize") deleted those files and pointed every
kustomization.yamlat
config/base/generated-crds/*.yamldirectly, but never updatedthis script to match. As a result, running
make sync-helm-crds(adocumented prerequisite for any CRD-affecting API change) silently
resurrects ~4,700 lines of unreferenced, dead manifests on every use
— nothing in the repo consumes them (verified via
git grepacrossthe tree; the only reference was the script itself).
This surfaced concretely in
#3828, where an unrelated
OpenShiftPipelinesAsCodeNetworkPolicy change ballooned into a~8,900-line diff because the author correctly ran
make sync-helm-crdsand the stale script did the rest.This PR drops the dead
write_config_crdstep (and the now-unusedBASE_DIR/K8S_DIR/OPENSHIFT_DIRvariables) so the script onlydoes what's still needed: assembling the Helm chart CRD bundles
(
charts/tekton-operator/templates/{kubernetes,openshift}-crds.yaml)from
config/base/generated-crds/, which remains kustomize's solesource of truth.
2.
TektonConfig.spec.hub.optionswas implicitly requiredWhile investigating #1, I found that running
make generate-crdsonunmodified
main(independent of any other PR) produces arequired: [options]entry underspec.hubinconfig/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml—the file kustomize applies directly to clusters. That would reject
any
TektonConfigCR that doesn't explicitly setspec.hub.options,a breaking regression for existing users on upgrade.
Root cause:
Hub.Optionsintektonconfig_types.gowas the onlyOptions AdditionalOptionsfield across all components missing the// +optionalmarker (every other component — Pipeline, Trigger,Chain, Dashboard, Addon, ManualApprovalGate, Pruner, Scheduler,
MulticlusterProxyAAE, SyncerService — has it). Almost certainly an
oversight from the earlier "fix: options field should be optional in
all components" work.
Fixed by adding
+optional/omitemptyto match every othercomponent, then regenerating
operator.tekton.dev_tektonconfigs.yamland
charts/tekton-operator/templates/kubernetes-crds.yamlviamake sync-helm-crds. The regeneration also picks up an unrelated,already-correct doc fix: the
networkPolicyfield description nowmentions
TektonPipelinealongsideTektonTrigger.charts/tekton-operator/templates/openshift-crds.yamlisintentionally left untouched: it has independently drifted to bundle
only 3 of the 13 CRDs it should, and re-syncing it pulls in a large,
unrelated diff that deserves its own review/PR.
Verification
bash -n hack/sync-helm-crds.sh— syntax OKgo build ./pkg/...andgo vet ./pkg/apis/operator/v1alpha1/...— cleango test ./pkg/apis/operator/v1alpha1/...— no new failures (3pre-existing failures in
TestSetPACControllerDefaultSettings*remain, caused by an unrelated
pipelines-as-codev0.48.0→v0.48.1default
hub-catalog-typechange, already broken on unmodifiedmain)main: noconfig/*/base/*_crd.yamlfiles are recreated, and (before the hub.options fix)
kubernetes-crds.yamlregenerated byte-for-byte identical towhat's committed, confirming it was already in sync
Submitter Checklist
make test lintbefore submitting a PRRelease Notes
Made with Cursor