Copy labels and annotations from SubnamespaceAnchor to child namespace

Added new struct

Fixes

changes

Added validation for anchor labels/annotations

Refactor

Add broken test for label propagation

Fix typo

Add key and value parameter names to test

Add listening rule to update subnamespace represented by anchor

Add extra steps to label and annotation test cases

api/v1alpha2/subnamespace_anchor.go

@@ -68,9 +68,22 @@ type SubnamespaceAnchor struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
+	Spec   SubnamespaceAnchorSpec   `json:"spec,omitempty"`
 	Status SubnamespaceAnchorStatus `json:"status,omitempty"`
 }
 
+type SubnamespaceAnchorSpec struct {
+	// Lables is a list of labels and values to apply to the current namespace and all of its
+	// descendants. All label keys must match a regex specified on the command line by
+	// --managed-namespace-label.
+	Labels []MetaKVP `json:"labels,omitempty"`
+
+	// Annotations is a list of annotations and values to apply to the current namespace and all of
+	// its descendants. All annotation keys must match a regex specified on the command line by
+	// --managed-namespace-annotation.
+	Annotations []MetaKVP `json:"annotations,omitempty"`
+}
+
 // +kubebuilder:object:root=true
 
 // SubnamespaceAnchorList contains a list of SubnamespaceAnchor.

config/crd/bases/hnc.x-k8s.io_subnamespaceanchors.yaml

@@ -35,6 +35,55 @@ spec:
             type: string
           metadata:
             type: object
+          spec:
+            properties:
+              annotations:
+                description: Annotations is a list of annotations and values to apply
+                  to the current namespace and all of its descendants. All annotation
+                  keys must match a regex specified on the command line by --managed-namespace-annotation.
+                items:
+                  description: MetaKVP represents a label or annotation
+                  properties:
+                    key:
+                      description: Key is the name of the label or annotation. It
+                        must conform to the normal rules for Kubernetes label/annotation
+                        keys.
+                      type: string
+                    value:
+                      description: Value is the value of the label or annotation.
+                        It must confirm to the normal rules for Kubernetes label or
+                        annoation values, which are far more restrictive for labels
+                        than for anntations.
+                      type: string
+                  required:
+                  - key
+                  - value
+                  type: object
+                type: array
+              labels:
+                description: Lables is a list of labels and values to apply to the
+                  current namespace and all of its descendants. All label keys must
+                  match a regex specified on the command line by --managed-namespace-label.
+                items:
+                  description: MetaKVP represents a label or annotation
+                  properties:
+                    key:
+                      description: Key is the name of the label or annotation. It
+                        must conform to the normal rules for Kubernetes label/annotation
+                        keys.
+                      type: string
+                    value:
+                      description: Value is the value of the label or annotation.
+                        It must confirm to the normal rules for Kubernetes label or
+                        annoation values, which are far more restrictive for labels
+                        than for anntations.
+                      type: string
+                  required:
+                  - key
+                  - value
+                  type: object
+                type: array
+            type: object
           status:
             description: SubnamespaceAnchorStatus defines the observed state of SubnamespaceAnchor.
             properties:

internal/anchor/reconciler.go

