Copy labels and annotations from SubnamespaceAnchor to child namespace

Allows managed labels and annotations to be created in the
subnamespaceanchor.spec. See documentation on managed labels and managed
annotations.

Tested: Added integration tests and tested manually

api/v1alpha2/subnamespace_anchor.go

@@ -68,9 +68,24 @@ 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 {
+	// Labels is a list of labels and values to apply to the current subnamespace and all of its
+	// descendants. All label keys must match a regex specified on the command line by
+	// --managed-namespace-label.
+	// All label keys must be managed labels (see HNC docs) and must match a regex
+	Labels []MetaKVP `json:"labels,omitempty"`
+
+	// Annotations is a list of annotations and values to apply to the current subnamespace and all of
+	// its descendants. All annotation keys must match a regex specified on the command line by
+	// --managed-namespace-annotation.
+	// All annotation keys must be managed annotations (see HNC docs) and must match a regex
+	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,59 @@ spec:
             type: string
           metadata:
             type: object
+          spec:
+            properties:
+              annotations:
+                description: Annotations is a list of annotations and values to apply
+                  to the current subnamespace and all of its descendants. All annotation
+                  keys must match a regex specified on the command line by --managed-namespace-annotation.
+                  All annotation keys must be managed annotations (see HNC docs) and
+                  must match a regex
+                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: Labels is a list of labels and values to apply to the
+                  current subnamespace and all of its descendants. All label keys
+                  must match a regex specified on the command line by --managed-namespace-label.
+                  All label keys must be managed labels (see HNC docs) and must match
+                  a regex
+                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_test.go

@@ -26,11 +26,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 +113,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 := config.ValidateManagedLabels(req.anchor.Spec.Labels)
+	annotationErrs := config.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