[OpenShift] Optimize rbac creation in TektonConfig extention

pkg/reconciler/common/name_test.go

@@ -17,9 +17,10 @@ limitations under the License.
 package common
 
 import (
-	utilrand "k8s.io/apimachinery/pkg/util/rand"
 	"strings"
 	"testing"
+
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
 )
 
 func TestRestrictLengthWithRandomSuffix(t *testing.T) {

pkg/reconciler/openshift/tektonconfig/controller.go

@@ -25,6 +25,7 @@ import (
 	namespaceinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace"
 	"knative.dev/pkg/configmap"
 	"knative.dev/pkg/controller"
+	"knative.dev/pkg/kmeta"
 )
 
 // NewController initializes the controller and is called by the generated code
@@ -41,8 +42,13 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
 // This is required because we want to get our TektonConfig reconciler triggered
 // for already existing and new namespaces, without manual intervention like adding
 // a label/annotation on namespace to make it manageable by Tekton controller.
+// This will also filter the namespaces by regex `^(openshift|kube)-`
+// and enqueue only when namespace doesn't match the regex
 func enqueueCustomName(impl *controller.Impl, name string) func(obj interface{}) {
 	return func(obj interface{}) {
-		impl.EnqueueKey(types.NamespacedName{Namespace: "", Name: name})
+		object, err := kmeta.DeletionHandlingAccessor(obj)
+		if err == nil && !nsRegex.MatchString(object.GetName()) {
+			impl.EnqueueKey(types.NamespacedName{Namespace: "", Name: name})
+		}
 	}
 }

pkg/reconciler/openshift/tektonconfig/extension.go

@@ -88,9 +88,15 @@ func (oe openshiftExtension) PostReconcile(ctx context.Context, comp v1alpha1.Te
 func (oe openshiftExtension) Finalize(ctx context.Context, comp v1alpha1.TektonComponent) error {
 	configInstance := comp.(*v1alpha1.TektonConfig)
 	if configInstance.Spec.Profile == common.ProfileAll {
-		return extension.TektonAddonCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonAddons(), common.AddonResourceName)
+		if err := extension.TektonAddonCRDelete(oe.operatorClientSet.OperatorV1alpha1().TektonAddons(), common.AddonResourceName); err != nil {
+			return err
+		}
 	}
-	return nil
+
+	r := rbac{
+		kubeClientSet: oe.kubeClientSet,
+	}
+	return r.cleanUp(ctx)
 }
 
 // configOwnerRef returns owner reference pointing to passed instance

pkg/reconciler/openshift/tektonconfig/rbac.go

@@ -18,20 +18,19 @@ package tektonconfig
 
 import (
 	"context"
+	"fmt"
 	"regexp"
 
+	mf "github.com/manifestival/manifestival"
+	clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned"
 	"github.com/tektoncd/operator/pkg/reconciler/common"
-
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
-	"knative.dev/pkg/logging"
-
-	mf "github.com/manifestival/manifestival"
-	clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	"knative.dev/pkg/logging"
 )
 
 const (
@@ -42,39 +41,78 @@ const (
 	serviceCABundleConfigMap = "config-service-cabundle"
 	trustedCABundleConfigMap = "config-trusted-cabundle"
 	clusterInterceptors      = "openshift-pipelines-clusterinterceptors"
+	namespaceRbacLabel       = "openshift-pipelines.tekton.dev/namespace-ready"
 )
 
+// Namespace Regex to ignore the namespace for creating rbac resources.
+var nsRegex = regexp.MustCompile(common.NamespaceIgnorePattern)
+
 type rbac struct {
 	kubeClientSet     kubernetes.Interface
 	operatorClientSet clientset.Interface
 	manifest          mf.Manifest
 	ownerRef          metav1.OwnerReference
 }
 
-func (r *rbac) createResources(ctx context.Context) error {
+func (r *rbac) cleanUp(ctx context.Context) error {
 
-	logger := logging.FromContext(ctx)
-
-	// Maintaining a separate cluster role for the scc declaration.
-	// to assist us in managing this the scc association in a
-	// granular way.
-	if err := r.ensurePipelinesSCClusterRole(ctx); err != nil {
+	// fetch the list of all namespaces which have label
+	// `openshift-pipelines.tekton.dev/namespace-ready: true`
+	namespaces, err := r.kubeClientSet.CoreV1().Namespaces().List(ctx, metav1.ListOptions{
+		LabelSelector: fmt.Sprintf("%s = true", namespaceRbacLabel),
+	})
+	if err != nil {
 		return err
 	}
+	// loop on namespaces and remove label if exist
+	for _, n := range namespaces.Items {
+		labels := n.GetLabels()
+		delete(labels, namespaceRbacLabel)
+		n.SetLabels(labels)
+		if _, err := r.kubeClientSet.CoreV1().Namespaces().Update(ctx, &n, metav1.UpdateOptions{}); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (r *rbac) createResources(ctx context.Context) error {
+
+	logger := logging.FromContext(ctx)
 
-	namespaces, err := r.kubeClientSet.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
+	// fetch the list of all namespaces which doesn't have label
+	// `openshift-pipelines.tekton.dev/namespace-ready: true`
+	namespaces, err := r.kubeClientSet.CoreV1().Namespaces().List(ctx, metav1.ListOptions{
+		LabelSelector: fmt.Sprintf("%s != true", namespaceRbacLabel),
+	})
 	if err != nil {
 		return err
 	}
 
-	re := regexp.MustCompile(common.NamespaceIgnorePattern)
+	// list of namespaces rbac resources need to be created
+	var rbacNamespaces []corev1.Namespace
 
+	// filter namespaces:
+	// ignore ns with name passing regex `^(openshift|kube)-`
 	for _, n := range namespaces.Items {
-
-		if ignore := re.MatchString(n.GetName()); ignore {
-			logger.Info("IGNORES Namespace: ", n.GetName())
+		if ignore := nsRegex.MatchString(n.GetName()); ignore {
 			continue
 		}
+		rbacNamespaces = append(rbacNamespaces, n)
+	}
+
+	if len(rbacNamespaces) == 0 {
+		return nil
+	}
+
+	// Maintaining a separate cluster role for the scc declaration.
+	// to assist us in managing this the scc association in a
+	// granular way.
+	if err := r.ensurePipelinesSCClusterRole(ctx); err != nil {
+		return err
+	}
+
+	for _, n := range rbacNamespaces {
 
 		logger.Infow("Inject CA bundle configmap in ", "Namespace", n.GetName())
 		if err := r.ensureCABundles(ctx, &n); err != nil {
@@ -98,6 +136,15 @@ func (r *rbac) createResources(ctx context.Context) error {
 		if err := r.ensureClusterRoleBindings(ctx, sa); err != nil {
 			return err
 		}
+
+		// Add `openshift-pipelines.tekton.dev/namespace-ready` label to namespace
+		// so that rbac won't loop on it again
+		nsLabels := n.GetLabels()
+		nsLabels[namespaceRbacLabel] = "true"
+		n.SetLabels(nsLabels)
+		if _, err := r.kubeClientSet.CoreV1().Namespaces().Update(ctx, &n, metav1.UpdateOptions{}); err != nil {
+			return err
+		}
 	}
 
 	return nil

test/e2e-tests.sh

@@ -32,6 +32,9 @@ failed=0
 header "Setting up environment"
 install_operator_resources
 
+echo "Wait for TektonConfig creation"
+sleep 30
+
 header "Running Go e2e tests"
 go_test_e2e -timeout=20m ./test/e2e/common ${KUBECONFIG_PARAM} || failed=1
 go_test_e2e -timeout=20m ./test/e2e/${TARGET} ${KUBECONFIG_PARAM} || failed=1

test/resources/tektonconfigs.go

@@ -26,6 +26,7 @@ import (
 
 	mfc "github.com/manifestival/client-go-client"
 	mf "github.com/manifestival/manifestival"
+	"github.com/tektoncd/operator/pkg/reconciler/common"
 
 	"knative.dev/pkg/test/logging"
 
@@ -63,9 +64,22 @@ func EnsureTektonConfigExists(kubeClientSet *kubernetes.Clientset, clients confi
 				Name: names.TektonConfig,
 			},
 			Spec: v1alpha1.TektonConfigSpec{
+				Profile: common.ProfileAll,
 				CommonSpec: v1alpha1.CommonSpec{
 					TargetNamespace: cm.Data["DEFAULT_TARGET_NAMESPACE"],
 				},
+				Addon: v1alpha1.Addon{
+					Params: []v1alpha1.Param{
+						{
+							Name:  "pipelineTemplates",
+							Value: "true",
+						},
+						{
+							Name:  "clusterTasks",
+							Value: "true",
+						},
+					},
+				},
 			},
 		}
 		return clients.Create(context.TODO(), tcCR, metav1.CreateOptions{})