Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

Commit e75b225

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #149 from mishamo/master
Copy labels and annotations from SubnamespaceAnchor to child namespace
2 parents 3d56dcf + b726cbf commit e75b225

File tree

10 files changed

+381
-62
lines changed

10 files changed

+381
-62
lines changed

‎api/v1alpha2/subnamespace_anchor.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,24 @@ type SubnamespaceAnchor struct {
6666
metav1.TypeMeta `json:",inline"`
6767
metav1.ObjectMeta `json:"metadata,omitempty"`
6868

69+
Spec SubnamespaceAnchorSpec `json:"spec,omitempty"`
6970
Status SubnamespaceAnchorStatus `json:"status,omitempty"`
7071
}
7172

73+
type SubnamespaceAnchorSpec struct {
74+
// Labels is a list of labels and values to apply to the current subnamespace and all of its
75+
// descendants. All label keys must match a regex specified on the command line by
76+
// --managed-namespace-label.
77+
// All label keys must be managed labels (see HNC docs) and must match a regex
78+
Labels []MetaKVP `json:"labels,omitempty"`
79+
80+
// Annotations is a list of annotations and values to apply to the current subnamespace and all of
81+
// its descendants. All annotation keys must match a regex specified on the command line by
82+
// --managed-namespace-annotation.
83+
// All annotation keys must be managed annotations (see HNC docs) and must match a regex
84+
Annotations []MetaKVP `json:"annotations,omitempty"`
85+
}
86+
7287
// +kubebuilder:object:root=true
7388

7489
// SubnamespaceAnchorList contains a list of SubnamespaceAnchor.

‎api/v1alpha2/zz_generated.deepcopy.go‎

Lines changed: 26 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,59 @@ spec:
3535
type: string
3636
metadata:
3737
type: object
38+
spec:
39+
properties:
40+
annotations:
41+
description: Annotations is a list of annotations and values to apply
42+
to the current subnamespace and all of its descendants. All annotation
43+
keys must match a regex specified on the command line by --managed-namespace-annotation.
44+
All annotation keys must be managed annotations (see HNC docs) and
45+
must match a regex
46+
items:
47+
description: MetaKVP represents a label or annotation
48+
properties:
49+
key:
50+
description: Key is the name of the label or annotation. It
51+
must conform to the normal rules for Kubernetes label/annotation
52+
keys.
53+
type: string
54+
value:
55+
description: Value is the value of the label or annotation.
56+
It must confirm to the normal rules for Kubernetes label or
57+
annoation values, which are far more restrictive for labels
58+
than for anntations.
59+
type: string
60+
required:
61+
- key
62+
- value
63+
type: object
64+
type: array
65+
labels:
66+
description: Labels is a list of labels and values to apply to the
67+
current subnamespace and all of its descendants. All label keys
68+
must match a regex specified on the command line by --managed-namespace-label.
69+
All label keys must be managed labels (see HNC docs) and must match
70+
a regex
71+
items:
72+
description: MetaKVP represents a label or annotation
73+
properties:
74+
key:
75+
description: Key is the name of the label or annotation. It
76+
must conform to the normal rules for Kubernetes label/annotation
77+
keys.
78+
type: string
79+
value:
80+
description: Value is the value of the label or annotation.
81+
It must confirm to the normal rules for Kubernetes label or
82+
annoation values, which are far more restrictive for labels
83+
than for anntations.
84+
type: string
85+
required:
86+
- key
87+
- value
88+
type: object
89+
type: array
90+
type: object
3891
status:
3992
description: SubnamespaceAnchorStatus defines the observed state of SubnamespaceAnchor.
4093
properties:

