Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(servicecatalogappregistry): default stack name is not meaningful and causes conflict when multiple stacks deployed to the same account-region #23823

Merged
merged 9 commits into from
Feb 8, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ abstract class StackAssociatorBase implements IAspect {
if (Stage.isStage(childNode)) {
var stageAssociated = this.applicationAssociator?.isStageAssociated(childNode);
if (stageAssociated === false) {
this.error(childNode, 'Associate Stage: ' + childNode.stageName + ' to ensure all stacks in your cdk app are associated with AppRegistry. '
this.warning(childNode, 'Associate Stage: ' + childNode.stageName + ' to ensure all stacks in your cdk app are associated with AppRegistry. '
+ 'You can use ApplicationAssociator.associateStage to associate any stage.');
}
}
Expand All @@ -45,16 +45,6 @@ abstract class StackAssociatorBase implements IAspect {
this.application.associateApplicationWithStack(node);
}

/**
* Adds an error annotation to a node.
*
* @param node The scope to add the error to.
* @param message The error message.
*/
private error(node: IConstruct, message: string): void {
Annotations.of(node).addError(message);
}

/**
* Adds a warning annotation to a node.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class CreateTargetApplication extends TargetApplication {
}
public bind(scope: Construct): BindTargetApplicationResult {
const stackId = this.applicationOptions.stackId ?? 'ApplicationAssociatorStack';
(this.applicationOptions.stackName as string) =
this.applicationOptions.stackName || `Application-${this.applicationOptions.applicationName}-Stack`;
(this.applicationOptions.description as string) =
this.applicationOptions.description || 'Stack to create AppRegistry application';
(this.applicationOptions.env as cdk.Environment) =
Expand All @@ -117,7 +119,11 @@ class ExistingTargetApplication extends TargetApplication {
super();
}
public bind(scope: Construct): BindTargetApplicationResult {
const arnComponents = cdk.Arn.split(this.applicationOptions.applicationArnValue, cdk.ArnFormat.SLASH_RESOURCE_SLASH_RESOURCE_NAME);
const applicationId = arnComponents.resourceName;
const stackId = this.applicationOptions.stackId ?? 'ApplicationAssociatorStack';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it looks like stackId originally existed to change the stack name (but was the wrong mechanism for that), can we also deprecate stackId?

Add:

  * @deprecated Use `stackName` instead to control the name of the stack

To its docstring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this will be more helpful to the user. I'll add a deprecated statement to the docstring in the next commit.

(this.applicationOptions.stackName as string) =
this.applicationOptions.stackName || `Application-${applicationId}-Stack`;
const applicationStack = new cdk.Stack(scope, stackId, this.applicationOptions);
const appRegApplication = Application.fromApplicationArn(applicationStack, 'ExistingApplication', this.applicationOptions.applicationArnValue);
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@aws-cdk/aws-codecommit": "0.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
associateStage: false,
});
app.synth();
Annotations.fromStack(pipelineStack).hasError('*',
Annotations.fromStack(pipelineStack).hasWarning('*',
'Associate Stage: SampleStage to ensure all stacks in your cdk app are associated with AppRegistry. You can use ApplicationAssociator.associateStage to associate any stage.');
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"version": "29.0.0",
"files": {
"c4d674e9642d6dbbd0df5c93890473101fc95bbc70de7e28d79ba771b5284de8": {
"source": {
"path": "ApplicationAssociatorStack.template.json",
"packaging": "file"
},
"destinations": {
"416623072619-us-east-1": {
"bucketName": "cdk-hnb659fds-assets-416623072619-us-east-1",
"objectKey": "c4d674e9642d6dbbd0df5c93890473101fc95bbc70de7e28d79ba771b5284de8.json",
"region": "us-east-1",
"assumeRoleArn": "arn:${AWS::Partition}:iam::416623072619:role/cdk-hnb659fds-file-publishing-role-416623072619-us-east-1"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{
"Description": "Stack to create AppRegistry application",
"Resources": {
"DefaultCdkApplication4573D5A3": {
"Type": "AWS::ServiceCatalogAppRegistry::Application",
"Properties": {
"Name": "AppRegistryAssociatedApplication",
"Description": "Application containing stacks deployed via CDK.",
"Tags": {
"managedBy": "CDK_Application_Associator"
}
}
},
"AppRegistryAssociation": {
"Type": "AWS::ServiceCatalogAppRegistry::ResourceAssociation",
"Properties": {
"Application": {
"Fn::GetAtt": [
"DefaultCdkApplication4573D5A3",
"Id"
]
},
"Resource": {
"Ref": "AWS::StackId"
},
"ResourceType": "CFN_STACK"
}
}
},
"Outputs": {
"DefaultCdkApplicationApplicationManagerUrl27C138EF": {
"Description": "Application manager url for the application created.",
"Value": "https://us-east-1.console.aws.amazon.com/systems-manager/appmanager/application/AWS_AppRegistry_Application-AppRegistryAssociatedApplication"
}
},
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "29.0.0",
"files": {
"19dd33f3c17e59cafd22b9459b0a8d9bedbd42252737fedb06b2bcdbcf7809cc": {
"source": {
"path": "ApplicationAssociatorTestDefaultTestDeployAssert2A5F2DB9.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "19dd33f3c17e59cafd22b9459b0a8d9bedbd42252737fedb06b2bcdbcf7809cc.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
"Resources": {
"AppRegistryAssociation": {
"Type": "AWS::ServiceCatalogAppRegistry::ResourceAssociation",
"Properties": {
"Application": "AppRegistryAssociatedApplication",
"Resource": {
"Ref": "AWS::StackId"
},
"ResourceType": "CFN_STACK"
}
}
},
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"version":"29.0.0"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "29.0.0",
"files": {
"19dd33f3c17e59cafd22b9459b0a8d9bedbd42252737fedb06b2bcdbcf7809cc": {
"source": {
"path": "integ-servicecatalogappregistry-application.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "19dd33f3c17e59cafd22b9459b0a8d9bedbd42252737fedb06b2bcdbcf7809cc.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
"Resources": {
"AppRegistryAssociation": {
"Type": "AWS::ServiceCatalogAppRegistry::ResourceAssociation",
"Properties": {
"Application": "AppRegistryAssociatedApplication",
"Resource": {
"Ref": "AWS::StackId"
},
"ResourceType": "CFN_STACK"
}
}
},
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"version": "29.0.0",
"testCases": {
"ApplicationAssociatorTest/DefaultTest": {
"stacks": [
"integ-servicecatalogappregistry-application"
],
"assertionStack": "ApplicationAssociatorTest/DefaultTest/DeployAssert",
"assertionStackName": "ApplicationAssociatorTestDefaultTestDeployAssert2A5F2DB9"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "29.0.0",
"files": {
"19dd33f3c17e59cafd22b9459b0a8d9bedbd42252737fedb06b2bcdbcf7809cc": {
"source": {
"path": "integservicecatalogappregistryapplicationresourcesStack4399A149.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "19dd33f3c17e59cafd22b9459b0a8d9bedbd42252737fedb06b2bcdbcf7809cc.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
"Resources": {
"AppRegistryAssociation": {
"Type": "AWS::ServiceCatalogAppRegistry::ResourceAssociation",
"Properties": {
"Application": "AppRegistryAssociatedApplication",
"Resource": {
"Ref": "AWS::StackId"
},
"ResourceType": "CFN_STACK"
}
}
},
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Loading