From 7fcaf80f671f70c48c5d5b30b3ff4c8832359775 Mon Sep 17 00:00:00 2001
From: Satnam Singh <satnam@google.com>
Date: Fri, 22 Aug 2014 14:44:21 -0700
Subject: [PATCH] Make validation check for legal serive port numbers.

---
 pkg/api/validation.go                |  4 ++++
 pkg/api/validation_test.go           | 17 ++++++++++++++++-
 pkg/registry/service/storage_test.go | 11 ++++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/pkg/api/validation.go b/pkg/api/validation.go
index b1847c99b78..c8ec703714c 100644
--- a/pkg/api/validation.go
+++ b/pkg/api/validation.go
@@ -260,6 +260,10 @@ func ValidateService(service *Service) errs.ErrorList {
 	} else if !util.IsDNS952Label(service.ID) {
 		allErrs = append(allErrs, errs.NewInvalid("Service.ID", service.ID))
 	}
+	// Make sure the service port number is valid.
+	if !util.IsValidPortNum(service.Port) {
+		allErrs = append(allErrs, errs.NewInvalid("Service.Port", service.Port))
+	}
 	if labels.Set(service.Selector).AsSelector().Empty() {
 		allErrs = append(allErrs, errs.NewInvalid("Service.Selector", service.Selector))
 	}
diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go
index 2d13d246862..1e52c2869ca 100644
--- a/pkg/api/validation_test.go
+++ b/pkg/api/validation_test.go
@@ -333,17 +333,31 @@ func TestValidatePod(t *testing.T) {
 }
 
 func TestValidateService(t *testing.T) {
+	// This test should fail because the port number is invalid i.e.
+	// the Port field has a default value of 0.
 	errs := ValidateService(&Service{
 		JSONBase: JSONBase{ID: "foo"},
 		Selector: map[string]string{
 			"foo": "bar",
 		},
 	})
+	if len(errs) != 1 {
+		t.Errorf("Unexpected error list: %#v", errs)
+	}
+
+	errs = ValidateService(&Service{
+		Port:     6502,
+		JSONBase: JSONBase{ID: "foo"},
+		Selector: map[string]string{
+			"foo": "bar",
+		},
+	})
 	if len(errs) != 0 {
 		t.Errorf("Unexpected non-zero error list: %#v", errs)
 	}
 
 	errs = ValidateService(&Service{
+		Port: 6502,
 		Selector: map[string]string{
 			"foo": "bar",
 		},
@@ -353,6 +367,7 @@ func TestValidateService(t *testing.T) {
 	}
 
 	errs = ValidateService(&Service{
+		Port:     6502,
 		JSONBase: JSONBase{ID: "foo"},
 	})
 	if len(errs) != 1 {
@@ -360,7 +375,7 @@ func TestValidateService(t *testing.T) {
 	}
 
 	errs = ValidateService(&Service{})
-	if len(errs) != 2 {
+	if len(errs) != 3 {
 		t.Errorf("Unexpected error list: %#v", errs)
 	}
 }
diff --git a/pkg/registry/service/storage_test.go b/pkg/registry/service/storage_test.go
index e86f499efac..5685acb57c2 100644
--- a/pkg/registry/service/storage_test.go
+++ b/pkg/registry/service/storage_test.go
@@ -21,7 +21,7 @@ import (
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake"
+	fake_cloud "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/minion"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest"
@@ -34,6 +34,7 @@ func TestServiceRegistryCreate(t *testing.T) {
 	machines := []string{"foo", "bar", "baz"}
 	storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines))
 	svc := &api.Service{
+		Port:     6502,
 		JSONBase: api.JSONBase{ID: "foo"},
 		Selector: map[string]string{"bar": "baz"},
 	}
@@ -60,6 +61,7 @@ func TestServiceStorageValidatesCreate(t *testing.T) {
 	storage := NewRegistryStorage(registry, nil, nil)
 	failureCases := map[string]api.Service{
 		"empty ID": {
+			Port:     6502,
 			JSONBase: api.JSONBase{ID: ""},
 			Selector: map[string]string{"bar": "baz"},
 		},
@@ -82,11 +84,13 @@ func TestServiceStorageValidatesCreate(t *testing.T) {
 func TestServiceRegistryUpdate(t *testing.T) {
 	registry := registrytest.NewServiceRegistry()
 	registry.CreateService(api.Service{
+		Port:     6502,
 		JSONBase: api.JSONBase{ID: "foo"},
 		Selector: map[string]string{"bar": "baz1"},
 	})
 	storage := NewRegistryStorage(registry, nil, nil)
 	c, err := storage.Update(&api.Service{
+		Port:     6502,
 		JSONBase: api.JSONBase{ID: "foo"},
 		Selector: map[string]string{"bar": "baz2"},
 	})
@@ -109,16 +113,19 @@ func TestServiceRegistryUpdate(t *testing.T) {
 func TestServiceStorageValidatesUpdate(t *testing.T) {
 	registry := registrytest.NewServiceRegistry()
 	registry.CreateService(api.Service{
+		Port:     6502,
 		JSONBase: api.JSONBase{ID: "foo"},
 		Selector: map[string]string{"bar": "baz"},
 	})
 	storage := NewRegistryStorage(registry, nil, nil)
 	failureCases := map[string]api.Service{
 		"empty ID": {
+			Port:     6502,
 			JSONBase: api.JSONBase{ID: ""},
 			Selector: map[string]string{"bar": "baz"},
 		},
 		"empty selector": {
+			Port:     6502,
 			JSONBase: api.JSONBase{ID: "foo"},
 			Selector: map[string]string{},
 		},
@@ -140,6 +147,7 @@ func TestServiceRegistryExternalService(t *testing.T) {
 	machines := []string{"foo", "bar", "baz"}
 	storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines))
 	svc := &api.Service{
+		Port:                       6502,
 		JSONBase:                   api.JSONBase{ID: "foo"},
 		Selector:                   map[string]string{"bar": "baz"},
 		CreateExternalLoadBalancer: true,
@@ -166,6 +174,7 @@ func TestServiceRegistryExternalServiceError(t *testing.T) {
 	machines := []string{"foo", "bar", "baz"}
 	storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines))
 	svc := &api.Service{
+		Port:                       6502,
 		JSONBase:                   api.JSONBase{ID: "foo"},
 		Selector:                   map[string]string{"bar": "baz"},
 		CreateExternalLoadBalancer: true,