Allowing dash on Tenant namespace (#118)

* Allowing dashes in the Tenant name as DNS RFC-1123

* Allowing force tenant prefix with Namespaces with dash

e2e/custom_capsule_group_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-group", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantassignedcustomgroup",
+			Name: "tenant-assigned-custom-group",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/force_tenant_prefix_test.go

@@ -49,52 +49,57 @@ var _ = Describe("creating a Namespace with --force-tenant-name flag", func() {
 			ResourceQuota:      []corev1.ResourceQuotaSpec{},
 		},
 	}
-	t2 := &v1alpha1.Tenant{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "second",
-		},
-		Spec: v1alpha1.TenantSpec{
-			Owner: v1alpha1.OwnerSpec{
-				Name: "john",
-				Kind: "User",
-			},
-			NamespacesMetadata: v1alpha1.AdditionalMetadata{},
-			ServicesMetadata:   v1alpha1.AdditionalMetadata{},
-			IngressClasses:     v1alpha1.IngressClassesSpec{},
-			StorageClasses:     v1alpha1.StorageClassesSpec{},
-			LimitRanges:        []corev1.LimitRangeSpec{},
-			NamespaceQuota:     10,
-			NodeSelector:       map[string]string{},
-			ResourceQuota:      []corev1.ResourceQuotaSpec{},
-		},
-	}
 	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())
+		ModifyCapsuleManagerPodArgs(append(defaulManagerPodArgs, []string{"--force-tenant-prefix"}...))
 	})
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), t1)).Should(Succeed())
-		Expect(k8sClient.Delete(context.TODO(), t2)).Should(Succeed())
+		ModifyCapsuleManagerPodArgs(defaulManagerPodArgs)
 	})
