Skip to content

Commit

Permalink
fix(servicecatalogappregistry): synth error when associating a nested…
Browse files Browse the repository at this point in the history 
… stack (#23248)

In this PR, we are fixing the following:
- For nested stack association, customers have observed a synth error `Nested stack cannot depend on a parent stack`. In this PR we are providing the fix for the same.

- Adding Application manager URL as CDK output. Customers will now have ability to directly access Application Manager for the application created by AppRegistry L2 construct using the link provide in CDK and CFN output.
- Unit test to verify conditional nested stack association can be handled by this L2 construct gracefully as the `ResourceAssociation` is getting created within the child stack.

### All Submissions:

* [ X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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
santanugho authored Dec 14, 2022
1 parent b34d0b7 commit 30301d9
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 47 deletions.
13 changes: 11 additions & 2 deletions packages/@aws-cdk/aws-servicecatalogappregistry/lib/application.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
/**
* Associate stack with the application in the stack passed as parameter.
*
* If the stack is already associated, it will ignore duplicate request.
* A stack can only be associated with one application.
*/
public associateApplicationWithStack(stack: cdk.Stack): void {
Expand All @@ -146,7 +145,7 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
});

this.associatedResources.add(stack.node.addr);
if (stack !== cdk.Stack.of(this) && this.isSameAccount(stack) && !this.isStageScope(stack)) {
if (stack !== cdk.Stack.of(this) && this.isSameAccount(stack) && !this.isStageScope(stack) && !stack.nested) {
stack.addDependency(cdk.Stack.of(this));
}
}
Expand Down Expand Up @@ -251,6 +250,11 @@ export class Application extends ApplicationBase {
});
}

/**
* Application manager URL for the Application.
* @attribute
*/
public readonly applicationManagerUrl?: cdk.CfnOutput;
public readonly applicationArn: string;
public readonly applicationId: string;
public readonly applicationName?: string;
Expand All @@ -270,6 +274,11 @@ export class Application extends ApplicationBase {
this.applicationId = application.attrId;
this.applicationName = props.applicationName;
this.nodeAddress = cdk.Names.nodeUniqueId(application.node);

this.applicationManagerUrl = new cdk.CfnOutput(this, 'ApplicationManagerUrl', {
value: `https://${this.env.region}.console.aws.amazon.com/systems-manager/appmanager/application/AWS_AppRegistry_Application-${this.applicationName}`,
description: `Application manager url for application ${this.applicationName}`,
});
}

