From be6695b598c605908664db584e368e42621fef0e Mon Sep 17 00:00:00 2001 From: Zorian Motso Date: Thu, 19 Dec 2024 17:17:58 +0200 Subject: [PATCH] feat: Add Stage CR namespace validation for multitenancy (#96) This change adds validation for the `Stage.spec.namespace` field to avoid conflicts when Stage CRs from different namespaces can point to the same namespace. Before creating the Stage, the webhook checks namespaces and returns an error if the namespace(with tenant label) already exists in the cluster. --- config/rbac/role.yaml | 14 ++++++++ .../templates/validation_webhook_rbac.yaml | 8 +++++ pkg/webhook/stage_webhook.go | 32 +++++++++++++++-- pkg/webhook/stage_webhook_test.go | 34 +++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 6ade38b..1d954d1 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -1,5 +1,19 @@ --- apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: manager-role +rules: +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: manager-role diff --git a/deploy-templates/templates/validation_webhook_rbac.yaml b/deploy-templates/templates/validation_webhook_rbac.yaml index 2d546b9..b015bb1 100644 --- a/deploy-templates/templates/validation_webhook_rbac.yaml +++ b/deploy-templates/templates/validation_webhook_rbac.yaml @@ -13,6 +13,14 @@ rules: - get - update - patch +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get + - list + - watch --- diff --git a/pkg/webhook/stage_webhook.go b/pkg/webhook/stage_webhook.go index 057f34a..b6691e3 100644 --- a/pkg/webhook/stage_webhook.go +++ b/pkg/webhook/stage_webhook.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -16,6 +18,7 @@ import ( const listLimit = 1000 //+kubebuilder:webhook:path=/validate-v2-edp-epam-com-v1-stage,mutating=false,failurePolicy=fail,sideEffects=None,groups=v2.edp.epam.com,resources=stages,verbs=create;update,versions=v1,name=stage.epam.com,admissionReviewVersions=v1 +//+kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch // StageValidationWebhook is a webhook for validating Stage CRD. type StageValidationWebhook struct { @@ -49,7 +52,11 @@ func (r *StageValidationWebhook) ValidateCreate(ctx context.Context, obj runtime return errors.New("the wrong object given, expected Stage") } - return r.uniqueNamespaces(ctx, createdStage) + if err := r.uniqueTargetNamespaces(ctx, createdStage); err != nil { + return err + } + + return r.uniqueTargetNamespaceAcrossCluster(ctx, createdStage) } // ValidateUpdate is a webhook for validating the updating of the Stage CR. @@ -63,7 +70,7 @@ func (*StageValidationWebhook) ValidateDelete(_ context.Context, _ runtime.Objec return nil } -func (r *StageValidationWebhook) uniqueNamespaces(ctx context.Context, stage *pipelineApi.Stage) error { +func (r *StageValidationWebhook) uniqueTargetNamespaces(ctx context.Context, stage *pipelineApi.Stage) error { stages := &pipelineApi.StageList{} if err := r.client.List( @@ -92,3 +99,24 @@ func (r *StageValidationWebhook) uniqueNamespaces(ctx context.Context, stage *pi return nil } + +func (r *StageValidationWebhook) uniqueTargetNamespaceAcrossCluster(ctx context.Context, stage *pipelineApi.Stage) error { + namespaces := &corev1.NamespaceList{} + if err := r.client.List( + ctx, + namespaces, + client.MatchingLabels{ + util.TenantLabelName: stage.Spec.Namespace, + }, + ); err != nil { + return fmt.Errorf("failed to list namespaces: %w", err) + } + + for i := range namespaces.Items { + if namespaces.Items[i].Name == stage.Spec.Namespace { + return fmt.Errorf("namespace %s is already used in the cluster", stage.Spec.Namespace) + } + } + + return nil +} diff --git a/pkg/webhook/stage_webhook_test.go b/pkg/webhook/stage_webhook_test.go index 0b1da74..6f43286 100644 --- a/pkg/webhook/stage_webhook_test.go +++ b/pkg/webhook/stage_webhook_test.go @@ -2,6 +2,8 @@ package webhook import ( "context" + "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" + corev1 "k8s.io/api/core/v1" "testing" "github.com/stretchr/testify/assert" @@ -20,6 +22,7 @@ func TestStageValidationWebhook_ValidateCreate(t *testing.T) { scheme := runtime.NewScheme() require.NoError(t, pipelineApi.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) tests := []struct { name string @@ -112,6 +115,37 @@ func TestStageValidationWebhook_ValidateCreate(t *testing.T) { require.Contains(t, err.Error(), "namespace stage1-ns is already used in CDPipeline pipeline Stage stage2") }, }, + { + name: "namespace already used in the cluster", + obj: &pipelineApi.Stage{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "stage1", + Namespace: "default", + }, + Spec: pipelineApi.StageSpec{ + Name: "stage1", + CdPipeline: "pipeline", + ClusterName: pipelineApi.InCluster, + Namespace: "ns1", + }, + }, + client: func(t *testing.T) client.Client { + ns1 := &corev1.Namespace{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "ns1", + Labels: map[string]string{ + util.TenantLabelName: "ns1", + }, + }, + } + + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(ns1).Build() + }, + wantErr: func(t require.TestingT, err error, i ...interface{}) { + require.Error(t, err) + require.Contains(t, err.Error(), "namespace ns1 is already used in the cluster") + }, + }, { name: "invalid object given", obj: &codebaseApi.Codebase{},