Skip to content

Commit

Permalink
Add name hashing for long MS names
Browse files Browse the repository at this point in the history
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
  • Loading branch information
killianmuldoon committed Dec 12, 2022
1 parent 4ee2872 commit 8160de6
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 8 deletions.
5 changes: 3 additions & 2 deletions api/v1beta1/machineset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"sigs.k8s.io/cluster-api/internal/labelvalue"
"sigs.k8s.io/cluster-api/util/version"
)

Expand Down Expand Up @@ -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] = labelvalue.MustGetForInput(m.Name)
m.Spec.Template.Labels[MachineSetNameLabel] = labelvalue.MustGetForInput(m.Name)
}

if m.Spec.Template.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Template.Spec.Version, "v") {
Expand Down
3 changes: 2 additions & 1 deletion controlplane/kubeadm/internal/cluster_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package internal
import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
"sigs.k8s.io/cluster-api/internal/labelvalue"
)

// ControlPlaneMachineLabelsForCluster returns a set of labels to add to a control plane machine for this specific cluster.
Expand All @@ -34,6 +35,6 @@ func ControlPlaneMachineLabelsForCluster(kcp *controlplanev1.KubeadmControlPlane
// Always force these labels over the ones coming from the spec.
labels[clusterv1.ClusterNameLabel] = clusterName
labels[clusterv1.MachineControlPlaneLabel] = ""
labels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name
labels[clusterv1.MachineControlPlaneNameLabel] = labelvalue.MustGetForInput(kcp.Name)
return labels
}
5 changes: 3 additions & 2 deletions controlplane/kubeadm/internal/controllers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/feature"
"sigs.k8s.io/cluster-api/internal/labelvalue"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/collections"
Expand Down Expand Up @@ -340,8 +341,8 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster *
// NOTE: cluster.x-k8s.io/control-plane is already set at this stage (it is used when reading controlPlane.Machines).
for i := range controlPlane.Machines {
machine := controlPlane.Machines[i]
if value, ok := machine.Labels[clusterv1.MachineControlPlaneNameLabel]; !ok || value != kcp.Name {
machine.Labels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name
if value, ok := machine.Labels[clusterv1.MachineControlPlaneNameLabel]; !ok || !labelvalue.MustMatchLabelValueForInput(value, kcp.Name) {
machine.Labels[clusterv1.MachineControlPlaneNameLabel] = labelvalue.MustGetForInput(kcp.Name)
}
}

Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/labelvalue"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/collections"
Expand Down Expand Up @@ -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 && labelvalue.MustMatchLabelValueForInput(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
Expand All @@ -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] = labelvalue.MustGetForInput(machineSet.Name)
// Propagate the MachineDeploymentNameLabel from MachineSet to Machine if it is set on the MachineSet.
if mdNameSetOnMachineSet {
machine.Labels[clusterv1.MachineDeploymentNameLabel] = mdNameOnMachineSet
Expand Down Expand Up @@ -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] = labelvalue.MustGetForInput(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
Expand Down
63 changes: 63 additions & 0 deletions internal/labelvalue/labelvalues.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copyright 2021 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 labelvalue contains functions to validate and compare values used in Kubernetes labels.
package labelvalue

import (
"encoding/base64"
"hash/fnv"

"k8s.io/apimachinery/pkg/util/validation"
)

// MustGetForInput returns the passed inputLabelValue if it meets the standards for a Kubernetes label value.
// If the name is not a valid label value this function returns a hash which meets the requirements.
func MustGetForInput(inputLabelValue string) string {
// a valid Kubernetes label value must:
// - be less than 64 characters long.
// - be an empty string OR consist of alphanumeric characters, '-', '_' or '.'.
// - start and end with an alphanumeric character
if len(validation.IsValidLabelValue(inputLabelValue)) == 0 {
return inputLabelValue
}
hasher := fnv.New32a()
_, err := hasher.Write([]byte(inputLabelValue))
if err != nil {
// At time of writing the implementation of fnv's Write function can never return an error.
// If this changes in a future go version this function will panic.
panic(err)
}
return base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
}

// MustMatchLabelValueForInput returns true if the actualLabelValue equals either the inputLabelValue or the hashed
// value of the inputLabelValue.
func MustMatchLabelValueForInput(inputLabelValue, actualLabelValue string) bool {
expectedLabelValue := MustGetForInput(inputLabelValue)
if actualLabelValue == expectedLabelValue {
return true
}
hasher := fnv.New32a()
// The Write function should never return an error.
_, err := hasher.Write([]byte(actualLabelValue))
if err != nil {
// At time of writing the implementation of fnv's Write function can never return an error.
// If this changes in a future go version this function will panic.
panic(err)
}
return expectedLabelValue == base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil))
}
90 changes: 90 additions & 0 deletions internal/labelvalue/labelvalues_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
Copyright 2021 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 labelvalue

import (
"testing"

"github.com/onsi/gomega"
)

func TestNameLabelValue(t *testing.T) {
g := gomega.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 := MustGetForInput(tt.machineSetName)
g.Expect(got).To(gomega.Equal(tt.want))
})
}
}

func TestMustMatchLabelValueForName(t *testing.T) {
g := gomega.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 := MustMatchLabelValueForInput(tt.machineSetName, tt.labelValue)
g.Expect(got).To(gomega.Equal(tt.want))
})
}
}

0 comments on commit 8160de6

Please sign in to comment.