fix(servicecatalogappregistry): RAM Share is replaced on every change…

… to Application (aws#24760) `Application.shareApplication` can be called multiple times which generates different resources within the same `Application` construct. The share construct id generation was based on child length of `Application` construct. This makes every change to the `Application` construct can unnecessarily replace the RAM share when the share actually needs no update. Similar to the solution for `addAttributeGroup` introduced in the [previous update](aws#24672), this commit starts to require both construct id and share name in the `Application.shareApplication` method input, which are used to create RAM share construct. For `ApplicationAssociator`, `Application.shareApplication` is called for cross-account stack associations. Share construct id depends on the node unique id of the application in `ApplicationAssociator` and the node unique id of the stack to be associated. This reduces unnecessary share replacement in `ApplicationAssociator` stack. BREAKING CHANGE: This commit involves share replacement during the deployment of `ApplicationAssociator` due to share construct id update. After this change, frequent share replacements due to structural change in `Application` construct should be avoided. `Application.shareApplication` starts to require construct id (first argument) and share name (added in `ShareOption`) as input. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk · Mar 28, 2023 · 0d16dc7 · 0d16dc7
1 parent 8b36442
commit 0d16dc7
Show file tree

Hide file tree

Showing 19 changed files with 111 additions and 83 deletions.
diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/README.md b/packages/@aws-cdk/aws-servicecatalogappregistry/README.md
@@ -289,7 +289,8 @@ import * as iam from '@aws-cdk/aws-iam';
 declare const application: appreg.Application;
 declare const myRole: iam.IRole;
 declare const myUser: iam.IUser;
-application.shareApplication({
+application.shareApplication('MyShareId', {
+  name:'MyShare',
   accounts: ['123456789012'],
   organizationArns: ['arn:aws:organizations::123456789012:organization/o-my-org-id'],
   roles: [myRole],
@@ -302,7 +303,8 @@ E.g., sharing an application with multiple accounts and allowing the accounts to
 ```ts
 import * as iam from '@aws-cdk/aws-iam';
 declare const application: appreg.Application;
-application.shareApplication({
+application.shareApplication('MyShareId', {
+  name: 'MyShare',
   accounts: ['123456789012', '234567890123'],
   sharePermission: appreg.SharePermission.ALLOW_ACCESS,
 });
@@ -315,7 +317,8 @@ import * as iam from '@aws-cdk/aws-iam';
 declare const attributeGroup: appreg.AttributeGroup;
 declare const myRole: iam.IRole;
 declare const myUser: iam.IUser;
-attributeGroup.shareAttributeGroup({
+attributeGroup.shareAttributeGroup('MyShareId', {
+  name: 'MyShare',
   accounts: ['123456789012'],
   organizationArns: ['arn:aws:organizations::123456789012:organization/o-my-org-id'],
   roles: [myRole],
@@ -328,7 +331,8 @@ E.g., sharing an application with multiple accounts and allowing the accounts to
 ```ts
 import * as iam from '@aws-cdk/aws-iam';
 declare const attributeGroup: appreg.AttributeGroup;
-attributeGroup.shareAttributeGroup({
+attributeGroup.shareAttributeGroup('MyShareId', {
+  name: 'MyShare',
   accounts: ['123456789012', '234567890123'],
   sharePermission: appreg.SharePermission.ALLOW_ACCESS,
 });

diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/lib/application.ts b/packages/@aws-cdk/aws-servicecatalogappregistry/lib/application.ts
@@ -1,6 +1,5 @@
 import { CfnResourceShare } from '@aws-cdk/aws-ram';
 import * as cdk from '@aws-cdk/core';
-import { Names } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { StageStackAssociator } from './aspects/stack-associator';
 import { AttributeGroup, IAttributeGroup } from './attribute-group';
@@ -90,9 +89,10 @@ export interface IApplication extends cdk.IResource {
   /**
    * Share this application with other IAM entities, accounts, or OUs.
    *
+   * @param id The construct name for the share.
    * @param shareOptions The options for the share.
    */
-  shareApplication(shareOptions: ShareOptions): void;
+  shareApplication(id: string, shareOptions: ShareOptions): void;
 
   /**
    * Associate this application with all stacks under the construct node.
@@ -205,13 +205,13 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
    * Share an application with accounts, organizations and OUs, and IAM roles and users.
    * The application will become available to end users within those principals.
    *
+   * @param id The construct name for the share.
    * @param shareOptions The options for the share.
    */
-  public shareApplication(shareOptions: ShareOptions): void {
+  public shareApplication(id: string, shareOptions: ShareOptions): void {
     const principals = getPrincipalsforSharing(shareOptions);
-    const shareName = `RAMShare${hashValues(Names.nodeUniqueId(this.node), this.node.children.length.toString())}`;
-    new CfnResourceShare(this, shareName, {
-      name: shareName,
+    new CfnResourceShare(this, id, {
+      name: shareOptions.name,
       allowExternalPrincipals: false,
       principals: principals,
       resourceArns: [this.applicationArn],

diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/lib/aspects/stack-associator.ts b/packages/@aws-cdk/aws-servicecatalogappregistry/lib/aspects/stack-associator.ts
@@ -1,8 +1,8 @@
-import { IAspect, Stack, Stage, Annotations } from '@aws-cdk/core';
+import { IAspect, Stack, Stage, Annotations, Names } from '@aws-cdk/core';
 import { IConstruct } from 'constructs';
 import { IApplication } from '../application';
 import { ApplicationAssociator } from '../application-associator';
-import { SharePermission } from '../common';
+import { hashValues, SharePermission } from '../common';
 import { isRegionUnresolved, isAccountUnresolved } from '../private/utils';
 
 export interface StackAssociatorBaseProps {
@@ -112,7 +112,9 @@ abstract class StackAssociatorBase implements IAspect {
 
     if (node.account != this.application.env.account && !this.sharedAccounts.has(node.account)) {
       if (this.associateCrossAccountStacks) {
-        this.application.shareApplication({
+        const shareId = `ApplicationShare${hashValues(Names.nodeUniqueId(this.application.node), Names.nodeUniqueId(node.node))}`;
+        this.application.shareApplication(shareId, {
+          name: shareId,
           accounts: [node.account],
           sharePermission: SharePermission.ALLOW_ACCESS,
         });

diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/lib/attribute-group.ts b/packages/@aws-cdk/aws-servicecatalogappregistry/lib/attribute-group.ts
@@ -1,6 +1,5 @@
 import { CfnResourceShare } from '@aws-cdk/aws-ram';
 import * as cdk from '@aws-cdk/core';
-import { Names } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { IApplication } from './application';
 import { getPrincipalsforSharing, hashValues, ShareOptions, SharePermission } from './common';
@@ -29,9 +28,10 @@ export interface IAttributeGroup extends cdk.IResource {
   /**
    * Share the attribute group resource with other IAM entities, accounts, or OUs.
    *
+   * @param id The construct name for the share.
    * @param shareOptions The options for the share.
    */
-  shareAttributeGroup(shareOptions: ShareOptions): void;
+  shareAttributeGroup(id: string, shareOptions: ShareOptions): void;
 }
 
 /**
@@ -77,11 +77,10 @@ abstract class AttributeGroupBase extends cdk.Resource implements IAttributeGrou
     }
   }
 
-  public shareAttributeGroup(shareOptions: ShareOptions): void {
+  public shareAttributeGroup(id: string, shareOptions: ShareOptions): void {
     const principals = getPrincipalsforSharing(shareOptions);
-    const shareName = `RAMShare${hashValues(Names.nodeUniqueId(this.node), this.node.children.length.toString())}`;
-    new CfnResourceShare(this, shareName, {
-      name: shareName,
+    new CfnResourceShare(this, id, {
+      name: shareOptions.name,
       allowExternalPrincipals: false,
       principals: principals,
       resourceArns: [this.attributeGroupArn],

diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/lib/common.ts b/packages/@aws-cdk/aws-servicecatalogappregistry/lib/common.ts
@@ -20,6 +20,11 @@ export enum SharePermission {
  * The options that are passed into a share of an Application or Attribute Group.
  */
 export interface ShareOptions {
+  /**
+   * Name of the share.
+   *
+   */
+  readonly name: string;
   /**
    * A list of AWS accounts that the application will be shared with.
    *

diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/test/application.test.ts b/packages/@aws-cdk/aws-servicecatalogappregistry/test/application.test.ts
@@ -269,32 +269,36 @@ describe('Application', () => {
 
     test('fails for sharing application without principals', () => {
       expect(() => {
-        application.shareApplication({});
+        application.shareApplication('MyShareId', {
+          name: 'MyShare',
+        });
       }).toThrow(/An entity must be provided for the share/);
     });
 
     test('share application with an organization', () => {
-      application.shareApplication({
+      application.shareApplication('MyShareId', {
+        name: 'MyShare',
         organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMSharee6e0e560e6f8',
+        Name: 'MyShare',
         Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
       });
     });
 
     test('share application with an account', () => {
-      application.shareApplication({
+      application.shareApplication('MyShareId', {
+        name: 'MyShare',
         accounts: ['123456789012'],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMSharee6e0e560e6f8',
+        Name: 'MyShare',
         Principals: ['123456789012'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
@@ -304,13 +308,14 @@ describe('Application', () => {
     test('share application with an IAM role', () => {
       const myRole = iam.Role.fromRoleArn(stack, 'MyRole', 'arn:aws:iam::123456789012:role/myRole');
 
-      application.shareApplication({
+      application.shareApplication('MyShareId', {
+        name: 'MyShare',
         roles: [myRole],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMSharee6e0e560e6f8',
+        Name: 'MyShare',
         Principals: ['arn:aws:iam::123456789012:role/myRole'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
@@ -320,43 +325,46 @@ describe('Application', () => {
     test('share application with an IAM user', () => {
       const myUser = iam.User.fromUserArn(stack, 'MyUser', 'arn:aws:iam::123456789012:user/myUser');
 
-      application.shareApplication({
+      application.shareApplication('MyShareId', {
+        name: 'MyShare',
         users: [myUser],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMSharee6e0e560e6f8',
+        Name: 'MyShare',
         Principals: ['arn:aws:iam::123456789012:user/myUser'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
       });
     });
 
     test('share application with organization, give explicit read only access to an application', () => {
-      application.shareApplication({
+      application.shareApplication('MyShareId', {
+        name: 'MyShare',
         organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         sharePermission: appreg.SharePermission.READ_ONLY,
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMSharee6e0e560e6f8',
+        Name: 'MyShare',
         Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
       });
     });
 
     test('share application with organization, allow access to associate resources and attribute group with an application', () => {
-      application.shareApplication({
+      application.shareApplication('MyShareId', {
+        name: 'MyShare',
         organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         sharePermission: appreg.SharePermission.ALLOW_ACCESS,
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMSharee6e0e560e6f8',
+        Name: 'MyShare',
         Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationAllowAssociation'],

diff --git a/packages/@aws-cdk/aws-servicecatalogappregistry/test/attribute-group.test.ts b/packages/@aws-cdk/aws-servicecatalogappregistry/test/attribute-group.test.ts
@@ -212,32 +212,36 @@ describe('Attribute Group', () => {
 
     test('fails for sharing attribute group without principals', () => {
       expect(() => {
-        attributeGroup.shareAttributeGroup({});
+        attributeGroup.shareAttributeGroup('MyShareId', {
+          name: 'MyShare',
+        });
       }).toThrow(/An entity must be provided for the share/);
     });
 
     test('share attribute group with an organization', () => {
-      attributeGroup.shareAttributeGroup({
+      attributeGroup.shareAttributeGroup('MyShareId', {
+        name: 'MyShare',
         organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMShare76d2681489c0',
+        Name: 'MyShare',
         Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
       });
     });
 
     test('share attribute group with an account', () => {
-      attributeGroup.shareAttributeGroup({
+      attributeGroup.shareAttributeGroup('MyShareId', {
+        name: 'MyShare',
         accounts: ['123456789012'],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMShare76d2681489c0',
+        Name: 'MyShare',
         Principals: ['123456789012'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
@@ -247,13 +251,14 @@ describe('Attribute Group', () => {
     test('share attribute group with an IAM role', () => {
       const myRole = iam.Role.fromRoleArn(stack, 'MyRole', 'arn:aws:iam::123456789012:role/myRole');
 
-      attributeGroup.shareAttributeGroup({
+      attributeGroup.shareAttributeGroup('MyShareId', {
+        name: 'MyShare',
         roles: [myRole],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMShare76d2681489c0',
+        Name: 'MyShare',
         Principals: ['arn:aws:iam::123456789012:role/myRole'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
@@ -263,43 +268,46 @@ describe('Attribute Group', () => {
     test('share attribute group with an IAM user', () => {
       const myUser = iam.User.fromUserArn(stack, 'MyUser', 'arn:aws:iam::123456789012:user/myUser');
 
-      attributeGroup.shareAttributeGroup({
+      attributeGroup.shareAttributeGroup('MyShareId', {
+        name: 'MyShare',
         users: [myUser],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMShare76d2681489c0',
+        Name: 'MyShare',
         Principals: ['arn:aws:iam::123456789012:user/myUser'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
       });
     });
 
     test('share attribute group with organization, give explicit read only access to the attribute group', () => {
-      attributeGroup.shareAttributeGroup({
+      attributeGroup.shareAttributeGroup('MyShareId', {
+        name: 'MyShare',
         organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         sharePermission: appreg.SharePermission.READ_ONLY,
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMShare76d2681489c0',
+        Name: 'MyShare',
         Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly'],
       });
     });
 
     test('share attribute group with organization, give access to mutate attribute groups', () => {
-      attributeGroup.shareAttributeGroup({
+      attributeGroup.shareAttributeGroup('MyShareId', {
+        name: 'MyShare',
         organizationArns: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         sharePermission: appreg.SharePermission.ALLOW_ACCESS,
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
         AllowExternalPrincipals: false,
-        Name: 'RAMShare76d2681489c0',
+        Name: 'MyShare',
         Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
         ResourceArns: [{ 'Fn::GetAtt': ['MyAttributeGroup99099500', 'Arn'] }],
         PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupAllowAssociation'],

diff --git a/...est/integ.application.js.snapshot/integ-servicecatalogappregistry-application.assets.json b/...est/integ.application.js.snapshot/integ-servicecatalogappregistry-application.assets.json
@@ -1,15 +1,15 @@
 {
   "version": "31.0.0",
   "files": {
-    "5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765": {
+    "461d235e9497deb16b9209be4a927c7d0dc7aa06d668e38bfb19a90db8e4a4b2": {
       "source": {
         "path": "integ-servicecatalogappregistry-application.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765.json",
+          "objectKey": "461d235e9497deb16b9209be4a927c7d0dc7aa06d668e38bfb19a90db8e4a4b2.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }

diff --git a/...t/integ.application.js.snapshot/integ-servicecatalogappregistry-application.template.json b/...t/integ.application.js.snapshot/integ-servicecatalogappregistry-application.template.json
@@ -79,10 +79,10 @@
     }
    }
   },
-  "TestApplicationRAMShare004736f08f8e57044D5D": {
+  "TestApplicationMyShareIdE1044482": {
    "Type": "AWS::RAM::ResourceShare",
    "Properties": {
-    "Name": "RAMShare004736f08f8e",
+    "Name": "MyShare",
     "AllowExternalPrincipals": false,
     "PermissionArns": [
      "arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly"