Merge pull request #10911 from olemarkus/automated-cherry-pick-of-#10…

…910-origin-release-1.20 Automated cherry pick of #10910: Further improve cloudLabel validation
kubernetes · Feb 28, 2021 · 28ec3ce · 28ec3ce
2 parents 887d941 + 735bb9d
commit 28ec3ce
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 5 deletions.
diff --git a/pkg/apis/kops/validation/cluster.go b/pkg/apis/kops/validation/cluster.go
@@ -127,6 +127,10 @@ func validateEtcdMemberUpdate(fp *field.Path, obj kops.EtcdMemberSpec, status *k
 
 func validateClusterCloudLabels(cluster *kops.Cluster, fldPath *field.Path) (allErrs field.ErrorList) {
 	labels := cluster.Spec.CloudLabels
+	return validateCloudLabels(labels, fldPath)
+}
+
+func validateCloudLabels(labels map[string]string, fldPath *field.Path) (allErrs field.ErrorList) {
 	if labels == nil {
 		return allErrs
 	}
@@ -142,11 +146,10 @@ func validateClusterCloudLabels(cluster *kops.Cluster, fldPath *field.Path) (all
 			allErrs = append(allErrs, field.Forbidden(fldPath.Child(reservedKey), fmt.Sprintf("%q is a reserved label and cannot be used as a custom label", reservedKey)))
 		}
 	}
-
 	reservedPrefixes := []string{
+		"kubernetes.io/cluster/",
+		"k8s.io/role/",
 		"kops.k8s.io/",
-		"k8s.io/",
-		"kubernetes.io/",
 	}
 
 	for _, reservedPrefix := range reservedPrefixes {

diff --git a/pkg/apis/kops/validation/instancegroup.go b/pkg/apis/kops/validation/instancegroup.go
@@ -281,11 +281,21 @@ func validateIGCloudLabels(ig *kops.InstanceGroup, fldPath *field.Path) (allErrs
 		return allErrs
 	}
 
+	genericLabels := make(map[string]string)
+
 	for key, value := range labels {
-		if key == aws.CloudTagInstanceGroupName && value != ig.ObjectMeta.Name {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child(aws.CloudTagInstanceGroupName), key, "Node label may only contain a single slash"))
+		if key == aws.CloudTagInstanceGroupName {
+
+			if value != ig.ObjectMeta.Name {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child(aws.CloudTagInstanceGroupName), key, "Node label may only contain a single slash"))
+			}
+		} else {
+			genericLabels[key] = value
 		}
 	}
+
+	allErrs = append(allErrs, validateCloudLabels(genericLabels, fldPath)...)
+
 	return allErrs
 }
 

diff --git a/pkg/apis/kops/validation/instancegroup_test.go b/pkg/apis/kops/validation/instancegroup_test.go
@@ -270,6 +270,44 @@ func TestValidNodeLabels(t *testing.T) {
 	}
 }
 
+func TestValidateIGCloudLabels(t *testing.T) {
+
+	grid := []struct {
+		label    string
+		expected []string
+	}{
+
+		{
+			label: "k8s.io/cluster-autoscaler/test.example.com",
+		},
+		{
+			label:    "KubernetesCluster",
+			expected: []string{"Forbidden::spec.cloudLabels.KubernetesCluster"},
+		},
+		{
+			label: "MyBillingLabel",
+		},
+		{
+			label: "subdomain.domain.tld/foo/bar",
+		},
+	}
+
+	for _, g := range grid {
+		ig := &kops.InstanceGroup{
+			ObjectMeta: v1.ObjectMeta{
+				Name: "some-ig",
+			},
+			Spec: kops.InstanceGroupSpec{
+				Role:        "Node",
+				CloudLabels: make(map[string]string),
+			},
+		}
+		ig.Spec.CloudLabels[g.label] = "placeholder"
+		errs := ValidateInstanceGroup(ig, nil)
+		testErrors(t, g.label, errs, g.expected)
+	}
+}
+
 func TestIGCloudLabelIsIGName(t *testing.T) {
 
 	grid := []struct {