From a6927c57777e3bb30215df902e44fbfe6ec1d09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=A4hler?= <26610571+oliverbaehler@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:29:18 +0200 Subject: [PATCH] fix: allow managed metadata defined per tenant (#1947) * fix: allow managed metadata defined per tenant Signed-off-by: Oliver Baehler * fix: allow managed metadata defined per tenant Signed-off-by: Oliver Baehler --------- Signed-off-by: Oliver Baehler --- Makefile | 3 +- e2e/namespace_metadata_forbidden_test.go | 245 +++++++++++++++++- internal/controllers/tenant/limitranges.go | 5 +- internal/controllers/tenant/manager.go | 3 +- .../controllers/tenant/networkpolicies.go | 3 + internal/controllers/tenant/status.go | 1 + .../namespace/validation/user_metadata.go | 131 +++++++--- pkg/template/fast.go | 22 +- 8 files changed, 365 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index 4ad24158..040d59c3 100644 --- a/Makefile +++ b/Makefile @@ -185,8 +185,9 @@ dev-setup: --namespace capsule-system \ --create-namespace \ --version=$(CHART_VERSION) \ - --set 'proxy.enabled=true' \ + --set 'proxy.enabled=false' \ --set 'proxy.certManager.generateCertificates=false' \ + --set 'proxy.options.generateCertificates=true' \ --set 'crds.install=true' \ --set 'crds.exclusive=true'\ --set 'crds.createConfig=true'\ diff --git a/e2e/namespace_metadata_forbidden_test.go b/e2e/namespace_metadata_forbidden_test.go index 4d143189..1dbf503b 100644 --- a/e2e/namespace_metadata_forbidden_test.go +++ b/e2e/namespace_metadata_forbidden_test.go @@ -31,11 +31,22 @@ var _ = Describe("creating a Namespace with user-specified labels and annotation NamespaceOptions: &capsulev1beta2.NamespaceOptions{ ForbiddenLabels: api.ForbiddenListSpec{ Exact: []string{"foo", "bar"}, - Regex: "^gatsby-.*$", + Regex: "^gatsby-.*$|^managed\\.projectcapsule\\.dev/", }, ForbiddenAnnotations: api.ForbiddenListSpec{ Exact: []string{"foo", "bar"}, - Regex: "^gatsby-.*$", + Regex: "^gatsby-.*$|^managed\\.projectcapsule\\.dev/", + }, + AdditionalMetadataList: []api.AdditionalMetadataSelectorSpec{ + { + Labels: map[string]string{ + "managed.projectcapsule.dev/customer": "acme", + "managed.projectcapsule.dev/tenant": "{{ tenant.name }}", + }, + Annotations: map[string]string{ + "managed.projectcapsule.dev/source": "capsule", + }, + }, }, }, Owners: rbac.OwnerListSpec{ @@ -54,10 +65,13 @@ var _ = Describe("creating a Namespace with user-specified labels and annotation JustBeforeEach(func() { EventuallyCreation(func() error { tnt.ResourceVersion = "" + return k8sClient.Create(context.TODO(), tnt) }).Should(Succeed()) + TenantReady(tnt, metav1.ConditionTrue, defaultTimeoutInterval) }) + JustAfterEach(func() { EventuallyDeletion(tnt) }) @@ -68,18 +82,37 @@ var _ = Describe("creating a Namespace with user-specified labels and annotation "bim": "baz", meta.TenantLabel: tnt.GetName(), }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) - }) + By("specifying non-forbidden annotations", func() { ns := NewNamespace("", map[string]string{ meta.TenantLabel: tnt.GetName(), }) ns.SetAnnotations(map[string]string{"bim": "baz"}) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + }) + + By("allowing forbidden-prefix labels and annotations when they are managed by Capsule", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(ns.GetLabels()).To(HaveKeyWithValue("managed.projectcapsule.dev/customer", "acme")) + g.Expect(ns.GetLabels()).To(HaveKeyWithValue("managed.projectcapsule.dev/tenant", tnt.GetName())) + g.Expect(ns.GetAnnotations()).To(HaveKeyWithValue("managed.projectcapsule.dev/source", "capsule")) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) }) }) @@ -89,35 +122,61 @@ var _ = Describe("creating a Namespace with user-specified labels and annotation "foo": "bar", meta.TenantLabel: tnt.GetName(), }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).ShouldNot(Succeed()) NamespaceIsNotPartOfTenant(tnt, ns).Should(Succeed()) - }) + By("specifying forbidden labels using regex match", func() { ns := NewNamespace("", map[string]string{ meta.TenantLabel: tnt.GetName(), "gatsby-foo": "bar", }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).ShouldNot(Succeed()) NamespaceIsNotPartOfTenant(tnt, ns).Should(Succeed()) }) + By("specifying forbidden annotations using exact match", func() { ns := NewNamespace("", map[string]string{ meta.TenantLabel: tnt.GetName(), }) ns.SetAnnotations(map[string]string{"foo": "bar"}) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).ShouldNot(Succeed()) NamespaceIsNotPartOfTenant(tnt, ns).Should(Succeed()) - }) + By("specifying forbidden annotations using regex match", func() { ns := NewNamespace("", map[string]string{ meta.TenantLabel: tnt.GetName(), }) ns.SetAnnotations(map[string]string{"gatsby-foo": "bar"}) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).ShouldNot(Succeed()) NamespaceIsNotPartOfTenant(tnt, ns).Should(Succeed()) + }) + By("specifying forbidden-prefix labels that are not managed by Capsule", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + "managed.projectcapsule.dev/user-label": "should-be-denied", + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).ShouldNot(Succeed()) + NamespaceIsNotPartOfTenant(tnt, ns).Should(Succeed()) + }) + + By("specifying forbidden-prefix annotations that are not managed by Capsule", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + ns.SetAnnotations(map[string]string{ + "managed.projectcapsule.dev/user-annotation": "should-be-denied", + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).ShouldNot(Succeed()) + NamespaceIsNotPartOfTenant(tnt, ns).Should(Succeed()) }) }) @@ -169,81 +228,249 @@ var _ = Describe("creating a Namespace with user-specified labels and annotation ns := NewNamespace("forbidden-labels-exact-match", map[string]string{ meta.TenantLabel: tnt.GetName(), }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) rbacPatch(ns.GetName()) + Consistently(func() error { if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns); err != nil { return nil } - ns.SetLabels(map[string]string{"foo": "bar"}) + labels := ns.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels["foo"] = "bar" + ns.SetLabels(labels) _, err := cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) return err }, 10*time.Second, time.Second).ShouldNot(Succeed()) }) + By("specifying forbidden labels using regex match", func() { ns := NewNamespace("forbidden-labels-regex-match", map[string]string{ meta.TenantLabel: tnt.GetName(), }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) rbacPatch(ns.GetName()) + Consistently(func() error { if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns); err != nil { return nil } - ns.SetLabels(map[string]string{"gatsby-foo": "bar"}) + labels := ns.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels["gatsby-foo"] = "bar" + ns.SetLabels(labels) _, err := cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) return err }, 3*time.Second, time.Second).ShouldNot(Succeed()) }) + By("specifying forbidden annotations using exact match", func() { ns := NewNamespace("forbidden-annotations-exact-match", map[string]string{ meta.TenantLabel: tnt.GetName(), }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) rbacPatch(ns.GetName()) + Consistently(func() error { if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns); err != nil { return nil } - ns.SetAnnotations(map[string]string{"foo": "bar"}) + annotations := ns.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations["foo"] = "bar" + ns.SetAnnotations(annotations) _, err := cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) return err }, 10*time.Second, time.Second).ShouldNot(Succeed()) }) + By("specifying forbidden annotations using regex match", func() { ns := NewNamespace("forbidden-annotations-regex-match", map[string]string{ meta.TenantLabel: tnt.GetName(), }) + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) rbacPatch(ns.GetName()) + Consistently(func() error { if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns); err != nil { return nil } - ns.SetAnnotations(map[string]string{"gatsby-foo": "bar"}) + annotations := ns.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations["gatsby-foo"] = "bar" + ns.SetAnnotations(annotations) _, err := cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) return err }, 10*time.Second, time.Second).ShouldNot(Succeed()) }) + + By("specifying forbidden-prefix labels that are not managed by Capsule", func() { + ns := NewNamespace("forbidden-managed-prefix-label-update", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + rbacPatch(ns.GetName()) + + Consistently(func() error { + if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns); err != nil { + return nil + } + + labels := ns.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels["managed.projectcapsule.dev/user-label"] = "should-be-denied" + ns.SetLabels(labels) + + _, err := cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) + + return err + }, 10*time.Second, time.Second).ShouldNot(Succeed()) + }) + + By("specifying forbidden-prefix annotations that are not managed by Capsule", func() { + ns := NewNamespace("forbidden-managed-prefix-annotation-update", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + rbacPatch(ns.GetName()) + + Consistently(func() error { + if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns); err != nil { + return nil + } + + annotations := ns.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations["managed.projectcapsule.dev/user-annotation"] = "should-be-denied" + ns.SetAnnotations(annotations) + + _, err := cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) + + return err + }, 10*time.Second, time.Second).ShouldNot(Succeed()) + }) + + By("restoring a Capsule-managed forbidden-prefix label value on update", func() { + ns := NewNamespace("forbidden-managed-label-value-update", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ns.GetLabels()).To(HaveKeyWithValue("managed.projectcapsule.dev/customer", "acme")) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) + + rbacPatch(ns.GetName()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + + labels := ns.GetLabels() + if labels == nil { + labels = map[string]string{} + } + + labels["managed.projectcapsule.dev/customer"] = "tampered" + ns.SetLabels(labels) + + _, err = cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(ns.GetLabels()).To(HaveKeyWithValue("managed.projectcapsule.dev/customer", "acme")) + g.Expect(ns.GetLabels()).To(HaveKeyWithValue("managed.projectcapsule.dev/tenant", tnt.GetName())) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) + }) + + By("restoring a Capsule-managed forbidden-prefix annotation value on update", func() { + ns := NewNamespace("forbidden-managed-annotation-value-update", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ns.GetAnnotations()).To(HaveKeyWithValue("managed.projectcapsule.dev/source", "capsule")) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) + + rbacPatch(ns.GetName()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + + annotations := ns.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + + annotations["managed.projectcapsule.dev/source"] = "tampered" + ns.SetAnnotations(annotations) + + _, err = cs.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.GetName()}, ns) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(ns.GetAnnotations()).To(HaveKeyWithValue("managed.projectcapsule.dev/source", "capsule")) + }, defaultTimeoutInterval, time.Second).Should(Succeed()) + }) }) }) diff --git a/internal/controllers/tenant/limitranges.go b/internal/controllers/tenant/limitranges.go index 83739ddf..f94b099b 100644 --- a/internal/controllers/tenant/limitranges.go +++ b/internal/controllers/tenant/limitranges.go @@ -8,12 +8,12 @@ import ( "fmt" "strconv" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "github.com/go-logr/logr" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" "github.com/projectcapsule/capsule/pkg/api/meta" ) @@ -23,8 +23,10 @@ import ( func (r *Manager) syncLimitRanges(ctx context.Context, log logr.Logger, tenant *capsulev1beta2.Tenant) error { // getting requested LimitRange keys + //nolint:staticcheck keys := make([]string, 0, len(tenant.Spec.LimitRanges.Items)) + //nolint:staticcheck for i := range tenant.Spec.LimitRanges.Items { keys = append(keys, strconv.Itoa(i)) } @@ -45,6 +47,7 @@ func (r *Manager) syncLimitRange( return err } + //nolint:staticcheck for i, spec := range tenant.Spec.LimitRanges.Items { target := &corev1.LimitRange{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controllers/tenant/manager.go b/internal/controllers/tenant/manager.go index da77512a..08bb25ca 100644 --- a/internal/controllers/tenant/manager.go +++ b/internal/controllers/tenant/manager.go @@ -342,7 +342,8 @@ func (r *Manager) reconcile(ctx context.Context, log logr.Logger, instance *caps } // Ensuring LimitRange resources - r.Log.V(4).Info("Starting processing of Limit Ranges", "items", len(instance.Spec.LimitRanges.Items)) + //nolint:staticcheck + log.V(4).Info("Starting processing of Limit Ranges", "items", len(instance.Spec.LimitRanges.Items)) if err = r.syncLimitRanges(ctx, log, instance); err != nil { errs = append(errs, fmt.Errorf("cannot sync limitrange items: %w", err)) diff --git a/internal/controllers/tenant/networkpolicies.go b/internal/controllers/tenant/networkpolicies.go index 4fb43492..94bf31cb 100644 --- a/internal/controllers/tenant/networkpolicies.go +++ b/internal/controllers/tenant/networkpolicies.go @@ -23,8 +23,10 @@ import ( // func (r *Manager) syncNetworkPolicies(ctx context.Context, log logr.Logger, tenant *capsulev1beta2.Tenant) error { + //nolint:staticcheck keys := make([]string, 0, len(tenant.Spec.NetworkPolicies.Items)) + //nolint:staticcheck for i := range tenant.Spec.NetworkPolicies.Items { keys = append(keys, strconv.Itoa(i)) } @@ -39,6 +41,7 @@ func (r *Manager) syncNetworkPolicy(ctx context.Context, log logr.Logger, tenant return err } + //nolint:staticcheck for i, spec := range tenant.Spec.NetworkPolicies.Items { target := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controllers/tenant/status.go b/internal/controllers/tenant/status.go index dd6787d1..13d85da0 100644 --- a/internal/controllers/tenant/status.go +++ b/internal/controllers/tenant/status.go @@ -342,6 +342,7 @@ func listObjectNamesBySelector( var regex *regexp.Regexp + //nolint:staticcheck if allowed.Regex != "" { regex, err = regexp.Compile(allowed.Regex) if err != nil { diff --git a/internal/webhook/namespace/validation/user_metadata.go b/internal/webhook/namespace/validation/user_metadata.go index 6786307e..0357079e 100644 --- a/internal/webhook/namespace/validation/user_metadata.go +++ b/internal/webhook/namespace/validation/user_metadata.go @@ -5,6 +5,7 @@ package validation import ( "context" + "maps" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -17,6 +18,7 @@ import ( ad "github.com/projectcapsule/capsule/pkg/runtime/admission" evt "github.com/projectcapsule/capsule/pkg/runtime/events" "github.com/projectcapsule/capsule/pkg/runtime/handlers" + "github.com/projectcapsule/capsule/pkg/tenant" "github.com/projectcapsule/capsule/pkg/users" ) @@ -36,23 +38,14 @@ func (h *userMetadataHandler) OnCreate( tnt *capsulev1beta2.Tenant, ) handlers.Func { return func(ctx context.Context, req admission.Request) *admission.Response { - ns.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Namespace")) - if tnt.Spec.NamespaceOptions != nil { - err := api.ValidateForbidden(ns.Annotations, tnt.Spec.NamespaceOptions.ForbiddenAnnotations) + labels, annotations, err := userMetadataForValidation(ns, nil, tnt) if err != nil { - err = errors.Wrap(err, "namespace annotations validation failed") - recorder.Eventf(ns, ns, corev1.EventTypeWarning, evt.ReasonForbiddenAnnotation, evt.ActionValidationDenied, err.Error()) - - return ad.Deny(err.Error()) + return ad.ErroredResponse(err) } - err = api.ValidateForbidden(ns.Labels, tnt.Spec.NamespaceOptions.ForbiddenLabels) - if err != nil { - err = errors.Wrap(err, "namespace labels validation failed") - recorder.Eventf(ns, ns, corev1.EventTypeWarning, evt.ReasonForbiddenLabel, evt.ActionValidationDenied, err.Error()) - - return ad.Deny(err.Error()) + if response := validateUserMetadata(ns, labels, annotations, tnt.Spec.NamespaceOptions, recorder); response != nil { + return response } } @@ -90,15 +83,8 @@ func (h *userMetadataHandler) OnUpdate( } } - labels, annotations := oldNs.GetLabels(), oldNs.GetAnnotations() - - if labels == nil { - labels = make(map[string]string) - } - - if annotations == nil { - annotations = make(map[string]string) - } + labels := maps.Clone(oldNs.GetLabels()) + annotations := maps.Clone(oldNs.GetAnnotations()) for key, value := range newNs.GetLabels() { v, ok := labels[key] @@ -131,20 +117,13 @@ func (h *userMetadataHandler) OnUpdate( } if tnt.Spec.NamespaceOptions != nil { - err := api.ValidateForbidden(annotations, tnt.Spec.NamespaceOptions.ForbiddenAnnotations) + labels, annotations, err := userMetadataForValidation(newNs, oldNs, tnt) if err != nil { - err = errors.Wrap(err, "namespace annotations validation failed") - recorder.Eventf(oldNs, oldNs, corev1.EventTypeWarning, evt.ReasonForbiddenAnnotation, evt.ActionValidationDenied, err.Error()) - - return ad.Deny(err.Error()) + return ad.ErroredResponse(err) } - err = api.ValidateForbidden(labels, tnt.Spec.NamespaceOptions.ForbiddenLabels) - if err != nil { - err = errors.Wrap(err, "namespace labels validation failed") - recorder.Eventf(oldNs, oldNs, corev1.EventTypeWarning, evt.ReasonForbiddenLabel, evt.ActionValidationDenied, err.Error()) - - return ad.Deny(err.Error()) + if response := validateUserMetadata(oldNs, labels, annotations, tnt.Spec.NamespaceOptions, recorder); response != nil { + return response } } @@ -165,3 +144,89 @@ func (h *userMetadataHandler) OnDelete( return nil } } + +func validateUserMetadata( + ns *corev1.Namespace, + labels map[string]string, + annotations map[string]string, + options *capsulev1beta2.NamespaceOptions, + recorder events.EventRecorder, +) *admission.Response { + err := api.ValidateForbidden(annotations, options.ForbiddenAnnotations) + if err != nil { + err = errors.Wrap(err, "namespace annotations validation failed") + recorder.Eventf(ns, ns, corev1.EventTypeWarning, evt.ReasonForbiddenAnnotation, evt.ActionValidationDenied, err.Error()) + + return ad.Deny(err.Error()) + } + + err = api.ValidateForbidden(labels, options.ForbiddenLabels) + if err != nil { + err = errors.Wrap(err, "namespace labels validation failed") + recorder.Eventf(ns, ns, corev1.EventTypeWarning, evt.ReasonForbiddenLabel, evt.ActionValidationDenied, err.Error()) + + return ad.Deny(err.Error()) + } + + return nil +} + +func userMetadataForValidation( + newNs *corev1.Namespace, + oldNs *corev1.Namespace, + tnt *capsulev1beta2.Tenant, +) (map[string]string, map[string]string, error) { + labels := metadataForValidation(newNs.GetLabels(), nil) + annotations := metadataForValidation(newNs.GetAnnotations(), nil) + + // On update, validate only metadata that was added or changed by the request. + if oldNs != nil { + labels = metadataForValidation(newNs.GetLabels(), oldNs.GetLabels()) + annotations = metadataForValidation(newNs.GetAnnotations(), oldNs.GetAnnotations()) + } + + managedLabels, managedAnnotations, err := tenant.BuildNamespaceMetadataForTenant(newNs, tnt) + if err != nil { + return nil, nil, err + } + + tenant.AddNamespaceNameLabels(managedLabels, newNs) + tenant.AddTenantNameLabel(managedLabels, tnt) + + removeManagedMetadata(labels, managedLabels) + removeManagedMetadata(annotations, managedAnnotations) + + return labels, annotations, nil +} + +func metadataForValidation(newMetadata, oldMetadata map[string]string) map[string]string { + if oldMetadata == nil { + return maps.Clone(newMetadata) + } + + metadata := make(map[string]string) + + for key, newValue := range newMetadata { + oldValue, ok := oldMetadata[key] + if !ok || oldValue != newValue { + metadata[key] = newValue + } + } + + return metadata +} + +func removeManagedMetadata(metadata map[string]string, managed map[string]string) { + for key, managedValue := range managed { + value, ok := metadata[key] + if !ok { + continue + } + + // Only ignore metadata Capsule itself would manage. + // Same key with a different value is still user-controlled and must be validated. + if value == managedValue { + delete(metadata, key) + } + } +} diff --git a/pkg/template/fast.go b/pkg/template/fast.go index dcf6fd33..e5be672f 100644 --- a/pkg/template/fast.go +++ b/pkg/template/fast.go @@ -20,6 +20,17 @@ var AllowedNamespaceMetadataTemplates = sets.New[string]( "namespace", ) +func AllowedNamespaceMetadataTemplatesString() string { + templates := AllowedNamespaceMetadataTemplates.UnsortedList() + slices.Sort(templates) + + for i, tpl := range templates { + templates[i] = fmt.Sprintf("{{%s}}", tpl) + } + + return strings.Join(templates, ", ") +} + var FastTemplateExpression = regexp.MustCompile(`{{\s*([^{}]+)\s*}}`) func ValidateKubernetesStringOrAllowedTemplates( @@ -63,15 +74,16 @@ func validateAllowedTemplatesAndReplace( for _, match := range matches { raw := match[0] - name := strings.TrimSpace(match[1]) + name := FastTemplateNormalize(match[1]) if !AllowedNamespaceMetadataTemplates.Has(name) { return value, []string{ fmt.Sprintf( - "%s: unsupported template %q in %q, allowed templates are {{tenant.name}} and {{namespace}}", + "%s: unsupported template %q in %q, allowed templates are %s", fieldPath, name, value, + AllowedNamespaceMetadataTemplatesString(), ), } } @@ -123,7 +135,7 @@ func FastTemplate( t := fasttemplate.New(template, "{{", "}}") return t.ExecuteFuncString(func(w io.Writer, tag string) (int, error) { - key := strings.TrimSpace(tag) + key := FastTemplateNormalize(tag) if v, ok := templateContext[key]; ok { return w.Write([]byte(v)) } @@ -132,6 +144,10 @@ func FastTemplate( }) } +func FastTemplateNormalize(key string) string { + return strings.TrimSpace(key) +} + // FastTemplateMap applies templating to all values in the provided map in place. func FastTemplateMap( m map[string]string,