feat(s3): add grantReplicationPermission for IAM Role permissions

[hassaku63]

Issue # (if applicable)

Closes #34119

Reason for this change

This change introduces a new method, grantReplicationPermission, to the aws-cdk-lib.aws_s3.Bucket construct. The purpose of this addition is to provide a more convenient and programmatic way for AWS CDK users to grant the necessary IAM permissions to a user-provided IAM Role that will be used for S3 bucket replication.

Description of changes

This pull request includes the following code changes:

• Added a new public method grantReplicationPermission to the Bucket class.
• The implementation of this method programmatically attaches the necessary IAM permissions for S3 bucket replication to the provided identity. This change refactors the renderReplicationConfiguration method by extracting the IAM permission granting functionality into a dedicated grantReplicationPermission method.
• unit and integ test
• The README was updated to show that users can now grant replication rights to custom IAM roles.

Describe any new or updated permissions being added

No new IAM permissions are being added at the CDK level. The permissions granted by the grantReplicationPermission method are the same as those already handled internally by the existing replication configuration logic. This change simply exposes that functionality through a dedicated method.

Description of how you validated changes

• Added unit tests to verify the functionality of the grantReplicationPermission method, ensuring that the correct IAM policies are attached to the provided role. Notably, the unit tests specifically cover scenarios where an explicit replicationRole is provided.
• Existing integration tests were run to confirm that no regressions were introduced by this change. In addition, the existing test scenario integ.bucket-replication-use-custom-role.ts was refactored to use the new grantReplicationPermission method instead of manually attaching the required permissions to the IAM role, and its behavior was verified to remain equivalent.

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

[hassaku63]

Regarding the design of the grantReplicationPermission method, I've aimed to follow the common patterns of other grant methods in the CDK, where returning an iam.Grant object is prevalent. However, this method currently has a void return type. While I intend to explore and implement the refactoring to return iam.Grant myself, I'm currently unsure about the best way to structure the Grant object for this feature, especially given the need to grant multiple distinct permissions. If the reviewers have any insights or suggestions on the most appropriate approach, I would be grateful for the guidance, as I work towards a suitable implementation.

[badmintoncryer]

Thank you for your contribution! I've added a small comment.

[badmintoncryer]

Could you please add unit test for throwing this error?

[hassaku63]

@badmintoncryer Thank you for your review, yes I have added a unit test that you noted.

[shikha372]

Regarding the design of the grantReplicationPermission method, I've aimed to follow the common patterns of other grant methods in the CDK, where returning an iam.Grant object is prevalent. However, this method currently has a void return type. While I intend to explore and implement the refactoring to return iam.Grant myself, I'm currently unsure about the best way to structure the Grant object for this feature, especially given the need to grant multiple distinct permissions. If the reviewers have any insights or suggestions on the most appropriate approach, I would be grateful for the guidance, as I work towards a suitable implementation.

Thank you @hassaku63, i checked the implementation for other grant methods in repo and we usually return the grant object back which allows user to further modify the grant if needed, i would like to follow the same pattern for this grant function as well.
I acknowledge that this grant involves dealing with both source and destination bucket, imo we should return the source level grant permissions from this method

  const sourcePermissions = iam.Grant.addToPrincipalOrResource({
    grantee: identity,
    actions: ['s3:GetReplicationConfiguration', 's3:ListBucket'],
    resourceArns: [Lazy.string({ produce: () => this.bucketArn })],
    resource: this,
  });

return sourcePermissions;

also i think we can still leverage the internal private grant method instead of calling iam.Grant.addToPrincipalOrResource , let me know your thoughts

  const sourcePermissions = this.grant({
    identity,
    actions: ['s3:GetReplicationConfiguration', 's3:ListBucket'],
    resourceArns: [Lazy.string({ produce: () => this.bucketArn })],
    resource: this,
  });

[hassaku63]

@shikha372 Thank you for the review. I'd like to share my thoughts in response to your feedback:

(1) Whether to follow the common pattern of returning a Grant object, and how to do so

In principle, I agree with your direction to align with the common pattern seen in other grant methods.

However, the approach you suggested only reflects part of the actual permissions granted in grantReplicationPermission. I believe this might cause confusion for users if they try to utilize the returned Grant object expecting it to cover all permissions granted by the method.

To better reflect the full behavior of grantReplicationPermission, I propose combining all the individual grants into a single Grant using iam.Grant.combine, and returning that combined result. Although this would be slightly more verbose, I believe it would most accurately represent what the method is doing. Here's an outline of what I have in mind:

    let result = iam.Grant.addToPrincipalOrResource({
      grantee: identity,
      actions: ['s3:GetReplicationConfiguration', 's3:ListBucket'],
      resourceArns: [Lazy.string({ produce: () => this.bucketArn })],
      resource: this,
    });

    const g2 = iam.Grant.addToPrincipalOrResource({
      grantee: identity,
      actions: ['s3:GetObjectVersionForReplication', 's3:GetObjectVersionAcl', 's3:GetObjectVersionTagging'],
      resourceArns: [Lazy.string({ produce: () => this.arnForObjects('*') })],
      resource: this,
    });
    result = result.combine(g2);

    const destinationBuckets = props.destinations.map(destination => destination.bucket);
    if (destinationBuckets.length > 0) {
      const g3 = iam.Grant.addToPrincipalOrResource({
        grantee: identity,
        actions: ['s3:ReplicateObject', 's3:ReplicateDelete', 's3:ReplicateTags', 's3:ObjectOwnerOverrideToBucketOwner'],
        resourceArns: destinationBuckets.map(bucket => Lazy.string({ produce: () => bucket.arnForObjects('*') })),
        resource: this,
      });
      result = result.combine(g3);
    }

    props.destinations.forEach(destination => {
      const g = destination.encryptionKey?.grantEncrypt(identity);
      if (g !== undefined) {
        result = result.combine(g);
      }
    });

    // If KMS key encryption is enabled on the source bucket, configure the decrypt permissions.
    const g4 = this.encryptionKey?.grantDecrypt(identity);
    if (g4 !== undefined) {
      result = result.combine(g4);
    }

    return result;

(2) Using the existing private grant method on the Bucket class

While I don't consider it strictly mandatory, I agree with the direction you suggested. I intend to update the implementation accordingly to leverage the existing private grant method in the Bucket class.

Since also includes a refactoring aspect, I think it's a good opportunity to align the implementation with the existing capabilities of the class.

Also, I believe this change does not conflict with the discussion in (1).

[shikha372]

q : why we're overriding the logical id's here ? is this only for the template match below ?

[shikha372]

if yes, then there might be a change in future that can cause logical id changes which in turn leads to resource replacements so overriding it here imo is not something i would recommend.
can we please keep them as it is, and populate the values from test logs ?

[hassaku63]

@shikha372 Thanks for the suggestion — I’ll go ahead and revise the implementation in line with your recommendation.

Just to share some context, the current implementation uses overrideLogicalId() intentionally. If you're open to it, I’d appreciate your thoughts on my intent behind using overrideLogicalId() in this context.

The reason I chose to override the logical ID was to ensure predictability and to decouple the test from internal implementation details such as CDK's logical ID generation. From my perspective, changes in how logical IDs are assigned shouldn't affect this particular test case.

Since the focus here is on verifying that the grant for IAM Role permissions works correctly in the context of S3 replication, I considered logical ID assignments to be outside the scope of what this test is meant to validate. In that sense, I was concerned that allowing logical ID changes to affect this test might introduce noise rather than meaningful signal.

[hassaku63]

(p.s.) I’m aware that my initial implementation follows a different approach from other unit tests in CDK, so I’m totally open to changing it.

[shikha372]

Thank you @hassaku63 for submitting the PR, this has been reviewed by our security team and received a go ahead, just had one last comment about overriding the logical id changes, good to approve it once addressed.

[ozelalisen]

The JSDoc for destinations property says default tag - empty array, but the implementation throws an error if the array is empty. I would suggest removing the default tag since there is no default value, and clarify that at least one destination is required

[hassaku63]

@ozelalisen Thanks. The points you suggested are absolutely right. How about adopting the following policy?

  /**
   * The destination buckets for replication.
   * Specify the KMS key to use for encryption if a destination bucket needs to be encrypted with a customer-managed KMS key.
   *
   * One or more destination buckets are required if replication configuration is enabled (i.e., `replicationRole` is specified).
   *
   * @default - empty array (valid only if the `replicationRole` property is NOT specified)
   */
  readonly destinations: GrantReplicationPermissionDestinationProps[];

I aimed to clearly convey the behavior of this property by distinguishing between the default value itself and the conditions under which that default is valid or invalid.

[ozelalisen]

Thank you for addressing the issue, that looks good to me!

[ozelalisen]

The error message "destinations must be specified" could be more descriptive. Maybe something like: At least one destination bucket must be specified in the destinations array

[hassaku63]

@ozelalisen Thanks for your review. I will revise the error message as you suggested.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: a0df50e
• 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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.