fix(s3): cannot deploy multiple replication source buckets (under feature flag)

[badmintoncryer]

Issue # (if applicable)

Closes #33355.

Reason for this change

We cannot deploy multiple source buckets for object replication due to the explicitly set replication role name.

Description of changes

Set replication role name by PhysicalName.GENERATE_IF_NEEDED.

Describe any new or updated permissions being added

None

Description of how you validated changes

Update both unit and integ test.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.00%. Comparing base (70263b2) to head (0eba6a1).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #33360   +/-   ##
=======================================
  Coverage   81.00%   81.00%           
=======================================
  Files         238      238           
  Lines       14271    14271           
  Branches     2492     2492           
=======================================
  Hits        11560    11560           
  Misses       2425     2425           
  Partials      286      286           

Flag	Coverage Δ
suite.unit	81.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	79.92% <ø> (ø)
packages/aws-cdk-lib/core	82.16% <ø> (ø)

[jochemd]

How does this work in a scenario where multiple stages are deployed to 1 account? So we have 2 buckets from the TEST stage being replicated and 2 buckets from the PRD stage being replicated? Will they use the same role? And what if the replication is cross-account?

I was more thinking along the lines of adding an optional replicationRole: Role property to the ReplicationRule interface and use that role if it is passed in. That gives the control to the user on when to use the same and when to use different roles. The destination bucket already allows the user to specify the role (ARN) in the addReplicationPolicy method. Or possibly refactor renderReplicationConfiguration so that the part that creates the policy for the source bucket becomes a public method to allow people to set up permissions easily when they cutomize their stack (similar to how the addReplicationPolicy method is public for the destination bucket).

BTW, https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-s3/lib/bucket.ts#L2853 appears to be confusing the buckets: addReplicationPolicy() can be used to set permissions on the destination bucket.

[badmintoncryer]

@jochemd
I'm sorry for my late reply. Thank you for your feedback!

How does this work in a scenario where multiple stages are deployed to 1 account? So we have 2 buckets from the TEST stage being replicated and 2 buckets from the PRD stage being replicated? Will they use the same role? And what if the replication is cross-account?

One replication role is generated for each source bucket, regardless of stage differences or whether cross-account access is involved.
In the case of cross-account access, an explicit role name is specified, but there are no other differences.

I was more thinking along the lines of adding an optional replicationRole: Role property to the ReplicationRule interface and use that role if it is passed in.

Since one replication role executes replication according to multiple replication rules, it seems difficult to simply add a role property to the ReplicationRule interface. However, since I think the idea itself is excellent, how about posting it as a separate issue?
For this PR, I think it's better to focus on fixing the bug where multiple source buckets cannot be defined in the first place.

BTW, https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-s3/lib/bucket.ts#L2853 appears to be confusing the buckets: addReplicationPolicy() can be used to set permissions on the destination bucket.

Oh! I've not realized this. I'll resolve this problem in another PR.

[badmintoncryer]

@jochemd
I noticed that in the original issue you suggested making the role receivable as an argument.

From my perspective, since the current implementation is problematic, I think it would be best to first merge this PR to remove the hardcoded role name, and then create a new PR as a new feature to accept an optional replication role.

[jochemd]

Sounds good to me. Unfortunately I am not able to give an actual review or serious testing of this PR as when I try to go through the steps in the contributing guide I get errors on the linking step.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 0eba6a1
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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