diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 2d033e31c10..e9f399b6653 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -492,3 +492,13 @@ func ValidateBoundPod(pod *api.BoundPod) (errors []error) { } return errors } + +// ValidateMinion tests if required fields in the minion are set. +func ValidateMinion(minion *api.Minion) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + if len(minion.Name) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("name", minion.Name)) + } + allErrs = append(allErrs, validateLabels(minion.Labels)...) + return allErrs +} diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index a82a05ca32c..ef48034a1ea 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1020,3 +1020,49 @@ func TestValidateBoundPodNoName(t *testing.T) { } } } + +func TestValidateMinion(t *testing.T) { + validSelector := map[string]string{"a": "b"} + invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} + successCases := []api.Minion{ + { + ObjectMeta: api.ObjectMeta{Name: "abc"}, + HostIP: "something", + Labels: validSelector, + }, + { + ObjectMeta: api.ObjectMeta{Name: "abc"}, + HostIP: "something", + }, + } + for _, successCase := range successCases { + if errs := ValidateMinion(&successCase); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + } + + errorCases := map[string]api.Minion{ + "zero-length Name": { + ObjectMeta: api.ObjectMeta{Name: ""}, + HostIP: "something", + Labels: validSelector, + }, + "invalid-labels": { + ObjectMeta: api.ObjectMeta{Name: "abc-123"}, + Labels: invalidSelector, + }, + } + for k, v := range errorCases { + errs := ValidateMinion(&v) + if len(errs) == 0 { + t.Errorf("expected failure for %s", k) + } + for i := range errs { + field := errs[i].(errors.ValidationError).Field + if field != "name" && + field != "label" { + t.Errorf("%s: missing prefix for: %v", k, errs[i]) + } + } + } +} diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index d4291b42d5f..7f668ba7187 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -23,6 +23,8 @@ import ( "strconv" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/master/ports" @@ -50,8 +52,9 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE if !ok { return nil, fmt.Errorf("not a minion: %#v", obj) } - if minion.Name == "" { - return nil, fmt.Errorf("ID should not be empty: %#v", minion) + + if errs := validation.ValidateMinion(minion); len(errs) > 0 { + return nil, kerrors.NewInvalid("minion", minion.Name, errs) } minion.CreationTimestamp = util.Now() diff --git a/pkg/registry/minion/rest_test.go b/pkg/registry/minion/rest_test.go index aa0141fcc12..bb94ec830ab 100644 --- a/pkg/registry/minion/rest_test.go +++ b/pkg/registry/minion/rest_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" ) @@ -114,3 +115,30 @@ func contains(nodes *api.MinionList, nodeID string) bool { } return false } + +func TestMinionStorageValidatesCreate(t *testing.T) { + storage := NewREST(registrytest.NewMinionRegistry([]string{"foo", "bar"}, api.NodeResources{})) + ctx := api.NewContext() + validSelector := map[string]string{"a": "b"} + invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} + failureCases := map[string]api.Minion{ + "zero-length Name": { + ObjectMeta: api.ObjectMeta{Name: ""}, + HostIP: "something", + Labels: validSelector, + }, + "invalid-labels": { + ObjectMeta: api.ObjectMeta{Name: "abc-123"}, + Labels: invalidSelector, + }, + } + for _, failureCase := range failureCases { + c, err := storage.Create(ctx, &failureCase) + if c != nil { + t.Errorf("Expected nil channel") + } + if !errors.IsInvalid(err) { + t.Errorf("Expected to get an invalid resource error, got %v", err) + } + } +}