From b30173658f7c0679c7aecca3124065024969de0b Mon Sep 17 00:00:00 2001
From: Clayton Coleman <ccoleman@redhat.com>
Date: Sun, 1 Feb 2015 19:03:04 -0500
Subject: [PATCH] Slightly relax annotation validation

The more aggressive validation on annotations broke openshift (and
backwards compat for our data).  This change relaxes to allow mixed
case so we can continue working in the short term, try to see if we
can agree on relaxation, and if we can't, apply the stronger
validation here.
---
 pkg/api/validation/validation.go      | 40 +++++++++++++++++----------
 pkg/api/validation/validation_test.go | 37 +++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
index 2271f8e20ea..3df7a793149 100644
--- a/pkg/api/validation/validation.go
+++ b/pkg/api/validation/validation.go
@@ -29,6 +29,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)
@@ -71,7 +93,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
@@ -106,7 +128,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
@@ -490,17 +512,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{}
@@ -607,7 +618,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 {
@@ -623,7 +633,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
diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go
index d91db387e92..c0fc2b05fd1 100644
--- a/pkg/api/validation/validation_test.go
+++ b/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"},