From 92a65a74a51cccbe8703f60acd8fbb159db78ea7 Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Tue, 11 Feb 2025 22:35:46 +0200 Subject: [PATCH 1/3] Handle StorageID in S3 Gateway --- pkg/catalog/catalog.go | 4 ++++ pkg/gateway/operations/listobjects.go | 1 + pkg/gateway/operations/putobject.go | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index d1fd27f77f5..97abc38e8fb 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -2738,6 +2738,10 @@ func (c *Catalog) CopyEntry(ctx context.Context, srcRepository, srcRef, srcPath, if err != nil { return nil, err } + + if srcRepo.StorageID != destRepo.StorageID { + return nil, fmt.Errorf("copy between different blockstores is not allowed: %w", graveler.ErrInvalidValue) + } } // copy data to a new physical address diff --git a/pkg/gateway/operations/listobjects.go b/pkg/gateway/operations/listobjects.go index 39b9227d3c1..0df8d948703 100644 --- a/pkg/gateway/operations/listobjects.go +++ b/pkg/gateway/operations/listobjects.go @@ -442,6 +442,7 @@ func handleListMultipartUploads(w http.ResponseWriter, req *http.Request, o *Rep opts.KeyMarker = &keyMarker } mpuResp, err := o.BlockStore.ListMultipartUploads(req.Context(), block.ObjectPointer{ + StorageID: o.Repository.StorageID, StorageNamespace: o.Repository.StorageNamespace, IdentifierType: block.IdentifierTypeRelative, }, opts) diff --git a/pkg/gateway/operations/putobject.go b/pkg/gateway/operations/putobject.go index 4206544b5d7..653b376418d 100644 --- a/pkg/gateway/operations/putobject.go +++ b/pkg/gateway/operations/putobject.go @@ -177,6 +177,12 @@ func handleUploadPart(w http.ResponseWriter, req *http.Request, o *PathOperation } } + if srcRepo.StorageID != o.Repository.StorageID { + o.Log(req).WithField("copy_source", copySource).WithError(err).Error("copy between different blockstores is not allowed") + _ = o.EncodeError(w, req, err, gatewayErrors.Codes.ToAPIErr(gatewayErrors.ErrInvalidCopySource)) + return + } + src := block.ObjectPointer{ StorageID: srcRepo.StorageID, StorageNamespace: srcRepo.StorageNamespace, From f60493350b4b664c39c0e71774987a7d44fab2b4 Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Tue, 11 Feb 2025 22:45:20 +0200 Subject: [PATCH 2/3] Some minor additions --- pkg/block/adapter.go | 1 + pkg/gateway/operations/putobject_test.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/block/adapter.go b/pkg/block/adapter.go index 9704965b38b..b7e360fdc30 100644 --- a/pkg/block/adapter.go +++ b/pkg/block/adapter.go @@ -211,6 +211,7 @@ type Adapter interface { BlockstoreType() string BlockstoreMetadata(ctx context.Context) (*BlockstoreMetadata, error) + // GetStorageNamespaceInfo returns the StorageNamespaceInfo for storageID or nil if not found. GetStorageNamespaceInfo(storageID string) *StorageNamespaceInfo ResolveNamespace(storageID, storageNamespace, key string, identifierType IdentifierType) (QualifiedKey, error) diff --git a/pkg/gateway/operations/putobject_test.go b/pkg/gateway/operations/putobject_test.go index 69feef1a94f..63b47c88306 100644 --- a/pkg/gateway/operations/putobject_test.go +++ b/pkg/gateway/operations/putobject_test.go @@ -10,6 +10,7 @@ import ( "github.com/treeverse/lakefs/pkg/api/apiutil" "github.com/treeverse/lakefs/pkg/block" + "github.com/treeverse/lakefs/pkg/config" "github.com/treeverse/lakefs/pkg/testutil" "github.com/treeverse/lakefs/pkg/upload" ) @@ -46,7 +47,7 @@ func TestWriteBlob(t *testing.T) { reader := bytes.NewReader(data) adapter := testutil.NewMockAdapter() objectPointer := block.ObjectPointer{ - StorageID: "", + StorageID: config.SingleBlockstoreID, StorageNamespace: storageNamespace, IdentifierType: block.IdentifierTypeRelative, Identifier: upload.DefaultPathProvider.NewPath(), From 20dd150ce1c0d060a5d7413c097603a728b0ddda Mon Sep 17 00:00:00 2001 From: Itai Gilo Date: Thu, 13 Feb 2025 09:32:04 +0200 Subject: [PATCH 3/3] Fix PR comments --- pkg/catalog/catalog.go | 2 +- pkg/gateway/operations/putobject.go | 2 +- .../operations/putobject_test.go => upload/write_blob_test.go} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename pkg/{gateway/operations/putobject_test.go => upload/write_blob_test.go} (99%) diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index 97abc38e8fb..09a936c2550 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -2740,7 +2740,7 @@ func (c *Catalog) CopyEntry(ctx context.Context, srcRepository, srcRef, srcPath, } if srcRepo.StorageID != destRepo.StorageID { - return nil, fmt.Errorf("copy between different blockstores is not allowed: %w", graveler.ErrInvalidValue) + return nil, fmt.Errorf("%w: cannot copy between repos with different StorageIDs", graveler.ErrInvalidStorageID) } } diff --git a/pkg/gateway/operations/putobject.go b/pkg/gateway/operations/putobject.go index 653b376418d..ad3c5de9b5e 100644 --- a/pkg/gateway/operations/putobject.go +++ b/pkg/gateway/operations/putobject.go @@ -178,7 +178,7 @@ func handleUploadPart(w http.ResponseWriter, req *http.Request, o *PathOperation } if srcRepo.StorageID != o.Repository.StorageID { - o.Log(req).WithField("copy_source", copySource).WithError(err).Error("copy between different blockstores is not allowed") + o.Log(req).WithField("copy_source", copySource).Error("copy between repos with different StorageIDs is not allowed") _ = o.EncodeError(w, req, err, gatewayErrors.Codes.ToAPIErr(gatewayErrors.ErrInvalidCopySource)) return } diff --git a/pkg/gateway/operations/putobject_test.go b/pkg/upload/write_blob_test.go similarity index 99% rename from pkg/gateway/operations/putobject_test.go rename to pkg/upload/write_blob_test.go index 63b47c88306..5d9228f7b64 100644 --- a/pkg/gateway/operations/putobject_test.go +++ b/pkg/upload/write_blob_test.go @@ -1,4 +1,4 @@ -package operations_test +package upload_test import ( "bytes"