protected generateUniqueHash(resourceAddress: string): string {
Expand Down
67 changes: 60 additions & 7 deletions packages/@aws-cdk/aws-servicecatalogappregistry/test/application.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ describe('Application', () => {

test('application with explicit description', () => {
const description = 'my test application description';
new appreg.Application(stack, 'MyApplication', {
const application = new appreg.Application(stack, 'MyApplication', {
applicationName: 'testApplication',
description: description,
});

Template.fromStack(stack).hasOutput('MyApplicationApplicationManagerUrlB79EF34D', {});
expect(application.applicationManagerUrl?.value).toContain('AWS_AppRegistry_Application-testApplication');
Template.fromStack(stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::Application', {
Description: description,
});
Expand Down Expand Up @@ -254,7 +256,7 @@ describe('Application', () => {

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'RAMShare5bb637032063',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -268,7 +270,7 @@ describe('Application', () => {

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'RAMShare5bb637032063',
Principals: ['123456789012'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -284,7 +286,7 @@ describe('Application', () => {

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'RAMShare5bb637032063',
Principals: ['arn:aws:iam::123456789012:role/myRole'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -300,7 +302,7 @@ describe('Application', () => {

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'RAMShare5bb637032063',
Principals: ['arn:aws:iam::123456789012:user/myUser'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -315,7 +317,7 @@ describe('Application', () => {

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'RAMShare5bb637032063',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly'],
Expand All @@ -330,7 +332,7 @@ describe('Application', () => {

Template.fromStack(stack).hasResourceProperties('AWS::RAM::ResourceShare', {
AllowExternalPrincipals: false,
Name: 'RAMSharee6e0e560e6f8',
Name: 'RAMShare5bb637032063',
Principals: ['arn:aws:organizations::123456789012:organization/o-70oi5564q1'],
ResourceArns: [{ 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Arn'] }],
PermissionArns: ['arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationAllowAssociation'],
Expand Down Expand Up @@ -446,8 +448,59 @@ describe('Scope based Associations with Application with Cross Region/Account',
});
});

describe('Conditional nested stack Associations with Application within Same Account', () => {
let app: cdk.App;
beforeEach(() => {
app = new cdk.App({
context: {
'@aws-cdk/core:newStyleStackSynthesis': false,
},
});
});

test('Associate conditional nested stack with application', () => {
const stack = new MainStack(app, 'cdkApplication');
const application = new appreg.Application(stack, 'MyApplication', {
applicationName: 'MyApplication',
});
application.associateApplicationWithStack(stack);
application.associateApplicationWithStack(stack.nestedStack);
Template.fromStack(stack.nestedStack).hasResource('AWS::ServiceCatalogAppRegistry::ResourceAssociation', {
Properties: {
Application: 'MyApplication',
Resource: { Ref: 'AWS::StackId' },
ResourceType: 'CFN_STACK',
},
});
Template.fromStack(stack.nestedStack).hasCondition('ShouldCreateStackCondition', {
'Fn::Equals': ['us-east-1'],
});
});

});


class AppRegistrySampleStack extends cdk.Stack {
public constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);
}
}

class MainStack extends cdk.Stack {
public readonly nestedStack: cdk.Stack;
public constructor(parent: cdk.App, id: string, props?: cdk.StackProps) {
super(parent, id, props);
this.nestedStack = new AppRegistryNestedStack(this, 'nested-stack');
}
}

class AppRegistryNestedStack extends cdk.NestedStack {
public constructor(scope: Construct, id: string, props?: cdk.NestedStackProps) {
super(scope, id, props);

const shouldCreateStack = new cdk.CfnCondition(this, 'ShouldCreateStackCondition', {
expression: cdk.Fn.conditionEquals(process.env.CDK_DEFAULT_REGION, 'us-east-1'),
});
(this.nestedStackResource as cdk.CfnStack).cfnOptions.condition = shouldCreateStack;
}
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.js.snapshot/cdk.out
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"21.0.0"}
{"version":"22.0.0"}
6 changes: 3 additions & 3 deletions ...est/integ.application.js.snapshot/integ-servicecatalogappregistry-application.assets.json
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "20.0.0",
"version": "22.0.0",
"files": {
"d03aa6239eb3b20f4b72fb3dd44a4082d06d7a5451d0ac3855bd1aa78aecfbe9": {
"806ae543572346400832fe865c1bdfa243ef2c1f07c3ce10aa0bd27ed4613a42": {
"source": {
"path": "integ-servicecatalogappregistry-application.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "d03aa6239eb3b20f4b72fb3dd44a4082d06d7a5451d0ac3855bd1aa78aecfbe9.json",
"objectKey": "806ae543572346400832fe865c1bdfa243ef2c1f07c3ce10aa0bd27ed4613a42.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
25 changes: 21 additions & 4 deletions ...t/integ.application.js.snapshot/integ-servicecatalogappregistry-application.template.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"TestApplication2FBC585F": {
"Type": "AWS::ServiceCatalogAppRegistry::Application",
"Properties": {
"Name": "myApplicationtest",
"Name": "myCdkApplication",
"Description": "my application description"
}
},
Expand Down Expand Up @@ -39,10 +39,10 @@
}
}
},
"TestApplicationRAMSharead8ba81b8cdd40199FD1": {
"TestApplicationRAMShare3dc6227daec11BF3E108": {
"Type": "AWS::RAM::ResourceShare",
"Properties": {
"Name": "RAMSharead8ba81b8cdd",
"Name": "RAMShare3dc6227daec1",
"AllowExternalPrincipals": false,
"PermissionArns": [
"arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly"
Expand Down Expand Up @@ -84,7 +84,7 @@
"release": "go time"
}
},
"Name": "myAttributeGroupTest",
"Name": "myCdkAttributeGroup",
"Description": "my attribute group description"
}
},
Expand Down Expand Up @@ -121,6 +121,23 @@
}
}
},
"Outputs": {
"TestApplicationApplicationManagerUrlE1058321": {
"Description": "Application manager url for application myCdkApplication",
"Value": {
"Fn::Join": [
"",
[
"https://",
{
"Ref": "AWS::Region"
},
".console.aws.amazon.com/systems-manager/appmanager/application/AWS_AppRegistry_Application-myCdkApplication"
]
]
}
}
},
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
Expand Down
2 changes: 1 addition & 1 deletion ...ages/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.js.snapshot/integ.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "22.0.0",
"testCases": {
"integ.application": {
"stacks": [
Expand Down
26 changes: 16 additions & 10 deletions ...s/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.js.snapshot/manifest.json
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
{
"version": "20.0.0",
"version": "22.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
"properties": {
"file": "tree.json"
}
},
"integ-servicecatalogappregistry-application.assets": {
"type": "cdk:asset-manifest",
"properties": {
Expand All @@ -23,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}/d03aa6239eb3b20f4b72fb3dd44a4082d06d7a5451d0ac3855bd1aa78aecfbe9.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/806ae543572346400832fe865c1bdfa243ef2c1f07c3ce10aa0bd27ed4613a42.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand All @@ -45,6 +39,12 @@
"data": "TestApplication2FBC585F"
}
],
"/integ-servicecatalogappregistry-application/TestApplication/ApplicationManagerUrl": [
{
"type": "aws:cdk:logicalId",
"data": "TestApplicationApplicationManagerUrlE1058321"
}
],
"/integ-servicecatalogappregistry-application/TestApplication/ResourceAssociationd232b63e52a8": [
{
"type": "aws:cdk:logicalId",
Expand All @@ -57,10 +57,10 @@
"data": "TestApplicationAttributeGroupAssociation4ba7f5842818B8EE1C6F"
}
],
"/integ-servicecatalogappregistry-application/TestApplication/RAMSharead8ba81b8cdd": [
"/integ-servicecatalogappregistry-application/TestApplication/RAMShare3dc6227daec1": [
{
"type": "aws:cdk:logicalId",
"data": "TestApplicationRAMSharead8ba81b8cdd40199FD1"
"data": "TestApplicationRAMShare3dc6227daec11BF3E108"
}
],
"/integ-servicecatalogappregistry-application/TestAttributeGroup/Resource": [
Expand Down Expand Up @@ -89,6 +89,12 @@
]
},
"displayName": "integ-servicecatalogappregistry-application"
},
"Tree": {
"type": "cdk:tree",
"properties": {
"file": "tree.json"
}
}
}
}
Loading

0 comments on commit 30301d9

Please sign in to comment.