Fix virt-api responses

Instead of proxying apiserver errors, virt-api translated them to
internal server errors.

pkg/middleware/middleware.go

@@ -7,8 +7,11 @@ import (
 	"golang.org/x/net/context"
 	"runtime/debug"
 
+	"encoding/json"
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/levels"
+	"k8s.io/client-go/pkg/apis/meta/v1"
+	"k8s.io/client-go/rest"
 	"kubevirt.io/kubevirt/pkg/precond"
 )
 
@@ -41,6 +44,44 @@ type ResourceNotFoundError struct{ appError } // Can be thrown before or by a se
 type PreconditionError struct{ appError }     // Precondition not met, most likely a bug in a service (service)
 type InternalServerError struct{ appError }   // Unknown internal error, most likely a bug in a service or a library
 
+type KubernetesError struct {
+	result rest.Result
+}
+
+func (k *KubernetesError) Cause() error {
+	return k.result.Error()
+}
+
+func (k *KubernetesError) Error() string {
+	return k.result.Error().Error()
+}
+
+func (k *KubernetesError) Status() (*v1.Status, error) {
+	b, _ := k.result.Raw()
+	status := v1.Status{}
+	err := json.Unmarshal(b, &status)
+	if err != nil {
+		return &status, nil
+	}
+	return nil, err
+}
+
+func (k *KubernetesError) StatusCode() int {
+	status, err := k.Status()
+	if err != nil {
+		return int(status.Code)
+	} else {
+		var s int
+		k.result.StatusCode(&s)
+		return s
+	}
+}
+
+func (k *KubernetesError) Body() []byte {
+	body, _ := k.result.Raw()
+	return body
+}
+
 // InternalErrorMiddleware is a convenience middleware which can be used in combination with panics.
 // After data is sanitized and validated, services can expect to get reasonable valid data passed (e.g.
 // object not nil, string not empty, ...). With this middleware in place the service can throw an exception with a
