Merge pull request #4017 from smarterclayton/relax_annotation_validations

Slightly relax annotation validation

pkg/api/validation/validation.go

@@ -30,6 +30,28 @@ import (
 	"github.com/golang/glog"
 )
 
+// ValidateLabels validates that a set of labels are correctly defined.
+func ValidateLabels(labels map[string]string, field string) errs.ValidationErrorList {
+	allErrs := errs.ValidationErrorList{}
+	for k := range labels {
+		if !util.IsQualifiedName(k) {
+			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
+		}
+	}
+	return allErrs
+}
+
+// ValidateAnnotations validates that a set of annotations are correctly defined.
+func ValidateAnnotations(annotations map[string]string, field string) errs.ValidationErrorList {
+	allErrs := errs.ValidationErrorList{}
+	for k := range annotations {
+		if !util.IsQualifiedName(strings.ToLower(k)) {
+			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
+		}
+	}
+	return allErrs
+}
+
 // ValidateNameFunc validates that the provided name is valid for a given resource type.
 // Not all resources have the same validation rules for names.
 type ValidateNameFunc func(name string) (bool, string)
@@ -72,7 +94,7 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val
 		}
 	}
 	allErrs = append(allErrs, ValidateLabels(meta.Labels, "labels")...)
-	allErrs = append(allErrs, ValidateLabels(meta.Annotations, "annotations")...)
+	allErrs = append(allErrs, ValidateAnnotations(meta.Annotations, "annotations")...)
 
 	// Clear self link internally
 	// TODO: move to its own area
@@ -107,7 +129,7 @@ func ValidateObjectMetaUpdate(old, meta *api.ObjectMeta) errs.ValidationErrorLis
 	}
 
 	allErrs = append(allErrs, ValidateLabels(meta.Labels, "labels")...)
-	allErrs = append(allErrs, ValidateLabels(meta.Annotations, "annotations")...)
+	allErrs = append(allErrs, ValidateAnnotations(meta.Annotations, "annotations")...)
 
 	// Clear self link internally
 	// TODO: move to its own area
@@ -492,17 +514,6 @@ func ValidatePodSpec(spec *api.PodSpec) errs.ValidationErrorList {
 	return allErrs
 }
 
-// ValidateLabels validates that a set of labels are correctly defined.
-func ValidateLabels(labels map[string]string, field string) errs.ValidationErrorList {
-	allErrs := errs.ValidationErrorList{}
-	for k := range labels {
-		if !util.IsQualifiedName(k) {
-			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
-		}
-	}
-	return allErrs
-}
-
 // ValidatePodUpdate tests to see if the update is legal
 func ValidatePodUpdate(newPod, oldPod *api.Pod) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
@@ -609,7 +620,6 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
 		if !selector.Matches(labels) {
 			allErrs = append(allErrs, errs.NewFieldInvalid("template.labels", spec.Template.Labels, "selector does not match template"))
 		}
-		allErrs = append(allErrs, ValidateLabels(spec.Template.Annotations, "annotations")...)
 		allErrs = append(allErrs, ValidatePodTemplateSpec(spec.Template).Prefix("template")...)
 		// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
 		if spec.Template.Spec.RestartPolicy.Always == nil {
@@ -625,7 +635,7 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
 func ValidatePodTemplateSpec(spec *api.PodTemplateSpec) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
 	allErrs = append(allErrs, ValidateLabels(spec.Labels, "labels")...)
-	allErrs = append(allErrs, ValidateLabels(spec.Annotations, "annotations")...)
+	allErrs = append(allErrs, ValidateAnnotations(spec.Annotations, "annotations")...)
 	allErrs = append(allErrs, ValidatePodSpec(&spec.Spec).Prefix("spec")...)
 	allErrs = append(allErrs, ValidateReadOnlyPersistentDisks(spec.Spec.Volumes).Prefix("spec.volumes")...)
 	return allErrs

pkg/api/validation/validation_test.go

@@ -74,6 +74,43 @@ func TestValidateLabels(t *testing.T) {
 	}
 }
 
+func TestValidateAnnotations(t *testing.T) {
+	successCases := []map[string]string{
+		{"simple": "bar"},
+		{"now-with-dashes": "bar"},
+		{"1-starts-with-num": "bar"},
+		{"1234": "bar"},
+		{"simple/simple": "bar"},
+		{"now-with-dashes/simple": "bar"},
+		{"now-with-dashes/now-with-dashes": "bar"},
+		{"now.with.dots/simple": "bar"},
+		{"now-with.dashes-and.dots/simple": "bar"},
+		{"1-num.2-num/3-num": "bar"},
+		{"1234/5678": "bar"},
+		{"1.2.3.4/5678": "bar"},
+		{"UpperCase123": "bar"},
+	}
+	for i := range successCases {
+		errs := ValidateAnnotations(successCases[i], "field")
+		if len(errs) != 0 {
+			t.Errorf("case[%d] expected success, got %#v", i, errs)
+		}
+	}
+
+	errorCases := []map[string]string{
+		{"nospecialchars^=@": "bar"},
+		{"cantendwithadash-": "bar"},
+		{"only/one/slash": "bar"},
+		{strings.Repeat("a", 254): "bar"},
+	}
+	for i := range errorCases {
+		errs := ValidateAnnotations(errorCases[i], "field")
+		if len(errs) != 1 {
+			t.Errorf("case[%d] expected failure", i)
+		}
+	}
+}
+
 func TestValidateVolumes(t *testing.T) {
 	successCase := []api.Volume{
 		{Name: "abc"},