@@ -380,6 +380,7 @@ func (r *Reconciler) getNamespace(ctx context.Context, nm string) (*corev1.Names
 func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, nm, pnm string) error {
 	inst := &corev1.Namespace{}
 	inst.ObjectMeta.Name = nm
+
 	metadata.SetAnnotation(inst, api.SubnamespaceOf, pnm)
 
 	// It's safe to use create here since if the namespace is created by someone

internal/anchor/reconciler_test.go

@@ -2,15 +2,13 @@ package anchor_test
 
 import (
 	"context"
-	"testing"
-
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/types"
-
 	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
 	"sigs.k8s.io/hierarchical-namespaces/internal/config"
 	. "sigs.k8s.io/hierarchical-namespaces/internal/integtest"
+	"testing"
 )
 
 func TestInteg(t *testing.T) {
@@ -26,11 +24,13 @@ var _ = Describe("Anchor", func() {
 	var (
 		fooName string
 		barName string
+		bazName string
 	)
 
 	BeforeEach(func() {
 		fooName = CreateNS(ctx, "foo")
 		barName = CreateNSName("bar")
+		bazName = CreateNSName("baz")
 		config.SetNamespaces("")
 	})
 
@@ -111,6 +111,122 @@ var _ = Describe("Anchor", func() {
 			return barHier.Spec.Parent
 		}).Should(Equal(fooName))
 	})
+
+	It("should propagate managed labels and not unmanaged labels", func() {
+		Expect(config.SetManagedMeta([]string{"legal-.*"}, nil)).Should(Succeed())
+
+		// Set the managed label and confirm that it only exists on one namespace
+		foo_anchor_bar := newAnchor(barName, fooName)
+		foo_anchor_bar.Spec.Labels = []api.MetaKVP{
+			{Key: "legal-label", Value: "val-1"},
+			{Key: "unpropagated-label", Value: "not-allowed"},
+		}
+		updateAnchor(ctx, foo_anchor_bar)
+		Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("val-1"))
+
+		// Add 'baz' as a child and verify the right labels are propagated
+		bar_anchor_baz := newAnchor(bazName, barName)
+		updateAnchor(ctx, bar_anchor_baz)
+		Eventually(HasChild(ctx, barName, bazName)).Should(Equal(true))
+		Eventually(GetLabel(ctx, bazName, "legal-label")).Should(Equal("val-1"))
+
+		// Verify that the bad label isn't propagated and that a condition is set
+		Eventually(GetLabel(ctx, barName, "unpropagated-label")).Should(Equal(""))
+		Eventually(GetLabel(ctx, bazName, "unpropagated-label")).Should(Equal(""))
+		Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel)).Should(Equal(true))
+
+		// Remove the bad config and verify that the condition is removed
+		foo_anchor_bar = getAnchor(ctx, fooName, barName)
+		foo_anchor_bar.Spec.Labels = foo_anchor_bar.Spec.Labels[0:1]
+		updateAnchor(ctx, foo_anchor_bar)
+		Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel)).Should(Equal(false))
+
+		// Change the value of the label and verify that it's propagated
+		foo_anchor_bar = getAnchor(ctx, fooName, barName)
+		foo_anchor_bar.Spec.Labels[0].Value = "second-value"
+		updateAnchor(ctx, foo_anchor_bar)
+		Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value"))
+
+		//Remove label from hierarchyconfiguration and verify that the label is NOT removed
+		barHier := GetHierarchy(ctx, barName)
+		barHier.Spec.Labels = []api.MetaKVP{}
+		UpdateHierarchy(ctx, barHier)
+		Consistently(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value"))
+
+		//Remove subnamespace-of annotation from child namespace and verify anchor is in conflict
+		barNs := GetNamespace(ctx, barName)
+		delete(barNs.GetAnnotations(), api.SubnamespaceOf)
+		UpdateNamespace(ctx, barNs)
+		Eventually(getAnchorState(ctx, fooName, barName)).Should(Equal(api.Conflict))
+
+		//Delete parent anchor with labels and verify that label is not removed
+		DeleteObject(ctx, "subnamespaceanchors", fooName, barName)
+		Consistently(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value"))
+
+		//Remove label from hierarchyconfiguration and verify that label is removed
+		barHier = GetHierarchy(ctx, barName)
+		barHier.Spec.Labels = []api.MetaKVP{}
+		UpdateHierarchy(ctx, barHier)
+		Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal(""))
+	})
+
+	It("should propagate managed annotations and not unmanaged annotations", func() {
+		Expect(config.SetManagedMeta(nil, []string{"legal-.*"})).Should(Succeed())
+
+		// Set the managed annotation and confirm that it only exists on one namespace
+		foo_anchor_bar := newAnchor(barName, fooName)
+		foo_anchor_bar.Spec.Annotations = []api.MetaKVP{
+			{Key: "legal-annotation", Value: "val-1"},
+			{Key: "unpropagated-annotation", Value: "not-allowed"},
+		}
+		updateAnchor(ctx, foo_anchor_bar)
+		Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("val-1"))
+
+		// Add 'baz' as a child and verify the right annotations are propagated
+		bar_anchor_baz := newAnchor(bazName, barName)
+		updateAnchor(ctx, bar_anchor_baz)
+		Eventually(HasChild(ctx, barName, bazName)).Should(Equal(true))
+		Eventually(GetAnnotation(ctx, bazName, "legal-annotation")).Should(Equal("val-1"))
+
+		// Verify that the bad annotation isn't propagated and that a condition is set
+		Eventually(GetAnnotation(ctx, barName, "unpropagated-annotation")).Should(Equal(""))
+		Eventually(GetAnnotation(ctx, bazName, "unpropagated-annotation")).Should(Equal(""))
+		Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation)).Should(Equal(true))
+
+		// Remove the bad config and verify that the condition is removed
+		foo_anchor_bar = getAnchor(ctx, fooName, barName)
+		foo_anchor_bar.Spec.Annotations = foo_anchor_bar.Spec.Annotations[0:1]
+		updateAnchor(ctx, foo_anchor_bar)
+		Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation)).Should(Equal(false))
+
+		// Change the value of the annotation and verify that it's propagated
+		foo_anchor_bar = getAnchor(ctx, fooName, barName)
+		foo_anchor_bar.Spec.Annotations[0].Value = "second-value"
+		updateAnchor(ctx, foo_anchor_bar)
+		Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value"))
+
+		//Remove annotation from hierarchyconfiguration and verify that the annotation is NOT removed
+		barHier := GetHierarchy(ctx, barName)
+		barHier.Spec.Annotations = []api.MetaKVP{}
+		UpdateHierarchy(ctx, barHier)
+		Consistently(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value"))
+
+		//Remove subnamespace-of annotation from child namespace and verify anchor is in conflict
+		barNs := GetNamespace(ctx, barName)
+		delete(barNs.GetAnnotations(), api.SubnamespaceOf)
+		UpdateNamespace(ctx, barNs)
+		Eventually(getAnchorState(ctx, fooName, barName)).Should(Equal(api.Conflict))
+
+		//Delete parent anchor with annotations and verify that annotation is not removed
+		DeleteObject(ctx, "subnamespaceanchors", fooName, barName)
+		Consistently(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value"))
+
+		//Remove label from hierarchyconfiguration and verify that annotation is removed
+		barHier = GetHierarchy(ctx, barName)
+		barHier.Spec.Annotations = []api.MetaKVP{}
+		UpdateHierarchy(ctx, barHier)
+		Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal(""))
+	})
 })
 
 func getAnchorState(ctx context.Context, pnm, nm string) func() api.SubnamespaceAnchorState {

internal/anchor/validator.go

@@ -77,7 +77,6 @@ func (v *Validator) Handle(ctx context.Context, req admission.Request) admission
 func (v *Validator) handle(req *anchorRequest) admission.Response {
 	v.Forest.Lock()
 	defer v.Forest.Unlock()
-
 	pnm := req.anchor.Namespace
 	cnm := req.anchor.Name
 	cns := v.Forest.Get(cnm)
@@ -95,6 +94,15 @@ func (v *Validator) handle(req *anchorRequest) admission.Response {
 		return webhooks.DenyFromAPIError(err)
 	}
 
+	labelErrs := forest.ValidateManagedLabels(req.anchor.Spec.Labels)
+	annotationErrs := forest.ValidateManagedAnnotations(req.anchor.Spec.Annotations)
+	allErrs := append(labelErrs, annotationErrs...)
+	if len(allErrs) > 0 {
+		gk := schema.GroupKind{Group: api.GroupVersion.Group, Kind: "SubnamespaceAnchor"}
+		err := apierrors.NewInvalid(gk, req.anchor.Name, allErrs)
+		return webhooks.DenyFromAPIError(err)
+	}
+
 	switch req.op {
 	case k8sadm.Create:
 		// Can't create subnamespaces in unmanaged namespaces

internal/anchor/validator_test.go

@@ -55,6 +55,76 @@ func TestCreateSubnamespaces(t *testing.T) {
 	}
 }
 
+func TestManagedMeta(t *testing.T) {
+	f := foresttest.Create("-") // a
+	v := &Validator{Forest: f}
+	config.SetNamespaces("", "kube-system")
+	// For this test we accept any label or annotation not starting with 'h',
+	// to allow almost any meta - except the hnc.x-k8s.io labels/annotations,
+	// which cannot be managed anyway. And allows us to use that for testing.
+	if err := config.SetManagedMeta([]string{"[^h].*"}, []string{"[^h].*"}); err != nil {
+		t.Fatal(err)
+	}
+	defer config.SetManagedMeta(nil, nil)
+
+	tests := []struct {
+		name        string
+		labels      []api.MetaKVP
+		annotations []api.MetaKVP
+		allowed     bool
+	}{
+		{name: "ok: managed label", labels: []api.MetaKVP{{Key: "label.com/team"}}, allowed: true},
+		{name: "invalid: unmanaged label", labels: []api.MetaKVP{{Key: api.LabelIncludedNamespace}}},
+		{name: "ok: managed annotation", annotations: []api.MetaKVP{{Key: "annot.com/log-index"}}, allowed: true},
+		{name: "invalid: unmanaged annotation", annotations: []api.MetaKVP{{Key: api.AnnotationManagedBy}}},
+
+		{name: "ok: prefixed label key", labels: []api.MetaKVP{{Key: "foo.bar/team", Value: "v"}}, allowed: true},
+		{name: "ok: bare label key", labels: []api.MetaKVP{{Key: "team", Value: "v"}}, allowed: true},
+		{name: "invalid: label prefix key", labels: []api.MetaKVP{{Key: "foo;bar/team", Value: "v"}}},
+		{name: "invalid: label name key", labels: []api.MetaKVP{{Key: "foo.bar/-team", Value: "v"}}},
+		{name: "invalid: empty label key", labels: []api.MetaKVP{{Key: "", Value: "v"}}},
+
+		{name: "ok: label value", labels: []api.MetaKVP{{Key: "k", Value: "foo"}}, allowed: true},
+		{name: "ok: empty label value", labels: []api.MetaKVP{{Key: "k", Value: ""}}, allowed: true},
+		{name: "ok: label value special char", labels: []api.MetaKVP{{Key: "k", Value: "f-oo"}}, allowed: true},
+		{name: "invalid: label value", labels: []api.MetaKVP{{Key: "k", Value: "-foo"}}},
+
+		{name: "ok: prefixed annotation key", annotations: []api.MetaKVP{{Key: "foo.bar/team", Value: "v"}}, allowed: true},
+		{name: "ok: bare annotation key", annotations: []api.MetaKVP{{Key: "team", Value: "v"}}, allowed: true},
+		{name: "invalid: annotation prefix key", annotations: []api.MetaKVP{{Key: "foo;bar/team", Value: "v"}}},
+		{name: "invalid: annotation name key", annotations: []api.MetaKVP{{Key: "foo.bar/-team", Value: "v"}}},
+		{name: "invalid: empty annotation key", annotations: []api.MetaKVP{{Key: "", Value: "v"}}},
+
+		{name: "ok: annotation value", annotations: []api.MetaKVP{{Key: "k", Value: "foo"}}, allowed: true},
+		{name: "ok: empty annotation value", annotations: []api.MetaKVP{{Key: "k", Value: ""}}, allowed: true},
+		{name: "ok: special annotation value", annotations: []api.MetaKVP{{Key: "k", Value: ";$+:;/*'\""}}, allowed: true},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Setup
+			g := NewWithT(t)
+
+			anchor := &api.SubnamespaceAnchor{}
+			anchor.ObjectMeta.Namespace = "a"
+			anchor.ObjectMeta.Name = "brumpf"
+			anchor.Spec.Labels = tc.labels
+			anchor.Spec.Annotations = tc.annotations
+
+			req := &anchorRequest{
+				anchor: anchor,
+				op:     k8sadm.Create,
+			}
+
+			// Test
+			got := v.handle(req)
+
+			// Report
+			logResult(t, got.AdmissionResponse.Result)
+			g.Expect(got.AdmissionResponse.Allowed).Should(Equal(tc.allowed))
+		})
+	}
+}
+
 func TestAllowCascadingDeleteSubnamespaces(t *testing.T) {
 	// Create a chain of namespaces from "a" to "e", with "a" as the root. Among them,
 	// "b", "d" and "e" are subnamespaces. This is set up in a long chain to test that