Skip to content

Commit

Permalink
Copy labels and annotations from SubnamespaceAnchor to child namespace
Browse files Browse the repository at this point in the history 
Tested: Added integration tests and tested manually

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
  • Loading branch information
@mishamo
mishamo committed Mar 16, 2022
1 parent 281b24f commit d826577
Show file tree
Hide file tree
Showing 10 changed files with 383 additions and 62 deletions.
15 changes: 15 additions & 0 deletions api/v1alpha2/subnamespace_anchor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 26 additions & 0 deletions api/v1alpha2/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 53 additions & 0 deletions config/crd/bases/hnc.x-k8s.io_subnamespaceanchors.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
118 changes: 118 additions & 0 deletions internal/anchor/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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("")
})

Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 9 additions & 1 deletion internal/anchor/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
70 changes: 70 additions & 0 deletions internal/anchor/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit d826577

Please sign in to comment.