‎internal/anchor/reconciler_test.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ var _ = Describe("Anchor", func() {
2626
var (
2727
fooName string
2828
barName string
29+
bazName string
2930
)
3031

3132
BeforeEach(func() {
3233
fooName = CreateNS(ctx, "foo")
3334
barName = CreateNSName("bar")
35+
bazName = CreateNSName("baz")
3436
config.SetNamespaces("")
3537
})
3638

@@ -111,6 +113,122 @@ var _ = Describe("Anchor", func() {
111113
return barHier.Spec.Parent
112114
}).Should(Equal(fooName))
113115
})
116+
117+
It("should propagate managed labels and not unmanaged labels", func() {
118+
Expect(config.SetManagedMeta([]string{"legal-.*"}, nil)).Should(Succeed())
119+
120+
// Set the managed label and confirm that it only exists on one namespace
121+
foo_anchor_bar := newAnchor(barName, fooName)
122+
foo_anchor_bar.Spec.Labels = []api.MetaKVP{
123+
{Key: "legal-label", Value: "val-1"},
124+
{Key: "unpropagated-label", Value: "not-allowed"},
125+
}
126+
updateAnchor(ctx, foo_anchor_bar)
127+
Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("val-1"))
128+
129+
// Add 'baz' as a child and verify the right labels are propagated
130+
bar_anchor_baz := newAnchor(bazName, barName)
131+
updateAnchor(ctx, bar_anchor_baz)
132+
Eventually(HasChild(ctx, barName, bazName)).Should(Equal(true))
133+
Eventually(GetLabel(ctx, bazName, "legal-label")).Should(Equal("val-1"))
134+
135+
// Verify that the bad label isn't propagated and that a condition is set
136+
Eventually(GetLabel(ctx, barName, "unpropagated-label")).Should(Equal(""))
137+
Eventually(GetLabel(ctx, bazName, "unpropagated-label")).Should(Equal(""))
138+
Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel)).Should(Equal(true))
139+
140+
// Remove the bad config and verify that the condition is removed
141+
foo_anchor_bar = getAnchor(ctx, fooName, barName)
142+
foo_anchor_bar.Spec.Labels = foo_anchor_bar.Spec.Labels[0:1]
143+
updateAnchor(ctx, foo_anchor_bar)
144+
Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedLabel)).Should(Equal(false))
145+
146+
// Change the value of the label and verify that it's propagated
147+
foo_anchor_bar = getAnchor(ctx, fooName, barName)
148+
foo_anchor_bar.Spec.Labels[0].Value = "second-value"
149+
updateAnchor(ctx, foo_anchor_bar)
150+
Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value"))
151+
152+
//Remove label from hierarchyconfiguration and verify that the label is NOT removed
153+
barHier := GetHierarchy(ctx, barName)
154+
barHier.Spec.Labels = []api.MetaKVP{}
155+
UpdateHierarchy(ctx, barHier)
156+
Consistently(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value"))
157+
158+
//Remove subnamespace-of annotation from child namespace and verify anchor is in conflict
159+
barNs := GetNamespace(ctx, barName)
160+
delete(barNs.GetAnnotations(), api.SubnamespaceOf)
161+
UpdateNamespace(ctx, barNs)
162+
Eventually(getAnchorState(ctx, fooName, barName)).Should(Equal(api.Conflict))
163+
164+
//Delete parent anchor with labels and verify that label is not removed
165+
DeleteObject(ctx, "subnamespaceanchors", fooName, barName)
166+
Consistently(GetLabel(ctx, barName, "legal-label")).Should(Equal("second-value"))
167+
168+
//Remove label from hierarchyconfiguration and verify that label is removed
169+
barHier = GetHierarchy(ctx, barName)
170+
barHier.Spec.Labels = []api.MetaKVP{}
171+
UpdateHierarchy(ctx, barHier)
172+
Eventually(GetLabel(ctx, barName, "legal-label")).Should(Equal(""))
173+
})
174+
175+
It("should propagate managed annotations and not unmanaged annotations", func() {
176+
Expect(config.SetManagedMeta(nil, []string{"legal-.*"})).Should(Succeed())
177+
178+
// Set the managed annotation and confirm that it only exists on one namespace
179+
foo_anchor_bar := newAnchor(barName, fooName)
180+
foo_anchor_bar.Spec.Annotations = []api.MetaKVP{
181+
{Key: "legal-annotation", Value: "val-1"},
182+
{Key: "unpropagated-annotation", Value: "not-allowed"},
183+
}
184+
updateAnchor(ctx, foo_anchor_bar)
185+
Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("val-1"))
186+
187+
// Add 'baz' as a child and verify the right annotations are propagated
188+
bar_anchor_baz := newAnchor(bazName, barName)
189+
updateAnchor(ctx, bar_anchor_baz)
190+
Eventually(HasChild(ctx, barName, bazName)).Should(Equal(true))
191+
Eventually(GetAnnotation(ctx, bazName, "legal-annotation")).Should(Equal("val-1"))
192+
193+
// Verify that the bad annotation isn't propagated and that a condition is set
194+
Eventually(GetAnnotation(ctx, barName, "unpropagated-annotation")).Should(Equal(""))
195+
Eventually(GetAnnotation(ctx, bazName, "unpropagated-annotation")).Should(Equal(""))
196+
Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation)).Should(Equal(true))
197+
198+
// Remove the bad config and verify that the condition is removed
199+
foo_anchor_bar = getAnchor(ctx, fooName, barName)
200+
foo_anchor_bar.Spec.Annotations = foo_anchor_bar.Spec.Annotations[0:1]
201+
updateAnchor(ctx, foo_anchor_bar)
202+
Eventually(HasCondition(ctx, barName, api.ConditionBadConfiguration, api.ReasonIllegalManagedAnnotation)).Should(Equal(false))
203+
204+
// Change the value of the annotation and verify that it's propagated
205+
foo_anchor_bar = getAnchor(ctx, fooName, barName)
206+
foo_anchor_bar.Spec.Annotations[0].Value = "second-value"
207+
updateAnchor(ctx, foo_anchor_bar)
208+
Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value"))
209+
210+
//Remove annotation from hierarchyconfiguration and verify that the annotation is NOT removed
211+
barHier := GetHierarchy(ctx, barName)
212+
barHier.Spec.Annotations = []api.MetaKVP{}
213+
UpdateHierarchy(ctx, barHier)
214+
Consistently(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value"))
215+
216+
//Remove subnamespace-of annotation from child namespace and verify anchor is in conflict
217+
barNs := GetNamespace(ctx, barName)
218+
delete(barNs.GetAnnotations(), api.SubnamespaceOf)
219+
UpdateNamespace(ctx, barNs)
220+
Eventually(getAnchorState(ctx, fooName, barName)).Should(Equal(api.Conflict))
221+
222+
//Delete parent anchor with annotations and verify that annotation is not removed
223+
DeleteObject(ctx, "subnamespaceanchors", fooName, barName)
224+
Consistently(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal("second-value"))
225+
226+
//Remove label from hierarchyconfiguration and verify that annotation is removed
227+
barHier = GetHierarchy(ctx, barName)
228+
barHier.Spec.Annotations = []api.MetaKVP{}
229+
UpdateHierarchy(ctx, barHier)
230+
Eventually(GetAnnotation(ctx, barName, "legal-annotation")).Should(Equal(""))
231+
})
114232
})
115233

