From c7eb6f840466220f70b6a7308a2d3661d551fa9c Mon Sep 17 00:00:00 2001
From: Dillon Nys <nydillon@amazon.com>
Date: Wed, 14 Jun 2023 11:05:33 -0700
Subject: [PATCH] ci: Fix storage bucket name

Bucket names must be globally unique. To ensure the infra stacks can be deployed to multiple accounts, use the bucket name as a prefix instead of the actual bucket name.
---
 infra/lib/common.ts        | 34 +++++++++++++++++++---------------
 infra/lib/stack.ts         |  2 +-
 infra/lib/storage/stack.ts | 16 ++++++++++------
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/infra/lib/common.ts b/infra/lib/common.ts
index 0e88a829e7..495250acc4 100644
--- a/infra/lib/common.ts
+++ b/infra/lib/common.ts
@@ -80,7 +80,10 @@ export abstract class IntegrationTestStack<
   public get bucket(): s3.Bucket {
     if (!this._bucket) {
       this._bucket = new s3.Bucket(this, "Bucket", {
-        bucketName: this.bucketName,
+        bucketName: randomBucketName({
+          prefix: `amplify-test-${this.baseName.toLowerCase()}`,
+          stack: this,
+        }),
         removalPolicy: RemovalPolicy.DESTROY,
         autoDeleteObjects: true,
         enforceSSL: true,
@@ -89,20 +92,6 @@ export abstract class IntegrationTestStack<
     return this._bucket;
   }
 
-  /**
-   * The bucket name for this stack where configuration files can be saved.
-   *
-   * Naming matches Amplify CLI for discoverability, suffixed with a segment of the stack ID.
-   * https://github.com/aws-amplify/amplify-ci-support/blob/1abe7f7a1d75fa19675ad8ca17ab625a299b765e/src/integ_test_resources/flutter/amplify/cloudformation_template.yaml#L32
-   */
-  private get bucketName(): string {
-    return Fn.join("-", [
-      `amplify-test-${this.baseName.toLowerCase()}`,
-      // https://stackoverflow.com/questions/54897459/how-to-set-semi-random-name-for-s3-bucket-using-cloud-formation
-      Fn.select(0, Fn.split("-", Fn.select(2, Fn.split("/", this.stackId)))),
-    ]);
-  }
-
   /**
    * Saves the Amplify configuration for an environment as a CDK output.
    * @param config The Amplify config values.
@@ -167,6 +156,21 @@ export abstract class IntegrationTestStackEnvironment<
   }
 }
 
+/**
+ * Creates a semi-random bucket name which is stable for a particular stack and guaranteed to
+ * not conflict with other buckets of the same prefix.
+ *
+ * Naming matches Amplify CLI for discoverability, suffixed with a segment of the stack ID.
+ * https://github.com/aws-amplify/amplify-ci-support/blob/1abe7f7a1d75fa19675ad8ca17ab625a299b765e/src/integ_test_resources/flutter/amplify/cloudformation_template.yaml#L32
+ */
+export const randomBucketName = ({ prefix, stack }: { prefix: string, stack: cdk.Stack }): string => {
+  return Fn.join("-", [
+    prefix,
+    // https://stackoverflow.com/questions/54897459/how-to-set-semi-random-name-for-s3-bucket-using-cloud-formation
+    Fn.select(0, Fn.split("-", Fn.select(2, Fn.split("/", stack.stackId)))),
+  ]);
+}
+
 /**
  * Creates an amplifyconfiguration.dart compatible JSON representation of config.
  * @param region The stack's region.
diff --git a/infra/lib/stack.ts b/infra/lib/stack.ts
index 5e403907fc..5d51ae16df 100644
--- a/infra/lib/stack.ts
+++ b/infra/lib/stack.ts
@@ -275,7 +275,7 @@ export class AmplifyFlutterIntegStack extends cdk.Stack {
       {
         environmentName: "dots-in-name",
         enableTransferAcceleration: false,
-        bucketName: "amplify.integ-test.stack.com",
+        bucketNamePrefix: "amplify.integ-test.stack",
       },
     ]);
 
diff --git a/infra/lib/storage/stack.ts b/infra/lib/storage/stack.ts
index 343a51873a..43f3aad2fd 100644
--- a/infra/lib/storage/stack.ts
+++ b/infra/lib/storage/stack.ts
@@ -14,7 +14,8 @@ import {
   AmplifyCategory,
   IntegrationTestStack,
   IntegrationTestStackEnvironment,
-  IntegrationTestStackEnvironmentProps
+  IntegrationTestStackEnvironmentProps,
+  randomBucketName
 } from "../common";
 
 export enum StorageAccessLevel {
@@ -52,9 +53,12 @@ interface StorageIntegrationTestEnvironmentProps
   prefixOverrides?: Record<StorageAccessLevel, String>;
 
   /**
-   * The name of the bucket. If not provided, it will be auto-generated.
+   * The prefix name of the bucket. If not provided, it will be auto-generated.
+   * 
+   * Since bucket names are unique globally, a dedicated bucket name cannot be
+   * used since that would disallow multiple accounts to deploy this stack. 
    */
-  bucketName?: string;
+  bucketNamePrefix?: string;
 
   enableTransferAcceleration?: boolean;
 }
@@ -88,16 +92,16 @@ export class StorageIntegrationTestStack extends IntegrationTestStack<
 
 class StorageIntegrationTestEnvironment extends IntegrationTestStackEnvironment<StorageIntegrationTestEnvironmentProps> {
   constructor(
-    scope: Construct,
+    stack: StorageIntegrationTestStack,
     baseName: string,
     props: StorageIntegrationTestEnvironmentProps
   ) {
-    super(scope, baseName, props);
+    super(stack, baseName, props);
 
     // Create the bucket
 
     const bucket = new s3.Bucket(this, "Bucket", {
-      bucketName: props.bucketName,
+      bucketName: props.bucketNamePrefix ? randomBucketName({ prefix: props.bucketNamePrefix, stack: this }) : undefined,
       transferAcceleration: props.enableTransferAcceleration,
       removalPolicy: RemovalPolicy.DESTROY,
       autoDeleteObjects: true,