Merge pull request kubernetes#2124 from brendandburns/fix

Make endpoints return 400 instead of 500
duda · Nov 3, 2014 · 81785d8 · 81785d8
2 parents c92e156 + 32a04e4
commit 81785d8
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 3 deletions.
diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go
@@ -116,6 +116,22 @@ func NewInvalid(kind, name string, errs ValidationErrorList) error {
 	}}
 }
 
+// NewBadRequest creates an error that indicates that the request is invalid and can not be processed.
+func NewBadRequest(reason string) error {
+	return &statusError{
+		api.Status{
+			Status: api.StatusFailure,
+			Code:   400,
+			Reason: api.StatusReasonBadRequest,
+			Details: &api.StatusDetails{
+				Causes: []api.StatusCause{
+					{Message: reason},
+				},
+			},
+		},
+	}
+}
+
 // IsNotFound returns true if the specified error was created by NewNotFoundErr.
 func IsNotFound(err error) bool {
 	return reasonForError(err) == api.StatusReasonNotFound
@@ -136,6 +152,11 @@ func IsInvalid(err error) bool {
 	return reasonForError(err) == api.StatusReasonInvalid
 }
 
+// IsBadRequest determines if err is an error which indicates that the request is invalid.
+func IsBadRequest(err error) bool {
+	return reasonForError(err) == api.StatusReasonBadRequest
+}
+
 func reasonForError(err error) api.StatusReason {
 	switch t := err.(type) {
 	case *statusError:

diff --git a/pkg/api/types.go b/pkg/api/types.go
@@ -680,6 +680,12 @@ const (
 	//                   field attributes will be set.
 	// Status code 422
 	StatusReasonInvalid StatusReason = "Invalid"
+
+	// StatusReasonBadRequest means that the request itself was invalid, because the request
+	// doesn't make any sense, for example deleting a read-only object.  This is different than
+	// StatusReasonInvalid above which indicates that the API call could possibly succeed, but the
+	// data was invalid.  API calls that return BadRequest can never succeed.
+	StatusReasonBadRequest StatusReason = "BadRequest"
 )
 
 // StatusCause provides more information about an api.Status failure, including

diff --git a/pkg/registry/endpoint/rest.go b/pkg/registry/endpoint/rest.go
@@ -17,10 +17,10 @@ limitations under the License.
 package endpoint
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
@@ -48,7 +48,7 @@ func (rs *REST) Get(ctx api.Context, id string) (runtime.Object, error) {
 // List satisfies the RESTStorage interface.
 func (rs *REST) List(ctx api.Context, label, field labels.Selector) (runtime.Object, error) {
 	if !label.Empty() || !field.Empty() {
-		return nil, errors.New("label/field selectors are not supported on endpoints")
+		return nil, errors.NewBadRequest("label/field selectors are not supported on endpoints")
 	}
 	return rs.registry.ListEndpoints(ctx)
 }
@@ -95,7 +95,7 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE
 
 // Delete satisfies the RESTStorage interface but is unimplemented.
 func (rs *REST) Delete(ctx api.Context, id string) (<-chan apiserver.RESTResult, error) {
-	return nil, errors.New("unimplemented")
+	return nil, errors.NewBadRequest("Endpoints are read-only")
 }
 
 // New implements the RESTStorage interface.

diff --git a/pkg/registry/endpoint/rest_test.go b/pkg/registry/endpoint/rest_test.go
@@ -96,3 +96,14 @@ func TestEndpointsRegistryList(t *testing.T) {
 		t.Errorf("Unexpected resource version: %#v", sl)
 	}
 }
+
+func TestEndpointsRegistryDelete(t *testing.T) {
+	registry := registrytest.NewServiceRegistry()
+	storage := NewREST(registry)
+	_, err := storage.Delete(api.NewContext(), "n/a")
+	if err == nil {
+		t.Error("unexpected non-error")
+	} else if !errors.IsBadRequest(err) {
+		t.Errorf("unexpected error: %v", err)
+	}
+}