116234
func getAnchorState(ctx context.Context, pnm, nm string) func() api.SubnamespaceAnchorState {

‎internal/anchor/validator.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ func (v *Validator) Handle(ctx context.Context, req admission.Request) admission
7171
func (v *Validator) handle(req *anchorRequest) admission.Response {
7272
v.Forest.Lock()
7373
defer v.Forest.Unlock()
74-
7574
pnm := req.anchor.Namespace
7675
cnm := req.anchor.Name
7776
cns := v.Forest.Get(cnm)
@@ -86,6 +85,13 @@ func (v *Validator) handle(req *anchorRequest) admission.Response {
8685
return webhooks.DenyInvalid(api.SubnamespaceAnchorGK, cnm, allErrs)
8786
}
8887

88+
labelErrs := config.ValidateManagedLabels(req.anchor.Spec.Labels)
89+
annotationErrs := config.ValidateManagedAnnotations(req.anchor.Spec.Annotations)
90+
allErrs := append(labelErrs, annotationErrs...)
91+
if len(allErrs) > 0 {
92+
return webhooks.DenyInvalid(api.SubnamespaceAnchorGK, req.anchor.Name, allErrs)
93+
}
94+
8995
switch req.op {
9096
case k8sadm.Create:
9197
// Can't create subnamespaces in unmanaged namespaces

‎internal/anchor/validator_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,76 @@ func TestCreateSubnamespaces(t *testing.T) {
5555
}
5656
}
5757

58+
func TestManagedMeta(t *testing.T) {
59+
f := foresttest.Create("-") // a
60+
v := &Validator{Forest: f}
61+
config.SetNamespaces("", "kube-system")
62+
// For this test we accept any label or annotation not starting with 'h',
63+
// to allow almost any meta - except the hnc.x-k8s.io labels/annotations,
64+
// which cannot be managed anyway. And allows us to use that for testing.
65+
if err := config.SetManagedMeta([]string{"[^h].*"}, []string{"[^h].*"}); err != nil {
66+
t.Fatal(err)
67+
}
68+
defer config.SetManagedMeta(nil, nil)
69+
70+
tests := []struct {
71+
name string
72+
labels []api.MetaKVP
73+
annotations []api.MetaKVP
74+
allowed bool
75+
}{
76+
{name: "ok: managed label", labels: []api.MetaKVP{{Key: "label.com/team"}}, allowed: true},
77+
{name: "invalid: unmanaged label", labels: []api.MetaKVP{{Key: api.LabelIncludedNamespace}}},
78+
{name: "ok: managed annotation", annotations: []api.MetaKVP{{Key: "annot.com/log-index"}}, allowed: true},
79+
{name: "invalid: unmanaged annotation", annotations: []api.MetaKVP{{Key: api.AnnotationManagedBy}}},
80+
81+
{name: "ok: prefixed label key", labels: []api.MetaKVP{{Key: "foo.bar/team", Value: "v"}}, allowed: true},
82+
{name: "ok: bare label key", labels: []api.MetaKVP{{Key: "team", Value: "v"}}, allowed: true},
83+
{name: "invalid: label prefix key", labels: []api.MetaKVP{{Key: "foo;bar/team", Value: "v"}}},
84+
{name: "invalid: label name key", labels: []api.MetaKVP{{Key: "foo.bar/-team", Value: "v"}}},
85+
{name: "invalid: empty label key", labels: []api.MetaKVP{{Key: "", Value: "v"}}},
86+
87+
{name: "ok: label value", labels: []api.MetaKVP{{Key: "k", Value: "foo"}}, allowed: true},
88+
{name: "ok: empty label value", labels: []api.MetaKVP{{Key: "k", Value: ""}}, allowed: true},
89+
{name: "ok: label value special char", labels: []api.MetaKVP{{Key: "k", Value: "f-oo"}}, allowed: true},
90+
{name: "invalid: label value", labels: []api.MetaKVP{{Key: "k", Value: "-foo"}}},
91+
92+
{name: "ok: prefixed annotation key", annotations: []api.MetaKVP{{Key: "foo.bar/team", Value: "v"}}, allowed: true},
93+
{name: "ok: bare annotation key", annotations: []api.MetaKVP{{Key: "team", Value: "v"}}, allowed: true},
94+
{name: "invalid: annotation prefix key", annotations: []api.MetaKVP{{Key: "foo;bar/team", Value: "v"}}},
95+
{name: "invalid: annotation name key", annotations: []api.MetaKVP{{Key: "foo.bar/-team", Value: "v"}}},
96+
{name: "invalid: empty annotation key", annotations: []api.MetaKVP{{Key: "", Value: "v"}}},
97+
98+
{name: "ok: annotation value", annotations: []api.MetaKVP{{Key: "k", Value: "foo"}}, allowed: true},
99+
{name: "ok: empty annotation value", annotations: []api.MetaKVP{{Key: "k", Value: ""}}, allowed: true},
100+
{name: "ok: special annotation value", annotations: []api.MetaKVP{{Key: "k", Value: ";$+:;/*'\""}}, allowed: true},
101+
}
102+
for _, tc := range tests {
103+
t.Run(tc.name, func(t *testing.T) {
104+
// Setup
105+
g := NewWithT(t)
106+
107+
anchor := &api.SubnamespaceAnchor{}
108+
anchor.ObjectMeta.Namespace = "a"
109+
anchor.ObjectMeta.Name = "brumpf"
110+
anchor.Spec.Labels = tc.labels
111+
anchor.Spec.Annotations = tc.annotations
112+
113+
req := &anchorRequest{
114+
anchor: anchor,
115+
op: k8sadm.Create,
116+
}
117+
118+
// Test
119+
got := v.handle(req)
120+
121+
// Report
122+
logResult(t, got.AdmissionResponse.Result)
123+
g.Expect(got.AdmissionResponse.Allowed).Should(Equal(tc.allowed))
124+
})
125+
}
126+
}
127+
58128
func TestAllowCascadingDeleteSubnamespaces(t *testing.T) {
59129
// Create a chain of namespaces from "a" to "e", with "a" as the root. Among them,
60130
// "b", "d" and "e" are subnamespaces. This is set up in a long chain to test that

0 commit comments

Comments
 (0)