Skip to content

Commit

Permalink
Run rbac createresources for namespaces concurrently to avoid slowness
Browse files Browse the repository at this point in the history
  • Loading branch information
jkhelil committed Jul 10, 2024
1 parent 00a8507 commit dc4e7ac
Show file tree
Hide file tree
Showing 6 changed files with 293 additions and 39 deletions.
4 changes: 2 additions & 2 deletions pkg/common/scc.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func GetSecurityClient(ctx context.Context) *security.Clientset {
return securityClient
}

func VerifySCCExists(ctx context.Context, sccName string, securityClient *security.Clientset) error {
func VerifySCCExists(ctx context.Context, sccName string, securityClient security.Interface) error {
_, err := securityClient.SecurityV1().SecurityContextConstraints().Get(ctx, sccName, metav1.GetOptions{})
return err
}

func GetSCCRestrictiveList(ctx context.Context, securityClient *security.Clientset) ([]*securityv1.SecurityContextConstraints, error) {
func GetSCCRestrictiveList(ctx context.Context, securityClient security.Interface) ([]*securityv1.SecurityContextConstraints, error) {
logger := logging.FromContext(ctx)
sccList, err := securityClient.SecurityV1().SecurityContextConstraints().List(ctx, metav1.ListOptions{})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
PipelineNotFound = "tekton-pipelines not installed"
TriggerNotReady = "tekton-triggers not ready"
TriggerNotFound = "tekton-triggers not installed"
NamespaceIgnorePattern = "^(openshift|kube)-"
NamespaceIgnorePattern = "^(openshift|kube|open-cluster-management|package-operator)-"
)

func PipelineReady(informer informer.TektonPipelineInformer) (*v1alpha1.TektonPipeline, error) {
Expand Down
3 changes: 1 addition & 2 deletions pkg/reconciler/openshift/tektonconfig/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ func deleteInstallerSet(ctx context.Context, oc versioned.Interface, tc *v1alpha
// 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
func checkIfInstallerSetExist(ctx context.Context, oc versioned.Interface, relVersion string,
tc *v1alpha1.TektonConfig) (*v1alpha1.TektonInstallerSet, error) {
func checkIfInstallerSetExist(ctx context.Context, oc versioned.Interface, relVersion string) (*v1alpha1.TektonInstallerSet, error) {

labelSelector, err := common.LabelSelector(rbacInstallerSetSelector)
if err != nil {
Expand Down
102 changes: 68 additions & 34 deletions pkg/reconciler/openshift/tektonconfig/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"math"
"regexp"
"time"
"sync"

security "github.com/openshift/client-go/security/clientset/versioned"
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
Expand All @@ -41,6 +42,7 @@ import (
"k8s.io/client-go/kubernetes"
v1 "k8s.io/client-go/kubernetes/typed/core/v1"
"knative.dev/pkg/logging"
zap "go.uber.org/zap"
)

const (
Expand All @@ -65,6 +67,8 @@ const (
rbacInstallerSetNamePrefix = "rhosp-rbac-"
rbacParamName = "createRbacResource"
serviceAccountCreationLabel = "openshift-pipelines.tekton.dev/sa-created"

rbacMaxConcurrentCalls = 5
)

var (
Expand All @@ -82,7 +86,7 @@ var nsRegex = regexp.MustCompile(reconcilerCommon.NamespaceIgnorePattern)
type rbac struct {
kubeClientSet kubernetes.Interface
operatorClientSet clientset.Interface
securityClientSet *security.Clientset
securityClientSet security.Interface
rbacInformer rbacV1.ClusterRoleBindingInformer
nsInformer nsV1.NamespaceInformer
ownerRef metav1.OwnerReference
Expand Down Expand Up @@ -118,7 +122,7 @@ func (r *rbac) EnsureRBACInstallerSet(ctx context.Context) (*v1alpha1.TektonInst
return nil, err
}

rbacISet, err := checkIfInstallerSetExist(ctx, r.operatorClientSet, r.version, r.tektonConfig)
rbacISet, err := checkIfInstallerSetExist(ctx, r.operatorClientSet, r.version)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -393,54 +397,84 @@ func (r *rbac) createResources(ctx context.Context) error {
return err
}

sem := make(chan struct{}, rbacMaxConcurrentCalls) // Semaphore with buffer size of rbacMaxConcurrentCalls
var wg sync.WaitGroup
for _, ns := range namespacesToBeReconciled {
logger.Infow("Inject CA bundle configmap in ", "Namespace", ns.GetName())
if err := r.ensureCABundles(ctx, &ns); err != nil {
return fmt.Errorf("failed to ensure ca bundles presence in namespace %s, %w", ns.Name, err)
}
wg.Add(1)
namespace := ns
go func() {
defer wg.Done()
r.processResourcesForSingleNamespace(ctx, namespace, sem)
}()
}
wg.Wait()
return nil
}

logger.Infow("Ensures Default SA in ", "Namespace", ns.GetName())
sa, err := r.ensureSA(ctx, &ns)
if err != nil {
return fmt.Errorf("failed to ensure default SA in namespace %s, %w", ns.Name, err)
}
func (r *rbac) processResourcesForSingleNamespace(ctx context.Context, ns corev1.Namespace, sem chan struct{}) {
sem <- struct{}{} // Acquire a slot in the semaphore
defer func() { <-sem }() // Release the slot in the semaphore

// If "operator.tekton.dev/scc" exists in the namespace, then bind
// that SCC to the SA
err = r.handleSCCInNamespace(ctx, &ns)
if err != nil {
return fmt.Errorf("failed to bind scc to namespace %s; %w", ns.Name, err)
}
baseLogger := logging.FromContext(ctx)
logger := baseLogger.Desugar().With(zap.String("namespace", ns.Name)).Sugar()

// We use a namespace scoped Role when SCC annotation is present, and
// a cluster scoped ClusterRole when the default SCC is used
roleRef := r.getSCCRoleInNamespace(&ns)
if err := r.ensurePipelinesSCCRoleBinding(ctx, sa, roleRef); err != nil {
return fmt.Errorf("failed to create Pipeline Scc Role Binding in namespace %s, %w", ns.Name, err)
}
var hasError bool

if err := r.ensureRoleBindings(ctx, sa); err != nil {
return err
}
logger.Infof("ensure ca bundle configmap in this namespace")
if err := r.ensureCABundles(ctx, &ns); err != nil {
hasError = true
logger.Errorf("failed to ensure ca bundles presence in this namespace, %w", err)
}

if err := r.ensureClusterRoleBindings(ctx, sa); err != nil {
return err
}
logger.Infof("ensures default pipelines service account in this namespace")
sa, err := r.ensureSA(ctx, &ns)
if err != nil {
hasError = true
logger.Errorf("failed to ensure default SA in this namespace %w", err)
}

// If "operator.tekton.dev/scc" exists in the namespace, then bind
// that SCC to the SA
err = r.handleSCCInNamespace(ctx, &ns)
if err != nil {
hasError = true
logger.Errorf("failed to bind scc to this namespace %w", err)
}

// We use a namespace scoped Role when SCC annotation is present, and
// a cluster scoped ClusterRole when the default SCC is used
roleRef := r.getSCCRoleInNamespace(&ns)
if err := r.ensurePipelinesSCCRoleBinding(ctx, sa, roleRef); err != nil {
hasError = true
logger.Errorf("failed to create Pipeline Scc Role Binding in this namespace %w", err)
}

// Add `openshift-pipelines.tekton.dev/namespace-reconcile-version` label to namespace
// so that rbac won't loop on it again
if err := r.ensureRoleBindings(ctx, sa); err != nil {
hasError = true
logger.Errorf("failed to create rolebinding in this namespace %w", err)
}

if err := r.ensureClusterRoleBindings(ctx, sa); err != nil {
hasError = true
logger.Errorf("failed to create clusterrolebinding in this namespace %w", err)
}

// if No error add `openshift-pipelines.tekton.dev/namespace-reconcile-version` label to namespace
// so that rbac won't loop on it again
if !hasError {
logger.Infof("namespace %s sucessfully reconciled. Adding label namespace-reconcile-version to mark it as reconciled", ns.Name)
nsLabels := ns.GetLabels()
if len(nsLabels) == 0 {
nsLabels = map[string]string{}
}
nsLabels[namespaceVersionLabel] = r.version
ns.SetLabels(nsLabels)
if _, err := r.kubeClientSet.CoreV1().Namespaces().Update(ctx, &ns, metav1.UpdateOptions{}); err != nil {
return fmt.Errorf("failed to update namespace %s with label %s, %w", ns.Name, namespaceVersionLabel, err)
logger.Errorf("failed to update namespace %s with label %s, %w", ns.Name, namespaceVersionLabel, err)
}
} else {
logger.Errorf("failed to reconcile namespace %s, %w", ns.Name, err)
}

return nil
}

func (r *rbac) createSCCFailureEventInNamespace(ctx context.Context, namespace string, scc string) error {
Expand Down
179 changes: 179 additions & 0 deletions pkg/reconciler/openshift/tektonconfig/rbac_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package tektonconfig

import (
"context"
"os"
"testing"

"github.com/google/go-cmp/cmp"
v1 "github.com/openshift/api/security/v1"
fakesecurity "github.com/openshift/client-go/security/clientset/versioned/fake"
"github.com/stretchr/testify/require"
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"

//fakeoperator "github.com/tektoncd/operator/pkg/client/injection/client/fake"
fakeoperator "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake"
"github.com/tektoncd/operator/pkg/reconciler/common"
"github.com/tektoncd/pipeline/test/diff"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/informers"
fakek8s "k8s.io/client-go/kubernetes/fake"
duckv1 "knative.dev/pkg/apis/duck/v1"
ts "knative.dev/pkg/reconciler/testing"
)

func TestGetNamespacesToBeReconciled(t *testing.T) {
var deletionTime = metav1.Now()
for _, c := range []struct {
desc string
wantNamespaces []corev1.Namespace
objs []runtime.Object
ctx context.Context
}{
{
desc: "ignore system namespaces",
objs: []runtime.Object{
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "openshift-test"}},
},
wantNamespaces: nil,
ctx: context.Background(),
},
{
desc: "ignore namespaces with deletion timestamp",
objs: []runtime.Object{
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "openshift-test", DeletionTimestamp: &deletionTime}},
},
wantNamespaces: nil,
ctx: context.Background(),
},
{
desc: "add namespace to reconcile list if it has openshift scc operator.tekton.dev/scc annotation set ",
objs: []runtime.Object{
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", Annotations: map[string]string{"operator.tekton.dev/scc": "restricted"}}},
},
wantNamespaces: []corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Annotations: map[string]string{"operator.tekton.dev/scc": "restricted"},
},
},
},
ctx: context.Background(),
},
{
desc: "add namespace to reconcile list if it has bad label openshift-pipelines.tekton.dev/namespace-reconcile-version",
objs: []runtime.Object{
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", Labels: map[string]string{"openshift-pipelines.tekton.dev/namespace-reconcile-version": ""}}},
},
wantNamespaces: []corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"openshift-pipelines.tekton.dev/namespace-reconcile-version": ""},
},
},
},
ctx: context.Background(),
},
} {
t.Run(c.desc, func(t *testing.T) {
kubeclient := fakek8s.NewSimpleClientset(c.objs...)
r := rbac{
kubeClientSet: kubeclient,
version: "devel",
}
namespaces, err := r.getNamespacesToBeReconciled(c.ctx)
if err != nil {
t.Fatalf("getNamespacesToBeReconciled: %v", err)
}
if d := cmp.Diff(c.wantNamespaces, namespaces); d != "" {
t.Fatalf("Diff %s", diff.PrintWantGot(d))
}
})
}
}

