From 3aa39d54b6eca7114a5d56d606b9a77306bbc572 Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Tue, 5 May 2015 11:51:51 -0700
Subject: [PATCH] Service port names are required for multi-port

There is no provision for the first port to be unnamed.  I think I
initially allowed that, but then the Subset struct became a sorted
struct, so the first-ness of the port got lost.  If you have a Service
with one named and one unnamed port, what happens is that the
EndpointController fails to create Endpoints (validation error).
---
 pkg/api/validation/validation.go      | 19 +++++++++----------
 pkg/api/validation/validation_test.go |  8 ++++++++
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
index 2889a91f6c6..56ac14ee6cb 100644
--- a/pkg/api/validation/validation.go
+++ b/pkg/api/validation/validation.go
@@ -980,7 +980,7 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
 	}
 	allPortNames := util.StringSet{}
 	for i := range service.Spec.Ports {
-		allErrs = append(allErrs, validateServicePort(&service.Spec.Ports[i], i, &allPortNames).PrefixIndex(i).Prefix("spec.ports")...)
+		allErrs = append(allErrs, validateServicePort(&service.Spec.Ports[i], len(service.Spec.Ports) > 1, &allPortNames).PrefixIndex(i).Prefix("spec.ports")...)
 	}
 
 	if service.Spec.Selector != nil {
@@ -1018,18 +1018,17 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
 	return allErrs
 }
 
-func validateServicePort(sp *api.ServicePort, index int, allNames *util.StringSet) errs.ValidationErrorList {
+func validateServicePort(sp *api.ServicePort, requireName bool, allNames *util.StringSet) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
 
-	if len(sp.Name) == 0 {
-		// Allow empty names if they are the first port (mostly for compat).
-		if index != 0 {
-			allErrs = append(allErrs, errs.NewFieldRequired("name"))
+	if requireName && sp.Name == "" {
+		allErrs = append(allErrs, errs.NewFieldRequired("name"))
+	} else if sp.Name != "" {
+		if !util.IsDNS1123Label(sp.Name) {
+			allErrs = append(allErrs, errs.NewFieldInvalid("name", sp.Name, dns1123LabelErrorMsg))
+		} else if allNames.Has(sp.Name) {
+			allErrs = append(allErrs, errs.NewFieldDuplicate("name", sp.Name))
 		}
-	} else if !util.IsDNS1123Label(sp.Name) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("name", sp.Name, dns1123LabelErrorMsg))
-	} else if allNames.Has(sp.Name) {
-		allErrs = append(allErrs, errs.NewFieldDuplicate("name", sp.Name))
 	}
 
 	if !util.IsValidPortNum(sp.Port) {
diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go
index 3b8973a5603..e4b1b1f3da4 100644
--- a/pkg/api/validation/validation_test.go
+++ b/pkg/api/validation/validation_test.go
@@ -1600,6 +1600,14 @@ func TestValidateService(t *testing.T) {
 			},
 			numErrs: 1,
 		},
+		{
+			name: "empty multi-port port[0] name",
+			tweakSvc: func(s *api.Service) {
+				s.Spec.Ports[0].Name = ""
+				s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Protocol: "TCP", Port: 12345})
+			},
+			numErrs: 1,
+		},
 		{
 			name: "invalid port name",
 			tweakSvc: func(s *api.Service) {