add RFC952 label validation

duda · Oct 24, 2014 · e2365b1 · e2365b1
1 parent 15008e1
commit e2365b1
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 4 deletions.
diff --git a/docs/labels.md b/docs/labels.md
@@ -18,7 +18,9 @@ Label selectors may also be used to associate policies with sets of objects.
 
 We also [plan](https://github.com/GoogleCloudPlatform/kubernetes/issues/560) to make labels available inside pods and [lifecycle hooks](container-environment.md).
 
-[Namespacing of label keys](https://github.com/GoogleCloudPlatform/kubernetes/issues/1491) and [character/syntax restrictions](https://github.com/GoogleCloudPlatform/kubernetes/issues/1297) are under discussion.
+[Namespacing of label keys](https://github.com/GoogleCloudPlatform/kubernetes/issues/1491) is under discussion.
+
+Valid labels follow a slightly modified RFC952 format:  24 characters or less, all lowercase, begins with alpha, dashes (-) are allowed, and ends with alphanumeric.
 
 ## Motivation
 

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
@@ -339,6 +339,17 @@ func ValidatePod(pod *api.Pod) errs.ValidationErrorList {
 		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace))
 	}
 	allErrs = append(allErrs, ValidatePodState(&pod.DesiredState).Prefix("desiredState")...)
+	allErrs = append(allErrs, validateLabels(pod.Labels)...)
+	return allErrs
+}
+
+func validateLabels(labels map[string]string) errs.ValidationErrorList {
+	allErrs := errs.ValidationErrorList{}
+	for k := range labels {
+		if !util.IsDNS952Label(k) {
+			allErrs = append(allErrs, errs.NewFieldNotSupported("label", k))
+		}
+	}
 	return allErrs
 }
 
@@ -392,6 +403,8 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
 	if labels.Set(service.Selector).AsSelector().Empty() {
 		allErrs = append(allErrs, errs.NewFieldRequired("selector", service.Selector))
 	}
+	allErrs = append(allErrs, validateLabels(service.Labels)...)
+	allErrs = append(allErrs, validateLabels(service.Selector)...)
 	return allErrs
 }
 
@@ -405,6 +418,7 @@ func ValidateReplicationController(controller *api.ReplicationController) errs.V
 		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", controller.Namespace))
 	}
 	allErrs = append(allErrs, ValidateReplicationControllerState(&controller.DesiredState).Prefix("desiredState")...)
+	allErrs = append(allErrs, validateLabels(controller.Labels)...)
 	return allErrs
 }
 
@@ -419,6 +433,7 @@ func ValidateReplicationControllerState(state *api.ReplicationControllerState) e
 	if !selector.Matches(labels) {
 		allErrs = append(allErrs, errs.NewFieldInvalid("podTemplate.labels", state.PodTemplate))
 	}
+	allErrs = append(allErrs, validateLabels(labels)...)
 	if state.Replicas < 0 {
 		allErrs = append(allErrs, errs.NewFieldInvalid("replicas", state.Replicas))
 	}

diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go
@@ -421,6 +421,27 @@ func TestValidatePod(t *testing.T) {
 	if len(errs) != 1 {
 		t.Errorf("Unexpected error list: %#v", errs)
 	}
+	errs = ValidatePod(&api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name:      "foo",
+			Namespace: api.NamespaceDefault,
+			Labels: map[string]string{
+				"123cantbeginwithnumber":          "bar", //invalid
+				"NoUppercase123":                  "bar", //invalid
+				"nospecialchars^=@":               "bar", //invalid
+				"cantendwithadash-":               "bar", //invalid
+				"rfc952-mustbe24charactersorless": "bar", //invalid
+				"rfc952-dash-nodots-lower":        "bar", //good label
+				"rfc952-24chars-orless":           "bar", //good label
+			},
+		},
+		DesiredState: api.PodState{
+			Manifest: api.ContainerManifest{Version: "v1beta1", ID: "abc"},
+		},
+	})
+	if len(errs) != 5 {
+		t.Errorf("Unexpected non-zero error list: %#v", errs)
+	}
 }
 
 func TestValidatePodUpdate(t *testing.T) {
@@ -706,6 +727,21 @@ func TestValidateService(t *testing.T) {
 			},
 			numErrs: 0,
 		},
+		{
+			name: "invalid label",
+			svc: api.Service{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "abc123",
+					Namespace: api.NamespaceDefault,
+					Labels: map[string]string{
+						"NoUppercaseOrSpecialCharsLike=Equals": "bar",
+					},
+				},
+				Port:     80,
+				Selector: map[string]string{"foo": "bar", "NoUppercaseOrSpecialCharsLike=Equals": "bar"},
+			},
+			numErrs: 2,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -747,6 +783,15 @@ func TestValidateReplicationController(t *testing.T) {
 			},
 		},
 	}
+	invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
+	invalidPodTemplate := api.PodTemplate{
+		DesiredState: api.PodState{
+			Manifest: api.ContainerManifest{
+				Version: "v1beta1",
+			},
+		},
+		Labels: invalidSelector,
+	}
 	successCases := []api.ReplicationController{
 		{
 			ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: api.NamespaceDefault},
@@ -817,6 +862,31 @@ func TestValidateReplicationController(t *testing.T) {
 				ReplicaSelector: validSelector,
 			},
 		},
+		"invalid_label": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "abc-123",
+				Namespace: api.NamespaceDefault,
+				Labels: map[string]string{
+					"NoUppercaseOrSpecialCharsLike=Equals": "bar",
+				},
+			},
+			DesiredState: api.ReplicationControllerState{
+				ReplicaSelector: validSelector,
+				PodTemplate:     validPodTemplate,
+			},
+		},
+		"invalid_label 2": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "abc-123",
+				Namespace: api.NamespaceDefault,
+				Labels: map[string]string{
+					"NoUppercaseOrSpecialCharsLike=Equals": "bar",
+				},
+			},
+			DesiredState: api.ReplicationControllerState{
+				PodTemplate: invalidPodTemplate,
+			},
+		},
 	}
 	for k, v := range errorCases {
 		errs := ValidateReplicationController(&v)
@@ -830,7 +900,9 @@ func TestValidateReplicationController(t *testing.T) {
 				field != "namespace" &&
 				field != "desiredState.replicaSelector" &&
 				field != "GCEPersistentDisk.ReadOnly" &&
-				field != "desiredState.replicas" {
+				field != "desiredState.replicas" &&
+				field != "desiredState.label" &&
+				field != "label" {
 				t.Errorf("%s: missing prefix for: %v", k, errs[i])
 			}
 		}

diff --git a/pkg/controller/replication_controller.go b/pkg/controller/replication_controller.go
@@ -57,8 +57,6 @@ func (r RealPodControl) createReplica(ctx api.Context, controllerSpec api.Replic
 	for k, v := range controllerSpec.DesiredState.PodTemplate.Labels {
 		desiredLabels[k] = v
 	}
-	desiredLabels["replicationController"] = controllerSpec.Name
-
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{
 			Labels: desiredLabels,