Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added storage namespace validation on non-readonly bare repo creation #8364

Merged
merged 3 commits into from
Jan 14, 2025
Merged

added storage namespace validation on non-readonly bare repo creation #8364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a593aad
added storage namespace validation on non-readonly bare repo creation
tkalir Nov 13, 2024
57880b8
Merge remote-tracking branch 'origin/master' into fix/add-namespace-v…
tkalir Jan 12, 2025
c18ff68
changed StorageNamespace in "create bare repo success" to not conflic…
tkalir Jan 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions pkg/api/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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))
Expand Down
30 changes: 16 additions & 14 deletions pkg/api/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down
Loading