fix: allow restricted labels in pod affinity/nodeSelector

Signed-off-by: jwcesign <[email protected]>

pkg/apis/v1/labels.go

@@ -103,13 +103,22 @@ var (
 	}
 )
 
-// IsRestrictedLabel returns an error if the label is restricted.
-func IsRestrictedLabel(key string) error {
+// IsRestrictedRequirements returns an error if the label requirement is restricted.
+func IsRestrictedRequirements(key string) error {
 	if WellKnownLabels.Has(key) {
 		return nil
 	}
+
+	// In the node pool requirements configuration, labels like kops.k8s.io/x should not be allowed, as they do not make sense.
+	labelDomain := GetLabelDomain(key)
+	for exceptionLabelDomain := range LabelDomainExceptions {
+		if strings.HasSuffix(labelDomain, exceptionLabelDomain) {
+			return fmt.Errorf("requirement label key %s is restricted; specify a well known label: %v, or a custom label that does not use a restricted domain: %v", key, sets.List(WellKnownLabels), sets.List(RestrictedLabelDomains))
+		}
+	}
+
 	if IsRestrictedNodeLabel(key) {
-		return fmt.Errorf("label %s is restricted; specify a well known label: %v, or a custom label that does not use a restricted domain: %v", key, sets.List(WellKnownLabels), sets.List(RestrictedLabelDomains))
+		return fmt.Errorf("requirement label key %s is restricted; specify a well known label: %v, or a custom label that does not use a restricted domain: %v", key, sets.List(WellKnownLabels), sets.List(RestrictedLabelDomains))
 	}
 	return nil
 }

pkg/apis/v1/nodeclaim_validation.go

@@ -110,17 +110,24 @@ func (in *NodeClaimTemplateSpec) validateRequirements() (errs error) {
 	return errs
 }
 
+// ValidateRequirement checks if the requirements are valid, including restricted labels, and the format of the requirements.
 func ValidateRequirement(requirement NodeSelectorRequirementWithMinValues) error { //nolint:gocyclo
+	errs := ValidateBasicRequirements(requirement)
+	if e := IsRestrictedRequirements(requirement.Key); e != nil {
+		errs = multierr.Append(errs, e)
+	}
+	return errs
+}
+
+// ValidateBasicRequirements checks if the requirements are valid, including the format of the requirements.
+func ValidateBasicRequirements(requirement NodeSelectorRequirementWithMinValues) error { //nolint:gocyclo
 	var errs error
 	if normalized, ok := NormalizedLabels[requirement.Key]; ok {
 		requirement.Key = normalized
 	}
 	if !SupportedNodeSelectorOps.Has(string(requirement.Operator)) {
 		errs = multierr.Append(errs, fmt.Errorf("key %s has an unsupported operator %s not in %s", requirement.Key, requirement.Operator, SupportedNodeSelectorOps.UnsortedList()))
 	}
-	if e := IsRestrictedLabel(requirement.Key); e != nil {
-		errs = multierr.Append(errs, e)
-	}
 	for _, err := range validation.IsQualifiedName(requirement.Key) {
 		errs = multierr.Append(errs, fmt.Errorf("key %s is not a qualified name, %s", requirement.Key, err))
 	}

pkg/apis/v1/nodepool_validation.go

@@ -41,8 +41,8 @@ func (in *NodeClaimTemplate) validateLabels() (errs error) {
 		for _, err := range validation.IsValidLabelValue(value) {
 			errs = multierr.Append(errs, fmt.Errorf("invalid value: %s for label[%s], %s", value, key, err))
 		}
-		if err := IsRestrictedLabel(key); err != nil {
-			errs = multierr.Append(errs, fmt.Errorf("invalid key name %q in labels, %s", key, err.Error()))
+		if IsRestrictedNodeLabel(key) {
+			errs = multierr.Append(errs, fmt.Errorf("restricted key name %q in labels", key))
 		}
 	}
 	return errs

pkg/apis/v1/nodepool_validation_cel_test.go

@@ -411,26 +411,26 @@ var _ = Describe("CEL/Validation", func() {
 				Expect(nodePool.RuntimeValidate()).ToNot(Succeed())
 			}
 		})
-		It("should allow restricted domains exceptions", func() {
+		It("should fail for the allow restricted domains exceptions", func() {
 			oldNodePool := nodePool.DeepCopy()
 			for label := range LabelDomainExceptions {
 				nodePool.Spec.Template.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
 					{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
 				}
 				Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
-				Expect(nodePool.RuntimeValidate()).To(Succeed())
+				Expect(nodePool.RuntimeValidate()).ToNot(Succeed())
 				Expect(env.Client.Delete(ctx, nodePool)).To(Succeed())
 				nodePool = oldNodePool.DeepCopy()
 			}
 		})
-		It("should allow restricted subdomains exceptions", func() {
+		It("should fail for allow restricted subdomains exceptions", func() {
 			oldNodePool := nodePool.DeepCopy()
 			for label := range LabelDomainExceptions {
 				nodePool.Spec.Template.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
 					{NodeSelectorRequirement: 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(nodePool.RuntimeValidate()).ToNot(Succeed())
 				Expect(env.Client.Delete(ctx, nodePool)).To(Succeed())
 				nodePool = oldNodePool.DeepCopy()
 			}

pkg/controllers/provisioning/provisioner.go

@@ -512,7 +512,7 @@ func validateNodeSelectorTerm(term corev1.NodeSelectorTerm) (errs error) {
 	}
 	if term.MatchExpressions != nil {
 		for _, requirement := range term.MatchExpressions {
-			errs = multierr.Append(errs, v1.ValidateRequirement(v1.NodeSelectorRequirementWithMinValues{
+			errs = multierr.Append(errs, v1.ValidateBasicRequirements(v1.NodeSelectorRequirementWithMinValues{
 				NodeSelectorRequirement: requirement,
 			}))
 		}