Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy labels and annotations from SubnamespaceAnchor to child namespace #149

Merged
merged 1 commit into from
Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions api/v1alpha2/subnamespace_anchor.go
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.
mishamo marked this conversation as resolved.
Show resolved Hide resolved
// 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

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
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
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() {
mishamo marked this conversation as resolved.
Show resolved Hide resolved
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
8 changes: 7 additions & 1 deletion internal/anchor/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,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 @@ -86,6 +85,13 @@ func (v *Validator) handle(req *anchorRequest) admission.Response {
return webhooks.DenyInvalid(api.SubnamespaceAnchorGK, cnm, allErrs)
}

labelErrs := config.ValidateManagedLabels(req.anchor.Spec.Labels)
annotationErrs := config.ValidateManagedAnnotations(req.anchor.Spec.Annotations)
allErrs := append(labelErrs, annotationErrs...)
if len(allErrs) > 0 {
return webhooks.DenyInvalid(api.SubnamespaceAnchorGK, req.anchor.Name, allErrs)
}

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
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