From 706f74afcda746fc861a35c02a67d314b336b39f Mon Sep 17 00:00:00 2001 From: Deniz Surmeli <52484739+denizsurmeli@users.noreply.github.com> Date: Fri, 5 Jul 2024 19:06:48 +0300 Subject: [PATCH] feat: implement `metadata-directive` (#668) This PR allows to specify whether to keep the existing metadata of the object or replace with a new one when copying from `s3` to `s3`. Tests will pass after the [PR](https://github.com/igungor/gofakes3/pull/12) that will be merged to `gofakes3`. Resolves #666 Resolves #711 --- command/cp.go | 30 ++++++++++++++ e2e/cp_test.go | 39 ++++++++++++++++++- go.mod | 2 +- go.sum | 4 +- storage/s3.go | 4 ++ storage/storage.go | 5 +++ .../github.com/igungor/gofakes3/gofakes3.go | 9 ++++- vendor/modules.txt | 2 +- 8 files changed, 88 insertions(+), 7 deletions(-) diff --git a/command/cp.go b/command/cp.go index 7ff0530cc..98b856f8b 100644 --- a/command/cp.go +++ b/command/cp.go @@ -141,6 +141,17 @@ func NewSharedFlags() []cli.Flag { Name: "metadata", Usage: "set arbitrary metadata for the object, e.g. --metadata 'foo=bar' --metadata 'fizz=buzz'", }, + &cli.GenericFlag{ + Name: "metadata-directive", + Usage: "set metadata directive for the object: COPY or REPLACE", + Value: &EnumValue{ + Enum: []string{"COPY", "REPLACE", ""}, + Default: "", + ConditionFunction: func(str, target string) bool { + return strings.EqualFold(target, str) + }, + }, + }, &cli.StringFlag{ Name: "sse", Usage: "perform server side encryption of the data at its destination, e.g. aws:kms", @@ -305,6 +316,7 @@ type Copy struct { contentEncoding string contentDisposition string metadata map[string]string + metadataDirective string showProgress bool progressbar progressbar.ProgressBar @@ -382,6 +394,7 @@ func NewCopy(c *cli.Context, deleteSource bool) (*Copy, error) { contentEncoding: c.String("content-encoding"), contentDisposition: c.String("content-disposition"), metadata: metadata, + metadataDirective: c.String("metadata-directive"), showProgress: c.Bool("show-progress"), progressbar: commandProgressBar, @@ -514,10 +527,26 @@ func (c Copy) Run(ctx context.Context) error { switch { case srcurl.Type == c.dst.Type: // local->local or remote->remote + if c.metadataDirective == "" { + // default to COPY + c.metadataDirective = "COPY" + } task = c.prepareCopyTask(ctx, srcurl, c.dst, isBatch, c.metadata) case srcurl.IsRemote(): // remote->local + if c.metadataDirective != "" { + err := fmt.Errorf("metadata directive is not supported for download") + merrorObjects = multierror.Append(merrorObjects, err) + printError(c.fullCommand, c.op, err) + continue + } task = c.prepareDownloadTask(ctx, srcurl, c.dst, isBatch) case c.dst.IsRemote(): // local->remote + if c.metadataDirective != "" { + err := fmt.Errorf("metadata directive is not supported for upload") + merrorObjects = multierror.Append(merrorObjects, err) + printError(c.fullCommand, c.op, err) + continue + } task = c.prepareUploadTask(ctx, srcurl, c.dst, isBatch, c.metadata) default: panic("unexpected src-dst pair") @@ -765,6 +794,7 @@ func (c Copy) doCopy(ctx context.Context, srcurl, dsturl *url.URL, extradata map ContentDisposition: c.contentDisposition, EncryptionMethod: c.encryptionMethod, EncryptionKeyID: c.encryptionKeyID, + Directive: c.metadataDirective, } err = c.shouldOverride(ctx, srcurl, dsturl) diff --git a/e2e/cp_test.go b/e2e/cp_test.go index 48867bd68..2f0f69821 100644 --- a/e2e/cp_test.go +++ b/e2e/cp_test.go @@ -845,7 +845,42 @@ func TestCopySingleFileToS3WithArbitraryMetadata(t *testing.T) { } // cp s3://bucket2/obj2 s3://bucket1/obj1 --metadata key1=val1 --metadata key2=val2 ... -func TestCopyS3ToS3WithArbitraryMetadata(t *testing.T) { +func TestCopyS3ToS3WithArbitraryMetadataWithDefaultDirective(t *testing.T) { + t.Parallel() + + s3client, s5cmd := setup(t) + + bucket := s3BucketFromTestName(t) + createBucket(t, s3client, bucket) + + const ( + filename = "index" + content = "things" + foo = "Key1=foo" + bar = "Key2=bar" + ) + + // build assert map + srcmetadata := map[string]*string{ + "Key1": aws.String("value1"), + "Key2": aws.String("value2"), + } + + srcpath := fmt.Sprintf("s3://%v/%v", bucket, filename) + dstpath := fmt.Sprintf("s3://%v/%v_cp", bucket, filename) + + putFile(t, s3client, bucket, filename, content, putArbitraryMetadata(srcmetadata)) + cmd := s5cmd("cp", "--metadata", foo, "--metadata", bar, srcpath, dstpath) + result := icmd.RunCmd(cmd) + + result.Assert(t, icmd.Success) + + // assert S3 + assert.Assert(t, ensureS3Object(s3client, bucket, fmt.Sprintf("%s_cp", filename), content, ensureArbitraryMetadata(srcmetadata))) +} + +// cp s3://bucket2/obj2 s3://bucket1/obj1 --metadata-directive REPLACE --metadata key1=val1 --metadata key2=val2 ... +func TestCopyS3ToS3WithArbitraryMetadataWithReplaceDirective(t *testing.T) { t.Parallel() s3client, s5cmd := setup(t) @@ -875,7 +910,7 @@ func TestCopyS3ToS3WithArbitraryMetadata(t *testing.T) { dstpath := fmt.Sprintf("s3://%v/%v_cp", bucket, filename) putFile(t, s3client, bucket, filename, content, putArbitraryMetadata(srcmetadata)) - cmd := s5cmd("cp", "--metadata", foo, "--metadata", bar, srcpath, dstpath) + cmd := s5cmd("cp", "--metadata-directive", "REPLACE", "--metadata", foo, "--metadata", bar, srcpath, dstpath) result := icmd.RunCmd(cmd) result.Assert(t, icmd.Success) diff --git a/go.mod b/go.mod index df9a09752..eefb2c14f 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/google/go-cmp v0.5.9 github.com/hashicorp/go-multierror v1.1.1 github.com/iancoleman/strcase v0.0.0-20191112232945-16388991a334 - github.com/igungor/gofakes3 v0.0.15 + github.com/igungor/gofakes3 v0.0.16 github.com/karrick/godirwalk v1.15.3 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/lanrat/extsort v1.0.0 diff --git a/go.sum b/go.sum index 53f6cf4c4..e15dba0a8 100644 --- a/go.sum +++ b/go.sum @@ -24,8 +24,8 @@ github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+l github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/iancoleman/strcase v0.0.0-20191112232945-16388991a334 h1:VHgatEHNcBFEB7inlalqfNqw65aNkM1lGX2yt3NmbS8= github.com/iancoleman/strcase v0.0.0-20191112232945-16388991a334/go.mod h1:SK73tn/9oHe+/Y0h39VT4UCxmurVJkR5NA7kMEAOgSE= -github.com/igungor/gofakes3 v0.0.15 h1:/57KiuC2Nc0Heh1cjnTOe6mWrDNxIr8kfF7xgah55OA= -github.com/igungor/gofakes3 v0.0.15/go.mod h1:+rwAKRO9RTGCIeE8SRvRPLSj7PVhaMBLlm1zPXzu7Cs= +github.com/igungor/gofakes3 v0.0.16 h1:aMipkwE9s2u4T6GfgIPZ8ngJcReYsJvGRm6c4/lLAfY= +github.com/igungor/gofakes3 v0.0.16/go.mod h1:+rwAKRO9RTGCIeE8SRvRPLSj7PVhaMBLlm1zPXzu7Cs= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= diff --git a/storage/s3.go b/storage/s3.go index d22c58633..96c54f92d 100644 --- a/storage/s3.go +++ b/storage/s3.go @@ -551,6 +551,10 @@ func (s *S3) Copy(ctx context.Context, from, to *url.URL, metadata Metadata) err input.Metadata[metadataKeyRetryID] = generateRetryID() } + if metadata.Directive != "" { + input.MetadataDirective = aws.String(metadata.Directive) + } + if len(metadata.UserDefined) != 0 { m := make(map[string]*string) for k, v := range metadata.UserDefined { diff --git a/storage/storage.go b/storage/storage.go index 982341662..eb59e17b0 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -231,6 +231,11 @@ type Metadata struct { EncryptionKeyID string UserDefined map[string]string + + // MetadataDirective is used to specify whether the metadata is copied from + // the source object or replaced with metadata provided when copying S3 + // objects. If MetadataDirective is not set, it defaults to "COPY". + Directive string } func (o Object) ToBytes() []byte { diff --git a/vendor/github.com/igungor/gofakes3/gofakes3.go b/vendor/github.com/igungor/gofakes3/gofakes3.go index dfa756554..d76c33254 100644 --- a/vendor/github.com/igungor/gofakes3/gofakes3.go +++ b/vendor/github.com/igungor/gofakes3/gofakes3.go @@ -578,8 +578,15 @@ func (g *GoFakeS3) createObject(bucket, object string, w http.ResponseWriter, r if err != nil { return err } - size = src.Size + if meta["X-Amz-Metadata-Directive"] == "COPY" { + meta = src.Metadata + } + + // this is not actually a part of the metadata + delete(src.Metadata, "X-Amz-Metadata-Directive") + + size = src.Size body = src.Contents } else { diff --git a/vendor/modules.txt b/vendor/modules.txt index e243ec936..1a78d5cc7 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -90,7 +90,7 @@ github.com/hashicorp/go-multierror # github.com/iancoleman/strcase v0.0.0-20191112232945-16388991a334 ## explicit github.com/iancoleman/strcase -# github.com/igungor/gofakes3 v0.0.15 +# github.com/igungor/gofakes3 v0.0.16 ## explicit; go 1.13 github.com/igungor/gofakes3 github.com/igungor/gofakes3/backend/s3bolt