From 8abfcdbb78504639550c30dcc75dcf66edb4a5ad Mon Sep 17 00:00:00 2001 From: "Arati R." <33031346+suntala@users.noreply.github.com> Date: Mon, 21 Oct 2024 18:07:11 +0200 Subject: [PATCH] Ks8/Folders: Fix status codes returned on create (#95055) * Fix status codes returned by k8s folder handler * Add test for status code when creating duplicate folder --- pkg/api/apierrors/folder.go | 24 +++++++++++ pkg/registry/apis/folders/legacy_storage.go | 4 +- pkg/tests/apis/folder/folders_test.go | 48 +++++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/pkg/api/apierrors/folder.go b/pkg/api/apierrors/folder.go index edad9a40a0f..f05e35b6241 100644 --- a/pkg/api/apierrors/folder.go +++ b/pkg/api/apierrors/folder.go @@ -4,6 +4,9 @@ import ( "errors" "net/http" + 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/services/dashboards" "github.com/grafana/grafana/pkg/util" @@ -42,3 +45,24 @@ func ToFolderErrorResponse(err error) response.Response { return response.ErrOrFallback(http.StatusInternalServerError, "Folder API error", err) } + +func ToFolderStatusError(err error) k8sErrors.StatusError { + resp := ToFolderErrorResponse(err) + + normResp, ok := resp.(*response.NormalResponse) + if !ok { + return k8sErrors.StatusError{ + ErrStatus: metav1.Status{ + Message: "Folder API error", + Code: http.StatusInternalServerError, + }, + } + } + + return k8sErrors.StatusError{ + ErrStatus: metav1.Status{ + Message: normResp.ErrMessage(), + Code: int32(normResp.Status()), + }, + } +} diff --git a/pkg/registry/apis/folders/legacy_storage.go b/pkg/registry/apis/folders/legacy_storage.go index 858d2fdaef4..e168da572c0 100644 --- a/pkg/registry/apis/folders/legacy_storage.go +++ b/pkg/registry/apis/folders/legacy_storage.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" + "github.com/grafana/grafana/pkg/api/apierrors" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" @@ -186,7 +187,8 @@ func (s *legacyStorage) Create(ctx context.Context, ParentUID: parent, }) if err != nil { - return nil, err + statusErr := apierrors.ToFolderStatusError(err) + return nil, &statusErr } // #TODO can we directly convert instead of doing a Get? the result of the Create // has more data than the one of Get so there is more we can include in the k8s resource diff --git a/pkg/tests/apis/folder/folders_test.go b/pkg/tests/apis/folder/folders_test.go index 6c7afa8e2cc..6a8c2d1fe3a 100644 --- a/pkg/tests/apis/folder/folders_test.go +++ b/pkg/tests/apis/folder/folders_test.go @@ -308,6 +308,24 @@ func TestIntegrationFoldersApp(t *testing.T) { }, })) }) + + t.Run("with dual write (unified storage, mode 1, create existing folder)", func(t *testing.T) { + doCreateDuplicateFolderTest(t, apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ + AppModeProduction: true, + DisableAnonymous: true, + APIServerStorageType: "unified", + UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ + folderv0alpha1.RESOURCEGROUP: { + DualWriterMode: grafanarest.Mode1, + }, + }, + EnableFeatureToggles: []string{ + featuremgmt.FlagGrafanaAPIServerTestingWithExperimentalAPIs, + featuremgmt.FlagNestedFolders, + featuremgmt.FlagKubernetesFolders, + }, + })) + }) } func doFolderTests(t *testing.T, helper *apis.K8sTestHelper) *apis.K8sTestHelper { @@ -474,6 +492,36 @@ func doNestedCreateTest(t *testing.T, helper *apis.K8sTestHelper) { require.Equal(t, parentCreate.Result.URL, parent.URL) } +func doCreateDuplicateFolderTest(t *testing.T, helper *apis.K8sTestHelper) { + client := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + GVR: gvr, + }) + + payload := `{ + "title": "Test", + "uid": "" + }` + create := apis.DoRequest(helper, apis.RequestParams{ + User: client.Args.User, + Method: http.MethodPost, + Path: "/api/folders", + Body: []byte(payload), + }, &folder.Folder{}) + require.NotNil(t, create.Result) + parentUID := create.Result.UID + require.NotEmpty(t, parentUID) + + create2 := apis.DoRequest(helper, apis.RequestParams{ + User: client.Args.User, + Method: http.MethodPost, + Path: "/api/folders", + Body: []byte(payload), + }, &folder.Folder{}) + require.NotEmpty(t, create2.Response) + require.Equal(t, 409, create2.Response.StatusCode) +} + func TestIntegrationFolderCreatePermissions(t *testing.T) { if testing.Short() { t.Skip("skipping integration test")