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.

Sign up for GitHub

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

Jump to bottom

feat(servicecatalogappregistry): add attribute groups to an application #24672

Merged
merged 9 commits into from
Mar 22, 2023
Merged

feat(servicecatalogappregistry): add attribute groups to an application #24672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
570c77e
Add associateNewAttributeGroup to create and associate the attribute …
liwewang-amazon Mar 16, 2023
ddd8fab
Support create & associate attribute group in `Application` construct
liwewang-amazon Mar 17, 2023
aa97fd7
Change addAG param name
liwewang-amazon Mar 17, 2023
540af2e
fix readme
liwewang-amazon Mar 17, 2023
80496d1
Updating samples with new input
liwewang-amazon Mar 20, 2023
1dafe29
Add id into `addAttributeGroup` input to provide to AttributeGroup co…
liwewang-amazon Mar 21, 2023
f21f11b
Change doc for addAttributeGroup method.
liwewang-amazon Mar 21, 2023
d4753c6
Make attribute group name as required.
liwewang-amazon Mar 21, 2023
07c0a1e
n to N
liwewang-amazon Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 5 additions & 17 deletions packages/@aws-cdk/aws-servicecatalogappregistry/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,22 +126,6 @@ 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',
Expand All @@ -154,7 +138,11 @@ const associatedApp = new appreg.ApplicationAssociator(app, 'AssociatedApplicati
});

// Associate application to the attribute group.
customAttributeGroup.attributeGroup.associateWith(associatedApp.appRegistryApplication());
associatedApp.appRegistryApplication.addAttributeGroup('MyAttributeGroup' , {
attributeGroupName: 'MyAttributeGroupName',
description: 'Test attribute group',
attributes: {},
});

