Skip to content

Commit c1be9cb

Browse files
jkhelilcursoragent
andcommitted
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 <jkhelil@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 14c0037 commit c1be9cb

9 files changed

Lines changed: 259 additions & 25 deletions

File tree

config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,45 @@ spec:
10211021
type: string
10221022
type: object
10231023
type: object
1024+
secretBindings:
1025+
description: SecretBindings declares secrets to be automatically
1026+
bound to the pipeline SA in each namespace. Each entry must
1027+
set exactly one of labelSelector or secretName.
1028+
items:
1029+
description: SecretBinding describes a secret or class of
1030+
secrets to bind to the pipeline SA.
1031+
properties:
1032+
labelSelector:
1033+
description: LabelSelector selects secrets by label.
1034+
All matching secrets in a namespace are bound.
1035+
properties:
1036+
matchExpressions:
1037+
items:
1038+
properties:
1039+
key:
1040+
type: string
1041+
operator:
1042+
type: string
1043+
values:
1044+
items:
1045+
type: string
1046+
type: array
1047+
required:
1048+
- key
1049+
- operator
1050+
type: object
1051+
type: array
1052+
matchLabels:
1053+
additionalProperties:
1054+
type: string
1055+
type: object
1056+
type: object
1057+
secretName:
1058+
description: SecretName binds a specific named secret
1059+
in each namespace to the pipeline SA.
1060+
type: string
1061+
type: object
1062+
type: array
10241063
type: object
10251064
type: object
10261065
type: object

