Skip to content

Commit

Permalink
feat(servicecatalogappregistry-alpha): Introduce flag to control appl…
Browse files Browse the repository at this point in the history
…ication sharing and association behavior for cross-account stacks (#24408)

Problem:
* Currently, the ApplicationAssociator construct automatically shares the target Application with any accounts of cross-account stacks. [[code reference](https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-servicecatalogappregistry/lib/aspects/stack-associator.ts#L91-L95)]
* If the owner of a cross-account stack is not part of the same AWS Organization as the owner of the ApplicationAssociator stack, or otherwise have not enabled cross-account sharing, during deployment the ApplicationAssociator will fail when attempting to share the application with the stack owner, with a message like below:

```
Principal 123456789012 is not in your AWS organization. You do not have permission to add external
AWS accounts to a resource share. 
(Service: AWSRAM; Status Code: 400; Error Code: OperationNotPermittedException;
Request ID: aaa; Proxy: null)
```

Feature:
* We want to introduce a mechanism (`associateCrossAccountStacks` field in TargetApplicationOptions) where the user can specify if they want to allow sharing their application to any accounts of cross-account stacks in order to then subsequently associate the stack with the application.
* This flag will be `false` by default. This allows customers to have their stack deployments proceed without being blocked on application sharing or cross-account associations.
    * If set to `false`,  ApplicationAssociator will skip the application sharing and association for cross-account stacks. During synthesis, a warning will be displayed to notify that cross-account stacks were detected but sharing and association will be skipped.
    *  If set to `true`, the application will be shared and then associated for cross-account stacks. This relies on the user properly setting up cross-account sharing beforehand.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
jungle-amazon committed Mar 15, 2023
1 parent 274c3d5 commit 2167289
Show file tree
Hide file tree
Showing 19 changed files with 1,051 additions and 17 deletions.
18 changes: 18 additions & 0 deletions packages/@aws-cdk/aws-servicecatalogappregistry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,24 @@ const cdkPipeline = new ApplicationPipelineStack(app, 'CDKApplicationPipelineSta
});
```

By default, ApplicationAssociator will not perform cross-account stack associations with the target Application,
to avoid deployment failures for accounts which have not been setup for cross-account associations.
To enable cross-account stack associations, make sure all accounts are in the same organization as the
target Application's account and that resource sharing is enabled within the organization.
If you wish to turn on cross-account sharing and associations, set the `associateCrossAccountStacks` field to `true`,
as shown in the example below:

```ts
const app = new App();
const associatedApp = new appreg.ApplicationAssociator(app, 'AssociatedApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
associateCrossAccountStacks: true,
applicationName: 'MyAssociatedApplication',
env: { account: '123456789012', region: 'us-east-1' },
})],
});
```

## Attribute Group

An AppRegistry attribute group acts as a container for user-defined attributes for an application.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ export interface ApplicationAssociatorProps {
* in case of a `Pipeline` stack, stage underneath the pipeline will not automatically be associated and
* needs to be associated separately.
*
* If cross account stack is detected, then this construct will automatically share the application to consumer accounts.
* If cross account stack is detected and `associateCrossAccountStacks` in `TargetApplicationOptions` is `true`,
* then the application will automatically be shared with the consumer accounts to allow associations.
* Otherwise, the application will not be shared.
* Cross account feature will only work for non environment agnostic stacks.
*/
export class ApplicationAssociator extends Construct {
Expand All @@ -33,6 +35,7 @@ export class ApplicationAssociator extends Construct {
*/
private readonly application: IApplication;
private readonly associatedStages: Set<cdk.Stage> = new Set();
private readonly associateCrossAccountStacks?: boolean;

constructor(scope: cdk.App, id: string, props: ApplicationAssociatorProps) {
super(scope, id);
Expand All @@ -42,8 +45,12 @@ export class ApplicationAssociator extends Construct {
}

const targetApplication = props.applications[0];
this.application = targetApplication.bind(scope).application;
cdk.Aspects.of(scope).add(new CheckedStageStackAssociator(this));
const targetBindResult = targetApplication.bind(scope);
this.application = targetBindResult.application;
this.associateCrossAccountStacks = targetBindResult.associateCrossAccountStacks;
cdk.Aspects.of(scope).add(new CheckedStageStackAssociator(this, {
associateCrossAccountStacks: this.associateCrossAccountStacks,
}));
}

/**
Expand All @@ -52,7 +59,9 @@ export class ApplicationAssociator extends Construct {
*/
public associateStage(stage: cdk.Stage): cdk.Stage {
this.associatedStages.add(stage);
cdk.Aspects.of(stage).add(new CheckedStageStackAssociator(this));
cdk.Aspects.of(stage).add(new CheckedStageStackAssociator(this, {
associateCrossAccountStacks: this.associateCrossAccountStacks,
}));
return stage;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,16 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
}

/**
* Associate all stacks present in construct's aspect with application.
* Associate all stacks present in construct's aspect with application, including cross-account stacks.
*
* NOTE: This method won't automatically register stacks under pipeline stages,
* and requires association of each pipeline stage by calling this method with stage Construct.
*
*/
public associateAllStacksInScope(scope: Construct): void {
cdk.Aspects.of(scope).add(new StageStackAssociator(this));
cdk.Aspects.of(scope).add(new StageStackAssociator(this, {
associateCrossAccountStacks: true,
}));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ import { ApplicationAssociator } from '../application-associator';
import { SharePermission } from '../common';
import { isRegionUnresolved, isAccountUnresolved } from '../private/utils';

export interface StackAssociatorBaseProps {
/**
* Indicates if the target Application should be shared with the cross-account stack owners and then
* associated with the cross-account stacks.
*
* @default - false
*/
readonly associateCrossAccountStacks?: boolean;
}

/**
* Aspect class, this will visit each node from the provided construct once.
*
Expand All @@ -14,9 +24,14 @@ import { isRegionUnresolved, isAccountUnresolved } from '../private/utils';
abstract class StackAssociatorBase implements IAspect {
protected abstract readonly application: IApplication;
protected abstract readonly applicationAssociator?: ApplicationAssociator;
protected readonly associateCrossAccountStacks?: boolean;

protected readonly sharedAccounts: Set<string> = new Set();

constructor(props?: StackAssociatorBaseProps) {
this.associateCrossAccountStacks = props?.associateCrossAccountStacks ?? false;
}

public visit(node: IConstruct): void {
// verify if a stage in a particular stack is associated to Application.
node.node.children.forEach((childNode) => {
Expand All @@ -42,6 +57,13 @@ abstract class StackAssociatorBase implements IAspect {
* @param node A Stage stack.
*/
private associate(node: Stack): void {
if (!isRegionUnresolved(this.application.env.region, node.region)
&& node.account != this.application.env.account
&& !this.associateCrossAccountStacks) {
// Skip association when cross-account sharing/association is not enabled.
// A warning will have been displayed as part of `handleCrossAccountStack()`.
return;
}
this.application.associateApplicationWithStack(node);
}

Expand Down Expand Up @@ -77,7 +99,7 @@ abstract class StackAssociatorBase implements IAspect {

/**
* Handle cross-account association.
* If any stack is evaluated as cross-account than that of application,
* If any stack is evaluated as cross-account than that of application, and cross-account option is enabled,
* then we will share the application to the stack owning account.
*
* @param node Cfn stack.
Expand All @@ -89,12 +111,17 @@ abstract class StackAssociatorBase implements IAspect {
}

if (node.account != this.application.env.account && !this.sharedAccounts.has(node.account)) {
this.application.shareApplication({
accounts: [node.account],
sharePermission: SharePermission.ALLOW_ACCESS,
});
if (this.associateCrossAccountStacks) {
this.application.shareApplication({
accounts: [node.account],
sharePermission: SharePermission.ALLOW_ACCESS,
});

this.sharedAccounts.add(node.account);
this.sharedAccounts.add(node.account);
} else {
this.warning(node, 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled.');
return;
}
}
}
}
Expand All @@ -103,8 +130,8 @@ export class CheckedStageStackAssociator extends StackAssociatorBase {
protected readonly application: IApplication;
protected readonly applicationAssociator?: ApplicationAssociator;

constructor(app: ApplicationAssociator) {
super();
constructor(app: ApplicationAssociator, props?: StackAssociatorBaseProps) {
super(props);
this.application = app.appRegistryApplication();
this.applicationAssociator = app;
}
Expand All @@ -114,8 +141,8 @@ export class StageStackAssociator extends StackAssociatorBase {
protected readonly application: IApplication;
protected readonly applicationAssociator?: ApplicationAssociator;

constructor(app: IApplication) {
super();
constructor(app: IApplication, props?: StackAssociatorBaseProps) {
super(props);
this.application = app;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ export interface TargetApplicationCommonOptions extends cdk.StackProps {
* @deprecated - Use `stackName` instead to control the name and id of the stack
*/
readonly stackId?: string;

/**
* Determines whether any cross-account stacks defined in the CDK app definition should be associated with the
* target application. If set to `true`, the application will first be shared with the accounts that own the stacks.
*
* @default - false
*/
readonly associateCrossAccountStacks?: boolean;
}


Expand Down Expand Up @@ -87,6 +95,10 @@ export interface BindTargetApplicationResult {
* Created or imported application.
*/
readonly application: IApplication;
/**
* Enables cross-account associations with the target application.
*/
readonly associateCrossAccountStacks: boolean;
}

/**
Expand Down Expand Up @@ -124,6 +136,7 @@ class CreateTargetApplication extends TargetApplication {

return {
application: appRegApplication,
associateCrossAccountStacks: this.applicationOptions.associateCrossAccountStacks ?? false,
};
}
}
Expand All @@ -144,6 +157,7 @@ class ExistingTargetApplication extends TargetApplication {
const appRegApplication = Application.fromApplicationArn(applicationStack, 'ExistingApplication', this.applicationOptions.applicationArnValue);
return {
application: appRegApplication,
associateCrossAccountStacks: this.applicationOptions.associateCrossAccountStacks ?? false,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,38 @@ describe('Scope based Associations with Application with Cross Region/Account',
});
}),

test('ApplicationAssociator with cross region stacks inside cdkApp throws error', () => {
test('ApplicationAssociator with cross account stacks inside cdkApp gives warning if associateCrossAccountStacks is not provided', () => {
new appreg.ApplicationAssociator(app, 'MyApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
applicationName: 'MyAssociatedApplication',
stackName: 'MyAssociatedApplicationStack',
env: { account: 'account2', region: 'region' },
})],
});

const crossAccountStack = new cdk.Stack(app, 'crossRegionStack', {
env: { account: 'account', region: 'region' },
});
Annotations.fromStack(crossAccountStack).hasWarning('*', 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled.');
});

test('ApplicationAssociator with cross account stacks inside cdkApp does not give warning if associateCrossAccountStacks is set to true', () => {
new appreg.ApplicationAssociator(app, 'MyApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
applicationName: 'MyAssociatedApplication',
stackName: 'MyAssociatedApplicationStack',
associateCrossAccountStacks: true,
env: { account: 'account', region: 'region' },
})],
});

const crossAccountStack = new cdk.Stack(app, 'crossRegionStack', {
env: { account: 'account2', region: 'region' },
});
Annotations.fromStack(crossAccountStack).hasNoWarning('*', 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled.');
});

test('ApplicationAssociator with cross region stacks inside cdkApp gives warning', () => {
new appreg.ApplicationAssociator(app, 'MyApplication', {
applications: [appreg.TargetApplication.createApplicationStack({
applicationName: 'MyAssociatedApplication',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "31.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,19 @@
{
"version": "31.0.0",
"files": {
"1db9045d198a45233cf79d4f87770e1d94d5efd69f70a11d40e3a6310ba3b26c": {
"source": {
"path": "TestAppRegistryApplicationStack.template.json",
"packaging": "file"
},
"destinations": {
"000000000000-current_region": {
"bucketName": "cdk-hnb659fds-assets-000000000000-${AWS::Region}",
"objectKey": "1db9045d198a45233cf79d4f87770e1d94d5efd69f70a11d40e3a6310ba3b26c.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::000000000000:role/cdk-hnb659fds-file-publishing-role-000000000000-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Loading

0 comments on commit 2167289

Please sign in to comment.