Errors: Update errutil to be compatible with k8s errors (#87605)

pkg/registry/apis/query/query.go

@@ -21,7 +21,6 @@ import (
 	"github.com/grafana/grafana/pkg/expr/mathexp"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/datasources"
-	"github.com/grafana/grafana/pkg/util/errutil"
 	"github.com/grafana/grafana/pkg/web"
 )
 
@@ -105,14 +104,14 @@ func (r *queryREST) Connect(ctx context.Context, name string, opts runtime.Objec
 					Message: "datasource not found",
 				}}
 			}
-			responder.Error(convertToK8sError(err))
+			responder.Error(err)
 			return
 		}
 
 		// Actually run the query
 		rsp, err := b.execute(ctx, req)
 		if err != nil {
-			responder.Error(convertToK8sError(err))
+			responder.Error(err)
 			return
 		}
 
@@ -122,20 +121,6 @@ func (r *queryREST) Connect(ctx context.Context, name string, opts runtime.Objec
 	}), nil
 }
 
-// Would be really nice if errutil was directly k8s compatible :(
-func convertToK8sError(err error) error {
-	var gErr errutil.Error
-	if errors.As(err, &gErr) {
-		return &errorsK8s.StatusError{ErrStatus: metav1.Status{
-			Status:  metav1.StatusFailure,
-			Code:    int32(gErr.Reason.Status().HTTPStatus()),
-			Reason:  metav1.StatusReason(gErr.Reason.Status()), // almost true
-			Message: gErr.PublicMessage,
-		}}
-	}
-	return err
-}
-
 func (b *QueryAPIBuilder) execute(ctx context.Context, req parsedRequestInfo) (qdr *backend.QueryDataResponse, err error) {
 	switch len(req.Requests) {
 	case 0:

pkg/tests/apis/query/query_test.go

@@ -144,7 +144,8 @@ func TestIntegrationSimpleQuery(t *testing.T) {
 			"metadata": {},
 			"status": "Failure",
 			"message": "did not execute expression [Y] due to a failure to of the dependent expression or query [X]",
-			"reason": "Bad request",
+			"reason": "BadRequest",
+			"details": { "uid": "sse.dependencyError" },
 			"code": 400
 		  }`, string(body))
 		// require.JSONEq(t, `{

pkg/util/errutil/errhttp/writer.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"reflect"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/endpoints/request"
 
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -23,14 +22,6 @@ type ErrorOptions struct {
 	logger   log.Logger
 }
 
-type k8sError struct {
-	metav1.Status `json:",inline"`
-
-	// Internal values that do not have a clean home in the standard Status object
-	MessageID string         `json:"messageId"`
-	Extra     map[string]any `json:"extra,omitempty"`
-}
-
 // Write writes an error to the provided [http.ResponseWriter] with the
 // appropriate HTTP status and JSON payload from [errutil.Error].
 // Write also logs the provided error to either the "request-errors"
@@ -63,37 +54,9 @@ func Write(ctx context.Context, err error, w http.ResponseWriter, opts ...func(E
 	// When running in k8s, this will return a v1 status
 	// Typically, k8s handlers should directly support error negotiation, however
 	// when implementing handlers directly this will maintain compatibility with client-go
-	info, ok := request.RequestInfoFrom(ctx)
+	_, ok := request.RequestInfoFrom(ctx)
 	if ok {
-		status := &k8sError{
+		rsp = gErr.Status()
-			Status: metav1.Status{
-				Status:  metav1.StatusFailure,
-				Code:    int32(pub.StatusCode),
-				Message: pub.Message,
-				Details: &metav1.StatusDetails{
-					Name:  info.Name,
-					Group: info.APIGroup,
-				},
-			},
-			// Add the internal values into
-			MessageID: pub.MessageID,
-			Extra:     pub.Extra,
-		}
-		switch pub.StatusCode {
-		case 400:
-			status.Reason = metav1.StatusReasonBadRequest
-		case 401:
-			status.Reason = metav1.StatusReasonUnauthorized
-		case 403:
-			status.Reason = metav1.StatusReasonForbidden
-		case 404:
-			status.Reason = metav1.StatusReasonNotFound
-		case 500: // many reasons things could map here
-			status.Reason = metav1.StatusReasonInternalError
-		case 504:
-			status.Reason = metav1.StatusReasonTimeout
-		}
-		rsp = status
 	}
 
 	err = json.NewEncoder(w).Encode(rsp)

pkg/util/errutil/errhttp/writer_test.go

@@ -27,9 +27,8 @@ func TestWrite(t *testing.T) {
 		"status": "Failure",
 		"reason": "Timeout",
 		"metadata": {},
-		"messageId": "test.thisIsExpected",
 		"message": "Timeout",
-		"details": { "group": "TestGroup" },
+		"details": { "uid": "test.thisIsExpected" },
 		"code": 504
 	  }`, recorder.Body.String())
 }

pkg/util/errutil/errors.go

@@ -1,8 +1,13 @@
 package errutil
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
+
+	errorsK8s "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 // Base represents the static information about a specific error.
@@ -294,6 +299,9 @@ func (b Base) Is(err error) bool {
 	}
 }
 
+// Allow errorutil errors to be returned as informative k8s errors
+var _ = errorsK8s.APIStatus(&Error{})
+
 // Error is the error type for errors within Grafana, extending
 // the Go error type with Grafana specific metadata to reduce
 // boilerplate error handling for status codes and internationalization
@@ -365,6 +373,34 @@ func (e Error) Error() string {
 	return fmt.Sprintf("[%s] %s", e.MessageID, e.LogMessage)
 }
 
+// When the error is rendered by an apiserver, this format is used
+func (e Error) Status() metav1.Status {
+	public := e.Public()
+	s := metav1.Status{
+		Status:  metav1.StatusFailure,
+		Code:    int32(public.StatusCode),
+		Reason:  metav1.StatusReason(e.Reason.Status()), // almost true
+		Message: public.Message,
+	}
+
+	// Shove the extra data into details
+	if public.Extra != nil || public.MessageID != "" {
+		s.Details = &metav1.StatusDetails{
+			UID: types.UID(public.MessageID),
+		}
+		for k, v := range public.Extra {
+			v, err := json.Marshal(v)
+			if err != nil {
+				s.Details.Causes = append(s.Details.Causes, metav1.StatusCause{
+					Field:   k,
+					Message: string(v),
+				})
+			}
+		}
+	}
+	return s
+}
+
 // Unwrap is used by errors.As to iterate over the sequence of
 // underlying errors until a matching type is found.
 func (e Error) Unwrap() error {

pkg/util/errutil/status.go

@@ -1,6 +1,10 @@
 package errutil
 
-import "net/http"
+import (
+	"net/http"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
 
 const (
 	// StatusUnknown implies an error that should be updated to contain
@@ -11,15 +15,15 @@ const (
 	// client's authentication, either because it has not been provided
 	// or is invalid for the operation.
 	// HTTP status code 401.
-	StatusUnauthorized CoreStatus = "Unauthorized"
+	StatusUnauthorized CoreStatus = CoreStatus(metav1.StatusReasonUnauthorized)
 	// StatusForbidden means that the server refuses to perform the
 	// requested action for the authenticated uer.
 	// HTTP status code 403.
-	StatusForbidden CoreStatus = "Forbidden"
+	StatusForbidden CoreStatus = CoreStatus(metav1.StatusReasonForbidden)
 	// StatusNotFound means that the server does not have any
 	// corresponding document to return to the request.
 	// HTTP status code 404.
-	StatusNotFound CoreStatus = "Not found"
+	StatusNotFound CoreStatus = CoreStatus(metav1.StatusReasonNotFound)
 	// StatusUnprocessableEntity means that the server understands the request,
 	// the content type and the syntax but it was unable to process the
 	// contained instructions.
@@ -28,15 +32,15 @@ const (
 	// StatusConflict means that the server cannot fulfill the request
 	// there is a conflict in the current state of a resource
 	// HTTP status code 409.
-	StatusConflict CoreStatus = "Conflict"
+	StatusConflict CoreStatus = CoreStatus(metav1.StatusReasonConflict)
 	// StatusTooManyRequests means that the client is rate limited
 	// by the server and should back-off before trying again.
 	// HTTP status code 429.
-	StatusTooManyRequests CoreStatus = "Too many requests"
+	StatusTooManyRequests CoreStatus = CoreStatus(metav1.StatusReasonTooManyRequests)
 	// StatusBadRequest means that the server was unable to parse the
 	// parameters or payload for the request.
 	// HTTP status code 400.
-	StatusBadRequest CoreStatus = "Bad request"
+	StatusBadRequest CoreStatus = CoreStatus(metav1.StatusReasonBadRequest)
 	// StatusClientClosedRequest means that a client closes the connection
 	// while the server is processing the request.
 	//
@@ -52,11 +56,11 @@ const (
 	// StatusInternal means that the server acknowledges that there's
 	// an error, but that there is nothing the client can do to fix it.
 	// HTTP status code 500.
-	StatusInternal CoreStatus = "Internal server error"
+	StatusInternal CoreStatus = CoreStatus(metav1.StatusReasonInternalError)
 	// StatusTimeout means that the server did not complete the request
 	// within the required time and aborted the action.
 	// HTTP status code 504.
-	StatusTimeout CoreStatus = "Timeout"
+	StatusTimeout CoreStatus = CoreStatus(metav1.StatusReasonTimeout)
 	// StatusNotImplemented means that the server does not support the
 	// requested action. Typically used during development of new
 	// features.
@@ -83,7 +87,7 @@ type StatusReason interface {
 	Status() CoreStatus
 }
 
-type CoreStatus string
+type CoreStatus metav1.StatusReason
 
 // Status implements the StatusReason interface.
 func (s CoreStatus) Status() CoreStatus {