fix: remove use of object ACLs

[nosnilmot]

trafficstars

Description of changes

Best practice for restricting access to Amazon S3 content by using an origin access identity (OAI) is to us an S3 bucket policy, which is already the case.

Per-object ACLs are NOT recommended, see:

https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/private-content-restricting-access-to-s3.html#private-content-updating-s3-acls

Issue

This change is motivated by the fact that AWS is currently experiencing an issue in us-east-2 whereby CloudFront OAI S3CanonicalUserId are not being accepted in S3 PutObject requests, resulting in inability to publish amplify frontends to that region:

Publish started for S3AndCloudFront ✖ Error has occurred during file upload. An error occurred during the publish operation: Invalid id ⚠️ Review the Amplify CLI troubleshooting guide for potential next steps: https://docs.amplify.aws/cli/project/troubleshooting/

(btw, that troubleshooting page was no help, and I had to modify amplify-cli source to identify what the source of the error was anyway)

Description of how you validated changes

amplify-dev publish -c works

Checklist

• [x] PR description included
• [x] yarn test passes

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

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging d292b2b7b501467c948396d0abfe2e7f0b3fe123 into f154b898c25f97175fae21e906f85a277a4dafd9 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 1 alert when merging e681291457198dd52db4e3fd40ae71b4df41d5bb into f154b898c25f97175fae21e906f85a277a4dafd9 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[codecov-commenter]

Codecov Report

Merging #10823 (66d23d0) into dev (f154b89) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev   #10823   +/-   ##
=======================================
  Coverage   47.47%   47.47%           
=======================================
  Files         673      673           
  Lines       33209    33193   -16     
  Branches     6708     6705    -3     
=======================================
- Hits        15765    15760    -5     
+ Misses      15760    15749   -11     
  Partials     1684     1684           

Impacted Files	Coverage Δ
...sting/lib/S3AndCloudFront/helpers/file-uploader.js	95.23% <100.00%> (+15.92%)	:arrow_up:
...li/src/domain/amplify-usageData/getUsageDataUrl.ts	100.00% <0.00%> (+12.50%)	:arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[danielleadams]

Hi @nosnilmot - thanks for your contribution! Running the test suite on it now.

[nosnilmot]

Hi @nosnilmot - thanks for your contribution! Running the test suite on it now.

Hi @danielleadams - I see the CI tests have failed, but I do not think the failures are related to my PR. Please advise on how to proceed. Might rebasing help?

[danielleadams]

Hi @nosnilmot - if you don't mind rebasing, that would be helpful and I can rerun them. Thank you!

[nosnilmot]

@danielleadams I have rebased, and the automated CI tests have passed. Could you rerun the test suite please?

[danielleadams]

@nosnilmot I'm running integration tests, but I would like to follow up with questions for other team members before we merge. I'll keep you posted on the progress of the PR.

[nosnilmot]

@nosnilmot I'm running integration tests, but I would like to follow up with questions for other team members before we merge. I'll keep you posted on the progress of the PR.

@danielleadams any updates?

[nosnilmot]

@danielleadams ping?

[danielleadams]

@nosnilmot thanks for your patience here. I'm working through my backlog - so just getting around to this. Can you give me a week and let me see if I can move this forward? I will report back.

[danielleadams]

@nosnilmot thanks for your patience here - we're finally ready to merge this. I'm just running tests on the change now.

[aws-eddy]

Hey @nosnilmot, your contribution was merged and released on 11.1.1. Thanks for your help 👍🏼