-	It("should fail", func() {
-		args := append(defaulManagerPodArgs, []string{"--force-tenant-prefix"}...)
-		ModifyCapsuleManagerPodArgs(args)
+	It("should fail with missing prefix", func() {
 		ns := NewNamespace("test")
 		NamespaceCreation(ns, t1, podRecreationTimeoutInterval).ShouldNot(Succeed())
 	})
-	It("should be assigned to the second Tenant", func() {
+	It("should succeed using prefix", func() {
+		ns := NewNamespace("first-test")
+		NamespaceCreation(ns, t1, podRecreationTimeoutInterval).Should(Succeed())
+	})
+	It("should fail with multiple Tenant assigned to user", func() {
+		t2 := &v1alpha1.Tenant{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "second",
+			},
+			Spec: v1alpha1.TenantSpec{
+				Owner: v1alpha1.OwnerSpec{
+					Name: "john",
+					Kind: "User",
+				},
+				NamespacesMetadata: v1alpha1.AdditionalMetadata{},
+				ServicesMetadata:   v1alpha1.AdditionalMetadata{},
+				IngressClasses:     v1alpha1.IngressClassesSpec{},
+				StorageClasses:     v1alpha1.StorageClassesSpec{},
+				LimitRanges:        []corev1.LimitRangeSpec{},
+				NamespaceQuota:     10,
+				NodeSelector:       map[string]string{},
+				ResourceQuota:      []corev1.ResourceQuotaSpec{},
+			},
+		}
+
+		EventuallyCreation(func() error {
+			return k8sClient.Create(context.TODO(), t2)
+		}).Should(Succeed())
+
+		defer func() {
+			Expect(k8sClient.Delete(context.TODO(), t2)).Should(Succeed())
+		}()
+
 		ns := NewNamespace("second-test")
 		ns2 := NewNamespace("second-test2")
-		NamespaceCreation(ns, t2, podRecreationTimeoutInterval).Should(Succeed())
-		TenantNamespaceList(t2, podRecreationTimeoutInterval).Should(ContainElement(ns.GetName()))
+		NamespaceCreation(ns, t2, podRecreationTimeoutInterval).ShouldNot(Succeed())
 		NamespaceCreation(ns2, t1, podRecreationTimeoutInterval).ShouldNot(Succeed())
-		args := defaulManagerPodArgs
-		ModifyCapsuleManagerPodArgs(args)
 	})
 })

e2e/ingress_class_test.go

@@ -33,7 +33,7 @@ import (
 var _ = Describe("when Tenant handles Ingress classes", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "ingressclass",
+			Name: "ingress-class",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/namespace_metadata_test.go

@@ -33,7 +33,7 @@ import (
 var _ = Describe("creating a Namespace for a Tenant with additional metadata", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantmetadata",
+			Name: "tenant-metadata",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/new_namespace_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace as Tenant owner", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantassigned",
+			Name: "tenant-assigned",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/overquota_namespace_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace over-quota", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "overquotatenant",
+			Name: "over-quota-tenant",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/owner_webhooks_test.go

@@ -36,7 +36,7 @@ import (
 var _ = Describe("when Tenant owner interacts with the webhooks", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantowner",
+			Name: "tenant-owner",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/protected_namespace_regex_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace with --protected-namespace-regex enabled", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantprotectednamespace",
+			Name: "tenant-protected-namespace",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/resource_quota_exceeded_test.go

@@ -39,7 +39,7 @@ import (
 var _ = Describe("exceeding Tenant resource quota", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantresourceschanges",
+			Name: "tenant-resources-changes",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/selecting_non_owned_tenant_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace trying to select a third Tenant", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantnonowned",
+			Name: "tenant-non-owned",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/selecting_tenant_fail_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace without a Tenant selector when user owns multiple Tenants", func() {
 	t1 := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantone",
+			Name: "tenant-one",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{
@@ -51,7 +51,7 @@ var _ = Describe("creating a Namespace without a Tenant selector when user owns
 	}
 	t2 := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenanttwo",
+			Name: "tenant-two",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{
@@ -70,7 +70,7 @@ var _ = Describe("creating a Namespace without a Tenant selector when user owns
 	}
 	t3 := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantthree",
+			Name: "tenant-three",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{
@@ -89,7 +89,7 @@ var _ = Describe("creating a Namespace without a Tenant selector when user owns
 	}
 	t4 := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantfour",
+			Name: "tenant-four",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/selecting_tenant_with_label_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace with Tenant selector when user owns multiple tenants", func() {
 	t1 := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantone",
+			Name: "tenant-one",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{
@@ -51,7 +51,7 @@ var _ = Describe("creating a Namespace with Tenant selector when user owns multi
 	}
 	t2 := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenanttwo",
+			Name: "tenant-two",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/service_metadata_test.go

@@ -34,7 +34,7 @@ import (
 var _ = Describe("creating a Service/Endpoint/EndpointSlice for a Tenant with additional metadata", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "servicemetadata",
+			Name: "service-metadata",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/storage_class_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("when Tenant handles Storage classes", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "storageclass",
+			Name: "storage-class",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/tenant_name_webhook_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Tenant with wrong name", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "wrong-name",
+			Name: "non_rfc_dns_1123",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/tenant_owner_group_test.go

@@ -32,7 +32,7 @@ import (
 var _ = Describe("creating a Namespace with group Tenant owner", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantgroupowner",
+			Name: "tenant-group-owner",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/tenant_resources_changes_test.go

@@ -37,7 +37,7 @@ import (
 var _ = Describe("changing Tenant managed Kubernetes resources", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantresourceschanges",
+			Name: "tenant-resources-changes",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

e2e/tenant_resources_test.go

@@ -38,7 +38,7 @@ import (
 var _ = Describe("creating namespaces within a Tenant with resources", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "tenantresources",
+			Name: "tenant-resources",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{

pkg/webhook/owner_reference/patching.go

@@ -79,73 +79,57 @@ func (h *handler) OnCreate(clt client.Client, decoder *admission.Decoder) capsul
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 		// If we already had TenantName label on NS -> assign to it
-		if len(ns.ObjectMeta.Labels) > 0 {
-			l, ok := ns.ObjectMeta.Labels[ln]
-			// assigning namespace to Tenant in case of label
-			if ok {
-				// retrieving the selected Tenant
-				t := &capsulev1alpha1.Tenant{}
-				if err := clt.Get(ctx, types.NamespacedName{Name: l}, t); err != nil {
-					return admission.Errored(http.StatusBadRequest, err)
-				}
-				// Tenant owner must adhere to user that asked for NS creation
-				if !h.isTenantOwner(t.Spec.Owner, req.UserInfo) {
-					return admission.Denied("Cannot assign the desired namespace to a non-owned Tenant")
-				}
-				// Patching the response
-				return h.patchResponseForOwnerRef(t, ns)
-			}
-
-		}
-		// If we forceTenantPrefix -> find Tenant from NS name
-		if h.forceTenantPrefix {
-			t := &v1alpha1.Tenant{}
-			tenantName := strings.Split(ns.GetName(), "-")[0]
-			if err := clt.Get(ctx, types.NamespacedName{Name: tenantName}, t); err != nil {
+		if l, ok := ns.ObjectMeta.Labels[ln]; ok {
+			// retrieving the selected Tenant
+			t := &capsulev1alpha1.Tenant{}
+			if err := clt.Get(ctx, types.NamespacedName{Name: l}, t); err != nil {
 				return admission.Errored(http.StatusBadRequest, err)
 			}
+			// Tenant owner must adhere to user that asked for NS creation
+			if !h.isTenantOwner(t.Spec.Owner, req.UserInfo) {
+				return admission.Denied("Cannot assign the desired namespace to a non-owned Tenant")
+			}
+			// Patching the response
 			return h.patchResponseForOwnerRef(t, ns)
 		}
 
-		tenants := []*capsulev1alpha1.Tenant{}
+		// If we forceTenantPrefix -> find Tenant from NS name
+		var tenants []*capsulev1alpha1.Tenant
 
 		// Find tenants belonging to user
-		tlu, err := h.listTenantsForOwnerKind(ctx, "User", req.UserInfo.Username, clt)
-		if err != nil {
-			return admission.Errored(http.StatusBadRequest, err)
-		}
-		// No groups single tenant short-circuit
-		if len(req.UserInfo.Groups) == 0 && len(tlu.Items) == 1 {
-			return h.patchResponseForOwnerRef(&tlu.Items[0], ns)
-		}
-
-		switch userTenants := len(tlu.Items); {
-		case userTenants > 1:
-			return admission.Denied("Unable to assign namespace to tenant. Please use " + ln + " label when creating a namespace")
-		case userTenants == 1:
-			tenants = append(tenants, &tlu.Items[0])
-		}
-
-		for _, group := range req.UserInfo.Groups {
-			tl, err := h.listTenantsForOwnerKind(ctx, "Group", group, clt)
+		{
+			tl, err := h.listTenantsForOwnerKind(ctx, "User", req.UserInfo.Username, clt)
 			if err != nil {
 				return admission.Errored(http.StatusBadRequest, err)
 			}
-			for _, item := range tl.Items {
-				tenants = append(tenants, &item)
+			for _, tnt := range tl.Items {
+				tenants = append(tenants, &tnt)
 			}
-			// more than one tenant found, returning error
-			if len(tenants) > 1 {
-				return admission.Denied("Unable to assign namespace to tenant. Please use " + ln + " label when creating a namespace")
+		}
+		// Find tenants belonging to user groups
+		{
+			for _, group := range req.UserInfo.Groups {
+				tl, err := h.listTenantsForOwnerKind(ctx, "Group", group, clt)
+				if err != nil {
+					return admission.Errored(http.StatusBadRequest, err)
+				}
+				for _, tnt := range tl.Items {
+					tenants = append(tenants, &tnt)
+				}
 			}
 		}
 
-		// Single tenant found for group
-		if len(tenants) == 1 {
+		switch len(tenants) {
+		case 0:
+			return admission.Denied("You do not have any Tenant assigned: please, reach out the system administrators")
+		case 1:
+			if h.forceTenantPrefix && !strings.HasPrefix(ns.GetName(), tenants[0].GetName()) {
+				return admission.Denied("The namespace doesn't match any assigned tenants prefix. Please, use the desired tenant name as prefix followed by a dash when creating a namespace")
+			}
 			return h.patchResponseForOwnerRef(tenants[0], ns)
+		default:
+			return admission.Denied("Unable to assign namespace to tenant. Please use " + ln + " label when creating a namespace")
 		}
-
-		return admission.Denied("You do not have any Tenant assigned: please, reach out the system administrators")
 	}
 }
 

pkg/webhook/tenant/validating.go

@@ -64,7 +64,7 @@ func (r *handler) OnCreate(client client.Client, decoder *admission.Decoder) cap
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 
-		matched, _ := regexp.MatchString(`^[a-z0-9]([a-z0-9]*[a-z0-9])?$`, tnt.GetName())
+		matched, _ := regexp.MatchString(`[a-z0-9]([-a-z0-9]*[a-z0-9])?`, tnt.GetName())
 		if !matched {
 			return admission.Denied("Tenant name has forbidden characters")
 		}