From 840523c76431a01c8be6572f9365d7c4c5c8f5ca Mon Sep 17 00:00:00 2001 From: killianmuldoon Date: Thu, 8 Dec 2022 17:47:08 +0000 Subject: [PATCH] Add name hashing for long MS names Signed-off-by: killianmuldoon --- api/v1beta1/machineset_webhook.go | 5 +- .../machineset/machineset_controller.go | 7 +- .../controllers/machineset/msutil/names.go | 47 ++++++++++ .../machineset/msutil/names_test.go | 90 +++++++++++++++++++ 4 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 internal/controllers/machineset/msutil/names.go create mode 100644 internal/controllers/machineset/msutil/names_test.go diff --git a/api/v1beta1/machineset_webhook.go b/api/v1beta1/machineset_webhook.go index 50e096de3bc9..83db90ca8f80 100644 --- a/api/v1beta1/machineset_webhook.go +++ b/api/v1beta1/machineset_webhook.go @@ -28,6 +28,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/cluster-api/internal/controllers/machineset/msutil" "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] = msutil.MachineSetNameLabelValue(m.Name) + m.Spec.Template.Labels[MachineSetNameLabel] = msutil.MachineSetNameLabelValue(m.Name) } if m.Spec.Template.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Template.Spec.Version, "v") { diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 485621aee4d7..ae69d08a0e26 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -41,6 +41,7 @@ import ( "sigs.k8s.io/cluster-api/controllers/noderefutil" "sigs.k8s.io/cluster-api/controllers/remote" "sigs.k8s.io/cluster-api/internal/controllers/machine" + "sigs.k8s.io/cluster-api/internal/controllers/machineset/msutil" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/collections" @@ -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 && msutil.LabelValueMatchesMachineSet(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] = msutil.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] = msutil.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 diff --git a/internal/controllers/machineset/msutil/names.go b/internal/controllers/machineset/msutil/names.go new file mode 100644 index 000000000000..7ef8b7b0dbda --- /dev/null +++ b/internal/controllers/machineset/msutil/names.go @@ -0,0 +1,47 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package msutil implements MachineDeployment utilities. +package msutil + +import ( + "encoding/base64" + "hash/fnv" +) + +// 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)) +} + +// LabelValueMatchesMachineSet returns true if the passed labelValue equals either the machineSetName or the hashed +// value of the name if it's overly long. +func LabelValueMatchesMachineSet(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)) +} diff --git a/internal/controllers/machineset/msutil/names_test.go b/internal/controllers/machineset/msutil/names_test.go new file mode 100644 index 000000000000..7fc38d85ad8d --- /dev/null +++ b/internal/controllers/machineset/msutil/names_test.go @@ -0,0 +1,90 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package msutil + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +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 := LabelValueMatchesMachineSet(tt.machineSetName, tt.labelValue) + g.Expect(got).To(Equal(tt.want)) + }) + } +}