func TestCreateResources(t *testing.T) {
ctx, _, _ := ts.SetupFakeContextWithCancel(t)
os.Setenv(common.KoEnvKey, "testdata")
scc := &v1.SecurityContextConstraints{ObjectMeta: metav1.ObjectMeta{Name: "PipelinesSCC"}}
tc := &v1alpha1.TektonConfig{
ObjectMeta: metav1.ObjectMeta{
Name: v1alpha1.ConfigResourceName,
Labels: map[string]string{},
},
Spec: v1alpha1.TektonConfigSpec{
CommonSpec: v1alpha1.CommonSpec{
TargetNamespace: "foo",
},
Platforms: v1alpha1.Platforms{
OpenShift: v1alpha1.OpenShift{
SCC: &v1alpha1.SCC{
Default: scc.Name,
},
},
},
},
Status: v1alpha1.TektonConfigStatus{
Status: duckv1.Status{},
TektonInstallerSet: map[string]string{},
},
}
for _, c := range []struct {
desc string
objs []runtime.Object
iSet *v1alpha1.TektonInstallerSet
err error
}{
{
desc: "No existing installer set",
objs: []runtime.Object{
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", Labels: map[string]string{"openshift-pipelines.tekton.dev/namespace-reconcile-version": ""}}},
},
err: v1alpha1.RECONCILE_AGAIN_ERR,
},
{
desc: "existing installer set",
objs: []runtime.Object{
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", Labels: map[string]string{"openshift-pipelines.tekton.dev/namespace-reconcile-version": ""}}},
},
iSet: &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: "devel", v1alpha1.TargetNamespaceKey: tc.Spec.TargetNamespace}}, Spec: v1alpha1.TektonInstallerSetSpec{}},
err: nil,
},
} {
t.Run(c.desc, func(t *testing.T) {
kubeclient := fakek8s.NewSimpleClientset(c.objs...)
fakeoperatorclient := fakeoperator.NewSimpleClientset()
fakesecurityclient := fakesecurity.NewSimpleClientset()
_, err := fakesecurityclient.SecurityV1().SecurityContextConstraints().Create(ctx, scc, metav1.CreateOptions{})
if err != nil {
t.Logf("Could not create fake scc %v", err)
}
if c.iSet != nil {
_, err := fakeoperatorclient.OperatorV1alpha1().TektonInstallerSets().Create(ctx, c.iSet, metav1.CreateOptions{})
if err != nil {
t.Logf("Could not create fake installerSet %v", err)
}
}
informers := informers.NewSharedInformerFactory(kubeclient, 0)
nsInformer := informers.Core().V1().Namespaces()
rbacinformer := informers.Rbac().V1().ClusterRoleBindings()

r := rbac{
kubeClientSet: kubeclient,
operatorClientSet: fakeoperatorclient,
securityClientSet: fakesecurityclient,
rbacInformer: rbacinformer,
nsInformer: nsInformer,
version: "devel",
tektonConfig: tc,
}
err = r.createResources(ctx)
require.ErrorIs(t, err, c.err)
})
}
}
Loading

0 comments on commit dc4e7ac

Please sign in to comment.