diff --git a/e2e/ingress_hostnames_test.go b/e2e/ingress_hostnames_test.go index be8efacf..f09c771c 100644 --- a/e2e/ingress_hostnames_test.go +++ b/e2e/ingress_hostnames_test.go @@ -13,6 +13,7 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" "github.com/projectcapsule/capsule/pkg/api" @@ -305,4 +306,33 @@ var _ = Describe("when Tenant handles Ingress hostnames", Ordered, Label("tenant } }) }) + + It("should deny updating allowed hostnames with an invalid regex", func() { + invalidRegex := "(" + + Eventually(func() error { + current := &capsulev1beta2.Tenant{} + if err := k8sClient.Get( + context.TODO(), + client.ObjectKeyFromObject(tnt), + current, + ); err != nil { + return err + } + + if current.Spec.IngressOptions.AllowedHostnames == nil { + current.Spec.IngressOptions.AllowedHostnames = &api.AllowedListSpec{} + } + + current.Spec.IngressOptions.AllowedHostnames.Regex = invalidRegex + + err := k8sClient.Update(context.TODO(), current) + if err == nil { + return fmt.Errorf("expected tenant update with invalid allowed hostname regex %q to be denied", invalidRegex) + } + + return nil + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + }) + }) diff --git a/e2e/tenant_forbidden_annotations_regex_test.go b/e2e/tenant_forbidden_annotations_regex_test.go index 29fbe9ba..de41b4db 100644 --- a/e2e/tenant_forbidden_annotations_regex_test.go +++ b/e2e/tenant_forbidden_annotations_regex_test.go @@ -10,6 +10,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" "github.com/projectcapsule/capsule/pkg/api" @@ -21,6 +22,7 @@ var _ = Describe("creating a tenant with various forbidden regexes", Ordered, La "", "(.*gitops|.*nsm)", } + for i, annotationValue := range successRegexes { It("should succeed using a valid regex on the annotation", func() { tnt := &capsulev1beta2.Tenant{ @@ -52,6 +54,7 @@ var _ = Describe("creating a tenant with various forbidden regexes", Ordered, La Regex: annotationValue, }, } + return k8sClient.Create(context.TODO(), tnt) }).Should(Succeed()) EventuallyDeletion(tnt) @@ -64,9 +67,95 @@ var _ = Describe("creating a tenant with various forbidden regexes", Ordered, La Regex: annotationValue, }, } + return k8sClient.Create(context.TODO(), tnt) }).Should(Succeed()) EventuallyDeletion(tnt) }) } + + failureRegexes := []string{ + "(", + "[", + "*invalid", + "(?P<", + } + + for i, invalidRegex := range failureRegexes { + It("should deny using an invalid regex on forbidden labels", func() { + tnt := &capsulev1beta2.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-namespace-label-regex-invalid-" + strconv.Itoa(i), + Labels: map[string]string{ + "env": "e2e", + }, + }, + Spec: capsulev1beta2.TenantSpec{ + Owners: rbac.OwnerListSpec{ + { + CoreOwnerSpec: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Name: "e2e-namespace-regex", + Kind: "User", + }, + }, + }, + }, + NamespaceOptions: &capsulev1beta2.NamespaceOptions{ + ForbiddenLabels: api.ForbiddenListSpec{ + Regex: invalidRegex, + }, + }, + }, + } + + EventuallyCreation(func() error { + tnt.SetResourceVersion("") + + return k8sClient.Create(context.TODO(), tnt) + }).Should(MatchError(ContainSubstring("unable to compile regex"))) + + Consistently(func() error { + return k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(tnt), &capsulev1beta2.Tenant{}) + }).ShouldNot(Succeed()) + }) + + It("should deny using an invalid regex on forbidden annotations", func() { + tnt := &capsulev1beta2.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-namespace-annotation-regex-invalid-" + strconv.Itoa(i), + Labels: map[string]string{ + "env": "e2e", + }, + }, + Spec: capsulev1beta2.TenantSpec{ + Owners: rbac.OwnerListSpec{ + { + CoreOwnerSpec: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Name: "e2e-namespace-regex", + Kind: "User", + }, + }, + }, + }, + NamespaceOptions: &capsulev1beta2.NamespaceOptions{ + ForbiddenAnnotations: api.ForbiddenListSpec{ + Regex: invalidRegex, + }, + }, + }, + } + + EventuallyCreation(func() error { + tnt.SetResourceVersion("") + + return k8sClient.Create(context.TODO(), tnt) + }).Should(MatchError(ContainSubstring("unable to compile regex"))) + + Consistently(func() error { + return k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(tnt), &capsulev1beta2.Tenant{}) + }).ShouldNot(Succeed()) + }) + } }) diff --git a/internal/webhook/tenant/validation/forbidden_annotations_regex.go b/internal/webhook/tenant/validation/forbidden_annotations_regex.go index 33a71a2f..5f4e512a 100644 --- a/internal/webhook/tenant/validation/forbidden_annotations_regex.go +++ b/internal/webhook/tenant/validation/forbidden_annotations_regex.go @@ -68,18 +68,27 @@ func (h *forbiddenAnnotationsRegexHandler) OnUpdate( } func (h *forbiddenAnnotationsRegexHandler) validate(tnt *capsulev1beta2.Tenant, req admission.Request) *admission.Response { - if tnt.Spec.NamespaceOptions == nil { + if tnt == nil || tnt.Spec.NamespaceOptions == nil { return nil } - annotationsToCheck := map[string]string{ + regexesToCheck := map[string]string{ "labels": tnt.Spec.NamespaceOptions.ForbiddenLabels.Regex, "annotations": tnt.Spec.NamespaceOptions.ForbiddenAnnotations.Regex, } - for scope, annotation := range annotationsToCheck { - if _, err := regexp.Compile(tnt.Spec.NamespaceOptions.ForbiddenLabels.Regex); err != nil { - return ad.Denyf("unable to compile %s regex for forbidden %s", annotation, scope) + for scope, expression := range regexesToCheck { + if expression == "" { + continue + } + + if _, err := regexp.Compile(expression); err != nil { + return ad.Denyf( + "unable to compile regex %q for forbidden %s: %v", + expression, + scope, + err, + ) } } diff --git a/internal/webhook/tenant/validation/hostname_regex.go b/internal/webhook/tenant/validation/hostname_regex.go index 700830b6..fb916c5f 100644 --- a/internal/webhook/tenant/validation/hostname_regex.go +++ b/internal/webhook/tenant/validation/hostname_regex.go @@ -54,8 +54,8 @@ func (h *hostnameRegexHandler) OnDelete( func (h *hostnameRegexHandler) OnUpdate( _ client.Client, _ client.Reader, - old *capsulev1beta2.Tenant, tnt *capsulev1beta2.Tenant, + _ *capsulev1beta2.Tenant, decoder admission.Decoder, _ events.EventRecorder, ) handlers.Func {