feat(api): Add forceTenantPrefix option to Tenant spec

Signed-off-by: samir-tahir <[email protected]>
projectcapsule · Nov 4, 2024 · bee7557 · bee7557
1 parent f82c2f4
commit bee7557
Show file tree

Hide file tree

Showing 5 changed files with 173 additions and 2 deletions.
diff --git a/api/v1beta2/tenant_types.go b/api/v1beta2/tenant_types.go
@@ -56,6 +56,12 @@ type TenantSpec struct {
 	// When enabled, the deletion request will be declined.
 	//+kubebuilder:default:=false
 	PreventDeletion bool `json:"preventDeletion,omitempty"`
+	// Use this if you want to enforce the Tenant name prefix to specific Tenants instead of globally with forceTenantPrefix in CapsuleConfiguration.
+	// Enforces Namespaces created for this Tenant to be named with the Tenant name prefix,
+	// separated by a dash (i.e. for Tenant 'foo', namespace names must be prefixed with 'foo-').
+	// This is useful to avoid Namespace name collision.
+	// +kubebuilder:default=false
+	ForceTenantPrefix bool `json:"forceTenantPrefix,omitempty"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml
@@ -1149,6 +1149,14 @@ spec:
                 description: Toggling the Tenant resources cordoning, when enable
                   resources cannot be deleted.
                 type: boolean
+              forceTenantPrefix:
+                default: false
+                description: |-
+                  Use this if you want to enforce the Tenant name prefix to specific Tenants instead of globally with forceTenantPrefix in CapsuleConfiguration.
+                  Enforces Namespaces created for this Tenant to be named with the Tenant name prefix,
+                  separated by a dash (i.e. for Tenant 'foo', namespace names must be prefixed with 'foo-').
+                  This is useful to avoid Namespace name collision.
+                type: boolean
               imagePullPolicies:
                 description: Specify the allowed values for the imagePullPolicies
                   option in Pod resources. Capsule assures that all Pod resources

diff --git a/e2e/force_tenant_prefix_tenant_scope_test.go b/e2e/force_tenant_prefix_tenant_scope_test.go
@@ -0,0 +1,128 @@
+//go:build e2e
+
+// Copyright 2020-2023 Project Capsule Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package e2e
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
+)
+
+var _ = Describe("creating a Namespace with Tenant name prefix enforcement at Tenant scope", func() {
+	t1 := &capsulev1beta2.Tenant{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "awesome",
+		},
+		Spec: capsulev1beta2.TenantSpec{
+			ForceTenantPrefix: true,
+			Owners: capsulev1beta2.OwnerListSpec{
+				{
+					Name: "john",
+					Kind: "User",
+				},
+			},
+		},
+	}
+	t2 := &capsulev1beta2.Tenant{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "awesome-tenant",
+		},
+		Spec: capsulev1beta2.TenantSpec{
+			Owners: capsulev1beta2.OwnerListSpec{
+				{
+					Name: "john",
+					Kind: "User",
+				},
+			},
+		},
+	}
+
+	JustBeforeEach(func() {
+		EventuallyCreation(func() error {
+			t1.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), t1)
+		}).Should(Succeed())
+		EventuallyCreation(func() error {
+			t2.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), t2)
+		}).Should(Succeed())
+	})
+	JustAfterEach(func() {
+		Expect(k8sClient.Delete(context.TODO(), t1)).Should(Succeed())
+		Expect(k8sClient.Delete(context.TODO(), t2)).Should(Succeed())
+
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = false
+		})
+	})
+
+	It("should fail when not using prefix, with tenant label for a tenant with ForceTenantPrefix true", func() {
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should succeed when not using prefix, with tenant label for a tenant with ForceTenantPrefix false", func() {
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t2.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t2.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+	})
+
+	It("should fail using prefix without capsule.clastix.io/tenant label, for a tenant with ForceTenantPrefix true", func() {
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should succeed and be assigned with prefix and label, for a tenant with ForceTenantPrefix true", func() {
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome-tenant", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+
+		TenantNamespaceList(t1, defaultTimeoutInterval).Should(ContainElement(ns.GetName()))
+	})
+
+	It("should fail when not using prefix, with tenant label for a tenant with ForceTenantPrefix true and global ForceTenantPrefix true in capsuleConfiguration", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should succeed and be assigned with prefix and label, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix true in capsuleConfiguration", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome-tenant", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+
+		TenantNamespaceList(t1, defaultTimeoutInterval).Should(ContainElement(ns.GetName()))
+	})
+
+	It("should fail using prefix without capsule.clastix.io/tenant label, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix true in capsuleConfiguration", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+})
diff --git a/e2e/utils_test.go b/e2e/utils_test.go
@@ -53,14 +53,20 @@ func ServiceCreation(svc *corev1.Service, owner capsulev1beta2.OwnerSpec, timeou
 	}, timeout, defaultPollInterval)
 }
 
-func NewNamespace(name string) *corev1.Namespace {
+func NewNamespace(name string, labels ...map[string]string) *corev1.Namespace {
 	if len(name) == 0 {
 		name = rand.String(10)
 	}
 
+	var namespaceLabels map[string]string
+	if len(labels) > 0 {
+		namespaceLabels = labels[0]
+	}
+
 	return &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Name:   name,
+			Labels: namespaceLabels,
 		},
 	}
 }

diff --git a/pkg/webhook/ownerreference/patching.go b/pkg/webhook/ownerreference/patching.go
@@ -144,6 +144,7 @@ func (h *handler) setOwnerRef(ctx context.Context, req admission.Request, client
 		return &response
 	}
 	// If we already had TenantName label on NS -> assign to it
+
 	if label, ok := ns.ObjectMeta.Labels[ln]; ok {
 		// retrieving the selected Tenant
 		tnt := &capsulev1beta2.Tenant{}
@@ -160,6 +161,10 @@ func (h *handler) setOwnerRef(ctx context.Context, req admission.Request, client
 
 			return &response
 		}
+		// Check if namespace needs Tenant name prefix
+		if errResponse := h.validateNamespacePrefix(ns, tnt); errResponse != nil {
+			return errResponse
+		}
 		// Patching the response
 		response := h.patchResponseForOwnerRef(tnt, ns, recorder)
 
@@ -221,6 +226,11 @@ func (h *handler) setOwnerRef(ctx context.Context, req admission.Request, client
 	}
 
 	if len(tenants) == 1 {
+		// Check if namespace needs Tenant name prefix
+		if errResponse := h.validateNamespacePrefix(ns, &tenants[0]); errResponse != nil {
+			return errResponse
+		}
+
 		response := h.patchResponseForOwnerRef(&tenants[0], ns, recorder)
 
 		return &response
@@ -281,6 +291,19 @@ func (h *handler) listTenantsForOwnerKind(ctx context.Context, ownerKind string,
 	return tntList, err
 }
 
+func (h *handler) validateNamespacePrefix(ns *corev1.Namespace, tenant *capsulev1beta2.Tenant) *admission.Response {
+	// Check if ForceTenantPrefix is true
+	if tenant.Spec.ForceTenantPrefix {
+		if !strings.HasPrefix(ns.GetName(), fmt.Sprintf("%s-", tenant.GetName())) {
+			response := admission.Denied(fmt.Sprintf("The Namespace name must start with '%s-' when ForceTenantPrefix is enabled in the Tenant.", tenant.GetName()))
+
+			return &response
+		}
+	}
+
+	return nil
+}
+
 type sortedTenants []capsulev1beta2.Tenant
 
 func (s sortedTenants) Len() int {