diff --git a/pkg/api/validation.go b/pkg/api/validation.go index b1847c99b78..70b098ea8cc 100644 --- a/pkg/api/validation.go +++ b/pkg/api/validation.go @@ -260,6 +260,9 @@ func ValidateService(service *Service) errs.ErrorList { } else if !util.IsDNS952Label(service.ID) { allErrs = append(allErrs, errs.NewInvalid("Service.ID", service.ID)) } + 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 b9304360127..d7d6f597923 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" + 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" @@ -30,10 +30,11 @@ import ( func TestServiceRegistryCreate(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{} + fakeCloud := &cloud.FakeCloud{} 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"}, } @@ -63,6 +64,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"}, }, @@ -79,17 +81,20 @@ func TestServiceStorageValidatesCreate(t *testing.T) { if err == nil { t.Errorf("Expected to get an error") } + } } 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"}, }) @@ -112,16 +117,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{}, }, @@ -139,10 +147,11 @@ func TestServiceStorageValidatesUpdate(t *testing.T) { func TestServiceRegistryExternalService(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{} + fakeCloud := &cloud.FakeCloud{} 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, @@ -163,12 +172,13 @@ func TestServiceRegistryExternalService(t *testing.T) { func TestServiceRegistryExternalServiceError(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{ + fakeCloud := &cloud.FakeCloud{ Err: fmt.Errorf("test error"), } 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, @@ -185,7 +195,7 @@ func TestServiceRegistryExternalServiceError(t *testing.T) { func TestServiceRegistryDelete(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{} + fakeCloud := &cloud.FakeCloud{} machines := []string{"foo", "bar", "baz"} storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines)) svc := api.Service{ @@ -205,7 +215,7 @@ func TestServiceRegistryDelete(t *testing.T) { func TestServiceRegistryDeleteExternal(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{} + fakeCloud := &cloud.FakeCloud{} machines := []string{"foo", "bar", "baz"} storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines)) svc := api.Service{ @@ -248,7 +258,7 @@ func TestServiceRegistryMakeLinkVariables(t *testing.T) { func TestServiceRegistryGet(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{} + fakeCloud := &cloud.FakeCloud{} machines := []string{"foo", "bar", "baz"} storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines)) registry.CreateService(api.Service{ @@ -266,7 +276,7 @@ func TestServiceRegistryGet(t *testing.T) { func TestServiceRegistryList(t *testing.T) { registry := registrytest.NewServiceRegistry() - fakeCloud := &fake_cloud.FakeCloud{} + fakeCloud := &cloud.FakeCloud{} machines := []string{"foo", "bar", "baz"} storage := NewRegistryStorage(registry, fakeCloud, minion.NewRegistry(machines)) registry.CreateService(api.Service{