Fix issue #15 - RAM Sharing to an OU no longer working (#16)

- Resolves bug around RAM Sharing and Principal ARN construction. - Historically the Ou ARN could be constructed using the 'this' account ID and it would share. That no longer works. - Added the `organizationMainAccountId` to the global section - Added a bypass to use legacy method (this.account) for existing shares that are working `legacyRamShare`. - Updated test cases.
aws-samples · Jul 25, 2023 · 92c0437 · 92c0437
1 parent d6e5b9c
commit 92c0437
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 29 deletions.
diff --git a/config/config-walkthrough.yaml b/config/config-walkthrough.yaml
@@ -6,6 +6,9 @@ global:
   # OPTIONAL:DEPENDANT - If you will RAM share to Organizational units this is required
   # This must start with 'o-'.  You can find it in the Organizations service on the left pane
   organizationId: o-1234
+  # OPTIONAL:DEPENDANT - If you will RAM share to Organizational units this is required
+  # This is the AWS Account identifier for account owning the Organization
+  organizationMainAccountId: 123456789012
   # REQUIRED - This prefix will pre-pend ALL stacks created, and will be used within the SSM Prefix below
   # This assures that you can deploy multiple environments with similar resources without name collision.
   stackNamePrefix: Core
@@ -50,6 +53,10 @@ vpcs:
     availabilityZones:
       - us-east-1a
       - us-east-1b
+    # OPTIONAL DEPENDANT: Historically we could RAM Share a Subnet by using 'this' accounts ID even if 'this' account didn't own the organization.
+    # That ARN construction no longer works.  If you have an existing RAM share under this method and want to not trigger an update, set this value to true
+    # See for more details: https://github.com/aws-samples/aws-vpc-builder-cdk/issues/15
+    legacyRamShare: true
     # OPTIONAL: Normally we dynamically decide whether to attach a VPC to a TGW based on it having routes defined in the transitGateways
     # section.  If you want to force, or prevent this VPC attaching to a TGW, then set this to true (always) or false (never).
     attachTgw: true

diff --git a/config/sample-complex.vpcBuilder.yaml b/config/sample-complex.vpcBuilder.yaml
@@ -1,7 +1,8 @@
 global:
-  # ** Replace with overall organizational ID **
+  # ** Replace with overall organizational ID and Account ID owning the organization **
   # Uncomment below  if you'd like to use RAM Sharing
   # organizationId: o-REPLACEME
+  # organizationMainAccountId: 123456789012
   stackNamePrefix: sample-complex
   ssmPrefix: /sample-complex/network
   region: us-east-1

diff --git a/lib/config/config-schema.json b/lib/config/config-schema.json
@@ -72,6 +72,9 @@
                 "organizationId": {
                     "type": "string"
                 },
+                "organizationMainAccountId": {
+                    "type": "string"
+                },
                 "region": {
                     "type": "string"
                 },
@@ -440,6 +443,9 @@
                     },
                     "type": "array"
                 },
+                "legacyRamShare": {
+                    "type": "boolean"
+                },
                 "providerEndpoints": {
                     "type": "string"
                 },

diff --git a/lib/config/config-types.ts b/lib/config/config-types.ts
@@ -9,6 +9,7 @@ export interface IConfigConfigTag {
 export interface IConfigGlobal {
   stackNamePrefix: string;
   organizationId?: string;
+  organizationMainAccountId?: string;
   tags?: Array<IConfigConfigTag>;
   ssmPrefix: string;
   region: string;
@@ -150,6 +151,7 @@ export interface IConfigVpc {
   availabilityZones?: Array<string>;
   style: IConfigVpcStyles;
   subnets: IConfigVpcNamedSubnets;
+  legacyRamShare?: boolean;
   attachTgw?: boolean;
   providerEndpoints?: string;
   providerInternet?: string;

diff --git a/lib/config/parser.ts b/lib/config/parser.ts
@@ -315,6 +315,18 @@ export class ConfigParser {
             `When sharing with an OU, Global option 'organizationId' must be present and set to the Organization ID (begins with o- from the Organizations service page)`,
           );
         }
+        // Share with an OU.  Verify we have our global organization ID present and it is 12 digits
+        if (this.configRaw.global.organizationMainAccountId) {
+          if (this.configRaw.global.organizationMainAccountId.length != 12) {
+            throw new Error(
+              `Global option organizationMainAccountId must be a 12 digit AWS Account identifier.  Use the ID of the account owning the Organization.`,
+            );
+          }
+        } else {
+          throw new Error(
+            `When sharing with an OU, Global option 'organizationMainAccountId' must be present and set to the AWS Account ID that owns the organization`,
+          );
+        }
       }
     } else {
       // Direct share with an AWS Account ID.  Assure it is 12 digits

diff --git a/lib/stack-builder.ts b/lib/stack-builder.ts
@@ -195,6 +195,8 @@ export class StackBuilderClass {
       const stackProps: IVpcWorkloadProps = {
         globalPrefix: this.c.global.stackNamePrefix,
         organizationId: this.c.global.organizationId,
+        organizationMainAccountId: this.c.global.organizationMainAccountId,
+        legacyRamShare: configStanza.legacyRamShare,
         namePrefix: workloadVpcName,
         availabilityZones: configStanza.availabilityZones
           ? configStanza.availabilityZones

diff --git a/lib/types.ts b/lib/types.ts
@@ -106,6 +106,8 @@ export interface IBuilderVpcProps extends IBuilderBaseProps {
 export interface IVpcWorkloadProps extends IBuilderVpcProps {
   createSubnets: Array<SubnetNamedMasks>;
   organizationId?: string;
+  organizationMainAccountId?: string;
+  legacyRamShare?: boolean
 }
 
 /*

diff --git a/lib/vpc-workload-isolated-stack.ts b/lib/vpc-workload-isolated-stack.ts
@@ -92,15 +92,21 @@ export class VpcWorkloadIsolatedStack extends BuilderVpc {
         ramPrincipals.push(`${sharedWith}`);
       } else {
         const sharedWithString = sharedWith.toString();
+        let organizationMainAccountId = this.props.organizationMainAccountId
+        // Historically we could use the deployment accounts ID to form our OU ARN.  That no longer works
+        // However we want to allow users to annotate existing VPCs to use this old approach to not trigger an update
+        if(this.props.legacyRamShare) {
+          organizationMainAccountId = this.account
+        }
         // Entire Organization share
         if (sharedWithString.startsWith("o-")) {
           ramPrincipals.push(
-            `arn:aws:organizations::${this.account}/${sharedWith}`
+            `arn:aws:organizations::${organizationMainAccountId}/${sharedWith}`
           );
         } else if (sharedWithString.startsWith("ou-")) {
           if (this.props.organizationId) {
             ramPrincipals.push(
-              `arn:aws:organizations::${this.account}:ou/${this.props.organizationId}/${sharedWith}`
+              `arn:aws:organizations::${organizationMainAccountId}:ou/${this.props.organizationId}/${sharedWith}`
             );
           }
         } else {

diff --git a/lib/vpc-workload-public-stack.ts b/lib/vpc-workload-public-stack.ts
@@ -85,14 +85,20 @@ export class VpcWorkloadPublicStack extends BuilderVpc {
       } else {
         const sharedWithString = sharedWith.toString();
         // Entire Organization share
+        let organizationMainAccountId = this.props.organizationMainAccountId
+        // Historically we could use the deployment accounts ID to form our OU ARN.  That no longer works
+        // However we want to allow users to annotate existing VPCs to use this old approach to not trigger an update
+        if(this.props.legacyRamShare) {
+          organizationMainAccountId = this.account
+        }
         if (sharedWithString.startsWith("o-")) {
           ramPrincipals.push(
-            `arn:aws:organizations::${this.account}/${sharedWith}`
+            `arn:aws:organizations::${organizationMainAccountId}/${sharedWith}`
           );
         } else if (sharedWithString.startsWith("ou-")) {
           if (this.props.organizationId) {
             ramPrincipals.push(
-              `arn:aws:organizations::${this.account}:ou/${this.props.organizationId}/${sharedWith}`
+              `arn:aws:organizations::${organizationMainAccountId}:ou/${this.props.organizationId}/${sharedWith}`
             );
           }
         } else {

diff --git a/test/vpc-workload-isolated-stack.test.ts b/test/vpc-workload-isolated-stack.test.ts
@@ -144,6 +144,7 @@ test("WorkloadIsolatedBaseWithSharedSubnets", () => {
         availabilityZones: ["us-east-1a", "us-east-1b"],
         withTgw: true,
         organizationId: "o-12345",
+        organizationMainAccountId: "012345678910",
         createSubnets: [
           {
             name: "testing",
@@ -167,34 +168,12 @@ test("WorkloadIsolatedBaseWithSharedSubnets", () => {
     });
     template.hasResourceProperties("AWS::RAM::ResourceShare", {
       Principals: Match.arrayWith([
-        {
-          "Fn::Join": [
-            "",
-            [
-              "arn:aws:organizations::",
-              {
-                Ref: "AWS::AccountId",
-              },
-              "/o-12345",
-            ],
-          ],
-        },
+        "arn:aws:organizations::012345678910/o-12345",
       ]),
     });
     template.hasResourceProperties("AWS::RAM::ResourceShare", {
       Principals: Match.arrayWith([
-        {
-          "Fn::Join": [
-            "",
-            [
-              "arn:aws:organizations::",
-              {
-                Ref: "AWS::AccountId",
-              },
-              ":ou/o-12345/ou-12345",
-            ],
-          ],
-        },
+        "arn:aws:organizations::012345678910:ou/o-12345/ou-12345",
       ]),
     });
   }