diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index eb614906d36..0845d82305f 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -154,12 +154,3 @@ func ParseSelector(selector string) (Selector, error) { } return andTerm(items), nil } - -// MustParseSelector parses the selection or panics. -func MustParseSelector(selector string) Selector { - s, err := ParseSelector(selector) - if err != nil { - panic(err) - } - return s -} diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index a91bed859a4..39fd1ebedf4 100644 --- a/pkg/registry/controller_registry.go +++ b/pkg/registry/controller_registry.go @@ -91,6 +91,10 @@ func (storage *ControllerRegistryStorage) Create(obj interface{}) (<-chan interf // Pod Manifest ID should be assigned by the pod API controller.DesiredState.PodTemplate.DesiredState.Manifest.ID = "" + if errs := api.ValidateReplicationController(&controller); len(errs) > 0 { + return nil, fmt.Errorf("Validation errors: %v", errs) + } + return apiserver.MakeAsync(func() (interface{}, error) { err := storage.registry.CreateController(controller) if err != nil { @@ -106,8 +110,8 @@ func (storage *ControllerRegistryStorage) Update(obj interface{}) (<-chan interf if !ok { return nil, fmt.Errorf("not a replication controller: %#v", obj) } - if len(controller.ID) == 0 { - return nil, fmt.Errorf("ID should not be empty: %#v", controller) + if errs := api.ValidateReplicationController(&controller); len(errs) > 0 { + return nil, fmt.Errorf("Validation errors: %v", errs) } return apiserver.MakeAsync(func() (interface{}, error) { err := storage.registry.UpdateController(controller) diff --git a/pkg/registry/controller_registry_test.go b/pkg/registry/controller_registry_test.go index ffcbb2a6ea9..8e9a46160df 100644 --- a/pkg/registry/controller_registry_test.go +++ b/pkg/registry/controller_registry_test.go @@ -187,12 +187,27 @@ func TestControllerParsing(t *testing.T) { } } +var validPodTemplate = api.PodTemplate{ + DesiredState: api.PodState{ + Manifest: api.ContainerManifest{ + Version: "v1beta1", + Containers: []api.Container{ + { + Name: "test", + Image: "test_image", + }, + }, + }, + }, +} + func TestCreateController(t *testing.T) { mockRegistry := MockControllerRegistry{} mockPodRegistry := MockPodRegistry{ pods: []api.Pod{ { JSONBase: api.JSONBase{ID: "foo"}, + Labels: map[string]string{"a": "b"}, }, }, } @@ -204,10 +219,15 @@ func TestCreateController(t *testing.T) { controller := api.ReplicationController{ JSONBase: api.JSONBase{ID: "test"}, DesiredState: api.ReplicationControllerState{ - Replicas: 2, + Replicas: 2, + ReplicaSelector: map[string]string{"a": "b"}, + PodTemplate: validPodTemplate, }, } channel, err := storage.Create(controller) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } expectNoError(t, err) select { @@ -221,9 +241,11 @@ func TestCreateController(t *testing.T) { mockPodRegistry.pods = []api.Pod{ { JSONBase: api.JSONBase{ID: "foo"}, + Labels: map[string]string{"a": "b"}, }, { JSONBase: api.JSONBase{ID: "bar"}, + Labels: map[string]string{"a": "b"}, }, } mockPodRegistry.Unlock() @@ -235,3 +257,65 @@ func TestCreateController(t *testing.T) { // Do nothing, this is expected } } + +func TestControllerStorageValidatesCreate(t *testing.T) { + mockRegistry := MockControllerRegistry{} + storage := ControllerRegistryStorage{ + registry: &mockRegistry, + podRegistry: nil, + pollPeriod: time.Millisecond * 1, + } + + failureCases := map[string]api.ReplicationController{ + "empty ID": api.ReplicationController{ + JSONBase: api.JSONBase{ID: ""}, + DesiredState: api.ReplicationControllerState{ + ReplicaSelector: map[string]string{"bar": "baz"}, + }, + }, + "empty selector": api.ReplicationController{ + JSONBase: api.JSONBase{ID: "abc"}, + DesiredState: api.ReplicationControllerState{}, + }, + } + for _, failureCase := range failureCases { + c, err := storage.Create(failureCase) + if c != nil { + t.Errorf("Expected nil channel") + } + if err == nil { + t.Errorf("Expected to get an error") + } + } +} + +func TestControllerStorageValidatesUpdate(t *testing.T) { + mockRegistry := MockControllerRegistry{} + storage := ControllerRegistryStorage{ + registry: &mockRegistry, + podRegistry: nil, + pollPeriod: time.Millisecond * 1, + } + + failureCases := map[string]api.ReplicationController{ + "empty ID": api.ReplicationController{ + JSONBase: api.JSONBase{ID: ""}, + DesiredState: api.ReplicationControllerState{ + ReplicaSelector: map[string]string{"bar": "baz"}, + }, + }, + "empty selector": api.ReplicationController{ + JSONBase: api.JSONBase{ID: "abc"}, + DesiredState: api.ReplicationControllerState{}, + }, + } + for _, failureCase := range failureCases { + c, err := storage.Update(failureCase) + if c != nil { + t.Errorf("Expected nil channel") + } + if err == nil { + t.Errorf("Expected to get an error") + } + } +} diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index 70f8bb75f51..efc48a36a74 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -147,8 +147,7 @@ func (sr *ServiceRegistryStorage) Extract(body []byte) (interface{}, error) { func (sr *ServiceRegistryStorage) Create(obj interface{}) (<-chan interface{}, error) { srv := obj.(api.Service) - errs := api.ValidateService(&srv) - if len(errs) > 0 { + if errs := api.ValidateService(&srv); len(errs) > 0 { return nil, fmt.Errorf("Validation errors: %v", errs) } return apiserver.MakeAsync(func() (interface{}, error) { @@ -187,6 +186,9 @@ func (sr *ServiceRegistryStorage) Update(obj interface{}) (<-chan interface{}, e if srv.ID == "" { return nil, fmt.Errorf("ID should not be empty: %#v", srv) } + if errs := api.ValidateService(&srv); len(errs) > 0 { + return nil, fmt.Errorf("Validation errors: %v", errs) + } return apiserver.MakeAsync(func() (interface{}, error) { // TODO: check to see if external load balancer status changed err := sr.registry.UpdateService(srv) diff --git a/pkg/registry/service_registry_test.go b/pkg/registry/service_registry_test.go index 67f7d74e348..1754d982827 100644 --- a/pkg/registry/service_registry_test.go +++ b/pkg/registry/service_registry_test.go @@ -49,6 +49,60 @@ func TestServiceRegistry(t *testing.T) { } } +func TestServiceStorageValidatesCreate(t *testing.T) { + memory := MakeMemoryRegistry() + storage := MakeServiceRegistryStorage(memory, nil, nil) + + failureCases := map[string]api.Service{ + "empty ID": api.Service{ + JSONBase: api.JSONBase{ID: ""}, + Selector: map[string]string{"bar": "baz"}, + }, + "empty selector": api.Service{ + JSONBase: api.JSONBase{ID: "foo"}, + Selector: map[string]string{}, + }, + } + for _, failureCase := range failureCases { + c, err := storage.Create(failureCase) + if c != nil { + t.Errorf("Expected nil channel") + } + if err == nil { + t.Errorf("Expected to get an error") + } + } +} + +func TestServiceStorageValidatesUpdate(t *testing.T) { + memory := MakeMemoryRegistry() + memory.CreateService(api.Service{ + JSONBase: api.JSONBase{ID: "foo"}, + Selector: map[string]string{"bar": "baz"}, + }) + storage := MakeServiceRegistryStorage(memory, nil, nil) + + failureCases := map[string]api.Service{ + "empty ID": api.Service{ + JSONBase: api.JSONBase{ID: ""}, + Selector: map[string]string{"bar": "baz"}, + }, + "empty selector": api.Service{ + JSONBase: api.JSONBase{ID: "foo"}, + Selector: map[string]string{}, + }, + } + for _, failureCase := range failureCases { + c, err := storage.Update(failureCase) + if c != nil { + t.Errorf("Expected nil channel") + } + if err == nil { + t.Errorf("Expected to get an error") + } + } +} + func TestServiceRegistryExternalService(t *testing.T) { memory := MakeMemoryRegistry() fakeCloud := &cloudprovider.FakeCloud{}