diff --git a/pkg/api/apierrors/dashboard.go b/pkg/api/apierrors/dashboard.go index b3d11f2f0d8..84584684b50 100644 --- a/pkg/api/apierrors/dashboard.go +++ b/pkg/api/apierrors/dashboard.go @@ -17,8 +17,9 @@ import ( // ToDashboardErrorResponse returns a different response status according to the dashboard error type func ToDashboardErrorResponse(ctx context.Context, pluginStore pluginstore.Store, err error) response.Response { + // --- Dashboard errors --- var dashboardErr dashboardaccess.DashboardErr - if ok := errors.As(err, &dashboardErr); ok { + if errors.As(err, &dashboardErr) { if body := dashboardErr.Body(); body != nil { return response.JSON(dashboardErr.StatusCode, body) } @@ -28,28 +29,32 @@ func ToDashboardErrorResponse(ctx context.Context, pluginStore pluginstore.Store return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), nil) } + // --- 400 Bad Request --- if errors.Is(err, dashboards.ErrFolderNotFound) { return response.Error(http.StatusBadRequest, err.Error(), nil) } var pluginErr dashboards.UpdatePluginDashboardError - if ok := errors.As(err, &pluginErr); ok { + if errors.As(err, &pluginErr) { message := fmt.Sprintf("The dashboard belongs to plugin %s.", pluginErr.PluginId) // look up plugin name if plugin, exists := pluginStore.Plugin(ctx, pluginErr.PluginId); exists { message = fmt.Sprintf("The dashboard belongs to plugin %s.", plugin.Name) } + // --- 412 Precondition Failed --- return response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": message}) } + // --- 413 Payload Too Large --- if apierrors.IsRequestEntityTooLargeError(err) { return response.Error(http.StatusRequestEntityTooLarge, fmt.Sprintf("Dashboard is too large, max is %d MB", apiserver.MaxRequestBodyBytes/1024/1024), err) } + // --- Kubernetes status errors --- var statusErr *apierrors.StatusError if errors.As(err, &statusErr) { return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err) } - return response.Error(http.StatusInternalServerError, "Failed to save dashboard", err) + return response.ErrOrFallback(http.StatusInternalServerError, fmt.Sprintf("Dashboard API error: %s", err.Error()), err) } diff --git a/pkg/api/apierrors/dashboard_test.go b/pkg/api/apierrors/dashboard_test.go new file mode 100644 index 00000000000..1e1512e07d3 --- /dev/null +++ b/pkg/api/apierrors/dashboard_test.go @@ -0,0 +1,128 @@ +package apierrors + +import ( + "context" + "errors" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/services/apiserver" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" + "github.com/grafana/grafana/pkg/util" +) + +type fakePluginStore struct { + pluginstore.Store + plugins map[string]pluginstore.Plugin +} + +func (f *fakePluginStore) Plugin(_ context.Context, id string) (pluginstore.Plugin, bool) { + p, ok := f.plugins[id] + return p, ok +} + +func TestToDashboardErrorResponse(t *testing.T) { + pluginStoreWithPlugin := &fakePluginStore{ + plugins: map[string]pluginstore.Plugin{ + "test-plugin": {JSONData: plugins.JSONData{Name: "Test Plugin"}}, + }, + } + pluginStoreWithoutPlugin := &fakePluginStore{ + plugins: map[string]pluginstore.Plugin{}, + } + + tests := []struct { + name string + pluginStore pluginstore.Store + input error + want response.Response + }{ + // --- 400 Bad Request --- + { + name: "dashboard error with a bad-request status", + pluginStore: pluginStoreWithoutPlugin, + input: dashboardaccess.DashboardErr{Reason: "Bad Request", StatusCode: http.StatusBadRequest}, + want: response.Error(http.StatusBadRequest, "Bad Request", nil), + }, + // --- 403 Forbidden --- + { + name: "dashboard error with a forbidden status", + pluginStore: pluginStoreWithoutPlugin, + input: &k8sErrors.StatusError{ErrStatus: metav1.Status{Code: http.StatusForbidden, Message: "access denied"}}, + want: response.Error(http.StatusForbidden, "access denied", &k8sErrors.StatusError{ErrStatus: metav1.Status{Code: http.StatusForbidden, Message: "access denied"}}), + }, + // --- 404 Not Found --- + { + name: "folder not found error", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrFolderNotFound, + want: response.Error(http.StatusBadRequest, dashboards.ErrFolderNotFound.Error(), nil), + }, + { + name: "dashboard error with a non-bad-request status", + pluginStore: pluginStoreWithoutPlugin, + input: dashboardaccess.DashboardErr{Reason: "Not Found", StatusCode: http.StatusNotFound}, + want: response.Error(http.StatusNotFound, "Not Found", dashboardaccess.DashboardErr{Reason: "Not Found", StatusCode: http.StatusNotFound}), + }, + { + name: "plugin dashboard error where plugin is found", + pluginStore: pluginStoreWithPlugin, + input: dashboards.UpdatePluginDashboardError{PluginId: "test-plugin"}, + want: response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": "The dashboard belongs to plugin Test Plugin."}), + }, + // --- 412 Precondition Failed --- + { + name: "plugin dashboard error where plugin is not found", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.UpdatePluginDashboardError{PluginId: "unknown-plugin"}, + want: response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": "The dashboard belongs to plugin unknown-plugin."}), + }, + // --- 413 Payload Too Large --- + { + name: "request entity too large error", + pluginStore: pluginStoreWithoutPlugin, + input: k8sErrors.NewRequestEntityTooLargeError("request is too large"), + want: response.Error(http.StatusRequestEntityTooLarge, fmt.Sprintf("Dashboard is too large, max is %d MB", apiserver.MaxRequestBodyBytes/1024/1024), k8sErrors.NewRequestEntityTooLargeError("request is too large")), + }, + // --- Kubernetes status errors --- + { + name: "kubernetes status error", + pluginStore: pluginStoreWithoutPlugin, + input: &k8sErrors.StatusError{ + ErrStatus: metav1.Status{ + Code: 412, + Message: "the dashboard has been changed by someone else", + }, + }, + want: response.Error(412, "the dashboard has been changed by someone else", &k8sErrors.StatusError{ + ErrStatus: metav1.Status{ + Code: 412, + Message: "the dashboard has been changed by someone else", + }, + }), + }, + // --- 500 Internal Server Error --- + { + name: "fallback error for an unknown error", + pluginStore: pluginStoreWithoutPlugin, + input: errors.New("an unexpected error"), + want: response.Error(http.StatusInternalServerError, "Dashboard API error: an unexpected error", errors.New("an unexpected error")), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := ToDashboardErrorResponse(context.Background(), tt.pluginStore, tt.input) + require.Equal(t, tt.want, res) + }) + } +} diff --git a/pkg/api/apierrors/folder.go b/pkg/api/apierrors/folder.go index 7ed21c04fb5..81b16c3899e 100644 --- a/pkg/api/apierrors/folder.go +++ b/pkg/api/apierrors/folder.go @@ -3,6 +3,7 @@ package apierrors import ( "encoding/json" "errors" + "fmt" "net/http" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -17,45 +18,49 @@ import ( // ToFolderErrorResponse returns a different response status according to the folder error type func ToFolderErrorResponse(err error) response.Response { + // --- Dashboard errors --- var dashboardErr dashboardaccess.DashboardErr if ok := errors.As(err, &dashboardErr); ok { return response.Error(dashboardErr.StatusCode, err.Error(), err) } + // --- 400 Bad Request --- if errors.Is(err, dashboards.ErrFolderTitleEmpty) || errors.Is(err, dashboards.ErrDashboardTypeMismatch) || errors.Is(err, dashboards.ErrDashboardInvalidUid) || - errors.Is(err, dashboards.ErrDashboardUidTooLong) { + errors.Is(err, dashboards.ErrDashboardUidTooLong) || + errors.Is(err, folder.ErrFolderCannotBeParentOfItself) { return response.Error(http.StatusBadRequest, err.Error(), nil) } + // --- 403 Forbidden --- if errors.Is(err, dashboards.ErrFolderAccessDenied) { return response.Error(http.StatusForbidden, "Access denied", err) } + // --- 404 Not Found --- if errors.Is(err, dashboards.ErrFolderNotFound) { return response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()}) } + // --- 409 Conflict --- if errors.Is(err, dashboards.ErrFolderWithSameUIDExists) { return response.Error(http.StatusConflict, err.Error(), nil) } + // --- 412 Precondition Failed --- if errors.Is(err, dashboards.ErrFolderVersionMismatch) || k8sErrors.IsAlreadyExists(err) { return response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "version-mismatch", "message": dashboards.ErrFolderVersionMismatch.Error()}) } - if errors.Is(err, folder.ErrMaximumDepthReached) { - return response.JSON(http.StatusBadRequest, util.DynMap{"messageId": "folder.maximum-depth-reached", "message": folder.ErrMaximumDepthReached.Error()}) - } - + // --- Kubernetes status errors --- var statusErr *k8sErrors.StatusError if errors.As(err, &statusErr) { return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err) } - return response.ErrOrFallback(http.StatusInternalServerError, "Folder API error", err) + return response.ErrOrFallback(http.StatusInternalServerError, fmt.Sprintf("Folder API error: %s", err.Error()), err) } func ToFolderStatusError(err error) k8sErrors.StatusError { diff --git a/pkg/api/apierrors/folder_test.go b/pkg/api/apierrors/folder_test.go index a1a02c1040e..2def7fb48a4 100644 --- a/pkg/api/apierrors/folder_test.go +++ b/pkg/api/apierrors/folder_test.go @@ -21,61 +21,109 @@ func TestToFolderErrorResponse(t *testing.T) { input error want response.Response }{ + // --- 400 Bad Request --- { name: "dashboard error", - input: dashboardaccess.DashboardErr{StatusCode: 400, Reason: "Dashboard Error", Status: "error"}, - want: response.Error(400, "Dashboard Error", dashboardaccess.DashboardErr{StatusCode: 400, Reason: "Dashboard Error", Status: "error"}), + input: dashboardaccess.DashboardErr{StatusCode: http.StatusBadRequest, Reason: "Dashboard Error", Status: "error"}, + want: response.Error(http.StatusBadRequest, "Dashboard Error", dashboardaccess.DashboardErr{StatusCode: http.StatusBadRequest, Reason: "Dashboard Error", Status: "error"}), + }, + { + name: "maximum depth reached", + input: folder.ErrMaximumDepthReached.Errorf("Maximum nested folder depth reached"), + want: response.Err(folder.ErrMaximumDepthReached.Errorf("Maximum nested folder depth reached")), + }, + { + name: "bad request errors", + input: folder.ErrBadRequest.Errorf("Bad request error"), + want: response.Err(folder.ErrBadRequest.Errorf("Bad request error")), + }, + { + name: "conflict error", + input: folder.ErrConflict.Errorf("Conflict error"), + want: response.Err(folder.ErrConflict.Errorf("Conflict error")), + }, + { + name: "circular reference error", + input: folder.ErrCircularReference.Errorf("Circular reference detected"), + want: response.Err(folder.ErrCircularReference.Errorf("Circular reference detected")), + }, + + { + name: "folder not empty error", + input: folder.ErrFolderNotEmpty.Errorf("Folder cannot be deleted: folder is not empty"), + want: response.Err(folder.ErrFolderNotEmpty.Errorf("Folder cannot be deleted: folder is not empty")), }, { name: "folder title empty", input: dashboards.ErrFolderTitleEmpty, - want: response.Error(400, "folder title cannot be empty", nil), + want: response.Error(http.StatusBadRequest, "folder title cannot be empty", nil), }, { name: "dashboard type mismatch", input: dashboards.ErrDashboardTypeMismatch, - want: response.Error(400, "Dashboard cannot be changed to a folder", dashboards.ErrDashboardTypeMismatch), + want: response.Error(http.StatusBadRequest, "Dashboard cannot be changed to a folder", dashboards.ErrDashboardTypeMismatch), }, { name: "dashboard invalid uid", input: dashboards.ErrDashboardInvalidUid, - want: response.Error(400, "uid contains illegal characters", dashboards.ErrDashboardInvalidUid), + want: response.Error(http.StatusBadRequest, "uid contains illegal characters", dashboards.ErrDashboardInvalidUid), }, { name: "dashboard uid too long", input: dashboards.ErrDashboardUidTooLong, - want: response.Error(400, "uid too long, max 40 characters", dashboards.ErrDashboardUidTooLong), + want: response.Error(http.StatusBadRequest, "uid too long, max 40 characters", dashboards.ErrDashboardUidTooLong), }, + { + name: "folder cannot be parent of itself", + input: folder.ErrFolderCannotBeParentOfItself, + want: response.Error(http.StatusBadRequest, folder.ErrFolderCannotBeParentOfItself.Error(), nil), + }, + // --- 403 Forbidden --- { name: "folder access denied", input: dashboards.ErrFolderAccessDenied, want: response.Error(http.StatusForbidden, "Access denied", dashboards.ErrFolderAccessDenied), }, + // --- 404 Not Found --- { name: "folder not found", input: dashboards.ErrFolderNotFound, want: response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrFolderNotFound.Error()}), }, + // --- 409 Conflict --- { name: "folder with same uid exists", input: dashboards.ErrFolderWithSameUIDExists, want: response.Error(http.StatusConflict, dashboards.ErrFolderWithSameUIDExists.Error(), nil), }, + // --- 412 Precondition Failed --- { name: "folder version mismatch", input: dashboards.ErrFolderVersionMismatch, want: response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "version-mismatch", "message": dashboards.ErrFolderVersionMismatch.Error()}), }, + // --- 500 Internal Server Error --- { - name: "folder max depth reached", - input: folder.ErrMaximumDepthReached, - want: response.JSON(http.StatusBadRequest, util.DynMap{"messageId": "folder.maximum-depth-reached", "message": folder.ErrMaximumDepthReached.Error()}), + name: "target registry srv conflict error", + input: folder.ErrTargetRegistrySrvConflict.Errorf("Target registry service conflict"), + want: response.Err(folder.ErrTargetRegistrySrvConflict.Errorf("Target registry service conflict")), }, { - name: "fallback error", - input: errors.New("some error"), - want: response.ErrOrFallback(http.StatusInternalServerError, "Folder API error", errors.New("some error")), + name: "internal error", + input: folder.ErrInternal.Errorf("Internal error"), + want: response.Err(folder.ErrInternal.Errorf("Internal error")), }, + { + name: "database error", + input: folder.ErrDatabaseError.Errorf("Database error"), + want: response.Err(folder.ErrDatabaseError.Errorf("Database error")), + }, + { + name: "fallback error for an unknown error", + input: errors.New("an unexpected error"), + want: response.Error(http.StatusInternalServerError, "Folder API error: an unexpected error", errors.New("an unexpected error")), + }, + // --- Kubernetes status errors --- { name: "kubernetes status error", input: &k8sErrors.StatusError{ diff --git a/pkg/registry/apis/folders/validate.go b/pkg/registry/apis/folders/validate.go index 4368f5e13d4..ca99aaa0a30 100644 --- a/pkg/registry/apis/folders/validate.go +++ b/pkg/registry/apis/folders/validate.go @@ -122,7 +122,7 @@ func validateOnUpdate(ctx context.Context, // if by moving a folder we exceed the max depth, return an error if len(info.Items)+1 >= maxDepth { - return folder.ErrMaximumDepthReached + return folder.ErrMaximumDepthReached.Errorf("maximum folder depth reached") } return nil } @@ -146,7 +146,7 @@ func validateOnDelete(ctx context.Context, for _, v := range resp.Stats { if v.Count > 0 { - return folder.ErrFolderNotEmpty + return folder.ErrFolderNotEmpty.Errorf("folder is not empty, contains %d resources", v.Count) } } return nil diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index f56e682283b..bad681c60a8 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -1401,13 +1401,13 @@ func (s *Service) validateParent(ctx context.Context, orgID int64, parentUID str // Create folder under itself is not allowed if parentUID == UID { - return folder.ErrCircularReference + return folder.ErrCircularReference.Errorf("circular reference detected") } // check there is no circular reference for _, ancestor := range ancestors { if ancestor.UID == UID { - return folder.ErrCircularReference + return folder.ErrCircularReference.Errorf("circular reference detected") } } diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index 65cd5623ec4..0644cae2d3d 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -417,7 +417,7 @@ func (ss *FolderStoreImpl) GetHeight(ctx context.Context, foldrUID string, orgID ele := queue[0] queue = queue[1:] if parentUID != nil && *parentUID == ele { - return 0, folder.ErrCircularReference + return 0, folder.ErrCircularReference.Errorf("circular reference detected") } folders, err := ss.GetChildren(ctx, folder.GetChildrenQuery{UID: ele, OrgID: orgID}) if err != nil { diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 08f4df69f5f..007767f2a6a 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -840,7 +840,7 @@ func TestIntegrationGetHeight(t *testing.T) { t.Run("should failed when the parent folder exist in the subtree", func(t *testing.T) { _, err = folderStore.GetHeight(context.Background(), parent.UID, orgID, &subTree[0]) - require.Error(t, err, folder.ErrCircularReference) + require.Error(t, err, folder.ErrCircularReference.Errorf("circular reference detected")) }) } diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index 0487af8aa40..28ced5f2321 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -298,7 +298,7 @@ func (ss *FolderUnifiedStoreImpl) GetHeight(ctx context.Context, foldrUID string ele := queue[0] queue = queue[1:] if parentUID != nil && *parentUID == ele { - return 0, folder.ErrCircularReference + return 0, folder.ErrCircularReference.Errorf("circular reference detected") } folders, err := ss.GetChildren(ctx, folder.GetChildrenQuery{UID: ele, OrgID: orgID}) if err != nil {