```

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-servicecatalogappregistry/lib/application-associator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class ApplicationAssociator extends Construct {
* Get the AppRegistry application.
*
*/
public appRegistryApplication(): IApplication {
public get appRegistryApplication(): IApplication {
return this.application;
}
}
50 changes: 49 additions & 1 deletion packages/@aws-cdk/aws-servicecatalogappregistry/lib/application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as cdk from '@aws-cdk/core';
import { Names } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { StageStackAssociator } from './aspects/stack-associator';
import { IAttributeGroup } from './attribute-group';
import { AttributeGroup, IAttributeGroup } from './attribute-group';
import { getPrincipalsforSharing, hashValues, ShareOptions, SharePermission } from './common';
import { isAccountUnresolved } from './private/utils';
import { InputValidator } from './private/validation';
Expand All @@ -12,6 +12,29 @@ import { CfnApplication, CfnAttributeGroupAssociation, CfnResourceAssociation }
const APPLICATION_READ_ONLY_RAM_PERMISSION_ARN = 'arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly';
const APPLICATION_ALLOW_ACCESS_RAM_PERMISSION_ARN = 'arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationAllowAssociation';

/**
* Properties for a Service Catalog AppRegistry Attribute Group
*/
export interface AttributeGroupAssociationProps {
/**
* Name for attribute group.
*
*/
readonly attributeGroupName: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Attribute group name is the key to calculate attribute group construct id to make sure that multiple addAttributeGroup calls won't generate the same construct ID. Since name is unique, if addAttributeGroup is called twice with the same attributeGroupName, construct duplicate error will be thrown. Both Names.uniqueNodeId() and Names.uniqueResourceName() generate id from the path of the construct, i.e. path of the application. As a result, two addAttributeGroup calls with different names\attributes\descriptions generate two AttributeGroup constructs with the same construct ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be required even if we use customer provided construct id for attribute group.

Attribute group name is the key human readable identifier our service offers for customer to find the attribute group, we want to make it explicit, so that it will can make use of the name to look for the attribute group in other platforms like console or SDK client.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are aware that 99% of other constructs allow the user to not specify a name and have the system one generate for them?


/**
* Description for attribute group.
* @default - No description provided
*/
readonly description?: string;

/**
* A JSON of nested key-value pairs that represent the attributes in the group.
* Attributes maybe an empty JSON '{}', but must be explicitly stated.
liwewang-amazon marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly attributes: { [key: string]: any };
}

/**
* A Service Catalog AppRegistry Application.
*/
Expand Down Expand Up @@ -41,6 +64,14 @@ export interface IApplication extends cdk.IResource {
*/
associateAttributeGroup(attributeGroup: IAttributeGroup): void;

/**
* Create an attribute group and associate this application with the created attribute group.
*
* @param id name of the AttributeGroup construct to be created.
* @param attributeGroupProps AppRegistry attribute group props
*/
addAttributeGroup(id: string, attributeGroupProps: AttributeGroupAssociationProps): IAttributeGroup;

/**
* Associate this application with a CloudFormation stack.
*
Expand Down Expand Up @@ -114,6 +145,23 @@ abstract class ApplicationBase extends cdk.Resource implements IApplication {
}
}

/**
* Create an attribute group and associate this application with the created attribute group.
*/
public addAttributeGroup(id: string, props: AttributeGroupAssociationProps): IAttributeGroup {
const attributeGroup = new AttributeGroup(this, id, {
attributeGroupName: props.attributeGroupName,
attributes: props.attributes,
description: props.description,
});
new CfnAttributeGroupAssociation(this, `AttributeGroupAssociation${this.generateUniqueHash(attributeGroup.node.addr)}`, {
Copy link
Contributor

Choose a reason for hiding this comment

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

node.addr is already a hash :)

application: this.applicationId,
attributeGroup: attributeGroup.attributeGroupId,
});
this.associatedAttributeGroups.add(attributeGroup.node.addr);
return attributeGroup;
}

/**
* Associate a stack with the application
* If the resource is already associated, it will ignore duplicate request.
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-servicecatalogappregistry/lib/aspects/stack-associator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export class CheckedStageStackAssociator extends StackAssociatorBase {

constructor(app: ApplicationAssociator, props?: StackAssociatorBaseProps) {
super(props);
this.application = app.appRegistryApplication();
this.application = app.appRegistryApplication;
this.applicationAssociator = app;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-servicecatalogappregistry/lib/target-application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export interface CreateTargetApplicationOptions extends TargetApplicationCommonO
/**
* Whether create cloudFormation Output for application manager URL.
*
* @default - Application containing stacks deployed via CDK.
* @default - true
*/
readonly emitApplicationManagerUrlAsOutput?: boolean;
}
Expand Down
18 changes: 9 additions & 9 deletions packages/@aws-cdk/aws-servicecatalogappregistry/test/application-associator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ describe('Scope based Associations with Application within Same Account', () =>
});

const anotherStack = new AppRegistrySampleStack(app, 'SampleStack');
Template.fromStack(appAssociator.appRegistryApplication().stack).resourceCountIs('AWS::ServiceCatalogAppRegistry::Application', 1);
Template.fromStack(appAssociator.appRegistryApplication().stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::Application', {
Template.fromStack(appAssociator.appRegistryApplication.stack).resourceCountIs('AWS::ServiceCatalogAppRegistry::Application', 1);
Template.fromStack(appAssociator.appRegistryApplication.stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::Application', {
Name: 'MyAssociatedApplication',
Tags: { managedBy: 'CDK_Application_Associator' },
});
Template.fromStack(appAssociator.appRegistryApplication().stack).hasOutput('DefaultCdkApplicationApplicationManagerUrl27C138EF', {});
Template.fromStack(appAssociator.appRegistryApplication.stack).hasOutput('DefaultCdkApplicationApplicationManagerUrl27C138EF', {});
Template.fromStack(anotherStack).resourceCountIs('AWS::ServiceCatalogAppRegistry::ResourceAssociation', 1);
Template.fromStack(anotherStack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::ResourceAssociation', {
Application: 'MyAssociatedApplication',
Expand All @@ -46,14 +46,14 @@ describe('Scope based Associations with Application within Same Account', () =>
});

const anotherStack = new AppRegistrySampleStack(app, 'SampleStack');
Template.fromStack(appAssociator.appRegistryApplication().stack).resourceCountIs('AWS::ServiceCatalogAppRegistry::Application', 1);
Template.fromStack(appAssociator.appRegistryApplication().stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::Application', {
Template.fromStack(appAssociator.appRegistryApplication.stack).resourceCountIs('AWS::ServiceCatalogAppRegistry::Application', 1);
Template.fromStack(appAssociator.appRegistryApplication.stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::Application', {
Name: 'MyAssociatedApplication',
Tags: { managedBy: 'CDK_Application_Associator' },
});

expect(
Template.fromStack(appAssociator.appRegistryApplication().stack)
Template.fromStack(appAssociator.appRegistryApplication.stack)
.findOutputs('*', {}),
).toEqual({});
Template.fromStack(anotherStack).resourceCountIs('AWS::ServiceCatalogAppRegistry::ResourceAssociation', 1);
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('Associate attribute group with Application', () => {
})],
});

customAttributeGroup.attributeGroup.associateWith(appAssociator.appRegistryApplication());
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',
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
});

expect(
Template.fromStack(appAssociator.appRegistryApplication().stack).findOutputs('*', {}),
Template.fromStack(appAssociator.appRegistryApplication.stack).findOutputs('*', {}),
).toEqual({});
Template.fromStack(firstStack).resourceCountIs('AWS::ServiceCatalogAppRegistry::ResourceAssociation', 1);
Template.fromStack(nestedStack).resourceCountIs('AWS::ServiceCatalogAppRegistry::ResourceAssociation', 1);
Expand Down Expand Up @@ -268,7 +268,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
associateStage: true,
});
app.synth();
Template.fromStack(application.appRegistryApplication().stack).hasOutput('DefaultCdkApplicationApplicationManagerUrl27C138EF', {});
Template.fromStack(application.appRegistryApplication.stack).hasOutput('DefaultCdkApplicationApplicationManagerUrl27C138EF', {});
Template.fromStack(pipelineStack).resourceCountIs('AWS::ServiceCatalogAppRegistry::ResourceAssociation', 1);
});
});
Expand Down
25 changes: 25 additions & 0 deletions packages/@aws-cdk/aws-servicecatalogappregistry/test/application.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,31 @@ describe('Application', () => {
});
}),

test('associate new attribute group', () => {
application.addAttributeGroup('AttributeGroup', {
attributeGroupName: 'AttributeGroupName',
attributes: {},
description: 'Description for Attribute Group',
});

Template.fromStack(stack).hasResourceProperties('AWS::ServiceCatalogAppRegistry::AttributeGroupAssociation', {
Application: { 'Fn::GetAtt': ['MyApplication5C63EC1D', 'Id'] },
AttributeGroup: { 'Fn::GetAtt': ['MyApplicationAttributeGroup0BD166B6', 'Id'] },
});

Template.fromStack(stack).templateMatches({
Resources: {
MyApplicationAttributeGroup0BD166B6: {
Type: 'AWS::ServiceCatalogAppRegistry::AttributeGroup',
Properties: {
Name: 'AttributeGroupName',
Attributes: {},
},
},
},
});
}),

test('duplicate attribute group association are idempotent', () => {
const attributeGroup = new appreg.AttributeGroup(stack, 'AttributeGroup', {
attributeGroupName: 'attributeGroupName',
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.js.snapshot/cdk.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"30.1.0"}
{"version":"31.0.0"}
6 changes: 3 additions & 3 deletions ...est/integ.application.js.snapshot/integ-servicecatalogappregistry-application.assets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{
"version": "30.1.0",
"version": "31.0.0",
"files": {
"2332c6df6777cc571585060fa4888d6d3b9ef548aa00dcbfc53fbdde386d7591": {
"5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765": {
"source": {
"path": "integ-servicecatalogappregistry-application.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "2332c6df6777cc571585060fa4888d6d3b9ef548aa00dcbfc53fbdde386d7591.json",
"objectKey": "5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
44 changes: 42 additions & 2 deletions ...t/integ.application.js.snapshot/integ-servicecatalogappregistry-application.template.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,50 @@
}
}
},
"TestApplicationRAMSharead8ba81b8cdd40199FD1": {
"TestApplicationmyAnotherAttributeGroup375F79DB": {
"Type": "AWS::ServiceCatalogAppRegistry::AttributeGroup",
"Properties": {
"Attributes": {
"stage": "alpha",
"teamMembers": [
"markI",
"markII",
"markIII"
],
"public": false,
"publishYear": 2021,
"plannedRoadMap": {
"alpha": "some time",
"beta": "another time",
"gamma": "penultimate time",
"release": "go time"
}
},
"Name": "myAnotherAttributeGroup",
"Description": "my another attribute group description"
}
},
"TestApplicationAttributeGroupAssociationb6f47e836a8c4FCAC29E": {
"Type": "AWS::ServiceCatalogAppRegistry::AttributeGroupAssociation",
"Properties": {
"Application": {
"Fn::GetAtt": [
"TestApplication2FBC585F",
"Id"
]
},
"AttributeGroup": {
"Fn::GetAtt": [
"TestApplicationmyAnotherAttributeGroup375F79DB",
"Id"
]
}
}
},
"TestApplicationRAMShare004736f08f8e57044D5D": {
"Type": "AWS::RAM::ResourceShare",
"Properties": {
"Name": "RAMSharead8ba81b8cdd",
"Name": "RAMShare004736f08f8e",
"AllowExternalPrincipals": false,
"PermissionArns": [
"arn:aws:ram::aws:permission/AWSRAMPermissionServiceCatalogAppRegistryApplicationReadOnly"
Expand Down
2 changes: 1 addition & 1 deletion ...ages/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.js.snapshot/integ.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.1.0",
"version": "31.0.0",
"testCases": {
"integ.application": {
"stacks": [
Expand Down
29 changes: 25 additions & 4 deletions ...s/@aws-cdk/aws-servicecatalogappregistry/test/integ.application.js.snapshot/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "30.1.0",
"version": "31.0.0",
"artifacts": {
"integ-servicecatalogappregistry-application.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}/2332c6df6777cc571585060fa4888d6d3b9ef548aa00dcbfc53fbdde386d7591.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/5fbf2a286122f4bc412b1730f96351e289444b1122006f36e4ade8fae8442765.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -51,10 +51,22 @@
"data": "TestApplicationAttributeGroupAssociation4ba7f5842818B8EE1C6F"
}
],
"/integ-servicecatalogappregistry-application/TestApplication/RAMSharead8ba81b8cdd": [
"/integ-servicecatalogappregistry-application/TestApplication/myAnotherAttributeGroup/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "TestApplicationRAMSharead8ba81b8cdd40199FD1"
"data": "TestApplicationmyAnotherAttributeGroup375F79DB"
}
],
"/integ-servicecatalogappregistry-application/TestApplication/AttributeGroupAssociationb6f47e836a8c": [
{
"type": "aws:cdk:logicalId",
"data": "TestApplicationAttributeGroupAssociationb6f47e836a8c4FCAC29E"
}
],
"/integ-servicecatalogappregistry-application/TestApplication/RAMShare004736f08f8e": [
{
"type": "aws:cdk:logicalId",
"data": "TestApplicationRAMShare004736f08f8e57044D5D"
}
],
"/integ-servicecatalogappregistry-application/TestAttributeGroup/Resource": [
Expand All @@ -80,6 +92,15 @@
"type": "aws:cdk:logicalId",
"data": "CheckBootstrapVersion"
}
],
"TestApplicationRAMSharead8ba81b8cdd40199FD1": [
{
"type": "aws:cdk:logicalId",
"data": "TestApplicationRAMSharead8ba81b8cdd40199FD1",
"trace": [
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
]
}
]
},
"displayName": "integ-servicecatalogappregistry-application"
Expand Down
Loading