From 9ef790f553fed823f6e99384ff8ed3dffc6ff031 Mon Sep 17 00:00:00 2001 From: Idan Novogroder <43949240+idanovo@users.noreply.github.com> Date: Sun, 11 Feb 2024 12:22:40 +0200 Subject: [PATCH] Skip ensureStorageNamespace when creating a read-only repository (#7449) --- pkg/api/controller.go | 54 ++++++++++++++++++------------------ pkg/api/controller_test.go | 56 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 26 deletions(-) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index b09f66c9c77..419b3275037 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -1858,33 +1858,35 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo return } - if err := c.ensureStorageNamespace(ctx, body.StorageNamespace); err != nil { - var ( - reason string - retErr error - urlErr *url.Error - ) - switch { - case errors.As(err, &urlErr) && urlErr.Op == "parse": - retErr = err - reason = "bad_url" - case errors.Is(err, block.ErrInvalidAddress): - retErr = fmt.Errorf("%w, must match: %s", err, c.BlockAdapter.BlockstoreType()) - reason = "invalid_namespace" - case errors.Is(err, ErrStorageNamespaceInUse): - retErr = err - reason = "already_in_use" - default: - retErr = ErrFailedToAccessStorage - reason = "unknown" + if !swag.BoolValue(body.ReadOnly) { + if err := c.ensureStorageNamespace(ctx, body.StorageNamespace); err != nil { + var ( + reason string + retErr error + urlErr *url.Error + ) + switch { + case errors.As(err, &urlErr) && urlErr.Op == "parse": + retErr = err + reason = "bad_url" + case errors.Is(err, block.ErrInvalidAddress): + retErr = fmt.Errorf("%w, must match: %s", err, c.BlockAdapter.BlockstoreType()) + reason = "invalid_namespace" + case errors.Is(err, ErrStorageNamespaceInUse): + retErr = err + reason = "already_in_use" + default: + retErr = ErrFailedToAccessStorage + reason = "unknown" + } + c.Logger. + WithError(err). + WithField("storage_namespace", body.StorageNamespace). + WithField("reason", reason). + Warn("Could not access storage namespace") + writeError(w, r, http.StatusBadRequest, fmt.Errorf("failed to create repository: %w", retErr)) + return } - c.Logger. - WithError(err). - WithField("storage_namespace", body.StorageNamespace). - WithField("reason", reason). - Warn("Could not access storage namespace") - writeError(w, r, http.StatusBadRequest, fmt.Errorf("failed to create repository: %w", retErr)) - return } newRepo, err := c.Catalog.CreateRepository(ctx, body.Name, body.StorageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly)) diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 83af421cf20..ec8a79d54b0 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -1073,6 +1073,62 @@ func TestController_CreateRepositoryHandler(t *testing.T) { } }) + t.Run("create read-only repo skip ensure storage namespace", func(t *testing.T) { + repoName := testUniqueRepoName() + path := "bucket-1/namespace-1" + resp, err := clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{ + DefaultBranch: apiutil.Ptr("main"), + Name: repoName, + StorageNamespace: onBlock(deps, path), + }) + verifyResponseOK(t, resp, err) + + response := resp.JSON201 + if response == nil { + t.Fatal("CreateRepository got bad response") + } + if response.Id != repoName { + t.Fatalf("CreateRepository id=%s, expected=%s", response.Id, repoName) + } + + // delete the repo but keeps the dummy file + err = deps.catalog.DeleteRepository(ctx, repoName) + if err != nil { + t.Fatal(err) + } + // try to create the same repo once as a "normal" repo and once as a read-only repo + + resp2, err := clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{ + DefaultBranch: apiutil.Ptr("main"), + Name: repoName, + StorageNamespace: onBlock(deps, path), + }) + if err != nil { + t.Fatal(err) + } + if resp2 == nil { + t.Fatal("CreateRepository missing response") + } + if resp2.JSON400 == nil { + t.Fatal("expected status code 400 creating duplicate repo, got ", resp.StatusCode()) + } + + resp3, err := clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{ + DefaultBranch: apiutil.Ptr("main"), + Name: repoName, + StorageNamespace: onBlock(deps, path), + ReadOnly: apiutil.Ptr(true), + }) + verifyResponseOK(t, resp3, err) + response = resp3.JSON201 + if response == nil { + t.Fatal("CreateRepository got bad response") + } + if response.Id != repoName { + t.Fatalf("CreateRepository id=%s, expected=%s", response.Id, repoName) + } + }) + t.Run("create repo user unauthorized", func(t *testing.T) { repo := testUniqueRepoName()