Skip to content

Commit 860af35

Browse files
committed
Relax cel and fix tests to satisfy new labels constraints
1 parent 0d848f7 commit 860af35

11 files changed

+56
-65
lines changed

hack/validation/labels.sh

-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
# checking for restricted labels while filtering out well-known labels
55
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.metadata.properties.labels.maxProperties = 100' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
66
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.metadata.properties.labels.x-kubernetes-validations += [
7-
{"message": "label domain \"kubernetes.io\" is restricted", "rule": "self.all(x, x in [\"beta.kubernetes.io/instance-type\", \"failure-domain.beta.kubernetes.io/region\", \"beta.kubernetes.io/os\", \"beta.kubernetes.io/arch\", \"failure-domain.beta.kubernetes.io/zone\", \"topology.kubernetes.io/zone\", \"topology.kubernetes.io/region\", \"kubernetes.io/arch\", \"kubernetes.io/os\", \"node.kubernetes.io/windows-build\"] || x.find(\"^([^/]+)\").endsWith(\"node.kubernetes.io\") || x.find(\"^([^/]+)\").endsWith(\"node-restriction.kubernetes.io\") || !x.find(\"^([^/]+)\").endsWith(\"kubernetes.io\"))"},
8-
{"message": "label domain \"k8s.io\" is restricted", "rule": "self.all(x, x.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !x.find(\"^([^/]+)\").endsWith(\"k8s.io\"))"},
97
{"message": "label domain \"karpenter.sh\" is restricted", "rule": "self.all(x, x in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !x.find(\"^([^/]+)\").endsWith(\"karpenter.sh\"))"},
108
{"message": "label \"karpenter.sh/nodepool\" is restricted", "rule": "self.all(x, x != \"karpenter.sh/nodepool\")"},
119
{"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self.all(x, x != \"kubernetes.io/hostname\")"}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml

hack/validation/requirements.sh

-4
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.req
55
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.pattern = "^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$"' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml
66
## checking for restricted labels while filtering out well-known labels
77
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [
8-
{"message": "label domain \"kubernetes.io\" is restricted", "rule": "self in [\"beta.kubernetes.io/instance-type\", \"failure-domain.beta.kubernetes.io/region\", \"beta.kubernetes.io/os\", \"beta.kubernetes.io/arch\", \"failure-domain.beta.kubernetes.io/zone\", \"topology.kubernetes.io/zone\", \"topology.kubernetes.io/region\", \"node.kubernetes.io/instance-type\", \"kubernetes.io/arch\", \"kubernetes.io/os\", \"node.kubernetes.io/windows-build\"] || self.find(\"^([^/]+)\").endsWith(\"node.kubernetes.io\") || self.find(\"^([^/]+)\").endsWith(\"node-restriction.kubernetes.io\") || !self.find(\"^([^/]+)\").endsWith(\"kubernetes.io\")"},
9-
{"message": "label domain \"k8s.io\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"},
108
{"message": "label domain \"karpenter.sh\" is restricted", "rule": "self in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.sh\")"},
119
{"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self != \"kubernetes.io/hostname\""}]' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml
1210
## operator enum values
@@ -21,8 +19,6 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.tem
2119
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.pattern = "^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$"' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
2220
## checking for restricted labels while filtering out well-known labels
2321
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [
24-
{"message": "label domain \"kubernetes.io\" is restricted", "rule": "self in [\"beta.kubernetes.io/instance-type\", \"failure-domain.beta.kubernetes.io/region\", \"beta.kubernetes.io/os\", \"beta.kubernetes.io/arch\", \"failure-domain.beta.kubernetes.io/zone\", \"topology.kubernetes.io/zone\", \"topology.kubernetes.io/region\", \"node.kubernetes.io/instance-type\", \"kubernetes.io/arch\", \"kubernetes.io/os\", \"node.kubernetes.io/windows-build\"] || self.find(\"^([^/]+)\").endsWith(\"node.kubernetes.io\") || self.find(\"^([^/]+)\").endsWith(\"node-restriction.kubernetes.io\") || !self.find(\"^([^/]+)\").endsWith(\"kubernetes.io\")"},
25-
{"message": "label domain \"k8s.io\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"},
2622
{"message": "label domain \"karpenter.sh\" is restricted", "rule": "self in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.sh\")"},
2723
{"message": "label \"karpenter.sh/nodepool\" is restricted", "rule": "self != \"karpenter.sh/nodepool\""},
2824
{"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self != \"kubernetes.io/hostname\""}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml

kwok/charts/crds/karpenter.sh_nodeclaims.yaml

-4
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,6 @@ spec:
125125
maxLength: 316
126126
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$
127127
x-kubernetes-validations:
128-
- message: label domain "kubernetes.io" is restricted
129-
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
130-
- message: label domain "k8s.io" is restricted
131-
rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io")
132128
- message: label domain "karpenter.sh" is restricted
133129
rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh")
134130
- message: label "kubernetes.io/hostname" is restricted

kwok/charts/crds/karpenter.sh_nodepools.yaml

-8
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,6 @@ spec:
196196
type: object
197197
maxProperties: 100
198198
x-kubernetes-validations:
199-
- message: label domain "kubernetes.io" is restricted
200-
rule: self.all(x, x in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || x.find("^([^/]+)").endsWith("node.kubernetes.io") || x.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !x.find("^([^/]+)").endsWith("kubernetes.io"))
201-
- message: label domain "k8s.io" is restricted
202-
rule: self.all(x, x.find("^([^/]+)").endsWith("kops.k8s.io") || !x.find("^([^/]+)").endsWith("k8s.io"))
203199
- message: label domain "karpenter.sh" is restricted
204200
rule: self.all(x, x in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !x.find("^([^/]+)").endsWith("karpenter.sh"))
205201
- message: label "karpenter.sh/nodepool" is restricted
@@ -267,10 +263,6 @@ spec:
267263
maxLength: 316
268264
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$
269265
x-kubernetes-validations:
270-
- message: label domain "kubernetes.io" is restricted
271-
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
272-
- message: label domain "k8s.io" is restricted
273-
rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io")
274266
- message: label domain "karpenter.sh" is restricted
275267
rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh")
276268
- message: label "karpenter.sh/nodepool" is restricted

pkg/apis/crds/karpenter.sh_nodeclaims.yaml

-4
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,6 @@ spec:
125125
maxLength: 316
126126
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$
127127
x-kubernetes-validations:
128-
- message: label domain "kubernetes.io" is restricted
129-
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
130-
- message: label domain "k8s.io" is restricted
131-
rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io")
132128
- message: label domain "karpenter.sh" is restricted
133129
rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh")
134130
- message: label "kubernetes.io/hostname" is restricted

pkg/apis/crds/karpenter.sh_nodepools.yaml

-8
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,6 @@ spec:
196196
type: object
197197
maxProperties: 100
198198
x-kubernetes-validations:
199-
- message: label domain "kubernetes.io" is restricted
200-
rule: self.all(x, x in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || x.find("^([^/]+)").endsWith("node.kubernetes.io") || x.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !x.find("^([^/]+)").endsWith("kubernetes.io"))
201-
- message: label domain "k8s.io" is restricted
202-
rule: self.all(x, x.find("^([^/]+)").endsWith("kops.k8s.io") || !x.find("^([^/]+)").endsWith("k8s.io"))
203199
- message: label domain "karpenter.sh" is restricted
204200
rule: self.all(x, x in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !x.find("^([^/]+)").endsWith("karpenter.sh"))
205201
- message: label "karpenter.sh/nodepool" is restricted
@@ -267,10 +263,6 @@ spec:
267263
maxLength: 316
268264
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$
269265
x-kubernetes-validations:
270-
- message: label domain "kubernetes.io" is restricted
271-
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
272-
- message: label domain "k8s.io" is restricted
273-
rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io")
274266
- message: label domain "karpenter.sh" is restricted
275267
rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh")
276268
- message: label "karpenter.sh/nodepool" is restricted

pkg/apis/v1/nodeclaim_validation_cel_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ var _ = Describe("Validation", func() {
141141
Expect(env.Client.Create(ctx, nodeClaim)).ToNot(Succeed())
142142
}
143143
})
144-
It("should allow restricted domains exceptions", func() {
144+
It("should allow kubernetes domains", func() {
145145
oldNodeClaim := nodeClaim.DeepCopy()
146-
for label := range LabelDomainExceptions {
146+
for label := range K8sLabelDomains {
147147
nodeClaim.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
148148
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
149149
}
@@ -152,9 +152,9 @@ var _ = Describe("Validation", func() {
152152
nodeClaim = oldNodeClaim.DeepCopy()
153153
}
154154
})
155-
It("should allow restricted subdomains exceptions", func() {
155+
It("should allow kubernetes subdomains", func() {
156156
oldNodeClaim := nodeClaim.DeepCopy()
157-
for label := range LabelDomainExceptions {
157+
for label := range K8sLabelDomains {
158158
nodeClaim.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
159159
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
160160
}

pkg/apis/v1/nodepool_validation_cel_test.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -412,9 +412,9 @@ var _ = Describe("CEL/Validation", func() {
412412
Expect(nodePool.RuntimeValidate()).ToNot(Succeed())
413413
}
414414
})
415-
It("should allow restricted domains exceptions", func() {
415+
It("should allow kubernetes domains", func() {
416416
oldNodePool := nodePool.DeepCopy()
417-
for label := range LabelDomainExceptions {
417+
for label := range K8sLabelDomains {
418418
nodePool.Spec.Template.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
419419
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
420420
}
@@ -424,9 +424,9 @@ var _ = Describe("CEL/Validation", func() {
424424
nodePool = oldNodePool.DeepCopy()
425425
}
426426
})
427-
It("should allow restricted subdomains exceptions", func() {
427+
It("should allow kubernetes subdomains exceptions", func() {
428428
oldNodePool := nodePool.DeepCopy()
429-
for label := range LabelDomainExceptions {
429+
for label := range K8sLabelDomains {
430430
nodePool.Spec.Template.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
431431
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
432432
}
@@ -560,9 +560,9 @@ var _ = Describe("CEL/Validation", func() {
560560
Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
561561
Expect(nodePool.RuntimeValidate()).To(Succeed())
562562
})
563-
It("should allow labels in restricted domains exceptions list", func() {
563+
It("should allow labels in kubernetes domains", func() {
564564
oldNodePool := nodePool.DeepCopy()
565-
for label := range LabelDomainExceptions {
565+
for label := range K8sLabelDomains {
566566
fmt.Println(label)
567567
nodePool.Spec.Template.Labels = map[string]string{
568568
label: "test-value",
@@ -573,9 +573,9 @@ var _ = Describe("CEL/Validation", func() {
573573
nodePool = oldNodePool.DeepCopy()
574574
}
575575
})
576-
It("should allow labels prefixed with the restricted domain exceptions", func() {
576+
It("should allow labels prefixed with the kubernetes domain", func() {
577577
oldNodePool := nodePool.DeepCopy()
578-
for label := range LabelDomainExceptions {
578+
for label := range K8sLabelDomains {
579579
nodePool.Spec.Template.Labels = map[string]string{
580580
fmt.Sprintf("%s/key", label): "test-value",
581581
}
@@ -587,7 +587,7 @@ var _ = Describe("CEL/Validation", func() {
587587
})
588588
It("should allow subdomain labels in restricted domains exceptions list", func() {
589589
oldNodePool := nodePool.DeepCopy()
590-
for label := range LabelDomainExceptions {
590+
for label := range K8sLabelDomains {
591591
nodePool.Spec.Template.Labels = map[string]string{
592592
fmt.Sprintf("subdomain.%s", label): "test-value",
593593
}
@@ -597,9 +597,9 @@ var _ = Describe("CEL/Validation", func() {
597597
nodePool = oldNodePool.DeepCopy()
598598
}
599599
})
600-
It("should allow subdomain labels prefixed with the restricted domain exceptions", func() {
600+
It("should allow subdomain labels prefixed with the kubernetes domain", func() {
601601
oldNodePool := nodePool.DeepCopy()
602-
for label := range LabelDomainExceptions {
602+
for label := range K8sLabelDomains {
603603
nodePool.Spec.Template.Labels = map[string]string{
604604
fmt.Sprintf("subdomain.%s/key", label): "test-value",
605605
}

pkg/controllers/nodeclaim/lifecycle/termination_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"sigs.k8s.io/controller-runtime/pkg/client"
3333

34+
"sigs.k8s.io/karpenter/pkg/apis"
3435
v1 "sigs.k8s.io/karpenter/pkg/apis/v1"
3536
"sigs.k8s.io/karpenter/pkg/cloudprovider"
3637
"sigs.k8s.io/karpenter/pkg/cloudprovider/fake"
@@ -300,7 +301,8 @@ var _ = Describe("Termination", func() {
300301
ExpectExists(ctx, env.Client, node)
301302
nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
302303

303-
Expect(nodeClaim.ObjectMeta.Annotations).To(BeNil())
304+
Expect(nodeClaim.ObjectMeta.Annotations).To(HaveLen(1))
305+
Expect(nodeClaim.ObjectMeta.Annotations).To(HaveKey(apis.Group + "/node-restricted-labels"))
304306
})
305307
It("should annotate the node if the NodeClaim has a terminationGracePeriod", func() {
306308
nodeClaim.Spec.TerminationGracePeriod = &metav1.Duration{Duration: time.Second * 300}
@@ -348,9 +350,8 @@ var _ = Describe("Termination", func() {
348350
ExpectExists(ctx, env.Client, node)
349351
nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
350352

351-
Expect(nodeClaim.ObjectMeta.Annotations).To(Equal(map[string]string{
352-
v1.NodeClaimTerminationTimestampAnnotationKey: "2024-04-01T12:00:00-05:00",
353-
}))
353+
Expect(nodeClaim.ObjectMeta.Annotations).To(HaveKeyWithValue(
354+
v1.NodeClaimTerminationTimestampAnnotationKey, "2024-04-01T12:00:00-05:00"))
354355
})
355356
It("should not delete Nodes if the NodeClaim is not registered", func() {
356357
ExpectApplied(ctx, env.Client, nodePool, nodeClaim)

0 commit comments

Comments
 (0)