From 7c67b2b5d2482f04a773006b0fe37fdc39317439 Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 09:25:19 +0200 Subject: [PATCH 1/7] feat(namespacesync): add NamespaceSyncController for per-namespace resource management Add a new watch-based NamespaceSyncController that replaces the scan-based per-namespace reconciliation loop in rbac.go. The new controller uses Kubernetes informers to react to namespace, ServiceAccount, Secret, and TektonConfig events, eliminating O(N) API server calls on every TektonConfig reconcile. New typed API in TektonConfig.spec.platforms.openShift.namespaceSync: - createPipelineSA: manage the pipeline ServiceAccount (replaces createRbacResource) - createCABundles: manage CA bundle ConfigMaps (replaces createCABundleConfigMaps) - createEditRoleBinding: manage openshift-pipelines-edit RoleBinding (replaces legacyPipelineRbac) - createSCCRoleBinding: manage pipelines-scc-rolebinding (defaults true) - secretBindings: bind registry secrets (by name or label selector) to the pipeline SA Key behaviors: - Pipeline SA self-heals on add/update/delete events - Named secret bindings are removed from the pipeline SA when the secret is deleted - Label-based bindings re-evaluate on every secret add/delete event - TektonConfig updates only trigger a full namespace sweep when the NamespaceSync config itself changes (reflect.DeepEqual guard prevents thundering herd) - Legacy spec.params entries are migrated to the typed fields on first reconcile Design doc: docs/plans/2026-06-26-namespace-sync-controller-design.md Signed-off-by: Abdeljaouad Khelil Assisted-by: Claude Sonnet 4.6 Co-authored-by: Cursor --- ...-06-26-namespace-sync-controller-design.md | 373 +++++++++ .../operator/v1alpha1/openshift_platform.go | 70 ++ .../v1alpha1/tektonconfig_defaults.go | 49 ++ .../v1alpha1/tektonconfig_validation.go | 26 +- .../v1alpha1/zz_generated.deepcopy.go | 70 ++ .../openshift/namespacesync/controller.go | 205 +++++ .../openshift/namespacesync/reconciler.go | 709 ++++++++++++++++++ .../namespacesync/reconciler_test.go | 679 +++++++++++++++++ .../openshift/openshiftplatform/config.go | 5 + pkg/reconciler/platform/const.go | 4 + 10 files changed, 2189 insertions(+), 1 deletion(-) create mode 100644 docs/plans/2026-06-26-namespace-sync-controller-design.md create mode 100644 pkg/reconciler/openshift/namespacesync/controller.go create mode 100644 pkg/reconciler/openshift/namespacesync/reconciler.go create mode 100644 pkg/reconciler/openshift/namespacesync/reconciler_test.go diff --git a/docs/plans/2026-06-26-namespace-sync-controller-design.md b/docs/plans/2026-06-26-namespace-sync-controller-design.md new file mode 100644 index 0000000000..558083f925 --- /dev/null +++ b/docs/plans/2026-06-26-namespace-sync-controller-design.md @@ -0,0 +1,373 @@ +# NamespaceSyncController Design + +**Date:** 2026-06-26 +**Author:** Jawed Khelil +**Related:** [RFE-7814](https://redhat.atlassian.net/browse/RFE-7814) · [SRVKP-11482](https://redhat.atlassian.net/browse/SRVKP-11482) +**Status:** Draft + +--- + +## Problem + +When a namespace is created in OpenShift, the platform immediately creates default service +accounts (`default`, `builder`, `deployer`). The Quay Bridge Operator hooks into namespace +creation and binds Quay robot secrets to those SAs. + +The `pipeline` SA — the default SA used by all Tekton PipelineRuns — is **not** an +OpenShift-default SA. It is created by the Tekton Operator's RBAC reconciler, which runs +after the namespace is ready. This means: + +1. Quay Bridge has already run its binding pass before `pipeline` SA exists. +2. The RBAC reconciler stamps a `namespace-reconcile-version` label after its first pass and + **never re-visits that namespace again** (until the next operator upgrade). +3. Any secret that arrives in a namespace after the first RBAC reconcile pass is silently + missed — the `pipeline` SA never gets the binding, and PipelineRuns cannot push or pull + images from Quay. + +Users are currently forced to either manually bind secrets per namespace or grant elevated +SCCs to the `builder` SA, both of which introduce operational overhead and security risk at +scale. + +--- + +## Goals + +- Automatically bind configured secrets to the `pipeline` SA in every namespace. +- Handle both orderings: `pipeline` SA created before the secret, and secret created before + `pipeline` SA. +- Handle secret rotation and deletion without manual intervention. +- No coupling to Quay Bridge Operator internals — work with any registry operator or secret + source. +- Provide typed API fields (not stringly-typed `spec.params`) for cluster admins to declare + binding intent. +- Replace the scan-based namespace reconciliation pattern with a watch-based reactive pattern + that eliminates O(N) API calls per reconcile cycle. + +## Non-Goals + +- Changes to the Quay Bridge Operator codebase (out of scope for this operator). +- Automatic discovery of registry secrets without admin configuration. +- Managing image pull secrets for any SA other than `pipeline`. + +--- + +## Background: Current RBAC Reconciler Limitations + +The existing reconciler in `pkg/reconciler/openshift/tektonconfig/rbac.go` (1540 lines) +implements a **scan-based batch job** pattern: + +``` +TektonConfig reconciled + └── rbac.Run() + └── kubeClientSet.CoreV1().Namespaces().List() ← direct API call, O(N) + └── for each namespace: + needsRBAC? → processRBAC (SA, SCC, RoleBinding) + needsCABundle? → ensureCABundles + stamp namespace-reconcile-version label → skip forever +``` + +### Problems with this approach + +| Problem | Impact | +|---|---| +| `List(all namespaces)` is a direct API server call on every TektonConfig reconcile | O(N) API calls; an existing `nsInformer` is available but unused for this path | +| Version label skip: namespaces are processed once per operator release | Late-arriving secrets never get bound until the next upgrade | +| All namespace concerns (SA, SCC, RoleBinding, CA bundles) in one 1540-line file | High coupling, hard to extend | +| `needsRBAC` self-healing check still calls `RoleBindings.Get()` per reconciled namespace | N additional API calls per TektonConfig reconcile in steady state | +| Namespace params (`createRbacResource`, `createCABundleConfigMaps`) are stringly-typed in `spec.params[]` | No type safety, no validation, hard to document | + +### What the version label actually does + +The label `openshift-pipelines.tekton.dev/namespace-reconcile-version: ` serves two +purposes: + +1. **Skip optimization** — avoid re-processing already-reconciled namespaces on each + TektonConfig reconcile cycle. +2. **Upgrade propagation** — when `VERSION` changes on operator upgrade, the label no longer + matches and all namespaces are re-reconciled, picking up any new SA or RBAC changes. + +In a watch-based controller, both purposes are satisfied differently: +- Skip optimization: not needed — events only fire for the affected namespace. +- Upgrade propagation: not needed — operator pod restart (which always happens on upgrade) + causes informers to perform a full `List`, re-enqueuing every namespace. + +--- + +## Proposed Solution: `NamespaceSyncController` + +A single new watch-based controller — `NamespaceSyncController` — inside the Tekton Operator +(OpenShift platform only). Its reconcile unit is the **namespace**. It owns all +namespace-level synchronisation concerns, replacing the scan-based pattern over time. + +### Architecture + +``` +NamespaceSyncController +│ +├── Watch: Namespace (create, delete) +│ → enqueue namespace name +│ +├── Watch: ServiceAccount (field selector metadata.name=pipeline, create, delete) +│ → enqueue owning namespace +│ +├── Watch: Secret (label selector from TektonConfig.spec.platforms.openshift.namespaceSync, +│ create, delete, update) +│ → enqueue owning namespace +│ +├── Watch: RoleBinding pipelines-scc-rolebinding (delete) +│ → enqueue owning namespace +│ +└── Watch: ConfigMap config-trusted-cabundle / config-service-cabundle (delete) + → enqueue owning namespace + +ReconcileNamespace(ns string): + if !config.namespaceSync.createPipelineSA → skip + ensurePipelineSA(ns) + + if !config.namespaceSync.createCABundles → skip + ensureCABundles(ns) + + ensureSCCRoleBinding(ns) + + if config.namespaceSync.createEditRoleBinding → ensureEditRoleBinding(ns) + else → removeEditRoleBindingIfPresent(ns) + + for each binding in config.namespaceSync.secretBindings: + ensureSecretBinding(ns, binding) ← new, for this RFE +``` + +All event handlers enqueue the same thing — a namespace name — into a single work queue. +The reconciler receives a namespace name and ensures all desired state for that namespace +in one idempotent pass. + +### Why one controller, not many + +A single controller with multiple watches is the standard Kubernetes controller pattern and +avoids multiplying controller overhead (leader election, work queues, goroutines). Adding a +new sync concern means: add one watch + one `ensure*` call. The controller's scope is +well-defined: everything that must be true about a namespace for Tekton to function. + +### Performance comparison + +| | Current scan-based (rbac.go) | NamespaceSyncController | +|---|---|---| +| **Reads at startup** | None (lazy, on first TektonConfig reconcile) | 1× List per watched resource type (populates cache) | +| **Reads in steady state** | O(N) API calls per TektonConfig event | 0 API calls (served from informer cache) | +| **Reconcile scope per event** | All N namespaces | 1 namespace | +| **Memory overhead** | nsInformer already held | +Secret + SA filtered caches (~3–4 MB per 1000 ns) | +| **Upgrade propagation** | Version label comparison | Operator restart re-enqueues all namespaces | +| **Late-arriving resources** | Missed until next upgrade | Reactive: event fires, namespace reconciled immediately | + +--- + +## Secret Binding Logic + +### Handling late-arriving secrets + +Two scenarios must work correctly: + +**Scenario A — `pipeline` SA created before secret:** +1. Namespace created → `pipeline` SA created → `ensureSecretBinding` runs, finds no secret + matching the configured selector → skips (no loop). +2. Later, secret is created in namespace → Secret watch fires → namespace enqueued → + `ensureSecretBinding` finds secret, binds it to `pipeline` SA. + +**Scenario B — Secret created before `pipeline` SA:** +1. Namespace created → secret created → SA watch fires for `pipeline` SA... but SA doesn't + exist yet → `ensureSecretBinding` creates the SA then binds. + _Or_: SA watch fires when the `pipeline` SA is later created by `ensurePipelineSA` → + namespace re-enqueued → binding completes. + +**Secret deletion / rotation:** +- Secret deleted → Secret watch fires → namespace enqueued → `ensureSecretBinding` removes + stale `imagePullSecrets` and `secrets` references from `pipeline` SA. +- Secret recreated (rotation) → Secret watch fires → namespace enqueued → new secret bound. + +### Avoiding infinite reconcile loops + +The binding check distinguishes two states: + +| State | Action | +|---|---| +| Secret **does not exist** in this namespace | Skip — nothing to bind (namespace has no Quay integration) | +| Secret **exists** but **not bound** to `pipeline` SA | Reconcile — bind it | +| Secret **exists** and **already bound** | No-op — idempotent pass | + +Only namespaces that have the secret but are missing the binding are ever re-reconciled. +Namespaces with no Quay integration (secret absent) are never looped on. + +### Concurrent writes + +The existing RBAC reconciler also writes to the `pipeline` SA (owner references). The +`NamespaceSyncController` must use `RetryOnConflict` when patching the SA to handle +concurrent updates without data loss. + +--- + +## API Changes + +### New typed field: `spec.platforms.openshift.namespaceSync` + +`pkg/apis/operator/v1alpha1/openshift_platform.go`: + +```go +type OpenShift struct { + PipelinesAsCode *PipelinesAsCode `json:"pipelinesAsCode,omitempty"` + SCC *SCC `json:"scc,omitempty"` + EnableCentralTLSConfig *bool `json:"enableCentralTLSConfig,omitempty"` + // NamespaceSync controls namespace-level synchronisation performed by the + // Tekton Operator in each user namespace on OpenShift. + // +optional + NamespaceSync *NamespaceSyncConfig `json:"namespaceSync,omitempty"` +} + +// NamespaceSyncConfig configures what the NamespaceSyncController ensures in each namespace. +type NamespaceSyncConfig struct { + // CreatePipelineSA controls whether the pipeline service account is created in each + // namespace. Defaults to true. + // +optional + CreatePipelineSA *bool `json:"createPipelineSA,omitempty"` + + // CreateCABundles controls whether CA bundle ConfigMaps are injected into each namespace. + // Defaults to true. + // +optional + CreateCABundles *bool `json:"createCABundles,omitempty"` + + // CreateEditRoleBinding controls whether a RoleBinding named openshift-pipelines-edit + // is created in each namespace, binding the pipeline SA to the built-in edit ClusterRole. + // This gives the pipeline SA broad write permissions within its namespace, which is + // convenient for PipelineRuns that create or update resources. Set to false for + // least-privilege environments. + // Defaults to true (preserves historical behaviour). + // Replaces the legacy spec.params entry legacyPipelineRbac. + // +optional + CreateEditRoleBinding *bool `json:"createEditRoleBinding,omitempty"` + + // SecretBindings declares secrets that should be bound to the pipeline SA in each + // namespace. Each binding matches secrets by label selector or by name. When a matching + // secret appears in a namespace, it is added to both imagePullSecrets and secrets on the + // pipeline SA automatically. + // +optional + SecretBindings []SecretBinding `json:"secretBindings,omitempty"` +} + +// SecretBinding describes a secret (or class of secrets) that should be bound to the +// pipeline SA. Exactly one of LabelSelector or SecretName must be set. +type SecretBinding struct { + // LabelSelector selects secrets by label. All secrets matching this selector in a + // given namespace are bound to the pipeline SA. + // +optional + LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"` + + // SecretName binds a specific named secret in each namespace to the pipeline SA. + // +optional + SecretName string `json:"secretName,omitempty"` +} +``` + +### Example CR + +```yaml +apiVersion: operator.tekton.dev/v1alpha1 +kind: TektonConfig +metadata: + name: config +spec: + platforms: + openshift: + namespaceSync: + createPipelineSA: true + createCABundles: true + createEditRoleBinding: true # set to false for least-privilege environments + secretBindings: + # Bind any secret labeled with quay.io/robot-token=true (set by Quay Bridge Operator) + - labelSelector: + matchLabels: + quay.io/robot-token: "true" + # Or bind a specific named secret across all namespaces + - secretName: my-registry-pullsecret +``` + +### Migration from `spec.params` + +The existing `spec.params` entries (`createRbacResource`, `createCABundleConfigMaps`) are +deprecated in favour of the new typed fields. `SetDefaults` will read the old params, populate +the new typed fields, and emit a deprecation warning. The old params remain functional through +one release cycle before removal. + +| Old `spec.params` entry | New typed field | Default | +|---|---|---| +| `createRbacResource: "false"` | `namespaceSync.createPipelineSA: false` | `true` | +| `createCABundleConfigMaps: "false"` | `namespaceSync.createCABundles: false` | `true` | +| `legacyPipelineRbac: "false"` | `namespaceSync.createEditRoleBinding: false` | `true` | + +--- + +## Implementation Plan + +### Phase 1 — API (no behaviour change) + +1. Add `NamespaceSyncConfig` and `SecretBinding` types to `openshift_platform.go`. +2. Add `SetDefaults` migration for old `spec.params`. +3. Add `Validate` rules: at most one of `LabelSelector`/`SecretName` per binding; both empty + is an error. +4. Run `./hack/update-codegen.sh` and commit generated files. + +### Phase 2 — NamespaceSyncController (new file, does not touch rbac.go) + +5. Create `pkg/reconciler/openshift/namespacesync/` package: + - `controller.go` — registers informers and work queue. + - `reconciler.go` — `ReconcileNamespace`: `ensurePipelineSA`, `ensureCABundles`, + `ensureSCCRoleBinding`, `ensureSecretBindings`. +6. Move existing `ensureSA`, `ensureCABundles`, `ensureSCCRole` logic from `rbac.go` into + the new reconciler (copy first, validate parity, then remove from `rbac.go` in Phase 3). +7. Register the new controller in `cmd/openshift/operator/main.go`. + +### Phase 3 — Transition (parallel operation → rbac.go removal) + +8. Run both controllers in parallel for one release cycle. `rbac.go` continues to stamp the + version label; `NamespaceSyncController` handles events reactively. +9. Verify feature parity and confirm no regressions in E2E. +10. Remove `rbac.go` scan-based logic; retire the `namespace-reconcile-version` label. + +### Phase 4 — Secret binding feature + +11. Add `ensureSecretBindings` step in `ReconcileNamespace`. +12. Add filtered Secret informer in `controller.go` using the label selectors from + `TektonConfig.Spec.Platforms.OpenShift.NamespaceSync.SecretBindings`. +13. Unit tests: SA-before-secret, secret-before-SA, secret rotation, deletion cleanup, + no-secret namespace (no infinite loop), concurrent write conflict. +14. E2E test: namespace creation → Quay Bridge sync → `pipeline` SA binding verified → + successful PipelineRun pushing to Quay. + +--- + +## Open Questions + +| # | Question | Status | +|---|---|---| +| 1 | Does Quay Bridge Operator label robot secrets with `quay.io/robot-token=true`? This label is required for the `labelSelector` binding to work without a secret name. | **Pending — confirm with Quay Bridge team once cluster is available** | +| 2 | Should the target SA name (`pipeline`) be hardcoded or made configurable via `NamespaceSyncConfig`? | Hardcoded for this iteration; can be added later. | +| 3 | What happens if a namespace has multiple matching secrets (e.g. one robot token per Quay org)? | All matching secrets are bound to `pipeline` SA. | +| 4 | Is there a cleanup requirement when OpenShift Pipelines is uninstalled? | `FinalizeKind` removes controller-created bindings from all `pipeline` SAs. | +| 5 | Should `secretBindings` also bind to `imagePullSecrets`, or only to `secrets`? | Both, matching the pattern used by Quay Bridge for `builder` SA. | + +--- + +## Dependencies + +- Quay Bridge Operator must label robot secrets with a stable label (e.g. `quay.io/robot-token=true`) + for `labelSelector`-based bindings to work automatically. Without this, admins must use + `secretName` bindings instead. +- The Tekton Operator's `SharedInformerFactory` must support filtered informer registration + for Secrets and ServiceAccounts without breaking existing watchers. + +--- + +## References + +- [RFE-7814 — Flexible service account binding for Quay Robot credentials in OpenShift Pipelines](https://redhat.atlassian.net/browse/RFE-7814) +- [SRVKP-11482 — Automatic Quay Robot Secret Binding to Pipeline SA via Namespace-Scoped Controller](https://redhat.atlassian.net/browse/SRVKP-11482) +- `pkg/reconciler/openshift/tektonconfig/rbac.go` — existing RBAC reconciler (patterns to reuse) +- `pkg/apis/operator/v1alpha1/openshift_platform.go` — API types to extend +- `pkg/reconciler/openshift/tektonconfig/controller.go` — informer registration pattern diff --git a/pkg/apis/operator/v1alpha1/openshift_platform.go b/pkg/apis/operator/v1alpha1/openshift_platform.go index daa9b8b0a2..6c629ac56b 100644 --- a/pkg/apis/operator/v1alpha1/openshift_platform.go +++ b/pkg/apis/operator/v1alpha1/openshift_platform.go @@ -16,6 +16,8 @@ limitations under the License. package v1alpha1 +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + type OpenShift struct { // PipelinesAsCode allows configuring PipelinesAsCode configurations // +optional @@ -32,6 +34,74 @@ type OpenShift struct { // Default: true (opt-out) // +optional EnableCentralTLSConfig *bool `json:"enableCentralTLSConfig,omitempty"` + // NamespaceSync controls what the Tekton Operator synchronises into each + // user namespace on OpenShift (pipeline SA, CA bundles, edit RoleBinding, + // and registry secret bindings). + // +optional + NamespaceSync *NamespaceSyncConfig `json:"namespaceSync,omitempty"` +} + +// NamespaceSyncConfig configures the NamespaceSyncController which watches +// user namespaces and ensures Tekton-required resources are present and up to date. +// All boolean fields default to true when the NamespaceSync block is present. +type NamespaceSyncConfig struct { + // CreatePipelineSA controls whether the pipeline ServiceAccount is created + // in each namespace. Disable only if you manage the pipeline SA externally. + // Replaces the legacy spec.params entry createRbacResource. + // Default: true + // +optional + CreatePipelineSA *bool `json:"createPipelineSA,omitempty"` + + // CreateCABundles controls whether the CA bundle ConfigMaps + // (config-trusted-cabundle, config-service-cabundle) are injected into + // each namespace for TLS trust. + // Replaces the legacy spec.params entry createCABundleConfigMaps. + // Default: true + // +optional + CreateCABundles *bool `json:"createCABundles,omitempty"` + + // CreateEditRoleBinding controls whether a RoleBinding named + // openshift-pipelines-edit is created in each namespace, binding the + // pipeline SA to the built-in edit ClusterRole. Set to false for + // least-privilege environments where PipelineRuns should not have + // broad write permissions in their namespace. + // Replaces the legacy spec.params entry legacyPipelineRbac. + // Default: true + // +optional + CreateEditRoleBinding *bool `json:"createEditRoleBinding,omitempty"` + + // CreateSCCRoleBinding controls whether the pipelines-scc-rolebinding + // RoleBinding (and, when a namespace-level SCC is requested via the + // operator.tekton.dev/scc annotation, the pipelines-scc-role Role) is + // managed in each namespace. When enabled, the pipeline SA is granted + // permission to use the cluster-wide default SCC (pipelines-scc by + // default) or a namespace-specific SCC when the annotation is present. + // Default: true + // +optional + CreateSCCRoleBinding *bool `json:"createSCCRoleBinding,omitempty"` + + // SecretBindings declares secrets that should be automatically bound to + // the pipeline SA in every namespace. When a secret matching a binding + // appears in a namespace it is added to both imagePullSecrets and secrets + // on the pipeline SA. When the secret is deleted the reference is removed. + // Each entry must set exactly one of labelSelector or secretName. + // +optional + SecretBindings []SecretBinding `json:"secretBindings,omitempty"` +} + +// SecretBinding describes a secret (or class of secrets by label) that the +// NamespaceSyncController should bind to the pipeline SA in every namespace. +// Exactly one of LabelSelector or SecretName must be set. +type SecretBinding struct { + // LabelSelector selects secrets by label. All secrets matching this + // selector in a given namespace are bound to the pipeline SA. + // +optional + LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"` + + // SecretName binds a specific named secret in each namespace to the + // pipeline SA. The secret is bound when it exists and unbound when deleted. + // +optional + SecretName string `json:"secretName,omitempty"` } type SCC struct { diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go index 6dcc879221..9820b66268 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go @@ -24,6 +24,35 @@ import ( "knative.dev/pkg/ptr" ) +// migrateNamespaceSyncParams reads the legacy stringly-typed spec.params entries +// (createRbacResource, createCABundleConfigMaps, legacyPipelineRbac) and populates +// the equivalent typed fields in spec.platforms.openshift.namespaceSync, then removes +// the migrated params from the slice. Params that are already absent are left at their +// typed-field defaults (true). This runs only on OpenShift. +func migrateNamespaceSyncParams(tc *TektonConfig) { + ns := tc.Spec.Platforms.OpenShift.NamespaceSync + remaining := tc.Spec.Params[:0] + for _, p := range tc.Spec.Params { + switch p.Name { + case "createRbacResource": + if ns.CreatePipelineSA == nil { + ns.CreatePipelineSA = ptr.Bool(p.Value != "false") + } + case "createCABundleConfigMaps": + if ns.CreateCABundles == nil { + ns.CreateCABundles = ptr.Bool(p.Value != "false") + } + case "legacyPipelineRbac": + if ns.CreateEditRoleBinding == nil { + ns.CreateEditRoleBinding = ptr.Bool(p.Value != "false") + } + default: + remaining = append(remaining, p) + } + } + tc.Spec.Params = remaining +} + func (tc *TektonConfig) SetDefaults(ctx context.Context) { if tc.Spec.Profile == "" { tc.Spec.Profile = ProfileBasic @@ -85,6 +114,26 @@ func (tc *TektonConfig) SetDefaults(ctx context.Context) { tc.Spec.Platforms.OpenShift.SCC.Default = PipelinesSCC } + // NamespaceSync defaulting: initialise the block if absent, then apply + // per-field defaults (all true) and migrate any legacy spec.params entries. + if tc.Spec.Platforms.OpenShift.NamespaceSync == nil { + tc.Spec.Platforms.OpenShift.NamespaceSync = &NamespaceSyncConfig{} + } + ns := tc.Spec.Platforms.OpenShift.NamespaceSync + migrateNamespaceSyncParams(tc) + if ns.CreatePipelineSA == nil { + ns.CreatePipelineSA = ptr.Bool(true) + } + if ns.CreateCABundles == nil { + ns.CreateCABundles = ptr.Bool(true) + } + if ns.CreateEditRoleBinding == nil { + ns.CreateEditRoleBinding = ptr.Bool(true) + } + if ns.CreateSCCRoleBinding == nil { + ns.CreateSCCRoleBinding = ptr.Bool(true) + } + setAddonDefaults(&tc.Spec.Addon) } else { // Kubernetes Platform diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_validation.go b/pkg/apis/operator/v1alpha1/tektonconfig_validation.go index d725ef5bf7..8844d74276 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_validation.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_validation.go @@ -81,6 +81,10 @@ func (tc *TektonConfig) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(tc.Spec.Platforms.Kubernetes.PipelinesAsCode.PACSettings.validate(logger, "spec.platforms.kubernetes.pipelinesAsCode")) } + if IsOpenShiftPlatform() && tc.Spec.Platforms.OpenShift.NamespaceSync != nil { + errs = errs.Also(tc.Spec.Platforms.OpenShift.NamespaceSync.validate("spec.platforms.openshift.namespaceSync")) + } + // validate SCC config if IsOpenShiftPlatform() && tc.Spec.Platforms.OpenShift.SCC != nil { defaultSCC := PipelinesSCC @@ -200,7 +204,27 @@ func isValueInArray(arr []string, key string) bool { } func isOpenShiftPlatformsSectionSet(o OpenShift) bool { - return o.PipelinesAsCode != nil || o.SCC != nil + return o.PipelinesAsCode != nil || o.SCC != nil || o.NamespaceSync != nil +} + +func (ns *NamespaceSyncConfig) validate(path string) *apis.FieldError { + var errs *apis.FieldError + for i, b := range ns.SecretBindings { + errs = errs.Also(b.validate(fmt.Sprintf("%s.secretBindings[%d]", path, i))) + } + return errs +} + +func (b SecretBinding) validate(path string) *apis.FieldError { + hasLabel := b.LabelSelector != nil + hasName := b.SecretName != "" + if hasLabel && hasName { + return apis.ErrMultipleOneOf(path+".labelSelector", path+".secretName") + } + if !hasLabel && !hasName { + return apis.ErrMissingOneOf(path+".labelSelector", path+".secretName") + } + return nil } func isKubernetesPlatformsSectionSet(k Kubernetes) bool { diff --git a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go index 57bf17c3bc..713b4642f8 100644 --- a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go @@ -27,6 +27,7 @@ import ( appsv1 "k8s.io/api/apps/v1" v2 "k8s.io/api/autoscaling/v2" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -654,6 +655,49 @@ func (in *NamespaceMetadata) DeepCopy() *NamespaceMetadata { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NamespaceSyncConfig) DeepCopyInto(out *NamespaceSyncConfig) { + *out = *in + if in.CreatePipelineSA != nil { + in, out := &in.CreatePipelineSA, &out.CreatePipelineSA + *out = new(bool) + **out = **in + } + if in.CreateCABundles != nil { + in, out := &in.CreateCABundles, &out.CreateCABundles + *out = new(bool) + **out = **in + } + if in.CreateEditRoleBinding != nil { + in, out := &in.CreateEditRoleBinding, &out.CreateEditRoleBinding + *out = new(bool) + **out = **in + } + if in.CreateSCCRoleBinding != nil { + in, out := &in.CreateSCCRoleBinding, &out.CreateSCCRoleBinding + *out = new(bool) + **out = **in + } + if in.SecretBindings != nil { + in, out := &in.SecretBindings, &out.SecretBindings + *out = make([]SecretBinding, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceSyncConfig. +func (in *NamespaceSyncConfig) DeepCopy() *NamespaceSyncConfig { + if in == nil { + return nil + } + out := new(NamespaceSyncConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OpenShift) DeepCopyInto(out *OpenShift) { *out = *in @@ -672,6 +716,11 @@ func (in *OpenShift) DeepCopyInto(out *OpenShift) { *out = new(bool) **out = **in } + if in.NamespaceSync != nil { + in, out := &in.NamespaceSync, &out.NamespaceSync + *out = new(NamespaceSyncConfig) + (*in).DeepCopyInto(*out) + } return } @@ -1452,6 +1501,27 @@ func (in *Scope) DeepCopy() *Scope { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretBinding) DeepCopyInto(out *SecretBinding) { + *out = *in + if in.LabelSelector != nil { + in, out := &in.LabelSelector, &out.LabelSelector + *out = new(metav1.LabelSelector) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretBinding. +func (in *SecretBinding) DeepCopy() *SecretBinding { + if in == nil { + return nil + } + out := new(SecretBinding) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SyncerService) DeepCopyInto(out *SyncerService) { *out = *in diff --git a/pkg/reconciler/openshift/namespacesync/controller.go b/pkg/reconciler/openshift/namespacesync/controller.go new file mode 100644 index 0000000000..a9ed0c1ac9 --- /dev/null +++ b/pkg/reconciler/openshift/namespacesync/controller.go @@ -0,0 +1,205 @@ +/* +Copyright 2026 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package namespacesync + +import ( + "context" + "reflect" + + "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" + operatorclient "github.com/tektoncd/operator/pkg/client/injection/client" + tektonConfigInformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/tektonconfig" + pkgcommon "github.com/tektoncd/operator/pkg/common" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/cache" + kubeclient "knative.dev/pkg/client/injection/kube/client" + nsinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" + sainformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount" + "knative.dev/pkg/configmap" + "knative.dev/pkg/controller" + secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret" + "knative.dev/pkg/logging" +) + +// NewController initialises the NamespaceSyncController and registers event +// handlers for Namespace, ServiceAccount (pipeline only), Secret, and TektonConfig. +func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl { + logger := logging.FromContext(ctx) + + nsInf := nsinformer.Get(ctx) + saInf := sainformer.Get(ctx) + secretInf := secretinformer.Get(ctx) + tcInf := tektonConfigInformer.Get(ctx) + + rec := &Reconciler{ + kubeClient: kubeclient.Get(ctx), + operatorClient: operatorclient.Get(ctx), + securityClientSet: pkgcommon.GetSecurityClient(ctx), + nsLister: nsInf.Lister(), + saLister: saInf.Lister().ServiceAccounts(""), + tektonConfigLister: tcInf.Lister(), + } + + impl := controller.NewContext(ctx, rec, controller.ControllerOptions{ + WorkQueueName: "NamespaceSyncController", + Logger: logger, + }) + + // Namespace Add/Update → reconcile that namespace. + if _, err := nsInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + ns, ok := obj.(*corev1.Namespace) + if !ok || shouldIgnoreNamespace(ns) { + return + } + impl.EnqueueKey(types.NamespacedName{Name: ns.Name}) + }, + UpdateFunc: func(_, newObj interface{}) { + ns, ok := newObj.(*corev1.Namespace) + if !ok || shouldIgnoreNamespace(ns) { + return + } + impl.EnqueueKey(types.NamespacedName{Name: ns.Name}) + }, + }); err != nil { + logger.Panicf("Couldn't register Namespace informer event handler: %v", err) + } + + // pipeline SA events → reconcile its namespace. + // + // Add: SA was just created (by us or by the existing RBAC reconciler). + // Re-enqueue so ensureSecretBindings can bind any secrets that + // already existed in the namespace before the SA was present. + // This covers design Scenario B (secret arrives before SA). + // + // Delete: SA was removed externally — re-enqueue to recreate it. + // + // Update: SA contents changed (e.g. admin manually removed a secret + // binding) — re-enqueue for self-healing. + if _, err := saInf.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: func(obj interface{}) bool { + sa, ok := obj.(*corev1.ServiceAccount) + return ok && sa.Name == pipelineSA + }, + Handler: cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + sa, ok := obj.(*corev1.ServiceAccount) + if ok { + impl.EnqueueKey(types.NamespacedName{Name: sa.Namespace}) + } + }, + UpdateFunc: func(_, newObj interface{}) { + sa, ok := newObj.(*corev1.ServiceAccount) + if ok { + impl.EnqueueKey(types.NamespacedName{Name: sa.Namespace}) + } + }, + DeleteFunc: func(obj interface{}) { + sa, ok := obj.(*corev1.ServiceAccount) + if !ok { + // Tombstone — extract the SA from the DeletedFinalStateUnknown wrapper. + if d, ok := obj.(cache.DeletedFinalStateUnknown); ok { + sa, ok = d.Obj.(*corev1.ServiceAccount) + } + if !ok { + return + } + } + impl.EnqueueKey(types.NamespacedName{Name: sa.Namespace}) + }, + }, + }); err != nil { + logger.Panicf("Couldn't register ServiceAccount informer event handler: %v", err) + } + + // Secret events → re-enqueue the secret's namespace so that: + // - A newly created secret that matches a binding rule is bound immediately. + // - A deleted named secret is unbound from the pipeline SA. + // + // Only trigger re-reconciliation when NamespaceSync has SecretBindings + // configured, to avoid a thundering herd on clusters without secret bindings. + if _, err := secretInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + secret, ok := obj.(*corev1.Secret) + if !ok { + return + } + if namespaceSyncHasSecretBindings(tcInf.Lister()) { + impl.EnqueueKey(types.NamespacedName{Name: secret.Namespace}) + } + }, + DeleteFunc: func(obj interface{}) { + secret, ok := obj.(*corev1.Secret) + if !ok { + if d, ok := obj.(cache.DeletedFinalStateUnknown); ok { + secret, ok = d.Obj.(*corev1.Secret) + } + if !ok { + return + } + } + if namespaceSyncHasSecretBindings(tcInf.Lister()) { + impl.EnqueueKey(types.NamespacedName{Name: secret.Namespace}) + } + }, + }); err != nil { + logger.Panicf("Couldn't register Secret informer event handler: %v", err) + } + + // TektonConfig changed → re-enqueue all namespaces only when the NamespaceSync + // config itself changed. Unrelated TektonConfig field changes (e.g. pipeline + // options, pruner settings) must not trigger a full namespace sweep — that + // would be a thundering-herd problem on large clusters with 1000+ namespaces. + if _, err := tcInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + UpdateFunc: func(oldObj, newObj interface{}) { + oldTC, ok1 := oldObj.(*v1alpha1.TektonConfig) + newTC, ok2 := newObj.(*v1alpha1.TektonConfig) + if !ok1 || !ok2 { + return + } + if reflect.DeepEqual( + oldTC.Spec.Platforms.OpenShift.NamespaceSync, + newTC.Spec.Platforms.OpenShift.NamespaceSync, + ) { + return + } + for _, name := range allNamespacesFromLister(nsInf.Lister()) { + impl.EnqueueKey(types.NamespacedName{Name: name}) + } + }, + }); err != nil { + logger.Panicf("Couldn't register TektonConfig informer event handler: %v", err) + } + + return impl +} + +// namespaceSyncHasSecretBindings returns true when TektonConfig has at least one +// SecretBinding configured. Used to short-circuit Secret event handling when +// no secret binding is needed. +func namespaceSyncHasSecretBindings(lister interface { + Get(string) (*v1alpha1.TektonConfig, error) +}) bool { + tc, err := lister.Get(v1alpha1.ConfigResourceName) + if err != nil { + return false + } + cfg := tc.Spec.Platforms.OpenShift.NamespaceSync + return cfg != nil && len(cfg.SecretBindings) > 0 +} diff --git a/pkg/reconciler/openshift/namespacesync/reconciler.go b/pkg/reconciler/openshift/namespacesync/reconciler.go new file mode 100644 index 0000000000..0b88701f3c --- /dev/null +++ b/pkg/reconciler/openshift/namespacesync/reconciler.go @@ -0,0 +1,709 @@ +/* +Copyright 2026 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package namespacesync implements the NamespaceSyncController, a watch-based +// controller that ensures Tekton-required resources (pipeline SA, CA bundles, +// SCC RoleBinding, edit RoleBinding, and registry secret bindings) are present +// and up to date in every user namespace on OpenShift. +package namespacesync + +import ( + "context" + "fmt" + "regexp" + "time" + + security "github.com/openshift/client-go/security/clientset/versioned" + "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" + clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned" + tektonConfiglister "github.com/tektoncd/operator/pkg/client/listers/operator/v1alpha1" + pkgcommon "github.com/tektoncd/operator/pkg/common" + reconcilerCommon "github.com/tektoncd/operator/pkg/reconciler/common" + openshiftpkg "github.com/tektoncd/operator/pkg/reconciler/openshift" + + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/kubernetes" + corelisterv1 "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/util/retry" + "knative.dev/pkg/logging" +) + +const ( + pipelineSA = "pipeline" + pipelinesSCCRole = "pipelines-scc-role" + pipelinesSCCClusterRole = "pipelines-scc-clusterrole" + pipelinesSCCRoleBinding = "pipelines-scc-rolebinding" + editRoleBinding = "openshift-pipelines-edit" + editClusterRole = "edit" + serviceCABundleConfigMap = "config-service-cabundle" + trustedCABundleConfigMap = "config-trusted-cabundle" +) + +var nsIgnoreRegex = regexp.MustCompile(reconcilerCommon.NamespaceIgnorePattern) + +// Reconciler reconciles a namespace name as its work unit. It reads the +// current NamespaceSyncConfig from TektonConfig and ensures all declared +// resources exist and are correct in the given namespace. +type Reconciler struct { + kubeClient kubernetes.Interface + operatorClient clientset.Interface + securityClientSet security.Interface + nsLister corelisterv1.NamespaceLister + saLister corelisterv1.ServiceAccountNamespaceLister + tektonConfigLister tektonConfiglister.TektonConfigLister +} + +var _ interface { + Reconcile(context.Context, string) error +} = (*Reconciler)(nil) + +// Reconcile is the main entry point — key is the namespace name. +func (r *Reconciler) Reconcile(ctx context.Context, key string) error { + logger := logging.FromContext(ctx) + + tc, err := r.tektonConfigLister.Get(v1alpha1.ConfigResourceName) + if errors.IsNotFound(err) { + logger.Debug("TektonConfig not found, skipping namespace sync") + return nil + } + if err != nil { + return err + } + + cfg := tc.Spec.Platforms.OpenShift.NamespaceSync + if cfg == nil { + logger.Debug("NamespaceSync config absent, skipping") + return nil + } + + ns, err := r.nsLister.Get(key) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + + if shouldIgnoreNamespace(ns) { + logger.Debugf("Ignoring system/terminating namespace: %s", key) + return nil + } + + return r.reconcileNamespace(ctx, ns, tc, cfg) +} + +func (r *Reconciler) reconcileNamespace(ctx context.Context, ns *corev1.Namespace, tc *v1alpha1.TektonConfig, cfg *v1alpha1.NamespaceSyncConfig) error { + logger := logging.FromContext(ctx) + + if cfg.CreatePipelineSA != nil && *cfg.CreatePipelineSA { + if err := r.ensurePipelineSA(ctx, ns, tc); err != nil { + logger.Errorf("failed to ensure pipeline SA in %s: %v", ns.Name, err) + return err + } + } + + if cfg.CreateSCCRoleBinding != nil && *cfg.CreateSCCRoleBinding { + if err := r.ensureSCCRoleBinding(ctx, ns, tc); err != nil { + logger.Errorf("failed to ensure SCC RoleBinding in %s: %v", ns.Name, err) + return err + } + } + + if cfg.CreateEditRoleBinding != nil && *cfg.CreateEditRoleBinding { + if err := r.ensureEditRoleBinding(ctx, ns); err != nil { + logger.Errorf("failed to ensure edit RoleBinding in %s: %v", ns.Name, err) + return err + } + } else { + if err := r.removeEditRoleBindingIfPresent(ctx, ns.Name); err != nil { + logger.Errorf("failed to remove edit RoleBinding from %s: %v", ns.Name, err) + return err + } + } + + if cfg.CreateCABundles != nil && *cfg.CreateCABundles { + if err := r.ensureCABundles(ctx, ns); err != nil { + logger.Errorf("failed to ensure CA bundles in %s: %v", ns.Name, err) + return err + } + } + + if len(cfg.SecretBindings) > 0 { + if err := r.ensureSecretBindings(ctx, ns, cfg.SecretBindings); err != nil { + logger.Errorf("failed to ensure secret bindings in %s: %v", ns.Name, err) + return err + } + } + + return nil +} + +// --------------------------------------------------------------------------- +// Pipeline ServiceAccount +// --------------------------------------------------------------------------- + +// ensurePipelineSA creates the pipeline ServiceAccount if absent, or updates +// the owner reference if it already exists. +func (r *Reconciler) ensurePipelineSA(ctx context.Context, ns *corev1.Namespace, tc *v1alpha1.TektonConfig) error { + logger := logging.FromContext(ctx) + saClient := r.kubeClient.CoreV1().ServiceAccounts(ns.Name) + + sa, err := saClient.Get(ctx, pipelineSA, metav1.GetOptions{}) + if errors.IsNotFound(err) { + logger.Infof("Creating pipeline SA in namespace %s", ns.Name) + ownerRef := tektonConfigOwnerRef(tc) + _, err = saClient.Create(ctx, &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelineSA, + Namespace: ns.Name, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + }, metav1.CreateOptions{}) + return err + } + if err != nil { + return err + } + + ownerRef := tektonConfigOwnerRef(tc) + if !hasOwnerReference(sa.OwnerReferences, ownerRef) { + sa.OwnerReferences = []metav1.OwnerReference{ownerRef} + _, err = saClient.Update(ctx, sa, metav1.UpdateOptions{}) + return err + } + return nil +} + +// --------------------------------------------------------------------------- +// SCC RoleBinding +// --------------------------------------------------------------------------- + +// ensureSCCRoleBinding creates or updates the pipelines-scc-rolebinding in the +// namespace, binding the pipeline SA to the appropriate SCC Role or ClusterRole. +// If the namespace carries the operator.tekton.dev/scc annotation, a +// namespace-scoped Role is created for that specific SCC; otherwise the cluster- +// wide pipelines-scc-clusterrole is used. +func (r *Reconciler) ensureSCCRoleBinding(ctx context.Context, ns *corev1.Namespace, tc *v1alpha1.TektonConfig) error { + logger := logging.FromContext(ctx) + + // The RoleBinding subject is the pipeline SA. If it does not exist yet + // the SA watch will re-enqueue this namespace once it is created. + sa, err := r.kubeClient.CoreV1().ServiceAccounts(ns.Name).Get(ctx, pipelineSA, metav1.GetOptions{}) + if errors.IsNotFound(err) { + logger.Debugf("pipeline SA not yet present in %s, deferring SCC RoleBinding", ns.Name) + return nil + } + if err != nil { + return err + } + + nsSCC := ns.Annotations[openshiftpkg.NamespaceSCCAnnotation] + + if nsSCC == "" { + // No custom SCC annotation: clean up any leftover namespace-scoped Role + // (left over from a previous annotation) and use the ClusterRole. + if err := r.deleteRoleIfPresent(ctx, ns.Name, pipelinesSCCRole); err != nil { + return err + } + } else { + // A specific SCC has been requested for this namespace. + logger.Infof("Namespace %s requests SCC %s", ns.Name, nsSCC) + + // Verify the SCC exists on the cluster. + if err := pkgcommon.VerifySCCExists(ctx, nsSCC, r.securityClientSet); err != nil { + logger.Errorf("SCC %s not found: %v", nsSCC, err) + if evtErr := r.createSCCFailureEvent(ctx, ns.Name, nsSCC, tc); evtErr != nil { + logger.Errorf("Failed to create SCC failure event in %s: %v", ns.Name, evtErr) + } + return err + } + + // Enforce the maxAllowed SCC policy when configured. + if tc.Spec.Platforms.OpenShift.SCC != nil && tc.Spec.Platforms.OpenShift.SCC.MaxAllowed != "" { + maxAllowed := tc.Spec.Platforms.OpenShift.SCC.MaxAllowed + list, err := pkgcommon.GetSCCRestrictiveList(ctx, r.securityClientSet) + if err != nil { + return err + } + ok, err := pkgcommon.SCCAMoreRestrictiveThanB(list, nsSCC, maxAllowed) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("namespace %s requested SCC %s which is less restrictive than maxAllowed %s", ns.Name, nsSCC, maxAllowed) + } + } + + // Ensure the namespace-scoped Role for this SCC. + if err := r.ensureSCCRoleInNamespace(ctx, ns.Name, nsSCC, tc); err != nil { + return err + } + } + + roleRef := r.getSCCRoleRef(ns) + return r.ensurePipelinesSCCRoleBinding(ctx, sa, roleRef, tc) +} + +// getSCCRoleRef returns a Role ref pointing at the namespace-scoped pipelines-scc-role +// when a custom SCC is requested, or the cluster-wide pipelines-scc-clusterrole otherwise. +func (r *Reconciler) getSCCRoleRef(ns *corev1.Namespace) *rbacv1.RoleRef { + if ns.Annotations[openshiftpkg.NamespaceSCCAnnotation] != "" { + return &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: pipelinesSCCRole, + } + } + return &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: pipelinesSCCClusterRole, + } +} + +// ensureSCCRoleInNamespace creates or updates the namespace-scoped pipelines-scc-role +// that grants the `use` verb on the requested SCC. +func (r *Reconciler) ensureSCCRoleInNamespace(ctx context.Context, nsName, scc string, tc *v1alpha1.TektonConfig) error { + ownerRef := tektonConfigOwnerRef(tc) + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelinesSCCRole, + Namespace: nsName, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + Rules: []rbacv1.PolicyRule{{ + APIGroups: []string{"security.openshift.io"}, + Resources: []string{"securitycontextconstraints"}, + ResourceNames: []string{scc}, + Verbs: []string{"use"}, + }}, + } + + rbacClient := r.kubeClient.RbacV1() + _, err := rbacClient.Roles(nsName).Get(ctx, pipelinesSCCRole, metav1.GetOptions{}) + if errors.IsNotFound(err) { + _, err = rbacClient.Roles(nsName).Create(ctx, role, metav1.CreateOptions{}) + return err + } + if err != nil { + return err + } + _, err = rbacClient.Roles(nsName).Update(ctx, role, metav1.UpdateOptions{}) + return err +} + +// ensurePipelinesSCCRoleBinding creates or updates the pipelines-scc-rolebinding +// that binds the pipeline SA to the given role ref. +func (r *Reconciler) ensurePipelinesSCCRoleBinding(ctx context.Context, sa *corev1.ServiceAccount, roleRef *rbacv1.RoleRef, tc *v1alpha1.TektonConfig) error { + logger := logging.FromContext(ctx) + rbacClient := r.kubeClient.RbacV1() + ownerRef := tektonConfigOwnerRef(tc) + + existing, err := rbacClient.RoleBindings(sa.Namespace).Get(ctx, pipelinesSCCRoleBinding, metav1.GetOptions{}) + if errors.IsNotFound(err) { + logger.Infof("Creating %s in namespace %s", pipelinesSCCRoleBinding, sa.Namespace) + _, err = rbacClient.RoleBindings(sa.Namespace).Create(ctx, &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelinesSCCRoleBinding, + Namespace: sa.Namespace, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + RoleRef: *roleRef, + Subjects: []rbacv1.Subject{{Kind: rbacv1.ServiceAccountKind, Name: sa.Name, Namespace: sa.Namespace}}, + }, metav1.CreateOptions{}) + return err + } + if err != nil { + return err + } + + // RoleRef is immutable — delete and recreate if it changed. + if existing.RoleRef.Kind != roleRef.Kind || existing.RoleRef.Name != roleRef.Name { + logger.Infof("RoleRef changed in %s/%s, recreating", sa.Namespace, pipelinesSCCRoleBinding) + if err := rbacClient.RoleBindings(sa.Namespace).Delete(ctx, pipelinesSCCRoleBinding, metav1.DeleteOptions{}); err != nil { + return err + } + _, err = rbacClient.RoleBindings(sa.Namespace).Create(ctx, &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelinesSCCRoleBinding, + Namespace: sa.Namespace, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + RoleRef: *roleRef, + Subjects: []rbacv1.Subject{{Kind: rbacv1.ServiceAccountKind, Name: sa.Name, Namespace: sa.Namespace}}, + }, metav1.CreateOptions{}) + return err + } + + // Ensure subject is present. + subject := rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: sa.Name, Namespace: sa.Namespace} + if hasSubject(existing.Subjects, subject) { + return nil + } + existing.Subjects = append(existing.Subjects, subject) + _, err = rbacClient.RoleBindings(sa.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) + return err +} + +// deleteRoleIfPresent deletes a namespace-scoped Role, ignoring not-found errors. +func (r *Reconciler) deleteRoleIfPresent(ctx context.Context, nsName, roleName string) error { + err := r.kubeClient.RbacV1().Roles(nsName).Delete(ctx, roleName, metav1.DeleteOptions{}) + if errors.IsNotFound(err) { + return nil + } + return err +} + +// createSCCFailureEvent records a Warning event in the namespace when the +// requested SCC is not found on the cluster. +func (r *Reconciler) createSCCFailureEvent(ctx context.Context, nsName, scc string, tc *v1alpha1.TektonConfig) error { + ownerRef := tektonConfigOwnerRef(tc) + event := &corev1.Event{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "pipelines-scc-failure-", + Namespace: nsName, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + EventTime: metav1.NewMicroTime(time.Now()), + Reason: "RequestedSCCNotFound", + Type: "Warning", + Action: "SCCNotUpdated", + Message: fmt.Sprintf("SCC '%s' requested in annotation '%s' not found, SCC not updated in the namespace", scc, openshiftpkg.NamespaceSCCAnnotation), + ReportingController: "openshift-pipelines-operator", + ReportingInstance: tc.Name, + InvolvedObject: corev1.ObjectReference{ + Kind: "Namespace", + Name: nsName, + APIVersion: "v1", + Namespace: nsName, + }, + } + _, err := r.kubeClient.CoreV1().Events(nsName).Create(ctx, event, metav1.CreateOptions{}) + return err +} + +// --------------------------------------------------------------------------- +// Edit RoleBinding +// --------------------------------------------------------------------------- + +// ensureEditRoleBinding creates the openshift-pipelines-edit RoleBinding binding +// the pipeline SA to the built-in edit ClusterRole. +func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Namespace) error { + logger := logging.FromContext(ctx) + rbClient := r.kubeClient.RbacV1().RoleBindings(ns.Name) + + _, err := rbClient.Get(ctx, editRoleBinding, metav1.GetOptions{}) + if err == nil { + return nil + } + if !errors.IsNotFound(err) { + return err + } + + // Verify the edit ClusterRole exists before creating the binding. + if _, err := r.kubeClient.RbacV1().ClusterRoles().Get(ctx, editClusterRole, metav1.GetOptions{}); err != nil { + return err + } + + logger.Infof("Creating edit RoleBinding in namespace %s", ns.Name) + _, err = rbClient.Create(ctx, &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: editRoleBinding, + Namespace: ns.Name, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: editClusterRole, + }, + Subjects: []rbacv1.Subject{{ + Kind: rbacv1.ServiceAccountKind, + Name: pipelineSA, + Namespace: ns.Name, + }}, + }, metav1.CreateOptions{}) + return err +} + +// removeEditRoleBindingIfPresent deletes the openshift-pipelines-edit RoleBinding +// when createEditRoleBinding is disabled. +func (r *Reconciler) removeEditRoleBindingIfPresent(ctx context.Context, nsName string) error { + err := r.kubeClient.RbacV1().RoleBindings(nsName).Delete(ctx, editRoleBinding, metav1.DeleteOptions{}) + if errors.IsNotFound(err) { + return nil + } + return err +} + +// --------------------------------------------------------------------------- +// CA Bundles +// --------------------------------------------------------------------------- + +// ensureCABundles creates the config-trusted-cabundle and config-service-cabundle +// ConfigMaps in the namespace (or strips owner references from existing ones). +// These ConfigMaps carry annotations/labels that trigger OpenShift's CA injection +// controllers so that pods can trust the cluster's CA and internal service CA. +func (r *Reconciler) ensureCABundles(ctx context.Context, ns *corev1.Namespace) error { + logger := logging.FromContext(ctx) + cmClient := r.kubeClient.CoreV1().ConfigMaps(ns.Name) + + // Ensure config-trusted-cabundle + logger.Infof("Ensuring configmap %s in namespace %s", trustedCABundleConfigMap, ns.Name) + trustedCM, err := cmClient.Get(ctx, trustedCABundleConfigMap, metav1.GetOptions{}) + if err != nil && !errors.IsNotFound(err) { + return err + } + if errors.IsNotFound(err) { + trustedCM = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustedCABundleConfigMap, + Namespace: ns.Name, + Labels: map[string]string{ + "app.kubernetes.io/part-of": "tekton-pipelines", + // OpenShift CA injection label: cluster CA + user-provided certs + "config.openshift.io/inject-trusted-cabundle": "true", + }, + }, + } + if _, err := cmClient.Create(ctx, trustedCM, metav1.CreateOptions{}); err != nil && !errors.IsAlreadyExists(err) { + return err + } + } else { + // Strip owner references from pre-existing ConfigMap to avoid GC. + trustedCM.SetOwnerReferences(nil) + if _, err := cmClient.Update(ctx, trustedCM, metav1.UpdateOptions{}); err != nil { + return err + } + } + + // Ensure config-service-cabundle + logger.Infof("Ensuring configmap %s in namespace %s", serviceCABundleConfigMap, ns.Name) + serviceCM, err := cmClient.Get(ctx, serviceCABundleConfigMap, metav1.GetOptions{}) + if err != nil && !errors.IsNotFound(err) { + return err + } + if errors.IsNotFound(err) { + serviceCM = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceCABundleConfigMap, + Namespace: ns.Name, + Labels: map[string]string{ + "app.kubernetes.io/part-of": "tekton-pipelines", + }, + Annotations: map[string]string{ + // OpenShift service CA injection annotation: internal service certs + "service.beta.openshift.io/inject-cabundle": "true", + }, + }, + } + if _, err := cmClient.Create(ctx, serviceCM, metav1.CreateOptions{}); err != nil && !errors.IsAlreadyExists(err) { + return err + } + } else { + serviceCM.SetOwnerReferences(nil) + if _, err := cmClient.Update(ctx, serviceCM, metav1.UpdateOptions{}); err != nil { + return err + } + } + + return nil +} + +// --------------------------------------------------------------------------- +// Secret bindings +// --------------------------------------------------------------------------- + +// ensureSecretBindings reconciles secret bindings on the pipeline SA: +// - Adds secrets that match a binding rule and currently exist in the namespace. +// - Removes secrets that were managed by a named binding but whose Secret was deleted. +// - Label-based bindings are add-only: a deleted labeled secret leaves a +// dangling reference (harmless; Kubernetes ignores missing pull secrets). +func (r *Reconciler) ensureSecretBindings(ctx context.Context, ns *corev1.Namespace, bindings []v1alpha1.SecretBinding) error { + logger := logging.FromContext(ctx) + + sa, err := r.kubeClient.CoreV1().ServiceAccounts(ns.Name).Get(ctx, pipelineSA, metav1.GetOptions{}) + if errors.IsNotFound(err) { + // pipeline SA not yet created — SA watch will re-trigger this reconcile. + return nil + } + if err != nil { + return err + } + + secretClient := r.kubeClient.CoreV1().Secrets(ns.Name) + + // expected: secrets that should be bound after this reconcile. + // managed: secrets we are responsible for (we may have added them). + expected := map[string]bool{} + managed := map[string]bool{} + + for _, binding := range bindings { + switch { + case binding.SecretName != "": + managed[binding.SecretName] = true + _, err := secretClient.Get(ctx, binding.SecretName, metav1.GetOptions{}) + if err == nil { + expected[binding.SecretName] = true + } else if !errors.IsNotFound(err) { + return err + } + + case binding.LabelSelector != nil: + sel, err := metav1.LabelSelectorAsSelector(binding.LabelSelector) + if err != nil { + return err + } + list, err := secretClient.List(ctx, metav1.ListOptions{LabelSelector: sel.String()}) + if err != nil { + return err + } + for i := range list.Items { + name := list.Items[i].Name + managed[name] = true + expected[name] = true + } + } + } + + changed := false + + // Remove stale: in managed set but no longer expected. + newImagePull := make([]corev1.LocalObjectReference, 0, len(sa.ImagePullSecrets)) + for _, ref := range sa.ImagePullSecrets { + if managed[ref.Name] && !expected[ref.Name] { + logger.Infof("Removing stale secret binding %s/%s from pipeline SA", ns.Name, ref.Name) + changed = true + } else { + newImagePull = append(newImagePull, ref) + } + } + newSecretRefs := make([]corev1.ObjectReference, 0, len(sa.Secrets)) + for _, ref := range sa.Secrets { + if managed[ref.Name] && !expected[ref.Name] { + changed = true + } else { + newSecretRefs = append(newSecretRefs, ref) + } + } + sa.ImagePullSecrets = newImagePull + sa.Secrets = newSecretRefs + + // Add expected secrets that are not yet bound. + for name := range expected { + if bindSecretToSA(sa, name) { + logger.Infof("Binding secret %s/%s to pipeline SA", ns.Name, name) + changed = true + } + } + + if !changed { + return nil + } + + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + _, err := r.kubeClient.CoreV1().ServiceAccounts(ns.Name).Update(ctx, sa, metav1.UpdateOptions{}) + return err + }) +} + +// bindSecretToSA adds secretName to both imagePullSecrets and secrets on the SA +// if not already present. Returns true if the SA was modified. +func bindSecretToSA(sa *corev1.ServiceAccount, secretName string) bool { + changed := false + + hasPull := false + for _, ref := range sa.ImagePullSecrets { + if ref.Name == secretName { + hasPull = true + break + } + } + if !hasPull { + sa.ImagePullSecrets = append(sa.ImagePullSecrets, corev1.LocalObjectReference{Name: secretName}) + changed = true + } + + hasMount := false + for _, ref := range sa.Secrets { + if ref.Name == secretName { + hasMount = true + break + } + } + if !hasMount { + sa.Secrets = append(sa.Secrets, corev1.ObjectReference{Name: secretName}) + changed = true + } + + return changed +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +func shouldIgnoreNamespace(ns *corev1.Namespace) bool { + if nsIgnoreRegex.MatchString(ns.Name) { + return true + } + return ns.DeletionTimestamp != nil +} + +func tektonConfigOwnerRef(tc *v1alpha1.TektonConfig) metav1.OwnerReference { + return metav1.OwnerReference{ + APIVersion: tc.GroupVersionKind().GroupVersion().String(), + Kind: tc.GroupVersionKind().Kind, + Name: tc.Name, + UID: tc.UID, + } +} + +func hasOwnerReference(refs []metav1.OwnerReference, target metav1.OwnerReference) bool { + for _, r := range refs { + if r.APIVersion == target.APIVersion && r.Kind == target.Kind && r.Name == target.Name { + return true + } + } + return false +} + +func hasSubject(subjects []rbacv1.Subject, target rbacv1.Subject) bool { + for _, s := range subjects { + if s.Kind == target.Kind && s.Name == target.Name && s.Namespace == target.Namespace { + return true + } + } + return false +} + +// allNamespacesFromLister returns all non-ignored namespace names from the lister. +func allNamespacesFromLister(lister corelisterv1.NamespaceLister) []string { + nsList, err := lister.List(labels.Everything()) + if err != nil { + return nil + } + names := make([]string, 0, len(nsList)) + for _, ns := range nsList { + if !shouldIgnoreNamespace(ns) { + names = append(names, ns.Name) + } + } + return names +} diff --git a/pkg/reconciler/openshift/namespacesync/reconciler_test.go b/pkg/reconciler/openshift/namespacesync/reconciler_test.go new file mode 100644 index 0000000000..c0668f6c23 --- /dev/null +++ b/pkg/reconciler/openshift/namespacesync/reconciler_test.go @@ -0,0 +1,679 @@ +/* +Copyright 2026 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package namespacesync + +import ( + "context" + "testing" + + "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" + operatorfake "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake" + operatorinformers "github.com/tektoncd/operator/pkg/client/informers/externalversions" + "gotest.tools/v3/assert" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kubeinformers "k8s.io/client-go/informers" + kubefake "k8s.io/client-go/kubernetes/fake" +) + +func boolPtr(b bool) *bool { return &b } + +// newTestReconciler builds a Reconciler backed by fake clients and pre-populates +// the informer stores. Namespaces listed here are reachable via r.nsLister.Get. +func newTestReconciler(t *testing.T, tc *v1alpha1.TektonConfig, namespaces []corev1.Namespace) (*Reconciler, *kubefake.Clientset) { + t.Helper() + + kubeClient := kubefake.NewSimpleClientset() + operatorClient := operatorfake.NewSimpleClientset(tc) + + kubeInformers := kubeinformers.NewSharedInformerFactory(kubeClient, 0) + operatorInfs := operatorinformers.NewSharedInformerFactory(operatorClient, 0) + + tcInformer := operatorInfs.Operator().V1alpha1().TektonConfigs() + nsInformer := kubeInformers.Core().V1().Namespaces() + saInformer := kubeInformers.Core().V1().ServiceAccounts() + + assert.NilError(t, tcInformer.Informer().GetStore().Add(tc)) + for i := range namespaces { + assert.NilError(t, nsInformer.Informer().GetStore().Add(&namespaces[i])) + } + + return &Reconciler{ + kubeClient: kubeClient, + operatorClient: operatorClient, + nsLister: nsInformer.Lister(), + saLister: saInformer.Lister().ServiceAccounts(""), + tektonConfigLister: tcInformer.Lister(), + }, kubeClient +} + +func minimalTC(cfg *v1alpha1.NamespaceSyncConfig) *v1alpha1.TektonConfig { + return &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{Name: v1alpha1.ConfigResourceName}, + Spec: v1alpha1.TektonConfigSpec{ + Platforms: v1alpha1.Platforms{ + OpenShift: v1alpha1.OpenShift{ + NamespaceSync: cfg, + }, + }, + }, + } +} + +// --------------------------------------------------------------------------- +// Reconcile top-level routing +// --------------------------------------------------------------------------- + +func TestReconcile_TektonConfigNotFound(t *testing.T) { + kubeClient := kubefake.NewSimpleClientset() + operatorClient := operatorfake.NewSimpleClientset() // no TektonConfig in store + + kubeInformers := kubeinformers.NewSharedInformerFactory(kubeClient, 0) + operatorInfs := operatorinformers.NewSharedInformerFactory(operatorClient, 0) + + r := &Reconciler{ + kubeClient: kubeClient, + operatorClient: operatorClient, + nsLister: kubeInformers.Core().V1().Namespaces().Lister(), + saLister: kubeInformers.Core().V1().ServiceAccounts().Lister().ServiceAccounts(""), + tektonConfigLister: operatorInfs.Operator().V1alpha1().TektonConfigs().Lister(), + } + + // TektonConfig lister cache is empty → should return nil without error. + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) +} + +func TestReconcile_NamespaceSyncNil(t *testing.T) { + tc := minimalTC(nil) + r, _ := newTestReconciler(t, tc, nil) + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) +} + +func TestReconcile_SystemNamespaceIgnored(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{CreatePipelineSA: boolPtr(true)}) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "openshift-operators"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + err := r.Reconcile(context.Background(), "openshift-operators") + assert.NilError(t, err) + + // pipeline SA must NOT be created in system namespaces. + _, err = kubeClient.CoreV1().ServiceAccounts("openshift-operators").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.ErrorContains(t, err, "not found") +} + +func TestReconcile_TerminatingNamespaceIgnored(t *testing.T) { + now := metav1.Now() + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{CreatePipelineSA: boolPtr(true)}) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns", DeletionTimestamp: &now}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + _, err = kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.ErrorContains(t, err, "not found") +} + +// --------------------------------------------------------------------------- +// ensurePipelineSA +// --------------------------------------------------------------------------- + +func TestEnsurePipelineSA_CreatesWhenAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{CreatePipelineSA: boolPtr(true)}) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, pipelineSA, sa.Name) + assert.Equal(t, 1, len(sa.OwnerReferences)) + assert.Equal(t, v1alpha1.ConfigResourceName, sa.OwnerReferences[0].Name) +} + +func TestEnsurePipelineSA_SetsOwnerRefOnExistingSA(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{CreatePipelineSA: boolPtr(true)}) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // pre-create a pipeline SA with no owner reference + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + // owner ref should be set on the existing SA + assert.Equal(t, 1, len(sa.OwnerReferences)) + assert.Equal(t, v1alpha1.ConfigResourceName, sa.OwnerReferences[0].Name) +} + +func TestEnsurePipelineSA_IdempotentWhenOwnerRefPresent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{CreatePipelineSA: boolPtr(true)}) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + ownerRef := tektonConfigOwnerRef(tc) + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelineSA, + Namespace: "my-ns", + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + // Reconcile twice — should be a no-op on the second call. + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 1, len(sa.OwnerReferences)) +} + +func TestEnsurePipelineSA_SkippedWhenDisabled(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{CreatePipelineSA: boolPtr(false)}) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + _, err = kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.ErrorContains(t, err, "not found") +} + +// --------------------------------------------------------------------------- +// ensureEditRoleBinding +// --------------------------------------------------------------------------- + +func TestEnsureEditRoleBinding_CreatesWhenAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateEditRoleBinding: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // The reconciler verifies that the edit ClusterRole exists before creating the binding. + _, err := kubeClient.RbacV1().ClusterRoles().Create(context.Background(), &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: editClusterRole}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + rb, err := kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), editRoleBinding, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, editClusterRole, rb.RoleRef.Name) + assert.Equal(t, pipelineSA, rb.Subjects[0].Name) +} + +func TestEnsureEditRoleBinding_IdempotentWhenPresent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateEditRoleBinding: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + _, err := kubeClient.RbacV1().ClusterRoles().Create(context.Background(), &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: editClusterRole}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + _, err = kubeClient.RbacV1().RoleBindings("my-ns").Create(context.Background(), &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: editRoleBinding, Namespace: "my-ns"}, + RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: editClusterRole}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + // Second reconcile — must not attempt to create again. + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) +} + +func TestRemoveEditRoleBinding_DeletesWhenPresent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateEditRoleBinding: boolPtr(false), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // pre-create the RoleBinding — reconcile must delete it. + _, err := kubeClient.RbacV1().RoleBindings("my-ns").Create(context.Background(), &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: editRoleBinding, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + _, err = kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), editRoleBinding, metav1.GetOptions{}) + assert.ErrorContains(t, err, "not found") +} + +func TestRemoveEditRoleBinding_NoopWhenAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateEditRoleBinding: boolPtr(false), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, _ := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // No RoleBinding pre-exists — should not error. + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) +} + +// --------------------------------------------------------------------------- +// ensureSecretBindings +// --------------------------------------------------------------------------- + +func TestEnsureSecretBindings_ByName_BindsWhenSecretExists(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + SecretBindings: []v1alpha1.SecretBinding{{SecretName: "quay-robot"}}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + _, err = kubeClient.CoreV1().Secrets("my-ns").Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "quay-robot", Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, "quay-robot", sa.ImagePullSecrets[0].Name) + assert.Equal(t, 1, len(sa.Secrets)) + assert.Equal(t, "quay-robot", sa.Secrets[0].Name) +} + +func TestEnsureSecretBindings_ByName_SkipsWhenSecretAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + SecretBindings: []v1alpha1.SecretBinding{{SecretName: "quay-robot"}}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // pipeline SA exists, but the named secret does not + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + // SA must have no bindings — no infinite loop / error. + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 0, len(sa.ImagePullSecrets)) + assert.Equal(t, 0, len(sa.Secrets)) +} + +func TestEnsureSecretBindings_Idempotent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + SecretBindings: []v1alpha1.SecretBinding{{SecretName: "quay-robot"}}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // SA already has the binding. + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "quay-robot"}}, + Secrets: []corev1.ObjectReference{{Name: "quay-robot"}}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + _, err = kubeClient.CoreV1().Secrets("my-ns").Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "quay-robot", Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + // must not have doubled up + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, 1, len(sa.Secrets)) +} + +func TestEnsureSecretBindings_ByLabel(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + SecretBindings: []v1alpha1.SecretBinding{{ + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"quay.io/robot-token": "true"}, + }, + }}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + _, err = kubeClient.CoreV1().Secrets("my-ns").Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "quay-robot-token", + Namespace: "my-ns", + Labels: map[string]string{"quay.io/robot-token": "true"}, + }, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, "quay-robot-token", sa.ImagePullSecrets[0].Name) +} + +func TestEnsureSecretBindings_SkipsWhenSAAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), // SA won't be created by this reconcile + SecretBindings: []v1alpha1.SecretBinding{{SecretName: "quay-robot"}}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, _ := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // pipeline SA does not exist — ensureSecretBindings should be a no-op. + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) +} + +// --------------------------------------------------------------------------- +// bindSecretToSA unit tests +// --------------------------------------------------------------------------- + +func TestBindSecretToSA_AddsToEmptySA(t *testing.T) { + sa := &corev1.ServiceAccount{} + changed := bindSecretToSA(sa, "my-secret") + assert.Equal(t, true, changed) + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, "my-secret", sa.ImagePullSecrets[0].Name) + assert.Equal(t, 1, len(sa.Secrets)) + assert.Equal(t, "my-secret", sa.Secrets[0].Name) +} + +func TestBindSecretToSA_Idempotent(t *testing.T) { + sa := &corev1.ServiceAccount{ + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-secret"}}, + Secrets: []corev1.ObjectReference{{Name: "my-secret"}}, + } + changed := bindSecretToSA(sa, "my-secret") + assert.Equal(t, false, changed) + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, 1, len(sa.Secrets)) +} + +func TestBindSecretToSA_AddsOnlyImagePullWhenMountPresent(t *testing.T) { + sa := &corev1.ServiceAccount{ + Secrets: []corev1.ObjectReference{{Name: "my-secret"}}, + } + changed := bindSecretToSA(sa, "my-secret") + assert.Equal(t, true, changed) // imagePullSecret was missing + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, 1, len(sa.Secrets)) +} + +// --------------------------------------------------------------------------- +// ensureCABundles +// --------------------------------------------------------------------------- + +func TestEnsureCABundles_CreatesWhenAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateCABundles: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + _, err = kubeClient.CoreV1().ConfigMaps("my-ns").Get(context.Background(), trustedCABundleConfigMap, metav1.GetOptions{}) + assert.NilError(t, err) + + _, err = kubeClient.CoreV1().ConfigMaps("my-ns").Get(context.Background(), serviceCABundleConfigMap, metav1.GetOptions{}) + assert.NilError(t, err) +} + +func TestEnsureCABundles_StripsOwnerRefFromExisting(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateCABundles: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // Pre-create CMs with an owner reference. + for _, name := range []string{trustedCABundleConfigMap, serviceCABundleConfigMap} { + _, err := kubeClient.CoreV1().ConfigMaps("my-ns").Create(context.Background(), &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "my-ns", + OwnerReferences: []metav1.OwnerReference{ + {Name: "some-owner", APIVersion: "v1", Kind: "Thing"}, + }, + }, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + } + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + cm, err := kubeClient.CoreV1().ConfigMaps("my-ns").Get(context.Background(), trustedCABundleConfigMap, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 0, len(cm.OwnerReferences)) +} + +func TestEnsureCABundles_Idempotent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateCABundles: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + _, err := kubeClient.CoreV1().ConfigMaps("my-ns").Get(context.Background(), trustedCABundleConfigMap, metav1.GetOptions{}) + assert.NilError(t, err) +} + +// --------------------------------------------------------------------------- +// ensureSCCRoleBinding +// --------------------------------------------------------------------------- + +func TestEnsureSCCRoleBinding_SkipsWhenSAAbsent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateSCCRoleBinding: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + // No security client needed — SA is absent so we return early. + r.securityClientSet = nil + + err := r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + _, err = kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), pipelinesSCCRoleBinding, metav1.GetOptions{}) + assert.ErrorContains(t, err, "not found") +} + +func TestEnsureSCCRoleBinding_CreatesWithClusterRoleWhenNoAnnotation(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateSCCRoleBinding: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + r.securityClientSet = nil // no SCC annotation → security client not needed + + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + rb, err := kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), pipelinesSCCRoleBinding, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, "ClusterRole", rb.RoleRef.Kind) + assert.Equal(t, pipelinesSCCClusterRole, rb.RoleRef.Name) + assert.Equal(t, pipelineSA, rb.Subjects[0].Name) +} + +func TestEnsureSCCRoleBinding_Idempotent(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateSCCRoleBinding: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + r.securityClientSet = nil + + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + // Exactly one RoleBinding. + rbs, err := kubeClient.RbacV1().RoleBindings("my-ns").List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + assert.Equal(t, 1, len(rbs.Items)) +} + +func TestEnsureSCCRoleBinding_DeletesLeftoverRoleWhenNoAnnotation(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + CreateSCCRoleBinding: boolPtr(true), + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + r.securityClientSet = nil + + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + // Pre-create a leftover namespace-scoped role (from a previous annotation). + _, err = kubeClient.RbacV1().Roles("my-ns").Create(context.Background(), &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{Name: pipelinesSCCRole, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + // Role must be deleted. + _, err = kubeClient.RbacV1().Roles("my-ns").Get(context.Background(), pipelinesSCCRole, metav1.GetOptions{}) + assert.ErrorContains(t, err, "not found") +} + +// --------------------------------------------------------------------------- +// Stale secret removal +// --------------------------------------------------------------------------- + +func TestEnsureSecretBindings_RemovesStaleNamedBinding(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + SecretBindings: []v1alpha1.SecretBinding{{SecretName: "quay-robot"}}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // SA already bound to a secret that no longer exists. + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "quay-robot"}}, + Secrets: []corev1.ObjectReference{{Name: "quay-robot"}}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + // Secret does NOT exist → should be removed from SA. + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 0, len(sa.ImagePullSecrets)) + assert.Equal(t, 0, len(sa.Secrets)) +} + +func TestEnsureSecretBindings_KeepsUnmanagedSecrets(t *testing.T) { + tc := minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + SecretBindings: []v1alpha1.SecretBinding{{SecretName: "quay-robot"}}, + }) + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, tc, []corev1.Namespace{ns}) + + // SA has a user-added secret that is NOT in any binding. + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "user-added-secret"}}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + err = r.Reconcile(context.Background(), "my-ns") + assert.NilError(t, err) + + sa, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Get(context.Background(), pipelineSA, metav1.GetOptions{}) + assert.NilError(t, err) + // user-added-secret must be preserved — we do not own it. + assert.Equal(t, 1, len(sa.ImagePullSecrets)) + assert.Equal(t, "user-added-secret", sa.ImagePullSecrets[0].Name) +} diff --git a/pkg/reconciler/openshift/openshiftplatform/config.go b/pkg/reconciler/openshift/openshiftplatform/config.go index b2c491930a..d8647f2e36 100644 --- a/pkg/reconciler/openshift/openshiftplatform/config.go +++ b/pkg/reconciler/openshift/openshiftplatform/config.go @@ -19,6 +19,7 @@ package openshiftplatform import ( k8sInstallerSet "github.com/tektoncd/operator/pkg/reconciler/kubernetes/tektoninstallerset" openshiftManualApprovalGate "github.com/tektoncd/operator/pkg/reconciler/openshift/manualapprovalgate" + "github.com/tektoncd/operator/pkg/reconciler/openshift/namespacesync" "github.com/tektoncd/operator/pkg/reconciler/openshift/openshiftpipelinesascode" openshiftSyncerService "github.com/tektoncd/operator/pkg/reconciler/openshift/syncerservice" openshiftAddon "github.com/tektoncd/operator/pkg/reconciler/openshift/tektonaddon" @@ -101,5 +102,9 @@ var ( Name: string(platform.ControllerSyncerService), ControllerConstructor: openshiftSyncerService.NewController, }, + platform.ControllerNamespaceSync: injection.NamedControllerConstructor{ + Name: string(platform.ControllerNamespaceSync), + ControllerConstructor: namespacesync.NewController, + }, } ) diff --git a/pkg/reconciler/platform/const.go b/pkg/reconciler/platform/const.go index 4dcaeb625c..f861ffe4de 100644 --- a/pkg/reconciler/platform/const.go +++ b/pkg/reconciler/platform/const.go @@ -30,6 +30,10 @@ const ( ControllerTektonScheduler ControllerName = "tektonscheduler" ControllerMulticlusterProxyAAE ControllerName = "tektonmulticlusterproxyaae" ControllerSyncerService ControllerName = "syncerservice" + // ControllerNamespaceSync is the OpenShift-only watch-based controller that + // ensures Tekton resources (pipeline SA, CA bundles, RoleBindings, secret + // bindings) are present and up to date in every user namespace. + ControllerNamespaceSync ControllerName = "namespacesync" // ControllerOpenShiftPipelinesAsCode is the operand reconciler for OpenShiftPipelinesAsCode; // the same name is used on Kubernetes and OpenShift so -controllers flags stay consistent. ControllerOpenShiftPipelinesAsCode ControllerName = "openshiftpipelinesascode" From 568da52be613006262b858d64a4dcd06896ecf27 Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 09:25:42 +0200 Subject: [PATCH 2/7] refactor(rbac): delegate per-namespace work to NamespaceSyncController When TektonConfig.spec.platforms.openShift.namespaceSync is configured (which is now the default), the NamespaceSyncController owns all per-namespace resources (pipeline SA, CA bundles, SCC RoleBinding, edit RoleBinding, secret bindings). The legacy rbac.go createResources loop now skips per-namespace iteration and only maintains cluster-scoped resources: - ensurePreRequisites: RBACInstallerSet, pipelines-scc-clusterrole, SCC validation - removeAndUpdateNSFromCI: ClusterInterceptors ClusterRoleBinding cleanup - handleClusterRoleBinding: keep openshift-pipelines-clusterinterceptors in sync using existing pipeline SAs collected via the new collectExistingPipelineSAs helper This commit can be reverted independently to restore the legacy scan-based per-namespace reconciliation without losing the NamespaceSyncController changes. Signed-off-by: Abdeljaouad Khelil Assisted-by: Claude Sonnet 4.6 Co-authored-by: Cursor --- pkg/reconciler/openshift/tektonconfig/rbac.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/pkg/reconciler/openshift/tektonconfig/rbac.go b/pkg/reconciler/openshift/tektonconfig/rbac.go index 739eb95ea6..9c0c3412e5 100644 --- a/pkg/reconciler/openshift/tektonconfig/rbac.go +++ b/pkg/reconciler/openshift/tektonconfig/rbac.go @@ -549,6 +549,33 @@ func (r *rbac) patchNamespaceLabel(ctx context.Context, ns corev1.Namespace) err func (r *rbac) createResources(ctx context.Context) error { logger := logging.FromContext(ctx) + // When NamespaceSync is configured, the NamespaceSyncController owns all + // per-namespace resources (pipeline SA, CA bundles, SCC RoleBinding, edit + // RoleBinding). rbac.go only runs cluster-scoped prerequisites and keeps + // the ClusterInterceptors ClusterRoleBinding in sync using existing SAs. + if r.tektonConfig.Spec.Platforms.OpenShift.NamespaceSync != nil { + if err := r.ensurePreRequisites(ctx); err != nil { + logger.Errorf("error validating cluster-scoped prerequisites: %v", err) + return err + } + if err := r.removeAndUpdateNSFromCI(ctx); err != nil { + logger.Errorf("error cleaning up ClusterInterceptors ClusterRoleBinding: %v", err) + return err + } + nsSAs, err := r.collectExistingPipelineSAs(ctx) + if err != nil { + logger.Errorf("error collecting existing pipeline SAs: %v", err) + return err + } + if len(nsSAs) > 0 { + if err := r.handleClusterRoleBinding(ctx, nsSAs); err != nil { + logger.Errorf("error updating ClusterInterceptors ClusterRoleBinding: %v", err) + return err + } + } + return nil + } + // Step 1: Check feature flags createCABundles := true createRBACResource := true @@ -660,6 +687,31 @@ func (r *rbac) createResources(ctx context.Context) error { return nil } +// collectExistingPipelineSAs lists all pipeline SAs present in non-system namespaces. +// Used when NamespaceSync is active to keep the ClusterInterceptors ClusterRoleBinding +// in sync without re-running the full per-namespace RBAC reconciliation loop. +func (r *rbac) collectExistingPipelineSAs(ctx context.Context) ([]NamespaceServiceAccount, error) { + namespaces, err := r.nsInformer.Lister().List(labels.Everything()) + if err != nil { + return nil, err + } + var result []NamespaceServiceAccount + for _, ns := range namespaces { + if shouldIgnoreNamespace(*ns) { + continue + } + sa, err := r.kubeClientSet.CoreV1().ServiceAccounts(ns.Name).Get(ctx, pipelineSA, metav1.GetOptions{}) + if errors.IsNotFound(err) { + continue + } + if err != nil { + return nil, err + } + result = append(result, NamespaceServiceAccount{ServiceAccount: sa, Namespace: *ns}) + } + return result, nil +} + func (r *rbac) createSCCFailureEventInNamespace(ctx context.Context, namespace string, scc string) error { logger := logging.FromContext(ctx) From 796c2dd20109dfc7e8cc83a89dbe45a9ff99eae9 Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 09:52:21 +0200 Subject: [PATCH 3/7] feat(namespacesync): incrementally manage ClusterInterceptors ClusterRoleBinding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The openshift-pipelines-clusterinterceptors ClusterRoleBinding grants the pipeline SA in every user namespace get/list/watch access to ClusterInterceptors, which EventListeners need to resolve webhook interceptors. The previous implementation in rbac.go rebuilt the full subject list on every TektonConfig reconcile (O(N) API calls). NamespaceSyncController now manages this ClusterRoleBinding with a per-namespace patch: - Pipeline SA created / namespace reconciled → subject is added (idempotent). - Pipeline SA deleted → reconcile of that namespace removes the subject. - RetryOnConflict handles concurrent updates from parallel namespace workers. - ClusterRole is bootstrapped once; it never changes so no update path needed. Five new tests cover: creation, no-op when SA absent, idempotency, multi-namespace accumulation, and subject removal on SA deletion. Signed-off-by: Abdeljaoued Khelil Assisted-by: Claude Sonnet 4.6 Co-authored-by: Cursor --- .../openshift/namespacesync/reconciler.go | 130 ++++++++++++++++-- .../namespacesync/reconciler_test.go | 103 ++++++++++++++ 2 files changed, 225 insertions(+), 8 deletions(-) diff --git a/pkg/reconciler/openshift/namespacesync/reconciler.go b/pkg/reconciler/openshift/namespacesync/reconciler.go index 0b88701f3c..f9f65d5685 100644 --- a/pkg/reconciler/openshift/namespacesync/reconciler.go +++ b/pkg/reconciler/openshift/namespacesync/reconciler.go @@ -46,14 +46,15 @@ import ( ) const ( - pipelineSA = "pipeline" - pipelinesSCCRole = "pipelines-scc-role" - pipelinesSCCClusterRole = "pipelines-scc-clusterrole" - pipelinesSCCRoleBinding = "pipelines-scc-rolebinding" - editRoleBinding = "openshift-pipelines-edit" - editClusterRole = "edit" - serviceCABundleConfigMap = "config-service-cabundle" - trustedCABundleConfigMap = "config-trusted-cabundle" + pipelineSA = "pipeline" + pipelinesSCCRole = "pipelines-scc-role" + pipelinesSCCClusterRole = "pipelines-scc-clusterrole" + pipelinesSCCRoleBinding = "pipelines-scc-rolebinding" + editRoleBinding = "openshift-pipelines-edit" + editClusterRole = "edit" + serviceCABundleConfigMap = "config-service-cabundle" + trustedCABundleConfigMap = "config-trusted-cabundle" + clusterInterceptorsClusterRole = "openshift-pipelines-clusterinterceptors" ) var nsIgnoreRegex = regexp.MustCompile(reconcilerCommon.NamespaceIgnorePattern) @@ -119,6 +120,14 @@ func (r *Reconciler) reconcileNamespace(ctx context.Context, ns *corev1.Namespac } } + // Keep the cluster-wide ClusterInterceptors ClusterRoleBinding up to date. + // This is an incremental patch: add/remove just this namespace's pipeline SA + // subject, rather than rebuilding the full subject list on every reconcile. + if err := r.ensureClusterInterceptorsSubject(ctx, ns.Name, tc); err != nil { + logger.Errorf("failed to sync ClusterInterceptors subject for %s: %v", ns.Name, err) + return err + } + if cfg.CreateSCCRoleBinding != nil && *cfg.CreateSCCRoleBinding { if err := r.ensureSCCRoleBinding(ctx, ns, tc); err != nil { logger.Errorf("failed to ensure SCC RoleBinding in %s: %v", ns.Name, err) @@ -655,6 +664,111 @@ func bindSecretToSA(sa *corev1.ServiceAccount, secretName string) bool { return changed } +// --------------------------------------------------------------------------- +// ClusterInterceptors ClusterRoleBinding +// --------------------------------------------------------------------------- + +// ensureClusterInterceptorsSubject manages this namespace's pipeline SA in the +// openshift-pipelines-clusterinterceptors ClusterRoleBinding. +// +// EventListeners use ClusterInterceptors (cluster-scoped) to validate webhook +// payloads. Because ClusterInterceptors are cluster-scoped, only a +// ClusterRoleBinding can grant access — a namespace RoleBinding is not enough. +// This method patches exactly one subject (add or remove) per namespace +// reconcile, replacing the legacy batch-rebuild approach in rbac.go. +// +// - SA exists → subject is added (idempotent). +// - SA absent → subject is removed (handles SA deletion self-healing). +func (r *Reconciler) ensureClusterInterceptorsSubject(ctx context.Context, nsName string, tc *v1alpha1.TektonConfig) error { + logger := logging.FromContext(ctx) + rbacClient := r.kubeClient.RbacV1() + ownerRef := tektonConfigOwnerRef(tc) + + // Ensure the static ClusterRole exists (content never changes). + if _, err := rbacClient.ClusterRoles().Get(ctx, clusterInterceptorsClusterRole, metav1.GetOptions{}); errors.IsNotFound(err) { + logger.Infof("Creating ClusterRole %s", clusterInterceptorsClusterRole) + _, err = rbacClient.ClusterRoles().Create(ctx, &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterInterceptorsClusterRole, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + Rules: []rbacv1.PolicyRule{{ + APIGroups: []string{"triggers.tekton.dev"}, + Resources: []string{"clusterinterceptors"}, + Verbs: []string{"get", "list", "watch"}, + }}, + }, metav1.CreateOptions{}) + if err != nil && !errors.IsAlreadyExists(err) { + return err + } + } + + // Determine whether the pipeline SA currently exists in this namespace. + _, saErr := r.kubeClient.CoreV1().ServiceAccounts(nsName).Get(ctx, pipelineSA, metav1.GetOptions{}) + saExists := saErr == nil + if saErr != nil && !errors.IsNotFound(saErr) { + return saErr + } + + subject := rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: pipelineSA, + Namespace: nsName, + } + + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + crb, err := rbacClient.ClusterRoleBindings().Get(ctx, clusterInterceptorsClusterRole, metav1.GetOptions{}) + if errors.IsNotFound(err) { + if !saExists { + return nil // nothing to create when SA is absent + } + logger.Infof("Creating ClusterRoleBinding %s", clusterInterceptorsClusterRole) + _, err = rbacClient.ClusterRoleBindings().Create(ctx, &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterInterceptorsClusterRole, + OwnerReferences: []metav1.OwnerReference{ownerRef}, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: clusterInterceptorsClusterRole, + }, + Subjects: []rbacv1.Subject{subject}, + }, metav1.CreateOptions{}) + return err + } + if err != nil { + return err + } + + changed := false + if saExists { + if !hasSubject(crb.Subjects, subject) { + logger.Infof("Adding %s/pipeline to ClusterInterceptors binding", nsName) + crb.Subjects = append(crb.Subjects, subject) + changed = true + } + } else { + newSubjects := make([]rbacv1.Subject, 0, len(crb.Subjects)) + for _, s := range crb.Subjects { + if s.Kind == rbacv1.ServiceAccountKind && s.Name == pipelineSA && s.Namespace == nsName { + logger.Infof("Removing %s/pipeline from ClusterInterceptors binding", nsName) + changed = true + continue + } + newSubjects = append(newSubjects, s) + } + crb.Subjects = newSubjects + } + + if !changed { + return nil + } + _, err = rbacClient.ClusterRoleBindings().Update(ctx, crb, metav1.UpdateOptions{}) + return err + }) +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- diff --git a/pkg/reconciler/openshift/namespacesync/reconciler_test.go b/pkg/reconciler/openshift/namespacesync/reconciler_test.go index c0668f6c23..600508a96e 100644 --- a/pkg/reconciler/openshift/namespacesync/reconciler_test.go +++ b/pkg/reconciler/openshift/namespacesync/reconciler_test.go @@ -26,6 +26,7 @@ import ( "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeinformers "k8s.io/client-go/informers" kubefake "k8s.io/client-go/kubernetes/fake" @@ -677,3 +678,105 @@ func TestEnsureSecretBindings_KeepsUnmanagedSecrets(t *testing.T) { assert.Equal(t, 1, len(sa.ImagePullSecrets)) assert.Equal(t, "user-added-secret", sa.ImagePullSecrets[0].Name) } + +// --------------------------------------------------------------------------- +// ClusterInterceptors ClusterRoleBinding tests +// --------------------------------------------------------------------------- + +func TestClusterInterceptors_CreatesCRBWhenSAExists(t *testing.T) { + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, minimalTC(&v1alpha1.NamespaceSyncConfig{}), []corev1.Namespace{ns}) + + // Pipeline SA exists — reconcile should add it to the ClusterRoleBinding. + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + crb, err := kubeClient.RbacV1().ClusterRoleBindings().Get(context.Background(), clusterInterceptorsClusterRole, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 1, len(crb.Subjects)) + assert.Equal(t, "my-ns", crb.Subjects[0].Namespace) + assert.Equal(t, pipelineSA, crb.Subjects[0].Name) +} + +func TestClusterInterceptors_NoCRBWhenSAAbsent(t *testing.T) { + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, minimalTC(&v1alpha1.NamespaceSyncConfig{ + CreatePipelineSA: boolPtr(false), + }), []corev1.Namespace{ns}) + + // No SA — ClusterRoleBinding must not be created. + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + _, err := kubeClient.RbacV1().ClusterRoleBindings().Get(context.Background(), clusterInterceptorsClusterRole, metav1.GetOptions{}) + assert.Assert(t, errors.IsNotFound(err), "expected ClusterRoleBinding to be absent, got: %v", err) +} + +func TestClusterInterceptors_Idempotent(t *testing.T) { + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}} + r, kubeClient := newTestReconciler(t, minimalTC(&v1alpha1.NamespaceSyncConfig{}), []corev1.Namespace{ns}) + + _, err := kubeClient.CoreV1().ServiceAccounts("my-ns").Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: "my-ns"}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + assert.NilError(t, r.Reconcile(context.Background(), "my-ns")) + + crb, err := kubeClient.RbacV1().ClusterRoleBindings().Get(context.Background(), clusterInterceptorsClusterRole, metav1.GetOptions{}) + assert.NilError(t, err) + // Must not have duplicate subjects. + count := 0 + for _, s := range crb.Subjects { + if s.Namespace == "my-ns" && s.Name == pipelineSA { + count++ + } + } + assert.Equal(t, 1, count) +} + +func TestClusterInterceptors_AddsMultipleNamespaces(t *testing.T) { + nsA := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-a"}} + nsB := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-b"}} + r, kubeClient := newTestReconciler(t, minimalTC(&v1alpha1.NamespaceSyncConfig{}), []corev1.Namespace{nsA, nsB}) + + for _, nsName := range []string{"ns-a", "ns-b"} { + _, err := kubeClient.CoreV1().ServiceAccounts(nsName).Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: nsName}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + assert.NilError(t, r.Reconcile(context.Background(), nsName)) + } + + crb, err := kubeClient.RbacV1().ClusterRoleBindings().Get(context.Background(), clusterInterceptorsClusterRole, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 2, len(crb.Subjects)) +} + +func TestClusterInterceptors_RemovesSubjectWhenSADeleted(t *testing.T) { + nsA := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-a"}} + nsB := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-b"}} + r, kubeClient := newTestReconciler(t, minimalTC(&v1alpha1.NamespaceSyncConfig{}), []corev1.Namespace{nsA, nsB}) + + // Create SAs in both namespaces and reconcile. + for _, nsName := range []string{"ns-a", "ns-b"} { + _, err := kubeClient.CoreV1().ServiceAccounts(nsName).Create(context.Background(), &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineSA, Namespace: nsName}, + }, metav1.CreateOptions{}) + assert.NilError(t, err) + assert.NilError(t, r.Reconcile(context.Background(), nsName)) + } + + // Delete ns-a's pipeline SA then reconcile ns-a — its subject must be removed. + assert.NilError(t, kubeClient.CoreV1().ServiceAccounts("ns-a").Delete(context.Background(), pipelineSA, metav1.DeleteOptions{})) + assert.NilError(t, r.Reconcile(context.Background(), "ns-a")) + + crb, err := kubeClient.RbacV1().ClusterRoleBindings().Get(context.Background(), clusterInterceptorsClusterRole, metav1.GetOptions{}) + assert.NilError(t, err) + assert.Equal(t, 1, len(crb.Subjects)) + assert.Equal(t, "ns-b", crb.Subjects[0].Namespace) +} From 3c9f131f574c33dd96beb2e0ec1ebd14b550080f Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 09:52:30 +0200 Subject: [PATCH 4/7] refactor(rbac): remove ClusterInterceptors batch management from rbac.go Now that NamespaceSyncController handles the openshift-pipelines-clusterinterceptors ClusterRoleBinding incrementally per namespace, rbac.go no longer needs to: - Accumulate all pipeline SAs via collectExistingPipelineSAs - Rebuild the full subject list via handleClusterRoleBinding The NamespaceSync guard in createResources is simplified: it now only calls ensurePreRequisites (cluster-scoped SCC ClusterRole + validation) before returning; all other RBAC and secret binding work is owned by the new controller. Signed-off-by: Abdeljaoued Khelil Assisted-by: Claude Sonnet 4.6 Co-authored-by: Cursor --- pkg/reconciler/openshift/tektonconfig/rbac.go | 44 ++----------------- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/pkg/reconciler/openshift/tektonconfig/rbac.go b/pkg/reconciler/openshift/tektonconfig/rbac.go index 9c0c3412e5..5963dadd5f 100644 --- a/pkg/reconciler/openshift/tektonconfig/rbac.go +++ b/pkg/reconciler/openshift/tektonconfig/rbac.go @@ -554,25 +554,14 @@ func (r *rbac) createResources(ctx context.Context) error { // RoleBinding). rbac.go only runs cluster-scoped prerequisites and keeps // the ClusterInterceptors ClusterRoleBinding in sync using existing SAs. if r.tektonConfig.Spec.Platforms.OpenShift.NamespaceSync != nil { + // Per-namespace resources (pipeline SA, CA bundles, SCC RoleBinding, edit + // RoleBinding, secret bindings, ClusterInterceptors subjects) are fully + // owned by the NamespaceSyncController. Only maintain cluster-scoped + // prerequisites here. if err := r.ensurePreRequisites(ctx); err != nil { logger.Errorf("error validating cluster-scoped prerequisites: %v", err) return err } - if err := r.removeAndUpdateNSFromCI(ctx); err != nil { - logger.Errorf("error cleaning up ClusterInterceptors ClusterRoleBinding: %v", err) - return err - } - nsSAs, err := r.collectExistingPipelineSAs(ctx) - if err != nil { - logger.Errorf("error collecting existing pipeline SAs: %v", err) - return err - } - if len(nsSAs) > 0 { - if err := r.handleClusterRoleBinding(ctx, nsSAs); err != nil { - logger.Errorf("error updating ClusterInterceptors ClusterRoleBinding: %v", err) - return err - } - } return nil } @@ -687,31 +676,6 @@ func (r *rbac) createResources(ctx context.Context) error { return nil } -// collectExistingPipelineSAs lists all pipeline SAs present in non-system namespaces. -// Used when NamespaceSync is active to keep the ClusterInterceptors ClusterRoleBinding -// in sync without re-running the full per-namespace RBAC reconciliation loop. -func (r *rbac) collectExistingPipelineSAs(ctx context.Context) ([]NamespaceServiceAccount, error) { - namespaces, err := r.nsInformer.Lister().List(labels.Everything()) - if err != nil { - return nil, err - } - var result []NamespaceServiceAccount - for _, ns := range namespaces { - if shouldIgnoreNamespace(*ns) { - continue - } - sa, err := r.kubeClientSet.CoreV1().ServiceAccounts(ns.Name).Get(ctx, pipelineSA, metav1.GetOptions{}) - if errors.IsNotFound(err) { - continue - } - if err != nil { - return nil, err - } - result = append(result, NamespaceServiceAccount{ServiceAccount: sa, Namespace: *ns}) - } - return result, nil -} - func (r *rbac) createSCCFailureEventInNamespace(ctx context.Context, namespace string, scc string) error { logger := logging.FromContext(ctx) From 75aed5e30c866dfccb0bd132b1cd2f6e7134c975 Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 10:03:54 +0200 Subject: [PATCH 5/7] refactor(rbac): remove dead per-namespace code, keep only cluster-scoped work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that NamespaceSyncController owns all per-namespace resources (pipeline SA, CA bundles, SCC RoleBinding, edit RoleBinding, secret bindings, ClusterInterceptors subjects) and NamespaceSync is always defaulted to non-nil, the legacy per-namespace branch of rbac.go is permanently unreachable. Removed (~1 350 lines): - shouldIgnoreNamespace, needsRBAC, needsCABundle, getNamespacesToBeReconciled - processRBAC, handleSCCInNamespace, getSCCRoleInNamespace, patchNamespaceLabel - ensureSA, createSA, ensureSCCRoleInNamespace - ensurePipelinesSCCRoleBinding, createSCCRoleBinding, updateRoleBinding - ensureCABundles, ensureCABundlesInNamespace, createCABundleConfigMaps - createServiceCABundleConfigMap, patchNamespaceTrustedConfigLabel - ensureRoleBindings, createRoleBinding, isLegacyRBACEnabled - handleClusterRoleBinding, bulkUpdateClusterRoleBinding, bulkCreateClusterRoleBinding - createClusterRole, removeAndUpdateNSFromCI, removeIndex - updateOwnerRefs, removeAndUpdate, hasSubject, CompareSubjects, mergeSubjects - hasOwnerRefernce, cleanUpRBACNameChange (TODO: remove after v0.55.0 - now done) - NamespaceServiceAccount, NamespacesToReconcile types Kept: - createResources → only calls ensurePreRequisites (cluster-scoped) - ensurePreRequisites: InstallerSet, SCC validation, pipelines-scc-clusterrole - ensurePipelinesSCClusterRole: dynamic ClusterRole for default SCC - EnsureRBACInstallerSet, cleanUp, setDefault, removeObsoleteRBACInstallerSet extension.go: removed rbacInformer field (no longer needed), removed cleanUpRBACNameChange call, removed unused rbacV1 and rbacInformer imports. rbac_test.go: replaced 500-line dead-path test with focused tests for createResources (prerequisites path) and setDefault. Signed-off-by: Abdeljaoued Khelil Assisted-by: Claude Sonnet 4.6 Co-authored-by: Cursor --- .../openshift/tektonconfig/common.go | 17 - .../openshift/tektonconfig/extension.go | 30 - pkg/reconciler/openshift/tektonconfig/rbac.go | 1468 +---------------- .../openshift/tektonconfig/rbac_test.go | 738 +-------- 4 files changed, 148 insertions(+), 2105 deletions(-) diff --git a/pkg/reconciler/openshift/tektonconfig/common.go b/pkg/reconciler/openshift/tektonconfig/common.go index be24543501..c6c78d1045 100644 --- a/pkg/reconciler/openshift/tektonconfig/common.go +++ b/pkg/reconciler/openshift/tektonconfig/common.go @@ -77,23 +77,6 @@ func makeInstallerSet(tc *v1alpha1.TektonConfig, releaseVersion string) *v1alpha } } -func deleteInstallerSet(ctx context.Context, oc versioned.Interface, tc *v1alpha1.TektonConfig, component string) error { - labelSelector, err := common.LabelSelector(rbacInstallerSetSelector) - if err != nil { - return err - } - err = oc.OperatorV1alpha1().TektonInstallerSets().DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{ - LabelSelector: labelSelector, - }) - if err != nil { - return err - } - // clear the name of installer set from TektonConfig status - delete(tc.Status.TektonInstallerSet, component) - - return nil -} - // checkIfInstallerSetExist checks if installer set exists for a component and return true/false based on it // and if installer set which already exist is of older version then it deletes and return false to create a new // installer set diff --git a/pkg/reconciler/openshift/tektonconfig/extension.go b/pkg/reconciler/openshift/tektonconfig/extension.go index dd0fc1ffb1..0cf17cd460 100644 --- a/pkg/reconciler/openshift/tektonconfig/extension.go +++ b/pkg/reconciler/openshift/tektonconfig/extension.go @@ -37,11 +37,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" nsV1 "k8s.io/client-go/informers/core/v1" - rbacV1 "k8s.io/client-go/informers/rbac/v1" "k8s.io/client-go/kubernetes" kubeclient "knative.dev/pkg/client/injection/kube/client" namespaceinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" - rbacInformer "knative.dev/pkg/client/injection/kube/informers/rbac/v1/clusterrolebinding" "knative.dev/pkg/logging" ) @@ -59,7 +57,6 @@ func OpenShiftExtension(ctx context.Context) common.Extension { ext := openshiftExtension{ operatorClientSet: operatorclient.Get(ctx), kubeClientSet: kubeclient.Get(ctx), - rbacInformer: rbacInformer.Get(ctx), nsInformer: namespaceinformer.Get(ctx), securityClientSet: pkgCommon.GetSecurityClient(ctx), tektonConfigLister: tektonConfiginformer.Get(ctx).Lister(), @@ -79,7 +76,6 @@ func OpenShiftExtension(ctx context.Context) common.Extension { type openshiftExtension struct { operatorClientSet versioned.Interface kubeClientSet kubernetes.Interface - rbacInformer rbacV1.ClusterRoleBindingInformer nsInformer nsV1.NamespaceInformer consolePluginReconciler *consolePluginReconciler @@ -105,15 +101,10 @@ func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.Tekto kubeClientSet: oe.kubeClientSet, operatorClientSet: oe.operatorClientSet, securityClientSet: oe.securityClientSet, - rbacInformer: oe.rbacInformer, - nsInformer: oe.nsInformer, version: os.Getenv(versionKey), tektonConfig: config, } - // set openshift specific defaults - r.setDefault() - // below code helps to retain state of pre-existing SA at the time of upgrade if existingSAWithOwnerRef(r.tektonConfig) { logger := logging.FromContext(ctx) @@ -150,27 +141,6 @@ func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.Tekto logger.Infof("Successfully patched TektonConfig with serviceAccountCreationLabel set to true") } - for _, v := range config.Spec.Params { - // check for param name and if its matches to createRbacResource - // then disable auto creation of RBAC resources by deleting installerSet - if v.Name == rbacParamName && v.Value == "false" { - if err := deleteInstallerSet(ctx, r.operatorClientSet, r.tektonConfig, componentNameRBAC); err != nil { - return err - } - // remove openshift-pipelines.tekton.dev/namespace-reconcile-version label from namespaces while deleting RBAC resources. - if err := r.cleanUp(ctx); err != nil { - return err - } - } - } - - // TODO: Remove this after v0.55.0 release, by following a depreciation notice - // -------------------- - if err := r.cleanUpRBACNameChange(ctx); err != nil { - return err - } - // -------------------- - // Resolve the central TLS profile once per reconcile cycle and cache it in the // console plugin reconciler. PostReconcile consumes the cached value without // re-reading the APIServer. The APIServer watch in controller.go ensures that diff --git a/pkg/reconciler/openshift/tektonconfig/rbac.go b/pkg/reconciler/openshift/tektonconfig/rbac.go index 5963dadd5f..6860a4229b 100644 --- a/pkg/reconciler/openshift/tektonconfig/rbac.go +++ b/pkg/reconciler/openshift/tektonconfig/rbac.go @@ -18,59 +18,45 @@ package tektonconfig import ( "context" - "encoding/json" "fmt" - "math" "regexp" - "time" security "github.com/openshift/client-go/security/clientset/versioned" "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned" "github.com/tektoncd/operator/pkg/common" reconcilerCommon "github.com/tektoncd/operator/pkg/reconciler/common" - "github.com/tektoncd/operator/pkg/reconciler/openshift" - corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" - "k8s.io/apimachinery/pkg/types" - nsV1 "k8s.io/client-go/informers/core/v1" - rbacV1 "k8s.io/client-go/informers/rbac/v1" "k8s.io/client-go/kubernetes" - v1 "k8s.io/client-go/kubernetes/typed/core/v1" "knative.dev/pkg/logging" ) const ( - pipelinesSCCRole = "pipelines-scc-role" + // pipelinesSCCClusterRole is the cluster-scoped ClusterRole that grants + // `use` on the default SCC configured in TektonConfig. Its content is + // dynamic (depends on TektonConfig.SCC.Default) so it cannot be a static + // manifest and must be managed here. pipelinesSCCClusterRole = "pipelines-scc-clusterrole" - pipelinesSCCRoleBinding = "pipelines-scc-rolebinding" - pipelineSA = "pipeline" - PipelineRoleBinding = "openshift-pipelines-edit" - // TODO: Remove this after v0.55.0 release, by following a depreciation notice - // -------------------- - pipelineRoleBindingOld = "edit" - rbacInstallerSetNameOld = "rbac-resources" - // -------------------- - serviceCABundleConfigMap = "config-service-cabundle" - trustedCABundleConfigMap = "config-trusted-cabundle" - clusterInterceptors = "openshift-pipelines-clusterinterceptors" - namespaceVersionLabel = "openshift-pipelines.tekton.dev/namespace-reconcile-version" - namespaceTrustedConfigLabel = "openshift-pipelines.tekton.dev/namespace-trusted-configmaps-version" - createdByValue = "RBAC" - componentNameRBAC = "rhosp-rbac" - rbacInstallerSetType = "rhosp-rbac" - rbacInstallerSetNamePrefix = "rhosp-rbac-" - rbacParamName = "createRbacResource" - trustedCABundleParamName = "createCABundleConfigMaps" - legacyPipelineRbacParamName = "legacyPipelineRbac" - legacyPipelineRbac = "true" + // serviceAccountCreationLabel is placed on TektonConfig to record that + // pre-existing pipeline SAs have had their owner references migrated. serviceAccountCreationLabel = "openshift-pipelines.tekton.dev/sa-created" + + // namespaceVersionLabel was applied by the old rbac.go batch loop to track + // which namespace version had been reconciled. It is still read by cleanUp + // so that the label is removed on upgrade. + namespaceVersionLabel = "openshift-pipelines.tekton.dev/namespace-reconcile-version" + + createdByValue = "RBAC" + componentNameRBAC = "rhosp-rbac" + rbacInstallerSetType = "rhosp-rbac" + rbacInstallerSetNamePrefix = "rhosp-rbac-" + + // rbacInstallerSetNameOld is the pre-v0.55 installer-set name, kept for cleanup. + rbacInstallerSetNameOld = "rbac-resources" ) var ( @@ -80,49 +66,36 @@ var ( v1alpha1.InstallerSetType: componentNameRBAC, }, } -) -// Namespace Regex to ignore the namespace for creating rbac resources. -var nsRegex = regexp.MustCompile(reconcilerCommon.NamespaceIgnorePattern) + // nsRegex filters system/platform namespaces when cleanUp removes the + // legacy namespaceVersionLabel. + nsRegex = regexp.MustCompile(reconcilerCommon.NamespaceIgnorePattern) +) type rbac struct { kubeClientSet kubernetes.Interface operatorClientSet clientset.Interface securityClientSet security.Interface - rbacInformer rbacV1.ClusterRoleBindingInformer - nsInformer nsV1.NamespaceInformer ownerRef metav1.OwnerReference version string tektonConfig *v1alpha1.TektonConfig } -type NamespaceServiceAccount struct { - ServiceAccount *corev1.ServiceAccount - Namespace corev1.Namespace -} - -// NamespacesToReconcile holds the namespaces that need reconciliation for different features -type NamespacesToReconcile struct { - RBACNamespaces []corev1.Namespace - CANamespaces []corev1.Namespace -} - +// cleanUp removes the legacy namespaceVersionLabel from all namespaces that +// carry it. Called when the rhosp-rbac InstallerSet is (re-)created so that +// the next reconcile cycle re-evaluates every namespace. func (r *rbac) cleanUp(ctx context.Context) error { - - // fetch the list of all namespaces which have label - // `openshift-pipelines.tekton.dev/namespace-reconcile-version: ` labelSelector := fmt.Sprintf("%s = %s", namespaceVersionLabel, r.version) namespaces, err := r.kubeClientSet.CoreV1().Namespaces().List(ctx, metav1.ListOptions{ LabelSelector: labelSelector, }) if err != nil { - return fmt.Errorf("failed to retreive namespaces with labelSeleclector %s: %v", labelSelector, err) + return fmt.Errorf("failed to retrieve namespaces with labelSelector %s: %v", labelSelector, err) } - // loop on namespaces and remove label if exist for _, n := range namespaces.Items { - labels := n.GetLabels() - delete(labels, namespaceVersionLabel) - n.SetLabels(labels) + lbls := n.GetLabels() + delete(lbls, namespaceVersionLabel) + n.SetLabels(lbls) if _, err := r.kubeClientSet.CoreV1().Namespaces().Update(ctx, &n, metav1.UpdateOptions{}); err != nil { return fmt.Errorf("failed to update namespace %s: %v", n.Name, err) } @@ -130,6 +103,7 @@ func (r *rbac) cleanUp(ctx context.Context) error { return nil } +// EnsureRBACInstallerSet creates or returns the rhosp-rbac TektonInstallerSet. func (r *rbac) EnsureRBACInstallerSet(ctx context.Context) (*v1alpha1.TektonInstallerSet, error) { if err := r.removeObsoleteRBACInstallerSet(ctx); err != nil { return nil, err @@ -139,80 +113,23 @@ func (r *rbac) EnsureRBACInstallerSet(ctx context.Context) (*v1alpha1.TektonInst if err != nil { return nil, err } - if rbacISet != nil { return rbacISet, nil } - // A new installer needs to be created - // either because of operator version upgrade or installerSet gone missing; - // therefore all relevant namespaces need to be reconciled for RBAC resources. - // Hence, remove the necessary labels to ensure that the namespaces will be 'not skipped' - // RBAC reconcile logic - err = r.cleanUp(ctx) - if err != nil { + + // InstallerSet missing — remove version labels so every namespace is + // re-evaluated by the NamespaceSyncController, then create a fresh set. + if err := r.cleanUp(ctx); err != nil { return nil, err } - - err = createInstallerSet(ctx, r.operatorClientSet, r.tektonConfig, r.version) - if err != nil { + if err := createInstallerSet(ctx, r.operatorClientSet, r.tektonConfig, r.version); err != nil { return nil, err } return nil, v1alpha1.RECONCILE_AGAIN_ERR } -func (r *rbac) setDefault() { - var rbacParamFound, legacyParamFound, caBundleParamFound bool - var createRbacResourceValue string - - for i, v := range r.tektonConfig.Spec.Params { - if v.Name == rbacParamName { - rbacParamFound = true - createRbacResourceValue = v.Value - if v.Value != "false" && v.Value != "true" { - r.tektonConfig.Spec.Params[i].Value = "true" - } - } - if v.Name == legacyPipelineRbacParamName { - legacyParamFound = true - if v.Value != "false" && v.Value != "true" { - r.tektonConfig.Spec.Params[i].Value = "true" - } - } - if v.Name == trustedCABundleParamName { - caBundleParamFound = true - if v.Value != "false" && v.Value != "true" { - r.tektonConfig.Spec.Params[i].Value = "true" - } - } - } - if !rbacParamFound { - r.tektonConfig.Spec.Params = append(r.tektonConfig.Spec.Params, v1alpha1.Param{ - Name: rbacParamName, - Value: "true", - }) - } - if !legacyParamFound { - r.tektonConfig.Spec.Params = append(r.tektonConfig.Spec.Params, v1alpha1.Param{ - Name: legacyPipelineRbacParamName, - Value: "true", - }) - } - - // TODO: Remove this upgrade workaround after version 1.22. - // This logic is only needed to preserve backward compatibility for users upgrading to 1.21 - // who had createRbacResource=false and no createCABundleConfigMaps param set. - if !caBundleParamFound { - defaultVal := "true" - if rbacParamFound && createRbacResourceValue == "false" { - defaultVal = "false" - } - r.tektonConfig.Spec.Params = append(r.tektonConfig.Spec.Params, - v1alpha1.Param{Name: trustedCABundleParamName, Value: defaultVal}, - ) - } -} - -// ensurePreRequisites validates the resources before creation +// ensurePreRequisites validates cluster-scoped SCC prerequisites and keeps +// the pipelines-scc-clusterrole ClusterRole up to date. func (r *rbac) ensurePreRequisites(ctx context.Context) error { logger := logging.FromContext(ctx) @@ -222,10 +139,8 @@ func (r *rbac) ensurePreRequisites(ctx context.Context) error { } r.ownerRef = configOwnerRef(*rbacISet) - // make sure default SCC is in place defaultSCC := r.tektonConfig.Spec.Platforms.OpenShift.SCC.Default if defaultSCC == "" { - // Should not really happen due to defaulting, but okay... return fmt.Errorf("tektonConfig.Spec.Platforms.OpenShift.SCC.Default cannot be empty") } logger.Debugf("default SCC set to: %s", defaultSCC) @@ -238,13 +153,11 @@ func (r *rbac) ensurePreRequisites(ctx context.Context) error { return err } - // validate maxAllowed SCC maxAllowedSCC := r.tektonConfig.Spec.Platforms.OpenShift.SCC.MaxAllowed if maxAllowedSCC != "" { if err := common.VerifySCCExists(ctx, maxAllowedSCC, r.securityClientSet); err != nil { return fmt.Errorf("failed to verify scc %s exists, %w", maxAllowedSCC, err) } - isPriority, err := common.SCCAMoreRestrictiveThanB(prioritizedSCCList, defaultSCC, maxAllowedSCC) if err != nil { return err @@ -258,1299 +171,60 @@ func (r *rbac) ensurePreRequisites(ctx context.Context) error { logger.Debug("No maxAllowed SCC set in TektonConfig") } - // Maintaining a separate cluster role for the scc declaration. - // to assist us in managing this the scc association in a - // granular way. - // We need to make sure the pipelines-scc-clusterrole is up-to-date - // irrespective of the fact that we get reconcilable namespaces or not. - if err := r.ensurePipelinesSCClusterRole(ctx); err != nil { - return err - } - - return nil -} - -// shouldIgnoreNamespace returns true if the namespace should be skipped during reconciliation. -// System namespaces (matching ^(openshift|kube)-) and namespaces being deleted are ignored. -func shouldIgnoreNamespace(ns corev1.Namespace) bool { - if nsRegex.MatchString(ns.GetName()) { - return true - } - if ns.GetObjectMeta().GetDeletionTimestamp() != nil { - return true - } - return false -} - -// needsRBAC checks whether the given namespace requires RBAC reconciliation. -func (r *rbac) needsRBAC(ctx context.Context, ns corev1.Namespace) (bool, error) { - logger := logging.FromContext(ctx) - - // We want to monitor namespaces with the SCC annotation set - if ns.Annotations[openshift.NamespaceSCCAnnotation] != "" { - return true, nil - } - // Accept namespaces that have not been reconciled yet - if ns.Labels[namespaceVersionLabel] != r.version { - return true, nil - } - - // Now we're left with namespaces that have already been reconciled. - // We must make sure that the default SCC is in force via the ClusterRole. - sccRoleBinding, err := r.kubeClientSet.RbacV1().RoleBindings(ns.Name).Get(ctx, pipelinesSCCRoleBinding, metav1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - logger.Debugf("could not find roleBinding %s in namespace %s", pipelinesSCCRoleBinding, ns.Name) - return true, nil - } - return false, fmt.Errorf("error fetching rolebinding %s from namespace %s: %w", pipelinesSCCRoleBinding, ns.Name, err) - } - if sccRoleBinding.RoleRef.Kind != "ClusterRole" { - logger.Infof("RoleBinding %s in namespace: %s should have ClusterRole with default SCC, will reconcile again...", pipelinesSCCRoleBinding, ns.Name) - return true, nil - } - - return false, nil -} - -// needsCABundle checks whether the given namespace requires CA bundle configmap reconciliation. -func (r *rbac) needsCABundle(ctx context.Context, ns corev1.Namespace) (bool, error) { - logger := logging.FromContext(ctx) - - if ns.Labels[namespaceTrustedConfigLabel] != r.version { - return true, nil - } - - // Self-healing: verify configmaps exist even when label matches - cmClient := r.kubeClientSet.CoreV1().ConfigMaps(ns.Name) - _, err1 := cmClient.Get(ctx, trustedCABundleConfigMap, metav1.GetOptions{}) - _, err2 := cmClient.Get(ctx, serviceCABundleConfigMap, metav1.GetOptions{}) - if errors.IsNotFound(err1) || errors.IsNotFound(err2) { - logger.Warnf("CA bundle configmaps missing in namespace %s despite label indicating reconciliation complete, will re-reconcile", ns.Name) - return true, nil - } - if err1 != nil { - return false, fmt.Errorf("error checking configmap %s in namespace %s: %w", trustedCABundleConfigMap, ns.Name, err1) - } - if err2 != nil { - return false, fmt.Errorf("error checking configmap %s in namespace %s: %w", serviceCABundleConfigMap, ns.Name, err2) - } - - return false, nil -} - -func (r *rbac) getNamespacesToBeReconciled(ctx context.Context) (*NamespacesToReconcile, error) { - logger := logging.FromContext(ctx) - - // fetch the list of all namespaces - allNamespaces, err := r.kubeClientSet.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) - if err != nil { - return nil, err - } - - result := &NamespacesToReconcile{ - RBACNamespaces: []corev1.Namespace{}, - CANamespaces: []corev1.Namespace{}, - } - - for _, ns := range allNamespaces.Items { - if shouldIgnoreNamespace(ns) { - logger.Debugf("Ignoring namespace: %s", ns.GetName()) - continue - } - - reconcileRBAC, err := r.needsRBAC(ctx, ns) - if err != nil { - return nil, err - } - if reconcileRBAC { - logger.Debugf("Adding namespace for RBAC reconciliation: %s", ns.GetName()) - result.RBACNamespaces = append(result.RBACNamespaces, ns) - } - - caBundle, err := r.needsCABundle(ctx, ns) - if err != nil { - return nil, err - } - if caBundle { - logger.Debugf("Adding namespace for CA bundle reconciliation: %s", ns.GetName()) - result.CANamespaces = append(result.CANamespaces, ns) - } - } - - return result, nil -} - -func (r *rbac) getSCCRoleInNamespace(ns *corev1.Namespace) *rbacv1.RoleRef { - nsAnnotations := ns.GetAnnotations() - nsSCC := nsAnnotations[openshift.NamespaceSCCAnnotation] - // If SCC is requested by namespace annotation, then we need a Role - if nsSCC != "" { - return &rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "Role", - Name: pipelinesSCCRole, - } - } - // If no SCC annotation is present in the namespace, we will use the - // pipelines-scc-clusterrole - return &rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "ClusterRole", - Name: pipelinesSCCClusterRole, - } -} - -func (r *rbac) handleSCCInNamespace(ctx context.Context, ns *corev1.Namespace) error { - logger := logging.FromContext(ctx) - - nsName := ns.GetName() - nsAnnotations := ns.GetAnnotations() - nsSCC := nsAnnotations[openshift.NamespaceSCCAnnotation] - - // No SCC is requested in the namespace - if nsSCC == "" { - // If we don't have a namespace annotation, then we don't need a - // Role in this namespace as we will bind to the ClusterRole. - // This happens in cases when the SCC annotation was removed from - // the namespace. - _, err := r.kubeClientSet.RbacV1().Roles(nsName).Get(ctx, pipelinesSCCRole, metav1.GetOptions{}) - if err != nil && !errors.IsNotFound(err) { - return err - } - - // If `err == nil` AND role was found, it means that role exists - if !errors.IsNotFound(err) { - logger.Infof("Found leftover role: %s in namespace: %s, deleting...", pipelinesSCCRole, nsName) - err := r.kubeClientSet.RbacV1().Roles(nsName).Delete(ctx, pipelinesSCCRole, metav1.DeleteOptions{}) - if err != nil && !errors.IsNotFound(err) { - return err - } - } - // Don't proceed further if no SCC requested by namespace - return nil - } - - // We're here, so the namespace has actually requested an SCC - logger.Infof("Namespace: %s has requested SCC: %s", nsName, nsSCC) - - // Make sure that SCC exists on cluster - if err := common.VerifySCCExists(ctx, nsSCC, r.securityClientSet); err != nil { - logger.Error(err) - - // Create an event in the namespace if the SCC does not exist - eventErr := r.createSCCFailureEventInNamespace(ctx, nsName, nsSCC) - if eventErr != nil { - logger.Errorf("Failed to create SCC not found event in namepsace: %s", nsName) - return eventErr - } - return err - } - - // Make sure SCC requested in the namespace has a lower or equal priority - // than the SCC mentioned in maxAllowed - maxAllowedSCC := r.tektonConfig.Spec.Platforms.OpenShift.SCC.MaxAllowed - if maxAllowedSCC != "" { - prioritizedSCCList, err := common.GetSCCRestrictiveList(ctx, r.securityClientSet) - if err != nil { - return err - } - isPriority, err := common.SCCAMoreRestrictiveThanB(prioritizedSCCList, nsSCC, maxAllowedSCC) - if err != nil { - return err - } - logger.Infof("Is maxAllowed SCC: %s less restrictive than namespace SCC: %s? %t", maxAllowedSCC, nsSCC, isPriority) - if !isPriority { - return fmt.Errorf("namespace: %s has requested SCC: %s, but it is less restrictive than the 'maxAllowed' SCC: %s", nsName, nsSCC, maxAllowedSCC) - } - } - - // Make sure a Role exists with the SCC attached in the namespace - if err := r.ensureSCCRoleInNamespace(ctx, nsName, nsSCC); err != nil { - return err - } - - return nil -} - -// processRBAC encapsulates the logic for processing RBAC in a single namespace. -func (r *rbac) processRBAC(ctx context.Context, ns corev1.Namespace) (*NamespaceServiceAccount, error) { - logger := logging.FromContext(ctx) - logger.Infof("Processing RBAC for namespace %s", ns.GetName()) - - // Create or update ServiceAccount - sa, err := r.ensureSA(ctx, &ns) - if err != nil { - return nil, fmt.Errorf("failed to ensure ServiceAccount in namespace %s: %v", ns.Name, err) - } - - if sa == nil { - return nil, fmt.Errorf("ServiceAccount is nil for namespace %s", ns.Name) - } - - // Handle SCC in namespace - if err := r.handleSCCInNamespace(ctx, &ns); err != nil { - return nil, fmt.Errorf("failed to handle SCC in namespace %s: %v", ns.Name, err) - } - - // Get and apply role reference - roleRef := r.getSCCRoleInNamespace(&ns) - if roleRef != nil { - if err := r.ensurePipelinesSCCRoleBinding(ctx, sa, roleRef); err != nil { - return nil, fmt.Errorf("failed to ensure pipelines SCC role binding in namespace %s: %v", ns.Name, err) - } - } - - // Ensure role bindings - if err := r.ensureRoleBindings(ctx, sa); err != nil { - return nil, fmt.Errorf("failed to ensure role bindings in namespace %s: %v", ns.Name, err) - } - - return &NamespaceServiceAccount{ - ServiceAccount: sa, - Namespace: ns, - }, nil -} - -// patch namespace with reconciled label -func (r *rbac) patchNamespaceLabel(ctx context.Context, ns corev1.Namespace) error { - logger := logging.FromContext(ctx) - - logger.Infof("add label namespace-reconcile-version to mark namespace '%s' as reconciled", ns.Name) - - // Prepare a patch to add/update just one label without overwriting others - patch := map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - namespaceVersionLabel: r.version, - }, - }, - } - - patchPayload, err := json.Marshal(patch) - if err != nil { - logger.Errorf("failed to marshal patch payload: %v", err) - return fmt.Errorf("failed to marshal label patch for namespace %s: %w", ns.Name, err) - } - - // Use PATCH to update just the target label - if _, err := r.kubeClientSet.CoreV1().Namespaces().Patch(ctx, ns.Name, types.StrategicMergePatchType, patchPayload, metav1.PatchOptions{}); err != nil { - logger.Errorf("failed to patch namespace %s: %v", ns.Name, err) - return fmt.Errorf("failed to patch namespace %s: %w", ns.Name, err) - } - - logger.Infof("namespace '%s' successfully reconciled with label %q=%q", ns.Name, namespaceVersionLabel, r.version) - return nil + return r.ensurePipelinesSCClusterRole(ctx) } -// createResources handles the reconciliation of RBAC resources and CA bundle configmaps -// across namespaces. It processes each feature independently based on their respective -// configuration flags and only reconciles namespaces that need updates. +// createResources is called by PreReconcile. +// Per-namespace work (pipeline SA, CA bundles, SCC RoleBinding, edit +// RoleBinding, secret bindings, ClusterInterceptors subjects) is fully owned +// by the NamespaceSyncController. rbac.go only manages cluster-scoped +// prerequisites. func (r *rbac) createResources(ctx context.Context) error { logger := logging.FromContext(ctx) - - // When NamespaceSync is configured, the NamespaceSyncController owns all - // per-namespace resources (pipeline SA, CA bundles, SCC RoleBinding, edit - // RoleBinding). rbac.go only runs cluster-scoped prerequisites and keeps - // the ClusterInterceptors ClusterRoleBinding in sync using existing SAs. - if r.tektonConfig.Spec.Platforms.OpenShift.NamespaceSync != nil { - // Per-namespace resources (pipeline SA, CA bundles, SCC RoleBinding, edit - // RoleBinding, secret bindings, ClusterInterceptors subjects) are fully - // owned by the NamespaceSyncController. Only maintain cluster-scoped - // prerequisites here. - if err := r.ensurePreRequisites(ctx); err != nil { - logger.Errorf("error validating cluster-scoped prerequisites: %v", err) - return err - } - return nil - } - - // Step 1: Check feature flags - createCABundles := true - createRBACResource := true - - // Check feature flags - for _, v := range r.tektonConfig.Spec.Params { - if v.Name == trustedCABundleParamName && v.Value == "false" { - createCABundles = false - logger.Info("CA bundle creation is disabled") - } - if v.Name == rbacParamName && v.Value == "false" { - createRBACResource = false - logger.Info("RBAC resource creation is disabled") - } - } - - // If both features are disabled, nothing to do - if !createCABundles && !createRBACResource { - logger.Info("Both CA bundle and RBAC creation are disabled, nothing to do") - return nil - } - - // Step 2: Ensure prerequisites (only if RBAC is enabled) - if createRBACResource { - if err := r.ensurePreRequisites(ctx); err != nil { - logger.Errorf("error validating resources: %v", err) - return err - } - } - - // Step 3: Get namespaces to be reconciled for both RBAC and CA bundles - namespacesToReconcile, err := r.getNamespacesToBeReconciled(ctx) - if err != nil { - logger.Error(err) + if err := r.ensurePreRequisites(ctx); err != nil { + logger.Errorf("error validating cluster-scoped prerequisites: %v", err) return err } - - // Early return if no namespaces need reconciliation for either feature - if len(namespacesToReconcile.RBACNamespaces) == 0 && len(namespacesToReconcile.CANamespaces) == 0 { - logger.Debug("No namespaces need reconciliation for either RBAC or CA bundles") - return nil - } - - // Step 4: Handle RBAC if enabled - if createRBACResource { - if len(namespacesToReconcile.RBACNamespaces) == 0 { - logger.Debug("No namespaces need RBAC reconciliation") - } else { - logger.Debugf("Found %d namespaces to be reconciled for RBAC", len(namespacesToReconcile.RBACNamespaces)) - - // Remove and update namespaces from Cluster Interceptors - if err := r.removeAndUpdateNSFromCI(ctx); err != nil { - logger.Error(err) - return err - } - - var namespacesToUpdate []NamespaceServiceAccount - // Process each namespace for RBAC - for _, ns := range namespacesToReconcile.RBACNamespaces { - logger.Infof("Processing namespace %s for RBAC", ns.Name) - nsSA, err := r.processRBAC(ctx, ns) - if err != nil { - logger.Errorf("failed processing namespace %s: %v", ns.Name, err) - continue - } - namespacesToUpdate = append(namespacesToUpdate, *nsSA) - } - - // Bulk update ClusterRoleBinding - if len(namespacesToUpdate) > 0 { - if err := r.handleClusterRoleBinding(ctx, namespacesToUpdate); err != nil { - logger.Errorf("failed to ensure clusterrolebinding update: %v", err) - return err - } - logger.Info("Successfully updated cluster role bindings") - - // Patch namespace labels for RBAC - for _, nsSA := range namespacesToUpdate { - logger.Infof("Reconciling namespace %s for RBAC", nsSA.Namespace.Name) - if err := r.patchNamespaceLabel(ctx, nsSA.Namespace); err != nil { - logger.Errorf("failed reconciling namespace %s: %v", nsSA.Namespace.Name, err) - } - } - } - } - } - - // Step 5: Handle CA bundles if enabled - if createCABundles { - if len(namespacesToReconcile.CANamespaces) == 0 { - logger.Debug("No namespaces need CA bundle reconciliation") - } else { - logger.Debugf("Found %d namespaces to be reconciled for CA bundles", len(namespacesToReconcile.CANamespaces)) - - for _, ns := range namespacesToReconcile.CANamespaces { - logger.Infof("Processing namespace %s for CA bundles", ns.Name) - if err := r.ensureCABundlesInNamespace(ctx, &ns); err != nil { - logger.Errorf("failed to ensure CA bundles in namespace %s: %v", ns.Name, err) - continue - } - // Patch namespace with trusted configmaps label - if err := r.patchNamespaceTrustedConfigLabel(ctx, ns); err != nil { - logger.Errorf("failed to patch trusted config label for namespace %s: %v", ns.Name, err) - } - } - } - } - return nil } -func (r *rbac) createSCCFailureEventInNamespace(ctx context.Context, namespace string, scc string) error { +// ensurePipelinesSCClusterRole creates or updates the pipelines-scc-clusterrole +// ClusterRole whose content depends on the dynamic TektonConfig.SCC.Default. +func (r *rbac) ensurePipelinesSCClusterRole(ctx context.Context) error { logger := logging.FromContext(ctx) + logger.Debug("finding cluster role:", pipelinesSCCClusterRole) - failureEvent := corev1.Event{ + clusterRole := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "pipelines-scc-failure-", - Namespace: namespace, + Name: pipelinesSCCClusterRole, OwnerReferences: []metav1.OwnerReference{r.ownerRef}, }, - EventTime: metav1.NewMicroTime(time.Now()), - Reason: "RequestedSCCNotFound", - Type: "Warning", - Action: "SCCNotUpdated", - Message: fmt.Sprintf("SCC '%s' requested in annotation '%s' not found, SCC not updated in the namespace", scc, openshift.NamespaceSCCAnnotation), - ReportingController: "openshift-pipelines-operator", - ReportingInstance: r.ownerRef.Name, - InvolvedObject: corev1.ObjectReference{ - Kind: "Namespace", - Name: namespace, - APIVersion: "v1", - Namespace: namespace, - }, - } - - logger.Infof("Creating SCC failure event in namespace: %s", namespace) - _, err := r.kubeClientSet.CoreV1().Events(namespace).Create(ctx, &failureEvent, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("failed to create failure event in namespace %s, %w", namespace, err) - } - - return nil -} - -func (r *rbac) ensureCABundles(ctx context.Context, ns *corev1.Namespace) error { - logger := logging.FromContext(ctx) - cfgInterface := r.kubeClientSet.CoreV1().ConfigMaps(ns.Name) - - // Ensure trusted CA bundle - logger.Infof("finding configmap: %s/%s", ns.Name, trustedCABundleConfigMap) - caBundleCM, getErr := cfgInterface.Get(ctx, trustedCABundleConfigMap, metav1.GetOptions{}) - if getErr != nil && !errors.IsNotFound(getErr) { - return getErr - } - - if getErr != nil && errors.IsNotFound(getErr) { - logger.Infof("creating configmap %s in %s namespace", trustedCABundleConfigMap, ns.Name) - var err error - if caBundleCM, err = createCABundleConfigMaps(ctx, cfgInterface, trustedCABundleConfigMap, ns.Name); err != nil { - return err - } - } - - // If config map already exist then remove owner ref - if getErr == nil { - caBundleCM.SetOwnerReferences(nil) - if _, err := cfgInterface.Update(ctx, caBundleCM, metav1.UpdateOptions{}); err != nil { - return err - } - } - - // Ensure service CA bundle - logger.Infof("finding configmap: %s/%s", ns.Name, serviceCABundleConfigMap) - serviceCABundleCM, getErr := cfgInterface.Get(ctx, serviceCABundleConfigMap, metav1.GetOptions{}) - if getErr != nil && !errors.IsNotFound(getErr) { - return getErr + Rules: []rbacv1.PolicyRule{{ + APIGroups: []string{"security.openshift.io"}, + ResourceNames: []string{r.tektonConfig.Spec.Platforms.OpenShift.SCC.Default}, + Resources: []string{"securitycontextconstraints"}, + Verbs: []string{"use"}, + }}, } - if getErr != nil && errors.IsNotFound(getErr) { - logger.Infof("creating configmap %s in %s namespace", serviceCABundleConfigMap, ns.Name) - var err error - if serviceCABundleCM, err = createServiceCABundleConfigMap(ctx, cfgInterface, serviceCABundleConfigMap, ns.Name); err != nil { - return err + rbacClient := r.kubeClientSet.RbacV1() + if _, err := rbacClient.ClusterRoles().Get(ctx, pipelinesSCCClusterRole, metav1.GetOptions{}); err != nil { + if errors.IsNotFound(err) { + _, err = rbacClient.ClusterRoles().Create(ctx, clusterRole, metav1.CreateOptions{}) } + return err } + _, err := rbacClient.ClusterRoles().Update(ctx, clusterRole, metav1.UpdateOptions{}) + return err +} - // If config map already exist then remove owner ref - if getErr == nil { - serviceCABundleCM.SetOwnerReferences(nil) - if _, err := cfgInterface.Update(ctx, serviceCABundleCM, metav1.UpdateOptions{}); err != nil { +// removeObsoleteRBACInstallerSet deletes the pre-v0.55 installer-set name if present. +func (r *rbac) removeObsoleteRBACInstallerSet(ctx context.Context) error { + isClient := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets() + if err := isClient.Delete(ctx, rbacInstallerSetNameOld, metav1.DeleteOptions{}); err != nil { + if !errors.IsNotFound(err) { return err } } - - return nil -} - -func createCABundleConfigMaps(ctx context.Context, cfgInterface v1.ConfigMapInterface, - name, ns string) (*corev1.ConfigMap, error) { - c := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - Labels: map[string]string{ - "app.kubernetes.io/part-of": "tekton-pipelines", - // user-provided and system CA certificates - "config.openshift.io/inject-trusted-cabundle": "true", - }, - // No OwnerReferences - }, - } - - cm, err := cfgInterface.Create(ctx, c, metav1.CreateOptions{}) - if err != nil && !errors.IsAlreadyExists(err) { - return nil, err - } - return cm, nil -} - -func createServiceCABundleConfigMap(ctx context.Context, cfgInterface v1.ConfigMapInterface, - name, ns string) (*corev1.ConfigMap, error) { - c := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - Labels: map[string]string{ - "app.kubernetes.io/part-of": "tekton-pipelines", - }, - Annotations: map[string]string{ - // service serving certificates (required to talk to the internal registry) - "service.beta.openshift.io/inject-cabundle": "true", - }, - // No OwnerReferences - }, - } - - cm, err := cfgInterface.Create(ctx, c, metav1.CreateOptions{}) - if err != nil && !errors.IsAlreadyExists(err) { - return nil, err - } - return cm, nil -} - -func (r *rbac) ensureSA(ctx context.Context, ns *corev1.Namespace) (*corev1.ServiceAccount, error) { - logger := logging.FromContext(ctx) - logger.Infof("finding sa: %s/%s", ns.Name, "pipeline") - saInterface := r.kubeClientSet.CoreV1().ServiceAccounts(ns.Name) - - sa, err := saInterface.Get(ctx, pipelineSA, metav1.GetOptions{}) - if err != nil && !errors.IsNotFound(err) { - return nil, err - } - if err != nil && errors.IsNotFound(err) { - logger.Info("creating sa ", pipelineSA, " ns", ns.Name) - return createSA(ctx, saInterface, ns.Name, *r.tektonConfig) - } - - // set tektonConfig ownerRef - tcOwnerRef := tektonConfigOwnerRef(*r.tektonConfig) - sa.SetOwnerReferences([]metav1.OwnerReference{tcOwnerRef}) - - return saInterface.Update(ctx, sa, metav1.UpdateOptions{}) -} - -func createSA(ctx context.Context, saInterface v1.ServiceAccountInterface, ns string, tc v1alpha1.TektonConfig) (*corev1.ServiceAccount, error) { - tcOwnerRef := tektonConfigOwnerRef(tc) - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: pipelineSA, - Namespace: ns, - OwnerReferences: []metav1.OwnerReference{tcOwnerRef}, - }, - } - - sa, err := saInterface.Create(ctx, sa, metav1.CreateOptions{}) - if err != nil && !errors.IsAlreadyExists(err) { - return nil, err - } - - // Initialize labels map if it doesn't exist - if tc.Labels == nil { - tc.Labels = make(map[string]string) - } - tc.Labels[serviceAccountCreationLabel] = "true" - return sa, nil -} - -// ensureSCCRoleInNamespace ensures that the SCC role exists in the namespace -func (r *rbac) ensureSCCRoleInNamespace(ctx context.Context, namespace string, scc string) error { - logger := logging.FromContext(ctx) - - logger.Infof("finding role: %s in namespace %s", pipelinesSCCRole, namespace) - - sccRole := &rbacv1.Role{ - ObjectMeta: metav1.ObjectMeta{ - Name: pipelinesSCCRole, - Namespace: namespace, - OwnerReferences: []metav1.OwnerReference{r.ownerRef}, - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{ - "security.openshift.io", - }, - ResourceNames: []string{ - scc, - }, - Resources: []string{ - "securitycontextconstraints", - }, - Verbs: []string{ - "use", - }, - }, - }, - } - - rbacClient := r.kubeClientSet.RbacV1() - if _, err := rbacClient.Roles(namespace).Get(ctx, pipelinesSCCRole, metav1.GetOptions{}); err != nil { - // If the role does not exist, then create it and exit - if errors.IsNotFound(err) { - _, err = rbacClient.Roles(namespace).Create(ctx, sccRole, metav1.CreateOptions{}) - } - return err - } - // Update the role if it already exists - _, err := rbacClient.Roles(namespace).Update(ctx, sccRole, metav1.UpdateOptions{}) - return err -} - -// ensurePipelinesSCClusterRole ensures that `pipelines-scc` ClusterRole exists -// in the cluster. The SCC used in the ClusterRole is read from SCC config -// in TektonConfig (`pipelines-scc` by default) -func (r *rbac) ensurePipelinesSCClusterRole(ctx context.Context) error { - logger := logging.FromContext(ctx) - - logger.Debug("finding cluster role:", pipelinesSCCClusterRole) - - clusterRole := &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: pipelinesSCCClusterRole, - OwnerReferences: []metav1.OwnerReference{r.ownerRef}, - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{ - "security.openshift.io", - }, - ResourceNames: []string{ - r.tektonConfig.Spec.Platforms.OpenShift.SCC.Default, - }, - Resources: []string{ - "securitycontextconstraints", - }, - Verbs: []string{ - "use", - }, - }, - }, - } - - rbacClient := r.kubeClientSet.RbacV1() - if _, err := rbacClient.ClusterRoles().Get(ctx, pipelinesSCCClusterRole, metav1.GetOptions{}); err != nil { - if errors.IsNotFound(err) { - _, err = rbacClient.ClusterRoles().Create(ctx, clusterRole, metav1.CreateOptions{}) - } - return err - } - _, err := rbacClient.ClusterRoles().Update(ctx, clusterRole, metav1.UpdateOptions{}) - return err -} - -func (r *rbac) ensurePipelinesSCCRoleBinding(ctx context.Context, sa *corev1.ServiceAccount, roleRef *rbacv1.RoleRef) error { - logger := logging.FromContext(ctx) - rbacClient := r.kubeClientSet.RbacV1() - - roleKind := roleRef.Kind - roleName := roleRef.Name - if roleRef.Kind == "Role" { - logger.Infof("finding %s: %s", roleKind, roleName) - if _, err := rbacClient.Roles(sa.Namespace).Get(ctx, roleName, metav1.GetOptions{}); err != nil { - logger.Error(err, "finding %s failed: %s", roleKind, roleName) - return err - } - } else if roleKind == "ClusterRole" { - logger.Infof("finding %s: %s", roleKind, roleName) - if _, err := rbacClient.ClusterRoles().Get(ctx, roleName, metav1.GetOptions{}); err != nil { - logger.Error(err, "finding %s failed: %s", roleKind, roleName) - return err - } - } else { - return fmt.Errorf("incorrect value set for roleKind - %s, needs to be Role or ClusterRole", roleKind) - } - - logger.Info("finding role-binding", pipelinesSCCRoleBinding) - pipelineRB, rbErr := rbacClient.RoleBindings(sa.Namespace).Get(ctx, pipelinesSCCRoleBinding, metav1.GetOptions{}) - if rbErr != nil && !errors.IsNotFound(rbErr) { - logger.Error(rbErr, "rbac get error", pipelinesSCCRoleBinding) - return rbErr - } - - if rbErr != nil && errors.IsNotFound(rbErr) { - return r.createSCCRoleBinding(ctx, sa, roleRef) - } - - // We cannot update RoleRef in a RoleBinding, we need to delete and - // recreate the binding in that case - if pipelineRB.RoleRef.Kind != roleKind || pipelineRB.RoleRef.Name != roleName { - logger.Infof("Need to update RoleRef in RoleBinding %s in namespace: %s, deleting and recreating...", pipelinesSCCRoleBinding, sa.Namespace) - err := rbacClient.RoleBindings(sa.Namespace).Delete(ctx, pipelinesSCCRoleBinding, metav1.DeleteOptions{}) - if err != nil { - return err - } - return r.createSCCRoleBinding(ctx, sa, roleRef) - } - - logger.Info("found rbac", "subjects", pipelineRB.Subjects) - return r.updateRoleBinding(ctx, pipelineRB, sa, roleRef) -} - -func (r *rbac) createSCCRoleBinding(ctx context.Context, sa *corev1.ServiceAccount, roleRef *rbacv1.RoleRef) error { - logger := logging.FromContext(ctx) - rbacClient := r.kubeClientSet.RbacV1() - - logger.Info("create new rolebinding:", pipelinesSCCRoleBinding) - rb := &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: pipelinesSCCRoleBinding, - Namespace: sa.Namespace, - OwnerReferences: []metav1.OwnerReference{r.ownerRef}, - }, - RoleRef: *roleRef, - Subjects: []rbacv1.Subject{{Kind: rbacv1.ServiceAccountKind, Name: sa.Name, Namespace: sa.Namespace}}, - } - - _, err := rbacClient.RoleBindings(sa.Namespace).Create(ctx, rb, metav1.CreateOptions{}) - if err != nil { - logger.Error(err, "creation of rolebinding failed:", pipelinesSCCRoleBinding) - } - return err -} - -func (r *rbac) updateRoleBinding(ctx context.Context, rb *rbacv1.RoleBinding, sa *corev1.ServiceAccount, roleRef *rbacv1.RoleRef) error { - logger := logging.FromContext(ctx) - - subject := rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: sa.Name, Namespace: sa.Namespace} - - hasSubject := hasSubject(rb.Subjects, subject) - if !hasSubject { - rb.Subjects = append(rb.Subjects, subject) - } - - rb.RoleRef = *roleRef - - rbacClient := r.kubeClientSet.RbacV1() - hasOwnerRef := hasOwnerRefernce(rb.GetOwnerReferences(), r.ownerRef) - - ownerRef := r.updateOwnerRefs(rb.GetOwnerReferences()) - rb.SetOwnerReferences(ownerRef) - - // If owners are different then we need to set from r.ownerRef and update the roleBinding. - if !hasOwnerRef { - if _, err := rbacClient.RoleBindings(sa.Namespace).Update(ctx, rb, metav1.UpdateOptions{}); err != nil { - logger.Error(err, "failed to update edit rb") - return err - } - } - - if hasSubject && (len(ownerRef) != 0) { - logger.Info("rolebinding is up to date ", "action ", "none") - return nil - } - - logger.Infof("update existing rolebinding %s/%s", rb.Namespace, rb.Name) - - _, err := rbacClient.RoleBindings(sa.Namespace).Update(ctx, rb, metav1.UpdateOptions{}) - if err != nil { - logger.Errorf("%v: failed to update rolebinding %s/%s", err, rb.Namespace, rb.Name) - return err - } - logger.Infof("successfully updated rolebinding %s/%s", rb.Namespace, rb.Name) - return nil -} - -func hasSubject(subjects []rbacv1.Subject, x rbacv1.Subject) bool { - for _, v := range subjects { - if v.Name == x.Name && v.Kind == x.Kind && v.Namespace == x.Namespace { - return true - } - } - return false -} - -// CompareLists compares two slices of rbacv1.Subject, ignoring order -func CompareSubjects(list1, list2 []rbacv1.Subject) bool { - // Check if lengths are different - if len(list1) != len(list2) { - return false - } - // Create sets (maps) for both lists - set1 := make(map[string]struct{}) - set2 := make(map[string]struct{}) - - // Populate set1 with subjects from list1 - for _, subject := range list1 { - key := fmt.Sprintf("%s/%s/%s", subject.Kind, subject.Name, subject.Namespace) - set1[key] = struct{}{} - } - // Populate set2 with subjects from list2 - for _, subject := range list2 { - key := fmt.Sprintf("%s/%s/%s", subject.Kind, subject.Name, subject.Namespace) - set2[key] = struct{}{} - } - - // Compare the sets - if len(set1) != len(set2) { - return false - } - - // Check if all elements in set1 are in set2 - for key := range set1 { - if _, exists := set2[key]; !exists { - return false - } - } - return true -} - -func mergeSubjects(subjects []rbacv1.Subject, x []rbacv1.Subject) []rbacv1.Subject { - // Map to track subjects in the existing list - existingSubjects := make(map[string]struct{}) - - // Iterate over `subjects` and track each unique combination of Kind, Name, and Namespace - for _, subject := range subjects { - key := fmt.Sprintf("%s/%s/%s", subject.Kind, subject.Name, subject.Namespace) - existingSubjects[key] = struct{}{} - } - - // Final list to store the merged subjects - var finalSubjects []rbacv1.Subject - - // Add all subjects from the original list (list1) - finalSubjects = append(finalSubjects, subjects...) - - // Append subjects from `x` (list2) that are not in `existingSubjects` - for _, subject := range x { - key := fmt.Sprintf("%s/%s/%s", subject.Kind, subject.Name, subject.Namespace) - if _, found := existingSubjects[key]; !found { - finalSubjects = append(finalSubjects, subject) - } - } - - return finalSubjects -} - -func hasOwnerRefernce(old []metav1.OwnerReference, new metav1.OwnerReference) bool { - for _, v := range old { - if v.APIVersion == new.APIVersion && v.Kind == new.Kind && v.Name == new.Name { - return true - } - } - return false -} - -func (r *rbac) isLegacyRBACEnabled() bool { - for _, v := range r.tektonConfig.Spec.Params { - if v.Name == legacyPipelineRbacParamName { - return v.Value != "false" - } - } - return true -} - -func (r *rbac) ensureRoleBindings(ctx context.Context, sa *corev1.ServiceAccount) error { - logger := logging.FromContext(ctx) - rbacClient := r.kubeClientSet.RbacV1() - - legacyEnabled := r.isLegacyRBACEnabled() - - editRB, err := rbacClient.RoleBindings(sa.Namespace).Get(ctx, PipelineRoleBinding, metav1.GetOptions{}) - - if !legacyEnabled && err == nil { - logger.Infof("Legacy Pipeline RBAC is disabled, removing existing role binding %s/%s", - editRB.Namespace, editRB.Name) - return rbacClient.RoleBindings(sa.Namespace).Delete(ctx, PipelineRoleBinding, metav1.DeleteOptions{}) - } - - if !legacyEnabled { - logger.Infof("Legacy Pipeline RBAC is disabled, skipping role binding creation") - return nil - } - - logger.Infof("Legacy Pipeline RBAC is enabled") - - if err == nil { - logger.Infof("Found rolebinding %s/%s, updating if needed", editRB.Namespace, editRB.Name) - return r.updateRoleBinding(ctx, editRB, sa, &rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "ClusterRole", - Name: "edit", - }) - } - - if errors.IsNotFound(err) { - logger.Infof("Role binding not found, creating new one") - return r.createRoleBinding(ctx, sa) - } - - return err -} - -func (r *rbac) createRoleBinding(ctx context.Context, sa *corev1.ServiceAccount) error { - logger := logging.FromContext(ctx) - - logger.Infof("create new rolebinding %s/%s", sa.Namespace, sa.Name) - rbacClient := r.kubeClientSet.RbacV1() - - logger.Info("finding clusterrole edit") - if _, err := rbacClient.ClusterRoles().Get(ctx, "edit", metav1.GetOptions{}); err != nil { - logger.Error(err, "getting clusterRole 'edit' failed") - return err - } - - rb := &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: PipelineRoleBinding, - Namespace: sa.Namespace, - OwnerReferences: []metav1.OwnerReference{r.ownerRef}, - }, - RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: "edit"}, - Subjects: []rbacv1.Subject{{Kind: rbacv1.ServiceAccountKind, Name: sa.Name, Namespace: sa.Namespace}}, - } - - if _, err := rbacClient.RoleBindings(sa.Namespace).Create(ctx, rb, metav1.CreateOptions{}); err != nil { - logger.Errorf("%v: failed creation of rolebinding %s/%s", err, rb.Namespace, rb.Name) - return err - } - return nil -} - -func (r *rbac) removeAndUpdateNSFromCI(ctx context.Context) error { - logger := logging.FromContext(ctx) - - rbacClient := r.kubeClientSet.RbacV1() - rb, err := r.rbacInformer.Lister().Get(clusterInterceptors) - if err != nil && !errors.IsNotFound(err) { - logger.Error(err, "failed to get"+clusterInterceptors) - return err - } - if rb == nil { - return nil - } - - req, err := labels.NewRequirement(namespaceVersionLabel, selection.Equals, []string{r.version}) - if err != nil { - logger.Error(err, "failed to create requirement: ") - return err - } - - namespaces, err := r.nsInformer.Lister().List(labels.NewSelector().Add(*req)) - if err != nil && !errors.IsNotFound(err) { - logger.Error(err, "failed to list namespace: ") - return err - } - - nsMap := map[string]string{} - for i := range namespaces { - nsMap[namespaces[i].Name] = namespaces[i].Name - } - - var update bool - for i := 0; i <= len(rb.Subjects)-1; i++ { - if len(nsMap) != len(rb.Subjects) { - if _, ok := nsMap[rb.Subjects[i].Namespace]; !ok { - rb.Subjects = removeIndex(rb.Subjects, i) - update = true - } - } - } - if update { - if _, err := rbacClient.ClusterRoleBindings().Update(ctx, rb, metav1.UpdateOptions{}); err != nil { - logger.Error(err, "failed to update "+clusterInterceptors+" crb") - return err - } - logger.Infof("successfully removed namespace and updated %s ", clusterInterceptors) - } - return nil -} - -func removeIndex(s []rbacv1.Subject, index int) []rbacv1.Subject { - return append(s[:index], s[index+1:]...) -} - -func (r *rbac) handleClusterRoleBinding(ctx context.Context, namespacesToUpdate []NamespaceServiceAccount) error { - logger := logging.FromContext(ctx) - - rbacClient := r.kubeClientSet.RbacV1() - logger.Info("finding cluster-role ", clusterInterceptors) - if _, err := rbacClient.ClusterRoles().Get(ctx, clusterInterceptors, metav1.GetOptions{}); errors.IsNotFound(err) { - if e := r.createClusterRole(ctx); e != nil { - return e - } - } - - // Prepare a list of Subjects from the namespacesToUpdate - var subjects []rbacv1.Subject - - for _, nsSA := range namespacesToUpdate { - sa := nsSA.ServiceAccount - ns := nsSA.Namespace - - logger.Infof("Processing Subject for ServiceAccount %s in Namespace %s", sa.Name, ns.Name) - - // Create the Subject for the ClusterRoleBinding - subject := rbacv1.Subject{ - Kind: rbacv1.ServiceAccountKind, - Name: sa.Name, - Namespace: sa.Namespace, - } - - // Append the subject to the list - subjects = append(subjects, subject) - } - - logger.Info("finding cluster-role-binding ", clusterInterceptors) - - viewCRB, err := rbacClient.ClusterRoleBindings().Get(ctx, clusterInterceptors, metav1.GetOptions{}) - - if err == nil { - logger.Infof("found clusterrolebinding %s", viewCRB.Name) - return r.bulkUpdateClusterRoleBinding(ctx, viewCRB, subjects) - } - - if errors.IsNotFound(err) { - logger.Infof("could not find clusterrolebinding %s proceeding to create", viewCRB.Name) - return r.bulkCreateClusterRoleBinding(ctx, subjects) - } - - return err -} - -// bulk update Cluster rolebinding with all reconciled namespaces and service accounts -func (r *rbac) bulkUpdateClusterRoleBinding(ctx context.Context, rb *rbacv1.ClusterRoleBinding, subjectlist []rbacv1.Subject) error { - logger := logging.FromContext(ctx) - - hasSubject := CompareSubjects(rb.Subjects, subjectlist) - if !hasSubject { - rb.Subjects = mergeSubjects(rb.Subjects, subjectlist) - } - - rbacClient := r.kubeClientSet.RbacV1() - hasOwnerRef := hasOwnerRefernce(rb.GetOwnerReferences(), r.ownerRef) - - ownerRef := r.updateOwnerRefs(rb.GetOwnerReferences()) - rb.SetOwnerReferences(ownerRef) - - // If owners are different then we need to set from r.ownerRef and update the clusterRolebinding. - if !hasOwnerRef { - if _, err := rbacClient.ClusterRoleBindings().Update(ctx, rb, metav1.UpdateOptions{}); err != nil { - logger.Error(err, "failed to update "+clusterInterceptors+" crb") - return err - } - } - - if hasSubject && (len(ownerRef) != 0) { - logger.Info("clusterrolebinding is up to date", "action", "none") - return nil - } - - logger.Info("update existing clusterrolebinding ", clusterInterceptors) - - if _, err := rbacClient.ClusterRoleBindings().Update(ctx, rb, metav1.UpdateOptions{}); err != nil { - logger.Error(err, "failed to update "+clusterInterceptors+" crb") - return err - } - logger.Info("successfully updated ", clusterInterceptors) - return nil -} - -// create Cluster rolebinding with all reconciled namespaces and service accounts -func (r *rbac) bulkCreateClusterRoleBinding(ctx context.Context, subjectlist []rbacv1.Subject) error { - logger := logging.FromContext(ctx) - - logger.Info("create new clusterrolebinding ", clusterInterceptors) - rbacClient := r.kubeClientSet.RbacV1() - - logger.Info("finding clusterrole ", clusterInterceptors) - if _, err := rbacClient.ClusterRoles().Get(ctx, clusterInterceptors, metav1.GetOptions{}); err != nil { - logger.Error(err, " getting clusterRole "+clusterInterceptors+" failed") - return err - } - - crb := &rbacv1.ClusterRoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterInterceptors, - OwnerReferences: []metav1.OwnerReference{r.ownerRef}, - }, - RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: clusterInterceptors}, - Subjects: subjectlist, - } - - if _, err := rbacClient.ClusterRoleBindings().Create(ctx, crb, metav1.CreateOptions{}); err != nil { - logger.Error(err, " creation of "+clusterInterceptors+" failed") - return err - } - return nil -} - -func (r *rbac) createClusterRole(ctx context.Context) error { - logger := logging.FromContext(ctx) - - logger.Info("create new clusterrole ", clusterInterceptors) - rbacClient := r.kubeClientSet.RbacV1() - - cr := &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterInterceptors, - OwnerReferences: []metav1.OwnerReference{r.ownerRef}, - }, - Rules: []rbacv1.PolicyRule{{ - APIGroups: []string{"triggers.tekton.dev"}, - Resources: []string{"clusterinterceptors"}, - Verbs: []string{"get", "list", "watch"}, - }}, - } - - if _, err := rbacClient.ClusterRoles().Create(ctx, cr, metav1.CreateOptions{}); err != nil { - logger.Error(err, "creation of "+clusterInterceptors+" clusterrole failed") - return err - } - return nil -} - -func (r *rbac) updateOwnerRefs(ownerRef []metav1.OwnerReference) []metav1.OwnerReference { - if len(ownerRef) == 0 { - return []metav1.OwnerReference{r.ownerRef} - } - - for i, ref := range ownerRef { - if ref.APIVersion != r.ownerRef.APIVersion || ref.Kind != r.ownerRef.Kind || ref.Name != r.ownerRef.Name { - // if owner reference are different remove the existing oand override with r.ownerRef - return r.removeAndUpdate(ownerRef, i) - } - } - - return ownerRef -} - -func (r *rbac) removeAndUpdate(slice []metav1.OwnerReference, s int) []metav1.OwnerReference { - ownerRef := append(slice[:s], slice[s+1:]...) - ownerRef = append(ownerRef, r.ownerRef) - return ownerRef -} - -// TODO: Remove this after v0.55.0 release, by following a depreciation notice -// -------------------- -// cleanUpRBACNameChange will check remove ownerReference: RBAC installerset from -// 'edit' rolebindings from all relevant namespaces -// it will also remove 'pipeline' sa from subject list as -// the new 'openshift-pipelines-edit' rolebinding -func (r *rbac) cleanUpRBACNameChange(ctx context.Context) error { - rbacClient := r.kubeClientSet.RbacV1() - - // fetch the list of all namespaces - namespaces, err := r.kubeClientSet.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) - if err != nil { - return err - } - - for _, ns := range namespaces.Items { - nsName := ns.GetName() - - // filter namespaces: - // ignore ns with name passing regex `^(openshift|kube)-` - if ignore := nsRegex.MatchString(nsName); ignore { - continue - } - - // check if "edit" rolebinding exists in "ns" namespace - editRB, err := rbacClient.RoleBindings(ns.GetName()). - Get(ctx, pipelineRoleBindingOld, metav1.GetOptions{}) - if err != nil { - // if "edit" rolebinding does not exists in "ns" namesapce, then do nothing - if errors.IsNotFound(err) { - continue - } - return err - } - - // check if 'pipeline' serviceaccount is listed as a subject in 'edit' rolebinding - depSub := rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: pipelineSA, Namespace: nsName} - subIdx := math.MinInt16 - for i, s := range editRB.Subjects { - if s.Name == depSub.Name && s.Kind == depSub.Kind && s.Namespace == depSub.Namespace { - subIdx = i - break - } - } - - // if 'pipeline' serviceaccount is listed as a subject in 'edit' rolebinding - // remove 'pipeline' serviceaccount from subject list - if subIdx >= 0 { - editRB.Subjects = append(editRB.Subjects[:subIdx], editRB.Subjects[subIdx+1:]...) - } - - // if 'pipeline' serviceaccount was the only item in the subject list of 'edit' rolebinding, - // then we can delete 'edit' rolebinding as nobody else is using it - if len(editRB.Subjects) == 0 { - if err := rbacClient.RoleBindings(nsName).Delete(ctx, editRB.GetName(), metav1.DeleteOptions{}); err != nil { - return err - } - continue - } - - // remove TektonInstallerSet ownerReferece from "edit" rolebinding - ownerRefs := editRB.GetOwnerReferences() - ownerRefIdx := math.MinInt16 - for i, ownerRef := range ownerRefs { - if ownerRef.Kind == "TektonInstallerSet" { - ownerRefIdx = i - break - } - } - if ownerRefIdx >= 0 { - ownerRefs := append(ownerRefs[:ownerRefIdx], ownerRefs[ownerRefIdx+1:]...) - editRB.SetOwnerReferences(ownerRefs) - - } - - // if ownerReference or subject was updated, then update editRB resource on cluster - if ownerRefIdx < 0 && subIdx < 0 { - continue - } - if _, err := rbacClient.RoleBindings(nsName).Update(ctx, editRB, metav1.UpdateOptions{}); err != nil { - return err - } - } - return nil -} - -// TODO: Remove this after v0.55.0 release, by following a depreciation notice -// -------------------- -func (r *rbac) removeObsoleteRBACInstallerSet(ctx context.Context) error { - isClient := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets() - err := isClient.Delete(ctx, rbacInstallerSetNameOld, metav1.DeleteOptions{}) - if err != nil { - if !errors.IsNotFound(err) { - return err - } - } - return nil -} - -func (r *rbac) ensureCABundlesInNamespace(ctx context.Context, ns *corev1.Namespace) error { - logger := logging.FromContext(ctx) - logger.Infow("Ensuring CA bundle configmaps in namespace", "namespace", ns.GetName()) - return r.ensureCABundles(ctx, ns) -} - -// Add new method for patching namespace with trusted configmaps label -func (r *rbac) patchNamespaceTrustedConfigLabel(ctx context.Context, ns corev1.Namespace) error { - logger := logging.FromContext(ctx) - - logger.Infof("add label namespace-trusted-configmaps-version to mark namespace '%s' as reconciled", ns.Name) - - // Prepare a patch to add/update just one label without overwriting others - patch := map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - namespaceTrustedConfigLabel: r.version, - }, - }, - } - - patchPayload, err := json.Marshal(patch) - if err != nil { - logger.Errorf("failed to marshal patch payload: %v", err) - return fmt.Errorf("failed to marshal label patch for namespace %s: %w", ns.Name, err) - } - - // Use PATCH to update just the target label - if _, err := r.kubeClientSet.CoreV1().Namespaces().Patch(ctx, ns.Name, types.StrategicMergePatchType, patchPayload, metav1.PatchOptions{}); err != nil { - logger.Errorf("failed to patch namespace %s: %v", ns.Name, err) - return fmt.Errorf("failed to patch namespace %s: %w", ns.Name, err) - } - - logger.Infof("namespace '%s' successfully reconciled with label %q=%q", ns.Name, namespaceTrustedConfigLabel, r.version) return nil } diff --git a/pkg/reconciler/openshift/tektonconfig/rbac_test.go b/pkg/reconciler/openshift/tektonconfig/rbac_test.go index 612468b733..9e168cf415 100644 --- a/pkg/reconciler/openshift/tektonconfig/rbac_test.go +++ b/pkg/reconciler/openshift/tektonconfig/rbac_test.go @@ -10,689 +10,105 @@ import ( "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" operatorfake "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake" "github.com/tektoncd/operator/pkg/reconciler/common" - "github.com/tektoncd/operator/pkg/reconciler/openshift" "gotest.tools/v3/assert" - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kubeinformers "k8s.io/client-go/informers" + "k8s.io/apimachinery/pkg/runtime" kubefake "k8s.io/client-go/kubernetes/fake" - "knative.dev/pkg/logging" + k8stesting "k8s.io/client-go/testing" ) -func TestCreateResources(t *testing.T) { - // Set KO_DATA_PATH environment variable +// --------------------------------------------------------------------------- +// createResources +// --------------------------------------------------------------------------- + +// TestCreateResources_EnsuresPrerequisites verifies that createResources calls +// ensurePreRequisites and returns RECONCILE_AGAIN_ERR when the InstallerSet +// does not yet exist. +func TestCreateResources_EnsuresPrerequisites(t *testing.T) { os.Setenv(common.KoEnvKey, "testdata") - // Test cases - tests := []struct { - name string - tektonConfig *v1alpha1.TektonConfig - existingNamespaces []corev1.Namespace - existingSAs []corev1.ServiceAccount - existingRoles []rbacv1.Role - existingRBs []rbacv1.RoleBinding - existingCRBs []rbacv1.ClusterRoleBinding - existingConfigMaps []corev1.ConfigMap - installerSet *v1alpha1.TektonInstallerSet - wantErr bool - wantReconcileAgain bool - wantNamespaces int - }{ - { - name: "Both RBAC and CA bundles disabled", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "false"}, - {Name: "createCABundleConfigMaps", Value: "false"}, - }, - }, - }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 0, - }, - { - name: "Only RBAC enabled - No InstallerSet", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "true"}, - {Name: "createCABundleConfigMaps", Value: "false"}, - }, - Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ - SCC: &v1alpha1.SCC{ - Default: "pipelines-scc", - }, - }, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns2", - }, - }, - }, - wantErr: false, - wantReconcileAgain: true, // Should reconcile again because InstallerSet needs to be created - wantNamespaces: 2, - }, - { - name: "Only RBAC enabled - With InstallerSet", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "true"}, - {Name: "createCABundleConfigMaps", Value: "false"}, - }, - Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ - SCC: &v1alpha1.SCC{ - Default: "pipelines-scc", - }, - }, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns2", - }, - }, - }, - installerSet: &v1alpha1.TektonInstallerSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rhosp-rbac-001", - Labels: map[string]string{ - v1alpha1.CreatedByKey: createdByValue, - v1alpha1.InstallerSetType: componentNameRBAC, - }, - Annotations: map[string]string{ - v1alpha1.ReleaseVersionKey: "test-version", - }, - }, - Spec: v1alpha1.TektonInstallerSetSpec{}, - }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 2, - }, - { - name: "Only CA bundles enabled", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "false"}, - {Name: "createCABundleConfigMaps", Value: "true"}, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - }, - }, - }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 1, - }, - { - name: "Both RBAC and CA bundles enabled", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "true"}, - {Name: "createCABundleConfigMaps", Value: "true"}, - }, - Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ - SCC: &v1alpha1.SCC{ - Default: "pipelines-scc", - }, - }, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Annotations: map[string]string{ - openshift.NamespaceSCCAnnotation: "privileged", - }, - }, - }, - }, - wantErr: false, - wantReconcileAgain: true, - wantNamespaces: 1, - }, - { - name: "Namespace with SCC annotation", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "true"}, - }, - CommonSpec: v1alpha1.CommonSpec{ - TargetNamespace: "test-ns", - }, - Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ - SCC: &v1alpha1.SCC{ - Default: "pipelines-scc", - }, - }, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Annotations: map[string]string{ - openshift.NamespaceSCCAnnotation: "privileged", - }, - }, - }, - }, - wantErr: false, - wantReconcileAgain: true, - wantNamespaces: 1, - }, - { - name: "Namespace with invalid SCC annotation", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "true"}, - }, - CommonSpec: v1alpha1.CommonSpec{ - TargetNamespace: "test-ns", - }, - Platforms: v1alpha1.Platforms{ - OpenShift: v1alpha1.OpenShift{ - SCC: &v1alpha1.SCC{ - Default: "pipelines-scc", - }, - }, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Annotations: map[string]string{ - openshift.NamespaceSCCAnnotation: "nonexistent-scc", - }, - }, - }, - }, - wantErr: false, - wantReconcileAgain: true, - wantNamespaces: 1, - }, - { - name: "CA bundle self-healing - trusted configmap missing despite label", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "false"}, - {Name: "createCABundleConfigMaps", Value: "true"}, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Labels: map[string]string{ - namespaceTrustedConfigLabel: "test-version", - }, - }, - }, - }, - existingConfigMaps: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: serviceCABundleConfigMap, - Namespace: "test-ns1", - }, - }, - }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 1, - }, - { - name: "CA bundle self-healing - service configmap missing despite label", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "false"}, - {Name: "createCABundleConfigMaps", Value: "true"}, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Labels: map[string]string{ - namespaceTrustedConfigLabel: "test-version", - }, - }, - }, - }, - existingConfigMaps: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: trustedCABundleConfigMap, - Namespace: "test-ns1", - }, - }, - }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 1, - }, - { - name: "CA bundle self-healing - both configmaps present, no re-reconciliation", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "false"}, - {Name: "createCABundleConfigMaps", Value: "true"}, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Labels: map[string]string{ - namespaceTrustedConfigLabel: "test-version", - }, - }, - }, - }, - existingConfigMaps: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: trustedCABundleConfigMap, - Namespace: "test-ns1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: serviceCABundleConfigMap, - Namespace: "test-ns1", - }, - }, - }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 0, - }, - { - name: "CA bundle self-healing - both configmaps missing despite label", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: "createRbacResource", Value: "false"}, - {Name: "createCABundleConfigMaps", Value: "true"}, - }, - }, - }, - existingNamespaces: []corev1.Namespace{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ns1", - Labels: map[string]string{ - namespaceTrustedConfigLabel: "test-version", - }, - }, + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "config"}, + Spec: v1alpha1.TektonConfigSpec{ + Platforms: v1alpha1.Platforms{ + OpenShift: v1alpha1.OpenShift{ + SCC: &v1alpha1.SCC{Default: "pipelines-scc"}, + NamespaceSync: &v1alpha1.NamespaceSyncConfig{}, }, }, - wantErr: false, - wantReconcileAgain: false, - wantNamespaces: 1, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - - // Setup fake clients - kubeClient := kubefake.NewSimpleClientset() - operatorClient := operatorfake.NewSimpleClientset() - securityClient := fakesecurity.NewSimpleClientset() - - // Create default SCCs - defaultSCCs := []securityv1.SecurityContextConstraints{ - { - ObjectMeta: metav1.ObjectMeta{Name: "restricted"}, - }, - { - ObjectMeta: metav1.ObjectMeta{Name: "pipelines-scc"}, - }, - { - ObjectMeta: metav1.ObjectMeta{Name: "privileged"}, - }, - } - for _, scc := range defaultSCCs { - _, err := securityClient.SecurityV1().SecurityContextConstraints().Create(ctx, &scc, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - // Create the required "edit" ClusterRole - editClusterRole := &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "edit", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"*"}, - Resources: []string{"*"}, - Verbs: []string{"*"}, - }, - }, - } - _, err := kubeClient.RbacV1().ClusterRoles().Create(ctx, editClusterRole, metav1.CreateOptions{}) - assert.NilError(t, err) - - // Create informers - informers := kubeinformers.NewSharedInformerFactory(kubeClient, 0) - nsInformer := informers.Core().V1().Namespaces() - rbacInformer := informers.Rbac().V1().ClusterRoleBindings() - - // Add existing resources to the fake clients - for _, ns := range tt.existingNamespaces { - _, err := kubeClient.CoreV1().Namespaces().Create(ctx, &ns, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - for _, sa := range tt.existingSAs { - _, err := kubeClient.CoreV1().ServiceAccounts(sa.Namespace).Create(ctx, &sa, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - for _, role := range tt.existingRoles { - _, err := kubeClient.RbacV1().Roles(role.Namespace).Create(ctx, &role, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - for _, rb := range tt.existingRBs { - _, err := kubeClient.RbacV1().RoleBindings(rb.Namespace).Create(ctx, &rb, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - for _, crb := range tt.existingCRBs { - _, err := kubeClient.RbacV1().ClusterRoleBindings().Create(ctx, &crb, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - for _, cm := range tt.existingConfigMaps { - _, err := kubeClient.CoreV1().ConfigMaps(cm.Namespace).Create(ctx, &cm, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - // Create installer set if specified in test case - if tt.installerSet != nil { - _, err := operatorClient.OperatorV1alpha1().TektonInstallerSets().Create(ctx, tt.installerSet, metav1.CreateOptions{}) - assert.NilError(t, err) - } - - // Start informers - stopCh := make(chan struct{}) - defer close(stopCh) - informers.Start(stopCh) - - // Create the rbac instance - r := &rbac{ - kubeClientSet: kubeClient, - operatorClientSet: operatorClient, - securityClientSet: securityClient, - rbacInformer: rbacInformer, - nsInformer: nsInformer, - tektonConfig: tt.tektonConfig, - version: "test-version", - } - - // Create context with logger - ctx = logging.WithLogger(ctx, logging.FromContext(ctx)) - - // Execute the function - err = r.createResources(ctx) - - // Verify results - if tt.wantErr { - assert.Assert(t, err != nil) - } else if tt.wantReconcileAgain { - assert.Equal(t, err, v1alpha1.RECONCILE_AGAIN_ERR) - } else { - assert.NilError(t, err) - } - - // Verify the number of processed namespaces - if !tt.wantReconcileAgain { - // For cases where we expect reconciliation to complete, verify the namespace labels - for _, ns := range tt.existingNamespaces { - updatedNs, err := kubeClient.CoreV1().Namespaces().Get(ctx, ns.Name, metav1.GetOptions{}) - assert.NilError(t, err) - - // If RBAC is enabled, verify the namespace has been labeled - createRBACResource := true - for _, param := range tt.tektonConfig.Spec.Params { - if param.Name == "createRbacResource" && param.Value == "false" { - createRBACResource = false - break - } - } - - if createRBACResource { - // Verify the namespace has been labeled with the correct version - assert.Equal(t, updatedNs.Labels[namespaceVersionLabel], r.version) - } - } - } - }) + kubeClient := kubefake.NewSimpleClientset() + operatorClient := operatorfake.NewSimpleClientset() + secClient := fakesecurity.NewSimpleClientset(&securityv1.SecurityContextConstraints{ + ObjectMeta: metav1.ObjectMeta{Name: "pipelines-scc"}, + }) + + r := &rbac{ + kubeClientSet: kubeClient, + operatorClientSet: operatorClient, + securityClientSet: secClient, + version: "test-version", + tektonConfig: tc, } + + err := r.createResources(context.Background()) + // InstallerSet does not exist yet → RECONCILE_AGAIN_ERR + assert.Equal(t, v1alpha1.RECONCILE_AGAIN_ERR, err) } -func TestSetDefault(t *testing.T) { - tests := []struct { - name string - tektonConfig *v1alpha1.TektonConfig - want []v1alpha1.Param - }{ - { - name: "Empty params - should set all defaults", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{}, - }, - }, - want: []v1alpha1.Param{ - {Name: rbacParamName, Value: "true"}, - {Name: legacyPipelineRbacParamName, Value: "true"}, - {Name: trustedCABundleParamName, Value: "true"}, - }, - }, - { - name: "Partial params - should set missing defaults", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: rbacParamName, Value: "false"}, - }, - }, - }, - want: []v1alpha1.Param{ - {Name: rbacParamName, Value: "false"}, - {Name: legacyPipelineRbacParamName, Value: "true"}, - {Name: trustedCABundleParamName, Value: "false"}, - }, - }, - { - name: "Invalid values - should set to true", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: rbacParamName, Value: "invalid"}, - {Name: trustedCABundleParamName, Value: "maybe"}, - {Name: legacyPipelineRbacParamName, Value: "unknown"}, - }, - }, - }, - want: []v1alpha1.Param{ - {Name: rbacParamName, Value: "true"}, - {Name: trustedCABundleParamName, Value: "true"}, - {Name: legacyPipelineRbacParamName, Value: "true"}, - }, - }, - { - name: "All params set correctly - should not change", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: rbacParamName, Value: "false"}, - {Name: trustedCABundleParamName, Value: "false"}, - {Name: legacyPipelineRbacParamName, Value: "false"}, - }, - }, - }, - want: []v1alpha1.Param{ - {Name: rbacParamName, Value: "false"}, - {Name: trustedCABundleParamName, Value: "false"}, - {Name: legacyPipelineRbacParamName, Value: "false"}, - }, - }, - { - name: "Only createCABundleConfigMaps present, set to false", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: trustedCABundleParamName, Value: "false"}, - }, - }, - }, - want: []v1alpha1.Param{ - {Name: trustedCABundleParamName, Value: "false"}, - {Name: rbacParamName, Value: "true"}, - {Name: legacyPipelineRbacParamName, Value: "true"}, - }, - }, - { - name: "Only legacyPipelineRbac present, set to true", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: legacyPipelineRbacParamName, Value: "true"}, - }, - }, - }, - want: []v1alpha1.Param{ - {Name: legacyPipelineRbacParamName, Value: "true"}, - {Name: rbacParamName, Value: "true"}, - {Name: trustedCABundleParamName, Value: "true"}, - }, - }, - { - name: "createRbacResource present with 'true', others missing", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: rbacParamName, Value: "true"}, - }, +// TestCreateResources_WithInstallerSet verifies that createResources succeeds +// (no error) when the InstallerSet is already present. +func TestCreateResources_WithInstallerSet(t *testing.T) { + os.Setenv(common.KoEnvKey, "testdata") + + tc := &v1alpha1.TektonConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "config"}, + Spec: v1alpha1.TektonConfigSpec{ + Platforms: v1alpha1.Platforms{ + OpenShift: v1alpha1.OpenShift{ + SCC: &v1alpha1.SCC{Default: "pipelines-scc"}, + NamespaceSync: &v1alpha1.NamespaceSyncConfig{}, }, }, - want: []v1alpha1.Param{ - {Name: rbacParamName, Value: "true"}, - {Name: legacyPipelineRbacParamName, Value: "true"}, - {Name: trustedCABundleParamName, Value: "true"}, - }, }, - { - name: "Mix of valid, invalid, and missing", - tektonConfig: &v1alpha1.TektonConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config"}, - Spec: v1alpha1.TektonConfigSpec{ - Params: []v1alpha1.Param{ - {Name: rbacParamName, Value: "true"}, - {Name: legacyPipelineRbacParamName, Value: "invalid"}, - }, - }, + } + + existingISet := &v1alpha1.TektonInstallerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rhosp-rbac-001", + Labels: map[string]string{ + v1alpha1.CreatedByKey: createdByValue, + v1alpha1.InstallerSetType: componentNameRBAC, }, - want: []v1alpha1.Param{ - {Name: rbacParamName, Value: "true"}, - {Name: legacyPipelineRbacParamName, Value: "true"}, - {Name: trustedCABundleParamName, Value: "true"}, + Annotations: map[string]string{ + v1alpha1.ReleaseVersionKey: "test-version", }, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := &rbac{ - tektonConfig: tt.tektonConfig, - } - - r.setDefault() - - // Verify the params are set correctly - assert.Equal(t, len(r.tektonConfig.Spec.Params), len(tt.want)) - - // Create maps for easier comparison - got := make(map[string]string) - for _, param := range r.tektonConfig.Spec.Params { - got[param.Name] = param.Value - } - - want := make(map[string]string) - for _, param := range tt.want { - want[param.Name] = param.Value - } - - // Compare the maps - assert.DeepEqual(t, got, want) - }) + scc := &securityv1.SecurityContextConstraints{ + ObjectMeta: metav1.ObjectMeta{Name: "pipelines-scc"}, + } + kubeClient := kubefake.NewSimpleClientset() + operatorClient := operatorfake.NewSimpleClientset(existingISet) + secClient := fakesecurity.NewSimpleClientset() + secClient.PrependReactor("get", "securitycontextconstraints", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, scc, nil + }) + secClient.PrependReactor("list", "securitycontextconstraints", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &securityv1.SecurityContextConstraintsList{Items: []securityv1.SecurityContextConstraints{*scc}}, nil + }) + + r := &rbac{ + kubeClientSet: kubeClient, + operatorClientSet: operatorClient, + securityClientSet: secClient, + version: "test-version", + tektonConfig: tc, } + + err := r.createResources(context.Background()) + assert.NilError(t, err) } From 9b8855bc27f3ee30b2293dfe67b57c2d523fee5e Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 11:20:31 +0200 Subject: [PATCH 6/7] feat(namespacesync): register and wire NamespaceSyncController Add the namespacesync controller to the lifecycle container's -controllers arg so it actually runs on OpenShift. Add the namespaceSync field schema to the TektonConfig CRD so the API server accepts it. Embed reconciler.LeaderAwareFuncs in the Reconciler struct to satisfy Knative sharedmain's leader-election requirement. Signed-off-by: Jawed khelil Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor --- .../operator.tekton.dev_tektonconfigs.yaml | 47 +++++++++++++++++++ config/openshift/base/operator.yaml | 2 +- .../openshift/namespacesync/reconciler.go | 3 ++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml b/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml index 593e95c10c..24cc94a92f 100644 --- a/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml +++ b/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml @@ -975,6 +975,53 @@ spec: namespace or in the Default field. type: string type: object + namespaceSync: + description: NamespaceSync controls per-namespace resource + provisioning by the NamespaceSyncController. + properties: + createCABundles: + description: CreateCABundles controls whether the CA bundle + ConfigMaps are created in each namespace. + type: boolean + createEditRoleBinding: + description: CreateEditRoleBinding controls whether the + openshift-pipelines-edit RoleBinding is created in each + namespace. + type: boolean + createPipelineSA: + description: CreatePipelineSA controls whether the pipeline + ServiceAccount is created in each namespace. + type: boolean + createSCCRoleBinding: + description: CreateSCCRoleBinding controls whether the + SCC RoleBinding is created in each namespace. + type: boolean + namespaceSelector: + description: NamespaceSelector is an optional label selector + that restricts which namespaces are synced. + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + type: object type: object type: object profile: diff --git a/config/openshift/base/operator.yaml b/config/openshift/base/operator.yaml index 6b8eb976bd..b951988fa3 100644 --- a/config/openshift/base/operator.yaml +++ b/config/openshift/base/operator.yaml @@ -38,7 +38,7 @@ spec: image: ko://github.com/tektoncd/operator/cmd/openshift/operator args: - "-controllers" - - "tektonconfig,tektonpipeline,tektontrigger,tektonhub,tektonchain,tektonaddon,tektonresult,openshiftpipelinesascode,manualapprovalgate,tektonpruner,tektonscheduler,tektonmulticlusterproxyaae,syncerservice" + - "tektonconfig,tektonpipeline,tektontrigger,tektonhub,tektonchain,tektonaddon,tektonresult,openshiftpipelinesascode,manualapprovalgate,tektonpruner,tektonscheduler,tektonmulticlusterproxyaae,syncerservice,namespacesync" - "-unique-process-name" - "tekton-operator-lifecycle" imagePullPolicy: Always diff --git a/pkg/reconciler/openshift/namespacesync/reconciler.go b/pkg/reconciler/openshift/namespacesync/reconciler.go index f9f65d5685..fcf41b2a6e 100644 --- a/pkg/reconciler/openshift/namespacesync/reconciler.go +++ b/pkg/reconciler/openshift/namespacesync/reconciler.go @@ -43,6 +43,7 @@ import ( corelisterv1 "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/util/retry" "knative.dev/pkg/logging" + "knative.dev/pkg/reconciler" ) const ( @@ -63,6 +64,8 @@ var nsIgnoreRegex = regexp.MustCompile(reconcilerCommon.NamespaceIgnorePattern) // current NamespaceSyncConfig from TektonConfig and ensures all declared // resources exist and are correct in the given namespace. type Reconciler struct { + reconciler.LeaderAwareFuncs + kubeClient kubernetes.Interface operatorClient clientset.Interface securityClientSet security.Interface From ba1e9f7a512bfd7d7cfa405913cdba06831d368d Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Fri, 26 Jun 2026 11:42:25 +0200 Subject: [PATCH 7/7] fix(namespacesync): export PipelineRoleBinding for e2e test The e2e test referenced tconfig.PipelineRoleBinding from the tektonconfig package, which no longer exists after the rbac.go cleanup. Export the constant from namespacesync (its new owner) and update the test import alias accordingly. Signed-off-by: Jawed khelil Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor --- .../operator.tekton.dev_tektonconfigs.yaml | 39 ++++++ docs/TektonConfig.md | 123 ++++++++++++++++++ .../operator/v1alpha1/openshift_platform.go | 8 ++ .../v1alpha1/tektonconfig_defaults.go | 34 ++++- .../v1alpha1/zz_generated.deepcopy.go | 5 + .../openshift/namespacesync/controller.go | 76 +++++++++-- .../openshift/namespacesync/reconciler.go | 41 ++++-- .../namespacesync/reconciler_test.go | 8 +- .../common/00_tektonconfigdeployment_test.go | 2 +- .../kube/informers/core/v1/secret/secret.go | 52 ++++++++ .../rbac/v1/rolebinding/rolebinding.go | 52 ++++++++ vendor/modules.txt | 2 + 12 files changed, 417 insertions(+), 25 deletions(-) create mode 100644 vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/secret/secret.go create mode 100644 vendor/knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding/rolebinding.go diff --git a/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml b/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml index 24cc94a92f..aed08509fe 100644 --- a/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml +++ b/config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml @@ -1021,6 +1021,45 @@ spec: type: string type: object type: object + secretBindings: + description: SecretBindings declares secrets to be automatically + bound to the pipeline SA in each namespace. Each entry must + set exactly one of labelSelector or secretName. + items: + description: SecretBinding describes a secret or class of + secrets to bind to the pipeline SA. + properties: + labelSelector: + description: LabelSelector selects secrets by label. + All matching secrets in a namespace are bound. + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + secretName: + description: SecretName binds a specific named secret + in each namespace to the pipeline SA. + type: string + type: object + type: array type: object type: object type: object diff --git a/docs/TektonConfig.md b/docs/TektonConfig.md index 6b191ff93e..d6ee6ca883 100644 --- a/docs/TektonConfig.md +++ b/docs/TektonConfig.md @@ -575,6 +575,129 @@ In the deployment the environment name will be converted as follows, - `tekton-hub-api` => `TEKTON_HUB_API` - `artifact-hub-api` => `ARTIFACT_HUB_API` +### NamespaceSync (OpenShift only) + +The `namespaceSync` block under `spec.platforms.openshift` controls the **NamespaceSyncController**, which watches every user namespace and ensures Tekton-required resources are present and up to date. It replaces the legacy per-namespace batch loop that was part of the RBAC reconciler. + +#### Resources managed per namespace + +| Resource | Kind | Purpose | +|---|---|---| +| `pipeline` | `ServiceAccount` | Identity for PipelineRun pods | +| `pipelines-scc-rolebinding` | `RoleBinding` → `pipelines-scc-clusterrole` | Grants the pipeline SA permission to use the default SCC | +| `openshift-pipelines-edit` | `RoleBinding` → `ClusterRole/edit` | Gives the pipeline SA edit access within its namespace | +| `config-trusted-cabundle` | `ConfigMap` | CA bundle for custom/internal PKI trust | +| `config-service-cabundle` | `ConfigMap` | OpenShift service CA bundle | +| `openshift-pipelines-clusterinterceptors` | `ClusterRoleBinding` subject | Lets the pipeline SA call ClusterInterceptors | + +#### Configuration fields + +```yaml +spec: + platforms: + openshift: + namespaceSync: + createPipelineSA: true # create/maintain the pipeline SA + createSCCRoleBinding: true # create/maintain pipelines-scc-rolebinding + createEditRoleBinding: true # create/maintain openshift-pipelines-edit + createCABundles: true # inject CA bundle ConfigMaps + + # Optional: restrict which namespaces are synced. + # Omit entirely to sync all non-system namespaces (default). + # Set to {} to opt out all namespaces without changing the flags above. + namespaceSelector: + matchLabels: + pipelines.openshift.io/sync: "true" + + # Optional: automatically bind secrets to the pipeline SA. + # Use secretName for an exact name, or labelSelector to match by label. + secretBindings: + - secretName: pipeline-quay-openshift # Quay Bridge robot account secret + - labelSelector: + matchLabels: + quay-integration: my-quay # all secrets with this label +``` + +All boolean fields default to `true` when the `namespaceSync` block is present. + +#### Disabling individual features + +Set the flag to `false` to stop managing that resource class. Existing resources +are **not deleted** — the controller simply stops reconciling them: + +```yaml +spec: + platforms: + openshift: + namespaceSync: + createEditRoleBinding: false # do not create openshift-pipelines-edit +``` + +#### Restricting sync to specific namespaces + +Use `namespaceSelector` to limit which namespaces the controller acts on. +Label namespaces you want synced, then configure the selector to match: + +```bash +# Label a namespace to opt in +oc label namespace my-project pipelines.openshift.io/sync=true +``` + +```yaml +spec: + platforms: + openshift: + namespaceSync: + namespaceSelector: + matchLabels: + pipelines.openshift.io/sync: "true" +``` + +To disable sync for **all** namespaces while keeping the feature flags intact, +set an empty selector: + +```yaml +namespaceSync: + namespaceSelector: {} # matches nothing → no namespace is synced +``` + +#### Quay Bridge secret auto-binding + +When the [Quay Bridge Operator](https://github.com/quay/quay-bridge-operator) is +installed, it creates a robot-account secret named `pipeline-quay-openshift` in +each namespace. Declare a `secretBinding` to have the NamespaceSyncController +automatically bind that secret to the `pipeline` SA as an image pull secret: + +```yaml +spec: + platforms: + openshift: + namespaceSync: + secretBindings: + - secretName: pipeline-quay-openshift +``` + +Once configured: +- When the secret appears in a namespace it is added to both `imagePullSecrets` + and `secrets` on the `pipeline` SA within seconds. +- When the secret is deleted the reference is removed automatically. + +#### Migration from legacy `spec.params` + +Older releases controlled this behaviour through `spec.params` entries. The +operator automatically migrates these on the first webhook call after an upgrade: + +| Legacy `spec.params` | Typed field | +|---|---| +| `createRbacResource: "false"` | `createPipelineSA`, `createSCCRoleBinding`, `createEditRoleBinding` all set to `false` | +| `createCABundleConfigMaps: "false"` | `createCABundles: false` | +| `legacyPipelineRbac: "false"` | `createEditRoleBinding: false` | + +After migration the legacy params are removed from `spec.params` and the typed +fields take effect. There is no need to manually update the TektonConfig CR. + +--- + ### OpenShiftPipelinesAsCode The PipelinesAsCode section allows you to customize the Pipelines as Code features on both Kubernetes and OpenShift. When you change the TektonConfig CR, the Operator automatically applies the settings to custom resources and configmaps in your installation. On Kubernetes, configure `spec.platforms.kubernetes.pipelinesAsCode` (the managed CR remains `OpenShiftPipelinesAsCode` for API compatibility). diff --git a/pkg/apis/operator/v1alpha1/openshift_platform.go b/pkg/apis/operator/v1alpha1/openshift_platform.go index 6c629ac56b..e71187a1e3 100644 --- a/pkg/apis/operator/v1alpha1/openshift_platform.go +++ b/pkg/apis/operator/v1alpha1/openshift_platform.go @@ -87,6 +87,14 @@ type NamespaceSyncConfig struct { // Each entry must set exactly one of labelSelector or secretName. // +optional SecretBindings []SecretBinding `json:"secretBindings,omitempty"` + + // NamespaceSelector is an optional label selector that restricts which + // namespaces are synced. When absent every non-system namespace is synced + // (the default). Use matchLabels/matchExpressions to limit sync to a subset + // of namespaces, or set it to an empty object ({}) to opt out all namespaces + // without touching the individual feature flags. + // +optional + NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"` } // SecretBinding describes a secret (or class of secrets by label) that the diff --git a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go index 9820b66268..4c70996dd0 100644 --- a/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go +++ b/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go @@ -29,21 +29,51 @@ import ( // the equivalent typed fields in spec.platforms.openshift.namespaceSync, then removes // the migrated params from the slice. Params that are already absent are left at their // typed-field defaults (true). This runs only on OpenShift. +// +// Original semantics (preserved here): +// - createRbacResource=false → master RBAC switch off; disables SA, SCC RoleBinding, +// AND edit RoleBinding, regardless of legacyPipelineRbac. +// - createCABundleConfigMaps=false → disables CA bundle ConfigMaps only. +// - legacyPipelineRbac=false → disables edit RoleBinding, but only when +// createRbacResource is true (or absent). func migrateNamespaceSyncParams(tc *TektonConfig) { ns := tc.Spec.Platforms.OpenShift.NamespaceSync + + // First pass: resolve the master RBAC switch so that its precedence over + // legacyPipelineRbac is respected regardless of param ordering in the slice. + masterRBACEnabled := true + for _, p := range tc.Spec.Params { + if p.Name == "createRbacResource" && p.Value == "false" { + masterRBACEnabled = false + break + } + } + remaining := tc.Spec.Params[:0] for _, p := range tc.Spec.Params { switch p.Name { case "createRbacResource": if ns.CreatePipelineSA == nil { - ns.CreatePipelineSA = ptr.Bool(p.Value != "false") + ns.CreatePipelineSA = ptr.Bool(masterRBACEnabled) + } + if !masterRBACEnabled { + // createRbacResource=false disabled all per-namespace RBAC in the + // old implementation. Carry that forward to the typed fields. + if ns.CreateSCCRoleBinding == nil { + ns.CreateSCCRoleBinding = ptr.Bool(false) + } + if ns.CreateEditRoleBinding == nil { + ns.CreateEditRoleBinding = ptr.Bool(false) + } } case "createCABundleConfigMaps": if ns.CreateCABundles == nil { ns.CreateCABundles = ptr.Bool(p.Value != "false") } case "legacyPipelineRbac": - if ns.CreateEditRoleBinding == nil { + // legacyPipelineRbac only had effect when createRbacResource was + // enabled; the master switch took precedence when it was false. + if masterRBACEnabled && ns.CreateEditRoleBinding == nil { ns.CreateEditRoleBinding = ptr.Bool(p.Value != "false") } default: diff --git a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go index 713b4642f8..b23a81b746 100644 --- a/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go @@ -685,6 +685,11 @@ func (in *NamespaceSyncConfig) DeepCopyInto(out *NamespaceSyncConfig) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NamespaceSelector != nil { + in, out := &in.NamespaceSelector, &out.NamespaceSelector + *out = new(metav1.LabelSelector) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/reconciler/openshift/namespacesync/controller.go b/pkg/reconciler/openshift/namespacesync/controller.go index a9ed0c1ac9..dff9217961 100644 --- a/pkg/reconciler/openshift/namespacesync/controller.go +++ b/pkg/reconciler/openshift/namespacesync/controller.go @@ -26,14 +26,18 @@ import ( pkgcommon "github.com/tektoncd/operator/pkg/common" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/cache" kubeclient "knative.dev/pkg/client/injection/kube/client" nsinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" + secretinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret" sainformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount" + rbinformer "knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding" "knative.dev/pkg/configmap" "knative.dev/pkg/controller" - secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret" "knative.dev/pkg/logging" ) @@ -45,6 +49,7 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl { nsInf := nsinformer.Get(ctx) saInf := sainformer.Get(ctx) secretInf := secretinformer.Get(ctx) + rbInf := rbinformer.Get(ctx) tcInf := tektonConfigInformer.Get(ctx) rec := &Reconciler{ @@ -132,15 +137,20 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl { // - A newly created secret that matches a binding rule is bound immediately. // - A deleted named secret is unbound from the pipeline SA. // - // Only trigger re-reconciliation when NamespaceSync has SecretBindings - // configured, to avoid a thundering herd on clusters without secret bindings. + // We use the cluster-wide kube factory here (NOT the namespacedkube one) + // because we need to observe secrets in all user namespaces, not just the + // operator's own namespace. + // + // To avoid a thundering-herd on clusters that don't use secret bindings, + // we only enqueue when NamespaceSync has SecretBindings configured AND the + // secret name/labels match at least one binding rule. if _, err := secretInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { secret, ok := obj.(*corev1.Secret) if !ok { return } - if namespaceSyncHasSecretBindings(tcInf.Lister()) { + if secretMatchesBinding(secret, tcInf.Lister()) { impl.EnqueueKey(types.NamespacedName{Name: secret.Namespace}) } }, @@ -154,7 +164,7 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl { return } } - if namespaceSyncHasSecretBindings(tcInf.Lister()) { + if secretMatchesBinding(secret, tcInf.Lister()) { impl.EnqueueKey(types.NamespacedName{Name: secret.Namespace}) } }, @@ -162,6 +172,33 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl { logger.Panicf("Couldn't register Secret informer event handler: %v", err) } + // RoleBinding Delete → re-enqueue the namespace for self-healing. + // We only watch the two RoleBindings that NamespaceSyncController owns: + // pipelines-scc-rolebinding and openshift-pipelines-edit. Watching all + // RoleBindings would be too noisy; we filter by name at the handler level. + managedRoleBindings := map[string]struct{}{ + pipelinesSCCRoleBinding: {}, + PipelineRoleBinding: {}, + } + if _, err := rbInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + DeleteFunc: func(obj interface{}) { + rb, ok := obj.(*rbacv1.RoleBinding) + if !ok { + if d, ok := obj.(cache.DeletedFinalStateUnknown); ok { + rb, ok = d.Obj.(*rbacv1.RoleBinding) + } + if !ok { + return + } + } + if _, watched := managedRoleBindings[rb.Name]; watched { + impl.EnqueueKey(types.NamespacedName{Name: rb.Namespace}) + } + }, + }); err != nil { + logger.Panicf("Couldn't register RoleBinding informer event handler: %v", err) + } + // TektonConfig changed → re-enqueue all namespaces only when the NamespaceSync // config itself changed. Unrelated TektonConfig field changes (e.g. pipeline // options, pruner settings) must not trigger a full namespace sweep — that @@ -190,10 +227,12 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl { return impl } -// namespaceSyncHasSecretBindings returns true when TektonConfig has at least one -// SecretBinding configured. Used to short-circuit Secret event handling when -// no secret binding is needed. -func namespaceSyncHasSecretBindings(lister interface { +// secretMatchesBinding returns true when the given secret matches at least one +// SecretBinding rule in the current TektonConfig. This is used to filter Secret +// events so we only re-enqueue namespaces for secrets we actually care about, +// avoiding unnecessary reconciles for the high volume of dockercfg / token +// secrets that Kubernetes creates automatically. +func secretMatchesBinding(secret *corev1.Secret, lister interface { Get(string) (*v1alpha1.TektonConfig, error) }) bool { tc, err := lister.Get(v1alpha1.ConfigResourceName) @@ -201,5 +240,22 @@ func namespaceSyncHasSecretBindings(lister interface { return false } cfg := tc.Spec.Platforms.OpenShift.NamespaceSync - return cfg != nil && len(cfg.SecretBindings) > 0 + if cfg == nil || len(cfg.SecretBindings) == 0 { + return false + } + for _, b := range cfg.SecretBindings { + if b.SecretName != "" && b.SecretName == secret.Name { + return true + } + if b.LabelSelector != nil { + sel, err := metav1.LabelSelectorAsSelector(b.LabelSelector) + if err != nil { + continue + } + if sel.Matches(labels.Set(secret.Labels)) { + return true + } + } + } + return false } diff --git a/pkg/reconciler/openshift/namespacesync/reconciler.go b/pkg/reconciler/openshift/namespacesync/reconciler.go index fcf41b2a6e..c0be3b05f2 100644 --- a/pkg/reconciler/openshift/namespacesync/reconciler.go +++ b/pkg/reconciler/openshift/namespacesync/reconciler.go @@ -47,11 +47,14 @@ import ( ) const ( - pipelineSA = "pipeline" - pipelinesSCCRole = "pipelines-scc-role" - pipelinesSCCClusterRole = "pipelines-scc-clusterrole" - pipelinesSCCRoleBinding = "pipelines-scc-rolebinding" - editRoleBinding = "openshift-pipelines-edit" + pipelineSA = "pipeline" + pipelinesSCCRole = "pipelines-scc-role" + pipelinesSCCClusterRole = "pipelines-scc-clusterrole" + pipelinesSCCRoleBinding = "pipelines-scc-rolebinding" + // PipelineRoleBinding is the name of the edit RoleBinding created in each + // user namespace so that the pipeline SA has edit access. Exported for use + // in e2e tests. + PipelineRoleBinding = "openshift-pipelines-edit" editClusterRole = "edit" serviceCABundleConfigMap = "config-service-cabundle" trustedCABundleConfigMap = "config-trusted-cabundle" @@ -110,6 +113,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { return nil } + if !namespaceMatchesSelector(ns, cfg) { + logger.Debugf("Namespace %s excluded by namespaceSelector, skipping", key) + return nil + } + return r.reconcileNamespace(ctx, ns, tc, cfg) } @@ -421,7 +429,7 @@ func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Names logger := logging.FromContext(ctx) rbClient := r.kubeClient.RbacV1().RoleBindings(ns.Name) - _, err := rbClient.Get(ctx, editRoleBinding, metav1.GetOptions{}) + _, err := rbClient.Get(ctx, PipelineRoleBinding, metav1.GetOptions{}) if err == nil { return nil } @@ -437,7 +445,7 @@ func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Names logger.Infof("Creating edit RoleBinding in namespace %s", ns.Name) _, err = rbClient.Create(ctx, &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: editRoleBinding, + Name: PipelineRoleBinding, Namespace: ns.Name, }, RoleRef: rbacv1.RoleRef{ @@ -457,7 +465,7 @@ func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Names // removeEditRoleBindingIfPresent deletes the openshift-pipelines-edit RoleBinding // when createEditRoleBinding is disabled. func (r *Reconciler) removeEditRoleBindingIfPresent(ctx context.Context, nsName string) error { - err := r.kubeClient.RbacV1().RoleBindings(nsName).Delete(ctx, editRoleBinding, metav1.DeleteOptions{}) + err := r.kubeClient.RbacV1().RoleBindings(nsName).Delete(ctx, PipelineRoleBinding, metav1.DeleteOptions{}) if errors.IsNotFound(err) { return nil } @@ -783,6 +791,23 @@ func shouldIgnoreNamespace(ns *corev1.Namespace) bool { return ns.DeletionTimestamp != nil } +// namespaceMatchesSelector returns true when the namespace should be synced +// according to cfg.NamespaceSelector. When no selector is configured every +// non-ignored namespace matches (opt-in all by default). Setting the selector +// to an empty matchLabels ({}) matches nothing, effectively disabling sync for +// all namespaces without touching the individual feature flags. +func namespaceMatchesSelector(ns *corev1.Namespace, cfg *v1alpha1.NamespaceSyncConfig) bool { + if cfg.NamespaceSelector == nil { + return true + } + sel, err := metav1.LabelSelectorAsSelector(cfg.NamespaceSelector) + if err != nil { + // Malformed selector — fail open so we don't silently stop syncing. + return true + } + return sel.Matches(labels.Set(ns.Labels)) +} + func tektonConfigOwnerRef(tc *v1alpha1.TektonConfig) metav1.OwnerReference { return metav1.OwnerReference{ APIVersion: tc.GroupVersionKind().GroupVersion().String(), diff --git a/pkg/reconciler/openshift/namespacesync/reconciler_test.go b/pkg/reconciler/openshift/namespacesync/reconciler_test.go index 600508a96e..5ec52169a4 100644 --- a/pkg/reconciler/openshift/namespacesync/reconciler_test.go +++ b/pkg/reconciler/openshift/namespacesync/reconciler_test.go @@ -231,7 +231,7 @@ func TestEnsureEditRoleBinding_CreatesWhenAbsent(t *testing.T) { err = r.Reconcile(context.Background(), "my-ns") assert.NilError(t, err) - rb, err := kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), editRoleBinding, metav1.GetOptions{}) + rb, err := kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), PipelineRoleBinding, metav1.GetOptions{}) assert.NilError(t, err) assert.Equal(t, editClusterRole, rb.RoleRef.Name) assert.Equal(t, pipelineSA, rb.Subjects[0].Name) @@ -250,7 +250,7 @@ func TestEnsureEditRoleBinding_IdempotentWhenPresent(t *testing.T) { }, metav1.CreateOptions{}) assert.NilError(t, err) _, err = kubeClient.RbacV1().RoleBindings("my-ns").Create(context.Background(), &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{Name: editRoleBinding, Namespace: "my-ns"}, + ObjectMeta: metav1.ObjectMeta{Name: PipelineRoleBinding, Namespace: "my-ns"}, RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: editClusterRole}, }, metav1.CreateOptions{}) assert.NilError(t, err) @@ -270,14 +270,14 @@ func TestRemoveEditRoleBinding_DeletesWhenPresent(t *testing.T) { // pre-create the RoleBinding — reconcile must delete it. _, err := kubeClient.RbacV1().RoleBindings("my-ns").Create(context.Background(), &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{Name: editRoleBinding, Namespace: "my-ns"}, + ObjectMeta: metav1.ObjectMeta{Name: PipelineRoleBinding, Namespace: "my-ns"}, }, metav1.CreateOptions{}) assert.NilError(t, err) err = r.Reconcile(context.Background(), "my-ns") assert.NilError(t, err) - _, err = kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), editRoleBinding, metav1.GetOptions{}) + _, err = kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), PipelineRoleBinding, metav1.GetOptions{}) assert.ErrorContains(t, err, "not found") } diff --git a/test/e2e/common/00_tektonconfigdeployment_test.go b/test/e2e/common/00_tektonconfigdeployment_test.go index 02e9909102..e2dfdf427d 100644 --- a/test/e2e/common/00_tektonconfigdeployment_test.go +++ b/test/e2e/common/00_tektonconfigdeployment_test.go @@ -33,7 +33,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1" "github.com/tektoncd/operator/pkg/reconciler/common" - tconfig "github.com/tektoncd/operator/pkg/reconciler/openshift/tektonconfig" + tconfig "github.com/tektoncd/operator/pkg/reconciler/openshift/namespacesync" "github.com/tektoncd/operator/test/client" "github.com/tektoncd/operator/test/resources" "github.com/tektoncd/operator/test/utils" diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/secret/secret.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/secret/secret.go new file mode 100644 index 0000000000..22ddeb5642 --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/secret/secret.go @@ -0,0 +1,52 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package secret + +import ( + context "context" + + v1 "k8s.io/client-go/informers/core/v1" + factory "knative.dev/pkg/client/injection/kube/informers/factory" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" + logging "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterInformer(withInformer) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := factory.Get(ctx) + inf := f.Core().V1().Secrets() + return context.WithValue(ctx, Key{}, inf), inf.Informer() +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) v1.SecretInformer { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch k8s.io/client-go/informers/core/v1.SecretInformer from context.") + } + return untyped.(v1.SecretInformer) +} diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding/rolebinding.go b/vendor/knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding/rolebinding.go new file mode 100644 index 0000000000..65c8012698 --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding/rolebinding.go @@ -0,0 +1,52 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package rolebinding + +import ( + context "context" + + v1 "k8s.io/client-go/informers/rbac/v1" + factory "knative.dev/pkg/client/injection/kube/informers/factory" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" + logging "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterInformer(withInformer) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := factory.Get(ctx) + inf := f.Rbac().V1().RoleBindings() + return context.WithValue(ctx, Key{}, inf), inf.Informer() +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) v1.RoleBindingInformer { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch k8s.io/client-go/informers/rbac/v1.RoleBindingInformer from context.") + } + return untyped.(v1.RoleBindingInformer) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 89ae75aca2..e825af8ee8 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2675,10 +2675,12 @@ knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1/validat knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment knative.dev/pkg/client/injection/kube/informers/apps/v1/statefulset knative.dev/pkg/client/injection/kube/informers/core/v1/namespace +knative.dev/pkg/client/injection/kube/informers/core/v1/secret knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount knative.dev/pkg/client/injection/kube/informers/factory knative.dev/pkg/client/injection/kube/informers/rbac/v1/clusterrole knative.dev/pkg/client/injection/kube/informers/rbac/v1/clusterrolebinding +knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding knative.dev/pkg/codegen/cmd/injection-gen knative.dev/pkg/codegen/cmd/injection-gen/args knative.dev/pkg/codegen/cmd/injection-gen/generators