feat(api): Add forceTenantPrefix option to Tenant spec (#1244)

Signed-off-by: samir-tahir <[email protected]>

api/v1beta2/tenant_types.go

@@ -56,6 +56,15 @@ 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 disable/enable the Tenant name prefix to specific Tenants, overriding global forceTenantPrefix in CapsuleConfiguration.
+	// When set to 'true', it 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.
+	// When set to 'false', it allows Namespaces created for this Tenant to be named anything.
+	// Overrides CapsuleConfiguration global forceTenantPrefix for the Tenant only.
+	// If unset, Tenant uses CapsuleConfiguration's forceTenantPrefix
+	// Optional
+	ForceTenantPrefix *bool `json:"forceTenantPrefix,omitempty"`
 }
 
 // +kubebuilder:object:root=true

charts/capsule/crds/capsule.clastix.io_tenants.yaml

@@ -1149,6 +1149,17 @@ spec:
                 description: Toggling the Tenant resources cordoning, when enable
                   resources cannot be deleted.
                 type: boolean
+              forceTenantPrefix:
+                description: |-
+                  Use this if you want to disable/enable the Tenant name prefix to specific Tenants, overriding global forceTenantPrefix in CapsuleConfiguration.
+                  When set to 'true', it 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.
+                  When set to 'false', it allows Namespaces created for this Tenant to be named anything.
+                  Overrides CapsuleConfiguration global forceTenantPrefix for the Tenant only.
+                  If unset, Tenant uses CapsuleConfiguration's forceTenantPrefix
+                  Optional
+                type: boolean
               imagePullPolicies:
                 description: Specify the allowed values for the imagePullPolicies
                   option in Pod resources. Capsule assures that all Pod resources

e2e/force_tenant_prefix_tenant_scope_test.go

@@ -0,0 +1,148 @@
+//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: &[]bool{true}[0],
+			Owners: capsulev1beta2.OwnerListSpec{
+				{
+					Name: "john",
+					Kind: "User",
+				},
+			},
+		},
+	}
+	t2 := &capsulev1beta2.Tenant{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "awesome-tenant",
+		},
+		Spec: capsulev1beta2.TenantSpec{
+			ForceTenantPrefix: &[]bool{false}[0],
+			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 and global ForceTenantPrefix false", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = false
+		})
+		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 fail using prefix without capsule.clastix.io/tenant label, where the user owns more than one Tenant, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix false", func() {
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should fail using prefix without capsule.clastix.io/tenant label, where the user owns more than one Tenant, for a tenant with ForceTenantPrefix false and global ForceTenantPrefix true", func() {
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t2.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 false", 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", 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", 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", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		ns := NewNamespace("awesome-namespace")
+		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 and global 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 succeed when not using prefix, with tenant label for a tenant with ForceTenantPrefix false and global ForceTenantPrefix true", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t2.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t2.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+	})
+})

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,
 		},
 	}
 }

pkg/webhook/namespace/prefix.go

@@ -59,6 +59,11 @@ func (r *prefixHandler) OnCreate(clt client.Client, decoder admission.Decoder, r
 					return utils.ErroredResponse(err)
 				}
 
+				// Check for Tenant-level ForceTenantPrefix override
+				if tnt.Spec.ForceTenantPrefix != nil && !*tnt.Spec.ForceTenantPrefix {
+					return nil
+				}
+
 				if e := fmt.Sprintf("%s-%s", tnt.GetName(), ns.GetName()); !strings.HasPrefix(ns.GetName(), fmt.Sprintf("%s-", tnt.GetName())) {
 					recorder.Eventf(tnt, corev1.EventTypeWarning, "InvalidTenantPrefix", "Namespace %s does not match the expected prefix for the current Tenant", ns.GetName())
 

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 != nil && *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 {