From 9e6dc6b00d4de6e892be3356ea6d24dd0d45bb2a Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 3 Mar 2021 11:31:31 +0100 Subject: [PATCH 01/10] chore(contrib-guide): add guidance around breaking changes (#13347) At the request of the ECS team, describe the two kinds of breaking changes separately and describe some tips for dealing with them. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- CONTRIBUTING.md | 138 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2b1a950a551fe..7615d7b10db4e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,7 @@ and let us know if it's not up-to-date (even better, submit a PR with your corr - [Step 4: Commit](#step-4-commit) - [Step 5: Pull Request](#step-5-pull-request) - [Step 6: Merge](#step-6-merge) +- [Breaking Changes](#breaking-changes) - [Tools](#tools) - [Main build scripts](#main-build-scripts) - [Partial build tools](#partial-build-tools) @@ -266,6 +267,143 @@ BREAKING CHANGE: Description of what broke and how to achieve this behavior now * Once approved and tested, a maintainer will squash-merge to master and will use your PR title/description as the commit message. +## Breaking Changes + +Whenever you are making changes, there is a chance for those changes to be +*breaking* existing users of the library. A change is breaking if there are +programs that customers could have been writing against the current version +of the CDK, that will no longer "work correctly" with the proposed new +version of the CDK. + +Breaking changes are not allowed in *stable* libraries¹. They are permissible +but still *highly discouraged* in experimental libraries, and require explicit +callouts in the bodies of Pull Requests that introduce them. + +> ¹) Note that starting in version 2 of the CDK, the majority of library code will be +> bundled into a single main CDK library which will be considered stable, and so +> no code in there can undergo breaking changes. + +Breaking changes come in two flavors: + +* API surface changes +* Behavior changes + +### API surface changes + +This encompasses any changes that affect the shape of the API. Changes that +will make existing programs fail to compile are not allowed. Typical examples +of that are: + +* Renaming classes or methods +* Adding required properties to a struct that is used as an input to a constructor + or method. This also includes changing a type from nullable to non-nullable. +* Removing properties from a struct that is returned from a method, or removing + properties from a class. This also includes changing a type from non-nullable + to nullable. + +To see why the latter is a problem, consider the following class: + +```ts +class SomeClass { + public readonly count: number; + // ❓ let's say I want to change this to 'count?: number', + // i.e. make it optional. +} + +// Someone could have written the following code: +const obj = new SomeClass(); +console.log(obj.count + 1); + +// After the proposed change, this code that used to compile fine will now throw: +console.log(obj.count + 1); +// ~~~~~~~~~ Error: Object is possibly 'undefined'. +``` + +CDK comes with build tooling to check whether changes you made introduce breaking +changes to the API surface. In a package directory, run: + +```shell +$ yarn build +$ yarn compat +``` + +To figure out if the changes you made were breaking. See the section [API Compatibility +Checks](#api-compatibility-checks) for more information. + +#### Dealing with breaking API surface changes + +If you need to change the type of some API element, introduce a new API +element and mark the old API element as `@deprecated`. + +If you need to pretend to have a value for the purposes of implementing an API +and you don't actually have a useful value to return, it is acceptable to make +the property a `getter` and throw an exception (keeping in mind to write error +messages that will be useful to a user of your construct): + +```ts +class SomeClass implements ICountable { + constructor(private readonly _count?: number) { + } + + public get count(): number { + if (this._count === undefined) { + // ✅ DO: throw a descriptive error that tells the user what to do + throw new Error('This operation requires that a \'count\' is specified when SomeClass is created.'); + // ❌ DO NOT: just throw an error like 'count is missing' + } + return this._count; + } +} +``` + +### Behavior changes + +These are changes that do not directly affect the compilation of programs +written against the previous API, but may change their meaning. In practice, +even though the user didn't change their code, the CloudFormation template +that gets synthesized is now different. + +**Not all template changes are breaking changes!** Consider a user that has +created a Stack using the previous version of the library, has updated their +version of the CDK library and is now deploying an update. A behavior change +is breaking if: + +* The update cannot be applied at all +* The update can be applied but causes service interruption or data loss. + +Data loss happens when the [Logical +ID](https://docs.aws.amazon.com/cdk/latest/guide/identifiers.html#identifiers_logical_ids) +of a stateful resource changes, or one of the [resource properties that requires +replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html) +is modified. In both of these cases, CloudFormation will delete the +resource, and if it was a stateful resource like a database the data in it is now gone. + +If a change applies cleanly and does not cause any service interruption, it +is not breaking. Nevertheless, it might still be wise to avoid those kinds of +changes as users are understandably wary of unexpected template changes, will +scrutinize them heavily, and we don't want to cause unnecessary panic and churn +in our use base. + +Determining whether or not behavioral changes are breaking requires expertise +and judgement on the part of the library owner, and testing. + +#### Dealing with breaking behavior changes + +Most of the time, behavioral changes will arise because we want to change the +default value or default behavior of some property (i.e., we want to change the +interpretation of what it means if the value is missing). + +If the new behavior is going to be breaking, the user must opt in to it, either by: + +* Adding a new API element (class, property, method, ...) to have users + explicitly opt in to the new behavior at the source code level (potentially + `@deprecate`ing the old API element); or +* Use the [feature flag](#feature-flags) mechanism to have the user opt in to the new + behavior without changing the source code. + +Of these two, the first one is preferred if possible (as feature flags have +non-local effects which can cause unintended effects). + ## Tools The CDK is a big project, and at the moment, all of the CDK modules are mastered in a single monolithic repository From b965589358f4c281aea36404276f08128e6ff3db Mon Sep 17 00:00:00 2001 From: Piotr Moszkowicz Date: Wed, 3 Mar 2021 15:30:59 +0100 Subject: [PATCH 02/10] feat(cognito): user pools - sign in with apple (#13160) Added Sign In With Apple provider to `@aws-cdk/aws-cognito`. That's my first PR here, so bear with me, I hope I haven't made any mistakes, I've been following the docs carefully :) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-cognito/README.md | 1 + .../aws-cognito/lib/user-pool-client.ts | 6 + .../aws-cognito/lib/user-pool-idps/apple.ts | 63 ++++++++++ .../aws-cognito/lib/user-pool-idps/base.ts | 9 ++ .../aws-cognito/lib/user-pool-idps/index.ts | 1 + packages/@aws-cdk/aws-cognito/package.json | 3 +- .../integ.user-pool-idp.apple.expected.json | 118 ++++++++++++++++++ .../test/integ.user-pool-idp.apple.ts | 41 ++++++ .../aws-cognito/test/user-pool-client.test.ts | 3 +- .../aws-cognito/test/user-pool-idps/apple.ts | 113 +++++++++++++++++ 10 files changed, 356 insertions(+), 2 deletions(-) create mode 100644 packages/@aws-cdk/aws-cognito/lib/user-pool-idps/apple.ts create mode 100644 packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.expected.json create mode 100644 packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.ts create mode 100644 packages/@aws-cdk/aws-cognito/test/user-pool-idps/apple.ts diff --git a/packages/@aws-cdk/aws-cognito/README.md b/packages/@aws-cdk/aws-cognito/README.md index 4a9a8f5b250cb..d3f90ff026b36 100644 --- a/packages/@aws-cdk/aws-cognito/README.md +++ b/packages/@aws-cdk/aws-cognito/README.md @@ -418,6 +418,7 @@ The following third-party identity providers are currently supported in the CDK - [Login With Amazon](https://developer.amazon.com/apps-and-games/login-with-amazon) - [Facebook Login](https://developers.facebook.com/docs/facebook-login/) - [Google Login](https://developers.google.com/identity/sign-in/web/sign-in) +- [Sign In With Apple](https://developer.apple.com/sign-in-with-apple/get-started/) The following code configures a user pool to federate with the third party provider, 'Login with Amazon'. The identity provider needs to be configured with a set of credentials that the Cognito backend can use to federate with the diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-client.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-client.ts index ea5693f45d1c4..866c11015ecfd 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-client.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-client.ts @@ -157,6 +157,12 @@ export class OAuthScope { * Identity providers supported by the UserPoolClient */ export class UserPoolClientIdentityProvider { + /** + * Allow users to sign in using 'Sign In With Apple'. + * A `UserPoolIdentityProviderApple` must be attached to the user pool. + */ + public static readonly APPLE = new UserPoolClientIdentityProvider('SignInWithApple'); + /** * Allow users to sign in using 'Facebook Login'. * A `UserPoolIdentityProviderFacebook` must be attached to the user pool. diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/apple.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/apple.ts new file mode 100644 index 0000000000000..c1fbd6d4ca9fa --- /dev/null +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/apple.ts @@ -0,0 +1,63 @@ +import { Construct } from 'constructs'; +import { CfnUserPoolIdentityProvider } from '../cognito.generated'; +import { UserPoolIdentityProviderProps } from './base'; +import { UserPoolIdentityProviderBase } from './private/user-pool-idp-base'; + +/** + * Properties to initialize UserPoolAppleIdentityProvider + */ +export interface UserPoolIdentityProviderAppleProps extends UserPoolIdentityProviderProps { + /** + * The client id recognized by Apple APIs. + * @see https://developer.apple.com/documentation/sign_in_with_apple/clientconfigi/3230948-clientid + */ + readonly clientId: string; + /** + * The teamId for Apple APIs to authenticate the client. + */ + readonly teamId: string; + /** + * The keyId (of the same key, which content has to be later supplied as `privateKey`) for Apple APIs to authenticate the client. + */ + readonly keyId: string; + /** + * The privateKey content for Apple APIs to authenticate the client. + */ + readonly privateKey: string; + /** + * The list of apple permissions to obtain for getting access to the apple profile + * @see https://developer.apple.com/documentation/sign_in_with_apple/clientconfigi/3230955-scope + * @default [ name ] + */ + readonly scopes?: string[]; +} + +/** + * Represents a identity provider that integrates with 'Apple' + * @resource AWS::Cognito::UserPoolIdentityProvider + */ +export class UserPoolIdentityProviderApple extends UserPoolIdentityProviderBase { + public readonly providerName: string; + + constructor(scope: Construct, id: string, props: UserPoolIdentityProviderAppleProps) { + super(scope, id, props); + + const scopes = props.scopes ?? ['name']; + + const resource = new CfnUserPoolIdentityProvider(this, 'Resource', { + userPoolId: props.userPool.userPoolId, + providerName: 'SignInWithApple', // must be 'SignInWithApple' when the type is 'SignInWithApple' + providerType: 'SignInWithApple', + providerDetails: { + client_id: props.clientId, + team_id: props.teamId, + key_id: props.keyId, + private_key: props.privateKey, + authorize_scopes: scopes.join(' '), + }, + attributeMapping: super.configureAttributeMapping(), + }); + + this.providerName = super.getResourceNameAttribute(resource.ref); + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/base.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/base.ts index be155fca69a6d..08278947b9e04 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/base.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/base.ts @@ -4,6 +4,15 @@ import { IUserPool } from '../user-pool'; * An attribute available from a third party identity provider. */ export class ProviderAttribute { + /** The email attribute provided by Apple */ + public static readonly APPLE_EMAIL = new ProviderAttribute('email'); + /** The name attribute provided by Apple */ + public static readonly APPLE_NAME = new ProviderAttribute('name'); + /** The first name attribute provided by Apple */ + public static readonly APPLE_FIRST_NAME = new ProviderAttribute('firstName'); + /** The last name attribute provided by Apple */ + public static readonly APPLE_LAST_NAME = new ProviderAttribute('lastName'); + /** The user id attribute provided by Amazon */ public static readonly AMAZON_USER_ID = new ProviderAttribute('user_id'); /** The email attribute provided by Amazon */ diff --git a/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/index.ts b/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/index.ts index dbc63a9854f37..321ee0ecad5d9 100644 --- a/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/index.ts +++ b/packages/@aws-cdk/aws-cognito/lib/user-pool-idps/index.ts @@ -1,4 +1,5 @@ export * from './base'; +export * from './apple'; export * from './amazon'; export * from './facebook'; export * from './google'; \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cognito/package.json b/packages/@aws-cdk/aws-cognito/package.json index c9bae0f953907..3a981167956a3 100644 --- a/packages/@aws-cdk/aws-cognito/package.json +++ b/packages/@aws-cdk/aws-cognito/package.json @@ -110,7 +110,8 @@ "props-physical-name:@aws-cdk/aws-cognito.UserPoolDomainProps", "props-physical-name:@aws-cdk/aws-cognito.UserPoolIdentityProviderFacebookProps", "props-physical-name:@aws-cdk/aws-cognito.UserPoolIdentityProviderAmazonProps", - "props-physical-name:@aws-cdk/aws-cognito.UserPoolIdentityProviderGoogleProps" + "props-physical-name:@aws-cdk/aws-cognito.UserPoolIdentityProviderGoogleProps", + "props-physical-name:@aws-cdk/aws-cognito.UserPoolIdentityProviderAppleProps" ] }, "stability": "stable", diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.expected.json b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.expected.json new file mode 100644 index 0000000000000..64baf88ef6807 --- /dev/null +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.expected.json @@ -0,0 +1,118 @@ +{ + "Resources": { + "pool056F3F7E": { + "Type": "AWS::Cognito::UserPool", + "Properties": { + "AccountRecoverySetting": { + "RecoveryMechanisms": [ + { + "Name": "verified_phone_number", + "Priority": 1 + }, + { + "Name": "verified_email", + "Priority": 2 + } + ] + }, + "AdminCreateUserConfig": { + "AllowAdminCreateUserOnly": true + }, + "EmailVerificationMessage": "The verification code to your new account is {####}", + "EmailVerificationSubject": "Verify your new account", + "SmsVerificationMessage": "The verification code to your new account is {####}", + "VerificationMessageTemplate": { + "DefaultEmailOption": "CONFIRM_WITH_CODE", + "EmailMessage": "The verification code to your new account is {####}", + "EmailSubject": "Verify your new account", + "SmsMessage": "The verification code to your new account is {####}" + } + }, + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + }, + "poolclient2623294C": { + "Type": "AWS::Cognito::UserPoolClient", + "Properties": { + "UserPoolId": { + "Ref": "pool056F3F7E" + }, + "AllowedOAuthFlows": [ + "implicit", + "code" + ], + "AllowedOAuthFlowsUserPoolClient": true, + "AllowedOAuthScopes": [ + "profile", + "phone", + "email", + "openid", + "aws.cognito.signin.user.admin" + ], + "CallbackURLs": [ + "https://example.com" + ], + "SupportedIdentityProviders": [ + { + "Ref": "apple9B5408AC" + }, + "COGNITO" + ] + } + }, + "pooldomain430FA744": { + "Type": "AWS::Cognito::UserPoolDomain", + "Properties": { + "Domain": "nija-test-pool", + "UserPoolId": { + "Ref": "pool056F3F7E" + } + } + }, + "apple9B5408AC": { + "Type": "AWS::Cognito::UserPoolIdentityProvider", + "Properties": { + "ProviderName": "SignInWithApple", + "ProviderType": "SignInWithApple", + "UserPoolId": { + "Ref": "pool056F3F7E" + }, + "AttributeMapping": { + "family_name": "lastName", + "given_name": "firstName" + }, + "ProviderDetails": { + "client_id": "com.amzn.cdk", + "team_id": "CDKTEAMCDK", + "key_id": "CDKKEYCDK1", + "private_key": "PRIV_KEY_CDK", + "authorize_scopes": "email name" + } + } + } + }, + "Outputs": { + "SignInLink": { + "Value": { + "Fn::Join": [ + "", + [ + "https://", + { + "Ref": "pooldomain430FA744" + }, + ".auth.", + { + "Ref": "AWS::Region" + }, + ".amazoncognito.com/login?client_id=", + { + "Ref": "poolclient2623294C" + }, + "&response_type=code&redirect_uri=https://example.com" + ] + ] + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.ts b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.ts new file mode 100644 index 0000000000000..fb8e15f26e308 --- /dev/null +++ b/packages/@aws-cdk/aws-cognito/test/integ.user-pool-idp.apple.ts @@ -0,0 +1,41 @@ +import { App, CfnOutput, RemovalPolicy, Stack } from '@aws-cdk/core'; +import { ProviderAttribute, UserPool, UserPoolIdentityProviderApple } from '../lib'; + +/* + * Stack verification steps + * * Visit the URL provided by stack output 'SignInLink' in a browser, and verify the 'Sign In With Apple' link shows up. + * * If you plug in valid 'Sign In With Apple' credentials, the federated log in should work. + */ +const app = new App(); +const stack = new Stack(app, 'integ-user-pool-idp-apple'); + +const userpool = new UserPool(stack, 'pool', { + removalPolicy: RemovalPolicy.DESTROY, +}); + +new UserPoolIdentityProviderApple(stack, 'apple', { + userPool: userpool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'CDKKEYCDK1', + privateKey: 'PRIV_KEY_CDK', + scopes: ['email', 'name'], + attributeMapping: { + familyName: ProviderAttribute.APPLE_LAST_NAME, + givenName: ProviderAttribute.APPLE_FIRST_NAME, + }, +}); + +const client = userpool.addClient('client'); + +const domain = userpool.addDomain('domain', { + cognitoDomain: { + domainPrefix: 'nija-test-pool', + }, +}); + +new CfnOutput(stack, 'SignInLink', { + value: domain.signInUrl(client, { + redirectUri: 'https://example.com', + }), +}); \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts b/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts index 3a056cd02dda7..02420721df344 100644 --- a/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts +++ b/packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts @@ -487,13 +487,14 @@ describe('User Pool Client', () => { UserPoolClientIdentityProvider.FACEBOOK, UserPoolClientIdentityProvider.AMAZON, UserPoolClientIdentityProvider.GOOGLE, + UserPoolClientIdentityProvider.APPLE, ], }); // THEN expect(stack).toHaveResource('AWS::Cognito::UserPoolClient', { ClientName: 'AllEnabled', - SupportedIdentityProviders: ['COGNITO', 'Facebook', 'LoginWithAmazon', 'Google'], + SupportedIdentityProviders: ['COGNITO', 'Facebook', 'LoginWithAmazon', 'Google', 'SignInWithApple'], }); }); diff --git a/packages/@aws-cdk/aws-cognito/test/user-pool-idps/apple.ts b/packages/@aws-cdk/aws-cognito/test/user-pool-idps/apple.ts new file mode 100644 index 0000000000000..5f4180bce5682 --- /dev/null +++ b/packages/@aws-cdk/aws-cognito/test/user-pool-idps/apple.ts @@ -0,0 +1,113 @@ +import '@aws-cdk/assert/jest'; +import { Stack } from '@aws-cdk/core'; +import { ProviderAttribute, UserPool, UserPoolIdentityProviderApple } from '../../lib'; + +describe('UserPoolIdentityProvider', () => { + describe('apple', () => { + test('defaults', () => { + // GIVEN + const stack = new Stack(); + const pool = new UserPool(stack, 'userpool'); + + // WHEN + new UserPoolIdentityProviderApple(stack, 'userpoolidp', { + userPool: pool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'CDKKEYCDK1', + privateKey: 'PRIV_KEY_CDK', + }); + + expect(stack).toHaveResource('AWS::Cognito::UserPoolIdentityProvider', { + ProviderName: 'SignInWithApple', + ProviderType: 'SignInWithApple', + ProviderDetails: { + client_id: 'com.amzn.cdk', + team_id: 'CDKTEAMCDK', + key_id: 'CDKKEYCDK1', + private_key: 'PRIV_KEY_CDK', + authorize_scopes: 'name', + }, + }); + }); + + test('scopes', () => { + // GIVEN + const stack = new Stack(); + const pool = new UserPool(stack, 'userpool'); + + // WHEN + new UserPoolIdentityProviderApple(stack, 'userpoolidp', { + userPool: pool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'CDKKEYCDK1', + privateKey: 'PRIV_KEY_CDK', + scopes: ['scope1', 'scope2'], + }); + + expect(stack).toHaveResource('AWS::Cognito::UserPoolIdentityProvider', { + ProviderName: 'SignInWithApple', + ProviderType: 'SignInWithApple', + ProviderDetails: { + client_id: 'com.amzn.cdk', + team_id: 'CDKTEAMCDK', + key_id: 'CDKKEYCDK1', + private_key: 'PRIV_KEY_CDK', + authorize_scopes: 'scope1 scope2', + }, + }); + }); + + test('registered with user pool', () => { + // GIVEN + const stack = new Stack(); + const pool = new UserPool(stack, 'userpool'); + + // WHEN + const provider = new UserPoolIdentityProviderApple(stack, 'userpoolidp', { + userPool: pool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'CDKKEYCDK1', + privateKey: 'PRIV_KEY_CDK', + }); + + // THEN + expect(pool.identityProviders).toContain(provider); + }); + + test('attribute mapping', () => { + // GIVEN + const stack = new Stack(); + const pool = new UserPool(stack, 'userpool'); + + // WHEN + new UserPoolIdentityProviderApple(stack, 'userpoolidp', { + userPool: pool, + clientId: 'com.amzn.cdk', + teamId: 'CDKTEAMCDK', + keyId: 'CDKKEYCDK1', + privateKey: 'PRIV_KEY_CDK', + attributeMapping: { + familyName: ProviderAttribute.APPLE_LAST_NAME, + givenName: ProviderAttribute.APPLE_FIRST_NAME, + custom: { + customAttr1: ProviderAttribute.APPLE_EMAIL, + customAttr2: ProviderAttribute.other('sub'), + }, + }, + }); + + // THEN + expect(stack).toHaveResource('AWS::Cognito::UserPoolIdentityProvider', { + AttributeMapping: { + family_name: 'firstName', + given_name: 'lastName', + customAttr1: 'email', + customAttr2: 'sub', + }, + }); + }); + }); +}); \ No newline at end of file From 3413b2f887596d11dfb53c0e99c2a1788095a2ad Mon Sep 17 00:00:00 2001 From: Robert Djurasaj Date: Wed, 3 Mar 2021 08:30:21 -0700 Subject: [PATCH 03/10] fix(core): custom resource provider NODEJS_12 now looks like Lambda's NODEJS_12_X, add Node 14 (#13301) Purpose of this PR is to align `runtime` options for custom resource providers with `aws-lambda` runtime options. New selections include `NODEJS_12_X` and `NODE_JS_14_X`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../test/integ.core-custom-resources.ts | 2 +- .../lib/experimental/edge-function.ts | 2 +- packages/@aws-cdk/aws-iam/lib/oidc-provider.ts | 2 +- packages/@aws-cdk/aws-route53/lib/record-set.ts | 2 +- .../test/integ.bucket-auto-delete-objects.ts | 2 +- .../test/integ.secret-name-parsed.ts | 2 +- packages/@aws-cdk/core/README.md | 6 +++--- .../custom-resource-provider.ts | 14 +++++++++++++- .../core/lib/private/cfn-utils-provider.ts | 2 +- .../custom-resource-provider.test.ts | 12 ++++++------ 10 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts b/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts index 7f5e9d49a95db..bdd4ae31af241 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/integ.core-custom-resources.ts @@ -23,7 +23,7 @@ class TestStack extends Stack { const serviceToken = CustomResourceProvider.getOrCreate(this, resourceType, { codeDirectory: `${__dirname}/core-custom-resource-provider-fixture`, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, description: 'veni vidi vici', }); diff --git a/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts b/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts index 69913aa07a2a1..b12a56fe67e80 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts @@ -180,7 +180,7 @@ export class EdgeFunction extends Resource implements lambda.IVersion { const resourceType = 'Custom::CrossRegionStringParameterReader'; const serviceToken = CustomResourceProvider.getOrCreate(this, resourceType, { codeDirectory: path.join(__dirname, 'edge-function'), - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, policyStatements: [{ Effect: 'Allow', Resource: parameterArnPrefix, diff --git a/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts b/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts index 91c221ec55652..ec70c6d152cbe 100644 --- a/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts +++ b/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts @@ -155,7 +155,7 @@ export class OpenIdConnectProvider extends Resource implements IOpenIdConnectPro private getOrCreateProvider() { return CustomResourceProvider.getOrCreate(this, RESOURCE_TYPE, { codeDirectory: path.join(__dirname, 'oidc-provider'), - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, policyStatements: [ { Effect: 'Allow', diff --git a/packages/@aws-cdk/aws-route53/lib/record-set.ts b/packages/@aws-cdk/aws-route53/lib/record-set.ts index 577af5c1a3a57..a86195f1d0055 100644 --- a/packages/@aws-cdk/aws-route53/lib/record-set.ts +++ b/packages/@aws-cdk/aws-route53/lib/record-set.ts @@ -604,7 +604,7 @@ export class CrossAccountZoneDelegationRecord extends CoreConstruct { const serviceToken = CustomResourceProvider.getOrCreate(this, CROSS_ACCOUNT_ZONE_DELEGATION_RESOURCE_TYPE, { codeDirectory: path.join(__dirname, 'cross-account-zone-delegation-handler'), - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, policyStatements: [{ Effect: 'Allow', Action: 'sts:AssumeRole', Resource: props.delegationRole.roleArn }], }); diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket-auto-delete-objects.ts b/packages/@aws-cdk/aws-s3/test/integ.bucket-auto-delete-objects.ts index 83243212409d7..8052dd340d888 100644 --- a/packages/@aws-cdk/aws-s3/test/integ.bucket-auto-delete-objects.ts +++ b/packages/@aws-cdk/aws-s3/test/integ.bucket-auto-delete-objects.ts @@ -17,7 +17,7 @@ class TestStack extends Stack { // Put objects in the bucket to ensure auto delete works as expected const serviceToken = CustomResourceProvider.getOrCreate(this, PUT_OBJECTS_RESOURCE_TYPE, { codeDirectory: path.join(__dirname, 'put-objects-handler'), - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, policyStatements: [{ Effect: 'Allow', Action: 's3:PutObject', diff --git a/packages/@aws-cdk/aws-secretsmanager/test/integ.secret-name-parsed.ts b/packages/@aws-cdk/aws-secretsmanager/test/integ.secret-name-parsed.ts index 2d245cc6e3e01..801077c5c8494 100644 --- a/packages/@aws-cdk/aws-secretsmanager/test/integ.secret-name-parsed.ts +++ b/packages/@aws-cdk/aws-secretsmanager/test/integ.secret-name-parsed.ts @@ -21,7 +21,7 @@ class SecretsManagerStack extends cdk.Stack { const resourceType = 'Custom::IntegVerificationSecretNameMatches'; const serviceToken = cdk.CustomResourceProvider.getOrCreate(this, resourceType, { codeDirectory: path.join(__dirname, 'integ.secret-name-parsed.handler'), - runtime: cdk.CustomResourceProviderRuntime.NODEJS_12, + runtime: cdk.CustomResourceProviderRuntime.NODEJS_12_X, policyStatements: [{ Effect: 'Allow', Resource: secrets.map(s => s.secretArn), diff --git a/packages/@aws-cdk/core/README.md b/packages/@aws-cdk/core/README.md index 714e7139f0807..bb519f066af55 100644 --- a/packages/@aws-cdk/core/README.md +++ b/packages/@aws-cdk/core/README.md @@ -427,7 +427,7 @@ stack-unique identifier and returns the service token: ```ts const serviceToken = CustomResourceProvider.getOrCreate(this, 'Custom::MyCustomResourceType', { codeDirectory: `${__dirname}/my-handler`, - runtime: CustomResourceProviderRuntime.NODEJS_12, // currently the only supported runtime + runtime: CustomResourceProviderRuntime.NODEJS_12_X, description: "Lambda function created by the custom resource provider", }); @@ -522,7 +522,7 @@ export class Sum extends Construct { const resourceType = 'Custom::Sum'; const serviceToken = CustomResourceProvider.getOrCreate(this, resourceType, { codeDirectory: `${__dirname}/sum-handler`, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, }); const resource = new CustomResource(this, 'Resource', { @@ -552,7 +552,7 @@ built-in singleton method: ```ts const provider = CustomResourceProvider.getOrCreateProvider(this, 'Custom::MyCustomResourceType', { codeDirectory: `${__dirname}/my-handler`, - runtime: CustomResourceProviderRuntime.NODEJS_12, // currently the only supported runtime + runtime: CustomResourceProviderRuntime.NODEJS_12_X, }); const roleArn = provider.roleArn; diff --git a/packages/@aws-cdk/core/lib/custom-resource-provider/custom-resource-provider.ts b/packages/@aws-cdk/core/lib/custom-resource-provider/custom-resource-provider.ts index d6b0a2db982c7..c7f3776339907 100644 --- a/packages/@aws-cdk/core/lib/custom-resource-provider/custom-resource-provider.ts +++ b/packages/@aws-cdk/core/lib/custom-resource-provider/custom-resource-provider.ts @@ -84,8 +84,20 @@ export interface CustomResourceProviderProps { export enum CustomResourceProviderRuntime { /** * Node.js 12.x + * + * @deprecated Use {@link NODEJS_12_X} + */ + NODEJS_12 = 'nodejs12.x', + + /** + * Node.js 12.x + */ + NODEJS_12_X = 'nodejs12.x', + + /** + * Node.js 14.x */ - NODEJS_12 = 'nodejs12.x' + NODEJS_14_X = 'nodejs14.x', } /** diff --git a/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts b/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts index dae7253720041..8200165fbfe34 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts @@ -7,7 +7,7 @@ import { CustomResourceProvider, CustomResourceProviderRuntime } from '../custom export class CfnUtilsProvider extends Construct { public static getOrCreate(scope: Construct) { return CustomResourceProvider.getOrCreate(scope, 'AWSCDKCfnUtilsProvider', { - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, codeDirectory: `${__dirname}/cfn-utils-provider`, }); } diff --git a/packages/@aws-cdk/core/test/custom-resource-provider/custom-resource-provider.test.ts b/packages/@aws-cdk/core/test/custom-resource-provider/custom-resource-provider.test.ts index 594f9c2936ff1..5fc12ecc17c2f 100644 --- a/packages/@aws-cdk/core/test/custom-resource-provider/custom-resource-provider.test.ts +++ b/packages/@aws-cdk/core/test/custom-resource-provider/custom-resource-provider.test.ts @@ -14,7 +14,7 @@ nodeunitShim({ // WHEN CustomResourceProvider.getOrCreate(stack, 'Custom:MyResourceType', { codeDirectory: TEST_HANDLER, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, }); // THEN @@ -149,7 +149,7 @@ nodeunitShim({ // WHEN CustomResourceProvider.getOrCreate(stack, 'Custom:MyResourceType', { codeDirectory: TEST_HANDLER, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, }); // THEN -- no exception @@ -167,7 +167,7 @@ nodeunitShim({ // WHEN CustomResourceProvider.getOrCreate(stack, 'Custom:MyResourceType', { codeDirectory: TEST_HANDLER, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, policyStatements: [ { statement1: 123 }, { statement2: { foo: 111 } }, @@ -194,7 +194,7 @@ nodeunitShim({ // WHEN CustomResourceProvider.getOrCreate(stack, 'Custom:MyResourceType', { codeDirectory: TEST_HANDLER, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, memorySize: Size.gibibytes(2), timeout: Duration.minutes(5), description: 'veni vidi vici', @@ -216,7 +216,7 @@ nodeunitShim({ // WHEN CustomResourceProvider.getOrCreate(stack, 'Custom:MyResourceType', { codeDirectory: TEST_HANDLER, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, environment: { B: 'b', A: 'a', @@ -242,7 +242,7 @@ nodeunitShim({ // WHEN const cr = CustomResourceProvider.getOrCreateProvider(stack, 'Custom:MyResourceType', { codeDirectory: TEST_HANDLER, - runtime: CustomResourceProviderRuntime.NODEJS_12, + runtime: CustomResourceProviderRuntime.NODEJS_12_X, }); // THEN From 1bb3650c5dd2087b05793a5e903cdfb80fc5c1ad Mon Sep 17 00:00:00 2001 From: Alban Esc Date: Wed, 3 Mar 2021 08:03:08 -0800 Subject: [PATCH 04/10] feat(events): dead letter queue for Lambda Targets (#11617) Add DLQ Configuration to Rule targets. Using a DLQ on a rule prevents the application to loose events after all retry attempts are exhausted. Resolves #11612 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-events-targets/README.md | 35 +++- .../@aws-cdk/aws-events-targets/lib/lambda.ts | 20 +- .../@aws-cdk/aws-events-targets/lib/util.ts | 45 ++++- .../test/lambda/integ.events.expected.json | 89 +++++++++ .../test/lambda/integ.events.ts | 12 ++ .../test/lambda/lambda.test.ts | 178 ++++++++++++++++++ packages/@aws-cdk/aws-events/lib/rule.ts | 1 + packages/@aws-cdk/aws-events/lib/target.ts | 6 + 8 files changed, 383 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/README.md b/packages/@aws-cdk/aws-events-targets/README.md index ba0e83c2a82ba..cd843ef130a3a 100644 --- a/packages/@aws-cdk/aws-events-targets/README.md +++ b/packages/@aws-cdk/aws-events-targets/README.md @@ -32,7 +32,40 @@ Currently supported are: See the README of the `@aws-cdk/aws-events` library for more information on EventBridge. -## LogGroup +## Invoke a Lambda function + +Use the `LambdaFunction` target to invoke a lambda function. + +The code snippet below creates an event rule with a Lambda function as a target +triggered for every events from `aws.ec2` source. You can optionally attach a +[dead letter queue](https://docs.aws.amazon.com/eventbridge/latest/userguide/rule-dlq.html). + +```ts +import * as lambda from "@aws-cdk/aws-lambda"; +import * as events from "@aws-cdk/aws-events"; +import * as sqs from "@aws-cdk/aws-sqs"; +import * as targets from "@aws-cdk/aws-events-targets"; + +const fn = new lambda.Function(this, 'MyFunc', { + runtime: lambda.Runtime.NODEJS_12_X, + handler: 'index.handler', + code: lambda.Code.fromInline(`exports.handler = ${handler.toString()}`), +}); + +const rule = new events.Rule(this, 'rule', { + eventPattern: { + source: ["aws.ec2"], + }, +}); + +const queue = new sqs.Queue(this, 'Queue'); + +rule.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, // Optional: add a dead letter queue +})); +``` + +## Log an event into a LogGroup Use the `LogGroup` target to log your events in a CloudWatch LogGroup. diff --git a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts index 780cd6d57162c..44315579a1300 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts @@ -1,6 +1,7 @@ import * as events from '@aws-cdk/aws-events'; import * as lambda from '@aws-cdk/aws-lambda'; -import { addLambdaPermission } from './util'; +import * as sqs from '@aws-cdk/aws-sqs'; +import { addLambdaPermission, addToDeadLetterQueueResourcePolicy } from './util'; /** * Customize the Lambda Event Target @@ -14,6 +15,18 @@ export interface LambdaFunctionProps { * @default the entire EventBridge event */ readonly event?: events.RuleTargetInput; + + /** + * The SQS queue to be used as deadLetterQueue. + * Check out the [considerations for using a dead-letter queue](https://docs.aws.amazon.com/eventbridge/latest/userguide/rule-dlq.html#dlq-considerations). + * + * The events not successfully delivered are automatically retried for a specified period of time, + * depending on the retry policy of the target. + * If an event is not delivered before all retry attempts are exhausted, it will be sent to the dead letter queue. + * + * @default - no dead-letter queue + */ + readonly deadLetterQueue?: sqs.IQueue; } /** @@ -32,9 +45,14 @@ export class LambdaFunction implements events.IRuleTarget { // Allow handler to be called from rule addLambdaPermission(rule, this.handler); + if (this.props.deadLetterQueue) { + addToDeadLetterQueueResourcePolicy(rule, this.props.deadLetterQueue); + } + return { id: '', arn: this.handler.functionArn, + deadLetterConfig: this.props.deadLetterQueue ? { arn: this.props.deadLetterQueue?.queueArn } : undefined, input: this.props.event, targetResource: this.handler, }; diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts index 1026d1ae35a1a..069b04a8c5131 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/util.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/util.ts @@ -1,7 +1,8 @@ import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; -import { ConstructNode, IConstruct, Names } from '@aws-cdk/core'; +import * as sqs from '@aws-cdk/aws-sqs'; +import { Annotations, ConstructNode, IConstruct, Names, Token, TokenComparison } from '@aws-cdk/core'; // keep this import separate from other imports to reduce chance for merge conflicts with v2-main // eslint-disable-next-line no-duplicate-imports, import/order @@ -50,3 +51,45 @@ export function addLambdaPermission(rule: events.IRule, handler: lambda.IFunctio }); } } + +/** + * Allow a rule to send events with failed invocation to an Amazon SQS queue. + */ +export function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sqs.IQueue) { + if (!sameEnvDimension(rule.env.region, queue.env.region)) { + throw new Error(`Cannot assign Dead Letter Queue in region ${queue.env.region} to the rule ${Names.nodeUniqueId(rule.node)} in region ${rule.env.region}. Both the queue and the rule must be in the same region.`); + } + + // Skip Resource Policy creation if the Queue is not in the same account. + // There is no way to add a target onto an imported rule, so we can assume we will run the following code only + // in the account where the rule is created. + if (sameEnvDimension(rule.env.account, queue.env.account)) { + const policyStatementId = `AllowEventRule${Names.nodeUniqueId(rule.node)}`; + + queue.addToResourcePolicy(new iam.PolicyStatement({ + sid: policyStatementId, + principals: [new iam.ServicePrincipal('events.amazonaws.com')], + effect: iam.Effect.ALLOW, + actions: ['sqs:SendMessage'], + resources: [queue.queueArn], + conditions: { + ArnEquals: { + 'aws:SourceArn': rule.ruleArn, + }, + }, + })); + } else { + Annotations.of(rule).addWarning(`Cannot add a resource policy to your dead letter queue associated with rule ${rule.ruleName} because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account ${queue.env.account}.`); + } +} + + +/** + * Whether two string probably contain the same environment dimension (region or account) + * + * Used to compare either accounts or regions, and also returns true if both + * are unresolved (in which case both are expted to be "current region" or "current account"). + */ +function sameEnvDimension(dim1: string, dim2: string) { + return [TokenComparison.SAME, TokenComparison.BOTH_UNRESOLVED].includes(Token.compareStrings(dim1, dim2)); +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json index c12d92ef34810..7df2a4becf021 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json @@ -123,6 +123,95 @@ ] } } + }, + "Timer30894E3BB": { + "Type": "AWS::Events::Rule", + "Properties": { + "ScheduleExpression": "rate(2 minutes)", + "State": "ENABLED", + "Targets": [ + { + "Arn": { + "Fn::GetAtt": [ + "MyFunc8A243A2C", + "Arn" + ] + }, + "DeadLetterConfig": { + "Arn": { + "Fn::GetAtt": [ + "Queue4A7E3555", + "Arn" + ] + } + }, + "Id": "Target0" + } + ] + } + }, + "Timer3AllowEventRulelambdaeventsMyFunc910E580F79317F73": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "MyFunc8A243A2C", + "Arn" + ] + }, + "Principal": "events.amazonaws.com", + "SourceArn": { + "Fn::GetAtt": [ + "Timer30894E3BB", + "Arn" + ] + } + } + }, + "Queue4A7E3555": { + "Type": "AWS::SQS::Queue", + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + }, + "QueuePolicy25439813": { + "Type": "AWS::SQS::QueuePolicy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "sqs:SendMessage", + "Condition": { + "ArnEquals": { + "aws:SourceArn": { + "Fn::GetAtt": [ + "Timer30894E3BB", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Principal": { + "Service": "events.amazonaws.com" + }, + "Resource": { + "Fn::GetAtt": [ + "Queue4A7E3555", + "Arn" + ] + }, + "Sid": "AllowEventRulelambdaeventsTimer3107B9373" + } + ], + "Version": "2012-10-17" + }, + "Queues": [ + { + "Ref": "Queue4A7E3555" + } + ] + } } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts index 17d17a291e244..c37c632d803ae 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts @@ -1,5 +1,6 @@ import * as events from '@aws-cdk/aws-events'; import * as lambda from '@aws-cdk/aws-lambda'; +import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; import * as targets from '../../lib'; @@ -23,6 +24,17 @@ const timer2 = new events.Rule(stack, 'Timer2', { }); timer2.addTarget(new targets.LambdaFunction(fn)); + +const timer3 = new events.Rule(stack, 'Timer3', { + schedule: events.Schedule.rate(cdk.Duration.minutes(2)), +}); + +const queue = new sqs.Queue(stack, 'Queue'); + +timer3.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, +})); + app.synth(); /* eslint-disable no-console */ diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts index 29be8973fe3c3..c5fd260715293 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts @@ -1,6 +1,7 @@ import '@aws-cdk/assert/jest'; import * as events from '@aws-cdk/aws-events'; import * as lambda from '@aws-cdk/aws-lambda'; +import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; import * as constructs from 'constructs'; import * as targets from '../../lib'; @@ -147,6 +148,183 @@ test('lambda handler and cloudwatch event across stacks', () => { expect(eventStack).toCountResources('AWS::Lambda::Permission', 1); }); +test('use a Dead Letter Queue for the rule target', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'Stack'); + + const fn = new lambda.Function(stack, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + }); + + const queue = new sqs.Queue(stack, 'Queue'); + + new events.Rule(stack, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + targets: [new targets.LambdaFunction(fn, { + deadLetterQueue: queue, + })], + }); + + expect(() => app.synth()).not.toThrow(); + + // the Permission resource should be in the event stack + expect(stack).toHaveResource('AWS::Events::Rule', { + Targets: [ + { + Arn: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + DeadLetterConfig: { + Arn: { + 'Fn::GetAtt': [ + 'Queue4A7E3555', + 'Arn', + ], + }, + }, + Id: 'Target0', + }, + ], + }); + + expect(stack).toHaveResource('AWS::SQS::QueuePolicy', { + PolicyDocument: { + Statement: [ + { + Action: 'sqs:SendMessage', + Condition: { + ArnEquals: { + 'aws:SourceArn': { + 'Fn::GetAtt': [ + 'Rule4C995B7F', + 'Arn', + ], + }, + }, + }, + Effect: 'Allow', + Principal: { + Service: 'events.amazonaws.com', + }, + Resource: { + 'Fn::GetAtt': [ + 'Queue4A7E3555', + 'Arn', + ], + }, + Sid: 'AllowEventRuleStackRuleF6E31DD0', + }, + ], + Version: '2012-10-17', + }, + Queues: [ + { + Ref: 'Queue4A7E3555', + }, + ], + }); +}); + +test('throw an error when using a Dead Letter Queue for the rule target in a different region', () => { + // GIVEN + const app = new cdk.App(); + const stack1 = new cdk.Stack(app, 'Stack1', { + env: { + region: 'eu-west-1', + }, + }); + const stack2 = new cdk.Stack(app, 'Stack2', { + env: { + region: 'eu-west-2', + }, + }); + + const fn = new lambda.Function(stack1, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + }); + + const queue = new sqs.Queue(stack2, 'Queue'); + + let rule = new events.Rule(stack1, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + }); + + + expect(() => { + rule.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, + })); + }).toThrow(/Cannot assign Dead Letter Queue in region eu-west-2 to the rule Stack1Rule92BA1111 in region eu-west-1. Both the queue and the rule must be in the same region./); +}); + +test('must display a warning when using a Dead Letter Queue from another account', () => { + // GIVEN + const app = new cdk.App(); + const stack1 = new cdk.Stack(app, 'Stack1', { + env: { + region: 'eu-west-1', + account: '111111111111', + }, + }); + + const stack2 = new cdk.Stack(app, 'Stack2', { + env: { + region: 'eu-west-1', + account: '222222222222', + }, + }); + + const fn = new lambda.Function(stack1, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + }); + + const queue = sqs.Queue.fromQueueArn(stack2, 'Queue', 'arn:aws:sqs:eu-west-1:444455556666:queue1'); + + new events.Rule(stack1, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + targets: [new targets.LambdaFunction(fn, { + deadLetterQueue: queue, + })], + }); + + expect(() => app.synth()).not.toThrow(); + + // the Permission resource should be in the event stack + expect(stack1).toHaveResource('AWS::Events::Rule', { + ScheduleExpression: 'rate(1 minute)', + State: 'ENABLED', + Targets: [ + { + Arn: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + DeadLetterConfig: { + Arn: 'arn:aws:sqs:eu-west-1:444455556666:queue1', + }, + Id: 'Target0', + }, + ], + }); + + expect(stack1).not.toHaveResource('AWS::SQS::QueuePolicy'); + + let rule = stack1.node.children.find(child => child instanceof events.Rule); + expect(rule?.node.metadata[0].data).toMatch(/Cannot add a resource policy to your dead letter queue associated with rule .* because the queue is in a different account\. You must add the resource policy manually to the dead letter queue in account 222222222222\./); +}); + function newTestLambda(scope: constructs.Construct, suffix = '') { return new lambda.Function(scope, `MyLambda${suffix}`, { code: new lambda.InlineCode('foo'), diff --git a/packages/@aws-cdk/aws-events/lib/rule.ts b/packages/@aws-cdk/aws-events/lib/rule.ts index 969965021f75d..2c71054b60828 100644 --- a/packages/@aws-cdk/aws-events/lib/rule.ts +++ b/packages/@aws-cdk/aws-events/lib/rule.ts @@ -295,6 +295,7 @@ export class Rule extends Resource implements IRule { kinesisParameters: targetProps.kinesisParameters, runCommandParameters: targetProps.runCommandParameters, batchParameters: targetProps.batchParameters, + deadLetterConfig: targetProps.deadLetterConfig, sqsParameters: targetProps.sqsParameters, input: inputProps && inputProps.input, inputPath: inputProps && inputProps.inputPath, diff --git a/packages/@aws-cdk/aws-events/lib/target.ts b/packages/@aws-cdk/aws-events/lib/target.ts index 319cf3d4f14da..da2ac9a1c7fb1 100644 --- a/packages/@aws-cdk/aws-events/lib/target.ts +++ b/packages/@aws-cdk/aws-events/lib/target.ts @@ -47,6 +47,12 @@ export interface RuleTargetConfig { */ readonly batchParameters?: CfnRule.BatchParametersProperty; + /** + * Contains information about a dead-letter queue configuration. + * @default no dead-letter queue set + */ + readonly deadLetterConfig?: CfnRule.DeadLetterConfigProperty; + /** * The Amazon ECS task definition and task count to use, if the event target * is an Amazon ECS task. From 75008bc4076a9fc9a0e9b74821c9205276e5fe84 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 3 Mar 2021 17:35:44 +0100 Subject: [PATCH 05/10] chore(guidelines): update ToC (#13372) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- DESIGN_GUIDELINES.md | 122 ++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/DESIGN_GUIDELINES.md b/DESIGN_GUIDELINES.md index 5924ec0321293..0a8566ab9d90c 100644 --- a/DESIGN_GUIDELINES.md +++ b/DESIGN_GUIDELINES.md @@ -1,65 +1,5 @@ # AWS Construct Library Design Guidelines -- [AWS Construct Library Design Guidelines](#aws-construct-library-design-guidelines) - - [What's Included](#what-s-included) - - [API Design](#api-design) - - [Modules](#modules) - - [Construct Class](#construct-class) - - [Construct Interface](#construct-interface) - - [Owned vs. Unowned Constructs](#owned-vs-unowned-constructs) - - [Abstract Base](#abstract-base) - - [Props](#props) - - [Types](#types) - - [Defaults](#defaults) - - [Flat](#flat) - - [Concise](#concise) - - [Naming](#naming) - - [Property Documentation](#property-documentation) - - [Enums](#enums) - - [Unions](#unions) - - [Attributes](#attributes) - - [Configuration](#configuration) - - [Prefer Additions](#prefer-additions) - - [Dropped Mutations](#dropped-mutations) - - [Factories](#factories) - - [Imports](#imports) - - [“from” Methods](#-from--methods) - - [From-attributes](#from-attributes) - - [Roles](#roles) - - [Resource Policies](#resource-policies) - - [VPC](#vpc) - - [Grants](#grants) - - [Metrics](#metrics) - - [Events](#events) - - [Connections](#connections) - - [Integrations](#integrations) - - [State](#state) - - [Physical Names - TODO](#physical-names---todo) - - [Tags](#tags) - - [Secrets](#secrets) - - [Project Structure](#project-structure) - - [Code Organization](#code-organization) - - [Implementation](#implementation) - - [General Principles](#general-principles) - - [Construct IDs](#construct-ids) - - [Errors](#errors) - - [Input Validation](#input-validation) - - [Avoid Errors If Possible](#avoid-errors-if-possible) - - [Never Catch Exceptions](#never-catch-exceptions) - - [Post Validation](#post-validation) - - [Attached Errors/Warnings](#attached-errors-warnings) - - [Tokens](#tokens) - - [Documentation](#documentation) - - [Inline Documentation](#inline-documentation) - - [Readme](#readme) - - [Testing](#testing) - - [Unit tests](#unit-tests) - - [Integration tests](#integration-tests) - - [Versioning](#versioning) - - [Naming & Style](#naming---style) - - [Naming Conventions](#naming-conventions) - - [Coding Style](#coding-style) - The AWS Construct Library is a rich class library of CDK constructs which represent all resources offered by the AWS Cloud and higher-level constructs for achieving common tasks. @@ -68,6 +8,68 @@ The purpose of this document is to provide guidelines for designing the APIs in the AWS Construct Library in order to ensure a consistent and integrated experience across the entire AWS surface area. +* [Preface](#preface) +* [What's Included](#what-s-included) +* [API Design](#api-design) + * [Modules](#modules) + * [Construct Class](#construct-class) + * [Construct Interface](#construct-interface) + * [Owned vs. Unowned Constructs](#owned-vs-unowned-constructs) + * [Abstract Base](#abstract-base) + * [Props](#props) + * [Types](#types) + * [Defaults](#defaults) + * [Flat](#flat) + * [Concise](#concise) + * [Naming](#naming) + * [Property Documentation](#property-documentation) + * [Enums](#enums) + * [Unions](#unions) + * [Attributes](#attributes) + * [Configuration](#configuration) + * [Prefer Additions](#prefer-additions) + * [Dropped Mutations](#dropped-mutations) + * [Factories](#factories) + * [Imports](#imports) + * [“from” Methods](#-from--methods) + * [From-attributes](#from-attributes) + * [Roles](#roles) + * [Resource Policies](#resource-policies) + * [VPC](#vpc) + * [Grants](#grants) + * [Metrics](#metrics) + * [Events](#events) + * [Connections](#connections) + * [Integrations](#integrations) + * [State](#state) + * [Physical Names - TODO](#physical-names---todo) + * [Tags](#tags) + * [Secrets](#secrets) +* [Project Structure](#project-structure) + * [Code Organization](#code-organization) +* [Implementation](#implementation) + * [General Principles](#general-principles) + * [Construct IDs](#construct-ids) + * [Errors](#errors) + * [Avoid Errors If Possible](#avoid-errors-if-possible) + * [Error reporting mechanism](#error-reporting-mechanism) + * [Throwing exceptions](#throwing-exceptions) + * [Never Catch Exceptions](#never-catch-exceptions) + * [Attaching (lazy) Validators](#attaching--lazy--validators) + * [Attaching Errors/Warnings](#attaching-errors-warnings) + * [Error messages](#error-messages) + * [Tokens](#tokens) +* [Documentation](#documentation) + * [Inline Documentation](#inline-documentation) + * [Readme](#readme) +* [Testing](#testing) + * [Unit tests](#unit-tests) + * [Integration tests](#integration-tests) + * [Versioning](#versioning) +* [Naming & Style](#naming---style) + * [Naming Conventions](#naming-conventions) + * [Coding Style](#coding-style) + ## Preface As much as possible, the guidelines in this document are enforced using the From 6126e499e5ca22b5f751af4f4f05d74f696829f1 Mon Sep 17 00:00:00 2001 From: Meng Xin Zhu Date: Thu, 4 Mar 2021 01:08:59 +0800 Subject: [PATCH 06/10] fix(stepfunctions): `SageMakeUpdateEndpoint` adds insufficient permissions (#13170) closes #11594 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/sagemaker/update-endpoint.ts | 7 +++ .../integ.call-sagemaker.expected.json | 60 ++++++++++++------ .../test/sagemaker/update-endpoint.test.ts | 63 +++++++++++++++++++ 3 files changed, 111 insertions(+), 19 deletions(-) diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker/update-endpoint.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker/update-endpoint.ts index 4a4e068d816d0..4ff5bf4d19bb3 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker/update-endpoint.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker/update-endpoint.ts @@ -73,6 +73,13 @@ export class SageMakerUpdateEndpoint extends sfn.TaskStateBase { // SageMaker uses lowercase for resource name in the arn resourceName: sfn.JsonPath.isEncodedJsonPath(this.props.endpointName) ? '*' : `${this.props.endpointName.toLowerCase()}`, }), + stack.formatArn({ + service: 'sagemaker', + resource: 'endpoint-config', + // If the endpointConfig name comes from input, we cannot target the policy to a particular ARN prefix reliably. + // SageMaker uses lowercase for resource name in the arn + resourceName: sfn.JsonPath.isEncodedJsonPath(this.props.endpointConfigName) ? '*' : `${this.props.endpointConfigName.toLowerCase()}`, + }), ], }), ]; diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/integ.call-sagemaker.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/integ.call-sagemaker.expected.json index 1e8f55ca57056..c163ebd11cf77 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/integ.call-sagemaker.expected.json +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/integ.call-sagemaker.expected.json @@ -465,26 +465,48 @@ { "Action": "sagemaker:updateEndpoint", "Effect": "Allow", - "Resource": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":sagemaker:", - { - "Ref": "AWS::Region" - }, - ":", - { - "Ref": "AWS::AccountId" - }, - ":endpoint/*" + "Resource": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":sagemaker:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":endpoint/*" + ] ] - ] - } + }, + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":sagemaker:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":endpoint-config/*" + ] + ] + } + ] } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/update-endpoint.test.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/update-endpoint.test.ts index f628ddbe9ed14..edd90392970b5 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/update-endpoint.test.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker/update-endpoint.test.ts @@ -1,4 +1,5 @@ import '@aws-cdk/assert/jest'; +import * as iam from '@aws-cdk/aws-iam'; import * as sfn from '@aws-cdk/aws-stepfunctions'; import * as cdk from '@aws-cdk/core'; import * as tasks from '../../lib'; @@ -80,3 +81,65 @@ test('Task throws if WAIT_FOR_TASK_TOKEN is supplied as service integration patt }); }).toThrow(/Unsupported service integration pattern. Supported Patterns: REQUEST_RESPONSE. Received: WAIT_FOR_TASK_TOKEN/i); }); + +test('PolicyStatement has sufficient permissions', () => { + // WHEN + const props = { + endpointName: 'MyEndpoint', + endpointConfigName: 'MyEndpointConfig', + }; + const task = new tasks.SageMakerUpdateEndpoint(stack, 'SagemakerEndpoint', props); + + const graph = new sfn.StateGraph(task, 'test'); + + // THEN + expect(graph.policyStatements).toEqual( + [ + new iam.PolicyStatement({ + actions: ['sagemaker:updateEndpoint'], + resources: [ + stack.formatArn({ + service: 'sagemaker', + resource: 'endpoint', + resourceName: props.endpointName.toLowerCase(), + }), + stack.formatArn({ + service: 'sagemaker', + resource: 'endpoint-config', + resourceName: props.endpointConfigName.toLowerCase(), + }), + ], + }), + ], + ); + + // WHEN + const props2 = { + endpointName: sfn.JsonPath.stringAt('$.Endpoint.Name'), + endpointConfigName: sfn.JsonPath.stringAt('$.Endpoint.Config'), + }; + const task2 = new tasks.SageMakerUpdateEndpoint(stack, 'SagemakerEndpoint2', props2); + + const graph2 = new sfn.StateGraph(task2, 'test'); + + // THEN + expect(graph2.policyStatements).toEqual( + [ + new iam.PolicyStatement({ + actions: ['sagemaker:updateEndpoint'], + resources: [ + stack.formatArn({ + service: 'sagemaker', + resource: 'endpoint', + resourceName: '*', + }), + stack.formatArn({ + service: 'sagemaker', + resource: 'endpoint-config', + resourceName: '*', + }), + ], + }), + ], + ); +}); \ No newline at end of file From 6f7cebdf61073cc1fb358fcac5f5b2156389cb81 Mon Sep 17 00:00:00 2001 From: rli1994hi Date: Wed, 3 Mar 2021 09:42:58 -0800 Subject: [PATCH 07/10] fix(events): imported ECS Task Definition cannot be used as target (#13293) ### Note Fixes #12811 This change allows creating `EcsTask` from an imported task definition. It does so by changing the `taskDefinition` type in `EcsTaskProps` from concrete class `TaskDefinition` to interface `ITaskDefinition`. ### Testing `buildup` aws-events-targets ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/base/_imported-task-definition.ts | 108 ++++++++ .../aws-ecs/lib/base/task-definition.ts | 73 +++++- .../aws-ecs/lib/ec2/ec2-task-definition.ts | 46 +++- .../lib/fargate/fargate-task-definition.ts | 42 ++- .../test/ec2/ec2-task-definition.test.ts | 81 +++++- .../fargate/fargate-task-definition.test.ts | 88 ++++++- .../aws-ecs/test/task-definition.test.ts | 131 ++++++++-- .../aws-events-targets/lib/ecs-task.ts | 11 +- .../test/ecs/event-rule-target.test.ts | 243 ++++++++++++++++++ 9 files changed, 774 insertions(+), 49 deletions(-) create mode 100644 packages/@aws-cdk/aws-ecs/lib/base/_imported-task-definition.ts diff --git a/packages/@aws-cdk/aws-ecs/lib/base/_imported-task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/_imported-task-definition.ts new file mode 100644 index 0000000000000..3c9c583b96de0 --- /dev/null +++ b/packages/@aws-cdk/aws-ecs/lib/base/_imported-task-definition.ts @@ -0,0 +1,108 @@ +import { IRole } from '@aws-cdk/aws-iam'; +import { Construct } from 'constructs'; +import { IEc2TaskDefinition } from '../ec2/ec2-task-definition'; +import { IFargateTaskDefinition } from '../fargate/fargate-task-definition'; +import { Compatibility, NetworkMode, isEc2Compatible, isFargateCompatible } from './task-definition'; +import { Resource } from '@aws-cdk/core'; + +/** + * The properties of ImportedTaskDefinition + */ +export interface ImportedTaskDefinitionProps { + /** + * The arn of the task definition + */ + readonly taskDefinitionArn: string; + + /** + * What launch types this task definition should be compatible with. + * + * @default Compatibility.EC2_AND_FARGATE + */ + readonly compatibility?: Compatibility; + + /** + * The networking mode to use for the containers in the task. + * + * @default Network mode cannot be provided to the imported task. + */ + readonly networkMode?: NetworkMode; + + /** + * The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf. + * + * @default Permissions cannot be granted to the imported task. + */ + readonly taskRole?: IRole; +} + +/** + * Task definition reference of an imported task + */ +export class ImportedTaskDefinition extends Resource implements IEc2TaskDefinition, IFargateTaskDefinition { + /** + * What launch types this task definition should be compatible with. + */ + readonly compatibility: Compatibility; + + /** + * ARN of this task definition + */ + readonly taskDefinitionArn: string; + + /** + * Execution role for this task definition + */ + readonly executionRole?: IRole = undefined; + + /** + * The networking mode to use for the containers in the task. + */ + readonly _networkMode?: NetworkMode; + + /** + * The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf. + */ + readonly _taskRole?: IRole; + + constructor(scope: Construct, id: string, props: ImportedTaskDefinitionProps) { + super(scope, id); + + this.compatibility = props.compatibility ?? Compatibility.EC2_AND_FARGATE; + this.taskDefinitionArn = props.taskDefinitionArn; + this._taskRole = props.taskRole; + this._networkMode = props.networkMode; + } + + public get networkMode(): NetworkMode { + if (this._networkMode == undefined) { + throw new Error('This operation requires the networkMode in ImportedTaskDefinition to be defined. ' + + 'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + } else { + return this._networkMode; + } + } + + public get taskRole(): IRole { + if (this._taskRole == undefined) { + throw new Error('This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + } else { + return this._taskRole; + } + } + + /** + * Return true if the task definition can be run on an EC2 cluster + */ + public get isEc2Compatible(): boolean { + return isEc2Compatible(this.compatibility); + } + + /** + * Return true if the task definition can be run on a Fargate cluster + */ + public get isFargateCompatible(): boolean { + return isFargateCompatible(this.compatibility); + } +} diff --git a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts index f7b0f05c92800..338fd59b39ea7 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts @@ -8,6 +8,7 @@ import { FirelensLogRouter, FirelensLogRouterDefinitionOptions, FirelensLogRoute import { AwsLogDriver } from '../log-drivers/aws-log-driver'; import { PlacementConstraint } from '../placement'; import { ProxyConfiguration } from '../proxy-configuration/proxy-configuration'; +import { ImportedTaskDefinition } from './_imported-task-definition'; /** * The interface for all task definitions. @@ -38,6 +39,16 @@ export interface ITaskDefinition extends IResource { * Return true if the task definition can be run on a Fargate cluster */ readonly isFargateCompatible: boolean; + + /** + * The networking mode to use for the containers in the task. + */ + readonly networkMode: NetworkMode; + + /** + * The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf. + */ + readonly taskRole: iam.IRole; } /** @@ -175,10 +186,48 @@ export interface TaskDefinitionProps extends CommonTaskDefinitionProps { readonly pidMode?: PidMode; } +/** + * The common task definition attributes used across all types of task definitions. + */ +export interface CommonTaskDefinitionAttributes { + /** + * The arn of the task definition + */ + readonly taskDefinitionArn: string; + + /** + * The networking mode to use for the containers in the task. + * + * @default Network mode cannot be provided to the imported task. + */ + readonly networkMode?: NetworkMode; + + /** + * The name of the IAM role that grants containers in the task permission to call AWS APIs on your behalf. + * + * @default Permissions cannot be granted to the imported task. + */ + readonly taskRole?: iam.IRole; +} + +/** + * A reference to an existing task definition + */ +export interface TaskDefinitionAttributes extends CommonTaskDefinitionAttributes { + /** + * What launch types this task definition should be compatible with. + * + * @default Compatibility.EC2_AND_FARGATE + */ + readonly compatibility?: Compatibility; +} + abstract class TaskDefinitionBase extends Resource implements ITaskDefinition { public abstract readonly compatibility: Compatibility; + public abstract readonly networkMode: NetworkMode; public abstract readonly taskDefinitionArn: string; + public abstract readonly taskRole: iam.IRole; public abstract readonly executionRole?: iam.IRole; /** @@ -207,13 +256,19 @@ export class TaskDefinition extends TaskDefinitionBase { * The task will have a compatibility of EC2+Fargate. */ public static fromTaskDefinitionArn(scope: Construct, id: string, taskDefinitionArn: string): ITaskDefinition { - class Import extends TaskDefinitionBase { - public readonly taskDefinitionArn = taskDefinitionArn; - public readonly compatibility = Compatibility.EC2_AND_FARGATE; - public readonly executionRole?: iam.IRole = undefined; - } + return new ImportedTaskDefinition(scope, id, { taskDefinitionArn: taskDefinitionArn }); + } - return new Import(scope, id); + /** + * Create a task definition from a task definition reference + */ + public static fromTaskDefinitionAttributes(scope: Construct, id: string, attrs: TaskDefinitionAttributes): ITaskDefinition { + return new ImportedTaskDefinition(scope, id, { + taskDefinitionArn: attrs.taskDefinitionArn, + compatibility: attrs.compatibility, + networkMode: attrs.networkMode, + taskRole: attrs.taskRole, + }); } /** @@ -248,7 +303,7 @@ export class TaskDefinition extends TaskDefinitionBase { public defaultContainer?: ContainerDefinition; /** - * The task launch type compatiblity requirement. + * The task launch type compatibility requirement. */ public readonly compatibility: Compatibility; @@ -890,13 +945,13 @@ export interface ITaskDefinitionExtension { /** * Return true if the given task definition can be run on an EC2 cluster */ -function isEc2Compatible(compatibility: Compatibility): boolean { +export function isEc2Compatible(compatibility: Compatibility): boolean { return [Compatibility.EC2, Compatibility.EC2_AND_FARGATE].includes(compatibility); } /** * Return true if the given task definition can be run on a Fargate cluster */ -function isFargateCompatible(compatibility: Compatibility): boolean { +export function isFargateCompatible(compatibility: Compatibility): boolean { return [Compatibility.FARGATE, Compatibility.EC2_AND_FARGATE].includes(compatibility); } diff --git a/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-task-definition.ts index 67d096830b9c7..ff571c884b73e 100644 --- a/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-task-definition.ts @@ -1,7 +1,16 @@ -import { Resource } from '@aws-cdk/core'; import { Construct } from 'constructs'; -import { CommonTaskDefinitionProps, Compatibility, IpcMode, ITaskDefinition, NetworkMode, PidMode, TaskDefinition } from '../base/task-definition'; +import { + CommonTaskDefinitionAttributes, + CommonTaskDefinitionProps, + Compatibility, + IpcMode, + ITaskDefinition, + NetworkMode, + PidMode, + TaskDefinition, +} from '../base/task-definition'; import { PlacementConstraint } from '../placement'; +import { ImportedTaskDefinition } from '../base/_imported-task-definition'; /** * The properties for a task definition run on an EC2 cluster. @@ -51,6 +60,13 @@ export interface IEc2TaskDefinition extends ITaskDefinition { } +/** + * Attributes used to import an existing EC2 task definition + */ +export interface Ec2TaskDefinitionAttributes extends CommonTaskDefinitionAttributes { + +} + /** * The details of a task definition run on an EC2 cluster. * @@ -62,13 +78,25 @@ export class Ec2TaskDefinition extends TaskDefinition implements IEc2TaskDefinit * Imports a task definition from the specified task definition ARN. */ public static fromEc2TaskDefinitionArn(scope: Construct, id: string, ec2TaskDefinitionArn: string): IEc2TaskDefinition { - class Import extends Resource implements IEc2TaskDefinition { - public readonly taskDefinitionArn = ec2TaskDefinitionArn; - public readonly compatibility = Compatibility.EC2; - public readonly isEc2Compatible = true; - public readonly isFargateCompatible = false; - } - return new Import(scope, id); + return new ImportedTaskDefinition(scope, id, { + taskDefinitionArn: ec2TaskDefinitionArn, + }); + } + + /** + * Imports an existing Ec2 task definition from its attributes + */ + public static fromEc2TaskDefinitionAttributes( + scope: Construct, + id: string, + attrs: Ec2TaskDefinitionAttributes, + ): IEc2TaskDefinition { + return new ImportedTaskDefinition(scope, id, { + taskDefinitionArn: attrs.taskDefinitionArn, + compatibility: Compatibility.EC2, + networkMode: attrs.networkMode, + taskRole: attrs.taskRole, + }); } /** diff --git a/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.ts index eba4ac4371ee8..3d8d113886709 100644 --- a/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.ts @@ -1,6 +1,14 @@ -import { Resource, Tokenization } from '@aws-cdk/core'; +import { Tokenization } from '@aws-cdk/core'; import { Construct } from 'constructs'; -import { CommonTaskDefinitionProps, Compatibility, ITaskDefinition, NetworkMode, TaskDefinition } from '../base/task-definition'; +import { + CommonTaskDefinitionAttributes, + CommonTaskDefinitionProps, + Compatibility, + ITaskDefinition, + NetworkMode, + TaskDefinition, +} from '../base/task-definition'; +import { ImportedTaskDefinition } from '../base/_imported-task-definition'; /** * The properties for a task definition. @@ -51,6 +59,13 @@ export interface IFargateTaskDefinition extends ITaskDefinition { } +/** + * Attributes used to import an existing Fargate task definition + */ +export interface FargateTaskDefinitionAttributes extends CommonTaskDefinitionAttributes { + +} + /** * The details of a task definition run on a Fargate cluster. * @@ -62,14 +77,23 @@ export class FargateTaskDefinition extends TaskDefinition implements IFargateTas * Imports a task definition from the specified task definition ARN. */ public static fromFargateTaskDefinitionArn(scope: Construct, id: string, fargateTaskDefinitionArn: string): IFargateTaskDefinition { - class Import extends Resource implements IFargateTaskDefinition { - public readonly taskDefinitionArn = fargateTaskDefinitionArn; - public readonly compatibility = Compatibility.FARGATE; - public readonly isEc2Compatible = false; - public readonly isFargateCompatible = true; - } + return new ImportedTaskDefinition(scope, id, { taskDefinitionArn: fargateTaskDefinitionArn }); + } - return new Import(scope, id); + /** + * Import an existing Fargate task definition from its attributes + */ + public static fromFargateTaskDefinitionAttributes( + scope: Construct, + id: string, + attrs: FargateTaskDefinitionAttributes, + ): IFargateTaskDefinition { + return new ImportedTaskDefinition(scope, id, { + taskDefinitionArn: attrs.taskDefinitionArn, + compatibility: Compatibility.FARGATE, + networkMode: attrs.networkMode, + taskRole: attrs.taskRole, + }); } /** diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts index bd13e8b83b524..4f713b06d2d71 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts @@ -1199,6 +1199,85 @@ describe('ec2 task definition', () => { }); }); + describe('When importing from an existing Ec2 TaskDefinition', () => { + test('can succeed using TaskDefinition Arn', () => { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + + // WHEN + const taskDefinition = ecs.Ec2TaskDefinition.fromEc2TaskDefinitionArn(stack, 'EC2_TD_ID', expectTaskDefinitionArn); + + // THEN + expect(taskDefinition.taskDefinitionArn).toBe(expectTaskDefinitionArn); + }); + }); + + describe('When importing from an existing Ec2 TaskDefinition using attributes', () => { + test('can set the imported task attribuets successfully', () => { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectNetworkMode = ecs.NetworkMode.AWS_VPC; + const expectTaskRole = new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }); + + // WHEN + const taskDefinition = ecs.Ec2TaskDefinition.fromEc2TaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + networkMode: expectNetworkMode, + taskRole: expectTaskRole, + }); + + // THEN + expect(taskDefinition.taskDefinitionArn).toBe(expectTaskDefinitionArn); + expect(taskDefinition.compatibility).toBe(ecs.Compatibility.EC2); + expect(taskDefinition.isEc2Compatible).toBeTruthy(); + expect(taskDefinition.isFargateCompatible).toBeFalsy(); + expect(taskDefinition.networkMode).toBe(expectNetworkMode); + expect(taskDefinition.taskRole).toBe(expectTaskRole); + }); + + test('returns an Ec2 TaskDefinition that will throw an error when trying to access its yet to defined networkMode', () => { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectTaskRole = new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }); + + // WHEN + const taskDefinition = ecs.Ec2TaskDefinition.fromEc2TaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + taskRole: expectTaskRole, + }); + + // THEN + expect(() => taskDefinition.networkMode).toThrow( + 'This operation requires the networkMode in ImportedTaskDefinition to be defined. ' + + 'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + }); + + test('returns an Ec2 TaskDefinition that will throw an error when trying to access its yet to defined taskRole', () => { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectNetworkMode = ecs.NetworkMode.AWS_VPC; + + // WHEN + const taskDefinition = ecs.Ec2TaskDefinition.fromEc2TaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + networkMode: expectNetworkMode, + }); + + // THEN + expect(() => { taskDefinition.taskRole; }).toThrow( + 'This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + }); + }); + test('throws when setting proxyConfiguration without networkMode AWS_VPC', () => { // GIVEN const stack = new cdk.Stack(); @@ -1218,7 +1297,5 @@ describe('ec2 task definition', () => { expect(() => { new ecs.Ec2TaskDefinition(stack, 'TaskDef', { networkMode: ecs.NetworkMode.BRIDGE, proxyConfiguration }); }).toThrow(/ProxyConfiguration can only be used with AwsVpc network mode, got: bridge/); - - }); }); diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/fargate-task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/fargate/fargate-task-definition.test.ts index 00ec0028ef1a3..b8fad1bc9316f 100644 --- a/packages/@aws-cdk/aws-ecs/test/fargate/fargate-task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/fargate/fargate-task-definition.test.ts @@ -5,7 +5,7 @@ import { nodeunitShim, Test } from 'nodeunit-shim'; import * as ecs from '../../lib'; nodeunitShim({ - 'When creating an Fargate TaskDefinition': { + 'When creating a Fargate TaskDefinition': { 'with only required properties set, it correctly sets default properties'(test: Test) { // GIVEN const stack = new cdk.Stack(); @@ -114,4 +114,90 @@ nodeunitShim({ test.done(); }, }, + + 'When importing from an existing Fargate TaskDefinition': { + 'can succeed using TaskDefinition Arn'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + + // WHEN + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionArn(stack, 'FARGATE_TD_ID', expectTaskDefinitionArn); + + // THEN + test.equal(taskDefinition.taskDefinitionArn, expectTaskDefinitionArn); + test.done(); + }, + + 'can succeed using attributes'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectNetworkMode = ecs.NetworkMode.AWS_VPC; + const expectTaskRole = new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }); + + // WHEN + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + networkMode: expectNetworkMode, + taskRole: expectTaskRole, + }); + + // THEN + test.equal(taskDefinition.taskDefinitionArn, expectTaskDefinitionArn); + test.equal(taskDefinition.compatibility, ecs.Compatibility.FARGATE); + test.ok(taskDefinition.isFargateCompatible); + test.equal(taskDefinition.isEc2Compatible, false); + test.equal(taskDefinition.networkMode, expectNetworkMode); + test.equal(taskDefinition.taskRole, expectTaskRole); + + test.done(); + }, + + 'returns a Fargate TaskDefinition that will throw an error when trying to access its networkMode but its networkMode is undefined'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectTaskRole = new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }); + + // WHEN + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + taskRole: expectTaskRole, + }); + + // THEN + test.throws(() => { + taskDefinition.networkMode; + }, 'This operation requires the networkMode in ImportedTaskDefinition to be defined. ' + + 'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + + test.done(); + }, + + 'returns a Fargate TaskDefinition that will throw an error when trying to access its taskRole but its taskRole is undefined'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectNetworkMode = ecs.NetworkMode.AWS_VPC; + + // WHEN + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + networkMode: expectNetworkMode, + }); + + // THEN + test.throws(() => { + taskDefinition.taskRole; + }, 'This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + + test.done(); + }, + }, }); diff --git a/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts index 34a7306106d66..709d3abd86026 100644 --- a/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts @@ -2,24 +2,121 @@ import { expect, haveResource } from '@aws-cdk/assert'; import * as cdk from '@aws-cdk/core'; import { nodeunitShim, Test } from 'nodeunit-shim'; import * as ecs from '../lib'; +import * as iam from '@aws-cdk/aws-iam'; nodeunitShim({ - 'A task definition with both compatibilities defaults to networkmode AwsVpc'(test: Test) { - // GIVEN - const stack = new cdk.Stack(); - - // WHEN - new ecs.TaskDefinition(stack, 'TD', { - cpu: '512', - memoryMiB: '512', - compatibility: ecs.Compatibility.EC2_AND_FARGATE, - }); - - // THEN - expect(stack).to(haveResource('AWS::ECS::TaskDefinition', { - NetworkMode: 'awsvpc', - })); - - test.done(); + 'When creating a new TaskDefinition': { + 'A task definition with both compatibilities defaults to networkmode AwsVpc'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + new ecs.TaskDefinition(stack, 'TD', { + cpu: '512', + memoryMiB: '512', + compatibility: ecs.Compatibility.EC2_AND_FARGATE, + }); + + // THEN + expect(stack).to(haveResource('AWS::ECS::TaskDefinition', { + NetworkMode: 'awsvpc', + })); + + test.done(); + }, + }, + + 'When importing from an existing Task definition': { + 'can import using a task definition arn'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const taskDefinitionArn = 'TDArn'; + + // WHEN + const taskDefinition = ecs.TaskDefinition.fromTaskDefinitionArn(stack, 'TD_ID', taskDefinitionArn); + + // THEN + test.equal(taskDefinition.taskDefinitionArn, taskDefinitionArn); + test.equal(taskDefinition.compatibility, ecs.Compatibility.EC2_AND_FARGATE); + test.equal(taskDefinition.executionRole, undefined); + + test.done(); + }, + + 'can import a Task Definition using attributes'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectCompatibility = ecs.Compatibility.EC2; + const expectNetworkMode = ecs.NetworkMode.AWS_VPC; + const expectTaskRole = new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }); + + // WHEN + const taskDefinition = ecs.TaskDefinition.fromTaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + compatibility: expectCompatibility, + networkMode: expectNetworkMode, + taskRole: expectTaskRole, + }); + + // THEN + test.equal(taskDefinition.taskDefinitionArn, expectTaskDefinitionArn); + test.equal(taskDefinition.compatibility, expectCompatibility); + test.equal(taskDefinition.executionRole, undefined); + test.equal(taskDefinition.networkMode, expectNetworkMode); + test.equal(taskDefinition.taskRole, expectTaskRole); + + test.done(); + }, + + 'returns an imported TaskDefinition that will throw an error when trying to access its yet to defined networkMode'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectCompatibility = ecs.Compatibility.EC2; + const expectTaskRole = new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }); + + // WHEN + const taskDefinition = ecs.TaskDefinition.fromTaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + compatibility: expectCompatibility, + taskRole: expectTaskRole, + }); + + // THEN + test.throws(() => { + taskDefinition.networkMode; + }, 'This operation requires the networkMode in ImportedTaskDefinition to be defined. ' + + 'Add the \'networkMode\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + + test.done(); + }, + + 'returns an imported TaskDefinition that will throw an error when trying to access its yet to defined taskRole'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const expectTaskDefinitionArn = 'TD_ARN'; + const expectCompatibility = ecs.Compatibility.EC2; + const expectNetworkMode = ecs.NetworkMode.AWS_VPC; + + // WHEN + const taskDefinition = ecs.TaskDefinition.fromTaskDefinitionAttributes(stack, 'TD_ID', { + taskDefinitionArn: expectTaskDefinitionArn, + compatibility: expectCompatibility, + networkMode: expectNetworkMode, + }); + + // THEN + test.throws(() => { + taskDefinition.taskRole; + }, 'This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); + + test.done(); + }, }, }); diff --git a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts index baff4c76cde66..a31fc0b68ca5c 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts @@ -18,7 +18,7 @@ export interface EcsTaskProps { /** * Task Definition of the task that should be started */ - readonly taskDefinition: ecs.TaskDefinition; + readonly taskDefinition: ecs.ITaskDefinition; /** * How many tasks should be started when this event is triggered @@ -103,7 +103,7 @@ export class EcsTask implements events.IRuleTarget { */ public readonly securityGroups?: ec2.ISecurityGroup[]; private readonly cluster: ecs.ICluster; - private readonly taskDefinition: ecs.TaskDefinition; + private readonly taskDefinition: ecs.ITaskDefinition; private readonly taskCount: number; private readonly role: iam.IRole; private readonly platformVersion?: ecs.FargatePlatformVersion; @@ -137,6 +137,13 @@ export class EcsTask implements events.IRuleTarget { this.securityGroups = props.securityGroups; return; } + + if (!cdk.Construct.isConstruct(this.taskDefinition)) { + throw new Error('Cannot create a security group for ECS task. ' + + 'The task definition in ECS task is not a Construct. ' + + 'Please pass a taskDefinition as a Construct in EcsTaskProps.'); + } + let securityGroup = props.securityGroup || this.taskDefinition.node.tryFindChild('SecurityGroup') as ec2.ISecurityGroup; securityGroup = securityGroup || new ec2.SecurityGroup(this.taskDefinition, 'SecurityGroup', { vpc: this.props.cluster.vpc }); this.securityGroup = securityGroup; // Maintain backwards-compatibility for customers that read the generated security group. diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts b/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts index 4ac8347d02ec3..f0ab7770fadbd 100644 --- a/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts @@ -58,6 +58,249 @@ test('Can use EC2 taskdef as EventRule target', () => { }); }); +test('Throws error for lacking of taskRole ' + + 'when importing from an EC2 task definition just from a task definition arn as EventRule target', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + cluster.addCapacity('DefaultAutoScalingGroup', { + instanceType: new ec2.InstanceType('t2.micro'), + }); + + const taskDefinition = ecs.Ec2TaskDefinition.fromEc2TaskDefinitionArn(stack, 'TaskDef', 'importedTaskDefArn'); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + + // THEN + expect(() => { + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', events.EventField.fromPath('$.detail.event')], + }], + })); + }).toThrow('This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); +}); + +test('Can import an EC2 task definition from task definition attributes as EventRule target', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + cluster.addCapacity('DefaultAutoScalingGroup', { + instanceType: new ec2.InstanceType('t2.micro'), + }); + + const taskDefinition = ecs.Ec2TaskDefinition.fromEc2TaskDefinitionAttributes(stack, 'TaskDef', { + taskDefinitionArn: 'importedTaskDefArn', + networkMode: ecs.NetworkMode.BRIDGE, + taskRole: new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }), + }); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + + // WHEN + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', events.EventField.fromPath('$.detail.event')], + }], + })); + + // THEN + expect(stack).toHaveResourceLike('AWS::Events::Rule', { + Targets: [ + { + Arn: { 'Fn::GetAtt': ['EcsCluster97242B84', 'Arn'] }, + EcsParameters: { + TaskCount: 1, + TaskDefinitionArn: 'importedTaskDefArn', + }, + InputTransformer: { + InputPathsMap: { + 'detail-event': '$.detail.event', + }, + InputTemplate: '{"containerOverrides":[{"name":"TheContainer","command":["echo",]}]}', + }, + RoleArn: { 'Fn::GetAtt': ['TaskDefEventsRoleFB3B67B8', 'Arn'] }, + Id: 'Target0', + }, + ], + }); +}); + +test('Throws error for lacking of taskRole ' + + 'when importing from a Fargate task definition just from a task definition arn as EventRule target', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionArn(stack, 'TaskDef', 'ImportedTaskDefArn'); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + + // THEN + expect(() => { + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', events.EventField.fromPath('$.detail.event')], + }], + })); + }).toThrow('This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); +}); + +test('Can import a Fargate task definition from task definition attributes as EventRule target', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionAttributes(stack, 'TaskDef', { + taskDefinitionArn: 'importedTaskDefArn', + networkMode: ecs.NetworkMode.AWS_VPC, + taskRole: new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }), + }); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + + // WHEN + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', events.EventField.fromPath('$.detail.event')], + }], + })); + + // THEN + expect(stack).toHaveResourceLike('AWS::Events::Rule', { + Targets: [ + { + Arn: { 'Fn::GetAtt': ['EcsCluster97242B84', 'Arn'] }, + EcsParameters: { + TaskCount: 1, + TaskDefinitionArn: 'importedTaskDefArn', + }, + InputTransformer: { + InputPathsMap: { + 'detail-event': '$.detail.event', + }, + InputTemplate: '{"containerOverrides":[{"name":"TheContainer","command":["echo",]}]}', + }, + RoleArn: { 'Fn::GetAtt': ['TaskDefEventsRoleFB3B67B8', 'Arn'] }, + Id: 'Target0', + }, + ], + }); +}); + +test('Throws error for lacking of taskRole ' + + 'when importing from a task definition just from a task definition arn as EventRule target', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + + const taskDefinition = ecs.TaskDefinition.fromTaskDefinitionArn(stack, 'TaskDef', 'ImportedTaskDefArn'); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + + // THEN + expect(() => { + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', events.EventField.fromPath('$.detail.event')], + }], + })); + }).toThrow('This operation requires the taskRole in ImportedTaskDefinition to be defined. ' + + 'Add the \'taskRole\' in ImportedTaskDefinitionProps to instantiate ImportedTaskDefinition'); +}); + +test('Can import a Task definition from task definition attributes as EventRule target', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 1 }); + const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc }); + + const taskDefinition = ecs.FargateTaskDefinition.fromFargateTaskDefinitionAttributes(stack, 'TaskDef', { + taskDefinitionArn: 'importedTaskDefArn', + networkMode: ecs.NetworkMode.AWS_VPC, + taskRole: new iam.Role(stack, 'TaskRole', { + assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), + }), + }); + + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.expression('rate(1 min)'), + }); + + // WHEN + rule.addTarget(new targets.EcsTask({ + cluster, + taskDefinition, + taskCount: 1, + containerOverrides: [{ + containerName: 'TheContainer', + command: ['echo', events.EventField.fromPath('$.detail.event')], + }], + })); + + // THEN + expect(stack).toHaveResourceLike('AWS::Events::Rule', { + Targets: [ + { + Arn: { 'Fn::GetAtt': ['EcsCluster97242B84', 'Arn'] }, + EcsParameters: { + TaskCount: 1, + TaskDefinitionArn: 'importedTaskDefArn', + }, + InputTransformer: { + InputPathsMap: { + 'detail-event': '$.detail.event', + }, + InputTemplate: '{"containerOverrides":[{"name":"TheContainer","command":["echo",]}]}', + }, + RoleArn: { 'Fn::GetAtt': ['TaskDefEventsRoleFB3B67B8', 'Arn'] }, + Id: 'Target0', + }, + ], + }); +}); + test('Can use Fargate taskdef as EventRule target', () => { // GIVEN const stack = new cdk.Stack(); From cbcc712e0c4c44c83c7f4d1e8a544bccfa26bb56 Mon Sep 17 00:00:00 2001 From: Jonne Kaunisto Date: Wed, 3 Mar 2021 10:15:59 -0800 Subject: [PATCH 08/10] fix(cloudwatch): metric `label` not rendered into Alarms (#13070) Cloudwatch allows adding labels to metrics. These can be seen when viewing alarms. Currently labels only work for math expression metrics. This change will fix it to work for single alarm metrics too. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 46 ++++++++++++----- .../test/integ.alarm-with-label.expected.json | 50 +++++++++++++++++++ .../test/integ.alarm-with-label.ts | 32 ++++++++++++ 3 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.expected.json create mode 100644 packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.ts diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index 5c1daacb54dd3..73c1209733b5f 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -227,18 +227,40 @@ export class Alarm extends AlarmBase { private renderMetric(metric: IMetric) { const self = this; return dispatchMetric(metric, { - withStat(st) { - self.validateMetricStat(st, metric); - - return dropUndefined({ - dimensions: st.dimensions, - namespace: st.namespace, - metricName: st.metricName, - period: st.period?.toSeconds(), - statistic: renderIfSimpleStatistic(st.statistic), - extendedStatistic: renderIfExtendedStatistic(st.statistic), - unit: st.unitFilter, - }); + withStat(stat, conf) { + self.validateMetricStat(stat, metric); + + if (conf.renderingProperties?.label == undefined) { + return dropUndefined({ + dimensions: stat.dimensions, + namespace: stat.namespace, + metricName: stat.metricName, + period: stat.period?.toSeconds(), + statistic: renderIfSimpleStatistic(stat.statistic), + extendedStatistic: renderIfExtendedStatistic(stat.statistic), + unit: stat.unitFilter, + }); + } + + return { + metrics: [ + { + metricStat: { + metric: { + metricName: stat.metricName, + namespace: stat.namespace, + dimensions: stat.dimensions, + }, + period: stat.period.toSeconds(), + stat: stat.statistic, + unit: stat.unitFilter, + }, + id: stat.metricName, + label: conf.renderingProperties?.label, + returnData: true, + } as CfnAlarm.MetricDataQueryProperty, + ], + }; }, withExpression() { diff --git a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.expected.json b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.expected.json new file mode 100644 index 0000000000000..9ab6e14f29a6e --- /dev/null +++ b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.expected.json @@ -0,0 +1,50 @@ +{ + "Resources": { + "Alarm1F9009D71": { + "Type": "AWS::CloudWatch::Alarm", + "Properties": { + "Metrics": [ + { + "Id": "Metric", + "Label": "Metric [AVG: ${AVG}]", + "MetricStat": { + "Metric": { + "MetricName": "Metric", + "Namespace": "CDK/Test" + }, + "Period": 300, + "Stat": "Average" + }, + "ReturnData": true + } + ], + "ComparisonOperator": "GreaterThanOrEqualToThreshold", + "EvaluationPeriods": 3, + "Threshold": 100 + } + }, + "Alarm2A7122E13": { + "Type": "AWS::CloudWatch::Alarm", + "Properties": { + "Metrics": [ + { + "Id": "Metric", + "Label": "Metric [AVG: ${AVG}]", + "MetricStat": { + "Metric": { + "MetricName": "Metric", + "Namespace": "CDK/Test" + }, + "Period": 300, + "Stat": "Average" + }, + "ReturnData": true + } + ], + "ComparisonOperator": "GreaterThanOrEqualToThreshold", + "EvaluationPeriods": 3, + "Threshold": 100 + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.ts b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.ts new file mode 100644 index 0000000000000..85aa1976b51aa --- /dev/null +++ b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-with-label.ts @@ -0,0 +1,32 @@ +import { App, Stack, StackProps } from '@aws-cdk/core'; +import { Alarm, Metric } from '../lib'; + +class AlarmWithLabelIntegrationTest extends Stack { + + constructor(scope: App, id: string, props?: StackProps) { + super(scope, id, props); + + const testMetric = new Metric({ + namespace: 'CDK/Test', + metricName: 'Metric', + label: 'Metric [AVG: ${AVG}]', + }); + + new Alarm(this, 'Alarm1', { + metric: testMetric, + threshold: 100, + evaluationPeriods: 3, + }); + + testMetric.createAlarm(this, 'Alarm2', { + threshold: 100, + evaluationPeriods: 3, + }); + } +} + +const app = new App(); + +new AlarmWithLabelIntegrationTest(app, 'AlarmWithLabelIntegrationTest'); + +app.synth(); From e244c5d7409980267d1fa48afbfea38c42e88a28 Mon Sep 17 00:00:00 2001 From: proxy-hatch <26352422+proxy-hatch@users.noreply.github.com> Date: Wed, 3 Mar 2021 10:48:54 -0800 Subject: [PATCH 09/10] chore(certificatemanager): update README.md (#12408) typo ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-certificatemanager/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-certificatemanager/README.md b/packages/@aws-cdk/aws-certificatemanager/README.md index 5c27597f6701b..320f22c91663d 100644 --- a/packages/@aws-cdk/aws-certificatemanager/README.md +++ b/packages/@aws-cdk/aws-certificatemanager/README.md @@ -76,7 +76,7 @@ const cert = new acm.Certificate(this, 'Certificate', { domainName: 'test.example.com', subjectAlternativeNames: ['cool.example.com', 'test.example.net'], validation: acm.CertificateValidation.fromDnsMultiZone({ - 'text.example.com': exampleCom, + 'test.example.com': exampleCom, 'cool.example.com': exampleCom, 'test.example.net': exampleNet, }), From 2c074de3e8948e9ce007603cb8f0eaae36a3afcd Mon Sep 17 00:00:00 2001 From: Robert Djurasaj Date: Wed, 3 Mar 2021 12:34:14 -0700 Subject: [PATCH 10/10] chore(apigateway): cleanup DomainName TLS docstring and tests (#13309) Cleanup of apigateway module (docs & tests). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-apigateway/lib/domain-name.ts | 18 +++++++-------- .../aws-apigateway/test/domains.test.ts | 22 ++----------------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/domain-name.ts b/packages/@aws-cdk/aws-apigateway/lib/domain-name.ts index 0437b986fdc74..c9f082121bff6 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/domain-name.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/domain-name.ts @@ -12,8 +12,9 @@ import { EndpointType, IRestApi } from './restapi'; export enum SecurityPolicy { /** Cipher suite TLS 1.0 */ TLS_1_0 = 'TLS_1_0', + /** Cipher suite TLS 1.2 */ - TLS_1_2 = 'TLS_1_2' + TLS_1_2 = 'TLS_1_2', } export interface DomainNameOptions { @@ -40,13 +41,13 @@ export interface DomainNameOptions { * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-domainname.html * @default SecurityPolicy.TLS_1_0 */ - readonly securityPolicy?: SecurityPolicy + readonly securityPolicy?: SecurityPolicy; /** * The mutual TLS authentication configuration for a custom domain name. * @default - mTLS is not configured. */ - readonly mtls?: MTLSConfig + readonly mtls?: MTLSConfig; } export interface DomainNameProps extends DomainNameOptions { @@ -83,7 +84,6 @@ export interface IDomainName extends IResource { * @attribute DistributionHostedZoneId,RegionalHostedZoneId */ readonly domainNameAliasHostedZoneId: string; - } export class DomainName extends Resource implements IDomainName { @@ -112,9 +112,9 @@ export class DomainName extends Resource implements IDomainName { const edge = endpointType === EndpointType.EDGE; if (!Token.isUnresolved(props.domainName) && /[A-Z]/.test(props.domainName)) { - throw new Error('domainName does not support uppercase letters. ' + - `got: '${props.domainName}'`); + throw new Error(`Domain name does not support uppercase letters. Got: ${props.domainName}`); } + const mtlsConfig = this.configureMTLS(props.mtls); const resource = new CfnDomainName(this, 'Resource', { domainName: props.domainName, @@ -176,10 +176,9 @@ export interface DomainNameAttributes { readonly domainNameAliasTarget: string; /** - * Thje Route53 hosted zone ID to use in order to connect a record set to this domain through an alias. + * The Route53 hosted zone ID to use in order to connect a record set to this domain through an alias. */ readonly domainNameAliasHostedZoneId: string; - } /** @@ -190,8 +189,9 @@ export interface MTLSConfig { * The bucket that the trust store is hosted in. */ readonly bucket: IBucket; + /** - * The key in S3 to look at for the trust store + * The key in S3 to look at for the trust store. */ readonly key: string; diff --git a/packages/@aws-cdk/aws-apigateway/test/domains.test.ts b/packages/@aws-cdk/aws-apigateway/test/domains.test.ts index ee70b4e9cb98e..2b7f49f9a3977 100644 --- a/packages/@aws-cdk/aws-apigateway/test/domains.test.ts +++ b/packages/@aws-cdk/aws-apigateway/test/domains.test.ts @@ -1,5 +1,5 @@ -import '@aws-cdk/assert/jest'; import { ABSENT } from '@aws-cdk/assert'; +import '@aws-cdk/assert/jest'; import * as acm from '@aws-cdk/aws-certificatemanager'; import { Bucket } from '@aws-cdk/aws-s3'; import { Stack } from '@aws-cdk/core'; @@ -43,8 +43,6 @@ describe('domains', () => { expect(stack.resolve(regionalDomain.domainNameAliasHostedZoneId)).toEqual({ 'Fn::GetAtt': ['mydomain592C948B', 'RegionalHostedZoneId'] }); expect(stack.resolve(edgeDomain.domainNameAliasDomainName)).toEqual({ 'Fn::GetAtt': ['yourdomain5FE30C81', 'DistributionDomainName'] }); expect(stack.resolve(edgeDomain.domainNameAliasHostedZoneId)).toEqual({ 'Fn::GetAtt': ['yourdomain5FE30C81', 'DistributionHostedZoneId'] }); - - }); test('default endpoint type is REGIONAL', () => { @@ -64,7 +62,6 @@ describe('domains', () => { 'EndpointConfiguration': { 'Types': ['REGIONAL'] }, 'RegionalCertificateArn': { 'Ref': 'Cert5C9FAEC1' }, }); - }); test('accepts different security policies', () => { @@ -111,7 +108,6 @@ describe('domains', () => { 'RegionalCertificateArn': { 'Ref': 'Cert5C9FAEC1' }, 'SecurityPolicy': ABSENT, }); - }); test('"mapping" can be used to automatically map this domain to the deployment stage of an API', () => { @@ -140,7 +136,6 @@ describe('domains', () => { 'Ref': 'apiDeploymentStageprod896C8101', }, }); - }); test('"addBasePathMapping" can be used to add base path mapping to the domain', () => { @@ -186,7 +181,6 @@ describe('domains', () => { 'Ref': 'api2DeploymentStageprod4120D74E', }, }); - }); test('a domain name can be defined with the API', () => { @@ -225,8 +219,6 @@ describe('domains', () => { 'Ref': 'apiDeploymentStageprod896C8101', }, }); - - }); test('a domain name can be added later', () => { @@ -265,8 +257,6 @@ describe('domains', () => { 'Ref': 'apiDeploymentStageprod896C8101', }, }); - - }); test('domain name cannot contain uppercase letters', () => { @@ -274,13 +264,10 @@ describe('domains', () => { const stack = new Stack(); const certificate = new acm.Certificate(stack, 'cert', { domainName: 'someDomainWithUpercase.domain.com' }); - // WHEN + // WHEN & THEN expect(() => { new apigw.DomainName(stack, 'someDomain', { domainName: 'someDomainWithUpercase.domain.com', certificate }); }).toThrow(/uppercase/); - - // THEN - }); test('multiple domain names can be added', () => { @@ -345,8 +332,6 @@ describe('domains', () => { 'Ref': 'apiDeploymentStageprod896C8101', }, }); - - }); test('"addBasePathMapping" can be used to add base path mapping to the domain with specific stage', () => { @@ -440,7 +425,6 @@ describe('domains', () => { 'RegionalCertificateArn': 'arn:aws:acm:us-east-1:1111111:certificate/11-3336f1-44483d-adc7-9cd375c5169d', 'MutualTlsAuthentication': { 'TruststoreUri': 's3://exampleBucket/someca.pem', 'TruststoreVersion': 'version' }, }); - }); test('base path mapping configures stage for RestApi creation', () => { @@ -466,8 +450,6 @@ describe('domains', () => { 'Ref': 'restApiWithStageDeploymentStageprodC82A6648', }, }); - - }); test('base path mapping configures stage for SpecRestApi creation', () => {