Allow RESTStorage objects to not implement methods

pkg/api/errors/errors.go

@@ -159,6 +159,19 @@ func NewBadRequest(reason string) error {
 	}}
 }
 
+// NewMethodNotSupported returns an error indicating the requested action is not supported on this kind.
+func NewMethodNotSupported(kind, action string) error {
+	return &StatusError{api.Status{
+		Status: api.StatusFailure,
+		Code:   http.StatusMethodNotAllowed,
+		Reason: api.StatusReasonMethodNotAllowed,
+		Details: &api.StatusDetails{
+			Kind: kind,
+		},
+		Message: fmt.Sprintf("%s is not supported on resources of kind %q", action, kind),
+	}}
+}
+
 // NewInternalError returns an error indicating the item is invalid and cannot be processed.
 func NewInternalError(err error) error {
 	return &StatusError{api.Status{
@@ -192,6 +205,12 @@ func IsInvalid(err error) bool {
 	return reasonForError(err) == api.StatusReasonInvalid
 }
 
+// IsMethodNotSupported determines if the err is an error which indicates the provided action could not
+// be performed because it is not supported by the server.
+func IsMethodNotSupported(err error) bool {
+	return reasonForError(err) == api.StatusReasonMethodNotAllowed
+}
+
 // IsBadRequest determines if err is an error which indicates that the request is invalid.
 func IsBadRequest(err error) bool {
 	return reasonForError(err) == api.StatusReasonBadRequest

pkg/api/errors/errors_test.go

@@ -40,6 +40,12 @@ func TestErrorNew(t *testing.T) {
 	if IsInvalid(err) {
 		t.Errorf("expected to not be %s", api.StatusReasonInvalid)
 	}
+	if IsBadRequest(err) {
+		t.Errorf("expected to not be %s", api.StatusReasonBadRequest)
+	}
+	if IsMethodNotSupported(err) {
+		t.Errorf("expected to not be %s", api.StatusReasonMethodNotAllowed)
+	}
 
 	if !IsConflict(NewConflict("test", "2", errors.New("message"))) {
 		t.Errorf("expected to be conflict")
@@ -50,6 +56,12 @@ func TestErrorNew(t *testing.T) {
 	if !IsInvalid(NewInvalid("test", "2", nil)) {
 		t.Errorf("expected to be %s", api.StatusReasonInvalid)
 	}
+	if !IsBadRequest(NewBadRequest("reason")) {
+		t.Errorf("expected to be %s", api.StatusReasonBadRequest)
+	}
+	if !IsMethodNotSupported(NewMethodNotSupported("foo", "delete")) {
+		t.Errorf("expected to be %s", api.StatusReasonMethodNotAllowed)
+	}
 }
 
 func TestNewInvalid(t *testing.T) {

pkg/api/types.go

@@ -931,6 +931,11 @@ const (
 	// data was invalid.  API calls that return BadRequest can never succeed.
 	StatusReasonBadRequest StatusReason = "BadRequest"
 
+	// StatusReasonMethodNotAllowed means that the action the client attempted to perform on the
+	// resource was not supported by the code - for instance, attempting to delete a resource that
+	// can only be created. API calls that return MethodNotAllowed can never succeed.
+	StatusReasonMethodNotAllowed StatusReason = "MethodNotAllowed"
+
 	// StatusReasonInternalError indicates that an internal error occurred, it is unexpected
 	// and the outcome of the call is unknown.
 	// Details (optional):

pkg/apiserver/apiserver.go

@@ -129,62 +129,70 @@ func registerResourceHandlers(ws *restful.WebService, version string, path strin
 	nameParam := ws.PathParameter("name", "name of the "+kind).DataType("string")
 	namespaceParam := ws.PathParameter("namespace", "object name and auth scope, such as for teams and projects").DataType("string")
 
-	ws.Route(
-		addParamIf(
-			ws.POST(path).To(h).
-				Doc("create a "+kind).
-				Operation("create"+kind).
-				Reads(versionedObject), // from the request
-			namespaceParam, namespaceScope))
-
-	// TODO: This seems like a hack. Add NewList() to storage?
-	listKind := kind + "List"
-	if _, ok := kinds[listKind]; !ok {
-		glog.V(1).Infof("no list type: %v\n", listKind)
+	createRoute := ws.POST(path).To(h).
+		Doc("create a " + kind).
+		Operation("create" + kind).
+		Reads(versionedObject) // from the request
+	addParamIf(createRoute, namespaceParam, namespaceScope)
+	if _, ok := storage.(RESTCreater); ok {
+		ws.Route(createRoute.Reads(versionedObject)) // from the request
 	} else {
+		ws.Route(createRoute.Returns(http.StatusMethodNotAllowed, "creating objects is not supported", nil))
+	}
+
+	listRoute := ws.GET(path).To(h).
+		Doc("list objects of kind " + kind).
+		Operation("list" + kind)
+	addParamIf(listRoute, namespaceParam, namespaceScope)
+	if lister, ok := storage.(RESTLister); ok {
+		list := lister.NewList()
+		_, listKind, err := api.Scheme.ObjectVersionAndKind(list)
 		versionedListPtr, err := api.Scheme.New(version, listKind)
 		if err != nil {
-			glog.Errorf("error making list: %v\n", err)
-		} else {
-			versionedList := indirectArbitraryPointer(versionedListPtr)
-			glog.V(3).Infoln("type: ", reflect.TypeOf(versionedList))
-			ws.Route(
-				addParamIf(
-					ws.GET(path).To(h).
-						Doc("list objects of kind "+kind).
-						Operation("list"+kind).
-						Returns(http.StatusOK, "OK", versionedList),
-					namespaceParam, namespaceScope))
+			glog.Errorf("error making list object: %v\n", err)
+			return
 		}
+		versionedList := indirectArbitraryPointer(versionedListPtr)
+		glog.V(3).Infoln("type: ", reflect.TypeOf(versionedList))
+		ws.Route(listRoute.Returns(http.StatusOK, "OK", versionedList))
+	} else {
+		ws.Route(listRoute.Returns(http.StatusMethodNotAllowed, "listing objects is not supported", nil))
 	}
 
-	ws.Route(
-		addParamIf(
-			ws.GET(path+"/{name}").To(h).
-				Doc("read the specified "+kind).
-				Operation("read"+kind).
-				Param(nameParam).
-				Writes(versionedObject), // on the response
-			namespaceParam, namespaceScope))
+	getRoute := ws.GET(path + "/{name}").To(h).
+		Doc("read the specified " + kind).
+		Operation("read" + kind).
+		Param(nameParam)
+	addParamIf(getRoute, namespaceParam, namespaceScope)
+	if _, ok := storage.(RESTGetter); ok {
+		ws.Route(getRoute.Writes(versionedObject)) // on the response
+	} else {
+		ws.Route(ws.GET(path+"/{name}").To(h).
+			Returns(http.StatusMethodNotAllowed, "reading individual objects is not supported", nil))
+	}
 
-	ws.Route(
-		addParamIf(
-			ws.PUT(path+"/{name}").To(h).
-				Doc("update the specified "+kind).
-				Operation("update"+kind).
-				Param(nameParam).
-				Reads(versionedObject), // from the request
-			namespaceParam, namespaceScope))
+	updateRoute := ws.PUT(path + "/{name}").To(h).
+		Doc("update the specified " + kind).
+		Operation("update" + kind).
+		Param(nameParam)
+	addParamIf(updateRoute, namespaceParam, namespaceScope)
+	if _, ok := storage.(RESTUpdater); ok {
+		ws.Route(updateRoute.Reads(versionedObject)) // from the request
+	} else {
+		ws.Route(updateRoute.Returns(http.StatusMethodNotAllowed, "updating objects is not supported", nil))
+	}
 
 	// TODO: Support PATCH
-
-	ws.Route(
-		addParamIf(
-			ws.DELETE(path+"/{name}").To(h).
-				Doc("delete the specified "+kind).
-				Operation("delete"+kind).
-				Param(nameParam),
-			namespaceParam, namespaceScope))
+	deleteRoute := ws.DELETE(path + "/{name}").To(h).
+		Doc("delete the specified " + kind).
+		Operation("delete" + kind).
+		Param(nameParam)
+	addParamIf(deleteRoute, namespaceParam, namespaceScope)
+	if _, ok := storage.(RESTDeleter); ok {
+		ws.Route(deleteRoute)
+	} else {
+		ws.Route(deleteRoute.Returns(http.StatusMethodNotAllowed, "deleting objects is not supported", nil))
+	}
 }
 
 // Adds the given param to the given route builder if shouldAdd is true. Does nothing if shouldAdd is false.

pkg/apiserver/apiserver_test.go

@@ -183,6 +183,10 @@ func (storage *SimpleRESTStorage) New() runtime.Object {
 	return &Simple{}
 }
 
+func (storage *SimpleRESTStorage) NewList() runtime.Object {
+	return &SimpleList{}
+}
+
 func (storage *SimpleRESTStorage) Create(ctx api.Context, obj runtime.Object) (<-chan RESTResult, error) {
 	storage.created = obj.(*Simple)
 	if err := storage.errors["create"]; err != nil {
@@ -288,6 +292,68 @@ func TestNotFound(t *testing.T) {
 	}
 }
 
+type UnimplementedRESTStorage struct{}
+
+func (UnimplementedRESTStorage) New() runtime.Object {
+	return &Simple{}
+}
+
+func TestMethodNotAllowed(t *testing.T) {
+	type T struct {
+		Method string
+		Path   string
+	}
+	cases := map[string]T{
+		"GET object":    {"GET", "/prefix/version/foo/bar"},
+		"GET list":      {"GET", "/prefix/version/foo"},
+		"POST list":     {"POST", "/prefix/version/foo"},
+		"PUT object":    {"PUT", "/prefix/version/foo/bar"},
+		"DELETE object": {"DELETE", "/prefix/version/foo/bar"},
+		//"watch list":      {"GET", "/prefix/version/watch/foo"},
+		//"watch object":    {"GET", "/prefix/version/watch/foo/bar"},
+		"proxy object":    {"GET", "/prefix/version/proxy/foo/bar"},
+		"redirect object": {"GET", "/prefix/version/redirect/foo/bar"},
+	}
+	handler := Handle(map[string]RESTStorage{
+		"foo": UnimplementedRESTStorage{},
+	}, codec, "/prefix", testVersion, selfLinker, admissionControl)
+	server := httptest.NewServer(handler)
+	defer server.Close()
+	client := http.Client{}
+	for k, v := range cases {
+		request, err := http.NewRequest(v.Method, server.URL+v.Path, bytes.NewReader([]byte(`{"kind":"Simple","apiVersion":"version"}`)))
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		response, err := client.Do(request)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+			continue
+		}
+		defer response.Body.Close()
+		data, _ := ioutil.ReadAll(response.Body)
+		t.Logf("resp: %s", string(data))
+		if response.StatusCode != http.StatusMethodNotAllowed {
+			t.Errorf("%s: expected %d for %s, Got %s", k, http.StatusMethodNotAllowed, v.Method, string(data))
+			continue
+		}
+		obj, err := codec.Decode(data)
+		if err != nil {
+			t.Errorf("%s: unexpected decode error: %v", k, err)
+			continue
+		}
+		status, ok := obj.(*api.Status)
+		if !ok {
+			t.Errorf("%s: unexpected object: %#v", k, obj)
+			continue
+		}
+		if status.Reason != api.StatusReasonMethodNotAllowed {
+			t.Errorf("%s: unexpected status: %#v", k, status)
+		}
+	}
+}
+
 func TestVersion(t *testing.T) {
 	handler := Handle(map[string]RESTStorage{}, codec, "/prefix", testVersion, selfLinker, admissionControl)
 	server := httptest.NewServer(handler)

pkg/apiserver/interfaces.go

@@ -24,28 +24,43 @@ import (
 )
 
 // RESTStorage is a generic interface for RESTful storage services.
-// Resources which are exported to the RESTful API of apiserver need to implement this interface.
+// Resources which are exported to the RESTful API of apiserver need to implement this interface. It is expected
+// that objects may implement any of the REST* interfaces.
+// TODO: implement dynamic introspection (so GenericREST objects can indicate what they implement)
 type RESTStorage interface {
 	// New returns an empty object that can be used with Create and Update after request data has been put into it.
 	// This object must be a pointer type for use with Codec.DecodeInto([]byte, runtime.Object)
 	New() runtime.Object
+}
 
+type RESTLister interface {
+	// NewList returns an empty object that can be used with the List call.
+	// This object must be a pointer type for use with Codec.DecodeInto([]byte, runtime.Object)
+	NewList() runtime.Object
 	// List selects resources in the storage which match to the selector.
 	List(ctx api.Context, label, field labels.Selector) (runtime.Object, error)
+}
 
+type RESTGetter interface {
 	// Get finds a resource in the storage by id and returns it.
 	// Although it can return an arbitrary error value, IsNotFound(err) is true for the
 	// returned error value err when the specified resource is not found.
 	Get(ctx api.Context, id string) (runtime.Object, error)
+}
 
+type RESTDeleter interface {
 	// Delete finds a resource in the storage and deletes it.
 	// Although it can return an arbitrary error value, IsNotFound(err) is true for the
 	// returned error value err when the specified resource is not found.
 	Delete(ctx api.Context, id string) (<-chan RESTResult, error)
+}
 
+type RESTCreater interface {
 	// Create creates a new version of a resource.
 	Create(ctx api.Context, obj runtime.Object) (<-chan RESTResult, error)
+}
 
+type RESTUpdater interface {
 	// Update finds a resource in the storage and updates it. Some implementations
 	// may allow updates creates the object - they should set the Created flag of
 	// the returned RESTResultto true. In the event of an asynchronous error returned

pkg/apiserver/proxy.go

@@ -30,6 +30,7 @@ import (
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/httplog"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -106,7 +107,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	redirector, ok := storage.(Redirector)
 	if !ok {
 		httplog.LogOf(req, w).Addf("'%v' is not a redirector", kind)
-		notFound(w, req)
+		errorJSON(errors.NewMethodNotSupported(kind, "proxy"), r.codec, w)
 		return
 	}
 

pkg/apiserver/redirect.go

@@ -20,6 +20,7 @@ import (
 	"net/http"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/httplog"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 )
@@ -53,7 +54,7 @@ func (r *RedirectHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	redirector, ok := storage.(Redirector)
 	if !ok {
 		httplog.LogOf(req, w).Addf("'%v' is not a redirector", kind)
-		notFound(w, req)
+		errorJSON(errors.NewMethodNotSupported(kind, "redirect"), r.codec, w)
 		return
 	}