@@ -101,3 +142,7 @@ func NewResourceConflictError(msg string) *ResourceNotFoundError {
 func NewInternalServerError(err error) *InternalServerError {
 	return &InternalServerError{appError{err: err}}
 }
+
+func NewKubernetesError(result rest.Result) *KubernetesError {
+	return &KubernetesError{result: result}
+}

pkg/rest/endpoints/encoders.go

@@ -3,40 +3,47 @@ package endpoints
 import (
 	"encoding/json"
 	"golang.org/x/net/context"
+	"kubevirt.io/kubevirt/pkg/logging"
 	"kubevirt.io/kubevirt/pkg/middleware"
 	"net/http"
+	"reflect"
 )
 
 func encodeApplicationErrors(_ context.Context, w http.ResponseWriter, response interface{}) error {
 	w.Header().Set("Content-Type", "text/plain")
+	var err error
 	switch t := response.(type) {
 	// More specific AppErrors  like 404 must be handled before the AppError case
-	case middleware.ResourceNotFoundError:
+	case *middleware.KubernetesError:
+		w.WriteHeader(t.StatusCode())
+		_, err = w.Write(t.Body())
+	case *middleware.ResourceNotFoundError:
 		w.WriteHeader(http.StatusNotFound)
-		w.Write([]byte(t.Cause().Error()))
+		_, err = w.Write([]byte(t.Cause().Error()))
 	case middleware.ResourceExistsError:
 		w.WriteHeader(http.StatusConflict)
-		w.Write([]byte(t.Cause().Error()))
+		_, err = w.Write([]byte(t.Cause().Error()))
 	case middleware.AppError:
 		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte(t.Cause().Error()))
+		_, err = w.Write([]byte(t.Cause().Error()))
 	default:
 		w.WriteHeader(http.StatusInternalServerError)
 		// TODO log the error but don't send it along
-		w.Write([]byte("Error handling failed, that should never happen."))
+		_, err = w.Write([]byte("Error handling failed, that should never happen."))
 	}
-	return json.NewEncoder(w).Encode(response)
+	return err
 }
 
 func EncodePostResponse(context context.Context, w http.ResponseWriter, response interface{}) error {
-	if _, ok := response.(middleware.AppError); ok != false {
+	logging.DefaultLogger().Info().Msg(reflect.TypeOf(response).Name())
+	if _, ok := response.(middleware.AppError); ok {
 		return encodeApplicationErrors(context, w, response)
 	}
 	return encodeJsonResponse(w, response, http.StatusCreated)
 }
 
 func EncodeGetResponse(context context.Context, w http.ResponseWriter, response interface{}) error {
-	if _, ok := response.(middleware.AppError); ok != false {
+	if _, ok := response.(middleware.AppError); ok {
 		return encodeApplicationErrors(context, w, response)
 	}
 	return encodeJsonResponse(w, response, http.StatusOK)

pkg/virt-api/rest/kubeproxy.go

@@ -15,7 +15,7 @@ import (
 	"reflect"
 )
 
-type ResponseHandlerFunc func(rest.Result) (runtime.Object, error)
+type ResponseHandlerFunc func(rest.Result) (interface{}, error)
 
 func AddGenericResourceProxy(ws *restful.WebService, ctx context.Context, gvr schema.GroupVersionResource, ptr runtime.Object, response ResponseHandlerFunc) error {
 	cli, err := kubecli.GetRESTClient()
@@ -76,17 +76,14 @@ func NewGenericGetEndpoint(cli *rest.RESTClient, gvr schema.GroupVersionResource
 
 //FIXME this is basically one big workaround because version and kind are not filled by the restclient
 func NewResponseHandler(gvk schema.GroupVersionKind, ptr runtime.Object) ResponseHandlerFunc {
-	return func(result rest.Result) (runtime.Object, error) {
-		if result.Error() != nil {
-			return nil, middleware.NewInternalServerError(result.Error())
-		}
+	return func(result rest.Result) (interface{}, error) {
 		obj, err := result.Get()
+		if err != nil {
+			return middleware.NewKubernetesError(result), nil
+		}
 		if reflect.TypeOf(obj).Elem() == reflect.TypeOf(ptr).Elem() {
 			obj.(runtime.Object).GetObjectKind().SetGroupVersionKind(gvk)
 		}
-		if err != nil {
-			return nil, middleware.NewInternalServerError(result.Error())
-		}
 		return obj, nil
 
 	}

tests/vmlifecycle_test.go

@@ -6,9 +6,12 @@ import (
 	. "github.com/onsi/gomega"
 	"k8s.io/client-go/pkg/api"
 	kubev1 "k8s.io/client-go/pkg/api/v1"
+	metav1 "k8s.io/client-go/pkg/apis/meta/v1"
+	"k8s.io/client-go/pkg/util/json"
 	"kubevirt.io/kubevirt/pkg/api/v1"
 	"kubevirt.io/kubevirt/pkg/kubecli"
 	"kubevirt.io/kubevirt/tests"
+	"net/http"
 	"strings"
 )
 
@@ -32,6 +35,26 @@ var _ = Describe("Vmlifecycle", func() {
 			Expect(err).To(BeNil())
 		})
 
+		It("Should reject posting the same VM a second time", func() {
+			err := restClient.Post().Resource("vms").Namespace(api.NamespaceDefault).Body(vm).Do().Error()
+			Expect(err).To(BeNil())
+			b, err := restClient.Post().Resource("vms").Namespace(api.NamespaceDefault).Body(vm).DoRaw()
+			Expect(err).ToNot(BeNil())
+			status := metav1.Status{}
+			err = json.Unmarshal(b, &status)
+			Expect(err).To(BeNil())
+			Expect(status.Code).To(Equal(int32(http.StatusConflict)))
+		})
+
+		It("Should return 404 if VM does not exist", func() {
+			b, err := restClient.Get().Resource("vms").Namespace(api.NamespaceDefault).Name("nonexistnt").DoRaw()
+			Expect(err).ToNot(BeNil())
+			status := metav1.Status{}
+			err = json.Unmarshal(b, &status)
+			Expect(err).To(BeNil())
+			Expect(status.Code).To(Equal(int32(http.StatusNotFound)))
+		})
+
 		It("Should start the VM on POST", func(done Done) {
 			result := restClient.Post().Resource("vms").Namespace(api.NamespaceDefault).Body(vm).Do()
 			Expect(result.Error()).To(BeNil())