From a6e96c2f6016835bc8b58b59dfe3eaaccc345b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20S=C3=BCrmeli?= Date: Sun, 24 Sep 2023 16:02:29 +0300 Subject: [PATCH 1/4] feat: implement `metadata-directive` --- command/cp.go | 30 ++++++++++++++++++++++++++++++ e2e/cp_test.go | 40 ++++++++++++++++++++++++++++++++++++++-- storage/s3.go | 4 ++++ storage/storage.go | 5 +++++ 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/command/cp.go b/command/cp.go index 775e31100..2ec0eba8a 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.ToLower(target) == strings.ToLower(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..c17c76595 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,8 +910,9 @@ 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) // assert S3 diff --git a/storage/s3.go b/storage/s3.go index f6ebcffd3..0537095ff 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 { From 79155b2e1a76c20dce99de7f716ff627a233881f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20S=C3=BCrmeli?= Date: Sun, 24 Sep 2023 16:05:28 +0300 Subject: [PATCH 2/4] check: happy --- command/cp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/cp.go b/command/cp.go index 2ec0eba8a..7536fcb56 100644 --- a/command/cp.go +++ b/command/cp.go @@ -148,7 +148,7 @@ func NewSharedFlags() []cli.Flag { Enum: []string{"COPY", "REPLACE", ""}, Default: "", ConditionFunction: func(str, target string) bool { - return strings.ToLower(target) == strings.ToLower(str) + return strings.EqualFold(target, str) }, }, }, From 79ab3b11bb4e96b748218bccf4e96ff6cbe5b06d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20S=C3=BCrmeli?= Date: Tue, 26 Sep 2023 10:59:01 +0300 Subject: [PATCH 3/4] chore: bump gofakes3 --- go.mod | 2 +- go.sum | 4 ++-- vendor/github.com/igungor/gofakes3/gofakes3.go | 9 ++++++++- vendor/modules.txt | 2 +- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 9162601ea..1757299ac 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 8569a392d..355ca5aaa 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,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/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 bb1512904..1696e471a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -88,7 +88,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 From 5ae754c0021340b3607a0cc92971557ec686c4a0 Mon Sep 17 00:00:00 2001 From: Deniz Surmeli <52484739+denizsurmeli@users.noreply.github.com> Date: Thu, 4 Jul 2024 10:57:11 +0300 Subject: [PATCH 4/4] Update e2e/cp_test.go --- e2e/cp_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/e2e/cp_test.go b/e2e/cp_test.go index c17c76595..2f0f69821 100644 --- a/e2e/cp_test.go +++ b/e2e/cp_test.go @@ -912,7 +912,6 @@ func TestCopyS3ToS3WithArbitraryMetadataWithReplaceDirective(t *testing.T) { putFile(t, s3client, bucket, filename, content, putArbitraryMetadata(srcmetadata)) cmd := s5cmd("cp", "--metadata-directive", "REPLACE", "--metadata", foo, "--metadata", bar, srcpath, dstpath) result := icmd.RunCmd(cmd) - result.Assert(t, icmd.Success) // assert S3