Skip to content

Commit

Permalink
fix(servicecatalogappregistry): Associate an application with attribu…
Browse files Browse the repository at this point in the history
…te group (aws#24378)

`Application-AttributeGroup` association happens in `ApplicationStack`. 
Therefore before the deployment of `ApplicationStack`, `AttributeGroup` stack should have been deployed. 
But with `ApplicationAssociator `where we associate all the stacks with the application (created as a part of `ApplicationAssociator`), attributeGroup stack now depends upon `ApplicationAssociator` stack to be created (since ResourceAssociation happens inside ResourceStack).
This creates a circular dependency and hence cdk synth fails. We found this bug during internal testing
This PR address this circular dependency issue, by allowing the customers of `ApplicationAssociator` to associate an attribute group in attribute group stack.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
santanugho authored and homakk committed Mar 28, 2023
1 parent 345a932 commit a8a6b61
Show file tree
Hide file tree
Showing 17 changed files with 180 additions and 25 deletions.
49 changes: 49 additions & 0 deletions packages/@aws-cdk/aws-servicecatalogappregistry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,45 @@ const associatedApp = new appreg.ApplicationAssociator(app, 'AssociatedApplicati
});
```

If you want to associate an Attribute Group with application created by `ApplicationAssociator`, then use as shown in the example below:

```ts
import * as cdk from "@aws-cdk/core";

const app = new App();

class CustomAppRegistryAttributeGroup extends cdk.Stack {
public readonly attributeGroup: appreg.AttributeGroup
constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);
const myAttributeGroup = new appreg.AttributeGroup(app, 'MyFirstAttributeGroup', {
attributeGroupName: 'MyAttributeGroupName',
description: 'Test attribute group',
attributes: {},
});

this.attributeGroup = myAttributeGroup;
}
}

const customAttributeGroup = new CustomAppRegistryAttributeGroup(app, 'AppRegistryAttributeGroup');

const associatedApp = new appreg.ApplicationAssociator(app, 'AssociatedApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
applicationName: 'MyAssociatedApplication',
// 'Application containing stacks deployed via CDK.' is the default
applicationDescription: 'Associated Application description',
stackName: 'MyAssociatedApplicationStack',
// AWS Account and Region that are implied by the current CLI configuration is the default
env: { account: '123456789012', region: 'us-east-1' },
})],
});

// Associate application to the attribute group.
customAttributeGroup.attributeGroup.associateWith(associatedApp.appRegistryApplication());

```

If you are using CDK Pipelines to deploy your application, the application stacks will be inside Stages, and
ApplicationAssociator will not be able to find them. Call `associateStage` on each Stage object before adding it to the
Pipeline, as shown in the example below:
Expand Down Expand Up @@ -191,6 +230,16 @@ declare const attributeGroup: appreg.AttributeGroup;
application.associateAttributeGroup(attributeGroup);
```

### Associating an attribute group with application

You can associate an application with an attribute group with `associateWith`:

```ts
declare const application: appreg.Application;
declare const attributeGroup: appreg.AttributeGroup;
attributeGroup.associateWith(application);
```

### Associating application with a Stack

