diff --git a/hack/validation/labels.sh b/hack/validation/labels.sh index 49ff9aaf3..1fdc33c21 100755 --- a/hack/validation/labels.sh +++ b/hack/validation/labels.sh @@ -4,8 +4,8 @@ ## checking for restricted labels while filtering out well-known labels 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 yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.metadata.properties.labels.x-kubernetes-validations += [ - {"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.startsWith(\"node.kubernetes.io\") || x.startsWith(\"node-restriction.kubernetes.io\") || !x.find(\"^([^/]+)\").endsWith(\"kubernetes.io\"))"}, - {"message": "label domain \"k8s.io\" is restricted", "rule": "self.all(x, x.startsWith(\"kops.k8s.io\") || !x.find(\"^([^/]+)\").endsWith(\"k8s.io\"))"}, + {"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\"))"}, + {"message": "label domain \"k8s.io\" is restricted", "rule": "self.all(x, x.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !x.find(\"^([^/]+)\").endsWith(\"k8s.io\"))"}, {"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\"))"}, {"message": "label \"karpenter.sh/nodepool\" is restricted", "rule": "self.all(x, x != \"karpenter.sh/nodepool\")"}, {"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self.all(x, x != \"kubernetes.io/hostname\")"}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml diff --git a/hack/validation/requirements.sh b/hack/validation/requirements.sh index d5929190d..e0ca78e25 100755 --- a/hack/validation/requirements.sh +++ b/hack/validation/requirements.sh @@ -7,8 +7,8 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.req 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 ## checking for restricted labels while filtering out well-known labels yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [ - {"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.startsWith(\"node.kubernetes.io/\") || self.startsWith(\"node-restriction.kubernetes.io/\") || !self.find(\"^([^/]+)\").endsWith(\"kubernetes.io\")"}, - {"message": "label domain \"k8s.io\" is restricted", "rule": "self.startsWith(\"kops.k8s.io/\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"}, + {"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\")"}, + {"message": "label domain \"k8s.io\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"}, {"message": "label domain \"karpenter.sh\" is restricted", "rule": "self in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.sh\")"}, {"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self != \"kubernetes.io/hostname\""}]' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml ## operator enum values @@ -24,8 +24,8 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.tem 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 ## checking for restricted labels while filtering out well-known labels yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [ - {"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.startsWith(\"node.kubernetes.io/\") || self.startsWith(\"node-restriction.kubernetes.io/\") || !self.find(\"^([^/]+)\").endsWith(\"kubernetes.io\")"}, - {"message": "label domain \"k8s.io\" is restricted", "rule": "self.startsWith(\"kops.k8s.io/\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"}, + {"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\")"}, + {"message": "label domain \"k8s.io\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"kops.k8s.io\") || !self.find(\"^([^/]+)\").endsWith(\"k8s.io\")"}, {"message": "label domain \"karpenter.sh\" is restricted", "rule": "self in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !self.find(\"^([^/]+)\").endsWith(\"karpenter.sh\")"}, {"message": "label \"karpenter.sh/nodepool\" is restricted", "rule": "self != \"karpenter.sh/nodepool\""}, {"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self != \"kubernetes.io/hostname\""}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml diff --git a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml index 5f624480d..2c4890c80 100644 --- a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml @@ -186,9 +186,9 @@ spec: 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]$ x-kubernetes-validations: - 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.startsWith("node.kubernetes.io/") || self.startsWith("node-restriction.kubernetes.io/") || !self.find("^([^/]+)").endsWith("kubernetes.io") + 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") - message: label domain "k8s.io" is restricted - rule: self.startsWith("kops.k8s.io/") || !self.find("^([^/]+)").endsWith("k8s.io") + rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io") - message: label domain "karpenter.sh" is restricted rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh") - message: label "kubernetes.io/hostname" is restricted diff --git a/pkg/apis/crds/karpenter.sh_nodepools.yaml b/pkg/apis/crds/karpenter.sh_nodepools.yaml index 5b362d874..79443edce 100644 --- a/pkg/apis/crds/karpenter.sh_nodepools.yaml +++ b/pkg/apis/crds/karpenter.sh_nodepools.yaml @@ -97,9 +97,9 @@ spec: maxProperties: 100 x-kubernetes-validations: - 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.startsWith("node.kubernetes.io") || x.startsWith("node-restriction.kubernetes.io") || !x.find("^([^/]+)").endsWith("kubernetes.io")) + 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")) - message: label domain "k8s.io" is restricted - rule: self.all(x, x.startsWith("kops.k8s.io") || !x.find("^([^/]+)").endsWith("k8s.io")) + rule: self.all(x, x.find("^([^/]+)").endsWith("kops.k8s.io") || !x.find("^([^/]+)").endsWith("k8s.io")) - 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")) - message: label "karpenter.sh/nodepool" is restricted @@ -236,9 +236,9 @@ spec: 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]$ x-kubernetes-validations: - 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.startsWith("node.kubernetes.io/") || self.startsWith("node-restriction.kubernetes.io/") || !self.find("^([^/]+)").endsWith("kubernetes.io") + 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") - message: label domain "k8s.io" is restricted - rule: self.startsWith("kops.k8s.io/") || !self.find("^([^/]+)").endsWith("k8s.io") + rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io") - message: label domain "karpenter.sh" is restricted rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh") - message: label "karpenter.sh/nodepool" is restricted diff --git a/pkg/apis/v1alpha5/labels.go b/pkg/apis/v1alpha5/labels.go index 6a64bccfb..a504c3912 100644 --- a/pkg/apis/v1alpha5/labels.go +++ b/pkg/apis/v1alpha5/labels.go @@ -120,9 +120,11 @@ func IsRestrictedNodeLabel(key string) bool { if WellKnownLabels.Has(key) { return true } - labelDomain := getLabelDomain(key) - if LabelDomainExceptions.Has(labelDomain) { - return false + labelDomain := GetLabelDomain(key) + for exceptionLabelDomain := range LabelDomainExceptions { + if strings.HasSuffix(labelDomain, exceptionLabelDomain) { + return false + } } for restrictedLabelDomain := range RestrictedLabelDomains { if strings.HasSuffix(labelDomain, restrictedLabelDomain) { @@ -132,7 +134,7 @@ func IsRestrictedNodeLabel(key string) bool { return RestrictedLabels.Has(key) } -func getLabelDomain(key string) string { +func GetLabelDomain(key string) string { if parts := strings.SplitN(key, "/", 2); len(parts) == 2 { return parts[0] } diff --git a/pkg/apis/v1beta1/labels.go b/pkg/apis/v1beta1/labels.go index 5281cf04c..49e91ffb0 100644 --- a/pkg/apis/v1beta1/labels.go +++ b/pkg/apis/v1beta1/labels.go @@ -116,9 +116,11 @@ func IsRestrictedNodeLabel(key string) bool { if WellKnownLabels.Has(key) { return true } - labelDomain := getLabelDomain(key) - if LabelDomainExceptions.Has(labelDomain) { - return false + labelDomain := GetLabelDomain(key) + for exceptionLabelDomain := range LabelDomainExceptions { + if strings.HasSuffix(labelDomain, exceptionLabelDomain) { + return false + } } for restrictedLabelDomain := range RestrictedLabelDomains { if strings.HasSuffix(labelDomain, restrictedLabelDomain) { @@ -128,7 +130,7 @@ func IsRestrictedNodeLabel(key string) bool { return RestrictedLabels.Has(key) } -func getLabelDomain(key string) string { +func GetLabelDomain(key string) string { if parts := strings.SplitN(key, "/", 2); len(parts) == 2 { return parts[0] } diff --git a/pkg/apis/v1beta1/nodeclaim_validation_cel_test.go b/pkg/apis/v1beta1/nodeclaim_validation_cel_test.go index 2f83dc273..93e6128a2 100644 --- a/pkg/apis/v1beta1/nodeclaim_validation_cel_test.go +++ b/pkg/apis/v1beta1/nodeclaim_validation_cel_test.go @@ -128,6 +128,17 @@ var _ = Describe("Validation", func() { nodeClaim = oldNodeClaim.DeepCopy() } }) + It("should allow restricted subdomains exceptions", func() { + oldNodeClaim := nodeClaim.DeepCopy() + for label := range LabelDomainExceptions { + nodeClaim.Spec.Requirements = []v1.NodeSelectorRequirement{ + {Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodeClaim)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodeClaim)).To(Succeed()) + nodeClaim = oldNodeClaim.DeepCopy() + } + }) It("should allow well known label exceptions", func() { oldNodeClaim := nodeClaim.DeepCopy() for label := range WellKnownLabels.Difference(sets.New(NodePoolLabelKey)) { diff --git a/pkg/apis/v1beta1/nodeclaim_validation_webhook_test.go b/pkg/apis/v1beta1/nodeclaim_validation_webhook_test.go index 4510c5f67..7b33d122c 100644 --- a/pkg/apis/v1beta1/nodeclaim_validation_webhook_test.go +++ b/pkg/apis/v1beta1/nodeclaim_validation_webhook_test.go @@ -124,6 +124,14 @@ var _ = Describe("Validation", func() { Expect(nodeClaim.Validate(ctx)).To(Succeed()) } }) + It("should allow restricted subdomains exceptions", func() { + for label := range LabelDomainExceptions { + nodeClaim.Spec.Requirements = []v1.NodeSelectorRequirement{ + {Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(nodeClaim.Validate(ctx)).To(Succeed()) + } + }) It("should allow well known label exceptions", func() { for label := range WellKnownLabels.Difference(sets.New(NodePoolLabelKey)) { nodeClaim.Spec.Requirements = []v1.NodeSelectorRequirement{ diff --git a/pkg/apis/v1beta1/nodepool_validation_cel_test.go b/pkg/apis/v1beta1/nodepool_validation_cel_test.go index 806ec4a32..4b8203ec5 100644 --- a/pkg/apis/v1beta1/nodepool_validation_cel_test.go +++ b/pkg/apis/v1beta1/nodepool_validation_cel_test.go @@ -517,6 +517,18 @@ var _ = Describe("CEL/Validation", func() { nodePool = oldNodePool.DeepCopy() } }) + It("should allow restricted subdomains exceptions", func() { + oldNodePool := nodePool.DeepCopy() + for label := range LabelDomainExceptions { + nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{ + {Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(env.Client.Create(ctx, nodePool)).To(Succeed()) + Expect(nodePool.RuntimeValidate()).To(Succeed()) + Expect(env.Client.Delete(ctx, nodePool)).To(Succeed()) + nodePool = oldNodePool.DeepCopy() + } + }) It("should allow well known label exceptions", func() { oldNodePool := nodePool.DeepCopy() for label := range WellKnownLabels.Difference(sets.New(NodePoolLabelKey)) { @@ -632,5 +644,29 @@ var _ = Describe("CEL/Validation", func() { nodePool = oldNodePool.DeepCopy() } }) + It("should allow subdomain labels in restricted domains exceptions list", func() { + oldNodePool := nodePool.DeepCopy() + for label := range LabelDomainExceptions { + nodePool.Spec.Template.Labels = map[string]string{ + fmt.Sprintf("subdomain.%s", label): "test-value", + } + Expect(env.Client.Create(ctx, nodePool)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodePool)).To(Succeed()) + Expect(nodePool.RuntimeValidate()).To(Succeed()) + nodePool = oldNodePool.DeepCopy() + } + }) + It("should allow subdomain labels prefixed with the restricted domain exceptions", func() { + oldNodePool := nodePool.DeepCopy() + for label := range LabelDomainExceptions { + nodePool.Spec.Template.Labels = map[string]string{ + fmt.Sprintf("subdomain.%s/key", label): "test-value", + } + Expect(env.Client.Create(ctx, nodePool)).To(Succeed()) + Expect(env.Client.Delete(ctx, nodePool)).To(Succeed()) + Expect(nodePool.RuntimeValidate()).To(Succeed()) + nodePool = oldNodePool.DeepCopy() + } + }) }) }) diff --git a/pkg/apis/v1beta1/nodepool_validation_webhook_test.go b/pkg/apis/v1beta1/nodepool_validation_webhook_test.go index 0ce68f62a..3742492ad 100644 --- a/pkg/apis/v1beta1/nodepool_validation_webhook_test.go +++ b/pkg/apis/v1beta1/nodepool_validation_webhook_test.go @@ -149,6 +149,22 @@ var _ = Describe("Webhook/Validation", func() { Expect(nodePool.Validate(ctx)).To(Succeed()) } }) + It("should allow subdomain labels in restricted domains exceptions list", func() { + for label := range LabelDomainExceptions { + nodePool.Spec.Template.Labels = map[string]string{ + fmt.Sprintf("subdomain.%s", label): "test-value", + } + Expect(nodePool.Validate(ctx)).To(Succeed()) + } + }) + It("should allow subdomain labels prefixed with the restricted domain exceptions", func() { + for label := range LabelDomainExceptions { + nodePool.Spec.Template.Labels = map[string]string{ + fmt.Sprintf("subdomain.%s/key", label): "test-value", + } + Expect(nodePool.Validate(ctx)).To(Succeed()) + } + }) }) Context("Taints", func() { It("should succeed for valid taints", func() { @@ -239,6 +255,14 @@ var _ = Describe("Webhook/Validation", func() { Expect(nodePool.Validate(ctx)).To(Succeed()) } }) + It("should allow restricted subdomains exceptions", func() { + for label := range LabelDomainExceptions { + nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{ + {Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, + } + Expect(nodePool.Validate(ctx)).To(Succeed()) + } + }) It("should allow well known label exceptions", func() { for label := range WellKnownLabels.Difference(sets.New(NodePoolLabelKey)) { nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{ diff --git a/pkg/controllers/provisioning/nodepool_test.go b/pkg/controllers/provisioning/nodepool_test.go index 7bc411d5d..068504764 100644 --- a/pkg/controllers/provisioning/nodepool_test.go +++ b/pkg/controllers/provisioning/nodepool_test.go @@ -219,6 +219,28 @@ var _ = Describe("NodePool/Provisioning", func() { Expect(n.Node.Name).ToNot(Equal(node.Name)) } }) + It("should label nodes with labels in the subdomain from LabelDomainExceptions list", func() { + for domain := range v1beta1.LabelDomainExceptions { + nodePool := test.NodePool(v1beta1.NodePool{ + Spec: v1beta1.NodePoolSpec{ + Template: v1beta1.NodeClaimTemplate{ + ObjectMeta: v1beta1.ObjectMeta{ + Labels: map[string]string{"subdomain." + domain + "/test": "test-value"}, + }, + }, + }, + }) + ExpectApplied(ctx, env.Client, nodePool) + pod := test.UnschedulablePod( + test.PodOptions{ + NodeRequirements: []v1.NodeSelectorRequirement{{Key: "subdomain." + domain + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test-value"}}}, + }, + ) + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + node := ExpectScheduled(ctx, env.Client, pod) + Expect(node.Labels).To(HaveKeyWithValue("subdomain."+domain+"/test", "test-value")) + } + }) Context("Resource Limits", func() { It("should not schedule when limits are exceeded", func() { ExpectApplied(ctx, env.Client, test.NodePool(v1beta1.NodePool{ diff --git a/pkg/controllers/provisioning/scheduling/nodepool_test.go b/pkg/controllers/provisioning/scheduling/nodepool_test.go index 71fad2846..8fcb811d8 100644 --- a/pkg/controllers/provisioning/scheduling/nodepool_test.go +++ b/pkg/controllers/provisioning/scheduling/nodepool_test.go @@ -65,6 +65,21 @@ var _ = Context("NodePool", func() { }) }) + It("should schedule pods that have node selectors with label in subdomain from restricted domains exceptions list", func() { + var requirements []v1.NodeSelectorRequirement + for domain := range v1beta1.LabelDomainExceptions { + requirements = append(requirements, v1.NodeSelectorRequirement{Key: "subdomain." + domain + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test-value"}}) + } + nodePool.Spec.Template.Spec.Requirements = requirements + ExpectApplied(ctx, env.Client, nodePool) + for domain := range v1beta1.LabelDomainExceptions { + pod := test.UnschedulablePod() + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + node := ExpectScheduled(ctx, env.Client, pod) + Expect(node.Labels).To(HaveKeyWithValue("subdomain."+domain+"/test", "test-value")) + } + }) + Describe("Custom Constraints", func() { Context("NodePool with Labels", func() { It("should schedule unconstrained pods that don't have matching node selectors", func() { diff --git a/pkg/controllers/provisioning/suite_test.go b/pkg/controllers/provisioning/suite_test.go index ce02a7844..a8b48f586 100644 --- a/pkg/controllers/provisioning/suite_test.go +++ b/pkg/controllers/provisioning/suite_test.go @@ -266,7 +266,6 @@ var _ = Describe("Combined/Provisioning", func() { It("should select a Provisioner if a NodePool is over its limit", func() { nodePool.Spec.Limits = v1beta1.Limits(v1.ResourceList{v1.ResourceCPU: resource.MustParse("0")}) ExpectApplied(ctx, env.Client, provisioner, nodePool) - pod := test.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod)