fix(s3-deployment): wrong URL in BucketDeployment.deployedBucket.bucketWebsiteUrl (#24055)

Fixes #23354

Without pass-through of all attribute values, it is currently not possible to automatically force a dependency on the deployment for every attribute.
This change merely sets the bucket's region & account, so that all computed website/domain attributes will now include the correct values. Other attributes, and manually set website/domain attributes are not supported.
Additionally the documentation has been extended to highlight the above issue and provide a workaround.

----

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

Author: mrgrain

packages/@aws-cdk/aws-s3-deployment/lib/bucket-deployment.ts

@@ -253,6 +253,7 @@ export class BucketDeployment extends Construct {
   private readonly cr: cdk.CustomResource;
   private _deployedBucket?: s3.IBucket;
   private requestDestinationArn: boolean = false;
+  private readonly destinationBucket: s3.IBucket;
   private readonly sources: SourceConfig[];
   private readonly handlerRole: iam.IRole;
 
@@ -274,6 +275,8 @@ export class BucketDeployment extends Construct {
       throw new Error('Vpc must be specified if useEfs is set');
     }
 
+    this.destinationBucket = props.destinationBucket;
+
     const accessPointPath = '/lambda';
     let accessPoint;
     if (props.useEfs && props.vpc) {
@@ -333,9 +336,9 @@ export class BucketDeployment extends Construct {
 
     this.sources = props.sources.map((source: ISource) => source.bind(this, { handlerRole: this.handlerRole }));
 
-    props.destinationBucket.grantReadWrite(handler);
+    this.destinationBucket.grantReadWrite(handler);
     if (props.accessControl) {
-      props.destinationBucket.grantPutAcl(handler);
+      this.destinationBucket.grantPutAcl(handler);
     }
     if (props.distribution) {
       handler.addToRolePolicy(new iam.PolicyStatement({
@@ -378,7 +381,7 @@ export class BucketDeployment extends Construct {
             }, [] as Array<Record<string, any>>);
           },
         }, { omitEmptyArray: true }),
-        DestinationBucketName: props.destinationBucket.bucketName,
+        DestinationBucketName: this.destinationBucket.bucketName,
         DestinationBucketKeyPrefix: props.destinationKeyPrefix,
         RetainOnDelete: props.retainOnDelete,
         Extract: props.extract,
@@ -389,8 +392,8 @@ export class BucketDeployment extends Construct {
         SystemMetadata: mapSystemMetadata(props),
         DistributionId: props.distribution?.distributionId,
         DistributionPaths: props.distributionPaths,
-        // Passing through the ARN sequences dependencees on the deployment
-        DestinationBucketArn: cdk.Lazy.string({ produce: () => this.requestDestinationArn ? props.destinationBucket.bucketArn : undefined }),
+        // Passing through the ARN sequences dependency on the deployment
+        DestinationBucketArn: cdk.Lazy.string({ produce: () => this.requestDestinationArn ? this.destinationBucket.bucketArn : undefined }),
       },
     });
 
@@ -447,7 +450,7 @@ export class BucketDeployment extends Construct {
      * want the contents of the bucket to be removed on bucket deletion, then `autoDeleteObjects` property should
      * be set to true on the Bucket.
      */
-    cdk.Tags.of(props.destinationBucket).add(tagKey, 'true');
+    cdk.Tags.of(this.destinationBucket).add(tagKey, 'true');
 
   }
 
@@ -458,11 +461,18 @@ export class BucketDeployment extends Construct {
    * bucket deployment has happened before the next operation is started, pass the other construct
    * a reference to `deployment.deployedBucket`.
    *
-   * Doing this replaces calling `otherResource.node.addDependency(deployment)`.
+   * Note that this only returns an immutable reference to the destination bucket.
+   * If sequenced access to the original destination bucket is required, you may add a dependency
+   * on the bucket deployment instead: `otherResource.node.addDependency(deployment)`
    */
   public get deployedBucket(): s3.IBucket {
     this.requestDestinationArn = true;
-    this._deployedBucket = this._deployedBucket ?? s3.Bucket.fromBucketArn(this, 'DestinationBucket', cdk.Token.asString(this.cr.getAtt('DestinationBucketArn')));
+    this._deployedBucket = this._deployedBucket ?? s3.Bucket.fromBucketAttributes(this, 'DestinationBucket', {
+      bucketArn: cdk.Token.asString(this.cr.getAtt('DestinationBucketArn')),
+      region: this.destinationBucket.env.region,
+      account: this.destinationBucket.env.account,
+      isWebsite: this.destinationBucket.isWebsite,
+    });
     return this._deployedBucket;
   }
 

packages/@aws-cdk/aws-s3-deployment/test/bucket-deployment.test.ts

@@ -1111,6 +1111,32 @@ test('s3 deployment bucket is identical to destination bucket', () => {
   });
 });
 
+test('s3 deployed bucket in a different region has correct website url', () => {
+  // GIVEN
+  const stack = new cdk.Stack(undefined, undefined, {
+    env: {
+      region: 'us-east-1',
+    },
+  });
+  const bucket = s3.Bucket.fromBucketAttributes(stack, 'Dest', {
+    bucketName: 'my-bucket',
+    // Bucket is in a different region than stack
+    region: 'eu-central-1',
+  });
+
+  // WHEN
+  const bd = new s3deploy.BucketDeployment(stack, 'Deployment', {
+    destinationBucket: bucket,
+    sources: [s3deploy.Source.asset(path.join(__dirname, 'my-website'))],
+  });
+  const websiteUrl = stack.resolve(bd.deployedBucket.bucketWebsiteUrl);
+
+  // THEN
+  // eu-central-1 uses website endpoint format with a `.`
+  // see https://docs.aws.amazon.com/general/latest/gr/s3.html#s3_website_region_endpoints
+  expect(JSON.stringify(websiteUrl)).toContain('.s3-website.eu-central-1.');
+});
+
 test('using deployment bucket references the destination bucket by means of the CustomResource', () => {
   // GIVEN
   const stack = new cdk.Stack();