added storage namespace validation on non-readonly bare repo creation (…

…#8364) * added storage namespace validation on non-readonly bare repo creation * changed StorageNamespace in "create bare repo success" to not conflict with the one in "create repo success"
treeverse · Jan 14, 2025 · 06312c7 · 06312c7
1 parent 9c9be9d
commit 06312c7
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 32 deletions.
diff --git a/pkg/api/controller.go b/pkg/api/controller.go
@@ -1976,24 +1976,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
-	}
-	// Since this is a read-only repository, there is no harm in case the storage namespace we use is already used by
-	// another repository or if we don't have write permissions for this namespace.
 	if !swag.BoolValue(body.ReadOnly) {
 		if err := c.ensureStorageNamespace(ctx, body.StorageNamespace); err != nil {
 			var (
@@ -2025,6 +2007,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 {
@@ -1035,7 +1037,7 @@ func TestController_CreateRepositoryHandler(t *testing.T) {
 			}, apigen.CreateRepositoryJSONRequestBody{
 				DefaultBranch:    apiutil.Ptr("main"),
 				Name:             repoName,
-				StorageNamespace: onBlock(deps, "foo-bucket-1"),
+				StorageNamespace: onBlock(deps, "foo-bucket-2"),
 			})
 		verifyResponseOK(t, resp, err)