Skip ensureStorageNamespace when creating a read-only repository (#7449)

treeverse · Feb 11, 2024 · 9ef790f · 9ef790f
1 parent eab29ed
commit 9ef790f
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 26 deletions.
diff --git 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
@@ -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()