diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 8c0ce67be80..8373221c1f6 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -1923,6 +1923,9 @@ func (c *Controller) ListRepositories(w http.ResponseWriter, r *http.Request, pa } func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, body apigen.CreateRepositoryJSONRequestBody, params apigen.CreateRepositoryParams) { + storageID := swag.StringValue(body.StorageId) + storageNamespace := body.StorageNamespace + if !c.authorize(w, r, permissions.Node{ Type: permissions.NodeTypeAnd, Nodes: []permissions.Node{ @@ -1935,7 +1938,7 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo { Permission: permissions.Permission{ Action: permissions.AttachStorageNamespaceAction, - Resource: permissions.StorageNamespace(body.StorageNamespace), + Resource: permissions.StorageNamespace(storageNamespace), }, }, }, @@ -1960,13 +1963,13 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo c.LogAction(ctx, "repo_sample_data", r, body.Name, "", "") } - if err := c.validateStorageNamespace(body.StorageNamespace); err != nil { + if err := c.validateStorageNamespace(storageNamespace); err != nil { writeError(w, r, http.StatusBadRequest, err) return } if !c.Config.Installation.AllowInterRegionStorage { - if err := block.ValidateInterRegionStorage(r.Context(), c.BlockAdapter, body.StorageNamespace); err != nil { + if err := block.ValidateInterRegionStorage(r.Context(), c.BlockAdapter, storageID, storageNamespace); err != nil { writeError(w, r, http.StatusBadRequest, err) return } @@ -1977,10 +1980,8 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo defaultBranch = "main" } - storageID := swag.StringValue(body.StorageId) - if !swag.BoolValue(body.ReadOnly) { - if err := c.ensureStorageNamespace(ctx, body.StorageNamespace); err != nil { + if err := c.ensureStorageNamespace(ctx, storageNamespace); err != nil { var ( reason string retErr error @@ -2002,7 +2003,7 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo } c.Logger. WithError(err). - WithField("storage_namespace", body.StorageNamespace). + WithField("storage_namespace", storageNamespace). WithField("reason", reason). Warn("Could not access storage namespace") writeError(w, r, http.StatusBadRequest, fmt.Errorf("failed to create repository: %w", retErr)) @@ -2013,7 +2014,7 @@ 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, storageID, body.StorageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly)) + repo, err := c.Catalog.CreateBareRepository(ctx, body.Name, storageID, storageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly)) if c.handleAPIError(ctx, w, r, err) { return } @@ -2028,7 +2029,7 @@ func (c *Controller) CreateRepository(w http.ResponseWriter, r *http.Request, bo return } - newRepo, err := c.Catalog.CreateRepository(ctx, body.Name, storageID, body.StorageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly)) + newRepo, err := c.Catalog.CreateRepository(ctx, body.Name, storageID, storageNamespace, defaultBranch, swag.BoolValue(body.ReadOnly)) if err != nil { c.handleAPIError(ctx, w, r, fmt.Errorf("error creating repository: %w", err)) return diff --git a/pkg/block/adapter.go b/pkg/block/adapter.go index 77f82e6822b..defc543bb17 100644 --- a/pkg/block/adapter.go +++ b/pkg/block/adapter.go @@ -198,7 +198,8 @@ type Adapter interface { GetStorageNamespaceInfo() StorageNamespaceInfo ResolveNamespace(storageNamespace, key string, identifierType IdentifierType) (QualifiedKey, error) - GetRegion(ctx context.Context, storageNamespace string) (string, error) + // GetRegion storageID is not actively used, and it's here mainly for completeness + GetRegion(ctx context.Context, storageID, storageNamespace string) (string, error) RuntimeStats() map[string]string } diff --git a/pkg/block/azure/adapter.go b/pkg/block/azure/adapter.go index 3c9a332d6e8..4d14adb446c 100644 --- a/pkg/block/azure/adapter.go +++ b/pkg/block/azure/adapter.go @@ -625,7 +625,7 @@ func (a *Adapter) ResolveNamespace(storageNamespace, key string, identifierType return block.DefaultResolveNamespace(storageNamespace, key, identifierType) } -func (a *Adapter) GetRegion(_ context.Context, _ string) (string, error) { +func (a *Adapter) GetRegion(_ context.Context, _, _ string) (string, error) { return "", block.ErrOperationNotSupported } diff --git a/pkg/block/gs/adapter.go b/pkg/block/gs/adapter.go index 8695532d642..ebfe32ad476 100644 --- a/pkg/block/gs/adapter.go +++ b/pkg/block/gs/adapter.go @@ -693,7 +693,7 @@ func (a *Adapter) ResolveNamespace(storageNamespace, key string, identifierType return qualifiedKey, nil } -func (a *Adapter) GetRegion(_ context.Context, _ string) (string, error) { +func (a *Adapter) GetRegion(_ context.Context, _, _ string) (string, error) { return "", block.ErrOperationNotSupported } diff --git a/pkg/block/local/adapter.go b/pkg/block/local/adapter.go index 0b6fb31dd54..121c63d661d 100644 --- a/pkg/block/local/adapter.go +++ b/pkg/block/local/adapter.go @@ -559,7 +559,7 @@ func (l *Adapter) ResolveNamespace(storageNamespace, key string, identifierType }, nil } -func (l *Adapter) GetRegion(_ context.Context, _ string) (string, error) { +func (l *Adapter) GetRegion(_ context.Context, _, _ string) (string, error) { return "", block.ErrOperationNotSupported } diff --git a/pkg/block/mem/adapter.go b/pkg/block/mem/adapter.go index d394945bbf2..0299856b89e 100644 --- a/pkg/block/mem/adapter.go +++ b/pkg/block/mem/adapter.go @@ -354,7 +354,7 @@ func (a *Adapter) ResolveNamespace(storageNamespace, key string, identifierType return block.DefaultResolveNamespace(storageNamespace, key, identifierType) } -func (a *Adapter) GetRegion(_ context.Context, _ string) (string, error) { +func (a *Adapter) GetRegion(_ context.Context, _, _ string) (string, error) { return "", block.ErrOperationNotSupported } diff --git a/pkg/block/metrics.go b/pkg/block/metrics.go index 36c697f330e..0c892ec2d8e 100644 --- a/pkg/block/metrics.go +++ b/pkg/block/metrics.go @@ -123,9 +123,9 @@ func (m *MetricsAdapter) ResolveNamespace(storageNamespace, key string, identifi return m.adapter.ResolveNamespace(storageNamespace, key, identifierType) } -func (m *MetricsAdapter) GetRegion(ctx context.Context, storageNamespace string) (string, error) { +func (m *MetricsAdapter) GetRegion(ctx context.Context, storageID, storageNamespace string) (string, error) { ctx = httputil.SetClientTrace(ctx, m.adapter.BlockstoreType()) - return m.adapter.GetRegion(ctx, storageNamespace) + return m.adapter.GetRegion(ctx, storageID, storageNamespace) } func (m *MetricsAdapter) RuntimeStats() map[string]string { diff --git a/pkg/block/s3/adapter.go b/pkg/block/s3/adapter.go index 2f8ff3060c0..2591dcdfc9d 100644 --- a/pkg/block/s3/adapter.go +++ b/pkg/block/s3/adapter.go @@ -890,7 +890,7 @@ func (a *Adapter) ResolveNamespace(storageNamespace, key string, identifierType return block.DefaultResolveNamespace(storageNamespace, key, identifierType) } -func (a *Adapter) GetRegion(ctx context.Context, storageNamespace string) (string, error) { +func (a *Adapter) GetRegion(ctx context.Context, _, storageNamespace string) (string, error) { namespaceURL, err := url.Parse(storageNamespace) if err != nil { return "", fmt.Errorf(`%s isn't a valid url': %w`, storageNamespace, block.ErrInvalidNamespace) diff --git a/pkg/block/transient/adapter.go b/pkg/block/transient/adapter.go index 1d98dd1834b..b5d70e1bca9 100644 --- a/pkg/block/transient/adapter.go +++ b/pkg/block/transient/adapter.go @@ -163,7 +163,7 @@ func (a *Adapter) ResolveNamespace(storageNamespace, key string, identifierType return block.DefaultResolveNamespace(storageNamespace, key, identifierType) } -func (a *Adapter) GetRegion(_ context.Context, _ string) (string, error) { +func (a *Adapter) GetRegion(_ context.Context, _, _ string) (string, error) { return "", block.ErrOperationNotSupported } diff --git a/pkg/block/validations.go b/pkg/block/validations.go index 5d93e854d0d..84f26c2d6a3 100644 --- a/pkg/block/validations.go +++ b/pkg/block/validations.go @@ -6,7 +6,7 @@ import ( "fmt" ) -func ValidateInterRegionStorage(ctx context.Context, adapter Adapter, storageNamespace string) error { +func ValidateInterRegionStorage(ctx context.Context, adapter Adapter, storageID, storageNamespace string) error { blockstoreMetadata, err := adapter.BlockstoreMetadata(ctx) if errors.Is(err, ErrOperationNotSupported) { // region detection not supported for the server's blockstore, skip validation @@ -15,7 +15,7 @@ func ValidateInterRegionStorage(ctx context.Context, adapter Adapter, storageNam return fmt.Errorf("failed to get blockstore region: %w", err) } - bucketRegion, err := adapter.GetRegion(ctx, storageNamespace) + bucketRegion, err := adapter.GetRegion(ctx, storageID, storageNamespace) if err != nil { return fmt.Errorf("failed to get region of storage namespace %s: %w", storageNamespace, ErrInvalidNamespace) } diff --git a/pkg/block/validations_test.go b/pkg/block/validations_test.go index e0867bb229e..07d23efb856 100644 --- a/pkg/block/validations_test.go +++ b/pkg/block/validations_test.go @@ -20,7 +20,7 @@ func TestController_ValidateInterRegionStorage(t *testing.T) { } adapter := testutil.NewMockAdapter(opts...) - err := block.ValidateInterRegionStorage(ctx, adapter, "s3://us-west-2/bucket-1") + err := block.ValidateInterRegionStorage(ctx, adapter, "", "s3://us-west-2/bucket-1") require.NoError(t, err) }) @@ -31,7 +31,7 @@ func TestController_ValidateInterRegionStorage(t *testing.T) { } adapter := testutil.NewMockAdapter(opts...) - err := block.ValidateInterRegionStorage(ctx, adapter, "s3://us-west-2/bucket-1") + err := block.ValidateInterRegionStorage(ctx, adapter, "", "s3://us-west-2/bucket-1") require.ErrorIs(t, err, block.ErrInvalidNamespace) }) } diff --git a/pkg/testutil/adapter.go b/pkg/testutil/adapter.go index 80923afa4c9..3a6f72e998d 100644 --- a/pkg/testutil/adapter.go +++ b/pkg/testutil/adapter.go @@ -152,7 +152,7 @@ func (a *MockAdapter) ResolveNamespace(storageNamespace, key string, identifierT return block.DefaultResolveNamespace(storageNamespace, key, identifierType) } -func (a *MockAdapter) GetRegion(_ context.Context, _ string) (string, error) { +func (a *MockAdapter) GetRegion(_ context.Context, _, _ string) (string, error) { if a.namespaceRegion != nil { return *a.namespaceRegion, nil } else {