taint propagation: implement validation

Author: chrischdi

internal/webhooks/machine.go

@@ -22,14 +22,17 @@ import (
 	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
+	"sigs.k8s.io/cluster-api/feature"
 	"sigs.k8s.io/cluster-api/util/labels"
 	"sigs.k8s.io/cluster-api/util/version"
 )
@@ -136,8 +139,82 @@ func (webhook *Machine) validate(oldM, newM *clusterv1.Machine) error {
 		}
 	}
 
+	allErrs = append(allErrs, validateMachineTaints(newM.Spec.Taints, specPath.Child("taints"))...)
+	allErrs = append(allErrs, validateMachineTaintsForWorkers(newM.Spec.Taints, newM, specPath.Child("taints"))...)
+
 	if len(allErrs) == 0 {
 		return nil
 	}
 	return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("Machine").GroupKind(), newM.Name, allErrs)
 }
+
+func validateMachineTaints(taints []clusterv1.MachineTaint, taintsPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if !feature.Gates.Enabled(feature.MachineTaintPropagation) {
+		if len(taints) > 0 {
+			allErrs = append(allErrs, field.Forbidden(taintsPath, "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled"))
+		}
+	}
+
+	for i, taint := range taints {
+		idxPath := taintsPath.Index(i)
+
+		// Validate key syntax.
+		if errs := metav1validation.ValidateLabelName(taint.Key, idxPath); len(errs) > 0 {
+			allErrs = append(allErrs, errs...)
+		}
+
+		// Validate value syntax.
+		if errs := validation.IsValidLabelValue(ptr.Deref(taint.Value, "")); len(errs) > 0 {
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("value"), taint.Value, strings.Join(errs, ";")))
+		}
+
+		// The following validations uses a switch statement, because if one of them matches, then the others won't.
+
+		switch {
+		// Validate for keys which are reserved for usage by the cluster-api or providers.
+		case taint.Key == "node.cluster.x-k8s.io/uninitialized":
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not be node.cluster.x-k8s.io/uninitialized"))
+		case taint.Key == "node.cluster.x-k8s.io/outdated-revision":
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not be node.cluster.x-k8s.io/uninitialized or node.cluster.x-k8s.io/outdated-revision"))
+		// Validate for key's which are reserved for usage by the node or node-lifecycle-controller, but allow `node.kubernetes.io/out-of-service`.
+		case strings.HasPrefix(taint.Key, "node.kubernetes.io/") && taint.Key != "node.kubernetes.io/out-of-service":
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not have the prefix node.kubernetes.io/, except for node.kubernetes.io/out-of-service"))
+		// Validate for keys which are reserved for usage by the cloud-controller-manager or kubelet.
+		case strings.HasPrefix(taint.Key, "node.cloudprovider.kubernetes.io/"):
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not have the prefix node.cloudprovider.kubernetes.io/"))
+		// Validate for the deprecated kubeadm node-role taint.
+		case taint.Key == "node-role.kubernetes.io/master":
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint is deprecated since 1.24 and should not be used anymore"))
+		}
+	}
+
+	return allErrs
+}
+
+func validateMachineTaintsForWorkers(taints []clusterv1.MachineTaint, machine *clusterv1.Machine, taintsPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if !feature.Gates.Enabled(feature.MachineTaintPropagation) {
+		return allErrs
+	}
+
+	// Skip for control-plane machines.
+	if machine != nil && machine.Labels != nil {
+		if _, ok := machine.Labels[clusterv1.MachineControlPlaneLabel]; ok {
+			return allErrs
+		}
+	}
+
+	// Validate taints for worker machines.
+	for i, taint := range taints {
+		idxPath := taintsPath.Index(i)
+
+		if taint.Key == "node-role.kubernetes.io/control-plane" {
+			allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint is not allowed for worker machines"))
+		}
+	}
+
+	return allErrs
+}

internal/webhooks/machinedeployment.go

@@ -254,6 +254,9 @@ func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeploy
 
 	allErrs = append(allErrs, validateMDMachineNaming(newMD.Spec.MachineNaming, specPath.Child("machineNaming"))...)
 
+	allErrs = append(allErrs, validateMachineTaints(newMD.Spec.Template.Spec.Taints, specPath.Child("template", "spec", "taints"))...)
+	allErrs = append(allErrs, validateMachineTaintsForWorkers(newMD.Spec.Template.Spec.Taints, nil, specPath.Child("template", "spec", "taints"))...)
+
 	// Validate the metadata of the template.
 	allErrs = append(allErrs, newMD.Spec.Template.Validate(specPath.Child("template", "metadata"))...)
 

internal/webhooks/machinepool.go

@@ -189,6 +189,14 @@ func (webhook *MachinePool) validate(oldObj, newObj *clusterv1.MachinePool) erro
 		}
 	}
 
+	if len(newObj.Spec.Template.Spec.Taints) > 0 {
+		if !feature.Gates.Enabled(feature.MachineTaintPropagation) {
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled"))
+		} else {
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints feature for MachinePools is not yet implemented"))
+		}
+	}
+
 	// Validate the metadata of the MachinePool template.
 	allErrs = append(allErrs, newObj.Spec.Template.Validate(specPath.Child("template", "metadata"))...)
 

internal/webhooks/machineset.go

@@ -229,6 +229,9 @@ func (webhook *MachineSet) validate(oldMS, newMS *clusterv1.MachineSet) error {
 		}
 	}
 
+	allErrs = append(allErrs, validateMachineTaints(newMS.Spec.Template.Spec.Taints, specPath.Child("template", "spec", "taints"))...)
+	allErrs = append(allErrs, validateMachineTaintsForWorkers(newMS.Spec.Template.Spec.Taints, nil, specPath.Child("template", "spec", "taints"))...)
+
 	allErrs = append(allErrs, validateMSMachineNaming(newMS.Spec.MachineNaming, specPath.Child("machineNaming"))...)
 
 	// Validate the metadata of the template.