feat: add TektonConfig controller for pipeline security settings#214
feat: add TektonConfig controller for pipeline security settings#214cwiklik merged 1 commit intokagenti:mainfrom
Conversation
cwiklik
left a comment
There was a problem hiding this comment.
Well-implemented controller that moves TektonConfig patching from Ansible into the operator as a proper reconciler. The reconciliation logic is correct — uses MergeFrom (three-way merge patch) which preserves unmodeled fields, includes idempotency checks to avoid unnecessary patches, and handles missing CRD gracefully at startup. RBAC is minimal (only get/list/watch/patch). Test suite is strong with 6 Ginkgo cases covering the key scenarios. All CI checks pass.
Two minor points:
- The CRD discovery check is one-shot at startup — if Tekton is installed after the operator starts, a restart would be needed.
- The local types approach is correct but should document its version-pinning rationale.
Also: the PR body has Fixes # (empty) and ??? in the pruner description — looks like WIP notes left in.
Areas reviewed: Go (controller, types, tests), Helm RBAC, Kustomize RBAC, Security
Commits: 1 commit, signed-off: yes
CI status: All 5 checks passing
kagenti-operator/internal/controller/tektonconfig_controller.go
Outdated
Show resolved
Hide resolved
pdettori
left a comment
There was a problem hiding this comment.
Well-structured controller that cleanly moves TektonConfig patching from Ansible into the operator. The conditional CRD registration pattern, local type stubs avoiding the heavyweight Tekton dependency, and the needsPatch guard are all good design choices. Test coverage is solid with 6 cases covering the key branches.
The one-shot CRD detection at startup (no retry on transient API failure) was already noted in the previous review — acceptable for an initial implementation but worth a follow-up issue.
No must-fix issues found. All comments below are suggestions/nits.
Areas reviewed: Go (controller, types, tests), Helm RBAC, Kustomize RBAC, Security
Commits: 1 commit, signed-off: yes
CI status: All 5 checks passing
Move TektonConfig patching from the Ansible installer into the operator as a reconciler. The controller detects the TektonConfig CRD at startup via the discovery API and, if present, watches TektonConfig resources and ensures: - spec.pipeline.set-security-context is true (fsGroup for build pods) - spec.platforms.openshift.scc.default is "pipelines-scc" (HyperShift) - spec.pruner.resources is populated (required by newer Tekton webhooks) Includes typed local structs in internal/tekton to avoid importing the Tekton operator module, RBAC rules in both Kustomize and Helm configs, and six Ginkgo test cases covering patch, no-op, not-found, and per-field scenarios. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ignas Baranauskas <ibaranau@redhat.com>
Summary
Move TektonConfig patching from the Ansible installer into the operator as a reconciler. The controller detects the TektonConfig CRD at startup via the discovery API and, if present, watches TektonConfig resources and ensures:
Includes typed local structs in internal/tekton to avoid importing the Tekton operator module, RBAC rules in both Kustomize and Helm configs, and six Ginkgo test cases covering patch, no-op, not-found, and per-field scenarios.
Related issue(s)
(Optional) Testing Instructions
go test -v ./internal/controller/ --ginkgo.focus="TektonConfig" --ginkgo.vFixes #