Skip to content

feat: add TektonConfig controller for pipeline security settings#214

Merged
cwiklik merged 1 commit intokagenti:mainfrom
Ygnas:tekton-patch
Mar 11, 2026
Merged

feat: add TektonConfig controller for pipeline security settings#214
cwiklik merged 1 commit intokagenti:mainfrom
Ygnas:tekton-patch

Conversation

@Ygnas
Copy link
Contributor

@Ygnas Ygnas commented Mar 11, 2026

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:

  • 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.

Related issue(s)

(Optional) Testing Instructions

go test -v ./internal/controller/ --ginkgo.focus="TektonConfig" --ginkgo.v

Fixes #

@Ygnas Ygnas requested a review from a team as a code owner March 11, 2026 11:42
Copy link
Collaborator

@cwiklik cwiklik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The CRD discovery check is one-shot at startup — if Tekton is installed after the operator starts, a restart would be needed.
  2. 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

Copy link
Contributor

@pdettori pdettori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@Ygnas Ygnas requested review from cwiklik and pdettori March 11, 2026 15:37
@cwiklik cwiklik merged commit f49ff3f into kagenti:main Mar 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants