Add name hashing for long MS names

Signed-off-by: killianmuldoon <kmuldoon@vmware.com>

api/v1beta1/machineset_webhook.go

@@ -28,6 +28,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
+	utillabels "sigs.k8s.io/cluster-api/util/labels"
 	"sigs.k8s.io/cluster-api/util/version"
 )
 
@@ -64,8 +65,8 @@ func (m *MachineSet) Default() {
 	}
 
 	if len(m.Spec.Selector.MatchLabels) == 0 && len(m.Spec.Selector.MatchExpressions) == 0 {
-		m.Spec.Selector.MatchLabels[MachineSetNameLabel] = m.Name
-		m.Spec.Template.Labels[MachineSetNameLabel] = m.Name
+		m.Spec.Selector.MatchLabels[MachineSetNameLabel] = utillabels.MachineSetNameLabelValue(m.Name)
+		m.Spec.Template.Labels[MachineSetNameLabel] = utillabels.MachineSetNameLabelValue(m.Name)
 	}
 
 	if m.Spec.Template.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Template.Spec.Version, "v") {

internal/controllers/machineset/machineset_controller.go

@@ -46,6 +46,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
+	utillabels "sigs.k8s.io/cluster-api/util/labels"
 	clog "sigs.k8s.io/cluster-api/util/log"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
@@ -298,7 +299,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
 		mdNameOnMachineSet, mdNameSetOnMachineSet := machineSet.Labels[clusterv1.MachineDeploymentNameLabel]
 		mdNameOnMachine := machine.Labels[clusterv1.MachineDeploymentNameLabel]
 
-		if msName, ok := machine.Labels[clusterv1.MachineSetNameLabel]; ok && msName == machineSet.Name &&
+		if msNameLabelValue, ok := machine.Labels[clusterv1.MachineSetNameLabel]; ok && utillabels.MustMatchLabelValueForName(machineSet.Name, msNameLabelValue) &&
 			(!mdNameSetOnMachineSet || mdNameOnMachineSet == mdNameOnMachine) {
 			// Continue if the MachineSet name label is already set correctly and
 			// either the MachineDeployment name label is not set on the MachineSet or
@@ -310,7 +311,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
 		if err != nil {
 			return ctrl.Result{}, errors.Wrapf(err, "failed to apply %s label to Machine %q", clusterv1.MachineSetNameLabel, machine.Name)
 		}
-		machine.Labels[clusterv1.MachineSetNameLabel] = machineSet.Name
+		machine.Labels[clusterv1.MachineSetNameLabel] = utillabels.MachineSetNameLabelValue(machineSet.Name)
 		// Propagate the MachineDeploymentNameLabel from MachineSet to Machine if it is set on the MachineSet.
 		if mdNameSetOnMachineSet {
 			machine.Labels[clusterv1.MachineDeploymentNameLabel] = mdNameOnMachineSet
@@ -558,7 +559,7 @@ func (r *Reconciler) getNewMachine(machineSet *clusterv1.MachineSet) *clusterv1.
 
 	// Enforce that the MachineSetNameLabel label is set
 	// Note: the MachineSetNameLabel is added by the default webhook to MachineSet.spec.template.labels if a spec.selector is empty.
-	machine.Labels[clusterv1.MachineSetNameLabel] = machineSet.Name
+	machine.Labels[clusterv1.MachineSetNameLabel] = utillabels.MachineSetNameLabelValue(machineSet.Name)
 	// Propagate the MachineDeploymentNameLabel from MachineSet to Machine if it exists.
 	if mdName, ok := machineSet.Labels[clusterv1.MachineDeploymentNameLabel]; ok {
 		machine.Labels[clusterv1.MachineDeploymentNameLabel] = mdName

util/labels/helpers.go

@@ -18,6 +18,9 @@ limitations under the License.
 package labels
 
 import (
+	"encoding/base64"
+	"hash/fnv"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -37,3 +40,27 @@ func HasWatchLabel(o metav1.Object, labelValue string) bool {
 	}
 	return val == labelValue
 }
+
+// MachineSetNameLabelValue returns the machineSetName if it meets the standards for a Kubernetes label value.
+// If the name can not be used as a label value this function returns a hash.
+func MachineSetNameLabelValue(machineSetName string) string {
+	if len(machineSetName) <= 63 {
+		return machineSetName
+	}
+	hasher := fnv.New32a()
+	var _, _ = hasher.Write([]byte(machineSetName)) // Can never return error.
+	return base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
+}
+
+// MustMatchLabelValueForName returns true if the passed labelValue equals either the machineSetName or the hashed
+// value of the name if it's overly long.
+func MustMatchLabelValueForName(machineSetName, labelValue string) bool {
+	expectedLabelValue := MachineSetNameLabelValue(machineSetName)
+	if labelValue == expectedLabelValue {
+		return true
+	}
+	hasher := fnv.New32a()
+	// The Write function should never return an error.
+	hasher.Write([]byte(labelValue)) //nolint:gosec
+	return expectedLabelValue == base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
+}

util/labels/helpers_test.go

@@ -84,3 +84,70 @@ func TestHasWatchLabel(t *testing.T) {
 		})
 	}
 }
+
+func TestMachineSetNameOrHash(t *testing.T) {
+	g := NewWithT(t)
+	tests := []struct {
+		name           string
+		machineSetName string
+		want           string
+	}{
+		{
+			name:           "return the name if it's less than 63 characters",
+			machineSetName: "machineSetName",
+			want:           "machineSetName",
+		},
+		{
+			name:           "return  for a name with more than 63 characters",
+			machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
+			want:           "FR_ghQ",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := MachineSetNameLabelValue(tt.machineSetName)
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
+func TestLabelValueMatchesMachineSet(t *testing.T) {
+	g := NewWithT(t)
+	tests := []struct {
+		name           string
+		machineSetName string
+		labelValue     string
+		want           bool
+	}{
+		{
+			name:           "match labels when MachineSet name is short",
+			machineSetName: "ms1",
+			labelValue:     "ms1",
+			want:           true,
+		},
+		{
+			name:           "don't match different labels when MachineSet name is short",
+			machineSetName: "ms1",
+			labelValue:     "notMS1",
+			want:           false,
+		},
+		{
+			name:           "don't match labels when MachineSet name is long",
+			machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
+			labelValue:     "Nx4RdE",
+			want:           false,
+		},
+		{
+			name:           "match labels when MachineSet name is long",
+			machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
+			labelValue:     "FR_ghQ",
+			want:           true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := MustMatchLabelValueForName(tt.machineSetName, tt.labelValue)
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}