K8s/Folders: Fix folder status error message (#95464)

* Fix folder status error message
* Add test for folder creation response message
* Add TestFoldersCreateAPIEndpointK8S fixes
* Fix message returned when user has no permissions
This commit is contained in:
Arati R. 2024-10-28 12:33:56 +01:00 committed by GitHub
parent 69f185b459
commit 4a13580a2f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 61 additions and 18 deletions

View File

@ -1,6 +1,7 @@
package apierrors
import (
"encoding/json"
"errors"
"net/http"
@ -48,20 +49,36 @@ func ToFolderErrorResponse(err error) response.Response {
func ToFolderStatusError(err error) k8sErrors.StatusError {
resp := ToFolderErrorResponse(err)
defaultErr := k8sErrors.StatusError{
ErrStatus: metav1.Status{
Message: "Folder API error",
Code: http.StatusInternalServerError,
},
}
normResp, ok := resp.(*response.NormalResponse)
if !ok {
return k8sErrors.StatusError{
ErrStatus: metav1.Status{
Message: "Folder API error",
Code: http.StatusInternalServerError,
},
}
return defaultErr
}
var dat map[string]interface{}
if err := json.Unmarshal(normResp.Body(), &dat); err != nil {
return defaultErr
}
m, ok := dat["message"]
if !ok {
return defaultErr
}
message, ok := m.(string)
if !ok {
return defaultErr
}
return k8sErrors.StatusError{
ErrStatus: metav1.Status{
Message: normResp.ErrMessage(),
Message: message,
Code: int32(normResp.Status()),
},
}

View File

@ -807,7 +807,14 @@ func (fk8s *folderK8sHandler) writeError(c *contextmodel.ReqContext, err error)
//nolint:errorlint
statusError, ok := err.(*k8sErrors.StatusError)
if ok {
c.JsonApiErr(int(statusError.Status().Code), statusError.Status().Message, err)
message := statusError.Status().Message
// #TODO: Is there a better way to set the correct meesage? Instead of "access denied to folder", currently we are
// returning something like `folders.folder.grafana.app is forbidden: User "" cannot create resource "folders" in
// API group "folder.grafana.app" in the namespace "default": folder``
if statusError.Status().Code == http.StatusForbidden {
message = dashboards.ErrFolderAccessDenied.Error()
}
c.JsonApiErr(int(statusError.Status().Code), message, err)
return
}
errhttp.Write(c.Req.Context(), err, c.Resp)

View File

@ -657,13 +657,14 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
folderWithoutParentInput := "{ \"uid\": \"uid\", \"title\": \"Folder\"}"
folderWithoutUID := "{ \"title\": \"Folder without UID\"}"
folderWithTitleEmpty := "{ \"title\": \"\"}"
folderWithInvalidUid := "{ \"uid\": \"------------\"}"
folderWithUIDTooLong := "{ \"uid\": \"asdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnm\"}"
folderWithInvalidUid := "{ \"uid\": \"::::::::::::\", \"title\": \"Another folder\"}"
folderWithUIDTooLong := "{ \"uid\": \"asdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnmasdfghjklqwertyuiopzxcvbnm\", \"title\": \"Third folder\"}"
folderWithSameName := "{\"title\": \"same name\"}"
type testCase struct {
description string
expectedCode int
expectedMessage string
expectedFolderSvcError error
permissions []resourcepermissions.SetResourcePermissionCommand
input string
@ -688,15 +689,19 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
permissions: folderCreatePermission,
},
{
description: "folder creation fails without permissions to create a folder",
input: folderWithoutParentInput,
expectedCode: http.StatusForbidden,
permissions: []resourcepermissions.SetResourcePermissionCommand{},
description: "folder creation fails without permissions to create a folder",
input: folderWithoutParentInput,
expectedCode: http.StatusForbidden,
expectedMessage: dashboards.ErrFolderAccessDenied.Error(),
permissions: []resourcepermissions.SetResourcePermissionCommand{},
},
{
description: "folder creation fails given folder service error %s",
input: folderWithoutUID,
expectedCode: http.StatusConflict,
// #TODO This test case doesn't set up the conditions it describes. We should have created a folder with the same UID before
// creating a second one and failing to do so successfully.
description: "folder creation fails given folder service error %s",
input: folderWithoutUID,
expectedCode: http.StatusConflict,
// expectedMessage: dashboards.ErrFolderWithSameUIDExists.Error(),
expectedFolderSvcError: dashboards.ErrFolderWithSameUIDExists,
createSecondRecord: true,
permissions: folderCreatePermission,
@ -705,6 +710,7 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
description: "folder creation fails given folder service error %s",
input: folderWithTitleEmpty,
expectedCode: http.StatusBadRequest,
expectedMessage: dashboards.ErrFolderTitleEmpty.Error(),
expectedFolderSvcError: dashboards.ErrFolderTitleEmpty,
permissions: folderCreatePermission,
},
@ -712,6 +718,7 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
description: "folder creation fails given folder service error %s",
input: folderWithInvalidUid,
expectedCode: http.StatusBadRequest,
expectedMessage: dashboards.ErrDashboardInvalidUid.Error(),
expectedFolderSvcError: dashboards.ErrDashboardInvalidUid,
permissions: folderCreatePermission,
},
@ -719,6 +726,7 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
description: "folder creation fails given folder service error %s",
input: folderWithUIDTooLong,
expectedCode: http.StatusBadRequest,
expectedMessage: dashboards.ErrDashboardUidTooLong.Error(),
expectedFolderSvcError: dashboards.ErrDashboardUidTooLong,
permissions: folderCreatePermission,
},
@ -726,6 +734,7 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
description: "folder creation fails given folder service error %s",
input: folderWithSameName,
expectedCode: http.StatusConflict,
expectedMessage: dashboards.ErrFolderSameNameExists.Error(),
expectedFolderSvcError: dashboards.ErrFolderSameNameExists,
createSecondRecord: true,
permissions: folderCreatePermission,
@ -734,6 +743,7 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
description: "folder creation fails given folder service error %s",
input: folderWithoutParentInput,
expectedCode: http.StatusPreconditionFailed,
expectedMessage: dashboards.ErrFolderVersionMismatch.Error(),
expectedFolderSvcError: dashboards.ErrFolderVersionMismatch,
createSecondRecord: true,
permissions: folderCreatePermission,
@ -792,7 +802,12 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
require.NotNil(t, resp)
require.Equal(t, tc.expectedCode, resp.StatusCode)
folder := dtos.Folder{}
type folderWithMessage struct {
dtos.Folder
Message string `json:"message"`
}
folder := folderWithMessage{}
err = json.NewDecoder(resp.Body).Decode(&folder)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
@ -801,6 +816,10 @@ func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
require.Equal(t, "uid", folder.UID)
require.Equal(t, "Folder", folder.Title)
}
if tc.expectedMessage != "" {
require.Equal(t, tc.expectedMessage, folder.Message)
}
})
}
}