Skip to content

Commit

Permalink
Merge pull request #1 from sbueringer/review-capilabels-move
Browse files Browse the repository at this point in the history
🌱 move to format subpackage
  • Loading branch information
Jont828 committed Jul 17, 2023
2 parents 9893fc3 + 151acdf commit ad1fe9a
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 113 deletions.
6 changes: 3 additions & 3 deletions api/v1beta1/machineset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"sigs.k8s.io/cluster-api/feature"
capilabels "sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/labels/format"
"sigs.k8s.io/cluster-api/util/version"
)

Expand Down Expand Up @@ -70,8 +70,8 @@ func (m *MachineSet) Default() {

if len(m.Spec.Selector.MatchLabels) == 0 && len(m.Spec.Selector.MatchExpressions) == 0 {
// Note: MustFormatValue is used here as the value of this label will be a hash if the MachineSet name is longer than 63 characters.
m.Spec.Selector.MatchLabels[MachineSetNameLabel] = capilabels.MustFormatValue(m.Name)
m.Spec.Template.Labels[MachineSetNameLabel] = capilabels.MustFormatValue(m.Name)
m.Spec.Selector.MatchLabels[MachineSetNameLabel] = format.MustFormatValue(m.Name)
m.Spec.Template.Labels[MachineSetNameLabel] = format.MustFormatValue(m.Name)
}

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

// ControlPlaneMachineLabelsForCluster returns a set of labels to add to a control plane machine for this specific cluster.
Expand All @@ -36,6 +36,6 @@ func ControlPlaneMachineLabelsForCluster(kcp *controlplanev1.KubeadmControlPlane
labels[clusterv1.ClusterNameLabel] = clusterName
labels[clusterv1.MachineControlPlaneLabel] = ""
// Note: MustFormatValue is used here as the label value can be a hash if the control plane name is longer than 63 characters.
labels[clusterv1.MachineControlPlaneNameLabel] = capilabels.MustFormatValue(kcp.Name)
labels[clusterv1.MachineControlPlaneNameLabel] = format.MustFormatValue(kcp.Name)
return labels
}
8 changes: 4 additions & 4 deletions exp/internal/controllers/machinepool_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import (
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/conditions"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
capilabels "sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/labels/format"
"sigs.k8s.io/cluster-api/util/patch"
)

Expand Down Expand Up @@ -342,7 +342,7 @@ func (r *MachinePoolReconciler) reconcileMachines(ctx context.Context, mp *expv1

infraMachineSelector := metav1.LabelSelector{
MatchLabels: map[string]string{
clusterv1.MachinePoolNameLabel: capilabels.MustFormatValue(mp.Name),
clusterv1.MachinePoolNameLabel: format.MustFormatValue(mp.Name),
clusterv1.ClusterNameLabel: mp.Spec.ClusterName,
},
}
Expand Down Expand Up @@ -503,7 +503,7 @@ func getNewMachine(mp *expv1.MachinePool, infraMachine *unstructured.Unstructure
}

// Enforce that the MachinePoolNameLabel and ClusterNameLabel are present on the Machine.
machine.Labels[clusterv1.MachinePoolNameLabel] = capilabels.MustFormatValue(mp.Name)
machine.Labels[clusterv1.MachinePoolNameLabel] = format.MustFormatValue(mp.Name)
machine.Labels[clusterv1.ClusterNameLabel] = mp.Spec.ClusterName

return machine
Expand All @@ -528,7 +528,7 @@ func (r *MachinePoolReconciler) infraMachineToMachinePoolMapper(ctx context.Cont
}

for _, mp := range machinePoolList.Items {
if capilabels.MustFormatValue(mp.Name) == poolNameHash {
if format.MustFormatValue(mp.Name) == poolNameHash {
return []ctrl.Request{
{
NamespacedName: client.ObjectKey{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import (
"sigs.k8s.io/cluster-api/internal/test/builder"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/kubeconfig"
capilabels "sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/labels/format"
)

const (
Expand Down Expand Up @@ -1398,7 +1398,7 @@ func TestInfraMachineToMachinePoolMapper(t *testing.T) {
"namespace": metav1.NamespaceDefault,
"labels": map[string]interface{}{
clusterv1.ClusterNameLabel: clusterName,
clusterv1.MachinePoolNameLabel: capilabels.MustFormatValue(machinePool1.Name),
clusterv1.MachinePoolNameLabel: format.MustFormatValue(machinePool1.Name),
},
},
},
Expand All @@ -1413,7 +1413,7 @@ func TestInfraMachineToMachinePoolMapper(t *testing.T) {
"namespace": metav1.NamespaceDefault,
"labels": map[string]interface{}{
clusterv1.ClusterNameLabel: "other-cluster",
clusterv1.MachinePoolNameLabel: capilabels.MustFormatValue(machinePoolLongName.Name),
clusterv1.MachinePoolNameLabel: format.MustFormatValue(machinePoolLongName.Name),
},
},
},
Expand All @@ -1428,7 +1428,7 @@ func TestInfraMachineToMachinePoolMapper(t *testing.T) {
"namespace": metav1.NamespaceDefault,
"labels": map[string]interface{}{
clusterv1.ClusterNameLabel: "other-cluster",
clusterv1.MachinePoolNameLabel: capilabels.MustFormatValue("missing-machinepool"),
clusterv1.MachinePoolNameLabel: format.MustFormatValue("missing-machinepool"),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import (
"sigs.k8s.io/cluster-api/util/collections"
"sigs.k8s.io/cluster-api/util/conditions"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
capilabels "sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/labels/format"
clog "sigs.k8s.io/cluster-api/util/log"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/predicates"
Expand Down Expand Up @@ -684,7 +684,7 @@ func machineLabelsFromMachineSet(machineSet *clusterv1.MachineSet) map[string]st
// Note: If a client tries to create a MachineSet without a selector, the MachineSet webhook
// will add this label automatically. But we want this label to always be present even if the MachineSet
// has a selector which doesn't include it. Therefore, we have to set it here explicitly.
machineLabels[clusterv1.MachineSetNameLabel] = capilabels.MustFormatValue(machineSet.Name)
machineLabels[clusterv1.MachineSetNameLabel] = format.MustFormatValue(machineSet.Name)
// Propagate the MachineDeploymentNameLabel from MachineSet to Machine if it exists.
if mdName, ok := machineSet.Labels[clusterv1.MachineDeploymentNameLabel]; ok {
machineLabels[clusterv1.MachineDeploymentNameLabel] = mdName
Expand Down
52 changes: 52 additions & 0 deletions util/labels/format/helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
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 format contains functions to format and compare formatted values used in Kubernetes labels.
package format

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

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

// MustFormatValue 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 MustFormatValue(str 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(str)) == 0 {
return str
}
hasher := fnv.New32a()
_, err := hasher.Write([]byte(str))
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 fmt.Sprintf("hash_%s_z", base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil)))
}

// MustEqualValue returns true if the actualLabelValue equals either the inputLabelValue or the hashed
// value of the inputLabelValue.
func MustEqualValue(str, labelValue string) bool {
return labelValue == MustFormatValue(str)
}
90 changes: 90 additions & 0 deletions util/labels/format/helpers_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 format

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: "hash_FR_ghQ_z",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := MustFormatValue(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: "hash_Nx4RdE_z",
want: false,
},
{
name: "match labels when MachineSet name is long",
machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
labelValue: "hash_FR_ghQ_z",
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := MustEqualValue(tt.machineSetName, tt.labelValue)
g.Expect(got).To(gomega.Equal(tt.want))
})
}
}
31 changes: 0 additions & 31 deletions util/labels/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ limitations under the License.
package labels

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

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)
Expand All @@ -42,29 +37,3 @@ func HasWatchLabel(o metav1.Object, labelValue string) bool {
}
return val == labelValue
}

// MustFormatValue 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 MustFormatValue(str 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(str)) == 0 {
return str
}
hasher := fnv.New32a()
_, err := hasher.Write([]byte(str))
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 fmt.Sprintf("hash_%s_z", base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(hasher.Sum(nil)))
}

// MustEqualValue returns true if the actualLabelValue equals either the inputLabelValue or the hashed
// value of the inputLabelValue.
func MustEqualValue(str, labelValue string) bool {
return labelValue == MustFormatValue(str)
}
67 changes: 0 additions & 67 deletions util/labels/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,70 +84,3 @@ func TestHasWatchLabel(t *testing.T) {
})
}
}

func TestNameLabelValue(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: "hash_FR_ghQ_z",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := MustFormatValue(tt.machineSetName)
g.Expect(got).To(Equal(tt.want))
})
}
}

func TestMustMatchLabelValueForName(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: "hash_Nx4RdE_z",
want: false,
},
{
name: "match labels when MachineSet name is long",
machineSetName: "machineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetNamemachineSetName",
labelValue: "hash_FR_ghQ_z",
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := MustEqualValue(tt.machineSetName, tt.labelValue)
g.Expect(got).To(Equal(tt.want))
})
}
}

0 comments on commit ad1fe9a

Please sign in to comment.