From da66f404629a202fb41f3e516da1f3d04b897623 Mon Sep 17 00:00:00 2001 From: Samir Tahir <30797145+samirtahir91@users.noreply.github.com> Date: Wed, 4 Dec 2024 05:45:05 +0000 Subject: [PATCH] feat(api): Add forceTenantPrefix option to Tenant spec (#1244) Signed-off-by: samir-tahir --- api/v1beta2/tenant_types.go | 9 ++ api/v1beta2/zz_generated.deepcopy.go | 5 + .../crds/capsule.clastix.io_tenants.yaml | 11 ++ e2e/force_tenant_prefix_tenant_scope_test.go | 148 ++++++++++++++++++ e2e/utils_test.go | 10 +- pkg/webhook/namespace/prefix.go | 5 + pkg/webhook/ownerreference/patching.go | 23 +++ 7 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 e2e/force_tenant_prefix_tenant_scope_test.go diff --git a/api/v1beta2/tenant_types.go b/api/v1beta2/tenant_types.go index bb67c3bd..beda86da 100644 --- a/api/v1beta2/tenant_types.go +++ b/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 diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index f9feb05e..79ab4aba 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -763,6 +763,11 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) { *out = new(api.DefaultAllowedListSpec) (*in).DeepCopyInto(*out) } + if in.ForceTenantPrefix != nil { + in, out := &in.ForceTenantPrefix, &out.ForceTenantPrefix + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TenantSpec. diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml index c2cade71..dcc820f9 100644 --- a/charts/capsule/crds/capsule.clastix.io_tenants.yaml +++ b/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 diff --git a/e2e/force_tenant_prefix_tenant_scope_test.go b/e2e/force_tenant_prefix_tenant_scope_test.go new file mode 100644 index 00000000..5cfcc298 --- /dev/null +++ b/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()) + }) +}) diff --git a/e2e/utils_test.go b/e2e/utils_test.go index 5ab07446..10a43693 100644 --- 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/namespace/prefix.go b/pkg/webhook/namespace/prefix.go index 734081a7..1bb9935e 100644 --- a/pkg/webhook/namespace/prefix.go +++ b/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()) diff --git a/pkg/webhook/ownerreference/patching.go b/pkg/webhook/ownerreference/patching.go index 8a7ac7cc..1163ab1f 100644 --- 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 != 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 {