You can associate a stack with an application with the `associateStack()` API:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
/**
* Associate an attribute group with application
* If the attribute group is already associated, it will ignore duplicate request.
*
* @deprecated Use `AttributeGroup.associateWith` instead.
*/
public associateAttributeGroup(attributeGroup: IAttributeGroup): void {
if (!this.associatedAttributeGroups.has(attributeGroup.node.addr)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ 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';
import { InputValidator } from './private/validation';
import { CfnAttributeGroup } from './servicecatalogappregistry.generated';
import { CfnAttributeGroup, CfnAttributeGroupAssociation } from './servicecatalogappregistry.generated';

const ATTRIBUTE_GROUP_READ_ONLY_RAM_PERMISSION_ARN = 'arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupReadOnly';
const ATTRIBUTE_GROUP_ALLOW_ACCESS_RAM_PERMISSION_ARN = 'arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryAttributeGroupAllowAssociation';
Expand Down Expand Up @@ -58,6 +59,23 @@ export interface AttributeGroupProps {
abstract class AttributeGroupBase extends cdk.Resource implements IAttributeGroup {
public abstract readonly attributeGroupArn: string;
public abstract readonly attributeGroupId: string;
private readonly associatedApplications: Set<string> = new Set();

/**
* Associate an application with attribute group
* If the attribute group is already associated, it will ignore duplicate request.
*/
public associateWith(application: IApplication): void {
if (!this.associatedApplications.has(application.node.addr)) {
const hashId = this.generateUniqueHash(application.node.addr);
new CfnAttributeGroupAssociation(this, `ApplicationAttributeGroupAssociation${hashId}`, {
application: application.stack === cdk.Stack.of(this) ? application.applicationId : application.applicationName ?? application.applicationId,
attributeGroup: this.attributeGroupId,
});

this.associatedApplications.add(application.node.addr);
}
}

public shareAttributeGroup(shareOptions: ShareOptions): void {
const principals = getPrincipalsforSharing(shareOptions);
Expand Down Expand Up @@ -85,6 +103,11 @@ abstract class AttributeGroupBase extends cdk.Resource implements IAttributeGrou
return shareOptions.sharePermission ?? ATTRIBUTE_GROUP_READ_ONLY_RAM_PERMISSION_ARN;
}
}

/**
* Create a unique hash
*/
protected abstract generateUniqueHash(resourceAddress: string): string;
}

/**
Expand All @@ -109,6 +132,10 @@ export class AttributeGroup extends AttributeGroupBase implements IAttributeGrou
class Import extends AttributeGroupBase {
public readonly attributeGroupArn = attributeGroupArn;
public readonly attributeGroupId = attributeGroupId!;

protected generateUniqueHash(resourceAddress: string): string {
return hashValues(this.attributeGroupArn, resourceAddress);
}
}

return new Import(scope, id, {
Expand All @@ -118,6 +145,7 @@ export class AttributeGroup extends AttributeGroupBase implements IAttributeGrou

public readonly attributeGroupArn: string;
public readonly attributeGroupId: string;
private readonly nodeAddress: string;

constructor(scope: Construct, id: string, props: AttributeGroupProps) {
super(scope, id);
Expand All @@ -132,6 +160,11 @@ export class AttributeGroup extends AttributeGroupBase implements IAttributeGrou

this.attributeGroupArn = attributeGroup.attrArn;
this.attributeGroupId = attributeGroup.attrId;
this.nodeAddress = cdk.Names.nodeUniqueId(attributeGroup.node);
}

protected generateUniqueHash(resourceAddress: string): string {
return hashValues(this.nodeAddress, resourceAddress);
}

private validateAttributeGroupProps(props: AttributeGroupProps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,38 @@ describe('Scope based Associations with Application within Same Account', () =>
});
});
});

describe('Associate attribute group with Application', () => {
let app: cdk.App;
beforeEach(() => {
app = new cdk.App({
context: {
'@aws-cdk/core:newStyleStackSynthesis': false,
},
});
});

test('Associate Attribute Group with application created by ApplicationAssociator', () => {

const customAttributeGroup = new CustomAppRegistryAttributeGroup(app, 'AppRegistryAttributeGroup');

const appAssociator = new appreg.ApplicationAssociator(app, 'TestApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
applicationName: 'TestAssociatedApplication',
stackName: 'TestAssociatedApplicationStack',
})],
});

customAttributeGroup.attributeGroup.associateWith(appAssociator.appRegistryApplication());
Template.fromStack(customAttributeGroup.attributeGroup.stack).resourceCountIs('AWS::ServiceCatalogAppRegistry::AttributeGroupAssociation', 1);
Template.fromStack(customAttributeGroup.attributeGroup.stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::AttributeGroupAssociation', {
Application: 'TestAssociatedApplication',
AttributeGroup: { 'Fn::GetAtt': ['MyFirstAttributeGroupDBC21379', 'Id'] },
});

});
});

describe('Scope based Associations with Application with Cross Region/Account', () => {
let app: cdk.App;
beforeEach(() => {
Expand Down Expand Up @@ -211,3 +243,18 @@ class AppRegistrySampleStack extends cdk.Stack {
super(scope, id, props);
}
}

class CustomAppRegistryAttributeGroup extends cdk.Stack {
public readonly attributeGroup: appreg.AttributeGroup;

constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);
const myAttributeGroup = new appreg.AttributeGroup(this, 'MyFirstAttributeGroup', {
attributeGroupName: 'MyFirstAttributeGroupName',
description: 'Test attribute group',
attributes: {},
});

this.attributeGroup = myAttributeGroup;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,30 @@ describe('Attribute Group', () => {
});
});

describe('Associate application to an attribute group', () => {
let attributeGroup: appreg.AttributeGroup;

beforeEach(() => {
attributeGroup = new appreg.AttributeGroup(stack, 'MyAttributeGroupForAssociation', {
attributeGroupName: 'MyAttributeGroupForAssociation',
attributes: {},
});
});

test('Associate an application to an attribute group', () => {
const application = new appreg.Application(stack, 'MyApplication', {
applicationName: 'MyTestApplication',
});
attributeGroup.associateWith(application);
Template.fromStack(stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::AttributeGroupAssociation', {
Application: { 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Id'] },
AttributeGroup: { 'Fn::GetAtt': ['MyAttributeGroupForAssociation6B3E1329', 'Id'] },
});

});

});

describe('Resource sharing of an attribute group', () => {
let attributeGroup: appreg.AttributeGroup;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "30.1.0",
"files": {
"6a426aab2239a5fb580c074adbf5b8e3acefa04209423d2b53989c73aed3f95b": {
"0d4e060fe5da6b164b9df46b0dc0cd20e7962c6cb531ffe08e6e5b99418f13de": {
"source": {
"path": "integ-servicecatalogappregistry-application.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "6a426aab2239a5fb580c074adbf5b8e3acefa04209423d2b53989c73aed3f95b.json",
"objectKey": "0d4e060fe5da6b164b9df46b0dc0cd20e7962c6cb531ffe08e6e5b99418f13de.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"TestApplication2FBC585F": {
"Type": "AWS::ServiceCatalogAppRegistry::Application",
"Properties": {
"Name": "MyTestApplication",
"Description": "Test application description"
"Name": "TestApplication",
"Description": "My application description"
}
},
"TestApplicationResourceAssociationd232b63e52a8414E905D": {
Expand Down Expand Up @@ -132,7 +132,7 @@
{
"Ref": "AWS::Region"
},
".console.aws.amazon.com/systems-manager/appmanager/application/AWS_AppRegistry_Application-MyTestApplication"
".console.aws.amazon.com/systems-manager/appmanager/application/AWS_AppRegistry_Application-TestApplication"
]
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/6a426aab2239a5fb580c074adbf5b8e3acefa04209423d2b53989c73aed3f95b.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/0d4e060fe5da6b164b9df46b0dc0cd20e7962c6cb531ffe08e6e5b99418f13de.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
"attributes": {
"aws:cdk:cloudformation:type": "AWS::ServiceCatalogAppRegistry::Application",
"aws:cdk:cloudformation:props": {
"name": "MyTestApplication",
"description": "Test application description"
"name": "TestApplication",
"description": "My application description"
}
},
"constructInfo": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const app = new cdk.App();
const stack = new cdk.Stack(app, 'integ-servicecatalogappregistry-application');

const application = new appreg.Application(stack, 'TestApplication', {
applicationName: 'MyTestApplication',
description: 'Test application description',
applicationName: 'TestApplication',
description: 'My application description',
});

const attributeGroup = new appreg.AttributeGroup(stack, 'TestAttributeGroup', {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"22.0.0"}
{"version":"30.1.0"}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "22.0.0",
"version": "30.1.0",
"files": {
"3dece22dad73361a79cb380f2880362a20ffc5c0cc75ddc6707e26b5a88cf93f": {
"9d37fdefa4311937f8f73f9556f1d9a03a2874545a0a262fd42bfde3823ab551": {
"source": {
"path": "integ-servicecatalogappregistry-attribute-group.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "3dece22dad73361a79cb380f2880362a20ffc5c0cc75ddc6707e26b5a88cf93f.json",
"objectKey": "9d37fdefa4311937f8f73f9556f1d9a03a2874545a0a262fd42bfde3823ab551.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
"beta": "time2"
}
},
"Name": "myAttributeGroupTest",
"Description": "my attribute group description"
"Name": "myFirstAttributeGroup",
"Description": "test attribute group description"
}
},
"TestAttributeGroupRAMSharec67f7d80e5baA10EFB4E": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "22.0.0",
"version": "30.1.0",
"testCases": {
"integ.attribute-group": {
"stacks": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "22.0.0",
"version": "30.1.0",
"artifacts": {
"integ-servicecatalogappregistry-attribute-group.assets": {
"type": "cdk:asset-manifest",
Expand All @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/3dece22dad73361a79cb380f2880362a20ffc5c0cc75ddc6707e26b5a88cf93f.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/9d37fdefa4311937f8f73f9556f1d9a03a2874545a0a262fd42bfde3823ab551.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"beta": "time2"
}
},
"name": "myAttributeGroupTest",
"description": "my attribute group description"
"name": "myFirstAttributeGroup",
"description": "test attribute group description"
}
},
"constructInfo": {
Expand Down Expand Up @@ -228,7 +228,7 @@
"path": "Tree",
"constructInfo": {
"fqn": "constructs.Construct",
"version": "10.1.189"
"version": "10.1.252"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const app = new cdk.App();
const stack = new cdk.Stack(app, 'integ-servicecatalogappregistry-attribute-group');

const attributeGroup = new appreg.AttributeGroup(stack, 'TestAttributeGroup', {
attributeGroupName: 'myAttributeGroupTest',
description: 'my attribute group description',
attributeGroupName: 'myFirstAttributeGroup',
description: 'test attribute group description',
attributes: {
stage: 'alpha',
teamMembers: [
Expand Down

0 comments on commit a8a6b61

Please sign in to comment.