diff --git a/pkg/registry/apis/query/query.go b/pkg/registry/apis/query/query.go index ecb42305caf..4eee1b82c9f 100644 --- a/pkg/registry/apis/query/query.go +++ b/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: diff --git a/pkg/tests/apis/query/query_test.go b/pkg/tests/apis/query/query_test.go index 738356a44a7..02e63b6ca02 100644 --- a/pkg/tests/apis/query/query_test.go +++ b/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, `{ diff --git a/pkg/util/errutil/errhttp/writer.go b/pkg/util/errutil/errhttp/writer.go index 9f0b24b918f..bbfcbb70df1 100644 --- a/pkg/util/errutil/errhttp/writer.go +++ b/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{ - 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 + rsp = gErr.Status() } err = json.NewEncoder(w).Encode(rsp) diff --git a/pkg/util/errutil/errhttp/writer_test.go b/pkg/util/errutil/errhttp/writer_test.go index d6c6427c88c..405055c5ce2 100644 --- a/pkg/util/errutil/errhttp/writer_test.go +++ b/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()) } diff --git a/pkg/util/errutil/errors.go b/pkg/util/errutil/errors.go index d8e26760b7b..7afc5d8c743 100644 --- a/pkg/util/errutil/errors.go +++ b/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 { diff --git a/pkg/util/errutil/status.go b/pkg/util/errutil/status.go index 69c9166ccae..6379dcdf447 100644 --- a/pkg/util/errutil/status.go +++ b/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 {