diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/BUILD b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/BUILD index ae1e06cc57e5a..6008b387dce93 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/BUILD @@ -14,6 +14,7 @@ go_library( importmap = "k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/serializer/protobuf", importpath = "k8s.io/apimachinery/pkg/runtime/serializer/protobuf", deps = [ + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/recognizer:go_default_library", diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go index 8d4ea7118099e..b99ba25c8cc43 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go @@ -20,10 +20,12 @@ import ( "bytes" "fmt" "io" + "net/http" "reflect" "github.com/gogo/protobuf/proto" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer/recognizer" @@ -50,6 +52,15 @@ func (e errNotMarshalable) Error() string { return fmt.Sprintf("object %v does not implement the protobuf marshalling interface and cannot be encoded to a protobuf message", e.t) } +func (e errNotMarshalable) Status() metav1.Status { + return metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusNotAcceptable, + Reason: metav1.StatusReason("NotAcceptable"), + Message: e.Error(), + } +} + func IsNotMarshalable(err error) bool { _, ok := err.(errNotMarshalable) return err != nil && ok diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 69dc93c2614a3..52738ce5417d6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -3667,10 +3667,12 @@ func TestWriteJSONDecodeError(t *testing.T) { responsewriters.WriteObjectNegotiated(codecs, newGroupVersion, w, req, http.StatusOK, &UnregisteredAPIObject{"Undecodable"}) })) defer server.Close() - // We send a 200 status code before we encode the object, so we expect OK, but there will - // still be an error object. This seems ok, the alternative is to validate the object before - // encoding, but this really should never happen, so it's wasted compute for every API request. - status := expectApiStatus(t, "GET", server.URL, nil, http.StatusOK) + // Decode error response behavior is dictated by + // apiserver/pkg/endpoints/handlers/responsewriters/status.go::ErrorToAPIStatus(). + // Unless specific metav1.Status() parameters are implemented for the particular error in question, such that + // the status code is defined, metav1 errors where error.status == metav1.StatusFailure + // will throw a '500 Internal Server Error'. Non-metav1 type errors will always throw a '500 Internal Server Error'. + status := expectApiStatus(t, "GET", server.URL, nil, http.StatusInternalServerError) if status.Reason != metav1.StatusReasonUnknown { t.Errorf("unexpected reason %#v", status) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go index c3d3728c568a5..fd335b5d7e57e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go @@ -35,6 +35,26 @@ import ( "k8s.io/apiserver/pkg/util/wsstream" ) +// httpResponseWriterWithInit wraps http.ResponseWriter, and implements the io.Writer interface to be used +// with encoding. The purpose is to allow for encoding to a stream, while accommodating a custom HTTP status code +// if encoding fails, and meeting the encoder's io.Writer interface requirement. +type httpResponseWriterWithInit struct { + hasWritten bool + mediaType string + statusCode int + innerW http.ResponseWriter +} + +func (w httpResponseWriterWithInit) Write(b []byte) (n int, err error) { + if !w.hasWritten { + w.innerW.Header().Set("Content-Type", w.mediaType) + w.innerW.WriteHeader(w.statusCode) + w.hasWritten = true + } + + return w.innerW.Write(b) +} + // WriteObject renders a returned runtime.Object to the response as a stream or an encoded object. If the object // returned by the response implements rest.ResourceStreamer that interface will be used to render the // response. The Accept header and current API version will be passed in, and the output will be copied @@ -90,12 +110,11 @@ func StreamObject(statusCode int, gv schema.GroupVersion, s runtime.NegotiatedSe // SerializeObject renders an object in the content type negotiated by the client using the provided encoder. // The context is optional and can be nil. -func SerializeObject(mediaType string, encoder runtime.Encoder, w http.ResponseWriter, req *http.Request, statusCode int, object runtime.Object) { - w.Header().Set("Content-Type", mediaType) - w.WriteHeader(statusCode) +func SerializeObject(mediaType string, encoder runtime.Encoder, innerW http.ResponseWriter, req *http.Request, statusCode int, object runtime.Object) { + w := httpResponseWriterWithInit{mediaType: mediaType, innerW: innerW, statusCode: statusCode} if err := encoder.Encode(object, w); err != nil { - errorJSONFatal(err, encoder, w) + errSerializationFatal(err, encoder, w) } } @@ -143,22 +162,23 @@ func ErrorNegotiated(err error, s runtime.NegotiatedSerializer, gv schema.GroupV return code } -// errorJSONFatal renders an error to the response, and if codec fails will render plaintext. +// errSerializationFatal renders an error to the response, and if codec fails will render plaintext. // Returns the HTTP status code of the error. -func errorJSONFatal(err error, codec runtime.Encoder, w http.ResponseWriter) int { +func errSerializationFatal(err error, codec runtime.Encoder, w httpResponseWriterWithInit) { utilruntime.HandleError(fmt.Errorf("apiserver was unable to write a JSON response: %v", err)) status := ErrorToAPIStatus(err) - code := int(status.Code) + candidateStatusCode := int(status.Code) + // If original statusCode was not successful, we need to return the original error. + // We cannot hide it behind serialization problems + if w.statusCode >= http.StatusOK && w.statusCode < http.StatusBadRequest { + w.statusCode = candidateStatusCode + } output, err := runtime.Encode(codec, status) if err != nil { - w.WriteHeader(code) - fmt.Fprintf(w, "%s: %s", status.Reason, status.Message) - return code + w.mediaType = "text/plain" + output = []byte(fmt.Sprintf("%s: %s", status.Reason, status.Message)) } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(code) w.Write(output) - return code } // WriteRawJSON writes a non-API object in JSON.