From b45ec858125ea35934bcdc06cda002ad36367486 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Sat, 2 May 2020 21:22:54 +0200 Subject: [PATCH 01/27] BREAKING CHANGE: requiredAttributes require an instance of IAttribute rather than taking boolean feat: allow mutable attributes for requiredAttributes --- .../@aws-cdk/aws-cognito/jest.config.gen.json | 1 + .../aws-cognito/lib/user-pool-attr.ts | 102 +++++++-------- .../@aws-cdk/aws-cognito/lib/user-pool.ts | 117 +++++++++++------- ...teg.user-pool-explicit-props.expected.json | 8 +- .../test/integ.user-pool-explicit-props.ts | 6 +- .../aws-cognito/test/user-pool-attr.test.ts | 16 +-- .../aws-cognito/test/user-pool.test.ts | 10 +- 7 files changed, 143 insertions(+), 117 deletions(-) create mode 100644 packages/@aws-cdk/aws-cognito/jest.config.gen.json diff --git a/packages/@aws-cdk/aws-cognito/jest.config.gen.json b/packages/@aws-cdk/aws-cognito/jest.config.gen.json new file mode 100644 index 0000000000000..cb6befc64d7e8 --- /dev/null +++ b/packages/@aws-cdk/aws-cognito/jest.config.gen.json @@ -0,0 +1 @@ +{"moduleFileExtensions":["js"],"testEnvironment":"node","coverageThreshold":{"global":{"branches":80,"statements":80}},"collectCoverage":true,"coverageReporters":["lcov","html","text-summary"],"coveragePathIgnorePatterns":["/lib/.*\\.generated.js"]} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index e2a76c64120ef..cc3971ec16cdf 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -6,121 +6,121 @@ export interface RequiredAttributes { /** * Whether the user's postal address is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly address?: boolean; + readonly address?: StringAttribute; /** * Whether the user's birthday, represented as an ISO 8601:2004 format, is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly birthdate?: boolean; + readonly birthdate?: StringAttribute; /** * Whether the user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly email?: boolean; + readonly email?: StringAttribute; /** * Whether the surname or last name of the user is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly familyName?: boolean; + readonly familyName?: StringAttribute; /** * Whether the user's gender is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly gender?: boolean; + readonly gender?: StringAttribute; /** * Whether the user's first name or give name is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly givenName?: boolean; + readonly givenName?: StringAttribute; /** * Whether the user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly locale?: boolean; + readonly locale?: StringAttribute; /** * Whether the user's middle name is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly middleName?: boolean; + readonly middleName?: StringAttribute; /** * Whether user's full name in displayable form, including all name parts, titles and suffixes, is a required attibute. - * @default false + * @default - Attribute is not required */ - readonly fullname?: boolean; + readonly fullname?: StringAttribute; /** * Whether the user's nickname or casual name is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly nickname?: boolean; + readonly nickname?: StringAttribute; /** * Whether the user's telephone number is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly phoneNumber?: boolean; + readonly phoneNumber?: StringAttribute; /** * Whether the URL to the user's profile picture is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly profilePicture?: boolean; + readonly profilePicture?: StringAttribute; /** * Whether the user's preffered username, different from the immutable user name, is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly preferredUsername?: boolean; + readonly preferredUsername?: StringAttribute; /** * Whether the URL to the user's profile page is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly profilePage?: boolean; + readonly profilePage?: StringAttribute; /** * Whether the user's time zone is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly timezone?: boolean; + readonly timezone?: StringAttribute; /** * Whether the time, the user's information was last updated, is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly lastUpdateTime?: boolean; + readonly lastUpdateTime?: NumberAttribute; /** * Whether the URL to the user's web page or blog is a required attribute. - * @default false + * @default - Attribute is not required */ - readonly website?: boolean; + readonly website?: StringAttribute; } /** * Represents a custom attribute type. */ -export interface ICustomAttribute { +export interface IAttribute { /** * Bind this custom attribute type to the values as expected by CloudFormation */ - bind(): CustomAttributeConfig; + bind(): AttributeConfig; } /** * Configuration that will be fed into CloudFormation for any custom attribute type. */ -export interface CustomAttributeConfig { +export interface AttributeConfig { // tslint:disable:max-line-length /** * The data type of the custom attribute. @@ -156,7 +156,7 @@ export interface CustomAttributeConfig { /** * Constraints that can be applied to a custom attribute of any type. */ -export interface CustomAttributeProps { +export interface AttributeProps { /** * Specifies whether the value of the attribute can be changed. * For any user pool attribute that's mapped to an identity provider attribute, you must set this parameter to true. @@ -188,13 +188,13 @@ export interface StringAttributeConstraints { /** * Props for constructing a StringAttr */ -export interface StringAttributeProps extends StringAttributeConstraints, CustomAttributeProps { +export interface StringAttributeProps extends StringAttributeConstraints, AttributeProps { } /** - * The String custom attribute type. + * The String attribute type. */ -export class StringAttribute implements ICustomAttribute { +export class StringAttribute implements IAttribute { private readonly minLen?: number; private readonly maxLen?: number; private readonly mutable?: boolean; @@ -211,7 +211,7 @@ export class StringAttribute implements ICustomAttribute { this.mutable = props?.mutable; } - public bind(): CustomAttributeConfig { + public bind(): AttributeConfig { let stringConstraints: StringAttributeConstraints | undefined; if (this.minLen || this.maxLen) { stringConstraints = { @@ -248,13 +248,13 @@ export interface NumberAttributeConstraints { /** * Props for NumberAttr */ -export interface NumberAttributeProps extends NumberAttributeConstraints, CustomAttributeProps { +export interface NumberAttributeProps extends NumberAttributeConstraints, AttributeProps { } /** * The Number custom attribute type. */ -export class NumberAttribute implements ICustomAttribute { +export class NumberAttribute implements IAttribute { private readonly min?: number; private readonly max?: number; private readonly mutable?: boolean; @@ -265,7 +265,7 @@ export class NumberAttribute implements ICustomAttribute { this.mutable = props?.mutable; } - public bind(): CustomAttributeConfig { + public bind(): AttributeConfig { let numberConstraints: NumberAttributeConstraints | undefined; if (this.min || this.max) { numberConstraints = { @@ -285,14 +285,14 @@ export class NumberAttribute implements ICustomAttribute { /** * The Boolean custom attribute type. */ -export class BooleanAttribute implements ICustomAttribute { +export class BooleanAttribute implements IAttribute { private readonly mutable?: boolean; - constructor(props: CustomAttributeProps = {}) { + constructor(props: AttributeProps = {}) { this.mutable = props?.mutable; } - public bind(): CustomAttributeConfig { + public bind(): AttributeConfig { return { dataType: 'Boolean', mutable: this.mutable, @@ -303,14 +303,14 @@ export class BooleanAttribute implements ICustomAttribute { /** * The DateTime custom attribute type. */ -export class DateTimeAttribute implements ICustomAttribute { +export class DateTimeAttribute implements IAttribute { private readonly mutable?: boolean; - constructor(props: CustomAttributeProps = {}) { + constructor(props: AttributeProps = {}) { this.mutable = props?.mutable; } - public bind(): CustomAttributeConfig { + public bind(): AttributeConfig { return { dataType: 'DateTime', mutable: this.mutable, diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts index a0bc9a32d2874..27fae96cf429d 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts @@ -2,8 +2,8 @@ import { IRole, PolicyDocument, PolicyStatement, Role, ServicePrincipal } from ' import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { CfnUserPool } from './cognito.generated'; -import { ICustomAttribute, RequiredAttributes } from './user-pool-attr'; -import { UserPoolClient, UserPoolClientOptions } from './user-pool-client'; +import { IAttribute, RequiredAttributes } from './user-pool-attr'; +import { IUserPoolClient, UserPoolClient, UserPoolClientOptions } from './user-pool-client'; import { UserPoolDomain, UserPoolDomainOptions } from './user-pool-domain'; /** @@ -465,7 +465,7 @@ export interface UserPoolProps { * * @default - No custom attributes. */ - readonly customAttributes?: { [key: string]: ICustomAttribute }; + readonly customAttributes?: { [key: string]: IAttribute }; /** * Configure whether users of this user pool can or are required use MFA to sign in. @@ -605,7 +605,7 @@ export class UserPool extends UserPoolBase { */ public readonly userPoolProviderUrl: string; - private triggers: CfnUserPool.LambdaConfigProperty = { }; + private triggers: CfnUserPool.LambdaConfigProperty = {}; constructor(scope: Construct, id: string, props: UserPoolProps = {}) { super(scope, id); @@ -848,51 +848,26 @@ export class UserPool extends UserPoolBase { const schema: CfnUserPool.SchemaAttributeProperty[] = []; if (props.requiredAttributes) { - const stdAttributes: StandardAttribute[] = []; - - if (props.requiredAttributes.address) { stdAttributes.push(StandardAttribute.ADDRESS); } - if (props.requiredAttributes.birthdate) { stdAttributes.push(StandardAttribute.BIRTHDATE); } - if (props.requiredAttributes.email) { stdAttributes.push(StandardAttribute.EMAIL); } - if (props.requiredAttributes.familyName) { stdAttributes.push(StandardAttribute.FAMILY_NAME); } - if (props.requiredAttributes.fullname) { stdAttributes.push(StandardAttribute.NAME); } - if (props.requiredAttributes.gender) { stdAttributes.push(StandardAttribute.GENDER); } - if (props.requiredAttributes.givenName) { stdAttributes.push(StandardAttribute.GIVEN_NAME); } - if (props.requiredAttributes.lastUpdateTime) { stdAttributes.push(StandardAttribute.LAST_UPDATE_TIME); } - if (props.requiredAttributes.locale) { stdAttributes.push(StandardAttribute.LOCALE); } - if (props.requiredAttributes.middleName) { stdAttributes.push(StandardAttribute.MIDDLE_NAME); } - if (props.requiredAttributes.nickname) { stdAttributes.push(StandardAttribute.NICKNAME); } - if (props.requiredAttributes.phoneNumber) { stdAttributes.push(StandardAttribute.PHONE_NUMBER); } - if (props.requiredAttributes.preferredUsername) { stdAttributes.push(StandardAttribute.PREFERRED_USERNAME); } - if (props.requiredAttributes.profilePage) { stdAttributes.push(StandardAttribute.PROFILE_URL); } - if (props.requiredAttributes.profilePicture) { stdAttributes.push(StandardAttribute.PICTURE_URL); } - if (props.requiredAttributes.timezone) { stdAttributes.push(StandardAttribute.TIMEZONE); } - if (props.requiredAttributes.website) { stdAttributes.push(StandardAttribute.WEBSITE); } - - schema.push(...stdAttributes.map((attr) => { - return { name: attr, required: true }; - })); + const stdAttributes = Object.keys(props.requiredAttributes) + .filter( + attrName => + !!props.requiredAttributes![attrName as keyof RequiredAttributes], + ) + .map(attrName => + this.renderAttribute( + StandardAttributeMap[attrName as keyof RequiredAttributes], + props.requiredAttributes![attrName as keyof RequiredAttributes]!, + ), + ) + .map(attrProp => ({ ...attrProp, required: true })); + + schema.push(...stdAttributes); } if (props.customAttributes) { - const customAttrs = Object.keys(props.customAttributes).map((attrName) => { - const attrConfig = props.customAttributes![attrName].bind(); - const numberConstraints: CfnUserPool.NumberAttributeConstraintsProperty = { - minValue: attrConfig.numberConstraints?.min?.toString(), - maxValue: attrConfig.numberConstraints?.max?.toString(), - }; - const stringConstraints: CfnUserPool.StringAttributeConstraintsProperty = { - minLength: attrConfig.stringConstraints?.minLen?.toString(), - maxLength: attrConfig.stringConstraints?.maxLen?.toString(), - }; - - return { - name: attrName, - attributeDataType: attrConfig.dataType, - numberAttributeConstraints: (attrConfig.numberConstraints) ? numberConstraints : undefined, - stringAttributeConstraints: (attrConfig.stringConstraints) ? stringConstraints : undefined, - mutable: attrConfig.mutable, - }; - }); + const customAttrs = Object.keys(props.customAttributes).map(attrName => + this.renderAttribute(attrName, props.customAttributes![attrName]), + ); schema.push(...customAttrs); } @@ -901,6 +876,33 @@ export class UserPool extends UserPoolBase { } return schema; } + + private renderAttribute( + name: string, + attribute: IAttribute, + ): CfnUserPool.SchemaAttributeProperty { + const attrConfig = attribute.bind(); + const numberConstraints: CfnUserPool.NumberAttributeConstraintsProperty = { + minValue: attrConfig.numberConstraints?.min?.toString(), + maxValue: attrConfig.numberConstraints?.max?.toString(), + }; + const stringConstraints: CfnUserPool.StringAttributeConstraintsProperty = { + minLength: attrConfig.stringConstraints?.minLen?.toString(), + maxLength: attrConfig.stringConstraints?.maxLen?.toString(), + }; + + return { + name, + attributeDataType: attrConfig.dataType, + numberAttributeConstraints: attrConfig.numberConstraints + ? numberConstraints + : undefined, + stringAttributeConstraints: attrConfig.stringConstraints + ? stringConstraints + : undefined, + mutable: attrConfig.mutable, + }; + } } const enum StandardAttribute { @@ -923,6 +925,29 @@ const enum StandardAttribute { WEBSITE = 'website', } +const StandardAttributeMap: Record< +keyof RequiredAttributes, +StandardAttribute +> = { + address: StandardAttribute.ADDRESS, + birthdate: StandardAttribute.BIRTHDATE, + email: StandardAttribute.EMAIL, + familyName: StandardAttribute.FAMILY_NAME, + gender: StandardAttribute.GENDER, + givenName: StandardAttribute.GIVEN_NAME, + locale: StandardAttribute.LOCALE, + middleName: StandardAttribute.MIDDLE_NAME, + fullname: StandardAttribute.NAME, + nickname: StandardAttribute.NICKNAME, + phoneNumber: StandardAttribute.PHONE_NUMBER, + profilePicture: StandardAttribute.PICTURE_URL, + preferredUsername: StandardAttribute.PREFERRED_USERNAME, + profilePage: StandardAttribute.PROFILE_URL, + timezone: StandardAttribute.TIMEZONE, + lastUpdateTime: StandardAttribute.LAST_UPDATE_TIME, + website: StandardAttribute.WEBSITE, +}; + function undefinedIfNoKeys(struct: object): object | undefined { const allUndefined = Object.values(struct).reduce((acc, v) => acc && (v === undefined), true); return allUndefined ? undefined : struct; diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json index 82f29c93ead24..c00eb84d963e2 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json @@ -780,11 +780,13 @@ }, "Schema": [ { - "Name": "email", + "AttributeDataType": "String", + "Name": "name", "Required": true }, { - "Name": "name", + "AttributeDataType": "String", + "Name": "email", "Required": true }, { @@ -873,4 +875,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts index 262fbb8670638..679872a99b38f 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts @@ -27,8 +27,8 @@ const userpool = new UserPool(stack, 'myuserpool', { phone: true, }, requiredAttributes: { - fullname: true, - email: true, + fullname: new StringAttribute(), + email: new StringAttribute(), }, customAttributes: { 'some-string-attr': new StringAttribute(), @@ -90,4 +90,4 @@ function dummyTrigger(name: string): IFunction { runtime: Runtime.NODEJS_12_X, code: Code.fromInline('foo'), }); -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts index f001712a802a7..7394c58f354c0 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts @@ -1,12 +1,12 @@ import '@aws-cdk/assert/jest'; -import { BooleanAttribute, CustomAttributeConfig, DateTimeAttribute, ICustomAttribute, NumberAttribute, StringAttribute } from '../lib'; +import { AttributeConfig, BooleanAttribute, DateTimeAttribute, IAttribute, NumberAttribute, StringAttribute } from '../lib'; describe('User Pool Attributes', () => { describe('mutable', () => { test('default', () => { // GIVEN - const allAttributes: ICustomAttribute[] = [ + const allAttributes: IAttribute[] = [ new StringAttribute(), new NumberAttribute(), new BooleanAttribute(), @@ -14,7 +14,7 @@ describe('User Pool Attributes', () => { ]; // WHEN - const bounds: CustomAttributeConfig[] = allAttributes.map((attr) => attr.bind() ); + const bounds: AttributeConfig[] = allAttributes.map((attr) => attr.bind() ); // THEN bounds.forEach((bound) => { @@ -27,7 +27,7 @@ describe('User Pool Attributes', () => { const allTrueProps = { mutable: true, }; - const allAttributeTypes: ICustomAttribute[] = [ + const allAttributeTypes: IAttribute[] = [ new StringAttribute(allTrueProps), new NumberAttribute(allTrueProps), new BooleanAttribute(allTrueProps), @@ -35,7 +35,7 @@ describe('User Pool Attributes', () => { ]; // WHEN - const bounds: CustomAttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); + const bounds: AttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); // THEN bounds.forEach((bound) => { @@ -50,7 +50,7 @@ describe('User Pool Attributes', () => { const allFalseProps = { mutable: false, }; - const allAttributeTypes: ICustomAttribute[] = [ + const allAttributeTypes: IAttribute[] = [ new StringAttribute(allFalseProps), new NumberAttribute(allFalseProps), new BooleanAttribute(allFalseProps), @@ -58,7 +58,7 @@ describe('User Pool Attributes', () => { ]; // WHEN - const bounds: CustomAttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); + const bounds: AttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); // THEN bounds.forEach((bound) => { @@ -165,4 +165,4 @@ describe('User Pool Attributes', () => { expect(bound.numberConstraints).toBeUndefined(); }); }); -}); \ No newline at end of file +}); diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts index 83d4863b751c3..504c7ada46fe4 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts @@ -491,8 +491,8 @@ describe('User Pool', () => { // WHEN new UserPool(stack, 'Pool', { requiredAttributes: { - fullname: true, - timezone: true, + fullname: new StringAttribute(), + timezone: new StringAttribute(), }, }); @@ -521,9 +521,7 @@ describe('User Pool', () => { }); new UserPool(stack, 'Pool2', { userPoolName: 'Pool2', - requiredAttributes: { - familyName: false, - }, + requiredAttributes: {}, }); // THEN @@ -854,4 +852,4 @@ function fooFunction(scope: Construct, name: string): lambda.IFunction { runtime: lambda.Runtime.NODEJS_12_X, handler: 'index.handler', }); -} \ No newline at end of file +} From b0cffae976634c062bc1c4c428a69bf2f42945bb Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Sat, 2 May 2020 21:57:48 +0200 Subject: [PATCH 02/27] Updated README --- packages/@aws-cdk/aws-cognito/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/README.md b/packages/@aws-cdk/aws-cognito/README.md index 229c6d1cbd00c..8204d6ac42c86 100644 --- a/packages/@aws-cdk/aws-cognito/README.md +++ b/packages/@aws-cdk/aws-cognito/README.md @@ -168,8 +168,8 @@ four optional attributes. new UserPool(this, 'myuserpool', { // ... requiredAttributes: { - fullname: true, - address: true, + fullname: new StringAttribute(), + address: new StringAttribute({ mutable: true }), }, customAttributes: { 'myappid': new StringAttribute({ minLen: 5, maxLen: 15, mutable: false }), From dfc0207947c0965d9d4e27e140c4fdefb64cdc46 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Sat, 2 May 2020 22:29:30 +0200 Subject: [PATCH 03/27] Introduced StandardAttribute --- packages/@aws-cdk/aws-cognito/README.md | 4 +- .../aws-cognito/lib/user-pool-attr.ts | 62 +++++++++++++------ ...teg.user-pool-explicit-props.expected.json | 2 - .../test/integ.user-pool-explicit-props.ts | 6 +- .../aws-cognito/test/user-pool-attr.test.ts | 35 ++++++++++- .../aws-cognito/test/user-pool.test.ts | 35 ++++++++++- 6 files changed, 114 insertions(+), 30 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/README.md b/packages/@aws-cdk/aws-cognito/README.md index 8204d6ac42c86..be012e5a63ea4 100644 --- a/packages/@aws-cdk/aws-cognito/README.md +++ b/packages/@aws-cdk/aws-cognito/README.md @@ -168,8 +168,8 @@ four optional attributes. new UserPool(this, 'myuserpool', { // ... requiredAttributes: { - fullname: new StringAttribute(), - address: new StringAttribute({ mutable: true }), + fullname: new StandardAttribute(), + address: StandardAttribute.asMutable(), }, customAttributes: { 'myappid': new StringAttribute({ minLen: 5, maxLen: 15, mutable: false }), diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index cc3971ec16cdf..440fd303223bb 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -8,103 +8,103 @@ export interface RequiredAttributes { * Whether the user's postal address is a required attribute. * @default - Attribute is not required */ - readonly address?: StringAttribute; + readonly address?: StandardAttribute; /** * Whether the user's birthday, represented as an ISO 8601:2004 format, is a required attribute. * @default - Attribute is not required */ - readonly birthdate?: StringAttribute; + readonly birthdate?: StandardAttribute; /** * Whether the user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. * @default - Attribute is not required */ - readonly email?: StringAttribute; + readonly email?: StandardAttribute; /** * Whether the surname or last name of the user is a required attribute. * @default - Attribute is not required */ - readonly familyName?: StringAttribute; + readonly familyName?: StandardAttribute; /** * Whether the user's gender is a required attribute. * @default - Attribute is not required */ - readonly gender?: StringAttribute; + readonly gender?: StandardAttribute; /** * Whether the user's first name or give name is a required attribute. * @default - Attribute is not required */ - readonly givenName?: StringAttribute; + readonly givenName?: StandardAttribute; /** * Whether the user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. * @default - Attribute is not required */ - readonly locale?: StringAttribute; + readonly locale?: StandardAttribute; /** * Whether the user's middle name is a required attribute. * @default - Attribute is not required */ - readonly middleName?: StringAttribute; + readonly middleName?: StandardAttribute; /** * Whether user's full name in displayable form, including all name parts, titles and suffixes, is a required attibute. * @default - Attribute is not required */ - readonly fullname?: StringAttribute; + readonly fullname?: StandardAttribute; /** * Whether the user's nickname or casual name is a required attribute. * @default - Attribute is not required */ - readonly nickname?: StringAttribute; + readonly nickname?: StandardAttribute; /** * Whether the user's telephone number is a required attribute. * @default - Attribute is not required */ - readonly phoneNumber?: StringAttribute; + readonly phoneNumber?: StandardAttribute; /** * Whether the URL to the user's profile picture is a required attribute. * @default - Attribute is not required */ - readonly profilePicture?: StringAttribute; + readonly profilePicture?: StandardAttribute; /** * Whether the user's preffered username, different from the immutable user name, is a required attribute. * @default - Attribute is not required */ - readonly preferredUsername?: StringAttribute; + readonly preferredUsername?: StandardAttribute; /** * Whether the URL to the user's profile page is a required attribute. * @default - Attribute is not required */ - readonly profilePage?: StringAttribute; + readonly profilePage?: StandardAttribute; /** * Whether the user's time zone is a required attribute. * @default - Attribute is not required */ - readonly timezone?: StringAttribute; + readonly timezone?: StandardAttribute; /** * Whether the time, the user's information was last updated, is a required attribute. * @default - Attribute is not required */ - readonly lastUpdateTime?: NumberAttribute; + readonly lastUpdateTime?: StandardAttribute; /** * Whether the URL to the user's web page or blog is a required attribute. * @default - Attribute is not required */ - readonly website?: StringAttribute; + readonly website?: StandardAttribute; } /** @@ -126,8 +126,9 @@ export interface AttributeConfig { * The data type of the custom attribute. * * @see https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_SchemaAttributeType.html#CognitoUserPools-Type-SchemaAttributeType-AttributeDataType + * @default - None. */ - readonly dataType: string; + readonly dataType?: string; // tslint:enable:max-line-length /** @@ -154,7 +155,7 @@ export interface AttributeConfig { } /** - * Constraints that can be applied to a custom attribute of any type. + * Constraints that can be applied to a attribute of any type. */ export interface AttributeProps { /** @@ -168,6 +169,29 @@ export interface AttributeProps { readonly mutable?: boolean } +/** + * A Standard attribute + * @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html#cognito-user-pools-standard-attributes + */ +export class StandardAttribute implements IAttribute { + /** + * Returns a mutable StandardAttribute. + */ + public static asMutable(): StandardAttribute { + return new StandardAttribute({ mutable: true }); + } + + private readonly mutable?: boolean; + + constructor(props: AttributeProps = {}) { + this.mutable = props.mutable; + } + + public bind(): AttributeConfig { + return { mutable: this.mutable }; + } +} + /** * Constraints that can be applied to a custom attribute of string type. */ diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json index c00eb84d963e2..bd91fc58f681a 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json @@ -780,12 +780,10 @@ }, "Schema": [ { - "AttributeDataType": "String", "Name": "name", "Required": true }, { - "AttributeDataType": "String", "Name": "email", "Required": true }, diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts index 679872a99b38f..e6c3a54b3d1cd 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts @@ -1,6 +1,6 @@ import { Code, Function, IFunction, Runtime } from '@aws-cdk/aws-lambda'; import { App, CfnOutput, Duration, Stack } from '@aws-cdk/core'; -import { BooleanAttribute, DateTimeAttribute, Mfa, NumberAttribute, StringAttribute, UserPool } from '../lib'; +import { BooleanAttribute, DateTimeAttribute, Mfa, NumberAttribute, StandardAttribute, StringAttribute, UserPool } from '../lib'; const app = new App(); const stack = new Stack(app, 'integ-user-pool'); @@ -27,8 +27,8 @@ const userpool = new UserPool(stack, 'myuserpool', { phone: true, }, requiredAttributes: { - fullname: new StringAttribute(), - email: new StringAttribute(), + fullname: new StandardAttribute(), + email: new StandardAttribute(), }, customAttributes: { 'some-string-attr': new StringAttribute(), diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts index 7394c58f354c0..63cd1bf81f1aa 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts @@ -1,5 +1,5 @@ import '@aws-cdk/assert/jest'; -import { AttributeConfig, BooleanAttribute, DateTimeAttribute, IAttribute, NumberAttribute, StringAttribute } from '../lib'; +import { AttributeConfig, BooleanAttribute, DateTimeAttribute, IAttribute, NumberAttribute, StandardAttribute, StringAttribute } from '../lib'; describe('User Pool Attributes', () => { @@ -7,6 +7,7 @@ describe('User Pool Attributes', () => { test('default', () => { // GIVEN const allAttributes: IAttribute[] = [ + new StandardAttribute(), new StringAttribute(), new NumberAttribute(), new BooleanAttribute(), @@ -28,6 +29,7 @@ describe('User Pool Attributes', () => { mutable: true, }; const allAttributeTypes: IAttribute[] = [ + new StandardAttribute(allTrueProps), new StringAttribute(allTrueProps), new NumberAttribute(allTrueProps), new BooleanAttribute(allTrueProps), @@ -51,6 +53,7 @@ describe('User Pool Attributes', () => { mutable: false, }; const allAttributeTypes: IAttribute[] = [ + new StandardAttribute(allFalseProps), new StringAttribute(allFalseProps), new NumberAttribute(allFalseProps), new BooleanAttribute(allFalseProps), @@ -136,6 +139,36 @@ describe('User Pool Attributes', () => { }); }); + describe('StandardAttribute', () => { + test('default', () => { + // GIVE + const attr = new StandardAttribute(); + + // WHEN + const bound = attr.bind(); + + // THEN + expect(bound.dataType).toBeUndefined(); + expect(bound.stringConstraints).toBeUndefined(); + expect(bound.numberConstraints).toBeUndefined(); + expect(bound.mutable).toBeUndefined(); + }); + + test('asMutable', () => { + // GIVE + const attr = StandardAttribute.asMutable(); + + // WHEN + const bound = attr.bind(); + + // THEN + expect(bound.dataType).toBeUndefined(); + expect(bound.stringConstraints).toBeUndefined(); + expect(bound.numberConstraints).toBeUndefined(); + expect(bound.mutable).toBeTruthy(); + }); + }); + describe('BooleanAttribute', () => { test('default', () => { // GIVEN diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts index 504c7ada46fe4..0ede5d2767cb8 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts @@ -3,7 +3,7 @@ import { ABSENT } from '@aws-cdk/assert/lib/assertions/have-resource'; import { Role } from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, Stack, Tag } from '@aws-cdk/core'; -import { Mfa, NumberAttribute, StringAttribute, UserPool, UserPoolOperation, VerificationEmailStyle } from '../lib'; +import { Mfa, NumberAttribute, StandardAttribute, StringAttribute, UserPool, UserPoolOperation, VerificationEmailStyle } from '../lib'; describe('User Pool', () => { test('default setup', () => { @@ -491,8 +491,8 @@ describe('User Pool', () => { // WHEN new UserPool(stack, 'Pool', { requiredAttributes: { - fullname: new StringAttribute(), - timezone: new StringAttribute(), + fullname: new StandardAttribute(), + timezone: new StandardAttribute(), }, }); @@ -511,6 +511,35 @@ describe('User Pool', () => { }); }); + test('mutable required attributes', () => { + // GIVEN + const stack = new Stack(); + + // WHEN + new UserPool(stack, 'Pool', { + requiredAttributes: { + fullname: new StandardAttribute({ mutable: true }), + timezone: new StandardAttribute({ mutable: true }), + }, + }); + + // THEN + expect(stack).toHaveResourceLike('AWS::Cognito::UserPool', { + Schema: [ + { + Mutable: true, + Name: 'name', + Required: true, + }, + { + Mutable: true, + Name: 'zoneinfo', + Required: true, + }, + ], + }); + }); + test('schema is absent when required attributes are specified but as false', () => { // GIVEN const stack = new Stack(); From e299ec7c8a433ad22913e0afc1fb8cd5eaa8d2a2 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Thu, 21 May 2020 22:25:42 +0200 Subject: [PATCH 04/27] simplified StandardAttribute as simple object --- packages/@aws-cdk/aws-cognito/README.md | 12 ++- .../aws-cognito/lib/user-pool-attr.ts | 91 ++++++++++--------- .../@aws-cdk/aws-cognito/lib/user-pool.ts | 60 ++++++++---- .../test/integ.user-pool-explicit-props.ts | 12 ++- .../aws-cognito/test/user-pool-attr.test.ts | 47 ++-------- .../aws-cognito/test/user-pool.test.ts | 35 +++++-- 6 files changed, 138 insertions(+), 119 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/README.md b/packages/@aws-cdk/aws-cognito/README.md index be012e5a63ea4..01c224fc7c8a3 100644 --- a/packages/@aws-cdk/aws-cognito/README.md +++ b/packages/@aws-cdk/aws-cognito/README.md @@ -167,9 +167,15 @@ four optional attributes. ```ts new UserPool(this, 'myuserpool', { // ... - requiredAttributes: { - fullname: new StandardAttribute(), - address: StandardAttribute.asMutable(), + standardAttributes: { + fullname: { + required: true, + mutable: false, + }, + address: { + required: false, + mutable: true, + }, }, customAttributes: { 'myappid': new StringAttribute({ minLen: 5, maxLen: 15, mutable: false }), diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index 440fd303223bb..1ed6045f05731 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -1,9 +1,9 @@ /** - * The set of standard attributes that can be marked as required. + * The set of standard attributes that can be marked as required or mutable. * * @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html#cognito-user-pools-standard-attributes */ -export interface RequiredAttributes { +export interface StandardAttributes { /** * Whether the user's postal address is a required attribute. * @default - Attribute is not required @@ -107,20 +107,44 @@ export interface RequiredAttributes { readonly website?: StandardAttribute; } +/** + * Standard attribute that can be marked as required or mutable. + * + * @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html#cognito-user-pools-standard-attributes + */ +export interface StandardAttribute { + /** + * Specifies whether the value of the attribute can be changed. + * For any user pool attribute that's mapped to an identity provider attribute, you must set this parameter to true. + * Amazon Cognito updates mapped attributes when users sign in to your application through an identity provider. + * If an attribute is immutable, Amazon Cognito throws an error when it attempts to update the attribute. + * + * @default false + */ + readonly mutable?: boolean; + /** + * Specifies whether the attribute is required upon user registration. + * If the attribute is required and the user does not provide a value, registration or sign-in will fail. + * + * @default false + */ + readonly required?: boolean; +} + /** * Represents a custom attribute type. */ -export interface IAttribute { +export interface ICustomAttribute { /** * Bind this custom attribute type to the values as expected by CloudFormation */ - bind(): AttributeConfig; + bind(): CustomAttributeConfig; } /** - * Configuration that will be fed into CloudFormation for any custom attribute type. + * Configuration that will be fed into CloudFormation for any attribute type. */ -export interface AttributeConfig { +export interface CustomAttributeConfig { // tslint:disable:max-line-length /** * The data type of the custom attribute. @@ -128,7 +152,7 @@ export interface AttributeConfig { * @see https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_SchemaAttributeType.html#CognitoUserPools-Type-SchemaAttributeType-AttributeDataType * @default - None. */ - readonly dataType?: string; + readonly dataType: string; // tslint:enable:max-line-length /** @@ -151,13 +175,13 @@ export interface AttributeConfig { * * @default false */ - readonly mutable?: boolean + readonly mutable?: boolean; } /** - * Constraints that can be applied to a attribute of any type. + * Constraints that can be applied to a custom attribute of any type. */ -export interface AttributeProps { +export interface CustomAttributeProps { /** * Specifies whether the value of the attribute can be changed. * For any user pool attribute that's mapped to an identity provider attribute, you must set this parameter to true. @@ -169,29 +193,6 @@ export interface AttributeProps { readonly mutable?: boolean } -/** - * A Standard attribute - * @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html#cognito-user-pools-standard-attributes - */ -export class StandardAttribute implements IAttribute { - /** - * Returns a mutable StandardAttribute. - */ - public static asMutable(): StandardAttribute { - return new StandardAttribute({ mutable: true }); - } - - private readonly mutable?: boolean; - - constructor(props: AttributeProps = {}) { - this.mutable = props.mutable; - } - - public bind(): AttributeConfig { - return { mutable: this.mutable }; - } -} - /** * Constraints that can be applied to a custom attribute of string type. */ @@ -212,13 +213,13 @@ export interface StringAttributeConstraints { /** * Props for constructing a StringAttr */ -export interface StringAttributeProps extends StringAttributeConstraints, AttributeProps { +export interface StringAttributeProps extends StringAttributeConstraints, CustomAttributeProps { } /** * The String attribute type. */ -export class StringAttribute implements IAttribute { +export class StringAttribute implements ICustomAttribute { private readonly minLen?: number; private readonly maxLen?: number; private readonly mutable?: boolean; @@ -235,7 +236,7 @@ export class StringAttribute implements IAttribute { this.mutable = props?.mutable; } - public bind(): AttributeConfig { + public bind(): CustomAttributeConfig { let stringConstraints: StringAttributeConstraints | undefined; if (this.minLen || this.maxLen) { stringConstraints = { @@ -272,13 +273,13 @@ export interface NumberAttributeConstraints { /** * Props for NumberAttr */ -export interface NumberAttributeProps extends NumberAttributeConstraints, AttributeProps { +export interface NumberAttributeProps extends NumberAttributeConstraints, CustomAttributeProps { } /** * The Number custom attribute type. */ -export class NumberAttribute implements IAttribute { +export class NumberAttribute implements ICustomAttribute { private readonly min?: number; private readonly max?: number; private readonly mutable?: boolean; @@ -289,7 +290,7 @@ export class NumberAttribute implements IAttribute { this.mutable = props?.mutable; } - public bind(): AttributeConfig { + public bind(): CustomAttributeConfig { let numberConstraints: NumberAttributeConstraints | undefined; if (this.min || this.max) { numberConstraints = { @@ -309,14 +310,14 @@ export class NumberAttribute implements IAttribute { /** * The Boolean custom attribute type. */ -export class BooleanAttribute implements IAttribute { +export class BooleanAttribute implements ICustomAttribute { private readonly mutable?: boolean; - constructor(props: AttributeProps = {}) { + constructor(props: CustomAttributeProps = {}) { this.mutable = props?.mutable; } - public bind(): AttributeConfig { + public bind(): CustomAttributeConfig { return { dataType: 'Boolean', mutable: this.mutable, @@ -327,14 +328,14 @@ export class BooleanAttribute implements IAttribute { /** * The DateTime custom attribute type. */ -export class DateTimeAttribute implements IAttribute { +export class DateTimeAttribute implements ICustomAttribute { private readonly mutable?: boolean; - constructor(props: AttributeProps = {}) { + constructor(props: CustomAttributeProps = {}) { this.mutable = props?.mutable; } - public bind(): AttributeConfig { + public bind(): CustomAttributeConfig { return { dataType: 'DateTime', mutable: this.mutable, diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts index 27fae96cf429d..b7bd5657ddb5b 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts @@ -2,7 +2,7 @@ import { IRole, PolicyDocument, PolicyStatement, Role, ServicePrincipal } from ' import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { CfnUserPool } from './cognito.generated'; -import { IAttribute, RequiredAttributes } from './user-pool-attr'; +import { ICustomAttribute, StandardAttributes } from './user-pool-attr'; import { IUserPoolClient, UserPoolClient, UserPoolClientOptions } from './user-pool-client'; import { UserPoolDomain, UserPoolDomainOptions } from './user-pool-domain'; @@ -456,16 +456,25 @@ export interface UserPoolProps { * The set of attributes that are required for every user in the user pool. * Read more on attributes here - https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html * - * @default - No attributes are required. + * @deprecated use {@link standardAttributes} + * @default - No required attributes. */ - readonly requiredAttributes?: RequiredAttributes; + readonly requiredAttributes?: StandardAttributes; + + /** + * The set of attributes that are required for every user in the user pool. + * Read more on attributes here - https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html + * + * @default - No standard attributes are required. + */ + readonly standardAttributes?: StandardAttributes; /** * Define a set of custom attributes that can be configured for each user in the user pool. * * @default - No custom attributes. */ - readonly customAttributes?: { [key: string]: IAttribute }; + readonly customAttributes?: { [key: string]: ICustomAttribute }; /** * Configure whether users of this user pool can or are required use MFA to sign in. @@ -605,7 +614,7 @@ export class UserPool extends UserPoolBase { */ public readonly userPoolProviderUrl: string; - private triggers: CfnUserPool.LambdaConfigProperty = {}; + private triggers: CfnUserPool.LambdaConfigProperty = { }; constructor(scope: Construct, id: string, props: UserPoolProps = {}) { super(scope, id); @@ -847,21 +856,38 @@ export class UserPool extends UserPoolBase { private schemaConfiguration(props: UserPoolProps): CfnUserPool.SchemaAttributeProperty[] | undefined { const schema: CfnUserPool.SchemaAttributeProperty[] = []; - if (props.requiredAttributes) { - const stdAttributes = Object.keys(props.requiredAttributes) + if (props.standardAttributes) { + const standardAttributes = props.standardAttributes; + const stdAttributes = Object.keys(standardAttributes) .filter( attrName => - !!props.requiredAttributes![attrName as keyof RequiredAttributes], - ) - .map(attrName => - this.renderAttribute( - StandardAttributeMap[attrName as keyof RequiredAttributes], - props.requiredAttributes![attrName as keyof RequiredAttributes]!, - ), + !!standardAttributes[attrName as keyof StandardAttributes], + ).filter( + attrName => + standardAttributes[attrName as keyof StandardAttributes]!.required || + standardAttributes[attrName as keyof StandardAttributes]!.mutable, ) - .map(attrProp => ({ ...attrProp, required: true })); + .map(attrName => ({ + name: StandardAttributeMap[attrName as keyof StandardAttributes], + ...(standardAttributes[attrName as keyof StandardAttributes] || {}), + })); schema.push(...stdAttributes); + } else if (props.requiredAttributes) { + const requiredAttributes = props.requiredAttributes; + const requiredAttrs = Object.keys(requiredAttributes) + .filter( + attrName => + !!requiredAttributes[attrName as keyof StandardAttributes], + ) + .map( + attrName => ({ + name: StandardAttributeMap[attrName as keyof StandardAttributes], + required: true, + }), + ); + + schema.push(...requiredAttrs); } if (props.customAttributes) { @@ -879,7 +905,7 @@ export class UserPool extends UserPoolBase { private renderAttribute( name: string, - attribute: IAttribute, + attribute: ICustomAttribute, ): CfnUserPool.SchemaAttributeProperty { const attrConfig = attribute.bind(); const numberConstraints: CfnUserPool.NumberAttributeConstraintsProperty = { @@ -926,7 +952,7 @@ const enum StandardAttribute { } const StandardAttributeMap: Record< -keyof RequiredAttributes, +keyof StandardAttributes, StandardAttribute > = { address: StandardAttribute.ADDRESS, diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts index e6c3a54b3d1cd..a55aa2697cf5f 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts @@ -1,6 +1,6 @@ import { Code, Function, IFunction, Runtime } from '@aws-cdk/aws-lambda'; import { App, CfnOutput, Duration, Stack } from '@aws-cdk/core'; -import { BooleanAttribute, DateTimeAttribute, Mfa, NumberAttribute, StandardAttribute, StringAttribute, UserPool } from '../lib'; +import { BooleanAttribute, DateTimeAttribute, Mfa, NumberAttribute, StringAttribute, UserPool } from '../lib'; const app = new App(); const stack = new Stack(app, 'integ-user-pool'); @@ -26,9 +26,13 @@ const userpool = new UserPool(stack, 'myuserpool', { email: true, phone: true, }, - requiredAttributes: { - fullname: new StandardAttribute(), - email: new StandardAttribute(), + standardAttributes: { + fullname: { + required: true, + }, + email: { + required: true, + }, }, customAttributes: { 'some-string-attr': new StringAttribute(), diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts index 63cd1bf81f1aa..dd6a7c3441c0f 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool-attr.test.ts @@ -1,13 +1,12 @@ import '@aws-cdk/assert/jest'; -import { AttributeConfig, BooleanAttribute, DateTimeAttribute, IAttribute, NumberAttribute, StandardAttribute, StringAttribute } from '../lib'; +import { BooleanAttribute, CustomAttributeConfig, DateTimeAttribute, ICustomAttribute, NumberAttribute, StringAttribute } from '../lib'; describe('User Pool Attributes', () => { describe('mutable', () => { test('default', () => { // GIVEN - const allAttributes: IAttribute[] = [ - new StandardAttribute(), + const allAttributes: ICustomAttribute[] = [ new StringAttribute(), new NumberAttribute(), new BooleanAttribute(), @@ -15,7 +14,7 @@ describe('User Pool Attributes', () => { ]; // WHEN - const bounds: AttributeConfig[] = allAttributes.map((attr) => attr.bind() ); + const bounds: CustomAttributeConfig[] = allAttributes.map((attr) => attr.bind() ); // THEN bounds.forEach((bound) => { @@ -28,8 +27,7 @@ describe('User Pool Attributes', () => { const allTrueProps = { mutable: true, }; - const allAttributeTypes: IAttribute[] = [ - new StandardAttribute(allTrueProps), + const allAttributeTypes: ICustomAttribute[] = [ new StringAttribute(allTrueProps), new NumberAttribute(allTrueProps), new BooleanAttribute(allTrueProps), @@ -37,7 +35,7 @@ describe('User Pool Attributes', () => { ]; // WHEN - const bounds: AttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); + const bounds: CustomAttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); // THEN bounds.forEach((bound) => { @@ -52,8 +50,7 @@ describe('User Pool Attributes', () => { const allFalseProps = { mutable: false, }; - const allAttributeTypes: IAttribute[] = [ - new StandardAttribute(allFalseProps), + const allAttributeTypes: ICustomAttribute[] = [ new StringAttribute(allFalseProps), new NumberAttribute(allFalseProps), new BooleanAttribute(allFalseProps), @@ -61,7 +58,7 @@ describe('User Pool Attributes', () => { ]; // WHEN - const bounds: AttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); + const bounds: CustomAttributeConfig[] = allAttributeTypes.map((attr) => attr.bind() ); // THEN bounds.forEach((bound) => { @@ -139,36 +136,6 @@ describe('User Pool Attributes', () => { }); }); - describe('StandardAttribute', () => { - test('default', () => { - // GIVE - const attr = new StandardAttribute(); - - // WHEN - const bound = attr.bind(); - - // THEN - expect(bound.dataType).toBeUndefined(); - expect(bound.stringConstraints).toBeUndefined(); - expect(bound.numberConstraints).toBeUndefined(); - expect(bound.mutable).toBeUndefined(); - }); - - test('asMutable', () => { - // GIVE - const attr = StandardAttribute.asMutable(); - - // WHEN - const bound = attr.bind(); - - // THEN - expect(bound.dataType).toBeUndefined(); - expect(bound.stringConstraints).toBeUndefined(); - expect(bound.numberConstraints).toBeUndefined(); - expect(bound.mutable).toBeTruthy(); - }); - }); - describe('BooleanAttribute', () => { test('default', () => { // GIVEN diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts index 0ede5d2767cb8..5e71abc38d174 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts @@ -3,7 +3,7 @@ import { ABSENT } from '@aws-cdk/assert/lib/assertions/have-resource'; import { Role } from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, Stack, Tag } from '@aws-cdk/core'; -import { Mfa, NumberAttribute, StandardAttribute, StringAttribute, UserPool, UserPoolOperation, VerificationEmailStyle } from '../lib'; +import { Mfa, NumberAttribute, StringAttribute, UserPool, UserPoolOperation, VerificationEmailStyle } from '../lib'; describe('User Pool', () => { test('default setup', () => { @@ -490,9 +490,13 @@ describe('User Pool', () => { // WHEN new UserPool(stack, 'Pool', { - requiredAttributes: { - fullname: new StandardAttribute(), - timezone: new StandardAttribute(), + standardAttributes: { + fullname: { + required: true, + }, + timezone: { + required: true, + }, }, }); @@ -511,15 +515,21 @@ describe('User Pool', () => { }); }); - test('mutable required attributes', () => { + test('mutable standard attributes', () => { // GIVEN const stack = new Stack(); // WHEN new UserPool(stack, 'Pool', { - requiredAttributes: { - fullname: new StandardAttribute({ mutable: true }), - timezone: new StandardAttribute({ mutable: true }), + standardAttributes: { + fullname: { + required: true, + mutable: true, + }, + timezone: { + required: true, + mutable: true, + }, }, }); @@ -540,7 +550,7 @@ describe('User Pool', () => { }); }); - test('schema is absent when required attributes are specified but as false', () => { + test('schema is absent when standard attributes are specified but not required and not mutable', () => { // GIVEN const stack = new Stack(); @@ -550,7 +560,12 @@ describe('User Pool', () => { }); new UserPool(stack, 'Pool2', { userPoolName: 'Pool2', - requiredAttributes: {}, + standardAttributes: { + familyName: { + required: false, + mutable: false, + }, + }, }); // THEN From 1ced138fce3af93b6781672fbef67f019dc1d42b Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Thu, 21 May 2020 22:27:54 +0200 Subject: [PATCH 05/27] Retouches in comments --- packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index 1ed6045f05731..15e45fd3db846 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -142,7 +142,7 @@ export interface ICustomAttribute { } /** - * Configuration that will be fed into CloudFormation for any attribute type. + * Configuration that will be fed into CloudFormation for any custom attribute type. */ export interface CustomAttributeConfig { // tslint:disable:max-line-length @@ -150,7 +150,6 @@ export interface CustomAttributeConfig { * The data type of the custom attribute. * * @see https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_SchemaAttributeType.html#CognitoUserPools-Type-SchemaAttributeType-AttributeDataType - * @default - None. */ readonly dataType: string; // tslint:enable:max-line-length @@ -217,7 +216,7 @@ export interface StringAttributeProps extends StringAttributeConstraints, Custom } /** - * The String attribute type. + * The String custom attribute type. */ export class StringAttribute implements ICustomAttribute { private readonly minLen?: number; From 35e238f73201deeec77293a8697f312fda65e501 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Thu, 21 May 2020 22:43:02 +0200 Subject: [PATCH 06/27] Reinclude RequiredAttributes interface --- .../aws-cognito/lib/user-pool-attr.ts | 110 ++++++++++++++++++ .../@aws-cdk/aws-cognito/lib/user-pool.ts | 4 +- 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index 15e45fd3db846..6d2dd20fbf1e8 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -1,3 +1,113 @@ +/** + * The set of standard attributes that can be marked as required. + * + * @deprecated use {@link StandardAttributes} + * @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html#cognito-user-pools-standard-attributes + */ +export interface RequiredAttributes { + /** + * Whether the user's postal address is a required attribute. + * @default - Attribute is not required + */ + readonly address?: boolean; + + /** + * Whether the user's birthday, represented as an ISO 8601:2004 format, is a required attribute. + * @default - Attribute is not required + */ + readonly birthdate?: boolean; + + /** + * Whether theb user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. + * @default - Attribute is not required + */ + readonly email?: boolean; + + /** + * Whether the surname or last name of the user is a required attribute. + * @default - Attribute is not required + */ + readonly familyName?: boolean; + + /** + * Whether the user's gender is a required attribute. + * @default - Attribute is not required + */ + readonly gender?: boolean; + + /** + * Whether the user's first name or give name is a required attribute. + * @default - Attribute is not required + */ + readonly givenName?: boolean; + + /** + * Whether the user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. + * @default - Attribute is not required + */ + readonly locale?: boolean; + + /** + * Whether the user's middle name is a required attribute. + * @default - Attribute is not required + */ + readonly middleName?: boolean; + + /** + * Whether user's full name in displayable form, including all name parts, titles and suffixes, is a required attibute. + * @default - Attribute is not required + */ + readonly fullname?: boolean; + + /** + * Whether the user's nickname or casual name is a required attribute. + * @default - Attribute is not required + */ + readonly nickname?: boolean; + + /** + * Whether the user's telephone number is a required attribute. + * @default - Attribute is not required + */ + readonly phoneNumber?: boolean; + + /** + * Whether the URL to the user's profile picture is a required attribute. + * @default - Attribute is not required + */ + readonly profilePicture?: boolean; + + /** + * Whether the user's preffered username, different from the immutable user name, is a required attribute. + * @default - Attribute is not required + */ + readonly preferredUsername?: boolean; + + /** + * Whether the URL to the user's profile page is a required attribute. + * @default - Attribute is not required + */ + readonly profilePage?: boolean; + + /** + * Whether the user's time zone is a required attribute. + * @default - Attribute is not required + */ + readonly timezone?: boolean; + + /** + * Whether the time, the user's information was last updated, is a required attribute. + * @default - Attribute is not required + */ + readonly lastUpdateTime?: boolean; + + /** + * Whether the URL to the user's web page or blog is a required attribute. + * @default - Attribute is not required + */ + readonly website?: boolean; +} + /** * The set of standard attributes that can be marked as required or mutable. * diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts index b7bd5657ddb5b..952998f14f725 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts @@ -2,7 +2,7 @@ import { IRole, PolicyDocument, PolicyStatement, Role, ServicePrincipal } from ' import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { CfnUserPool } from './cognito.generated'; -import { ICustomAttribute, StandardAttributes } from './user-pool-attr'; +import { ICustomAttribute, RequiredAttributes, StandardAttributes } from './user-pool-attr'; import { IUserPoolClient, UserPoolClient, UserPoolClientOptions } from './user-pool-client'; import { UserPoolDomain, UserPoolDomainOptions } from './user-pool-domain'; @@ -459,7 +459,7 @@ export interface UserPoolProps { * @deprecated use {@link standardAttributes} * @default - No required attributes. */ - readonly requiredAttributes?: StandardAttributes; + readonly requiredAttributes?: RequiredAttributes; /** * The set of attributes that are required for every user in the user pool. From 93be392c0e20f601e60017b023e9f33abc9d3f37 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Mon, 1 Jun 2020 15:18:29 +0200 Subject: [PATCH 07/27] BREAKING CHANGE: Replaced UserPool property for requiredAttributes with standardAttributes --- .../aws-cognito/lib/user-pool-attr.ts | 178 ++++-------------- .../@aws-cdk/aws-cognito/lib/user-pool.ts | 104 ++++------ ...teg.user-pool-explicit-props.expected.json | 6 +- .../test/integ.user-pool-explicit-props.ts | 1 + .../aws-cognito/test/user-pool.test.ts | 55 +++--- 5 files changed, 103 insertions(+), 241 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index 6d2dd20fbf1e8..13a2db6a1d7f2 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -1,113 +1,3 @@ -/** - * The set of standard attributes that can be marked as required. - * - * @deprecated use {@link StandardAttributes} - * @see https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html#cognito-user-pools-standard-attributes - */ -export interface RequiredAttributes { - /** - * Whether the user's postal address is a required attribute. - * @default - Attribute is not required - */ - readonly address?: boolean; - - /** - * Whether the user's birthday, represented as an ISO 8601:2004 format, is a required attribute. - * @default - Attribute is not required - */ - readonly birthdate?: boolean; - - /** - * Whether theb user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. - * @default - Attribute is not required - */ - readonly email?: boolean; - - /** - * Whether the surname or last name of the user is a required attribute. - * @default - Attribute is not required - */ - readonly familyName?: boolean; - - /** - * Whether the user's gender is a required attribute. - * @default - Attribute is not required - */ - readonly gender?: boolean; - - /** - * Whether the user's first name or give name is a required attribute. - * @default - Attribute is not required - */ - readonly givenName?: boolean; - - /** - * Whether the user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. - * @default - Attribute is not required - */ - readonly locale?: boolean; - - /** - * Whether the user's middle name is a required attribute. - * @default - Attribute is not required - */ - readonly middleName?: boolean; - - /** - * Whether user's full name in displayable form, including all name parts, titles and suffixes, is a required attibute. - * @default - Attribute is not required - */ - readonly fullname?: boolean; - - /** - * Whether the user's nickname or casual name is a required attribute. - * @default - Attribute is not required - */ - readonly nickname?: boolean; - - /** - * Whether the user's telephone number is a required attribute. - * @default - Attribute is not required - */ - readonly phoneNumber?: boolean; - - /** - * Whether the URL to the user's profile picture is a required attribute. - * @default - Attribute is not required - */ - readonly profilePicture?: boolean; - - /** - * Whether the user's preffered username, different from the immutable user name, is a required attribute. - * @default - Attribute is not required - */ - readonly preferredUsername?: boolean; - - /** - * Whether the URL to the user's profile page is a required attribute. - * @default - Attribute is not required - */ - readonly profilePage?: boolean; - - /** - * Whether the user's time zone is a required attribute. - * @default - Attribute is not required - */ - readonly timezone?: boolean; - - /** - * Whether the time, the user's information was last updated, is a required attribute. - * @default - Attribute is not required - */ - readonly lastUpdateTime?: boolean; - - /** - * Whether the URL to the user's web page or blog is a required attribute. - * @default - Attribute is not required - */ - readonly website?: boolean; -} - /** * The set of standard attributes that can be marked as required or mutable. * @@ -115,104 +5,104 @@ export interface RequiredAttributes { */ export interface StandardAttributes { /** - * Whether the user's postal address is a required attribute. - * @default - Attribute is not required + * The user's postal address is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly address?: StandardAttribute; /** - * Whether the user's birthday, represented as an ISO 8601:2004 format, is a required attribute. - * @default - Attribute is not required + * The user's birthday, represented as an ISO 8601:2004 format, is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly birthdate?: StandardAttribute; /** - * Whether the user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. - * @default - Attribute is not required + * Theb user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly email?: StandardAttribute; /** - * Whether the surname or last name of the user is a required attribute. - * @default - Attribute is not required + * The surname or last name of the user is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly familyName?: StandardAttribute; /** - * Whether the user's gender is a required attribute. - * @default - Attribute is not required + * The user's gender is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly gender?: StandardAttribute; /** - * Whether the user's first name or give name is a required attribute. - * @default - Attribute is not required + * The user's first name or give name is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly givenName?: StandardAttribute; /** - * Whether the user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. - * @default - Attribute is not required + * The user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly locale?: StandardAttribute; /** - * Whether the user's middle name is a required attribute. - * @default - Attribute is not required + * The user's middle name is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly middleName?: StandardAttribute; /** * Whether user's full name in displayable form, including all name parts, titles and suffixes, is a required attibute. - * @default - Attribute is not required + * @default - see the defaults under `StandardAttribute` */ readonly fullname?: StandardAttribute; /** - * Whether the user's nickname or casual name is a required attribute. - * @default - Attribute is not required + * The user's nickname or casual name is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly nickname?: StandardAttribute; /** - * Whether the user's telephone number is a required attribute. - * @default - Attribute is not required + * The user's telephone number is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly phoneNumber?: StandardAttribute; /** - * Whether the URL to the user's profile picture is a required attribute. - * @default - Attribute is not required + * The URL to the user's profile picture is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly profilePicture?: StandardAttribute; /** - * Whether the user's preffered username, different from the immutable user name, is a required attribute. - * @default - Attribute is not required + * The user's preffered username, different from the immutable user name, is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly preferredUsername?: StandardAttribute; /** - * Whether the URL to the user's profile page is a required attribute. - * @default - Attribute is not required + * The URL to the user's profile page is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly profilePage?: StandardAttribute; /** - * Whether the user's time zone is a required attribute. - * @default - Attribute is not required + * The user's time zone is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly timezone?: StandardAttribute; /** - * Whether the time, the user's information was last updated, is a required attribute. - * @default - Attribute is not required + * The time, the user's information was last updated, is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly lastUpdateTime?: StandardAttribute; /** - * Whether the URL to the user's web page or blog is a required attribute. - * @default - Attribute is not required + * The URL to the user's web page or blog is a required attribute. + * @default - see the defaults under `StandardAttribute` */ readonly website?: StandardAttribute; } @@ -229,7 +119,7 @@ export interface StandardAttribute { * Amazon Cognito updates mapped attributes when users sign in to your application through an identity provider. * If an attribute is immutable, Amazon Cognito throws an error when it attempts to update the attribute. * - * @default false + * @default true */ readonly mutable?: boolean; /** diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts index 952998f14f725..43b4b9474a750 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts @@ -2,7 +2,7 @@ import { IRole, PolicyDocument, PolicyStatement, Role, ServicePrincipal } from ' import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { CfnUserPool } from './cognito.generated'; -import { ICustomAttribute, RequiredAttributes, StandardAttributes } from './user-pool-attr'; +import { ICustomAttribute, StandardAttribute, StandardAttributes } from './user-pool-attr'; import { IUserPoolClient, UserPoolClient, UserPoolClientOptions } from './user-pool-client'; import { UserPoolDomain, UserPoolDomainOptions } from './user-pool-domain'; @@ -452,15 +452,6 @@ export interface UserPoolProps { */ readonly autoVerify?: AutoVerifiedAttrs; - /** - * The set of attributes that are required for every user in the user pool. - * Read more on attributes here - https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html - * - * @deprecated use {@link standardAttributes} - * @default - No required attributes. - */ - readonly requiredAttributes?: RequiredAttributes; - /** * The set of attributes that are required for every user in the user pool. * Read more on attributes here - https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html @@ -755,24 +746,24 @@ export class UserPool extends UserPoolBase { if (signIn.username) { aliasAttrs = []; - if (signIn.email) { aliasAttrs.push(StandardAttribute.EMAIL); } - if (signIn.phone) { aliasAttrs.push(StandardAttribute.PHONE_NUMBER); } - if (signIn.preferredUsername) { aliasAttrs.push(StandardAttribute.PREFERRED_USERNAME); } + if (signIn.email) { aliasAttrs.push(StandardAttributeNames.EMAIL); } + if (signIn.phone) { aliasAttrs.push(StandardAttributeNames.PHONE_NUMBER); } + if (signIn.preferredUsername) { aliasAttrs.push(StandardAttributeNames.PREFERRED_USERNAME); } if (aliasAttrs.length === 0) { aliasAttrs = undefined; } } else { usernameAttrs = []; - if (signIn.email) { usernameAttrs.push(StandardAttribute.EMAIL); } - if (signIn.phone) { usernameAttrs.push(StandardAttribute.PHONE_NUMBER); } + if (signIn.email) { usernameAttrs.push(StandardAttributeNames.EMAIL); } + if (signIn.phone) { usernameAttrs.push(StandardAttributeNames.PHONE_NUMBER); } } if (props.autoVerify) { autoVerifyAttrs = []; - if (props.autoVerify.email) { autoVerifyAttrs.push(StandardAttribute.EMAIL); } - if (props.autoVerify.phone) { autoVerifyAttrs.push(StandardAttribute.PHONE_NUMBER); } + if (props.autoVerify.email) { autoVerifyAttrs.push(StandardAttributeNames.EMAIL); } + if (props.autoVerify.phone) { autoVerifyAttrs.push(StandardAttributeNames.PHONE_NUMBER); } } else if (signIn.email || signIn.phone) { autoVerifyAttrs = []; - if (signIn.email) { autoVerifyAttrs.push(StandardAttribute.EMAIL); } - if (signIn.phone) { autoVerifyAttrs.push(StandardAttribute.PHONE_NUMBER); } + if (signIn.email) { autoVerifyAttrs.push(StandardAttributeNames.EMAIL); } + if (signIn.phone) { autoVerifyAttrs.push(StandardAttributeNames.PHONE_NUMBER); } } return { usernameAttrs, aliasAttrs, autoVerifyAttrs }; @@ -857,37 +848,15 @@ export class UserPool extends UserPoolBase { const schema: CfnUserPool.SchemaAttributeProperty[] = []; if (props.standardAttributes) { - const standardAttributes = props.standardAttributes; - const stdAttributes = Object.keys(standardAttributes) - .filter( - attrName => - !!standardAttributes[attrName as keyof StandardAttributes], - ).filter( - attrName => - standardAttributes[attrName as keyof StandardAttributes]!.required || - standardAttributes[attrName as keyof StandardAttributes]!.mutable, - ) - .map(attrName => ({ - name: StandardAttributeMap[attrName as keyof StandardAttributes], - ...(standardAttributes[attrName as keyof StandardAttributes] || {}), + const stdAttributes = (Object.entries(props.standardAttributes) as Array<[keyof StandardAttributes, StandardAttribute]>) + .filter(([, attr]) => !!attr) + .map(([attrName, attr]) => ({ + name: StandardAttributeMap[attrName], + mutable: attr.mutable ?? true, + required: attr.required ?? false, })); schema.push(...stdAttributes); - } else if (props.requiredAttributes) { - const requiredAttributes = props.requiredAttributes; - const requiredAttrs = Object.keys(requiredAttributes) - .filter( - attrName => - !!requiredAttributes[attrName as keyof StandardAttributes], - ) - .map( - attrName => ({ - name: StandardAttributeMap[attrName as keyof StandardAttributes], - required: true, - }), - ); - - schema.push(...requiredAttrs); } if (props.customAttributes) { @@ -931,7 +900,7 @@ export class UserPool extends UserPoolBase { } } -const enum StandardAttribute { +const enum StandardAttributeNames { ADDRESS = 'address', BIRTHDATE = 'birthdate', EMAIL = 'email', @@ -951,27 +920,24 @@ const enum StandardAttribute { WEBSITE = 'website', } -const StandardAttributeMap: Record< -keyof StandardAttributes, -StandardAttribute -> = { - address: StandardAttribute.ADDRESS, - birthdate: StandardAttribute.BIRTHDATE, - email: StandardAttribute.EMAIL, - familyName: StandardAttribute.FAMILY_NAME, - gender: StandardAttribute.GENDER, - givenName: StandardAttribute.GIVEN_NAME, - locale: StandardAttribute.LOCALE, - middleName: StandardAttribute.MIDDLE_NAME, - fullname: StandardAttribute.NAME, - nickname: StandardAttribute.NICKNAME, - phoneNumber: StandardAttribute.PHONE_NUMBER, - profilePicture: StandardAttribute.PICTURE_URL, - preferredUsername: StandardAttribute.PREFERRED_USERNAME, - profilePage: StandardAttribute.PROFILE_URL, - timezone: StandardAttribute.TIMEZONE, - lastUpdateTime: StandardAttribute.LAST_UPDATE_TIME, - website: StandardAttribute.WEBSITE, +const StandardAttributeMap: Record = { + address: StandardAttributeNames.ADDRESS, + birthdate: StandardAttributeNames.BIRTHDATE, + email: StandardAttributeNames.EMAIL, + familyName: StandardAttributeNames.FAMILY_NAME, + gender: StandardAttributeNames.GENDER, + givenName: StandardAttributeNames.GIVEN_NAME, + locale: StandardAttributeNames.LOCALE, + middleName: StandardAttributeNames.MIDDLE_NAME, + fullname: StandardAttributeNames.NAME, + nickname: StandardAttributeNames.NICKNAME, + phoneNumber: StandardAttributeNames.PHONE_NUMBER, + profilePicture: StandardAttributeNames.PICTURE_URL, + preferredUsername: StandardAttributeNames.PREFERRED_USERNAME, + profilePage: StandardAttributeNames.PROFILE_URL, + timezone: StandardAttributeNames.TIMEZONE, + lastUpdateTime: StandardAttributeNames.LAST_UPDATE_TIME, + website: StandardAttributeNames.WEBSITE, }; function undefinedIfNoKeys(struct: object): object | undefined { diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json index bd91fc58f681a..7625b4a9a80d7 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.expected.json @@ -781,11 +781,13 @@ "Schema": [ { "Name": "name", - "Required": true + "Required": true, + "Mutable": true }, { "Name": "email", - "Required": true + "Required": true, + "Mutable": true }, { "AttributeDataType": "String", diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts index a55aa2697cf5f..1f4f7fe8193c5 100644 --- a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts @@ -29,6 +29,7 @@ const userpool = new UserPool(stack, 'myuserpool', { standardAttributes: { fullname: { required: true, + mutable: true, }, email: { required: true, diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts index 5e71abc38d174..004b56b0fabc0 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts @@ -484,7 +484,7 @@ describe('User Pool', () => { }); }); - test('required attributes', () => { + test('standard attributes default to mutable', () => { // GIVEN const stack = new Stack(); @@ -506,10 +506,12 @@ describe('User Pool', () => { { Name: 'name', Required: true, + Mutable: true, }, { Name: 'zoneinfo', Required: true, + Mutable: true, }, ], }); @@ -521,6 +523,7 @@ describe('User Pool', () => { // WHEN new UserPool(stack, 'Pool', { + userPoolName: 'Pool', standardAttributes: { fullname: { required: true, @@ -533,8 +536,21 @@ describe('User Pool', () => { }, }); + new UserPool(stack, 'Pool1', { + userPoolName: 'Pool1', + standardAttributes: { + fullname: { + mutable: false, + }, + timezone: { + mutable: false, + }, + }, + }); + // THEN expect(stack).toHaveResourceLike('AWS::Cognito::UserPool', { + UserPoolName: 'Pool', Schema: [ { Mutable: true, @@ -548,34 +564,21 @@ describe('User Pool', () => { }, ], }); - }); - - test('schema is absent when standard attributes are specified but not required and not mutable', () => { - // GIVEN - const stack = new Stack(); - - // WHEN - new UserPool(stack, 'Pool1', { - userPoolName: 'Pool1', - }); - new UserPool(stack, 'Pool2', { - userPoolName: 'Pool2', - standardAttributes: { - familyName: { - required: false, - mutable: false, - }, - }, - }); - // THEN expect(stack).toHaveResourceLike('AWS::Cognito::UserPool', { UserPoolName: 'Pool1', - Schema: ABSENT, - }); - expect(stack).toHaveResourceLike('AWS::Cognito::UserPool', { - UserPoolName: 'Pool2', - Schema: ABSENT, + Schema: [ + { + Name: 'name', + Required: false, + Mutable: false, + }, + { + Name: 'zoneinfo', + Required: false, + Mutable: false, + }, + ], }); }); From 14dc5e297766a98efa4a02059624383e61e6e7b9 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Mon, 1 Jun 2020 15:33:49 +0200 Subject: [PATCH 08/27] Fix small spelling --- packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index 13a2db6a1d7f2..e3f28b542b2f7 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -17,7 +17,7 @@ export interface StandardAttributes { readonly birthdate?: StandardAttribute; /** - * Theb user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. + * The user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. * @default - see the defaults under `StandardAttribute` */ readonly email?: StandardAttribute; From aa06632234c47e8015ac0885c766b4a6b18e4413 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Mon, 1 Jun 2020 15:35:29 +0200 Subject: [PATCH 09/27] Removed jest gen file --- packages/@aws-cdk/aws-cognito/jest.config.gen.json | 1 - 1 file changed, 1 deletion(-) delete mode 100644 packages/@aws-cdk/aws-cognito/jest.config.gen.json diff --git a/packages/@aws-cdk/aws-cognito/jest.config.gen.json b/packages/@aws-cdk/aws-cognito/jest.config.gen.json deleted file mode 100644 index cb6befc64d7e8..0000000000000 --- a/packages/@aws-cdk/aws-cognito/jest.config.gen.json +++ /dev/null @@ -1 +0,0 @@ -{"moduleFileExtensions":["js"],"testEnvironment":"node","coverageThreshold":{"global":{"branches":80,"statements":80}},"collectCoverage":true,"coverageReporters":["lcov","html","text-summary"],"coveragePathIgnorePatterns":["/lib/.*\\.generated.js"]} \ No newline at end of file From 175fce965bcdd2c13494453cceb24cad3a39bb7a Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Mon, 1 Jun 2020 15:39:07 +0200 Subject: [PATCH 10/27] Removed accidently added Interface --- packages/@aws-cdk/aws-cognito/lib/user-pool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts index 43b4b9474a750..febdef19b0adc 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts @@ -3,7 +3,7 @@ import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, Duration, IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { CfnUserPool } from './cognito.generated'; import { ICustomAttribute, StandardAttribute, StandardAttributes } from './user-pool-attr'; -import { IUserPoolClient, UserPoolClient, UserPoolClientOptions } from './user-pool-client'; +import { UserPoolClient, UserPoolClientOptions } from './user-pool-client'; import { UserPoolDomain, UserPoolDomainOptions } from './user-pool-domain'; /** From 79bdb655068791b3880762d62a3ac31323ed6453 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 4 Jun 2020 15:47:55 +0300 Subject: [PATCH 11/27] real-synth --- allowed-breaking-changes.txt | 8 + .../lib/artifact-schema.ts | 22 +- .../cloud-assembly-schema/lib/schema.ts | 5 + .../schema/cloud-assembly.schema.json | 21 ++ .../schema/cloud-assembly.version.json | 2 +- packages/@aws-cdk/core/README.md | 64 +++- packages/@aws-cdk/core/lib/app.ts | 46 +-- packages/@aws-cdk/core/lib/assembly.ts | 308 ++++++++++++++++++ .../@aws-cdk/core/lib/construct-compat.ts | 18 +- packages/@aws-cdk/core/lib/deps.ts | 16 + packages/@aws-cdk/core/lib/index.ts | 2 + .../@aws-cdk/core/lib/private/prepare-app.ts | 25 +- packages/@aws-cdk/core/lib/private/refs.ts | 6 +- packages/@aws-cdk/core/lib/stack.ts | 145 +++++++-- packages/@aws-cdk/core/lib/stage.ts | 77 +++++ packages/@aws-cdk/core/test/test.stack.ts | 6 +- packages/@aws-cdk/core/test/test.stage.ts | 219 +++++++++++++ .../cx-api/design/NESTED_ASSEMBLIES.md | 93 ++++++ packages/@aws-cdk/cx-api/jest.config.js | 10 +- .../asset-manifest-artifact.ts | 4 +- .../cloudformation-artifact.ts | 24 +- .../embedded-cloud-assembly-artifact.ts | 49 +++ .../{ => artifacts}/tree-cloud-artifact.ts | 4 +- .../@aws-cdk/cx-api/lib/cloud-artifact.ts | 11 +- .../@aws-cdk/cx-api/lib/cloud-assembly.ts | 54 ++- packages/@aws-cdk/cx-api/lib/index.ts | 7 +- .../test/cloud-assembly-builder.test.ts | 34 +- .../@aws-cdk/cx-api/test/placeholders.test.ts | 29 +- 28 files changed, 1171 insertions(+), 138 deletions(-) create mode 100644 packages/@aws-cdk/core/lib/assembly.ts create mode 100644 packages/@aws-cdk/core/lib/stage.ts create mode 100644 packages/@aws-cdk/core/test/test.stage.ts create mode 100644 packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md rename packages/@aws-cdk/cx-api/lib/{ => artifacts}/asset-manifest-artifact.ts (90%) rename packages/@aws-cdk/cx-api/lib/{ => artifacts}/cloudformation-artifact.ts (89%) create mode 100644 packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts rename packages/@aws-cdk/cx-api/lib/{ => artifacts}/tree-cloud-artifact.ts (83%) diff --git a/allowed-breaking-changes.txt b/allowed-breaking-changes.txt index 8b137891791fe..e174e6ace55d6 100644 --- a/allowed-breaking-changes.txt +++ b/allowed-breaking-changes.txt @@ -1 +1,9 @@ +# Actually adding any artifact type will break the load() type signature because I could have written +# const x: A | B = Manifest.load(); +# and that won't typecheck if Manifest.load() adds a union arm and now returns A | B | C. +change-return-type:@aws-cdk/cloud-assembly-schema.Manifest.load +removed:@aws-cdk/core.BootstraplessSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN +removed:@aws-cdk/core.DefaultStackSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN +removed:@aws-cdk/core.DefaultStackSynthesizerProps.assetPublishingExternalId +removed:@aws-cdk/core.DefaultStackSynthesizerProps.assetPublishingRoleArn diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts index 866a1a6553c38..01acbc37fb8d6 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts @@ -84,7 +84,27 @@ export interface TreeArtifactProperties { readonly file: string; } +/** + * Artifact properties for embedded Cloud Assemblies + */ +export interface EmbeddedCloudAssemblyProperties { + /** + * Relative path to the embedded Cloud Assembly + */ + readonly directoryName: string; + + /** + * Display name for the Cloud Assembly + * + * @default - The artifact ID + */ + readonly displayName?: string; +} + /** * Properties for manifest artifacts */ -export type ArtifactProperties = AwsCloudFormationStackProperties | AssetManifestProperties | TreeArtifactProperties; \ No newline at end of file +export type ArtifactProperties = AwsCloudFormationStackProperties +| AssetManifestProperties +| TreeArtifactProperties +| EmbeddedCloudAssemblyProperties; \ No newline at end of file diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts index 1c4efd0cded5d..2874ae948d6c3 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts @@ -25,6 +25,11 @@ export enum ArtifactType { * Manifest for all assets in the Cloud Assembly */ ASSET_MANIFEST = 'cdk:asset-manifest', + + /** + * Embedded Cloud Assembly + */ + EMBEDDED_CLOUD_ASSEMBLY = 'cdk:cloud-assembly', } /** diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json index 73319145f8196..5652709b8d5ab 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json @@ -72,6 +72,9 @@ }, { "$ref": "#/definitions/TreeArtifactProperties" + }, + { + "$ref": "#/definitions/EmbeddedCloudAssemblyProperties" } ] } @@ -85,6 +88,7 @@ "enum": [ "aws:cloudformation:stack", "cdk:asset-manifest", + "cdk:cloud-assembly", "cdk:tree", "none" ], @@ -331,6 +335,23 @@ "file" ] }, + "EmbeddedCloudAssemblyProperties": { + "description": "Artifact properties for embedded Cloud Assemblies", + "type": "object", + "properties": { + "directoryName": { + "description": "Relative of the embedded Cloud Assembly's directory", + "type": "string" + }, + "displayName": { + "description": "Display name for the Cloud Assembly (Default - The artifact ID)", + "type": "string" + } + }, + "required": [ + "directoryName" + ] + }, "MissingContext": { "description": "Represents a missing piece of context.", "type": "object", diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json index 276fff8f8ba1f..78d33700c0698 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json @@ -1 +1 @@ -{"version":"4.0.0"} +{"version":"5.0.0"} diff --git a/packages/@aws-cdk/core/README.md b/packages/@aws-cdk/core/README.md index 2790600cc3da3..65f9067ff32d4 100644 --- a/packages/@aws-cdk/core/README.md +++ b/packages/@aws-cdk/core/README.md @@ -17,6 +17,42 @@ Guide](https://docs.aws.amazon.com/cdk/latest/guide/home.html) for information of most of the capabilities of this library. The rest of this README will only cover topics not already covered in the Developer Guide. +## Stacks and Stages + +A `Stack` is the smallest physical unit of deployment, and maps directly onto +a CloudFormation Stack. You define a Stack by defining a subclass of `Stack` +-- let's call it `MyStack` -- and instantiating the constructs that make up +your application in `MyStack`'s constructor. You then instantiate this stack +one or more times to define different instances of your application. For example, +you can instantiate it once using few and cheap EC2 instances for testing, +and once again using more and bigger EC2 instances for production. + +When your application grows, you may decide that it makes more sense to split it +out across multiple `Stack` classes. This can happen for a number of reasons: + +- You could be starting to reach the maximum number of resources allowed in a single + stack (this is currently 200). +- You could decide you want to separate out stateful resources and stateless resources + into separate stacks, so that it becomes easy to tear down and recreate the stacks + that don't have stateful resources. +- There could be a single stack with resources (like a VPC) that are shared + between multiple instances of other stacks containing your applications. + +As soon as your conceptual application starts to encompass multiple stacks, +it is convenient to wrap them in another construct that represents your +logical application. You can then treat that new unit the same way you used +to be able to treat a single stack: by instantiating it multiple times +for different instances of your application. + +You can define a custom subclass of `Construct`, holding one or more +`Stack`s, to represent a single logical instance of your application. + +As a final note: `Stack`s are not a unit of reuse. They describe physical +deployment layouts, and as such are best left to application builders to +organize their deployments with. If you want to vend a reusable construct, +define it as a subclasses of `Construct`: the consumers of your construct +will decide where to place it in their own stacks. + ## Nested Stacks [Nested stacks](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-nested-stacks.html) are stacks created as part of other stacks. You create a nested stack within another stack by using the `NestedStack` construct. @@ -36,7 +72,7 @@ class MyNestedStack extends cfn.NestedStack { constructor(scope: Construct, id: string, props?: cfn.NestedStackProps) { super(scope, id, props); - new s3.Bucket(this, 'NestedBucket'); + new s3.Bucket(this, 'NestedBucket'); } } @@ -236,7 +272,7 @@ new CustomResource(this, 'MyMagicalResource', { Property2: 'bar' }, - // the ARN of the provider (SNS/Lambda) which handles + // the ARN of the provider (SNS/Lambda) which handles // CREATE, UPDATE or DELETE events for this resource type // see next section for details serviceToken: 'ARN' @@ -292,7 +328,7 @@ function getOrCreate(scope: Construct): sns.Topic { Every time a resource event occurs (CREATE/UPDATE/DELETE), an SNS notification is sent to the SNS topic. Users must process these notifications (e.g. through a fleet of worker hosts) and submit success/failure responses to the -CloudFormation service. +CloudFormation service. Set `serviceToken` to `topic.topicArn` in order to use this provider: @@ -311,7 +347,7 @@ new CustomResource(this, 'MyResource', { An AWS lambda function is called *directly* by CloudFormation for all resource events. The handler must take care of explicitly submitting a success/failure -response to the CloudFormation service and handle various error cases. +response to the CloudFormation service and handle various error cases. Set `serviceToken` to `lambda.functionArn` to use this provider: @@ -361,7 +397,7 @@ exports.handler = async function(event) { const id = event.PhysicalResourceId; // only for "Update" and "Delete" const props = event.ResourceProperties; const oldProps = event.OldResourceProperties; // only for "Update"s - + switch (event.RequestType) { case "Create": // ... @@ -371,7 +407,7 @@ exports.handler = async function(event) { // if an error is thrown, a FAILED response will be submitted to CFN throw new Error('Failed!'); - + case "Delete": // ... } @@ -403,10 +439,10 @@ Here is an complete example of a custom resource that summarizes two numbers: ```js exports.handler = async e => { - return { - Data: { + return { + Data: { Result: e.ResourceProperties.lhs + e.ResourceProperties.rhs - } + } }; }; ``` @@ -463,7 +499,7 @@ Handlers are implemented as AWS Lambda functions, which means that they can be implemented in any Lambda-supported runtime. Furthermore, this provider has an asynchronous mode, which means that users can provide an `isComplete` lambda function which is called periodically until the operation is complete. This -allows implementing providers that can take up to two hours to stabilize. +allows implementing providers that can take up to two hours to stabilize. Set `serviceToken` to `provider.serviceToken` to use this type of provider: @@ -487,7 +523,7 @@ See the [documentation](https://docs.aws.amazon.com/cdk/api/latest/docs/custom-r Every time a resource event occurs (CREATE/UPDATE/DELETE), an SNS notification is sent to the SNS topic. Users must process these notifications (e.g. through a fleet of worker hosts) and submit success/failure responses to the -CloudFormation service. +CloudFormation service. Set `serviceToken` to `topic.topicArn` in order to use this provider: @@ -506,7 +542,7 @@ new CustomResource(this, 'MyResource', { An AWS lambda function is called *directly* by CloudFormation for all resource events. The handler must take care of explicitly submitting a success/failure -response to the CloudFormation service and handle various error cases. +response to the CloudFormation service and handle various error cases. Set `serviceToken` to `lambda.functionArn` to use this provider: @@ -532,7 +568,7 @@ Handlers are implemented as AWS Lambda functions, which means that they can be implemented in any Lambda-supported runtime. Furthermore, this provider has an asynchronous mode, which means that users can provide an `isComplete` lambda function which is called periodically until the operation is complete. This -allows implementing providers that can take up to two hours to stabilize. +allows implementing providers that can take up to two hours to stabilize. Set `serviceToken` to `provider.serviceToken` to use this provider: @@ -827,7 +863,7 @@ to use intrinsic functions in keys. Since JSON map keys must be strings, it is impossible to use intrinsics in keys and `CfnJson` can help. The following example defines an IAM role which can only be assumed by -principals that are tagged with a specific tag. +principals that are tagged with a specific tag. ```ts const tagParam = new CfnParameter(this, 'TagName'); diff --git a/packages/@aws-cdk/core/lib/app.ts b/packages/@aws-cdk/core/lib/app.ts index 0cc7a1a6ed8d1..8da26cefd28b9 100644 --- a/packages/@aws-cdk/core/lib/app.ts +++ b/packages/@aws-cdk/core/lib/app.ts @@ -1,7 +1,5 @@ import * as cxapi from '@aws-cdk/cx-api'; -import { Construct, ConstructNode } from './construct-compat'; -import { prepareApp } from './private/prepare-app'; -import { collectRuntimeInformation } from './private/runtime-info'; +import { Assembly } from './assembly'; import { TreeMetadata } from './private/tree-metadata'; const APP_SYMBOL = Symbol.for('@aws-cdk/core.App'); @@ -76,8 +74,7 @@ export interface AppProps { * * @see https://docs.aws.amazon.com/cdk/latest/guide/apps.html */ -export class App extends Construct { - +export class App extends Assembly { /** * Checks if an object is an instance of the `App` class. * @returns `true` if `obj` is an `App`. @@ -87,16 +84,14 @@ export class App extends Construct { return APP_SYMBOL in obj; } - private _assembly?: cxapi.CloudAssembly; - private readonly runtimeInfo: boolean; - private readonly outdir?: string; - /** * Initializes a CDK application. * @param props initialization properties */ constructor(props: AppProps = {}) { - super(undefined as any, ''); + super(undefined as any, '', { + outdir: props.outdir, + }); Object.defineProperty(this, APP_SYMBOL, { value: true }); @@ -110,10 +105,6 @@ export class App extends Construct { this.node.setContext(cxapi.DISABLE_VERSION_REPORTING, true); } - // both are reverse logic - this.runtimeInfo = this.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? false : true; - this.outdir = props.outdir || process.env[cxapi.OUTDIR_ENV]; - const autoSynth = props.autoSynth !== undefined ? props.autoSynth : cxapi.OUTDIR_ENV in process.env; if (autoSynth) { // synth() guarantuees it will only execute once, so a default of 'true' @@ -126,33 +117,6 @@ export class App extends Construct { } } - /** - * Synthesizes a cloud assembly for this app. Emits it to the directory - * specified by `outdir`. - * - * @returns a `CloudAssembly` which can be used to inspect synthesized - * artifacts such as CloudFormation templates and assets. - */ - public synth(): cxapi.CloudAssembly { - // we already have a cloud assembly, no-op for you - if (this._assembly) { - return this._assembly; - } - - const assembly = ConstructNode.synth(this.node, { - outdir: this.outdir, - runtimeInfo: this.runtimeInfo ? collectRuntimeInformation() : undefined, - }); - - this._assembly = assembly; - return assembly; - } - - protected prepare() { - super.prepare(); - prepareApp(this); - } - private loadContext(defaults: { [key: string]: string } = { }) { // prime with defaults passed through constructor for (const [ k, v ] of Object.entries(defaults)) { diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts new file mode 100644 index 0000000000000..d78a355b27b94 --- /dev/null +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -0,0 +1,308 @@ +import * as cxschema from '@aws-cdk/cloud-assembly-schema'; +import * as cxapi from '@aws-cdk/cx-api'; +import * as constructs from 'constructs'; +import { Construct, ConstructOrder, IConstruct, SynthesisOptions, ValidationError } from './construct-compat'; +import { Environment } from './environment'; +import { prepareApp } from './private/prepare-app'; +import { collectRuntimeInformation } from './private/runtime-info'; + +/** + * Initialization props for an assembly. + */ +export interface AssemblyProps { + /** + * Assembly name + * + * Will be prepended to default stack names of stacks defined under this assembly. + * + * Stack names can be overridden at the Stack level. + * + * @default - Same as the construct id + */ + readonly assemblyName?: string; + + /** + * Default AWS environment (account/region) for `Stack`s in this `Stage`. + * + * Stacks defined inside this `Stage` with either `region` or `account` missing + * from its env will use the corresponding field given here. + * + * If either `region` or `account`is is not configured for `Stack` (either on + * the `Stack` itself or on the containing `Stage`), the Stack will be + * *environment-agnostic*. + * + * Environment-agnostic stacks can be deployed to any environment, may not be + * able to take advantage of all features of the CDK. For example, they will + * not be able to use environmental context lookups, will not automatically + * translate Service Principals to the right format based on the environment's + * AWS partition, and other such enhancements. + * + * @example + * + * // Use a concrete account and region to deploy this Stage to + * new MyStage(app, 'Stage1', { + * env: { account: '123456789012', region: 'us-east-1' }, + * }); + * + * // Use the CLI's current credentials to determine the target environment + * new MyStage(app, 'Stage2', { + * env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, + * }); + * + * @default - The environments should be configured on the `Stack`s. + */ + readonly env?: Environment; + + /** + * The output directory into which to emit synthesized artifacts. + * + * @default - If this value is _not_ set, considers the environment variable `CDK_OUTDIR`. + * If `CDK_OUTDIR` is not defined, uses a temp directory. + */ + readonly outdir?: string; +} + +/** + * An application modeling unit consisting of Stacks that should be deployed together + * + * Derive a subclass of this construct and use it to model a single + * instance of your application. + * + * You can then instantiate your subclass multiple times to model multiple + * copies of your application which should be be deployed to different + * environments. + */ +export class Assembly extends Construct { + /** + * Return the containing Stage object of a construct, if available + */ + public static of(construct: IConstruct): Assembly | undefined { + return construct.node.scopes.reverse().slice(1).find(Assembly.isAssembly); + } + + /** + * Test whether the given construct is a Assembly object + */ + public static isAssembly(x: any ): x is Assembly { + return x !== null && x instanceof Assembly; + } + + /** + * The default region for all resources defined within this assembly. + */ + public readonly region?: string; + + /** + * The default account for all resources defined within this assembly. + */ + public readonly account?: string; + + /** + * The Cloud Assembly builder that is being used for this App + * + * @experimental + */ + public readonly assemblyBuilder: cxapi.CloudAssemblyBuilder; + + /** + * The name of the assembly. + */ + public readonly assemblyName: string; + + /** + * The parent assembly or `undefined` if this is the app. + */ + public readonly parentAssembly?: Assembly; + + /** + * The cached assembly if it was already built + */ + private assembly?: cxapi.CloudAssembly; + + constructor(scope: Construct, id: string, props: AssemblyProps = {}) { + super(scope, id); + + this.parentAssembly = Assembly.of(this); + + this.region = props.env?.region; + this.account = props.env?.account; + + // Need to determine fixed output directory already, because we must know where + // to write sub-assemblies (which must happen before we actually get to this app's + // synthesize() phase). + this.assemblyBuilder = this.parentAssembly + ? this.parentAssembly.assemblyBuilder.openEmbeddedAssembly(this.assemblyArtifactId) + : new cxapi.CloudAssemblyBuilder(props.outdir ?? process.env[cxapi.OUTDIR_ENV]); // TODO: << should; come; from; app; + + this.assemblyName = props.assemblyName ?? (this.parentAssembly?.assemblyName ? `${this.parentAssembly.assemblyName}-${id}` : id); + } + + /** + * Artifact ID of embedded assembly + * + * Derived from the construct path. + */ + public get assemblyArtifactId() { + return `sub-${this.node.path.replace(/\//g, '-').replace(/^-+|-+$/g, '')}`; + } + + /** + * Synthesize this assembly. + * + * Once an assembly has been synthesized, it cannot be modified. Subsequent calls will return the same assembly. + */ + public synth(options: AssemblySynthesisOptions = { }): cxapi.CloudAssembly { + if (!this.assembly) { + this.assembly = realSynth(this, options); + } + + return this.assembly; + } +} + +/** + * Options for assemly synthesis. + */ +export interface AssemblySynthesisOptions { + /** + * Should we skip construct validation. + * @default - false + */ + readonly skipValidation?: boolean; +} + +/** + * Interface which provides access to special methods of Construct + * + * @experimental + */ +interface IProtectedConstructMethods extends IConstruct { + /** + * Method that gets called when a construct should synthesize itself to an assembly + */ + onSynthesize(session: constructs.ISynthesisSession): void; + + /** + * Method that gets called to validate a construct + */ + onValidate(): string[]; + + /** + * Method that gets called to prepare a construct + */ + onPrepare(): void; +} + +export function realSynth(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly { + const builder = Assembly.isAssembly(root) + ? root.assemblyBuilder + : new cxapi.CloudAssemblyBuilder(options.outdir); + + // okay, now we start by calling "synth" on all nested assemblies (which will take care of all their children) + for (const child of root.node.findAll(ConstructOrder.POSTORDER)) { + if (child === root) { continue; } + if (!Assembly.isAssembly(child)) { continue; } + if (Assembly.of(child) !== root) { continue; } + child.synth(options); + } + + const inAssembly = (c: IConstruct) => { + // if the root is not an assembly (i.e. it's a stack in unit tests), then consider + // everything to be in an assembly + if (!Assembly.isAssembly(root)) { + return true; + } + + // always include self + if (c === root) { + return true; + } + + // if the child is an assembly, omit it + if (Assembly.isAssembly(c)) { + return false; + } + + return Assembly.of(c) === root; + }; + + // find all child constructs within this assembly (sorted depth-first), and + // include direct nested assemblies, but do not include any constructs from + // nested assemblies as they will be covered by their assembly's synth() + // method. + const findChildren = () => root.node + .findAll(ConstructOrder.POSTORDER) + .filter(inAssembly); + + const children = findChildren(); + + for (const child of children) { + + // hackery to be able to access some private members with strong types (yack!) + const node = child.node._actualNode as unknown as Omit & { + invokedAspects: constructs.IAspect[]; + _aspects: constructs.IAspect[]; + }; + + for (const aspect of node._aspects) { + if (node.invokedAspects.includes(aspect)) { + continue; + } + + child.node.findAll(ConstructOrder.PREORDER) + .filter(c => inAssembly(c)) + .forEach(c => aspect.visit(c)); + + node.invokedAspects.push(aspect); + } + } + + // invoke "prepare" on all of the children in this assembly. this is mostly here for legacy purposes + // the framework itself does not use prepare anymore. + for (const child of findChildren()) { + (child as IProtectedConstructMethods).onPrepare(); + } + + // resolve references + prepareApp(root); + + // give all children an opportunity to validate now that we've finished prepare + if (!options.skipValidation) { + const errors = new Array(); + for (const source of children) { + const messages = (source as IProtectedConstructMethods).onValidate(); + for (const message of messages) { + errors.push({ message, source: source as Construct }); + } + } + + if (errors.length > 0) { + const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); + throw new Error(`Validation failed with the following errors:\n ${errorList}`); + } + } + + // next, we invoke "onSynthesize" on all of our children. this will allow + // stacks to add themselves to the synthesized cloud assembly. + for (const child of children) { + (child as IProtectedConstructMethods).onSynthesize({ + outdir: builder.outdir, + assembly: builder, + }); + } + + // Add this assembly to the parent assembly manifest (if we have one) + if (Assembly.isAssembly(root) && root.parentAssembly) { + root.parentAssembly.assemblyBuilder.addArtifact(root.assemblyArtifactId, { + type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + properties: { + directoryName: root.assemblyArtifactId, + displayName: root.node.path, + } as cxschema.EmbeddedCloudAssemblyProperties, + }); + + } + + const runtimeInfo = root.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); + return builder.buildAssembly({ runtimeInfo }); +} \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/construct-compat.ts b/packages/@aws-cdk/core/lib/construct-compat.ts index 341943a748bca..c4a33bc57208a 100644 --- a/packages/@aws-cdk/core/lib/construct-compat.ts +++ b/packages/@aws-cdk/core/lib/construct-compat.ts @@ -222,21 +222,13 @@ export class ConstructNode { /** * Synthesizes a CloudAssembly from a construct tree. - * @param root The root of the construct tree. + * @param node The root of the construct tree. * @param options Synthesis options. */ - public static synth(root: ConstructNode, options: SynthesisOptions = { }): cxapi.CloudAssembly { - const builder = new cxapi.CloudAssemblyBuilder(options.outdir); - - root._actualNode.synthesize({ - outdir: builder.outdir, - skipValidation: options.skipValidation, - sessionContext: { - assembly: builder, - }, - }); - - return builder.buildAssembly(options); + public static synth(node: ConstructNode, options: SynthesisOptions = { }): cxapi.CloudAssembly { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const a: typeof import('./assembly') = require('./assembly'); + return a.realSynth(node.root, options); } /** diff --git a/packages/@aws-cdk/core/lib/deps.ts b/packages/@aws-cdk/core/lib/deps.ts index b26fa28cb187b..d4ce735397811 100644 --- a/packages/@aws-cdk/core/lib/deps.ts +++ b/packages/@aws-cdk/core/lib/deps.ts @@ -1,3 +1,4 @@ +import { Assembly } from './assembly'; import { CfnResource } from './cfn-resource'; import { Stack } from './stack'; import { findLastCommonElement, pathToTopLevelStack as pathToRoot } from './util'; @@ -29,7 +30,13 @@ export function addDependency(source: T, target: T, reason?: } const sourceStack = Stack.of(source); + const sourceAssembly = Assembly.of(sourceStack); const targetStack = Stack.of(target); + const targetAssembly = Assembly.of(targetStack); + + if (sourceAssembly !== targetAssembly) { + throw new Error(`You cannot add a dependency from '${source.node.path}' (in ${describeAssembly(sourceAssembly)}) to '${target.node.path}' (in ${describeAssembly(targetAssembly)}): dependency cannot cross stage boundaries`); + } // find the deepest common stack between the two elements const sourcePath = pathToRoot(sourceStack); @@ -88,3 +95,12 @@ export function addDependency(source: T, target: T, reason?: return resourceInCommonStackFor(resourceStack); } } + +/** + * Return a string representation of the given assembler, for use in error messages + */ +function describeAssembly(assembly: Assembly | undefined): string { + if (!assembly) { return 'an unrooted construct tree'; } + if (!assembly.parentAssembly) { return 'the App'; } + return `Stage '${assembly.node.path}'`; +} diff --git a/packages/@aws-cdk/core/lib/index.ts b/packages/@aws-cdk/core/lib/index.ts index 4e55122d5616f..0dc5128b2c527 100644 --- a/packages/@aws-cdk/core/lib/index.ts +++ b/packages/@aws-cdk/core/lib/index.ts @@ -22,6 +22,8 @@ export * from './cfn-resource'; export * from './cfn-resource-policy'; export * from './cfn-rule'; export * from './stack'; +export * from './stage'; +export * from './assembly'; export * from './cfn-element'; export * from './cfn-dynamic-reference'; export * from './cfn-tag'; diff --git a/packages/@aws-cdk/core/lib/private/prepare-app.ts b/packages/@aws-cdk/core/lib/private/prepare-app.ts index 6912f29a9fce5..6459d8d2aca3c 100644 --- a/packages/@aws-cdk/core/lib/private/prepare-app.ts +++ b/packages/@aws-cdk/core/lib/private/prepare-app.ts @@ -1,6 +1,7 @@ import { ConstructOrder } from 'constructs'; +import { Assembly } from '../assembly'; import { CfnResource } from '../cfn-resource'; -import { Construct, IConstruct } from '../construct-compat'; +import { IConstruct } from '../construct-compat'; import { Stack } from '../stack'; import { resolveReferences } from './refs'; @@ -14,9 +15,9 @@ import { resolveReferences } from './refs'; * * @param root The root of the construct tree. */ -export function prepareApp(root: Construct) { - if (root.node.scope) { - throw new Error('prepareApp must be called on the root node'); +export function prepareApp(root: IConstruct) { + if (root.node.scope && !Assembly.isAssembly(root)) { + throw new Error('prepareApp can only be called on an Assembly or a root construct'); } // apply dependencies between resources in depending subtrees @@ -32,7 +33,7 @@ export function prepareApp(root: Construct) { } // depth-first (children first) queue of nested stacks. We will pop a stack - // from the head of this queue to prepare it's template asset. + // from the head of this queue to prepare its template asset. const queue = findAllNestedStacks(root); while (true) { @@ -59,13 +60,23 @@ function defineNestedStackAsset(nestedStack: Stack) { nested._prepareTemplateAsset(); } -function findAllNestedStacks(root: Construct) { +function findAllNestedStacks(root: IConstruct) { const result = new Array(); + const includeStack = (stack: IConstruct): stack is Stack => { + if (!Stack.isStack(stack)) { return false; } + if (!stack.nested) { return false; } + + // test: if we are not within an assembly, then include it. + if (!Assembly.of(stack)) { return true; } + + return Assembly.of(stack) === root; + }; + // create a list of all nested stacks in depth-first post order this means // that we first prepare the leaves and then work our way up. for (const stack of root.node.findAll(ConstructOrder.POSTORDER /* <== important */)) { - if (Stack.isStack(stack) && stack.nested) { + if (includeStack(stack)) { result.push(stack); } } diff --git a/packages/@aws-cdk/core/lib/private/refs.ts b/packages/@aws-cdk/core/lib/private/refs.ts index baa92ff8202e3..62a568f8cd736 100644 --- a/packages/@aws-cdk/core/lib/private/refs.ts +++ b/packages/@aws-cdk/core/lib/private/refs.ts @@ -4,7 +4,7 @@ import { CfnElement } from '../cfn-element'; import { CfnOutput } from '../cfn-output'; import { CfnParameter } from '../cfn-parameter'; -import { Construct } from '../construct-compat'; +import { Construct, IConstruct } from '../construct-compat'; import { Reference } from '../reference'; import { IResolvable } from '../resolvable'; import { Stack } from '../stack'; @@ -18,7 +18,7 @@ import { makeUniqueId } from './uniqueid'; * This is called from the App level to resolve all references defined. Each * reference is resolved based on it's consumption context. */ -export function resolveReferences(scope: Construct): void { +export function resolveReferences(scope: IConstruct): void { const edges = findAllReferences(scope); for (const { source, value } of edges) { @@ -105,7 +105,7 @@ function resolveValue(consumer: Stack, reference: CfnReference): IResolvable { /** * Finds all the CloudFormation references in a construct tree. */ -function findAllReferences(root: Construct) { +function findAllReferences(root: IConstruct) { const result = new Array<{ source: CfnElement, value: CfnReference }>(); for (const consumer of root.node.findAll()) { diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 72980dfbfbfe2..4377bfc71842d 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -27,8 +27,40 @@ export interface StackProps { /** * The AWS environment (account/region) where this stack will be deployed. * - * @default - The `default-account` and `default-region` context parameters will be - * used. If they are undefined, it will not be possible to deploy the stack. + * Set the `region`/`account` fields of `env` to either a concrete value to + * select the indicated environment (recommended for production stacks), or + * to the values of environment variables + * `CDK_DEFAULT_REGION`/`CDK_DEFAULT_ACCOUNT` to let the target environment + * depend on the AWS credentials/configuration that the CDK CLI is executed + * under (recommended for development stacks). + * + * If the `Stack` is instantiated inside a `Stage` construct, any undefined + * `region`/`account` fields from `env` will default to the same field on + * the encompassing `Stage`, if configured there. + * + * If either of `region`/`account` is not set nor inherited from `Stage`, + * the Stack will be *environment-agnostic*. + * + * Environment-agnostic stacks can be deployed to any environment, may not be + * able to take advantage of all features of the CDK. For example, they will + * not be able to use environmental context lookups, will not automatically + * translate Service Principals to the right format based on the environment's + * AWS partition, and other such enhancements. + * + * @example + * + * // Use a concrete account and region to deploy this stack to + * new MyStack(app, 'Stack1', { + * env: { account: '123456789012', region: 'us-east-1' }, + * }); + * + * // Use the CLI's current credentials to determine the target environment + * new MyStack(app, 'Stack2', { + * env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, + * }); + * + * @default - The environment of the containing `Stage` if available, otherwise create + * the stack will be environment-agnostic. */ readonly env?: Environment; @@ -265,7 +297,7 @@ export class Stack extends Construct implements ITaggable { this.templateOptions.description = props.description; } - this._stackName = props.stackName !== undefined ? props.stackName : this.generateUniqueId(); + this._stackName = props.stackName !== undefined ? props.stackName : this.generateStackName(); this.tags = new TagManager(TagType.KEY_VALUE, 'aws:cdk:stack', props.tags); if (!VALID_STACK_NAME_REGEX.test(this.stackName)) { @@ -277,8 +309,12 @@ export class Stack extends Construct implements ITaggable { // the same name. however, this behavior is breaking for 1.x so it's only // applied under a feature flag which is applied automatically for new // projects created using `cdk init`. - this.artifactId = this.node.tryGetContext(cxapi.ENABLE_STACK_NAME_DUPLICATES_CONTEXT) - ? this.generateUniqueId() + // + // Also use the new behavior if we are using the new CI/CD-ready synthesizer; that way + // people only have to flip one flag. + // tslint:disable-next-line: max-line-length + this.artifactId = this.node.tryGetContext(cxapi.ENABLE_STACK_NAME_DUPLICATES_CONTEXT) || this.node.tryGetContext(cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT) + ? this.generateStackArtifactId() : this.stackName; this.templateFile = `${this.artifactId}.template.json`; @@ -781,12 +817,15 @@ export class Stack extends Construct implements ITaggable { */ private parseEnvironment(env: Environment = {}) { // if an environment property is explicitly specified when the stack is - // created, it will be used. if not, use tokens for account and region but - // they do not need to be scoped, the only situation in which - // export/fn::importvalue would work if { Ref: "AWS::AccountId" } is the - // same for provider and consumer anyway. - const account = env.account || Aws.ACCOUNT_ID; - const region = env.region || Aws.REGION; + // created, it will be used. if not, use tokens for account and region. + // + // (They do not need to be anchored to any construct like resource attributes + // are, because we'll never Export/Fn::ImportValue them -- the only situation + // in which Export/Fn::ImportValue would work is if the value are the same + // between producer and consumer anyway, so we can just assume that they are). + const containingAssembly = Assembly.of(this); + const account = env.account ?? containingAssembly?.account ?? Aws.ACCOUNT_ID; + const region = env.region ?? containingAssembly?.region ?? Aws.REGION; // this is the "aws://" env specification that will be written to the cloud assembly // manifest. it will use "unknown-account" and "unknown-region" to indicate @@ -818,24 +857,54 @@ export class Stack extends Construct implements ITaggable { } /** - * Calculcate the stack name based on the construct path + * Calculate the stack name based on the construct path + * + * The stack name is the name under which we'll deploy the stack, + * and incorporates containing Stage names by default. + * + * Generally this looks a lot like how logical IDs are calculated. + * The stack name is calculated based on the construct root path, + * as follows: + * + * - Path is calculated with respect to containing App or Stage (if any) + * - If the path is one component long just use that component, otherwise + * combine them with a hash. + * + * Since the hash is quite ugly and we'd like to avoid it if possible -- but + * we can't anymore in the general case since it has been written into legacy + * stacks. The introduction of Stages makes it possible to make this nicer however. + * When a Stack is nested inside a Stage, we use the path components below the + * Stage, and prefix the path components of the Stage before it. + */ + private generateStackName() { + const assembly = Assembly.of(this); + const prefix = (assembly && assembly.assemblyName) ? `${assembly.assemblyName}-` : ''; + return `${prefix}${this.generateStackId(assembly)}`; + } + + /** + * The artifact ID for this stack + * + * Stack artifact ID is unique within the App's Cloud Assembly. */ - private generateUniqueId() { - // In tests, it's possible for this stack to be the root object, in which case - // we need to use it as part of the root path. - const rootPath = this.node.scope !== undefined ? this.node.scopes.slice(1) : [this]; + private generateStackArtifactId() { + return this.generateStackId(this.node.root); + } + + /** + * Generate an ID with respect to the given container construct. + */ + private generateStackId(container: IConstruct | undefined) { + const rootPath = rootPathTo(this, container); const ids = rootPath.map(c => c.node.id); - // Special case, if rootPath is length 1 then just use ID (backwards compatibility) - // otherwise use a unique stack name (including hash). This logic is already - // in makeUniqueId, *however* makeUniqueId will also strip dashes from the name, - // which *are* allowed and also used, so we short-circuit it. - if (ids.length === 1) { - // Could be empty in a unit test, so just pretend it's named "Stack" then - return ids[0] || 'Stack'; + // In unit tests our Stack (which is the only component) may not have an + // id, so in that case just pretend it's "Stack". + if (ids.length === 1 && !ids[0]) { + ids[0] = 'Stack'; } - return makeUniqueId(ids); + return makeStackName(ids); } } @@ -950,8 +1019,36 @@ function cfnElements(node: IConstruct, into: CfnElement[] = []): CfnElement[] { return into; } +/** + * Return the construct root path of the given construct relative to the given ancestor + * + * If no ancestor is given or the ancestor is not found, return the entire root path. + */ +export function rootPathTo(construct: IConstruct, ancestor?: IConstruct): IConstruct[] { + const scopes = construct.node.scopes; + for (let i = scopes.length - 2; i >= 0; i--) { + if (scopes[i] === ancestor) { + return scopes.slice(i + 1); + } + } + return scopes; +} + +/** + * makeUniqueId, specialized for Stack names + * + * Stack names may contain '-', so we allow that character if the stack name + * has only one component. Otherwise we fall back to the regular "makeUniqueId" + * behavior. + */ +export function makeStackName(components: string[]) { + if (components.length === 1) { return components[0]; } + return makeUniqueId(components); +} + // These imports have to be at the end to prevent circular imports import { Arn, ArnComponents } from './arn'; +import { Assembly } from './assembly'; import { CfnElement } from './cfn-element'; import { Fn } from './cfn-fn'; import { Aws, ScopedAws } from './cfn-pseudo'; diff --git a/packages/@aws-cdk/core/lib/stage.ts b/packages/@aws-cdk/core/lib/stage.ts new file mode 100644 index 0000000000000..822c4bc887c4d --- /dev/null +++ b/packages/@aws-cdk/core/lib/stage.ts @@ -0,0 +1,77 @@ +import { Assembly } from './assembly'; +import { Construct } from './construct-compat'; +import { Environment } from './environment'; + +/** + * Properties for a Stage + */ +export interface StageProps { + /** + * Default AWS environment (account/region) for `Stack`s in this `Stage`. + * + * Stacks defined inside this `Stage` with either `region` or `account` missing + * from its env will use the corresponding field given here. + * + * If either `region` or `account`is is not configured for `Stack` (either on + * the `Stack` itself or on the containing `Stage`), the Stack will be + * *environment-agnostic*. + * + * Environment-agnostic stacks can be deployed to any environment, may not be + * able to take advantage of all features of the CDK. For example, they will + * not be able to use environmental context lookups, will not automatically + * translate Service Principals to the right format based on the environment's + * AWS partition, and other such enhancements. + * + * @example + * + * // Use a concrete account and region to deploy this Stage to + * new MyStage(app, 'Stage1', { + * env: { account: '123456789012', region: 'us-east-1' }, + * }); + * + * // Use the CLI's current credentials to determine the target environment + * new MyStage(app, 'Stage2', { + * env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, + * }); + * + * @default - The environments should be configured on the `Stack`s. + */ + readonly env?: Environment; + + /** + * Stage name + * + * Will be prepended to default Stack names of stacks in this Stage. + * + * Stack names can be overridden at the Stack level. + * + * @default - Same as the construct id + */ + readonly stageName?: string; +} + +/** + * An application modeling unit consisting of Stacks that should be deployed together + * + * Derive a subclass of this construct and use it to model a single + * instance of your application. + * + * You can then instantiate your subclass multiple times to model multiple + * copies of your application which should be be deployed to different + * environments. + */ +export class Stage extends Assembly { + /** + * Stage name of this stage + */ + public readonly stageName: string; + + constructor(scope: Construct, id: string, props: StageProps = {}) { + super(scope, id, { + env: props.env, + assemblyName: props.stageName, + }); + + this.stageName = this.assemblyName; + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/core/test/test.stack.ts b/packages/@aws-cdk/core/test/test.stack.ts index 7f8e625ee4428..a2d00cf4efdd6 100644 --- a/packages/@aws-cdk/core/test/test.stack.ts +++ b/packages/@aws-cdk/core/test/test.stack.ts @@ -524,7 +524,7 @@ export = { new CfnParameter(stack1, 'SomeParameter', { type: 'String', default: account2 }); test.throws(() => { - ConstructNode.prepare(app.node); + app.synth(); // tslint:disable-next-line:max-line-length }, "'Stack2' depends on 'Stack1' (Stack2/SomeParameter -> Stack1.AWS::AccountId). Adding this dependency (Stack1/SomeParameter -> Stack2.AWS::AccountId) would create a cyclic reference."); @@ -541,7 +541,7 @@ export = { // WHEN new CfnParameter(stack2, 'SomeParameter', { type: 'String', default: account1 }); - ConstructNode.prepare(app.node); + app.synth(); // THEN test.deepEqual(stack2.dependencies.map(s => s.node.id), ['Stack1']); @@ -560,7 +560,7 @@ export = { new CfnParameter(stack2, 'SomeParameter', { type: 'String', default: account1 }); test.throws(() => { - ConstructNode.prepare(app.node); + app.synth(); }, /Stack "Stack2" cannot consume a cross reference from stack "Stack1"/); test.done(); diff --git a/packages/@aws-cdk/core/test/test.stage.ts b/packages/@aws-cdk/core/test/test.stage.ts new file mode 100644 index 0000000000000..0cd11a7608f4f --- /dev/null +++ b/packages/@aws-cdk/core/test/test.stage.ts @@ -0,0 +1,219 @@ +import * as cxapi from '@aws-cdk/cx-api'; +import * as fs from 'fs'; +import { Test } from 'nodeunit'; +import * as path from 'path'; +import { App, CfnResource, Construct, IAspect, IConstruct, Stack, Stage } from '../lib'; + +let app: App; + +export = { + 'setUp'(cb: () => void) { + app = new App(); + cb(); + }, + + 'tearDown'(cb: () => void) { + rimraf(app.assemblyBuilder.outdir); + cb(); + }, + + 'Stack inherits unspecified part of the env from Stage'(test: Test) { + // GIVEN + const stage = new Stage(app, 'Stage', { + env: { account: 'account', region: 'region' }, + }); + + // WHEN + const stack1 = new Stack(stage, 'Stack1', { env: { region: 'elsewhere' } }); + const stack2 = new Stack(stage, 'Stack2', { env: { account: 'tnuocca' } }); + + // THEN + test.deepEqual([stack1.account, stack1.region], ['account', 'elsewhere']); + test.deepEqual([stack2.account, stack2.region], ['tnuocca', 'region']); + + test.done(); + }, + + 'The Stage Assembly is in the app Assembly\'s manifest'(test: Test) { + // WHEN + const stage = new Stage(app, 'Stage'); + new BogusStack(stage, 'Stack2'); + + // THEN -- app manifest contains an embedded cloud assembly + const appAsm = app.synth(); + + const artifact = appAsm.artifacts.find(isEmbeddedCloudAssemblyArtifact); + test.ok(artifact); + + test.done(); + }, + + 'Stacks in Stage are in a different cxasm than Stacks in App'(test: Test) { + // WHEN + const stack1 = new BogusStack(app, 'Stack1'); + const stage = new Stage(app, 'Stage'); + const stack2 = new BogusStack(stage, 'Stack2'); + + // THEN + const stageAsm = stage.synth(); + test.deepEqual(stageAsm.stacks.map(s => s.stackName), [stack2.stackName]); + + const appAsm = app.synth(); + test.deepEqual(appAsm.stacks.map(s => s.stackName), [stack1.stackName]); + + test.done(); + }, + + 'Default stack name in Stage objects incorporates the Stage name and no hash'(test: Test) { + // WHEN + const stage = new Stage(app, 'MyStage'); + const stack = new BogusStack(stage, 'MyStack'); + + // THEN + test.equal(stage.stageName, 'MyStage'); + test.equal(stack.stackName, 'MyStage-MyStack'); + + test.done(); + }, + + 'Can not have dependencies to stacks outside the embedded asm'(test: Test) { + // GIVEN + const stack1 = new BogusStack(app, 'Stack1'); + const stage = new Stage(app, 'MyStage'); + const stack2 = new BogusStack(stage, 'Stack2'); + + // WHEN + test.throws(() => { + stack2.addDependency(stack1); + }, /dependency cannot cross stage boundaries/); + + test.done(); + }, + + 'When we synth() a stage, prepare must be called on constructs in the stage'(test: Test) { + // GIVEN + let prepared = false; + const stage = new Stage(app, 'MyStage'); + const stack = new BogusStack(stage, 'Stack'); + class HazPrepare extends Construct { + protected prepare() { + prepared = true; + } + } + new HazPrepare(stack, 'Preparable'); + + // WHEN + stage.synth(); + + // THEN + test.equals(prepared, true); + + test.done(); + }, + + 'When we synth() a stage, aspects inside it must have been applied'(test: Test) { + // GIVEN + const stage = new Stage(app, 'MyStage'); + const stack = new BogusStack(stage, 'Stack'); + + // WHEN + const aspect = new TouchingAspect(); + stack.node.applyAspect(aspect); + + // THEN + app.synth(); + test.deepEqual(aspect.visits.map(c => c.node.path), [ + 'MyStage/Stack', + 'MyStage/Stack/Resource', + ]); + + test.done(); + }, + + 'Aspects do not apply inside a Stage'(test: Test) { + // GIVEN + const stage = new Stage(app, 'MyStage'); + new BogusStack(stage, 'Stack'); + + // WHEN + const aspect = new TouchingAspect(); + app.node.applyAspect(aspect); + + // THEN + app.synth(); + test.deepEqual(aspect.visits.map(c => c.node.path), [ + '', + 'Tree', + ]); + test.done(); + }, + + 'Automatic dependencies inside a stage are available immediately after synth'(test: Test) { + // GIVEN + const stage = new Stage(app, 'MyStage'); + const stack1 = new Stack(stage, 'Stack1'); + const stack2 = new Stack(stage, 'Stack2'); + + // WHEN + const resource1 = new CfnResource(stack1, 'Resource', { + type: 'CDK::Test::Resource', + }); + new CfnResource(stack2, 'Resource', { + type: 'CDK::Test::Resource', + properties: { + OtherThing: resource1.ref, + }, + }); + + const asm = stage.synth(); + + // THEN + test.deepEqual( + asm.getStackArtifact(stack2.artifactId).dependencies.map(d => d.id), + [stack1.artifactId]); + + test.done(); + }, +}; + +class TouchingAspect implements IAspect { + public readonly visits = new Array(); + public visit(node: IConstruct): void { + this.visits.push(node); + } +} + +/** + * rm -rf reimplementation, don't want to depend on an NPM package for this + */ +function rimraf(fsPath: string) { + try { + const isDir = fs.lstatSync(fsPath).isDirectory(); + + if (isDir) { + for (const file of fs.readdirSync(fsPath)) { + rimraf(path.join(fsPath, file)); + } + fs.rmdirSync(fsPath); + } else { + fs.unlinkSync(fsPath); + } + } catch (e) { + // We will survive ENOENT + if (e.code !== 'ENOENT') { throw e; } + } +} + +class BogusStack extends Stack { + constructor(scope: Construct, id: string) { + super(scope, id); + + new CfnResource(this, 'Resource', { + type: 'CDK::Test::Resource', + }); + } +} + +function isEmbeddedCloudAssemblyArtifact(a: any): a is cxapi.EmbeddedCloudAssemblyArtifact { + return a instanceof cxapi.EmbeddedCloudAssemblyArtifact; +} \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md b/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md new file mode 100644 index 0000000000000..7ee50453c6fbb --- /dev/null +++ b/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md @@ -0,0 +1,93 @@ +# Embedded Assemblies + +For the CI/CD project we need to be able to a final, authoratative, immutable +rendition of part of the construct tree. This is a part of the application +that we can ask the CI/CD system to deploy as a unit, and have it get a fighting +chance of getting it right. This is because: + +- The stacks will be known. +- Their interdependencies will be known, and won't change anymore. + +To that end, we're introducing the concept of an "embedded cloud assembly". +This is a part of the construct tree that is finalized independently of the +rest, so that other constructs can reflect on it. + +Constructs of type `Stage` will produce embedded cloud assemblies. + +## Restrictions + +### Assets + +Right now, if the same asset is used in multiple cloud assemblies, it will +be staged independently in ever Cloud Assembly (making it take up more +space than necessary). + +This is unfortunate. We can think about sharing the staging directories +between Stages, should be an easy optimization that can be applied later. + +### Dependencies + +It seems that it might be desirable to have dependencies that reach outside +a single `Stage`. Consider the case where we have shared resources that +may be shared between Stages. A typical example would be a VPC: + +``` + ┌───────────────┐ + │ │ + │ VpcStack │ + │ │ + └───────────────┘ + ▲ + │ + │ + ┌─────────────┴─────────────┐ + │ │ +┌───────────────┼──────────┐ ┌──────────┼───────────────┐ +│Stage │ │ │ │ Stage│ +│ │ │ │ │ │ +│ ┌───────────────┐ │ │ ┌───────────────┐ │ +│ │ │ │ │ │ │ │ +│ │ App1Stack │ │ │ │ App2Stack │ │ +│ │ │ │ │ │ │ │ +│ └───────────────┘ │ │ └───────────────┘ │ +│ │ │ │ +└──────────────────────────┘ └──────────────────────────┘ +``` + +This seems like a reasonable thing to want to be able to do. + + +Right now, for practical reasons we're disallowing dependencies outside +nested assemblies. That is not to say that this can never be made to work, +but as it's really rather a significant chunk of work it has not been +implemented yet. Things to consider: + +- Do artifact identifiers need to be globally unique? (Does that destroy + local assumptions around naming that constructs can make?) +- How are artifacts addressed across assembly boundaries? Are they just the + absolute name, wherever in the Cloud Assembly tree the artifact is? Do they + represent a path from the top-level cloud assembly + (`SubAsm/SubAsm/Artifact`)? Are they relative paths (`../SubAsm/Artifact`)? +- Can there be cyclic dependencies between nested assemblies? Is it okay to + have both dependencies `AsmA/Stack1 -> AsmB/Stack1`, and `AsmB/Stack2 -> + AsmA/Stack2`? Why, or why not? How will we ensure that? + +Even if we can make the addressing work at the artifact level, at the +construct tree level we'd be giving up the guarantees we are getting from +having `Stage` constructs produce isolated Cloud Assemblies by having +dependencies outside them. Consider having two stages, `StageA` with `StackA` +and `StageB` with `StackB`. We must `synth()` them in some order, either A or +B first. Let's say A goes first (but the same argument obviously holds in +reverse). What if during the `synth()` of `StageB`, we discover `StackB` +introduces a dependency on `StackA`? By that point, `StageA` has already +synthesized and `StackA` has produced a (so-called "immutable") template. +Obviously we can't change that anymore, so we can't introduce that dependency +anymore. + +Seems like we should be calling `synth()` on multiple stages consumer-first! + +The problem is that we are generally building a Pipeline *producer*-first, since +we are modeling and building it in deployment order, which is the reverse order +the pipeline would `synth()` each of the stages in, in order to build itself. + +Since this is all very tricky, let's consider it out of scope for now. \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/jest.config.js b/packages/@aws-cdk/cx-api/jest.config.js index cd664e1d069e5..d984ff822379b 100644 --- a/packages/@aws-cdk/cx-api/jest.config.js +++ b/packages/@aws-cdk/cx-api/jest.config.js @@ -1,2 +1,10 @@ const baseConfig = require('../../../tools/cdk-build-tools/config/jest.config'); -module.exports = baseConfig; +module.exports = { + ...baseConfig, + coverageThreshold: { + global: { + ...baseConfig.coverageThreshold.global, + branches: 75, + }, + }, +}; diff --git a/packages/@aws-cdk/cx-api/lib/asset-manifest-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts similarity index 90% rename from packages/@aws-cdk/cx-api/lib/asset-manifest-artifact.ts rename to packages/@aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts index 8146a276d7e4f..07414bafe4249 100644 --- a/packages/@aws-cdk/cx-api/lib/asset-manifest-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts @@ -1,7 +1,7 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as path from 'path'; -import { CloudArtifact } from './cloud-artifact'; -import { CloudAssembly } from './cloud-assembly'; +import { CloudArtifact } from '../cloud-artifact'; +import { CloudAssembly } from '../cloud-assembly'; /** * Asset manifest is a description of a set of assets which need to be built and published diff --git a/packages/@aws-cdk/cx-api/lib/cloudformation-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts similarity index 89% rename from packages/@aws-cdk/cx-api/lib/cloudformation-artifact.ts rename to packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts index 2373e45e0eabc..e22bc5764a798 100644 --- a/packages/@aws-cdk/cx-api/lib/cloudformation-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts @@ -1,16 +1,11 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as fs from 'fs'; import * as path from 'path'; -import { CloudArtifact } from './cloud-artifact'; -import { CloudAssembly } from './cloud-assembly'; -import { Environment, EnvironmentUtils } from './environment'; +import { CloudArtifact } from '../cloud-artifact'; +import { CloudAssembly } from '../cloud-assembly'; +import { Environment, EnvironmentUtils } from '../environment'; export class CloudFormationStackArtifact extends CloudArtifact { - /** - * The CloudFormation template for this stack. - */ - public readonly template: any; - /** * The file name of the template. */ @@ -87,6 +82,8 @@ export class CloudFormationStackArtifact extends CloudArtifact { */ public readonly terminationProtection?: boolean; + private _template: any | undefined; + constructor(assembly: CloudAssembly, artifactId: string, artifact: cxschema.ArtifactManifest) { super(assembly, artifactId, artifact); @@ -107,7 +104,6 @@ export class CloudFormationStackArtifact extends CloudArtifact { this.terminationProtection = properties.terminationProtection; this.stackName = properties.stackName || artifactId; - this.template = JSON.parse(fs.readFileSync(path.join(this.assembly.directory, this.templateFile), 'utf-8')); this.assets = this.findMetadataByType(cxschema.ArtifactMetadataEntryType.ASSET).map(e => e.data as cxschema.AssetMetadataEntry); this.displayName = this.stackName === artifactId @@ -117,4 +113,14 @@ export class CloudFormationStackArtifact extends CloudArtifact { this.name = this.stackName; // backwards compat this.originalName = this.stackName; } + + /** + * The CloudFormation template for this stack. + */ + public get template(): any { + if (this._template === undefined) { + this._template = JSON.parse(fs.readFileSync(path.join(this.assembly.directory, this.templateFile), 'utf-8')); + } + return this._template; + } } diff --git a/packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts new file mode 100644 index 0000000000000..d25b7639e3934 --- /dev/null +++ b/packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts @@ -0,0 +1,49 @@ +import * as cxschema from '@aws-cdk/cloud-assembly-schema'; +import * as path from 'path'; +import { CloudArtifact } from '../cloud-artifact'; +import { CloudAssembly } from '../cloud-assembly'; + +/** + * Asset manifest is a description of a set of assets which need to be built and published + */ +export class EmbeddedCloudAssemblyArtifact extends CloudArtifact { + /** + * The relative directory name of the asset manifest + */ + public readonly directoryName: string; + + /** + * Display name + */ + public readonly displayName: string; + + /** + * Cache for the inner assembly loading + */ + private _embeddedAssembly?: CloudAssembly; + + constructor(assembly: CloudAssembly, name: string, artifact: cxschema.ArtifactManifest) { + super(assembly, name, artifact); + + const properties = (this.manifest.properties || {}) as cxschema.EmbeddedCloudAssemblyProperties; + this.directoryName = properties.directoryName; + this.displayName = properties.displayName ?? name; + } + + /** + * Full path to the Embedded Assembly + */ + public get fullPath(): string { + return path.join(this.assembly.directory, this.directoryName); + } + + /** + * The embedded Assembly + */ + public get embeddedAssembly(): CloudAssembly { + if (!this._embeddedAssembly) { + this._embeddedAssembly = new CloudAssembly(this.fullPath); + } + return this._embeddedAssembly; + } +} diff --git a/packages/@aws-cdk/cx-api/lib/tree-cloud-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/tree-cloud-artifact.ts similarity index 83% rename from packages/@aws-cdk/cx-api/lib/tree-cloud-artifact.ts rename to packages/@aws-cdk/cx-api/lib/artifacts/tree-cloud-artifact.ts index 142671e882e23..689f3468ca252 100644 --- a/packages/@aws-cdk/cx-api/lib/tree-cloud-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/artifacts/tree-cloud-artifact.ts @@ -1,6 +1,6 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; -import { CloudArtifact } from './cloud-artifact'; -import { CloudAssembly } from './cloud-assembly'; +import { CloudArtifact } from '../cloud-artifact'; +import { CloudAssembly } from '../cloud-assembly'; export class TreeCloudArtifact extends CloudArtifact { public readonly file: string; diff --git a/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts b/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts index 55cd7567e1612..6b586c4206345 100644 --- a/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts @@ -49,6 +49,8 @@ export class CloudArtifact { return new TreeCloudArtifact(assembly, id, artifact); case cxschema.ArtifactType.ASSET_MANIFEST: return new AssetManifestArtifact(assembly, id, artifact); + case cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY: + return new EmbeddedCloudAssemblyArtifact(assembly, id, artifact); default: return undefined; } @@ -88,7 +90,7 @@ export class CloudArtifact { if (this._deps) { return this._deps; } this._deps = this._dependencyIDs.map(id => { - const dep = this.assembly.artifacts.find(a => a.id === id); + const dep = this.assembly.tryGetArtifact(id); if (!dep) { throw new Error(`Artifact ${this.id} depends on non-existing artifact ${id}`); } @@ -143,6 +145,7 @@ export class CloudArtifact { } // needs to be defined at the end to avoid a cyclic dependency -import { AssetManifestArtifact } from './asset-manifest-artifact'; -import { CloudFormationStackArtifact } from './cloudformation-artifact'; -import { TreeCloudArtifact } from './tree-cloud-artifact'; \ No newline at end of file +import { AssetManifestArtifact } from './artifacts/asset-manifest-artifact'; +import { CloudFormationStackArtifact } from './artifacts/cloudformation-artifact'; +import { EmbeddedCloudAssemblyArtifact } from './artifacts/embedded-cloud-assembly-artifact'; +import { TreeCloudArtifact } from './artifacts/tree-cloud-artifact'; \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts b/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts index 0cf2e3d2ea9e0..04b28710626f5 100644 --- a/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts +++ b/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts @@ -2,10 +2,10 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import { CloudFormationStackArtifact } from './artifacts/cloudformation-artifact'; +import { TreeCloudArtifact } from './artifacts/tree-cloud-artifact'; import { CloudArtifact } from './cloud-artifact'; -import { CloudFormationStackArtifact } from './cloudformation-artifact'; import { topologicalSort } from './toposort'; -import { TreeCloudArtifact } from './tree-cloud-artifact'; /** * The name of the root manifest file of the assembly. @@ -69,6 +69,8 @@ export class CloudAssembly { /** * Returns a CloudFormation stack artifact from this assembly. * + * Will only search the current assembly. + * * @param stackName the name of the CloudFormation stack. * @throws if there is no stack artifact by that name * @throws if there is more than one stack with the same stack name. You can @@ -173,6 +175,13 @@ export class CloudAssembly { * Can be used to build a cloud assembly. */ export class CloudAssemblyBuilder { + /** + * Turn the given optional output directory into a fixed output directory + */ + public static determineOutputDirectory(outdir?: string) { + return outdir ?? fs.mkdtempSync(path.join(os.tmpdir(), 'cdk.out')); + } + /** * The root directory of the resulting cloud assembly. */ @@ -186,7 +195,7 @@ export class CloudAssemblyBuilder { * @param outdir The output directory, uses temporary directory if undefined */ constructor(outdir?: string) { - this.outdir = outdir || fs.mkdtempSync(path.join(os.tmpdir(), 'cdk.out')); + this.outdir = CloudAssemblyBuilder.determineOutputDirectory(outdir); // we leverage the fact that outdir is long-lived to avoid staging assets into it // that were already staged (copying can be expensive). this is achieved by the fact @@ -198,7 +207,7 @@ export class CloudAssemblyBuilder { throw new Error(`${this.outdir} must be a directory`); } } else { - fs.mkdirSync(this.outdir); + fs.mkdirSync(this.outdir, { recursive: true }); } } @@ -251,6 +260,41 @@ export class CloudAssemblyBuilder { return new CloudAssembly(this.outdir); } + /** + * Begin an embedded assembly in this assembly + * + * If dirName is not given, it will default to artifactId. + */ + public openEmbeddedAssembly(artifactId: string, dirName?: string) { + const directoryName = dirName ?? artifactId; + const innerAsmDir = path.join(this.outdir, directoryName); + + // WHEN + this.addArtifact(artifactId, { + type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + properties: { + directoryName, + } as cxschema.EmbeddedCloudAssemblyProperties, + }); + return new CloudAssemblyBuilder(innerAsmDir); + } +} + +/** + * Options for opening an embedded assembly + */ +export interface EmbeddedAssemblyOptions { + /** + * Identifier for this assembly + */ + readonly id: string; + + /** + * Display name for this assembly + * + * @default - Artifact ID is used as display name + */ + readonly displayName?: string; } /** @@ -340,4 +384,4 @@ function filterUndefined(obj: any): any { function ignore(_x: any) { return; -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/lib/index.ts b/packages/@aws-cdk/cx-api/lib/index.ts index 916ee80b068d4..d73148b92d663 100644 --- a/packages/@aws-cdk/cx-api/lib/index.ts +++ b/packages/@aws-cdk/cx-api/lib/index.ts @@ -4,9 +4,10 @@ export * from './context/ami'; export * from './context/availability-zones'; export * from './context/endpoint-service-availability-zones'; export * from './cloud-artifact'; -export * from './asset-manifest-artifact'; -export * from './cloudformation-artifact'; -export * from './tree-cloud-artifact'; +export * from './artifacts/asset-manifest-artifact'; +export * from './artifacts/cloudformation-artifact'; +export * from './artifacts/tree-cloud-artifact'; +export * from './artifacts/embedded-cloud-assembly-artifact'; export * from './cloud-assembly'; export * from './assets'; export * from './environment'; diff --git a/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts b/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts index bc348d9442188..c7c939924d924 100644 --- a/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts +++ b/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts @@ -2,12 +2,12 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { CloudAssemblyBuilder } from '../lib'; +import * as cxapi from '../lib'; test('cloud assembly builder', () => { // GIVEN const outdir = fs.mkdtempSync(path.join(os.tmpdir(), 'cloud-assembly-builder-tests')); - const session = new CloudAssemblyBuilder(outdir); + const session = new cxapi.CloudAssemblyBuilder(outdir); const templateFile = 'foo.template.json'; // WHEN @@ -121,12 +121,12 @@ test('cloud assembly builder', () => { }); test('outdir must be a directory', () => { - expect(() => new CloudAssemblyBuilder(__filename)).toThrow('must be a directory'); + expect(() => new cxapi.CloudAssemblyBuilder(__filename)).toThrow('must be a directory'); }); test('duplicate missing values with the same key are only reported once', () => { const outdir = fs.mkdtempSync(path.join(os.tmpdir(), 'cloud-assembly-builder-tests')); - const session = new CloudAssemblyBuilder(outdir); + const session = new cxapi.CloudAssemblyBuilder(outdir); const props: cxschema.ContextQueryProperties = { account: '1234', @@ -141,3 +141,29 @@ test('duplicate missing values with the same key are only reported once', () => expect(assembly.manifest.missing!.length).toEqual(1); }); + +test('write and read embedded Cloud Assembly artifact', () => { + // GIVEN + const outdir = fs.mkdtempSync(path.join(os.tmpdir(), 'cloud-assembly-builder-tests')); + const session = new cxapi.CloudAssemblyBuilder(outdir); + + const innerAsmDir = path.join(outdir, 'hello'); + new cxapi.CloudAssemblyBuilder(innerAsmDir).buildAssembly(); + + // WHEN + session.addArtifact('Assembly', { + type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + properties: { + directoryName: 'hello', + } as cxschema.EmbeddedCloudAssemblyProperties, + }); + const asm = session.buildAssembly(); + + // THEN + const art = asm.tryGetArtifact('Assembly') as cxapi.EmbeddedCloudAssemblyArtifact | undefined; + expect(art).toBeInstanceOf(cxapi.EmbeddedCloudAssemblyArtifact); + expect(art?.fullPath).toEqual(path.join(outdir, 'hello')); + + const embeddedAsm = art?.embeddedAssembly; + expect(embeddedAsm?.artifacts.length).toEqual(0); +}); \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/test/placeholders.test.ts b/packages/@aws-cdk/cx-api/test/placeholders.test.ts index 9b39478abd611..658d8a4670433 100644 --- a/packages/@aws-cdk/cx-api/test/placeholders.test.ts +++ b/packages/@aws-cdk/cx-api/test/placeholders.test.ts @@ -1,4 +1,4 @@ -import { EnvironmentPlaceholders, IEnvironmentPlaceholderProvider } from '../lib'; +import { EnvironmentPlaceholders, EnvironmentPlaceholderValues, IEnvironmentPlaceholderProvider } from '../lib'; test('complex placeholder substitution', async () => { const replacer: IEnvironmentPlaceholderProvider = { @@ -25,3 +25,30 @@ test('complex placeholder substitution', async () => { }, }); }); + +test('sync placeholder substitution', () => { + const replacer: EnvironmentPlaceholderValues = { + accountId: 'current_account', + region: 'current_region', + partition: 'current_partition', + }; + + expect(EnvironmentPlaceholders.replace({ + destinations: { + theDestination: { + assumeRoleArn: 'arn:${AWS::Partition}:role-${AWS::AccountId}', + bucketName: 'some_bucket-${AWS::AccountId}-${AWS::Region}', + objectKey: 'some_key-${AWS::AccountId}-${AWS::Region}', + }, + }, + }, replacer)).toEqual({ + destinations: { + theDestination: { + assumeRoleArn: 'arn:current_partition:role-current_account', + bucketName: 'some_bucket-current_account-current_region', + objectKey: 'some_key-current_account-current_region', + }, + }, + }); + +}); From 137a32abba580085506b27e10cddc4170eee24ff Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 4 Jun 2020 15:50:32 +0300 Subject: [PATCH 12/27] extract "real synth" to a separate file --- packages/@aws-cdk/core/lib/assembly.ts | 145 +----------------- .../@aws-cdk/core/lib/construct-compat.ts | 4 +- .../@aws-cdk/core/lib/private/synthesis.ts | 143 +++++++++++++++++ 3 files changed, 148 insertions(+), 144 deletions(-) create mode 100644 packages/@aws-cdk/core/lib/private/synthesis.ts diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts index d78a355b27b94..d3608e10367f7 100644 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -1,10 +1,7 @@ -import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; -import * as constructs from 'constructs'; -import { Construct, ConstructOrder, IConstruct, SynthesisOptions, ValidationError } from './construct-compat'; +import { Construct, IConstruct } from './construct-compat'; import { Environment } from './environment'; -import { prepareApp } from './private/prepare-app'; -import { collectRuntimeInformation } from './private/runtime-info'; +import { synthesize } from './private/synthesis'; /** * Initialization props for an assembly. @@ -153,7 +150,7 @@ export class Assembly extends Construct { */ public synth(options: AssemblySynthesisOptions = { }): cxapi.CloudAssembly { if (!this.assembly) { - this.assembly = realSynth(this, options); + this.assembly = synthesize(this, options); } return this.assembly; @@ -170,139 +167,3 @@ export interface AssemblySynthesisOptions { */ readonly skipValidation?: boolean; } - -/** - * Interface which provides access to special methods of Construct - * - * @experimental - */ -interface IProtectedConstructMethods extends IConstruct { - /** - * Method that gets called when a construct should synthesize itself to an assembly - */ - onSynthesize(session: constructs.ISynthesisSession): void; - - /** - * Method that gets called to validate a construct - */ - onValidate(): string[]; - - /** - * Method that gets called to prepare a construct - */ - onPrepare(): void; -} - -export function realSynth(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly { - const builder = Assembly.isAssembly(root) - ? root.assemblyBuilder - : new cxapi.CloudAssemblyBuilder(options.outdir); - - // okay, now we start by calling "synth" on all nested assemblies (which will take care of all their children) - for (const child of root.node.findAll(ConstructOrder.POSTORDER)) { - if (child === root) { continue; } - if (!Assembly.isAssembly(child)) { continue; } - if (Assembly.of(child) !== root) { continue; } - child.synth(options); - } - - const inAssembly = (c: IConstruct) => { - // if the root is not an assembly (i.e. it's a stack in unit tests), then consider - // everything to be in an assembly - if (!Assembly.isAssembly(root)) { - return true; - } - - // always include self - if (c === root) { - return true; - } - - // if the child is an assembly, omit it - if (Assembly.isAssembly(c)) { - return false; - } - - return Assembly.of(c) === root; - }; - - // find all child constructs within this assembly (sorted depth-first), and - // include direct nested assemblies, but do not include any constructs from - // nested assemblies as they will be covered by their assembly's synth() - // method. - const findChildren = () => root.node - .findAll(ConstructOrder.POSTORDER) - .filter(inAssembly); - - const children = findChildren(); - - for (const child of children) { - - // hackery to be able to access some private members with strong types (yack!) - const node = child.node._actualNode as unknown as Omit & { - invokedAspects: constructs.IAspect[]; - _aspects: constructs.IAspect[]; - }; - - for (const aspect of node._aspects) { - if (node.invokedAspects.includes(aspect)) { - continue; - } - - child.node.findAll(ConstructOrder.PREORDER) - .filter(c => inAssembly(c)) - .forEach(c => aspect.visit(c)); - - node.invokedAspects.push(aspect); - } - } - - // invoke "prepare" on all of the children in this assembly. this is mostly here for legacy purposes - // the framework itself does not use prepare anymore. - for (const child of findChildren()) { - (child as IProtectedConstructMethods).onPrepare(); - } - - // resolve references - prepareApp(root); - - // give all children an opportunity to validate now that we've finished prepare - if (!options.skipValidation) { - const errors = new Array(); - for (const source of children) { - const messages = (source as IProtectedConstructMethods).onValidate(); - for (const message of messages) { - errors.push({ message, source: source as Construct }); - } - } - - if (errors.length > 0) { - const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); - throw new Error(`Validation failed with the following errors:\n ${errorList}`); - } - } - - // next, we invoke "onSynthesize" on all of our children. this will allow - // stacks to add themselves to the synthesized cloud assembly. - for (const child of children) { - (child as IProtectedConstructMethods).onSynthesize({ - outdir: builder.outdir, - assembly: builder, - }); - } - - // Add this assembly to the parent assembly manifest (if we have one) - if (Assembly.isAssembly(root) && root.parentAssembly) { - root.parentAssembly.assemblyBuilder.addArtifact(root.assemblyArtifactId, { - type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, - properties: { - directoryName: root.assemblyArtifactId, - displayName: root.node.path, - } as cxschema.EmbeddedCloudAssemblyProperties, - }); - - } - - const runtimeInfo = root.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); - return builder.buildAssembly({ runtimeInfo }); -} \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/construct-compat.ts b/packages/@aws-cdk/core/lib/construct-compat.ts index c4a33bc57208a..89755d75de30e 100644 --- a/packages/@aws-cdk/core/lib/construct-compat.ts +++ b/packages/@aws-cdk/core/lib/construct-compat.ts @@ -227,8 +227,8 @@ export class ConstructNode { */ public static synth(node: ConstructNode, options: SynthesisOptions = { }): cxapi.CloudAssembly { // eslint-disable-next-line @typescript-eslint/no-require-imports - const a: typeof import('./assembly') = require('./assembly'); - return a.realSynth(node.root, options); + const a: typeof import('././private/synthesis') = require('./private/synthesis'); + return a.synthesize(node.root, options); } /** diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts new file mode 100644 index 0000000000000..c961f59133e47 --- /dev/null +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -0,0 +1,143 @@ +import * as cxschema from '@aws-cdk/cloud-assembly-schema'; +import * as cxapi from '@aws-cdk/cx-api'; +import * as constructs from 'constructs'; +import { Assembly } from '../assembly'; +import { Construct, ConstructOrder, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat'; +import { prepareApp } from './prepare-app'; +import { collectRuntimeInformation } from './runtime-info'; + +export function synthesize(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly { + const builder = Assembly.isAssembly(root) + ? root.assemblyBuilder + : new cxapi.CloudAssemblyBuilder(options.outdir); + + // okay, now we start by calling "synth" on all nested assemblies (which will take care of all their children) + for (const child of root.node.findAll(ConstructOrder.POSTORDER)) { + if (child === root) { continue; } + if (!Assembly.isAssembly(child)) { continue; } + if (Assembly.of(child) !== root) { continue; } + child.synth(options); + } + + const inAssembly = (c: IConstruct) => { + // if the root is not an assembly (i.e. it's a stack in unit tests), then consider + // everything to be in an assembly + if (!Assembly.isAssembly(root)) { + return true; + } + + // always include self + if (c === root) { + return true; + } + + // if the child is an assembly, omit it + if (Assembly.isAssembly(c)) { + return false; + } + + return Assembly.of(c) === root; + }; + + // find all child constructs within this assembly (sorted depth-first), and + // include direct nested assemblies, but do not include any constructs from + // nested assemblies as they will be covered by their assembly's synth() + // method. + const findChildren = () => root.node + .findAll(ConstructOrder.POSTORDER) + .filter(inAssembly); + + const children = findChildren(); + + for (const child of children) { + + // hackery to be able to access some private members with strong types (yack!) + const node = child.node._actualNode as unknown as Omit & { + invokedAspects: constructs.IAspect[]; + _aspects: constructs.IAspect[]; + }; + + for (const aspect of node._aspects) { + if (node.invokedAspects.includes(aspect)) { + continue; + } + + child.node.findAll(ConstructOrder.PREORDER) + .filter(c => inAssembly(c)) + .forEach(c => aspect.visit(c)); + + node.invokedAspects.push(aspect); + } + } + + // invoke "prepare" on all of the children in this assembly. this is mostly here for legacy purposes + // the framework itself does not use prepare anymore. + for (const child of findChildren()) { + (child as IProtectedConstructMethods).onPrepare(); + } + + // resolve references + prepareApp(root); + + // give all children an opportunity to validate now that we've finished prepare + if (!options.skipValidation) { + const errors = new Array(); + for (const source of children) { + const messages = (source as IProtectedConstructMethods).onValidate(); + for (const message of messages) { + errors.push({ message, source: source as Construct }); + } + } + + if (errors.length > 0) { + const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); + throw new Error(`Validation failed with the following errors:\n ${errorList}`); + } + } + + // next, we invoke "onSynthesize" on all of our children. this will allow + // stacks to add themselves to the synthesized cloud assembly. + for (const child of children) { + (child as IProtectedConstructMethods).onSynthesize({ + outdir: builder.outdir, + assembly: builder, + }); + } + + // Add this assembly to the parent assembly manifest (if we have one) + if (Assembly.isAssembly(root) && root.parentAssembly) { + root.parentAssembly.assemblyBuilder.addArtifact(root.assemblyArtifactId, { + type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + properties: { + directoryName: root.assemblyArtifactId, + displayName: root.node.path, + } as cxschema.EmbeddedCloudAssemblyProperties, + }); + + } + + const runtimeInfo = root.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); + return builder.buildAssembly({ runtimeInfo }); +} + +/** + * Interface which provides access to special methods of Construct + * + * @experimental + */ +interface IProtectedConstructMethods extends IConstruct { + /** + * Method that gets called when a construct should synthesize itself to an assembly + */ + onSynthesize(session: constructs.ISynthesisSession): void; + + /** + * Method that gets called to validate a construct + */ + onValidate(): string[]; + + /** + * Method that gets called to prepare a construct + */ + onPrepare(): void; +} From 43630e03f3fda4660efe2b19337d406b53a2f706 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 5 Jun 2020 11:57:15 +0200 Subject: [PATCH 13/27] Making the synthesis file a little nicer --- packages/@aws-cdk/core/lib/assembly.ts | 3 +- .../@aws-cdk/core/lib/private/synthesis.ts | 215 +++++++++++------- packages/@aws-cdk/core/lib/stack.ts | 2 +- packages/@aws-cdk/core/test/test.stage.ts | 23 ++ 4 files changed, 157 insertions(+), 86 deletions(-) diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts index d3608e10367f7..c3377d6c9d595 100644 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -140,7 +140,8 @@ export class Assembly extends Construct { * Derived from the construct path. */ public get assemblyArtifactId() { - return `sub-${this.node.path.replace(/\//g, '-').replace(/^-+|-+$/g, '')}`; + if (!this.node.path) { return ''; } + return `stage-${this.node.path.replace(/\//g, '-').replace(/^-+|-+$/g, '')}`; } /** diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index c961f59133e47..a5ae9b571a9d4 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -2,122 +2,159 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import * as constructs from 'constructs'; import { Assembly } from '../assembly'; -import { Construct, ConstructOrder, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat'; +import { Construct, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat'; import { prepareApp } from './prepare-app'; import { collectRuntimeInformation } from './runtime-info'; export function synthesize(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly { + // we start by calling "synth" on all nested assemblies (which will take care of all their children) + synthNestedAssemblies(root, options); + + invokeAspects(root); + + // This is mostly here for legacy purposes as the framework itself does not use prepare anymore. + prepareTree(root); + + // resolve references + prepareApp(root); + + // give all children an opportunity to validate now that we've finished prepare + if (!options.skipValidation) { + validateTree(root); + } + const builder = Assembly.isAssembly(root) ? root.assemblyBuilder : new cxapi.CloudAssemblyBuilder(options.outdir); - // okay, now we start by calling "synth" on all nested assemblies (which will take care of all their children) - for (const child of root.node.findAll(ConstructOrder.POSTORDER)) { - if (child === root) { continue; } - if (!Assembly.isAssembly(child)) { continue; } - if (Assembly.of(child) !== root) { continue; } - child.synth(options); - } + // next, we invoke "onSynthesize" on all of our children. this will allow + // stacks to add themselves to the synthesized cloud assembly. + synthesizeChildren(root, builder); - const inAssembly = (c: IConstruct) => { - // if the root is not an assembly (i.e. it's a stack in unit tests), then consider - // everything to be in an assembly - if (!Assembly.isAssembly(root)) { - return true; - } + // Add this assembly to the parent assembly manifest (if we have one) + if (Assembly.isAssembly(root)) { + addToParentAssembly(root); + } - // always include self - if (c === root) { - return true; - } + const runtimeInfo = root.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); + return builder.buildAssembly({ runtimeInfo }); +} - // if the child is an assembly, omit it - if (Assembly.isAssembly(c)) { - return false; +/** + * Find Assemblies inside the construct and call 'synth' on them + * + * (They will in turn recurse again) + */ +function synthNestedAssemblies(root: IConstruct, options: SynthesisOptions) { + for (const child of root.node.children) { + if (Assembly.isAssembly(child)) { + child.synth(options); + } else { + synthNestedAssemblies(child, options); } + } +} - return Assembly.of(c) === root; - }; - - // find all child constructs within this assembly (sorted depth-first), and - // include direct nested assemblies, but do not include any constructs from - // nested assemblies as they will be covered by their assembly's synth() - // method. - const findChildren = () => root.node - .findAll(ConstructOrder.POSTORDER) - .filter(inAssembly); - - const children = findChildren(); - - for (const child of children) { +/** + * Invoke aspects on the given construct tree. + * + * Aspects are not propagated across Assembly boundaries. The same Aspect will not be invoked + * twice for the same construct. + */ +function invokeAspects(root: IConstruct) { + recurse(root, []); + function recurse(construct: IConstruct, inheritedAspects: constructs.IAspect[]) { // hackery to be able to access some private members with strong types (yack!) - const node = child.node._actualNode as unknown as Omit & { - invokedAspects: constructs.IAspect[]; - _aspects: constructs.IAspect[]; - }; - - for (const aspect of node._aspects) { - if (node.invokedAspects.includes(aspect)) { - continue; - } + const node: NodeWithPrivatesHangingOut = construct.node._actualNode as any; - child.node.findAll(ConstructOrder.PREORDER) - .filter(c => inAssembly(c)) - .forEach(c => aspect.visit(c)); + const allAspectsHere = [...inheritedAspects ?? [], ...node._aspects]; + for (const aspect of allAspectsHere) { + if (node.invokedAspects.includes(aspect)) { continue; } + aspect.visit(construct); node.invokedAspects.push(aspect); } - } - - // invoke "prepare" on all of the children in this assembly. this is mostly here for legacy purposes - // the framework itself does not use prepare anymore. - for (const child of findChildren()) { - (child as IProtectedConstructMethods).onPrepare(); - } - - // resolve references - prepareApp(root); - // give all children an opportunity to validate now that we've finished prepare - if (!options.skipValidation) { - const errors = new Array(); - for (const source of children) { - const messages = (source as IProtectedConstructMethods).onValidate(); - for (const message of messages) { - errors.push({ message, source: source as Construct }); + for (const child of construct.node.children) { + if (!Assembly.isAssembly(child)) { + recurse(child, allAspectsHere); } } - - if (errors.length > 0) { - const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); - throw new Error(`Validation failed with the following errors:\n ${errorList}`); - } } +} - // next, we invoke "onSynthesize" on all of our children. this will allow - // stacks to add themselves to the synthesized cloud assembly. - for (const child of children) { - (child as IProtectedConstructMethods).onSynthesize({ +/** + * Prepare all constructs in the given construct tree in post-order. + * + * Stop at Assembly boundaries. + */ +function prepareTree(root: IConstruct) { + visit(root, 'post', construct => { + (construct as IProtectedConstructMethods).onPrepare(); + }); +} + +/** + * Synthesize children in post-order into the given builder + * + * Stop at Assembly boundaries. + */ +function synthesizeChildren(root: IConstruct, builder: cxapi.CloudAssemblyBuilder) { + visit(root, 'post', construct => { + (construct as IProtectedConstructMethods).onSynthesize({ outdir: builder.outdir, assembly: builder, }); + }); +} + +/** + * Validate all constructs in the given construct tree + */ +function validateTree(root: IConstruct) { + const errors = new Array(); + + visit(root, 'pre', construct => { + for (const message of (construct as IProtectedConstructMethods).onValidate()) { + errors.push({ message, source: construct as Construct }); + } + }); + + if (errors.length > 0) { + const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); + throw new Error(`Validation failed with the following errors:\n ${errorList}`); } +} - // Add this assembly to the parent assembly manifest (if we have one) - if (Assembly.isAssembly(root) && root.parentAssembly) { - root.parentAssembly.assemblyBuilder.addArtifact(root.assemblyArtifactId, { - type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, - properties: { - directoryName: root.assemblyArtifactId, - displayName: root.node.path, - } as cxschema.EmbeddedCloudAssemblyProperties, - }); +function addToParentAssembly(root: Assembly) { + if (!root.parentAssembly) { return; } + + root.parentAssembly.assemblyBuilder.addArtifact(root.assemblyArtifactId, { + type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + properties: { + directoryName: root.assemblyArtifactId, + displayName: root.node.path, + } as cxschema.EmbeddedCloudAssemblyProperties, + }); +} +/** + * Visit the given construct tree in either pre or post order, stopping at Assemblies + */ +function visit(root: IConstruct, order: 'pre' | 'post', cb: (x: IConstruct) => void) { + if (order === 'pre') { + cb(root); } - const runtimeInfo = root.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); - return builder.buildAssembly({ runtimeInfo }); + for (const child of root.node.children) { + if (Assembly.isAssembly(child)) { continue; } + visit(child, order, cb); + } + + if (order === 'post') { + cb(root); + } } /** @@ -141,3 +178,13 @@ interface IProtectedConstructMethods extends IConstruct { */ onPrepare(): void; } + +/** + * The constructs Node type, but with some aspects-related fields public. + * + * Hackery! + */ +type NodeWithPrivatesHangingOut = Omit & { + readonly invokedAspects: constructs.IAspect[]; + readonly _aspects: constructs.IAspect[]; +}; \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 4377bfc71842d..6c3bff37c8ba5 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -1041,7 +1041,7 @@ export function rootPathTo(construct: IConstruct, ancestor?: IConstruct): IConst * has only one component. Otherwise we fall back to the regular "makeUniqueId" * behavior. */ -export function makeStackName(components: string[]) { +function makeStackName(components: string[]) { if (components.length === 1) { return components[0]; } return makeUniqueId(components); } diff --git a/packages/@aws-cdk/core/test/test.stage.ts b/packages/@aws-cdk/core/test/test.stage.ts index 0cd11a7608f4f..3f7898af43c8c 100644 --- a/packages/@aws-cdk/core/test/test.stage.ts +++ b/packages/@aws-cdk/core/test/test.stage.ts @@ -64,6 +64,22 @@ export = { test.done(); }, + 'Can nest Stages inside other Stages'(test: Test) { + // WHEN + const outer = new Stage(app, 'Outer'); + const inner = new Stage(outer, 'Inner'); + const stack = new BogusStack(inner, 'Stack'); + + // WHEN + const appAsm = app.synth(); + const outerAsm = embeddedAsm(appAsm, outer.assemblyArtifactId); + const innerAsm = embeddedAsm(outerAsm, inner.assemblyArtifactId); + + test.ok(innerAsm.tryGetArtifact(stack.artifactId)); + + test.done(); + }, + 'Default stack name in Stage objects incorporates the Stage name and no hash'(test: Test) { // WHEN const stage = new Stage(app, 'MyStage'); @@ -214,6 +230,13 @@ class BogusStack extends Stack { } } +function embeddedAsm(asm: cxapi.CloudAssembly, artifactId: string): cxapi.CloudAssembly { + const a = asm.tryGetArtifact(artifactId); + if (!a) { throw new Error(`No such artifact in CloudAssembly: '${artifactId}' (have: ${asm.artifacts.map(art => art.id)})`); } + if (!isEmbeddedCloudAssemblyArtifact(a)) { throw new Error(`Found artifact '${artifactId}' but it's not a Cloud Assembly!`); } + return a.embeddedAssembly; +} + function isEmbeddedCloudAssemblyArtifact(a: any): a is cxapi.EmbeddedCloudAssemblyArtifact { return a instanceof cxapi.EmbeddedCloudAssemblyArtifact; } \ No newline at end of file From b811afd8e7c1121d2bc59ee73e25cbe076725759 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 11:16:20 +0300 Subject: [PATCH 14/27] Update allowed-breaking-changes.txt From 431490c66ba879b818ba141d0281194c37f2d0b7 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 11:38:58 +0300 Subject: [PATCH 15/27] touch ups Co-authored-by: Rico Huijbers --- .../@aws-cdk/core/lib/private/synthesis.ts | 48 ++++++++++--------- packages/@aws-cdk/core/lib/stack.ts | 2 +- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index a5ae9b571a9d4..7a790ff50a94f 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -23,13 +23,15 @@ export function synthesize(root: IConstruct, options: SynthesisOptions = { }): c validateTree(root); } + // in unit tests, we support creating free-standing stacks, so we create the + // assembly builder here. const builder = Assembly.isAssembly(root) ? root.assemblyBuilder : new cxapi.CloudAssemblyBuilder(options.outdir); // next, we invoke "onSynthesize" on all of our children. this will allow // stacks to add themselves to the synthesized cloud assembly. - synthesizeChildren(root, builder); + synthesizeTree(root, builder); // Add this assembly to the parent assembly manifest (if we have one) if (Assembly.isAssembly(root)) { @@ -66,7 +68,7 @@ function invokeAspects(root: IConstruct) { function recurse(construct: IConstruct, inheritedAspects: constructs.IAspect[]) { // hackery to be able to access some private members with strong types (yack!) - const node: NodeWithPrivatesHangingOut = construct.node._actualNode as any; + const node: NodeWithAspectPrivatesHangingOut = construct.node._actualNode as any; const allAspectsHere = [...inheritedAspects ?? [], ...node._aspects]; @@ -90,9 +92,7 @@ function invokeAspects(root: IConstruct) { * Stop at Assembly boundaries. */ function prepareTree(root: IConstruct) { - visit(root, 'post', construct => { - (construct as IProtectedConstructMethods).onPrepare(); - }); + visit(root, 'post', construct => construct.onPrepare()); } /** @@ -100,13 +100,11 @@ function prepareTree(root: IConstruct) { * * Stop at Assembly boundaries. */ -function synthesizeChildren(root: IConstruct, builder: cxapi.CloudAssemblyBuilder) { - visit(root, 'post', construct => { - (construct as IProtectedConstructMethods).onSynthesize({ - outdir: builder.outdir, - assembly: builder, - }); - }); +function synthesizeTree(root: IConstruct, builder: cxapi.CloudAssemblyBuilder) { + visit(root, 'post', construct => construct.onSynthesize({ + outdir: builder.outdir, + assembly: builder, + })); } /** @@ -116,8 +114,8 @@ function validateTree(root: IConstruct) { const errors = new Array(); visit(root, 'pre', construct => { - for (const message of (construct as IProtectedConstructMethods).onValidate()) { - errors.push({ message, source: construct as Construct }); + for (const message of construct.onValidate()) { + errors.push({ message, source: construct as unknown as Construct }); } }); @@ -127,14 +125,18 @@ function validateTree(root: IConstruct) { } } -function addToParentAssembly(root: Assembly) { - if (!root.parentAssembly) { return; } +/** + * Adds an assembly to it's parent's assembly manifest (unless it's the root, and then this does nothing). + * @param assembly Assembly to add + */ +function addToParentAssembly(assembly: Assembly) { + if (!assembly.parentAssembly) { return; } - root.parentAssembly.assemblyBuilder.addArtifact(root.assemblyArtifactId, { + assembly.parentAssembly.assemblyBuilder.addArtifact(assembly.assemblyArtifactId, { type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, properties: { - directoryName: root.assemblyArtifactId, - displayName: root.node.path, + directoryName: assembly.assemblyArtifactId, + displayName: assembly.node.path, } as cxschema.EmbeddedCloudAssemblyProperties, }); } @@ -142,9 +144,9 @@ function addToParentAssembly(root: Assembly) { /** * Visit the given construct tree in either pre or post order, stopping at Assemblies */ -function visit(root: IConstruct, order: 'pre' | 'post', cb: (x: IConstruct) => void) { +function visit(root: IConstruct, order: 'pre' | 'post', cb: (x: IProtectedConstructMethods) => void) { if (order === 'pre') { - cb(root); + cb(root as IProtectedConstructMethods); } for (const child of root.node.children) { @@ -153,7 +155,7 @@ function visit(root: IConstruct, order: 'pre' | 'post', cb: (x: IConstruct) => v } if (order === 'post') { - cb(root); + cb(root as IProtectedConstructMethods); } } @@ -184,7 +186,7 @@ interface IProtectedConstructMethods extends IConstruct { * * Hackery! */ -type NodeWithPrivatesHangingOut = Omit & { +type NodeWithAspectPrivatesHangingOut = Omit & { readonly invokedAspects: constructs.IAspect[]; readonly _aspects: constructs.IAspect[]; }; \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 6c3bff37c8ba5..5ba041745c1fe 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -725,7 +725,7 @@ export class Stack extends Construct implements ITaggable { * Find all dependencies as well and add the appropriate DependsOn fields. */ protected prepare() { - // if this stack is a roort (e.g. in unit tests), call `prepareApp` so that + // if this stack is a root (e.g. in unit tests), call `prepareApp` so that // we resolve cross-references and nested stack assets. if (!this.node.scope) { prepareApp(this); From 420e6c9e741d7fd000ff7b3a766b124a72909d26 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 12:46:38 +0300 Subject: [PATCH 16/27] update stack `env` description --- packages/@aws-cdk/core/lib/stack.ts | 64 ++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 5ba041745c1fe..b12f4b0063fa3 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -28,39 +28,65 @@ export interface StackProps { * The AWS environment (account/region) where this stack will be deployed. * * Set the `region`/`account` fields of `env` to either a concrete value to - * select the indicated environment (recommended for production stacks), or - * to the values of environment variables + * select the indicated environment (recommended for production stacks), or to + * the values of environment variables * `CDK_DEFAULT_REGION`/`CDK_DEFAULT_ACCOUNT` to let the target environment * depend on the AWS credentials/configuration that the CDK CLI is executed * under (recommended for development stacks). * - * If the `Stack` is instantiated inside a `Stage` construct, any undefined - * `region`/`account` fields from `env` will default to the same field on - * the encompassing `Stage`, if configured there. + * If the `Stack` is instantiated inside a `Stage`, any undefined + * `region`/`account` fields from `env` will default to the same field on the + * encompassing `Stage`, if configured there. * - * If either of `region`/`account` is not set nor inherited from `Stage`, - * the Stack will be *environment-agnostic*. - * - * Environment-agnostic stacks can be deployed to any environment, may not be - * able to take advantage of all features of the CDK. For example, they will - * not be able to use environmental context lookups, will not automatically - * translate Service Principals to the right format based on the environment's - * AWS partition, and other such enhancements. + * If either `region` or `account` are not set nor inherited from `Stage`, the + * Stack will be considered "*environment-agnostic*"". Environment-agnostic + * stacks can be deployed to any environment but may not be able to take + * advantage of all features of the CDK. For example, they will not be able to + * use environmental context lookups such as `ec2.Vpc.fromLookup` and will not + * automatically translate Service Principals to the right format based on the + * environment's AWS partition, and other such enhancements. * * @example * - * // Use a concrete account and region to deploy this stack to + * // Use a concrete account and region to deploy this stack to: + * // `.account` and `.region` will simply return these values. * new MyStack(app, 'Stack1', { - * env: { account: '123456789012', region: 'us-east-1' }, + * env: { + * account: '123456789012', + * region: 'us-east-1' + * }, * }); * - * // Use the CLI's current credentials to determine the target environment + * // Use the CLI's current credentials to determine the target environment: + * // `.account` and `.region` will reflect the account+region the CLI + * // is configured to use (based on the user CLI credentials) * new MyStack(app, 'Stack2', { - * env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, + * env: { + * account: process.env.CDK_DEFAULT_ACCOUNT, + * region: process.env.CDK_DEFAULT_REGION + * }, + * }); + * + * // Define multiple stacks stage associated with an environment + * const myStage = new Stage(app, 'MyStage', { + * env: { + * account: '123456789012', + * region: 'us-east-1' + * } * }); * - * @default - The environment of the containing `Stage` if available, otherwise create - * the stack will be environment-agnostic. + * // both of these stavks will use the stage's account/region: + * // `.account` and `.region` will resolve to the concrete values as above + * new MyStack(myStage, 'Stack1'); + * new YourStack(myStage, 'Stack1'); + * + * // Define an environment-agnostic stack: + * // `.account` and `.region` will resolve to `{ "Ref": "AWS::AccountId" }` and `{ "Ref": "AWS::Region" }` respectively. + * // which will only resolve to actual values by CloudFormation during deployment. + * new MyStack(app, 'Stack1'); + * + * @default - The environment of the containing `Stage` if available, + * otherwise create the stack will be environment-agnostic. */ readonly env?: Environment; From 8a8951a4d078e5dd5550fc8dd314f2415ff31b9e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 12:50:17 +0300 Subject: [PATCH 17/27] assembly name validation & more tests --- packages/@aws-cdk/core/lib/assembly.ts | 52 ++++++---- packages/@aws-cdk/core/lib/stage.ts | 14 +-- packages/@aws-cdk/core/test/test.stage.ts | 110 +++++++++++++++------- 3 files changed, 109 insertions(+), 67 deletions(-) diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts index c3377d6c9d595..8d3bd227cfdfe 100644 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -7,17 +7,6 @@ import { synthesize } from './private/synthesis'; * Initialization props for an assembly. */ export interface AssemblyProps { - /** - * Assembly name - * - * Will be prepended to default stack names of stacks defined under this assembly. - * - * Stack names can be overridden at the Stack level. - * - * @default - Same as the construct id - */ - readonly assemblyName?: string; - /** * Default AWS environment (account/region) for `Stack`s in this `Stage`. * @@ -60,25 +49,33 @@ export interface AssemblyProps { } /** - * An application modeling unit consisting of Stacks that should be deployed together + * An abstract application modeling unit consisting of Stacks that should be + * deployed together. * - * Derive a subclass of this construct and use it to model a single - * instance of your application. + * Derive a subclass of `Stage` and use it to model a single instance of your + * application. * * You can then instantiate your subclass multiple times to model multiple * copies of your application which should be be deployed to different * environments. + * + * @experimental */ export class Assembly extends Construct { /** - * Return the containing Stage object of a construct, if available + * Return the containing assembly of a construct, if available. If called on a + * nested assembly, returns its parent. + * + * @experimental */ public static of(construct: IConstruct): Assembly | undefined { return construct.node.scopes.reverse().slice(1).find(Assembly.isAssembly); } /** - * Test whether the given construct is a Assembly object + * Test whether the given construct is an assembly. + * + * @experimental */ public static isAssembly(x: any ): x is Assembly { return x !== null && x instanceof Assembly; @@ -86,28 +83,37 @@ export class Assembly extends Construct { /** * The default region for all resources defined within this assembly. + * + * @experimental */ public readonly region?: string; /** * The default account for all resources defined within this assembly. + * + * @experimental */ public readonly account?: string; /** - * The Cloud Assembly builder that is being used for this App + * The cloud assembly builder that is being used for this App * * @experimental */ public readonly assemblyBuilder: cxapi.CloudAssemblyBuilder; /** - * The name of the assembly. + * The name of the assembly. Based on names of the parent assemblies separated + * by hypens. + * + * @experimental */ public readonly assemblyName: string; /** * The parent assembly or `undefined` if this is the app. + * * + * @experimental */ public readonly parentAssembly?: Assembly; @@ -119,6 +125,10 @@ export class Assembly extends Construct { constructor(scope: Construct, id: string, props: AssemblyProps = {}) { super(scope, id); + if (id !== '' && !/^[a-z][a-z0-9\-\_\.]+$/i.test(id)) { + throw new Error(`invalid assembly name "${id}". Assembly name must start with a letter and contain only alphanumeric characters, hypens ('-'), underscores ('_') and periods ('.')`); + } + this.parentAssembly = Assembly.of(this); this.region = props.env?.region; @@ -131,17 +141,19 @@ export class Assembly extends Construct { ? this.parentAssembly.assemblyBuilder.openEmbeddedAssembly(this.assemblyArtifactId) : new cxapi.CloudAssemblyBuilder(props.outdir ?? process.env[cxapi.OUTDIR_ENV]); // TODO: << should; come; from; app; - this.assemblyName = props.assemblyName ?? (this.parentAssembly?.assemblyName ? `${this.parentAssembly.assemblyName}-${id}` : id); + this.assemblyName = [ this.parentAssembly?.assemblyName, id ].filter(x => x).join('-'); } /** * Artifact ID of embedded assembly * * Derived from the construct path. + * + * @experimental */ public get assemblyArtifactId() { if (!this.node.path) { return ''; } - return `stage-${this.node.path.replace(/\//g, '-').replace(/^-+|-+$/g, '')}`; + return `assembly-${this.node.path.replace(/\//g, '-').replace(/^-+|-+$/g, '')}`; } /** diff --git a/packages/@aws-cdk/core/lib/stage.ts b/packages/@aws-cdk/core/lib/stage.ts index 822c4bc887c4d..e34fd3f2fff18 100644 --- a/packages/@aws-cdk/core/lib/stage.ts +++ b/packages/@aws-cdk/core/lib/stage.ts @@ -37,17 +37,6 @@ export interface StageProps { * @default - The environments should be configured on the `Stack`s. */ readonly env?: Environment; - - /** - * Stage name - * - * Will be prepended to default Stack names of stacks in this Stage. - * - * Stack names can be overridden at the Stack level. - * - * @default - Same as the construct id - */ - readonly stageName?: string; } /** @@ -59,6 +48,8 @@ export interface StageProps { * You can then instantiate your subclass multiple times to model multiple * copies of your application which should be be deployed to different * environments. + * + * @experimental */ export class Stage extends Assembly { /** @@ -69,7 +60,6 @@ export class Stage extends Assembly { constructor(scope: Construct, id: string, props: StageProps = {}) { super(scope, id, { env: props.env, - assemblyName: props.stageName, }); this.stageName = this.assemblyName; diff --git a/packages/@aws-cdk/core/test/test.stage.ts b/packages/@aws-cdk/core/test/test.stage.ts index 3f7898af43c8c..eb61087d6587b 100644 --- a/packages/@aws-cdk/core/test/test.stage.ts +++ b/packages/@aws-cdk/core/test/test.stage.ts @@ -1,24 +1,11 @@ import * as cxapi from '@aws-cdk/cx-api'; -import * as fs from 'fs'; import { Test } from 'nodeunit'; -import * as path from 'path'; import { App, CfnResource, Construct, IAspect, IConstruct, Stack, Stage } from '../lib'; -let app: App; - export = { - 'setUp'(cb: () => void) { - app = new App(); - cb(); - }, - - 'tearDown'(cb: () => void) { - rimraf(app.assemblyBuilder.outdir); - cb(); - }, - 'Stack inherits unspecified part of the env from Stage'(test: Test) { // GIVEN + const app = new App(); const stage = new Stage(app, 'Stage', { env: { account: 'account', region: 'region' }, }); @@ -36,6 +23,7 @@ export = { 'The Stage Assembly is in the app Assembly\'s manifest'(test: Test) { // WHEN + const app = new App(); const stage = new Stage(app, 'Stage'); new BogusStack(stage, 'Stack2'); @@ -50,6 +38,7 @@ export = { 'Stacks in Stage are in a different cxasm than Stacks in App'(test: Test) { // WHEN + const app = new App(); const stack1 = new BogusStack(app, 'Stack1'); const stage = new Stage(app, 'Stage'); const stack2 = new BogusStack(stage, 'Stack2'); @@ -66,6 +55,7 @@ export = { 'Can nest Stages inside other Stages'(test: Test) { // WHEN + const app = new App(); const outer = new Stage(app, 'Outer'); const inner = new Stage(outer, 'Inner'); const stack = new BogusStack(inner, 'Stack'); @@ -82,6 +72,7 @@ export = { 'Default stack name in Stage objects incorporates the Stage name and no hash'(test: Test) { // WHEN + const app = new App(); const stage = new Stage(app, 'MyStage'); const stack = new BogusStack(stage, 'MyStack'); @@ -94,6 +85,7 @@ export = { 'Can not have dependencies to stacks outside the embedded asm'(test: Test) { // GIVEN + const app = new App(); const stack1 = new BogusStack(app, 'Stack1'); const stage = new Stage(app, 'MyStage'); const stack2 = new BogusStack(stage, 'Stack2'); @@ -108,6 +100,7 @@ export = { 'When we synth() a stage, prepare must be called on constructs in the stage'(test: Test) { // GIVEN + const app = new App(); let prepared = false; const stage = new Stage(app, 'MyStage'); const stack = new BogusStack(stage, 'Stack'); @@ -129,6 +122,7 @@ export = { 'When we synth() a stage, aspects inside it must have been applied'(test: Test) { // GIVEN + const app = new App(); const stage = new Stage(app, 'MyStage'); const stack = new BogusStack(stage, 'Stack'); @@ -148,6 +142,7 @@ export = { 'Aspects do not apply inside a Stage'(test: Test) { // GIVEN + const app = new App(); const stage = new Stage(app, 'MyStage'); new BogusStack(stage, 'Stack'); @@ -166,6 +161,7 @@ export = { 'Automatic dependencies inside a stage are available immediately after synth'(test: Test) { // GIVEN + const app = new App(); const stage = new Stage(app, 'MyStage'); const stack1 = new Stack(stage, 'Stack1'); const stack2 = new Stack(stage, 'Stack2'); @@ -190,6 +186,71 @@ export = { test.done(); }, + + 'Assemblies can be deeply nested'(test: Test) { + // GIVEN + const app = new App({ runtimeInfo: false, treeMetadata: false }); + + const level1 = new Stage(app, 'StageLevel1'); + const level2 = new Stage(level1, 'StageLevel2'); + new Stage(level2, 'StageLevel3'); + + // WHEN + const rootAssembly = app.synth(); + + // THEN + test.deepEqual(rootAssembly.manifest.artifacts, { + 'assembly-StageLevel1': { + type: 'cdk:cloud-assembly', + properties: { + directoryName: 'assembly-StageLevel1', + displayName: 'StageLevel1', + }, + }, + }); + + const assemblyLevel1 = embeddedAsm(rootAssembly, 'assembly-StageLevel1'); + test.deepEqual(assemblyLevel1.manifest.artifacts, { + 'assembly-StageLevel1-StageLevel2': { + type: 'cdk:cloud-assembly', + properties: { + directoryName: 'assembly-StageLevel1-StageLevel2', + displayName: 'StageLevel1/StageLevel2', + }, + }, + }); + + const assemblyLevel2 = embeddedAsm(assemblyLevel1, 'assembly-StageLevel1-StageLevel2'); + test.deepEqual(assemblyLevel2.manifest.artifacts, { + 'assembly-StageLevel1-StageLevel2-StageLevel3': { + type: 'cdk:cloud-assembly', + properties: { + directoryName: 'assembly-StageLevel1-StageLevel2-StageLevel3', + displayName: 'StageLevel1/StageLevel2/StageLevel3', + }, + }, + }); + + test.done(); + }, + + 'assembly/stage name validation'(test: Test) { + const app = new App(); + + new Stage(app, 'abcd'); + new Stage(app, 'abcd123'); + new Stage(app, 'abcd123-588dfjjk'); + new Stage(app, 'abcd123-588dfjjk.sss'); + new Stage(app, 'abcd123-588dfjjk.sss_ajsid'); + + test.throws(() => new Stage(app, 'abcd123-588dfjjk.sss_ajsid '), /invalid assembly name "abcd123-588dfjjk.sss_ajsid "/); + test.throws(() => new Stage(app, 'abcd123-588dfjjk.sss_ajsid/dfo'), /invalid assembly name "abcd123-588dfjjk.sss_ajsid\/dfo"/); + test.throws(() => new Stage(app, '&'), /invalid assembly name "&"/); + test.throws(() => new Stage(app, '45hello'), /invalid assembly name "45hello"/); + test.throws(() => new Stage(app, 'f'), /invalid assembly name "f"/); + + test.done(); + }, }; class TouchingAspect implements IAspect { @@ -199,27 +260,6 @@ class TouchingAspect implements IAspect { } } -/** - * rm -rf reimplementation, don't want to depend on an NPM package for this - */ -function rimraf(fsPath: string) { - try { - const isDir = fs.lstatSync(fsPath).isDirectory(); - - if (isDir) { - for (const file of fs.readdirSync(fsPath)) { - rimraf(path.join(fsPath, file)); - } - fs.rmdirSync(fsPath); - } else { - fs.unlinkSync(fsPath); - } - } catch (e) { - // We will survive ENOENT - if (e.code !== 'ENOENT') { throw e; } - } -} - class BogusStack extends Stack { constructor(scope: Construct, id: string) { super(scope, id); From 94cc82775eaff2d36591eef5a850176b8f9b0035 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 13:16:34 +0300 Subject: [PATCH 18/27] rename "embedded" to "nested" and move outdir resolution to app --- .../lib/artifact-schema.ts | 10 +-- .../cloud-assembly-schema/lib/schema.ts | 4 +- packages/@aws-cdk/core/lib/app.ts | 2 +- packages/@aws-cdk/core/lib/assembly.ts | 7 +- .../@aws-cdk/core/lib/private/synthesis.ts | 4 +- packages/@aws-cdk/core/test/test.stage.ts | 25 ++----- .../cx-api/design/NESTED_ASSEMBLIES.md | 8 +- ...t.ts => nested-cloud-assembly-artifact.ts} | 18 ++--- .../@aws-cdk/cx-api/lib/cloud-artifact.ts | 6 +- .../@aws-cdk/cx-api/lib/cloud-assembly.ts | 75 +++++++++++-------- packages/@aws-cdk/cx-api/lib/index.ts | 2 +- .../test/cloud-assembly-builder.test.ts | 14 ++-- 12 files changed, 87 insertions(+), 88 deletions(-) rename packages/@aws-cdk/cx-api/lib/artifacts/{embedded-cloud-assembly-artifact.ts => nested-cloud-assembly-artifact.ts} (70%) diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts index 01acbc37fb8d6..dd1337d6d5e52 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/artifact-schema.ts @@ -85,16 +85,16 @@ export interface TreeArtifactProperties { } /** - * Artifact properties for embedded Cloud Assemblies + * Artifact properties for nested cloud assemblies */ -export interface EmbeddedCloudAssemblyProperties { +export interface NestedCloudAssemblyProperties { /** - * Relative path to the embedded Cloud Assembly + * Relative path to the nested cloud assembly */ readonly directoryName: string; /** - * Display name for the Cloud Assembly + * Display name for the cloud assembly * * @default - The artifact ID */ @@ -107,4 +107,4 @@ export interface EmbeddedCloudAssemblyProperties { export type ArtifactProperties = AwsCloudFormationStackProperties | AssetManifestProperties | TreeArtifactProperties -| EmbeddedCloudAssemblyProperties; \ No newline at end of file +| NestedCloudAssemblyProperties; \ No newline at end of file diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts index 2874ae948d6c3..1d351364e019d 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts +++ b/packages/@aws-cdk/cloud-assembly-schema/lib/schema.ts @@ -27,9 +27,9 @@ export enum ArtifactType { ASSET_MANIFEST = 'cdk:asset-manifest', /** - * Embedded Cloud Assembly + * Nested Cloud Assembly */ - EMBEDDED_CLOUD_ASSEMBLY = 'cdk:cloud-assembly', + NESTED_CLOUD_ASSEMBLY = 'cdk:cloud-assembly', } /** diff --git a/packages/@aws-cdk/core/lib/app.ts b/packages/@aws-cdk/core/lib/app.ts index 8da26cefd28b9..8a3888ce55315 100644 --- a/packages/@aws-cdk/core/lib/app.ts +++ b/packages/@aws-cdk/core/lib/app.ts @@ -90,7 +90,7 @@ export class App extends Assembly { */ constructor(props: AppProps = {}) { super(undefined as any, '', { - outdir: props.outdir, + outdir: props.outdir ?? process.env[cxapi.OUTDIR_ENV], }); Object.defineProperty(this, APP_SYMBOL, { value: true }); diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts index 8d3bd227cfdfe..dd002591372b2 100644 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -138,14 +138,15 @@ export class Assembly extends Construct { // to write sub-assemblies (which must happen before we actually get to this app's // synthesize() phase). this.assemblyBuilder = this.parentAssembly - ? this.parentAssembly.assemblyBuilder.openEmbeddedAssembly(this.assemblyArtifactId) - : new cxapi.CloudAssemblyBuilder(props.outdir ?? process.env[cxapi.OUTDIR_ENV]); // TODO: << should; come; from; app; + ? this.parentAssembly.assemblyBuilder.createNestedAssembly(this.assemblyArtifactId) + : new cxapi.CloudAssemblyBuilder(props.outdir); this.assemblyName = [ this.parentAssembly?.assemblyName, id ].filter(x => x).join('-'); } /** - * Artifact ID of embedded assembly + * Artifact ID of the assembly if it is a nested assembly. The root assembly + * (app) will return an empty string. * * Derived from the construct path. * diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index 7a790ff50a94f..29dc16842b5a2 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -133,11 +133,11 @@ function addToParentAssembly(assembly: Assembly) { if (!assembly.parentAssembly) { return; } assembly.parentAssembly.assemblyBuilder.addArtifact(assembly.assemblyArtifactId, { - type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + type: cxschema.ArtifactType.NESTED_CLOUD_ASSEMBLY, properties: { directoryName: assembly.assemblyArtifactId, displayName: assembly.node.path, - } as cxschema.EmbeddedCloudAssemblyProperties, + } as cxschema.NestedCloudAssemblyProperties, }); } diff --git a/packages/@aws-cdk/core/test/test.stage.ts b/packages/@aws-cdk/core/test/test.stage.ts index eb61087d6587b..137f8b2154a82 100644 --- a/packages/@aws-cdk/core/test/test.stage.ts +++ b/packages/@aws-cdk/core/test/test.stage.ts @@ -27,10 +27,10 @@ export = { const stage = new Stage(app, 'Stage'); new BogusStack(stage, 'Stack2'); - // THEN -- app manifest contains an embedded cloud assembly + // THEN -- app manifest contains a nested cloud assembly const appAsm = app.synth(); - const artifact = appAsm.artifacts.find(isEmbeddedCloudAssemblyArtifact); + const artifact = appAsm.artifacts.find(x => x instanceof cxapi.NestedCloudAssemblyArtifact); test.ok(artifact); test.done(); @@ -62,8 +62,8 @@ export = { // WHEN const appAsm = app.synth(); - const outerAsm = embeddedAsm(appAsm, outer.assemblyArtifactId); - const innerAsm = embeddedAsm(outerAsm, inner.assemblyArtifactId); + const outerAsm = appAsm.getNestedAssembly(outer.assemblyArtifactId); + const innerAsm = outerAsm.getNestedAssembly(inner.assemblyArtifactId); test.ok(innerAsm.tryGetArtifact(stack.artifactId)); @@ -83,7 +83,7 @@ export = { test.done(); }, - 'Can not have dependencies to stacks outside the embedded asm'(test: Test) { + 'Can not have dependencies to stacks outside the nested asm'(test: Test) { // GIVEN const app = new App(); const stack1 = new BogusStack(app, 'Stack1'); @@ -209,7 +209,7 @@ export = { }, }); - const assemblyLevel1 = embeddedAsm(rootAssembly, 'assembly-StageLevel1'); + const assemblyLevel1 = rootAssembly.getNestedAssembly('assembly-StageLevel1'); test.deepEqual(assemblyLevel1.manifest.artifacts, { 'assembly-StageLevel1-StageLevel2': { type: 'cdk:cloud-assembly', @@ -220,7 +220,7 @@ export = { }, }); - const assemblyLevel2 = embeddedAsm(assemblyLevel1, 'assembly-StageLevel1-StageLevel2'); + const assemblyLevel2 = assemblyLevel1.getNestedAssembly('assembly-StageLevel1-StageLevel2'); test.deepEqual(assemblyLevel2.manifest.artifacts, { 'assembly-StageLevel1-StageLevel2-StageLevel3': { type: 'cdk:cloud-assembly', @@ -269,14 +269,3 @@ class BogusStack extends Stack { }); } } - -function embeddedAsm(asm: cxapi.CloudAssembly, artifactId: string): cxapi.CloudAssembly { - const a = asm.tryGetArtifact(artifactId); - if (!a) { throw new Error(`No such artifact in CloudAssembly: '${artifactId}' (have: ${asm.artifacts.map(art => art.id)})`); } - if (!isEmbeddedCloudAssemblyArtifact(a)) { throw new Error(`Found artifact '${artifactId}' but it's not a Cloud Assembly!`); } - return a.embeddedAssembly; -} - -function isEmbeddedCloudAssemblyArtifact(a: any): a is cxapi.EmbeddedCloudAssemblyArtifact { - return a instanceof cxapi.EmbeddedCloudAssemblyArtifact; -} \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md b/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md index 7ee50453c6fbb..c58caf6ee938b 100644 --- a/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md +++ b/packages/@aws-cdk/cx-api/design/NESTED_ASSEMBLIES.md @@ -1,6 +1,6 @@ -# Embedded Assemblies +# Nested Assemblies -For the CI/CD project we need to be able to a final, authoratative, immutable +For the CI/CD project we need to be able to a final, authoritative, immutable rendition of part of the construct tree. This is a part of the application that we can ask the CI/CD system to deploy as a unit, and have it get a fighting chance of getting it right. This is because: @@ -8,11 +8,11 @@ chance of getting it right. This is because: - The stacks will be known. - Their interdependencies will be known, and won't change anymore. -To that end, we're introducing the concept of an "embedded cloud assembly". +To that end, we're introducing the concept of an "nested cloud assembly". This is a part of the construct tree that is finalized independently of the rest, so that other constructs can reflect on it. -Constructs of type `Stage` will produce embedded cloud assemblies. +Constructs of type `Stage` will produce nested cloud assemblies. ## Restrictions diff --git a/packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/nested-cloud-assembly-artifact.ts similarity index 70% rename from packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts rename to packages/@aws-cdk/cx-api/lib/artifacts/nested-cloud-assembly-artifact.ts index d25b7639e3934..bf3e378774d96 100644 --- a/packages/@aws-cdk/cx-api/lib/artifacts/embedded-cloud-assembly-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/artifacts/nested-cloud-assembly-artifact.ts @@ -6,7 +6,7 @@ import { CloudAssembly } from '../cloud-assembly'; /** * Asset manifest is a description of a set of assets which need to be built and published */ -export class EmbeddedCloudAssemblyArtifact extends CloudArtifact { +export class NestedCloudAssemblyArtifact extends CloudArtifact { /** * The relative directory name of the asset manifest */ @@ -20,30 +20,30 @@ export class EmbeddedCloudAssemblyArtifact extends CloudArtifact { /** * Cache for the inner assembly loading */ - private _embeddedAssembly?: CloudAssembly; + private _nestedAssembly?: CloudAssembly; constructor(assembly: CloudAssembly, name: string, artifact: cxschema.ArtifactManifest) { super(assembly, name, artifact); - const properties = (this.manifest.properties || {}) as cxschema.EmbeddedCloudAssemblyProperties; + const properties = (this.manifest.properties || {}) as cxschema.NestedCloudAssemblyProperties; this.directoryName = properties.directoryName; this.displayName = properties.displayName ?? name; } /** - * Full path to the Embedded Assembly + * Full path to the nested assembly directory */ public get fullPath(): string { return path.join(this.assembly.directory, this.directoryName); } /** - * The embedded Assembly + * The nested Assembly */ - public get embeddedAssembly(): CloudAssembly { - if (!this._embeddedAssembly) { - this._embeddedAssembly = new CloudAssembly(this.fullPath); + public get nestedAssembly(): CloudAssembly { + if (!this._nestedAssembly) { + this._nestedAssembly = new CloudAssembly(this.fullPath); } - return this._embeddedAssembly; + return this._nestedAssembly; } } diff --git a/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts b/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts index 6b586c4206345..9abfdb8d660d5 100644 --- a/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts +++ b/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts @@ -49,8 +49,8 @@ export class CloudArtifact { return new TreeCloudArtifact(assembly, id, artifact); case cxschema.ArtifactType.ASSET_MANIFEST: return new AssetManifestArtifact(assembly, id, artifact); - case cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY: - return new EmbeddedCloudAssemblyArtifact(assembly, id, artifact); + case cxschema.ArtifactType.NESTED_CLOUD_ASSEMBLY: + return new NestedCloudAssemblyArtifact(assembly, id, artifact); default: return undefined; } @@ -147,5 +147,5 @@ export class CloudArtifact { // needs to be defined at the end to avoid a cyclic dependency import { AssetManifestArtifact } from './artifacts/asset-manifest-artifact'; import { CloudFormationStackArtifact } from './artifacts/cloudformation-artifact'; -import { EmbeddedCloudAssemblyArtifact } from './artifacts/embedded-cloud-assembly-artifact'; +import { NestedCloudAssemblyArtifact } from './artifacts/nested-cloud-assembly-artifact'; import { TreeCloudArtifact } from './artifacts/tree-cloud-artifact'; \ No newline at end of file diff --git a/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts b/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts index 04b28710626f5..77a3cb2c8fcdc 100644 --- a/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts +++ b/packages/@aws-cdk/cx-api/lib/cloud-assembly.ts @@ -3,6 +3,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { CloudFormationStackArtifact } from './artifacts/cloudformation-artifact'; +import { NestedCloudAssemblyArtifact } from './artifacts/nested-cloud-assembly-artifact'; import { TreeCloudArtifact } from './artifacts/tree-cloud-artifact'; import { CloudArtifact } from './cloud-artifact'; import { topologicalSort } from './toposort'; @@ -118,6 +119,33 @@ export class CloudAssembly { return artifact; } + /** + * Returns a nested assembly artifact. + * + * @param artifactId The artifact ID of the nested assembly + */ + public getNestedAssemblyArtifact(artifactId: string): NestedCloudAssemblyArtifact { + const artifact = this.tryGetArtifact(artifactId); + if (!artifact) { + throw new Error(`Unable to find artifact with id "${artifactId}"`); + } + + if (!(artifact instanceof NestedCloudAssemblyArtifact)) { + throw new Error(`Found artifact '${artifactId}' but it's not a nested cloud assembly`); + } + + return artifact; + } + + /** + * Returns a nested assembly. + * + * @param artifactId The artifact ID of the nested assembly + */ + public getNestedAssembly(artifactId: string): CloudAssembly { + return this.getNestedAssemblyArtifact(artifactId).nestedAssembly; + } + /** * Returns the tree metadata artifact from this assembly. * @throws if there is no metadata artifact by that name @@ -175,13 +203,6 @@ export class CloudAssembly { * Can be used to build a cloud assembly. */ export class CloudAssemblyBuilder { - /** - * Turn the given optional output directory into a fixed output directory - */ - public static determineOutputDirectory(outdir?: string) { - return outdir ?? fs.mkdtempSync(path.join(os.tmpdir(), 'cdk.out')); - } - /** * The root directory of the resulting cloud assembly. */ @@ -195,7 +216,7 @@ export class CloudAssemblyBuilder { * @param outdir The output directory, uses temporary directory if undefined */ constructor(outdir?: string) { - this.outdir = CloudAssemblyBuilder.determineOutputDirectory(outdir); + this.outdir = determineOutputDirectory(outdir); // we leverage the fact that outdir is long-lived to avoid staging assets into it // that were already staged (copying can be expensive). this is achieved by the fact @@ -261,42 +282,23 @@ export class CloudAssemblyBuilder { } /** - * Begin an embedded assembly in this assembly - * - * If dirName is not given, it will default to artifactId. + * Creates a nested cloud assembly */ - public openEmbeddedAssembly(artifactId: string, dirName?: string) { - const directoryName = dirName ?? artifactId; + public createNestedAssembly(artifactId: string) { + const directoryName = artifactId; const innerAsmDir = path.join(this.outdir, directoryName); // WHEN this.addArtifact(artifactId, { - type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + type: cxschema.ArtifactType.NESTED_CLOUD_ASSEMBLY, properties: { directoryName, - } as cxschema.EmbeddedCloudAssemblyProperties, + } as cxschema.NestedCloudAssemblyProperties, }); return new CloudAssemblyBuilder(innerAsmDir); } } -/** - * Options for opening an embedded assembly - */ -export interface EmbeddedAssemblyOptions { - /** - * Identifier for this assembly - */ - readonly id: string; - - /** - * Display name for this assembly - * - * @default - Artifact ID is used as display name - */ - readonly displayName?: string; -} - /** * Backwards compatibility for when `RuntimeInfo` * was defined here. This is necessary because its used as an input in the stable @@ -384,4 +386,11 @@ function filterUndefined(obj: any): any { function ignore(_x: any) { return; -} \ No newline at end of file +} + +/** + * Turn the given optional output directory into a fixed output directory + */ +function determineOutputDirectory(outdir?: string) { + return outdir ?? fs.mkdtempSync(path.join(os.tmpdir(), 'cdk.out')); +} diff --git a/packages/@aws-cdk/cx-api/lib/index.ts b/packages/@aws-cdk/cx-api/lib/index.ts index d73148b92d663..a6ac4977a6d17 100644 --- a/packages/@aws-cdk/cx-api/lib/index.ts +++ b/packages/@aws-cdk/cx-api/lib/index.ts @@ -7,7 +7,7 @@ export * from './cloud-artifact'; export * from './artifacts/asset-manifest-artifact'; export * from './artifacts/cloudformation-artifact'; export * from './artifacts/tree-cloud-artifact'; -export * from './artifacts/embedded-cloud-assembly-artifact'; +export * from './artifacts/nested-cloud-assembly-artifact'; export * from './cloud-assembly'; export * from './assets'; export * from './environment'; diff --git a/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts b/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts index c7c939924d924..1512c86ff5044 100644 --- a/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts +++ b/packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts @@ -142,7 +142,7 @@ test('duplicate missing values with the same key are only reported once', () => expect(assembly.manifest.missing!.length).toEqual(1); }); -test('write and read embedded Cloud Assembly artifact', () => { +test('write and read nested cloud assembly artifact', () => { // GIVEN const outdir = fs.mkdtempSync(path.join(os.tmpdir(), 'cloud-assembly-builder-tests')); const session = new cxapi.CloudAssemblyBuilder(outdir); @@ -152,18 +152,18 @@ test('write and read embedded Cloud Assembly artifact', () => { // WHEN session.addArtifact('Assembly', { - type: cxschema.ArtifactType.EMBEDDED_CLOUD_ASSEMBLY, + type: cxschema.ArtifactType.NESTED_CLOUD_ASSEMBLY, properties: { directoryName: 'hello', - } as cxschema.EmbeddedCloudAssemblyProperties, + } as cxschema.NestedCloudAssemblyProperties, }); const asm = session.buildAssembly(); // THEN - const art = asm.tryGetArtifact('Assembly') as cxapi.EmbeddedCloudAssemblyArtifact | undefined; - expect(art).toBeInstanceOf(cxapi.EmbeddedCloudAssemblyArtifact); + const art = asm.tryGetArtifact('Assembly') as cxapi.NestedCloudAssemblyArtifact | undefined; + expect(art).toBeInstanceOf(cxapi.NestedCloudAssemblyArtifact); expect(art?.fullPath).toEqual(path.join(outdir, 'hello')); - const embeddedAsm = art?.embeddedAssembly; - expect(embeddedAsm?.artifacts.length).toEqual(0); + const nested = art?.nestedAssembly; + expect(nested?.artifacts.length).toEqual(0); }); \ No newline at end of file From 711d86113f5b0ef7d4115860950d5359c57cad75 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 13:43:24 +0300 Subject: [PATCH 19/27] hoist runtimeInfo collection to assembly --- packages/@aws-cdk/core/lib/assembly.ts | 8 +++++++- packages/@aws-cdk/core/lib/construct-compat.ts | 4 ++++ packages/@aws-cdk/core/lib/private/synthesis.ts | 10 +++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts index dd002591372b2..d659489989563 100644 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -1,6 +1,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import { Construct, IConstruct } from './construct-compat'; import { Environment } from './environment'; +import { collectRuntimeInformation } from './private/runtime-info'; import { synthesize } from './private/synthesis'; /** @@ -163,8 +164,13 @@ export class Assembly extends Construct { * Once an assembly has been synthesized, it cannot be modified. Subsequent calls will return the same assembly. */ public synth(options: AssemblySynthesisOptions = { }): cxapi.CloudAssembly { + const runtimeInfo = this.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); + if (!this.assembly) { - this.assembly = synthesize(this, options); + this.assembly = synthesize(this, { + skipValidation: options.skipValidation, + runtimeInfo, + }); } return this.assembly; diff --git a/packages/@aws-cdk/core/lib/construct-compat.ts b/packages/@aws-cdk/core/lib/construct-compat.ts index 89755d75de30e..86977dee57256 100644 --- a/packages/@aws-cdk/core/lib/construct-compat.ts +++ b/packages/@aws-cdk/core/lib/construct-compat.ts @@ -182,6 +182,8 @@ export enum ConstructOrder { /** * Options for synthesis. + * + * @deprecated use `app.synth()` or `stage.synth()` instead */ export interface SynthesisOptions extends cxapi.AssemblyBuildOptions { /** @@ -224,6 +226,7 @@ export class ConstructNode { * Synthesizes a CloudAssembly from a construct tree. * @param node The root of the construct tree. * @param options Synthesis options. + * @deprecated Use `app.synth()` or `stage.synth()` instead */ public static synth(node: ConstructNode, options: SynthesisOptions = { }): cxapi.CloudAssembly { // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -234,6 +237,7 @@ export class ConstructNode { /** * Invokes "prepare" on all constructs (depth-first, post-order) in the tree under `node`. * @param node The root node + * @deprecated Use `app.synth()` instead */ public static prepare(node: ConstructNode) { return node._actualNode.prepare(); diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index 29dc16842b5a2..b51748210608b 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -1,10 +1,9 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import * as constructs from 'constructs'; -import { Assembly } from '../assembly'; +import { Assembly, AssemblySynthesisOptions } from '../assembly'; import { Construct, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat'; import { prepareApp } from './prepare-app'; -import { collectRuntimeInformation } from './runtime-info'; export function synthesize(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly { // we start by calling "synth" on all nested assemblies (which will take care of all their children) @@ -38,8 +37,9 @@ export function synthesize(root: IConstruct, options: SynthesisOptions = { }): c addToParentAssembly(root); } - const runtimeInfo = root.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); - return builder.buildAssembly({ runtimeInfo }); + return builder.buildAssembly({ + runtimeInfo: options.runtimeInfo, + }); } /** @@ -47,7 +47,7 @@ export function synthesize(root: IConstruct, options: SynthesisOptions = { }): c * * (They will in turn recurse again) */ -function synthNestedAssemblies(root: IConstruct, options: SynthesisOptions) { +function synthNestedAssemblies(root: IConstruct, options: AssemblySynthesisOptions) { for (const child of root.node.children) { if (Assembly.isAssembly(child)) { child.synth(options); From 2bb8e62c7c6aa1715249f2661a1524d45ad92b7a Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 13:47:35 +0300 Subject: [PATCH 20/27] update cx schema --- .../schema/cloud-assembly.schema.json | 10 +++++----- .../schema/cloud-assembly.version.json | 2 +- .../cloud-assembly-schema/scripts/update-schema.sh | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json index 5652709b8d5ab..8c3e58485b12b 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json @@ -74,7 +74,7 @@ "$ref": "#/definitions/TreeArtifactProperties" }, { - "$ref": "#/definitions/EmbeddedCloudAssemblyProperties" + "$ref": "#/definitions/NestedCloudAssemblyProperties" } ] } @@ -335,16 +335,16 @@ "file" ] }, - "EmbeddedCloudAssemblyProperties": { - "description": "Artifact properties for embedded Cloud Assemblies", + "NestedCloudAssemblyProperties": { + "description": "Artifact properties for nested cloud assemblies", "type": "object", "properties": { "directoryName": { - "description": "Relative of the embedded Cloud Assembly's directory", + "description": "Relative path to the nested cloud assembly", "type": "string" }, "displayName": { - "description": "Display name for the Cloud Assembly (Default - The artifact ID)", + "description": "Display name for the cloud assembly (Default - The artifact ID)", "type": "string" } }, diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json index 78d33700c0698..42cb403235c06 100644 --- a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json +++ b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json @@ -1 +1 @@ -{"version":"5.0.0"} +{"version":"6.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/cloud-assembly-schema/scripts/update-schema.sh b/packages/@aws-cdk/cloud-assembly-schema/scripts/update-schema.sh index cde2aafa37aad..424e104e1dc85 100755 --- a/packages/@aws-cdk/cloud-assembly-schema/scripts/update-schema.sh +++ b/packages/@aws-cdk/cloud-assembly-schema/scripts/update-schema.sh @@ -1,7 +1,7 @@ #!/bin/bash set -euo pipefail scriptsdir=$(cd $(dirname $0) && pwd) -packagedir=$(realpath ${scriptsdir}/..) +packagedir=$(cd ${scriptsdir}/.. && pwd) # Output OUTPUT_DIR="${packagedir}/schema" From 7715885d1bff42dbb7edd81c4186772b22368008 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 14:37:43 +0300 Subject: [PATCH 21/27] remove "experimental" annotation from Assembly --- packages/@aws-cdk/core/lib/assembly.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts index d659489989563..28d85b02eb505 100644 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ b/packages/@aws-cdk/core/lib/assembly.ts @@ -59,8 +59,6 @@ export interface AssemblyProps { * You can then instantiate your subclass multiple times to model multiple * copies of your application which should be be deployed to different * environments. - * - * @experimental */ export class Assembly extends Construct { /** From 603c608bbfa3f4fcd9b8510a2fea5489441c3e52 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 7 Jun 2020 15:32:44 +0300 Subject: [PATCH 22/27] chore(cli): fix "iam diff" integration test (#8421) The PR #8403 changed the "IAM stack" to use the default environment and forgot to update the expected output (which now does not contain a token for the URL suffix). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/test/integ/cli/cli.integtest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/test/integ/cli/cli.integtest.ts b/packages/aws-cdk/test/integ/cli/cli.integtest.ts index 4c9896236155a..b31bbf314d60a 100644 --- a/packages/aws-cdk/test/integ/cli/cli.integtest.ts +++ b/packages/aws-cdk/test/integ/cli/cli.integtest.ts @@ -441,12 +441,12 @@ integTest('IAM diff', async () => { // ┌───┬─────────────────┬────────┬────────────────┬────────────────────────────-──┬───────────┐ // │ │ Resource │ Effect │ Action │ Principal │ Condition │ // ├───┼─────────────────┼────────┼────────────────┼───────────────────────────────┼───────────┤ - // │ + │ ${SomeRole.Arn} │ Allow │ sts:AssumeRole │ Service:ec2.${AWS::URLSuffix} │ │ + // │ + │ ${SomeRole.Arn} │ Allow │ sts:AssumeRole │ Service:ec2.amazonaws.com │ │ // └───┴─────────────────┴────────┴────────────────┴───────────────────────────────┴───────────┘ expect(output).toContain('${SomeRole.Arn}'); expect(output).toContain('sts:AssumeRole'); - expect(output).toContain('ec2.${AWS::URLSuffix}'); + expect(output).toContain('ec2.amazonaws.com'); }); integTest('fast deploy', async () => { From bb15b99d91f2ab482e31501c89d008358af50f76 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Sun, 7 Jun 2020 18:43:22 +0200 Subject: [PATCH 23/27] Addressing PR comments --- packages/@aws-cdk/aws-cognito/README.md | 6 +- .../aws-cognito/lib/user-pool-attr.ts | 2 +- .../@aws-cdk/aws-cognito/lib/user-pool.ts | 131 +++++++----------- .../aws-cognito/test/user-pool.test.ts | 41 ++++++ 4 files changed, 97 insertions(+), 83 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/README.md b/packages/@aws-cdk/aws-cognito/README.md index 01c224fc7c8a3..0a43f8b72edfe 100644 --- a/packages/@aws-cdk/aws-cognito/README.md +++ b/packages/@aws-cdk/aws-cognito/README.md @@ -161,8 +161,8 @@ attributes. Besides these, additional attributes can be further defined, and are Learn more on [attributes in Cognito's documentation](https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html). -The following code sample configures a user pool with two standard attributes (name and address) as required, and adds -four optional attributes. +The following code configures a user pool with two standard attributes (name and address) as required and mutable, and adds +four custom attributes. ```ts new UserPool(this, 'myuserpool', { @@ -480,4 +480,4 @@ const domain = userpool.addDomain('Domain', { const signInUrl = domain.signInUrl(client, { redirectUrl: 'https://myapp.com/home', // must be a URL configured under 'callbackUrls' with the client }) -``` \ No newline at end of file +``` diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index e3f28b542b2f7..e7a8ab8f8f77b 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -115,7 +115,7 @@ export interface StandardAttributes { export interface StandardAttribute { /** * Specifies whether the value of the attribute can be changed. - * For any user pool attribute that's mapped to an identity provider attribute, you must set this parameter to true. + * For any user pool attribute that's mapped to an identity provider attribute, this must be set to `true`. * Amazon Cognito updates mapped attributes when users sign in to your application through an identity provider. * If an attribute is immutable, Amazon Cognito throws an error when it attempts to update the attribute. * diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts index febdef19b0adc..1f1cdb1e81f69 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool.ts @@ -456,7 +456,7 @@ export interface UserPoolProps { * The set of attributes that are required for every user in the user pool. * Read more on attributes here - https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-attributes.html * - * @default - No standard attributes are required. + * @default - All standard attributes are optional and mutable. */ readonly standardAttributes?: StandardAttributes; @@ -746,24 +746,24 @@ export class UserPool extends UserPoolBase { if (signIn.username) { aliasAttrs = []; - if (signIn.email) { aliasAttrs.push(StandardAttributeNames.EMAIL); } - if (signIn.phone) { aliasAttrs.push(StandardAttributeNames.PHONE_NUMBER); } - if (signIn.preferredUsername) { aliasAttrs.push(StandardAttributeNames.PREFERRED_USERNAME); } + if (signIn.email) { aliasAttrs.push(StandardAttributeNames.email); } + if (signIn.phone) { aliasAttrs.push(StandardAttributeNames.phoneNumber); } + if (signIn.preferredUsername) { aliasAttrs.push(StandardAttributeNames.preferredUsername); } if (aliasAttrs.length === 0) { aliasAttrs = undefined; } } else { usernameAttrs = []; - if (signIn.email) { usernameAttrs.push(StandardAttributeNames.EMAIL); } - if (signIn.phone) { usernameAttrs.push(StandardAttributeNames.PHONE_NUMBER); } + if (signIn.email) { usernameAttrs.push(StandardAttributeNames.email); } + if (signIn.phone) { usernameAttrs.push(StandardAttributeNames.phoneNumber); } } if (props.autoVerify) { autoVerifyAttrs = []; - if (props.autoVerify.email) { autoVerifyAttrs.push(StandardAttributeNames.EMAIL); } - if (props.autoVerify.phone) { autoVerifyAttrs.push(StandardAttributeNames.PHONE_NUMBER); } + if (props.autoVerify.email) { autoVerifyAttrs.push(StandardAttributeNames.email); } + if (props.autoVerify.phone) { autoVerifyAttrs.push(StandardAttributeNames.phoneNumber); } } else if (signIn.email || signIn.phone) { autoVerifyAttrs = []; - if (signIn.email) { autoVerifyAttrs.push(StandardAttributeNames.EMAIL); } - if (signIn.phone) { autoVerifyAttrs.push(StandardAttributeNames.PHONE_NUMBER); } + if (signIn.email) { autoVerifyAttrs.push(StandardAttributeNames.email); } + if (signIn.phone) { autoVerifyAttrs.push(StandardAttributeNames.phoneNumber); } } return { usernameAttrs, aliasAttrs, autoVerifyAttrs }; @@ -851,7 +851,7 @@ export class UserPool extends UserPoolBase { const stdAttributes = (Object.entries(props.standardAttributes) as Array<[keyof StandardAttributes, StandardAttribute]>) .filter(([, attr]) => !!attr) .map(([attrName, attr]) => ({ - name: StandardAttributeMap[attrName], + name: StandardAttributeNames[attrName], mutable: attr.mutable ?? true, required: attr.required ?? false, })); @@ -860,9 +860,29 @@ export class UserPool extends UserPoolBase { } if (props.customAttributes) { - const customAttrs = Object.keys(props.customAttributes).map(attrName => - this.renderAttribute(attrName, props.customAttributes![attrName]), - ); + const customAttrs = Object.keys(props.customAttributes).map((attrName) => { + const attrConfig = props.customAttributes![attrName].bind(); + const numberConstraints: CfnUserPool.NumberAttributeConstraintsProperty = { + minValue: attrConfig.numberConstraints?.min?.toString(), + maxValue: attrConfig.numberConstraints?.max?.toString(), + }; + const stringConstraints: CfnUserPool.StringAttributeConstraintsProperty = { + minLength: attrConfig.stringConstraints?.minLen?.toString(), + maxLength: attrConfig.stringConstraints?.maxLen?.toString(), + }; + + return { + name: attrName, + attributeDataType: attrConfig.dataType, + numberAttributeConstraints: attrConfig.numberConstraints + ? numberConstraints + : undefined, + stringAttributeConstraints: attrConfig.stringConstraints + ? stringConstraints + : undefined, + mutable: attrConfig.mutable, + }; + }); schema.push(...customAttrs); } @@ -871,73 +891,26 @@ export class UserPool extends UserPoolBase { } return schema; } - - private renderAttribute( - name: string, - attribute: ICustomAttribute, - ): CfnUserPool.SchemaAttributeProperty { - const attrConfig = attribute.bind(); - const numberConstraints: CfnUserPool.NumberAttributeConstraintsProperty = { - minValue: attrConfig.numberConstraints?.min?.toString(), - maxValue: attrConfig.numberConstraints?.max?.toString(), - }; - const stringConstraints: CfnUserPool.StringAttributeConstraintsProperty = { - minLength: attrConfig.stringConstraints?.minLen?.toString(), - maxLength: attrConfig.stringConstraints?.maxLen?.toString(), - }; - - return { - name, - attributeDataType: attrConfig.dataType, - numberAttributeConstraints: attrConfig.numberConstraints - ? numberConstraints - : undefined, - stringAttributeConstraints: attrConfig.stringConstraints - ? stringConstraints - : undefined, - mutable: attrConfig.mutable, - }; - } -} - -const enum StandardAttributeNames { - ADDRESS = 'address', - BIRTHDATE = 'birthdate', - EMAIL = 'email', - FAMILY_NAME = 'family_name', - GENDER = 'gender', - GIVEN_NAME = 'given_name', - LOCALE = 'locale', - MIDDLE_NAME = 'middle_name', - NAME = 'name', - NICKNAME = 'nickname', - PHONE_NUMBER = 'phone_number', - PICTURE_URL = 'picture', - PREFERRED_USERNAME = 'preferred_username', - PROFILE_URL = 'profile', - TIMEZONE = 'zoneinfo', - LAST_UPDATE_TIME = 'updated_at', - WEBSITE = 'website', } -const StandardAttributeMap: Record = { - address: StandardAttributeNames.ADDRESS, - birthdate: StandardAttributeNames.BIRTHDATE, - email: StandardAttributeNames.EMAIL, - familyName: StandardAttributeNames.FAMILY_NAME, - gender: StandardAttributeNames.GENDER, - givenName: StandardAttributeNames.GIVEN_NAME, - locale: StandardAttributeNames.LOCALE, - middleName: StandardAttributeNames.MIDDLE_NAME, - fullname: StandardAttributeNames.NAME, - nickname: StandardAttributeNames.NICKNAME, - phoneNumber: StandardAttributeNames.PHONE_NUMBER, - profilePicture: StandardAttributeNames.PICTURE_URL, - preferredUsername: StandardAttributeNames.PREFERRED_USERNAME, - profilePage: StandardAttributeNames.PROFILE_URL, - timezone: StandardAttributeNames.TIMEZONE, - lastUpdateTime: StandardAttributeNames.LAST_UPDATE_TIME, - website: StandardAttributeNames.WEBSITE, +const StandardAttributeNames: Record = { + address: 'address', + birthdate: 'birthdate', + email: 'email', + familyName: 'family_name', + gender: 'gender', + givenName: 'given_name', + locale: 'locale', + middleName: 'middle_name', + fullname: 'name', + nickname: 'nickname', + phoneNumber: 'phone_number', + profilePicture: 'picture', + preferredUsername: 'preferred_username', + profilePage: 'profile', + timezone: 'zoneinfo', + lastUpdateTime: 'updated_at', + website: 'website', }; function undefinedIfNoKeys(struct: object): object | undefined { diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts index 004b56b0fabc0..84e20419a2678 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts @@ -582,6 +582,47 @@ describe('User Pool', () => { }); }); + test('schema is absent when attributes are not specified', () => { + // GIVEN + const stack = new Stack(); + + // WHEN + new UserPool(stack, 'Pool', { userPoolName: 'Pool' }); + + // THEN + expect(stack).toHaveResourceLike('AWS::Cognito::UserPool', { + UserPoolName: 'Pool', + Schema: ABSENT, + }); + }); + + test('optional mutable standardAttributes', () => { + // GIVEN + const stack = new Stack(); + + // WHEN + new UserPool(stack, 'Pool', { + userPoolName: 'Pool', + standardAttributes: { + timezone: { + mutable: true, + }, + }, + }); + + // THEN + expect(stack).toHaveResourceLike('AWS::Cognito::UserPool', { + UserPoolName: 'Pool', + Schema: [ + { + Mutable: true, + Required: false, + Name: 'zoneinfo', + } + ] + }); + }); + test('custom attributes with default constraints', () => { // GIVEN const stack = new Stack(); From 2fb20acfa0a7118a4adf6a09d7177407ade1b42c Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Sun, 7 Jun 2020 19:09:42 +0200 Subject: [PATCH 24/27] Fix linting --- packages/@aws-cdk/aws-cognito/test/user-pool.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts index 30c6b1bf82c4c..61eb7a0ed229c 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool.test.ts @@ -637,8 +637,8 @@ describe('User Pool', () => { Mutable: true, Required: false, Name: 'zoneinfo', - } - ] + }, + ], }); }); From 49ec34e16bffd82470d3728f85f7508b53b2029a Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 8 Jun 2020 13:34:59 +0300 Subject: [PATCH 25/27] revert core changes --- packages/@aws-cdk/core/lib/deps.ts | 7 ------- packages/@aws-cdk/core/lib/private/prepare-app.ts | 1 - packages/@aws-cdk/core/lib/stack.ts | 1 - packages/@aws-cdk/core/test/test.stack.ts | 6 +++--- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/core/lib/deps.ts b/packages/@aws-cdk/core/lib/deps.ts index 83da0580c3012..c34f11ef2c5a7 100644 --- a/packages/@aws-cdk/core/lib/deps.ts +++ b/packages/@aws-cdk/core/lib/deps.ts @@ -1,4 +1,3 @@ -import { Assembly } from './assembly'; import { CfnResource } from './cfn-resource'; import { Stack } from './stack'; import { Stage } from './stage'; @@ -31,13 +30,7 @@ export function addDependency(source: T, target: T, reason?: } const sourceStack = Stack.of(source); - const sourceAssembly = Assembly.of(sourceStack); const targetStack = Stack.of(target); - const targetAssembly = Assembly.of(targetStack); - - if (sourceAssembly !== targetAssembly) { - throw new Error(`You cannot add a dependency from '${source.node.path}' (in ${describeAssembly(sourceAssembly)}) to '${target.node.path}' (in ${describeAssembly(targetAssembly)}): dependency cannot cross stage boundaries`); - } const sourceStage = Stage.of(sourceStack); const targetStage = Stage.of(targetStack); diff --git a/packages/@aws-cdk/core/lib/private/prepare-app.ts b/packages/@aws-cdk/core/lib/private/prepare-app.ts index 93e0fedb33727..de5ef433fb1ad 100644 --- a/packages/@aws-cdk/core/lib/private/prepare-app.ts +++ b/packages/@aws-cdk/core/lib/private/prepare-app.ts @@ -1,5 +1,4 @@ import { ConstructOrder } from 'constructs'; -import { Assembly } from '../assembly'; import { CfnResource } from '../cfn-resource'; import { IConstruct } from '../construct-compat'; import { Stack } from '../stack'; diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index cb07dc63a4223..eeb65562837ec 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -1059,7 +1059,6 @@ function makeStackName(components: string[]) { // These imports have to be at the end to prevent circular imports import { Arn, ArnComponents } from './arn'; -import { Assembly } from './assembly'; import { CfnElement } from './cfn-element'; import { Fn } from './cfn-fn'; import { Aws, ScopedAws } from './cfn-pseudo'; diff --git a/packages/@aws-cdk/core/test/test.stack.ts b/packages/@aws-cdk/core/test/test.stack.ts index a2d00cf4efdd6..7f8e625ee4428 100644 --- a/packages/@aws-cdk/core/test/test.stack.ts +++ b/packages/@aws-cdk/core/test/test.stack.ts @@ -524,7 +524,7 @@ export = { new CfnParameter(stack1, 'SomeParameter', { type: 'String', default: account2 }); test.throws(() => { - app.synth(); + ConstructNode.prepare(app.node); // tslint:disable-next-line:max-line-length }, "'Stack2' depends on 'Stack1' (Stack2/SomeParameter -> Stack1.AWS::AccountId). Adding this dependency (Stack1/SomeParameter -> Stack2.AWS::AccountId) would create a cyclic reference."); @@ -541,7 +541,7 @@ export = { // WHEN new CfnParameter(stack2, 'SomeParameter', { type: 'String', default: account1 }); - app.synth(); + ConstructNode.prepare(app.node); // THEN test.deepEqual(stack2.dependencies.map(s => s.node.id), ['Stack1']); @@ -560,7 +560,7 @@ export = { new CfnParameter(stack2, 'SomeParameter', { type: 'String', default: account1 }); test.throws(() => { - app.synth(); + ConstructNode.prepare(app.node); }, /Stack "Stack2" cannot consume a cross reference from stack "Stack1"/); test.done(); From b305224c93737965f1f1398d134935b4cadec7d0 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 8 Jun 2020 13:35:52 +0300 Subject: [PATCH 26/27] remove extrenous file --- packages/@aws-cdk/core/lib/assembly.ts | 187 ------------------------- 1 file changed, 187 deletions(-) delete mode 100644 packages/@aws-cdk/core/lib/assembly.ts diff --git a/packages/@aws-cdk/core/lib/assembly.ts b/packages/@aws-cdk/core/lib/assembly.ts deleted file mode 100644 index 28d85b02eb505..0000000000000 --- a/packages/@aws-cdk/core/lib/assembly.ts +++ /dev/null @@ -1,187 +0,0 @@ -import * as cxapi from '@aws-cdk/cx-api'; -import { Construct, IConstruct } from './construct-compat'; -import { Environment } from './environment'; -import { collectRuntimeInformation } from './private/runtime-info'; -import { synthesize } from './private/synthesis'; - -/** - * Initialization props for an assembly. - */ -export interface AssemblyProps { - /** - * Default AWS environment (account/region) for `Stack`s in this `Stage`. - * - * Stacks defined inside this `Stage` with either `region` or `account` missing - * from its env will use the corresponding field given here. - * - * If either `region` or `account`is is not configured for `Stack` (either on - * the `Stack` itself or on the containing `Stage`), the Stack will be - * *environment-agnostic*. - * - * Environment-agnostic stacks can be deployed to any environment, may not be - * able to take advantage of all features of the CDK. For example, they will - * not be able to use environmental context lookups, will not automatically - * translate Service Principals to the right format based on the environment's - * AWS partition, and other such enhancements. - * - * @example - * - * // Use a concrete account and region to deploy this Stage to - * new MyStage(app, 'Stage1', { - * env: { account: '123456789012', region: 'us-east-1' }, - * }); - * - * // Use the CLI's current credentials to determine the target environment - * new MyStage(app, 'Stage2', { - * env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, - * }); - * - * @default - The environments should be configured on the `Stack`s. - */ - readonly env?: Environment; - - /** - * The output directory into which to emit synthesized artifacts. - * - * @default - If this value is _not_ set, considers the environment variable `CDK_OUTDIR`. - * If `CDK_OUTDIR` is not defined, uses a temp directory. - */ - readonly outdir?: string; -} - -/** - * An abstract application modeling unit consisting of Stacks that should be - * deployed together. - * - * Derive a subclass of `Stage` and use it to model a single instance of your - * application. - * - * You can then instantiate your subclass multiple times to model multiple - * copies of your application which should be be deployed to different - * environments. - */ -export class Assembly extends Construct { - /** - * Return the containing assembly of a construct, if available. If called on a - * nested assembly, returns its parent. - * - * @experimental - */ - public static of(construct: IConstruct): Assembly | undefined { - return construct.node.scopes.reverse().slice(1).find(Assembly.isAssembly); - } - - /** - * Test whether the given construct is an assembly. - * - * @experimental - */ - public static isAssembly(x: any ): x is Assembly { - return x !== null && x instanceof Assembly; - } - - /** - * The default region for all resources defined within this assembly. - * - * @experimental - */ - public readonly region?: string; - - /** - * The default account for all resources defined within this assembly. - * - * @experimental - */ - public readonly account?: string; - - /** - * The cloud assembly builder that is being used for this App - * - * @experimental - */ - public readonly assemblyBuilder: cxapi.CloudAssemblyBuilder; - - /** - * The name of the assembly. Based on names of the parent assemblies separated - * by hypens. - * - * @experimental - */ - public readonly assemblyName: string; - - /** - * The parent assembly or `undefined` if this is the app. - * * - * @experimental - */ - public readonly parentAssembly?: Assembly; - - /** - * The cached assembly if it was already built - */ - private assembly?: cxapi.CloudAssembly; - - constructor(scope: Construct, id: string, props: AssemblyProps = {}) { - super(scope, id); - - if (id !== '' && !/^[a-z][a-z0-9\-\_\.]+$/i.test(id)) { - throw new Error(`invalid assembly name "${id}". Assembly name must start with a letter and contain only alphanumeric characters, hypens ('-'), underscores ('_') and periods ('.')`); - } - - this.parentAssembly = Assembly.of(this); - - this.region = props.env?.region; - this.account = props.env?.account; - - // Need to determine fixed output directory already, because we must know where - // to write sub-assemblies (which must happen before we actually get to this app's - // synthesize() phase). - this.assemblyBuilder = this.parentAssembly - ? this.parentAssembly.assemblyBuilder.createNestedAssembly(this.assemblyArtifactId) - : new cxapi.CloudAssemblyBuilder(props.outdir); - - this.assemblyName = [ this.parentAssembly?.assemblyName, id ].filter(x => x).join('-'); - } - - /** - * Artifact ID of the assembly if it is a nested assembly. The root assembly - * (app) will return an empty string. - * - * Derived from the construct path. - * - * @experimental - */ - public get assemblyArtifactId() { - if (!this.node.path) { return ''; } - return `assembly-${this.node.path.replace(/\//g, '-').replace(/^-+|-+$/g, '')}`; - } - - /** - * Synthesize this assembly. - * - * Once an assembly has been synthesized, it cannot be modified. Subsequent calls will return the same assembly. - */ - public synth(options: AssemblySynthesisOptions = { }): cxapi.CloudAssembly { - const runtimeInfo = this.node.tryGetContext(cxapi.DISABLE_VERSION_REPORTING) ? undefined : collectRuntimeInformation(); - - if (!this.assembly) { - this.assembly = synthesize(this, { - skipValidation: options.skipValidation, - runtimeInfo, - }); - } - - return this.assembly; - } -} - -/** - * Options for assemly synthesis. - */ -export interface AssemblySynthesisOptions { - /** - * Should we skip construct validation. - * @default - false - */ - readonly skipValidation?: boolean; -} From daa9318c89d86cafde4a7b0aae27813ef33e3912 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Mon, 8 Jun 2020 11:48:24 +0100 Subject: [PATCH 27/27] Apply suggestions from code review --- .../aws-cognito/lib/user-pool-attr.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts index 75013ed28d696..60c011fd9a71b 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-attr.ts @@ -7,103 +7,103 @@ import { Token } from '@aws-cdk/core'; */ export interface StandardAttributes { /** - * The user's postal address is a required attribute. + * The user's postal address. * @default - see the defaults under `StandardAttribute` */ readonly address?: StandardAttribute; /** - * The user's birthday, represented as an ISO 8601:2004 format, is a required attribute. + * The user's birthday, represented as an ISO 8601:2004 format. * @default - see the defaults under `StandardAttribute` */ readonly birthdate?: StandardAttribute; /** - * The user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec, is a required attribute. + * The user's e-mail address, represented as an RFC 5322 [RFC5322] addr-spec. * @default - see the defaults under `StandardAttribute` */ readonly email?: StandardAttribute; /** - * The surname or last name of the user is a required attribute. + * The surname or last name of the user. * @default - see the defaults under `StandardAttribute` */ readonly familyName?: StandardAttribute; /** - * The user's gender is a required attribute. + * The user's gender. * @default - see the defaults under `StandardAttribute` */ readonly gender?: StandardAttribute; /** - * The user's first name or give name is a required attribute. + * The user's first name or give name. * @default - see the defaults under `StandardAttribute` */ readonly givenName?: StandardAttribute; /** - * The user's locale, represented as a BCP47 [RFC5646] language tag, is a required attribute. + * The user's locale, represented as a BCP47 [RFC5646] language tag. * @default - see the defaults under `StandardAttribute` */ readonly locale?: StandardAttribute; /** - * The user's middle name is a required attribute. + * The user's middle name. * @default - see the defaults under `StandardAttribute` */ readonly middleName?: StandardAttribute; /** - * Whether user's full name in displayable form, including all name parts, titles and suffixes, is a required attibute. + * The user's full name in displayable form, including all name parts, titles and suffixes. * @default - see the defaults under `StandardAttribute` */ readonly fullname?: StandardAttribute; /** - * The user's nickname or casual name is a required attribute. + * The user's nickname or casual name. * @default - see the defaults under `StandardAttribute` */ readonly nickname?: StandardAttribute; /** - * The user's telephone number is a required attribute. + * The user's telephone number. * @default - see the defaults under `StandardAttribute` */ readonly phoneNumber?: StandardAttribute; /** - * The URL to the user's profile picture is a required attribute. + * The URL to the user's profile picture. * @default - see the defaults under `StandardAttribute` */ readonly profilePicture?: StandardAttribute; /** - * The user's preffered username, different from the immutable user name, is a required attribute. + * The user's preffered username, different from the immutable user name. * @default - see the defaults under `StandardAttribute` */ readonly preferredUsername?: StandardAttribute; /** - * The URL to the user's profile page is a required attribute. + * The URL to the user's profile page. * @default - see the defaults under `StandardAttribute` */ readonly profilePage?: StandardAttribute; /** - * The user's time zone is a required attribute. + * The user's time zone. * @default - see the defaults under `StandardAttribute` */ readonly timezone?: StandardAttribute; /** - * The time, the user's information was last updated, is a required attribute. + * The time, the user's information was last updated. * @default - see the defaults under `StandardAttribute` */ readonly lastUpdateTime?: StandardAttribute; /** - * The URL to the user's web page or blog is a required attribute. + * The URL to the user's web page or blog. * @default - see the defaults under `StandardAttribute` */ readonly website?: StandardAttribute;