added storage namespace validation on non-readonly bare repo creation

treeverse · Nov 24, 2024 · a593aad · a593aad
1 parent 3d0db30
commit a593aad
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 30 deletions.
diff --git a/pkg/api/controller.go b/pkg/api/controller.go
@@ -1971,23 +1971,6 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo
 		defaultBranch = "main"
 	}
 
-	if swag.BoolValue(params.Bare) {
-		// create a bare repository. This is useful in conjunction with refs-restore to create a copy
-		// of another repository by e.g. copying the _lakefs/ directory and restoring its refs
-		repo, err := c.Catalog.CreateBareRepository(ctx, body.Name, body.StorageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly))
-		if c.handleAPIError(ctx, w, r, err) {
-			return
-		}
-		response := apigen.Repository{
-			CreationDate:     repo.CreationDate.Unix(),
-			DefaultBranch:    repo.DefaultBranch,
-			Id:               repo.Name,
-			StorageNamespace: repo.StorageNamespace,
-		}
-		writeResponse(w, r, http.StatusCreated, response)
-		return
-	}
-
 	if !swag.BoolValue(body.ReadOnly) {
 		if err := c.ensureStorageNamespace(ctx, body.StorageNamespace); err != nil {
 			var (
@@ -2019,6 +2002,23 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo
 		}
 	}
 
+	if swag.BoolValue(params.Bare) {
+		// create a bare repository. This is useful in conjunction with refs-restore to create a copy
+		// of another repository by e.g. copying the _lakefs/ directory and restoring its refs
+		repo, err := c.Catalog.CreateBareRepository(ctx, body.Name, body.StorageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly))
+		if c.handleAPIError(ctx, w, r, err) {
+			return
+		}
+		response := apigen.Repository{
+			CreationDate:     repo.CreationDate.Unix(),
+			DefaultBranch:    repo.DefaultBranch,
+			Id:               repo.Name,
+			StorageNamespace: repo.StorageNamespace,
+		}
+		writeResponse(w, r, http.StatusCreated, response)
+		return
+	}
+
 	newRepo, err := c.Catalog.CreateRepository(ctx, body.Name, body.StorageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly))
 	if err != nil {
 		c.handleAPIError(ctx, w, r, fmt.Errorf("error creating repository: %w", err))

diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go
@@ -198,21 +198,23 @@ func TestController_GetRepoHandler(t *testing.T) {
 		}
 	})
 
-	t.Run("use same storage namespace twice", func(t *testing.T) {
-		name := testUniqueRepoName()
-		resp, err := clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{
-			Name:             name,
-			StorageNamespace: onBlock(deps, name),
-		})
-		verifyResponseOK(t, resp, err)
+	for _, isBareRepo := range []bool{false, true} {
+		t.Run(fmt.Sprintf("use same storage namespace twice, isBareRepo=%v", isBareRepo), func(t *testing.T) {
+			name := testUniqueRepoName()
+			resp, err := clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{
+				Name:             name,
+				StorageNamespace: onBlock(deps, name),
+			})
+			verifyResponseOK(t, resp, err)
 
-		resp, err = clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{
-			Name:             name + "_2",
-			StorageNamespace: onBlock(deps, name),
+			resp, err = clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{Bare: &isBareRepo}, apigen.CreateRepositoryJSONRequestBody{
+				Name:             name + "_2",
+				StorageNamespace: onBlock(deps, name),
+			})
+			require.NoError(t, err)
+			require.Equal(t, http.StatusBadRequest, resp.StatusCode())
 		})
-		require.NoError(t, err)
-		require.Equal(t, http.StatusBadRequest, resp.StatusCode())
-	})
+	}
 }
 
 func testCommitEntries(t *testing.T, ctx context.Context, cat *catalog.Catalog, deps *dependencies, params commitEntriesParams) string {