pkg/apis/operator/v1alpha1/tektonconfig_defaults.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,51 @@ import (
2929
// the equivalent typed fields in spec.platforms.openshift.namespaceSync, then removes
3030
// the migrated params from the slice. Params that are already absent are left at their
3131
// typed-field defaults (true). This runs only on OpenShift.
32+
//
33+
// Original semantics (preserved here):
34+
// - createRbacResource=false → master RBAC switch off; disables SA, SCC RoleBinding,
35+
// AND edit RoleBinding, regardless of legacyPipelineRbac.
36+
// - createCABundleConfigMaps=false → disables CA bundle ConfigMaps only.
37+
// - legacyPipelineRbac=false → disables edit RoleBinding, but only when
38+
// createRbacResource is true (or absent).
3239
func migrateNamespaceSyncParams(tc *TektonConfig) {
3340
ns := tc.Spec.Platforms.OpenShift.NamespaceSync
41+
42+
// First pass: resolve the master RBAC switch so that its precedence over
43+
// legacyPipelineRbac is respected regardless of param ordering in the slice.
44+
masterRBACEnabled := true
45+
for _, p := range tc.Spec.Params {
46+
if p.Name == "createRbacResource" && p.Value == "false" {
47+
masterRBACEnabled = false
48+
break
49+
}
50+
}
51+
3452
remaining := tc.Spec.Params[:0]
3553
for _, p := range tc.Spec.Params {
3654
switch p.Name {
3755
case "createRbacResource":
3856
if ns.CreatePipelineSA == nil {
39-
ns.CreatePipelineSA = ptr.Bool(p.Value != "false")
57+
ns.CreatePipelineSA = ptr.Bool(masterRBACEnabled)
58+
}
59+
if !masterRBACEnabled {
60+
// createRbacResource=false disabled all per-namespace RBAC in the
61+
// old implementation. Carry that forward to the typed fields.
62+
if ns.CreateSCCRoleBinding == nil {
63+
ns.CreateSCCRoleBinding = ptr.Bool(false)
64+
}
65+
if ns.CreateEditRoleBinding == nil {
66+
ns.CreateEditRoleBinding = ptr.Bool(false)
67+
}
4068
}
4169
case "createCABundleConfigMaps":
4270
if ns.CreateCABundles == nil {
4371
ns.CreateCABundles = ptr.Bool(p.Value != "false")
4472
}
4573
case "legacyPipelineRbac":
46-
if ns.CreateEditRoleBinding == nil {
74+
// legacyPipelineRbac only had effect when createRbacResource was
75+
// enabled; the master switch took precedence when it was false.
76+
if masterRBACEnabled && ns.CreateEditRoleBinding == nil {
4777
ns.CreateEditRoleBinding = ptr.Bool(p.Value != "false")
4878
}
4979
default:

pkg/reconciler/openshift/namespacesync/controller.go

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,19 @@ import (
2525
tektonConfigInformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/tektonconfig"
2626
pkgcommon "github.com/tektoncd/operator/pkg/common"
2727

28+
rbacv1 "k8s.io/api/rbac/v1"
2829
corev1 "k8s.io/api/core/v1"
30+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/apimachinery/pkg/labels"
2932
"k8s.io/apimachinery/pkg/types"
3033
"k8s.io/client-go/tools/cache"
3134
kubeclient "knative.dev/pkg/client/injection/kube/client"
3235
nsinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace"
36+
secretinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret"
3337
sainformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
38+
rbinformer "knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding"
3439
"knative.dev/pkg/configmap"
3540
"knative.dev/pkg/controller"
36-
secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret"
3741
"knative.dev/pkg/logging"
3842
)
3943

@@ -45,6 +49,7 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl {
4549
nsInf := nsinformer.Get(ctx)
4650
saInf := sainformer.Get(ctx)
4751
secretInf := secretinformer.Get(ctx)
52+
rbInf := rbinformer.Get(ctx)
4853
tcInf := tektonConfigInformer.Get(ctx)
4954

5055
rec := &Reconciler{
@@ -132,15 +137,20 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl {
132137
// - A newly created secret that matches a binding rule is bound immediately.
133138
// - A deleted named secret is unbound from the pipeline SA.
134139
//
135-
// Only trigger re-reconciliation when NamespaceSync has SecretBindings
136-
// configured, to avoid a thundering herd on clusters without secret bindings.
140+
// We use the cluster-wide kube factory here (NOT the namespacedkube one)
141+
// because we need to observe secrets in all user namespaces, not just the
142+
// operator's own namespace.
143+
//
144+
// To avoid a thundering-herd on clusters that don't use secret bindings,
145+
// we only enqueue when NamespaceSync has SecretBindings configured AND the
146+
// secret name/labels match at least one binding rule.
137147
if _, err := secretInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
138148
AddFunc: func(obj interface{}) {
139149
secret, ok := obj.(*corev1.Secret)
140150
if !ok {
141151
return
142152
}
143-
if namespaceSyncHasSecretBindings(tcInf.Lister()) {
153+
if secretMatchesBinding(secret, tcInf.Lister()) {
144154
impl.EnqueueKey(types.NamespacedName{Name: secret.Namespace})
145155
}
146156
},
@@ -154,14 +164,41 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl {
154164
return
155165
}
156166
}
157-
if namespaceSyncHasSecretBindings(tcInf.Lister()) {
167+
if secretMatchesBinding(secret, tcInf.Lister()) {
158168
impl.EnqueueKey(types.NamespacedName{Name: secret.Namespace})
159169
}
160170
},
161171
}); err != nil {
162172
logger.Panicf("Couldn't register Secret informer event handler: %v", err)
163173
}
164174

175+
// RoleBinding Delete → re-enqueue the namespace for self-healing.
176+
// We only watch the two RoleBindings that NamespaceSyncController owns:
177+
// pipelines-scc-rolebinding and openshift-pipelines-edit. Watching all
178+
// RoleBindings would be too noisy; we filter by name at the handler level.
179+
managedRoleBindings := map[string]struct{}{
180+
pipelinesSCCRoleBinding: {},
181+
PipelineRoleBinding: {},
182+
}
183+
if _, err := rbInf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
184+
DeleteFunc: func(obj interface{}) {
185+
rb, ok := obj.(*rbacv1.RoleBinding)
186+
if !ok {
187+
if d, ok := obj.(cache.DeletedFinalStateUnknown); ok {
188+
rb, ok = d.Obj.(*rbacv1.RoleBinding)
189+
}
190+
if !ok {
191+
return
192+
}
193+
}
194+
if _, watched := managedRoleBindings[rb.Name]; watched {
195+
impl.EnqueueKey(types.NamespacedName{Name: rb.Namespace})
196+
}
197+
},
198+
}); err != nil {
199+
logger.Panicf("Couldn't register RoleBinding informer event handler: %v", err)
200+
}
201+
165202
// TektonConfig changed → re-enqueue all namespaces only when the NamespaceSync
166203
// config itself changed. Unrelated TektonConfig field changes (e.g. pipeline
167204
// options, pruner settings) must not trigger a full namespace sweep — that
@@ -190,16 +227,35 @@ func NewController(ctx context.Context, _ configmap.Watcher) *controller.Impl {
190227
return impl
191228
}
192229

193-
// namespaceSyncHasSecretBindings returns true when TektonConfig has at least one
194-
// SecretBinding configured. Used to short-circuit Secret event handling when
195-
// no secret binding is needed.
196-
func namespaceSyncHasSecretBindings(lister interface {
230+
// secretMatchesBinding returns true when the given secret matches at least one
231+
// SecretBinding rule in the current TektonConfig. This is used to filter Secret
232+
// events so we only re-enqueue namespaces for secrets we actually care about,
233+
// avoiding unnecessary reconciles for the high volume of dockercfg / token
234+
// secrets that Kubernetes creates automatically.
235+
func secretMatchesBinding(secret *corev1.Secret, lister interface {
197236
Get(string) (*v1alpha1.TektonConfig, error)
198237
}) bool {
199238
tc, err := lister.Get(v1alpha1.ConfigResourceName)
200239
if err != nil {
201240
return false
202241
}
203242
cfg := tc.Spec.Platforms.OpenShift.NamespaceSync
204-
return cfg != nil && len(cfg.SecretBindings) > 0
243+
if cfg == nil || len(cfg.SecretBindings) == 0 {
244+
return false
245+
}
246+
for _, b := range cfg.SecretBindings {
247+
if b.SecretName != "" && b.SecretName == secret.Name {
248+
return true
249+
}
250+
if b.LabelSelector != nil {
251+
sel, err := metav1.LabelSelectorAsSelector(b.LabelSelector)
252+
if err != nil {
253+
continue
254+
}
255+
if sel.Matches(labels.Set(secret.Labels)) {
256+
return true
257+
}
258+
}
259+
}
260+
return false
205261
}

pkg/reconciler/openshift/namespacesync/reconciler.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,14 @@ import (
4747
)
4848

4949
const (
50-
pipelineSA = "pipeline"
51-
pipelinesSCCRole = "pipelines-scc-role"
52-
pipelinesSCCClusterRole = "pipelines-scc-clusterrole"
53-
pipelinesSCCRoleBinding = "pipelines-scc-rolebinding"
54-
editRoleBinding = "openshift-pipelines-edit"
50+
pipelineSA = "pipeline"
51+
pipelinesSCCRole = "pipelines-scc-role"
52+
pipelinesSCCClusterRole = "pipelines-scc-clusterrole"
53+
pipelinesSCCRoleBinding = "pipelines-scc-rolebinding"
54+
// PipelineRoleBinding is the name of the edit RoleBinding created in each
55+
// user namespace so that the pipeline SA has edit access. Exported for use
56+
// in e2e tests.
57+
PipelineRoleBinding = "openshift-pipelines-edit"
5558
editClusterRole = "edit"
5659
serviceCABundleConfigMap = "config-service-cabundle"
5760
trustedCABundleConfigMap = "config-trusted-cabundle"
@@ -421,7 +424,7 @@ func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Names
421424
logger := logging.FromContext(ctx)
422425
rbClient := r.kubeClient.RbacV1().RoleBindings(ns.Name)
423426

424-
_, err := rbClient.Get(ctx, editRoleBinding, metav1.GetOptions{})
427+
_, err := rbClient.Get(ctx, PipelineRoleBinding, metav1.GetOptions{})
425428
if err == nil {
426429
return nil
427430
}
@@ -437,7 +440,7 @@ func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Names
437440
logger.Infof("Creating edit RoleBinding in namespace %s", ns.Name)
438441
_, err = rbClient.Create(ctx, &rbacv1.RoleBinding{
439442
ObjectMeta: metav1.ObjectMeta{
440-
Name: editRoleBinding,
443+
Name: PipelineRoleBinding,
441444
Namespace: ns.Name,
442445
},
443446
RoleRef: rbacv1.RoleRef{
@@ -457,7 +460,7 @@ func (r *Reconciler) ensureEditRoleBinding(ctx context.Context, ns *corev1.Names
457460
// removeEditRoleBindingIfPresent deletes the openshift-pipelines-edit RoleBinding
458461
// when createEditRoleBinding is disabled.
459462
func (r *Reconciler) removeEditRoleBindingIfPresent(ctx context.Context, nsName string) error {
460-
err := r.kubeClient.RbacV1().RoleBindings(nsName).Delete(ctx, editRoleBinding, metav1.DeleteOptions{})
463+
err := r.kubeClient.RbacV1().RoleBindings(nsName).Delete(ctx, PipelineRoleBinding, metav1.DeleteOptions{})
461464
if errors.IsNotFound(err) {
462465
return nil
463466
}

pkg/reconciler/openshift/namespacesync/reconciler_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func TestEnsureEditRoleBinding_CreatesWhenAbsent(t *testing.T) {
231231
err = r.Reconcile(context.Background(), "my-ns")
232232
assert.NilError(t, err)
233233

234-
rb, err := kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), editRoleBinding, metav1.GetOptions{})
234+
rb, err := kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), PipelineRoleBinding, metav1.GetOptions{})
235235
assert.NilError(t, err)
236236
assert.Equal(t, editClusterRole, rb.RoleRef.Name)
237237
assert.Equal(t, pipelineSA, rb.Subjects[0].Name)
@@ -250,7 +250,7 @@ func TestEnsureEditRoleBinding_IdempotentWhenPresent(t *testing.T) {
250250
}, metav1.CreateOptions{})
251251
assert.NilError(t, err)
252252
_, err = kubeClient.RbacV1().RoleBindings("my-ns").Create(context.Background(), &rbacv1.RoleBinding{
253-
ObjectMeta: metav1.ObjectMeta{Name: editRoleBinding, Namespace: "my-ns"},
253+
ObjectMeta: metav1.ObjectMeta{Name: PipelineRoleBinding, Namespace: "my-ns"},
254254
RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: editClusterRole},
255255
}, metav1.CreateOptions{})
256256
assert.NilError(t, err)
@@ -270,14 +270,14 @@ func TestRemoveEditRoleBinding_DeletesWhenPresent(t *testing.T) {
270270

271271
// pre-create the RoleBinding — reconcile must delete it.
272272
_, err := kubeClient.RbacV1().RoleBindings("my-ns").Create(context.Background(), &rbacv1.RoleBinding{
273-
ObjectMeta: metav1.ObjectMeta{Name: editRoleBinding, Namespace: "my-ns"},
273+
ObjectMeta: metav1.ObjectMeta{Name: PipelineRoleBinding, Namespace: "my-ns"},
274274
}, metav1.CreateOptions{})
275275
assert.NilError(t, err)
276276

277277
err = r.Reconcile(context.Background(), "my-ns")
278278
assert.NilError(t, err)
279279

280-
_, err = kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), editRoleBinding, metav1.GetOptions{})
280+
_, err = kubeClient.RbacV1().RoleBindings("my-ns").Get(context.Background(), PipelineRoleBinding, metav1.GetOptions{})
281281
assert.ErrorContains(t, err, "not found")
282282
}
283283

test/e2e/common/00_tektonconfigdeployment_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import (
3333
"github.com/stretchr/testify/suite"
3434
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
3535
"github.com/tektoncd/operator/pkg/reconciler/common"
36-
tconfig "github.com/tektoncd/operator/pkg/reconciler/openshift/tektonconfig"
36+
tconfig "github.com/tektoncd/operator/pkg/reconciler/openshift/namespacesync"
3737
"github.com/tektoncd/operator/test/client"
3838
"github.com/tektoncd/operator/test/resources"
3939
"github.com/tektoncd/operator/test/utils"

vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/secret/secret.go

Lines changed: 52 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)