fix(s3-deployment): User metadata keys have redundant triple x-amz prefix

[bmwalters]

Without this fix, keys are prefixed with x-amzn-meta- by the TypeScript code,
then the Python code runs and prefixes metadata keys again with x-amz-meta-.
Then, the Python shells out to aws-cli which makes a request to the S3 service.
There, the keys are prefixed yet again with x-amz-meta- unconditionally.

Thus no matter what keys the user specified, the keys in S3 would be:
x-amz-meta-x-amz-meta-x-amzn-meta-.

This issue was originally reported as #8459.

#10678 attempted to fix this issue, and fixed the Python, but missed the TS.
It also suffers from the S3 service adding the prefix unconditionally.

After this change, neither the TypeScript code nor the Python code will attempt
to prepend to the metadata key. Instead we rely on the S3 service to do it.

BREAKING CHANGE: User metadata keys of bucket objects will change from x-amz-meta-x-amz-meta-x-amzn-meta-mykey to x-amz-meta-mykey.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[bmwalters]

@iliapolo @ayush987goyal The S3 service which is accessed by the CLI command which the s3_deploy lambda shells out to here:

aws-cdk/packages/@aws-cdk/aws-s3-deployment/lib/lambda/index.py

Lines 142 to 143 in bf059fa

	s3_command.extend(create_metadata_args(user_metadata, system_metadata))
	aws_command(*s3_command)

seems to already adds the x-amz-meta- prefix. Can we just remove the logic from the TypeScript and Python code that attempts to add the prefix and rely on S3 doing it? Seems simpler and also might be where the snag is.

[bmwalters]

Yeah, I confirmed that without even using this library, running

$ AWS_SDK_LOAD_CONFIG=1 AWS_PROFILE=my-profile \
    aws s3 sync ./my-contents/ s3://my-bucket//my-path/ \
        --metadata '{ "x-amz-meta-sourceurl": "static" }'  \
        --metadata-directive REPLACE

results in duplicate x-amz-meta-x-amz-meta-sourceurl. This means that S3 is adding the prefix for us unconditionally when using the S3 CLI like this.

I'll update this patch to remove adding the prefix in TS and Python entirely.

[iliapolo]

@bmwalters This is great. It was indeed pretty messy, my only question/concern is how is this going to affect existing objects in the bucket? will their metadata change?

[bmwalters]

Yes, the metadata keys will change. For example, to access metadata created by s3-deployment using an SDK:

// before
let metadata = s3Object.metadata?["x-amz-meta-x-amzn-meta-mykey"]
// after
let metadata = s3Object.metadata?["mykey"]

The previous version is awkward so I'm not sure how many users there actually are. If we want to preserve the incorrect metadata keys as well we could internally duplicate each key, so if the user passes in "mykey" they end up with both "x-amz-meta-mykey" and "x-amz-meta-x-amz-meta-x-amzn-meta-mykey". Otherwise this could be listed as a potentially breaking change.

[iliapolo]

@bmwalters Ok, lets mention this as a breaking change 👍

P.S: Notice there are conflicts needs resolving

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: fd16d8d
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).