diff --git a/pkg/api/validation.go b/pkg/api/validation.go index ecef6e88985..3da4bdeda5b 100644 --- a/pkg/api/validation.go +++ b/pkg/api/validation.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/golang/glog" ) @@ -286,8 +287,21 @@ func ValidateService(service *Service) []error { if service.ID == "" { allErrs.Append(makeInvalidError("Service.ID", service.ID)) } - if len(service.Selector) == 0 { + if labels.Set(service.Selector).AsSelector().Empty() { allErrs.Append(makeInvalidError("Service.Selector", service.Selector)) } return []error(allErrs) } + +// ValidateReplicationController tests if required fields in the replication controller are set. +func ValidateReplicationController(controller *ReplicationController) []error { + errors := []error{} + if controller.ID == "" { + errors = append(errors, makeInvalidError("ReplicationController.ID", controller.ID)) + } + if labels.Set(controller.DesiredState.ReplicaSelector).AsSelector().Empty() { + errors = append(errors, makeInvalidError("ReplicationController.ReplicaSelector", controller.DesiredState.ReplicaSelector)) + } + errors = append(errors, ValidateManifest(&controller.DesiredState.PodTemplate.DesiredState.Manifest)...) + return errors +} diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go index 6916db9da51..ec5f432d1d4 100644 --- a/pkg/api/validation_test.go +++ b/pkg/api/validation_test.go @@ -294,3 +294,63 @@ func TestValidateService(t *testing.T) { t.Errorf("Unexpected error list: %#v", errs) } } + +func TestValidateReplicationController(t *testing.T) { + validSelector := map[string]string{"a": "b"} + validPodTemplate := PodTemplate{ + DesiredState: PodState{ + Manifest: ContainerManifest{ + Version: "v1beta1", + }, + }, + } + + successCases := []ReplicationController{ + { + JSONBase: JSONBase{ID: "abc"}, + DesiredState: ReplicationControllerState{ + ReplicaSelector: validSelector, + PodTemplate: validPodTemplate, + }, + }, + { + JSONBase: JSONBase{ID: "abc-123"}, + DesiredState: ReplicationControllerState{ + ReplicaSelector: validSelector, + PodTemplate: validPodTemplate, + }, + }, + } + for _, successCase := range successCases { + if errs := ValidateReplicationController(&successCase); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + } + + errorCases := map[string]ReplicationController{ + "zero-length ID": { + JSONBase: JSONBase{ID: ""}, + DesiredState: ReplicationControllerState{ + ReplicaSelector: validSelector, + PodTemplate: validPodTemplate, + }, + }, + "empty selector": { + JSONBase: JSONBase{ID: "abc"}, + DesiredState: ReplicationControllerState{ + PodTemplate: validPodTemplate, + }, + }, + "invalid manifest": { + JSONBase: JSONBase{ID: "abc"}, + DesiredState: ReplicationControllerState{ + ReplicaSelector: validSelector, + }, + }, + } + for k, v := range errorCases { + if errs := ValidateReplicationController(&v); len(errs) == 0 { + t.Errorf("expected failure for %s", k) + } + } +} diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index 42adf008acd..eb614906d36 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -27,6 +27,9 @@ type Selector interface { // Matches returns true if this selector matches the given set of labels. Matches(Labels) bool + // Empty returns true if this selector does not restrict the selection space. + Empty() bool + // String returns a human readable string that represents this selector. String() string } @@ -44,6 +47,10 @@ func (t *hasTerm) Matches(ls Labels) bool { return ls.Get(t.label) == t.value } +func (t *hasTerm) Empty() bool { + return false +} + func (t *hasTerm) String() string { return fmt.Sprintf("%v=%v", t.label, t.value) } @@ -56,6 +63,10 @@ func (t *notHasTerm) Matches(ls Labels) bool { return ls.Get(t.label) != t.value } +func (t *notHasTerm) Empty() bool { + return false +} + func (t *notHasTerm) String() string { return fmt.Sprintf("%v!=%v", t.label, t.value) } @@ -71,6 +82,21 @@ func (t andTerm) Matches(ls Labels) bool { return true } +func (t andTerm) Empty() bool { + if t == nil { + return true + } + if len([]Selector(t)) == 0 { + return true + } + for i := range t { + if !t[i].Empty() { + return false + } + } + return true +} + func (t andTerm) String() string { var terms []string for _, q := range t { @@ -87,8 +113,12 @@ func try(selectorPiece, op string) (lhs, rhs string, ok bool) { return "", "", false } -// SelectorFromSet returns a Selector which will match exactly the given Set. +// SelectorFromSet returns a Selector which will match exactly the given Set. A +// nil Set is considered equivalent to Everything(). func SelectorFromSet(ls Set) Selector { + if ls == nil { + return Everything() + } items := make([]Selector, 0, len(ls)) for label, value := range ls { items = append(items, &hasTerm{label: label, value: value}) @@ -124,3 +154,12 @@ 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/labels/selector_test.go b/pkg/labels/selector_test.go index 30d6f265895..6c5b81296e3 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -84,6 +84,9 @@ func TestEverything(t *testing.T) { if !Everything().Matches(Set{"x": "y"}) { t.Errorf("Nil selector didn't match") } + if !Everything().Empty() { + t.Errorf("Everything was not empty") + } } func TestSelectorMatches(t *testing.T) { @@ -132,3 +135,31 @@ func TestSetMatches(t *testing.T) { expectNoMatchDirect(t, Set{"baz": "=bar"}, labelset) expectNoMatchDirect(t, Set{"foo": "=bar", "foobar": "bar", "baz": "blah"}, labelset) } + +func TestNilMapIsValid(t *testing.T) { + selector := Set(nil).AsSelector() + if selector == nil { + t.Errorf("Selector for nil set should be Everything") + } + if !selector.Empty() { + t.Errorf("Selector for nil set should be Empty") + } +} + +func TestSetIsEmpty(t *testing.T) { + if !(Set{}).AsSelector().Empty() { + t.Errorf("Empty set should be empty") + } + if !(andTerm(nil)).Empty() { + t.Errorf("Nil andTerm should be empty") + } + if (&hasTerm{}).Empty() { + t.Errorf("hasTerm should not be empty") + } + if !(andTerm{andTerm{}}).Empty() { + t.Errorf("Nested andTerm should be empty") + } + if (andTerm{&hasTerm{"a", "b"}}).Empty() { + t.Errorf("Nested andTerm should not be empty") + } +}