From e6b414f70fcaad2f697749368f6768a0e187f629 Mon Sep 17 00:00:00 2001 From: Aaron Tsui Date: Mon, 21 Mar 2022 18:33:51 +0800 Subject: [PATCH 1/7] docs(iot): add example import (#19478) ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](../CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](../CONTRIBUTING.md/#adding-new-unconventional-dependencies) *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-iot/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@aws-cdk/aws-iot/README.md b/packages/@aws-cdk/aws-iot/README.md index 410657b9ad71d..d0afc29c1fa6c 100644 --- a/packages/@aws-cdk/aws-iot/README.md +++ b/packages/@aws-cdk/aws-iot/README.md @@ -36,6 +36,7 @@ Import it into your code: ```ts nofixture import * as iot from '@aws-cdk/aws-iot'; +import * as actions from '@aws-cdk/aws-iot-actions-alpha'; ``` ## `TopicRule` From 2d2a34d60df5da50d9782ac4405295f070d7a167 Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Mon, 21 Mar 2022 07:17:03 -0400 Subject: [PATCH 2/7] chore(prlint): add integration test check for feature request PRs (#19445) Updates `prlint` to require all new features have a corresponding update to integration tests. This requirement can be exempt by applying the `pr-linter/exempt-integ-test` label. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- tools/@aws-cdk/prlint/lint.ts | 22 +++++- tools/@aws-cdk/prlint/test/lint.test.ts | 95 +++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts index 17bc7db78050d..39e0eded94b43 100755 --- a/tools/@aws-cdk/prlint/lint.ts +++ b/tools/@aws-cdk/prlint/lint.ts @@ -7,6 +7,7 @@ const OWNER = 'aws'; const REPO = 'aws-cdk'; const EXEMPT_README = 'pr-linter/exempt-readme'; const EXEMPT_TEST = 'pr-linter/exempt-test'; +const EXEMPT_INTEG_TEST = 'pr-linter/exempt-integ-test'; const EXEMPT_BREAKING_CHANGE = 'pr-linter/exempt-breaking-change'; class LinterError extends Error { @@ -43,6 +44,10 @@ function testChanged(files: any[]) { return files.filter(f => f.filename.toLowerCase().includes("test")).length != 0; } +function integTestChanged(files: any[]) { + return files.filter(f => f.filename.toLowerCase().match(/^integ.*.ts$/)).length != 0; +} + function readmeChanged(files: any[]) { return files.filter(f => path.basename(f.filename) == "README.md").length != 0; } @@ -65,6 +70,12 @@ function fixContainsTest(issue: any, files: any[]) { } }; +function featureContainsIntegTest(issue: any, files: any[]) { + if (isFeature(issue) && !integTestChanged(files)) { + throw new LinterError("Features must contain a change to an integration test file"); + } +}; + function shouldExemptReadme(issue: any) { return hasLabel(issue, EXEMPT_README); } @@ -73,6 +84,10 @@ function shouldExemptTest(issue: any) { return hasLabel(issue, EXEMPT_TEST); } +function shouldExemptIntegTest(issue: any) { + return hasLabel(issue, EXEMPT_INTEG_TEST); +} + function shouldExemptBreakingChange(issue: any) { return hasLabel(issue, EXEMPT_BREAKING_CHANGE); } @@ -147,6 +162,12 @@ export async function validatePr(number: number) { featureContainsTest(issue, files); fixContainsTest(issue, files); } + + if (shouldExemptIntegTest(issue)) { + console.log(`Not validating integration test changes since the PR is labeled with '${EXEMPT_INTEG_TEST}'`) + } else { + featureContainsIntegTest(issue, files); + } validateBreakingChangeFormat(issue.title, issue.body); if (shouldExemptBreakingChange(issue)) { @@ -156,7 +177,6 @@ export async function validatePr(number: number) { } console.log("✅ Success"); - } require('make-runnable/custom')({ diff --git a/tools/@aws-cdk/prlint/test/lint.test.ts b/tools/@aws-cdk/prlint/test/lint.test.ts index 8b5938e392791..c77b1d05470a1 100644 --- a/tools/@aws-cdk/prlint/test/lint.test.ts +++ b/tools/@aws-cdk/prlint/test/lint.test.ts @@ -19,6 +19,7 @@ afterAll(() => { describe('breaking changes format', () => { test('disallow variations to "BREAKING CHANGE:"', async () => { const issue = { + title: 'chore: some title', body: 'BREAKING CHANGES:', labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-readme' }] }; @@ -28,6 +29,7 @@ describe('breaking changes format', () => { test('the first breaking change should immediately follow "BREAKING CHANGE:"', async () => { const issue = { + title: 'chore: some title', body: `BREAKING CHANGE:\x20 * **module:** another change`, labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-readme' }] @@ -112,6 +114,99 @@ describe('ban breaking changes in stable modules', () => { }); }); +describe('integration tests required on features', () => { + test('integ files changed', async () => { + const issue = { + title: 'feat(s3): some title', + body: ` + description of the commit + + closes #123456789 + `, + labels: [] + }; + const files = [ + { + filename: 'integ.some-integ-test.ts' + }, + { + filename: 'README.md' + } + ]; + configureMock(issue, files) + expect(await linter.validatePr(1000)).resolves; + }); + + test('integ files not changed', async () => { + const issue = { + title: 'feat(s3): some title', + body: ` + description of the commit + + closes #123456789 + `, + labels: [] + }; + const files = [ + { + filename: 'some-test.test.ts' + }, + { + filename: 'integ.some-test.expected.json' + }, + { + filename: 'README.md' + } + ]; + configureMock(issue, files) + await expect(linter.validatePr(1000)).rejects.toThrow('Features must contain a change to an integration test file'); + }); + + test('integ files not changed, pr exempt', async () => { + const issue = { + title: 'feat(s3): some title', + body: ` + description of the commit + + closes #123456789 + `, + labels: [{ name: 'pr-linter/exempt-integ-test' }] + }; + const files = [ + { + filename: 'some-test.test.ts' + }, + { + filename: 'README.md' + } + ]; + configureMock(issue, files) + expect(await linter.validatePr(1000)).resolves; + }); + + test('integ files not changed, not a feature', async () => { + const issue = { + title: 'fix(s3): some title', + body: ` + description of the commit + + closes #123456789 + `, + labels: [] + }; + const files = [ + { + filename: 'some-test.test.ts' + }, + { + filename: 'README.md' + } + ]; + configureMock(issue, files) + expect(await linter.validatePr(1000)).resolves; + }); +}); + function configureMock(issue: any, prFiles: any[] | undefined) { GitHub.mockImplementation(() => { return { From da388a53ffe1bb643eb4b7271d35375aa832f653 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 21 Mar 2022 11:59:34 +0000 Subject: [PATCH 3/7] chore(deps): Bump actions/cache from 2.1.7 to 3 (#19488) Bumps [actions/cache](https://github.com/actions/cache) from 2.1.7 to 3.
Release notes

Sourced from actions/cache's releases.

v3.0.0

  • This change adds a minimum runner version(node12 -> node16), which can break users using an out-of-date/fork of the runner. This would be most commonly affecting users on GHES 3.3 or before, as those runners do not support node16 actions and they can use actions from github.com via github connect or manually copying the repo to their GHES instance.

  • Few dependencies and cache action usage examples have also been updated.

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions/cache&package-manager=github_actions&previous-version=2.1.7&new-version=3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
--- .github/workflows/yarn-upgrade.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/yarn-upgrade.yml b/.github/workflows/yarn-upgrade.yml index 5c72ff4e4abf4..232a72fbe01df 100644 --- a/.github/workflows/yarn-upgrade.yml +++ b/.github/workflows/yarn-upgrade.yml @@ -27,7 +27,7 @@ jobs: run: echo "::set-output name=dir::$(yarn cache dir)" - name: Restore Yarn cache - uses: actions/cache@v2.1.7 + uses: actions/cache@v3 with: path: ${{ steps.yarn-cache.outputs.dir }} key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} From 73c6a1b26b75befc315cc4741490a30426ee8d6a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 21 Mar 2022 12:44:04 +0000 Subject: [PATCH 4/7] chore(deps): Bump awscli from 1.22.73 to 1.22.77 in /packages/@aws-cdk/lambda-layer-awscli (#19489) Bumps [awscli](https://github.com/aws/aws-cli) from 1.22.73 to 1.22.77.
Changelog

Sourced from awscli's changelog.

1.22.77

  • api-change:glue: Added 9 new APIs for AWS Glue Interactive Sessions: ListSessions, StopSession, CreateSession, GetSession, DeleteSession, RunStatement, GetStatement, ListStatements, CancelStatement

1.22.76

  • api-change:billingconductor: This is the initial SDK release for AWS Billing Conductor. The AWS Billing Conductor is a customizable billing service, allowing you to customize your billing data to match your desired business structure.
  • api-change:acm-pca: AWS Certificate Manager (ACM) Private Certificate Authority (CA) now supports customizable certificate subject names and extensions.
  • api-change:amplifybackend: Adding the ability to customize Cognito verification messages for email and SMS in CreateBackendAuth and UpdateBackendAuth. Adding deprecation documentation for ForgotPassword in CreateBackendAuth and UpdateBackendAuth
  • api-change:ssm-incidents: Removed incorrect validation pattern for IncidentRecordSource.invokedBy
  • api-change:s3outposts: S3 on Outposts is releasing a new API, ListSharedEndpoints, that lists all endpoints associated with S3 on Outpost, that has been shared by Resource Access Manager (RAM).

1.22.75

  • api-change:ec2: Adds the Cascade parameter to the DeleteIpam API. Customers can use this parameter to automatically delete their IPAM, including non-default scopes, pools, cidrs, and allocations. There mustn't be any pools provisioned in the default public scope to use this parameter.
  • api-change:rds: Various documentation improvements
  • api-change:dataexchange: This feature enables data providers to use the RevokeRevision operation to revoke subscriber access to a given revision. Subscribers are unable to interact with assets within a revoked revision.
  • api-change:robomaker: This release deprecates ROS, Ubuntu and Gazbeo from RoboMaker Simulation Service Software Suites in favor of user-supplied containers and Relaxed Software Suites.
  • api-change:location: New HERE style "VectorHereExplore" and "VectorHereExploreTruck".
  • api-change:cognito-idp: Updated EmailConfigurationType and SmsConfigurationType to reflect that you can now choose Amazon SES and Amazon SNS resources in the same Region.
  • api-change:keyspaces: Fixing formatting issues in CLI and SDK documentation
  • api-change:ecs: Documentation only update to address tickets

1.22.74

  • api-change:kendra: Amazon Kendra now provides a data source connector for Slack. For more information, see https://docs.aws.amazon.com/kendra/latest/dg/data-source-slack.html
  • api-change:config: Add resourceType enums for AWS::ECR::PublicRepository and AWS::EC2::LaunchTemplate
  • api-change:timestream-query: Amazon Timestream Scheduled Queries now support Timestamp datatype in a multi-measure record.
  • api-change:elasticache: Doc only update for ElastiCache
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=awscli&package-manager=pip&previous-version=1.22.73&new-version=1.22.77)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
--- packages/@aws-cdk/lambda-layer-awscli/layer/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/lambda-layer-awscli/layer/requirements.txt b/packages/@aws-cdk/lambda-layer-awscli/layer/requirements.txt index a0e7b8f872bb9..552b312f0edbb 100644 --- a/packages/@aws-cdk/lambda-layer-awscli/layer/requirements.txt +++ b/packages/@aws-cdk/lambda-layer-awscli/layer/requirements.txt @@ -1 +1 @@ -awscli==1.22.73 +awscli==1.22.77 From 8dd93a8936ddc74b799e0c6dafbe0511ee687e71 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 21 Mar 2022 14:27:10 +0100 Subject: [PATCH 5/7] chore(cli): integ test filtering is broken (#19462) Turns out that Jest's `test.concurrent()` does not play well when running `jest -t TESTNAME`: jest will actually start all tests, not just the one you were trying to run. Reported here: https://github.com/facebook/jest/issues/12588 We used to have something to opt-out of concurrent running: we have to flip that around, and opt in to concurrent running only for the canary and pipeline tests, where we *know* for a fact we're not filtering. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cdk/test/integ/helpers/test-helpers.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/test/integ/helpers/test-helpers.ts b/packages/aws-cdk/test/integ/helpers/test-helpers.ts index 8681e16c16499..e74d7d000a777 100644 --- a/packages/aws-cdk/test/integ/helpers/test-helpers.ts +++ b/packages/aws-cdk/test/integ/helpers/test-helpers.ts @@ -15,14 +15,18 @@ export function integTest( timeoutMillis?: number, ) { - // Integ tests can run concurrently, and are responsible for blocking themselves if they cannot. - // Because `test.concurrent` executes the test code immediately (to obtain a promise), we allow - // setting the `JEST_TEST_CONCURRENT` environment variable to 'false' in order to use `test` - // instead of `test.concurrent` (this is necessary when specifying a test pattern to verify). - const testKind = process.env.JEST_TEST_CONCURRENT === 'false' ? test : test.concurrent; + // Integ tests can run concurrently, and are responsible for blocking + // themselves if they cannot. Because `test.concurrent` executes the test + // code immediately, regardles of any `--testNamePattern`, this cannot be the + // default: test filtering simply does not work with `test.concurrent`. + // Instead, we make it opt-in only for the pipeline where we don't do any + // selection, but execute all tests unconditionally. + const testKind = process.env.JEST_TEST_CONCURRENT === 'true' ? test.concurrent : test; const runner = shouldSkip(name) ? testKind.skip : testKind; runner(name, async () => { + // eslint-disable-next-line no-console + console.log(`running test ${name} using ${runner.name}`); const output = new MemoryStream(); output.write('================================================================\n'); From 5ec86d0563864a184f4b5e42990a628370c285c9 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 21 Mar 2022 15:09:35 +0100 Subject: [PATCH 6/7] docs(aws-custom-resource): explain deviating policy names (#19458) Some API calls do not require the exact corresponding IAM permissions. We don't have a mapping to do this properly, so we have to punt to the user (and have to make them aware of this). Relates to #19355. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/custom-resources/README.md | 18 ++++++++++++++---- .../aws-custom-resource/aws-custom-resource.ts | 7 +++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/custom-resources/README.md b/packages/@aws-cdk/custom-resources/README.md index cb30aada65d7f..8bfcd02d5a411 100644 --- a/packages/@aws-cdk/custom-resources/README.md +++ b/packages/@aws-cdk/custom-resources/README.md @@ -354,7 +354,7 @@ This sample demonstrates the following concepts: ### Customizing Provider Function name -In multi-account environments or when the custom resource may be re-utilized across several +In multi-account environments or when the custom resource may be re-utilized across several stacks it may be useful to manually set a name for the Provider Function Lambda and therefore have a predefined service token ARN. @@ -401,9 +401,19 @@ the `installLatestAwsSdk` prop to `false`. You must provide the `policy` property defining the IAM Policy that will be applied to the API calls. The library provides two factory methods to quickly configure this: -* **`AwsCustomResourcePolicy.fromSdkCalls`** - Use this to auto-generate IAM Policy statements based on the configured SDK calls. -Note that you will have to either provide specific ARN's, or explicitly use `AwsCustomResourcePolicy.ANY_RESOURCE` to allow access to any resource. -* **`AwsCustomResourcePolicy.fromStatements`** - Use this to specify your own custom statements. +* **`AwsCustomResourcePolicy.fromSdkCalls`** - Use this to auto-generate IAM + Policy statements based on the configured SDK calls. Keep two things in mind + when using this policy: + * This policy variant assumes the IAM policy name has the same name as the API + call. This is true in 99% of cases, but there are exceptions (for example, + S3's `PutBucketLifecycleConfiguration` requires + `s3:PutLifecycleConfiguration` permissions, Lambda's `Invoke` requires + `lambda:InvokeFunction` permissions). Use `fromStatements` if you want to + do a call that requires different IAM action names. + * You will have to either provide specific ARNs, or explicitly use + `AwsCustomResourcePolicy.ANY_RESOURCE` to allow access to any resource. +* **`AwsCustomResourcePolicy.fromStatements`** - Use this to specify your own + custom statements. The custom resource also implements `iam.IGrantable`, making it possible to use the `grantXxx()` methods. diff --git a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts index 2e68b899cdcab..ccac23249940a 100644 --- a/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts +++ b/packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts @@ -199,6 +199,13 @@ export class AwsCustomResourcePolicy { * * Each SDK call with be translated to an IAM Policy Statement in the form of: `call.service:call.action` (e.g `s3:PutObject`). * + * This policy generator assumes the IAM policy name has the same name as the API + * call. This is true in 99% of cases, but there are exceptions (for example, + * S3's `PutBucketLifecycleConfiguration` requires + * `s3:PutLifecycleConfiguration` permissions, Lambda's `Invoke` requires + * `lambda:InvokeFunction` permissions). Use `fromStatements` if you want to + * do a call that requires different IAM action names. + * * @param options options for the policy generation */ public static fromSdkCalls(options: SdkCallsPolicyOptions) { From 01b538eb773f93d16d46b5c7f368ac83ecd8042b Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Mon, 21 Mar 2022 10:52:20 -0400 Subject: [PATCH 7/7] docs: add integration testing guide (#19469) This adds a new integration testing guide to help contributors better understand how CDK integration testing works and under what scenarios integration tests should be added. This is not meant to be a comprehensive guide and can serve as a starting point for us to add additional scenarios, etc. This also adds a checklist item to the pull request template for contributors to indicate that their new feature contains a corresponding integration test and that they have followed the integration test guide. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](../CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * ~[ ] This PR adds new unconventional dependencies following the process described [here](../CONTRIBUTING.md/#adding-new-unconventional-dependencies)~ *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .github/PULL_REQUEST_TEMPLATE.md | 9 +- CONTRIBUTING.md | 24 +++- INTEGRATION_TESTS.md | 225 +++++++++++++++++++++++++++++++ 3 files changed, 253 insertions(+), 5 deletions(-) create mode 100644 INTEGRATION_TESTS.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9f68b4fbc0156..639add0db8a3a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -3,10 +3,15 @@ ### All Submissions: -* [ ] Have you followed the guidelines in our [Contributing guide?](../CONTRIBUTING.md) +* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: -* [ ] This PR adds new unconventional dependencies following the process described [here](../CONTRIBUTING.md/#adding-new-unconventional-dependencies) +* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) + +### New Features + +* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? + * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8797024d1c1c6..c769e0f191e1c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -234,9 +234,17 @@ Integration tests perform a few functions in the CDK code base - 3. (Optionally) Acts as a way to validate that constructs set up the CloudFormation resources as expected. A successful CloudFormation deployment does not mean that the resources are set up correctly. -If you are working on a new feature that is using previously unused CloudFormation resource types, or involves -configuring resource types across services, you need to write integration tests that use these resource types or -features. +**When are integration tests required?** + +The following list contains common scenarios where we _know_ that integration tests are required. +This is not an exhaustive list and we will, by default, require integration tests for all +new features unless there is a good reason why one is not needed. + +1. Adding a new feature that is using previously unused CloudFormation resource types +2. Adding a new feature that is using previously unused (or untested) CloudFormation properties +3. Involves configuring resource types across services (i.e. integrations) +4. Adding a new supported version (e.g. a new [AuroraMysqlEngineVersion](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_rds.AuroraMysqlEngineVersion.html)) +5. Adding any functionality via a [Custom Resource](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources-readme.html) To the extent possible, include a section (like below) in the integration test file that specifies how the successfully deployed stack can be verified for correctness. Correctness here implies that the resources have been set up correctly. @@ -254,6 +262,16 @@ Examples: * [integ.destinations.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-lambda-destinations/test/integ.destinations.ts#L7) * [integ.token-authorizer.lit.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-apigateway/test/authorizers/integ.token-authorizer.lit.ts#L7-L12) +**What do do if you cannot run integration tests** + +If you are working on a PR that requires an update to an integration test and you are unable +to run the `cdk-integ` tool to perform a real deployment, please call this out on the pull request +so a maintainer can run the tests for you. Please **do not** run the `cdk-integ` tool with `--dry-run` +or manually update the snapshot. + +See the [integration test guide](./INTEGRATION_TESTS.md) for a more complete guide on running +CDK integration tests. + #### yarn watch (Optional) We've added a watch feature to the CDK that builds your code as you type it. Start this by running `yarn watch` for diff --git a/INTEGRATION_TESTS.md b/INTEGRATION_TESTS.md new file mode 100644 index 0000000000000..3f21037d95aa2 --- /dev/null +++ b/INTEGRATION_TESTS.md @@ -0,0 +1,225 @@ +# Integration Tests + +This document describes the purpose of integration tests as well as acting as a guide +on what type of changes require integrations tests and how you should write integration tests. + +- [What are CDK Integration Tests](#what-are-cdk-integration-tests) +- [When are integration tests required](#when-are-integration-tests-required) +- [How to write Integration Tests](#how-to-write-integration-tests) + - [Creating a test](#creating-a-test) + - [New L2 Constructs](#new-l2-constructs) + - [Existing L2 Constructs](#existing-l2-constructs) + - [Assertions](#assertions) + +## What are CDK Integration Tests + +All Construct libraries in the CDK code base have integration tests that serve to - + +1. Acts as a regression detector. It does this by running `cdk synth` on the integration test and comparing it against + the `*.expected.json` file. This highlights how a change affects the synthesized stacks. +2. Allows for a way to verify if the stacks are still valid CloudFormation templates, as part of an intrusive change. + This is done by running `yarn integ` which will run `cdk deploy` across all of the integration tests in that package. + If you are developing a new integration test or for some other reason want to work on a single integration test + over and over again without running through all the integration tests you can do so using + `yarn integ integ.test-name.js` .Remember to set up AWS credentials before doing this. +3. (Optionally) Acts as a way to validate that constructs set up the CloudFormation resources as expected. + A successful CloudFormation deployment does not mean that the resources are set up correctly. + + +## When are Integration Tests Required + +The following list contains common scenarios where we _know_ that integration tests are required. +This is not an exhaustive list and we will, by default, require integration tests for all +new features unless there is a good reason why one is not needed. + +**1. Adding a new feature that is using previously unused CloudFormation resource types** +For example, adding a new L2 construct for an L1 resource. There should be a new integration test +to test that the new L2 successfully creates the resources in AWS. + +**2. Adding a new feature that is using previously unused (or untested) CloudFormation properties** +For example, there is an existing L2 construct for a CloudFormation resource and you are adding +support for a new property. This could be either a new property that has been added to CloudFormation +or an existing property that the CDK did not have coverage for. You should either update and existing +integration test to cover this new property or create a new test. + +Sometimes the CloudFormation documentation is incorrect or unclear on the correct way to configure +a property. This can lead to introducing new features that don't actually work. Creating +an integration test for the new feature can ensure that it works and avoid unnecessary bugs. + +**3. Involves configuring resource types across services (i.e. integrations)** +For example, you are adding functionality that allows for service x to integrate with service y. +A good example of this is the [aws-stepfunctions-tasks](./packages/@aws-cdk/aws-stepfunctions-tasks) or +[aws-apigatewayv2-integrations](./packages/@aws-cdk/aws-apigatewayv2-integrations) modules. Both of these +have L2 constructs that provide functionality to integrate services. + +Sometimes these integrations involve configuring/formatting json/vtl or some other type of data. +For these types of features it is important to create an integration test that not only validates +that the infrastructure deploys successfully, but that the intended functionality works. This could +mean deploying the integration test and then manually making an HTTP request or invoking a Lambda function. + +**4. Adding a new supported version (e.g. a new [AuroraMysqlEngineVersion](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_rds.AuroraMysqlEngineVersion.html))** +Sometimes new versions introduce new CloudFormation properties or new required configuration. +For example Aurora MySQL version 8 introduced a new parameter and was not compatible with the +existing parameter (see [#19145](https://github.com/aws/aws-cdk/pull/19145)). + +**5. Adding any functionality via a [Custom Resource](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources-readme.html)** +Custom resources involve non-standard functionality and are at a higher risk of introducing bugs. + +## How to write Integration Tests + +This section will detail how to write integration tests, how they are executed and how to ensure +you have good test coverage. + +### Creating a Test + +An integration tests is any file located in the `test/` directory that has a name that starts with `integ.` +(e.g. `integ.*.ts`). + +To create a new integration test, first create a new file, for example `integ.my-new-construct.ts`. +The contents of this file should be a CDK app. For example, a very simple integration test for a +Lambda Function would look like this: + +_integ.lambda.ts_ +```ts +import * as iam from '@aws-cdk/aws-iam'; +import * as cdk from '@aws-cdk/core'; +import * as lambda from '../lib'; + +const app = new cdk.App(); + +const stack = new cdk.Stack(app, 'aws-cdk-lambda-1'); + +const fn = new lambda.Function(stack, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'index.handler', + runtime: lambda.Runtime.NODEJS_10_X, +}); + +app.synth(); +``` + +To run the test you would run: + +*Note - filename must be `*.js`* +``` +npm run cdk-integ integ.lambda.js +``` + +This will: +1. Synthesize the CDK app +2. `cdk deploy` to your AWS account +3. `cdk destroy` to delete the stack +4. Save a snapshot of the synthed CloudFormation template to `integ.lambda.expected.json` + +Now when you run `npm test` it will synth the integ app and compare the result with the snapshot. +If the snapshot has changed the same process must be followed to update the snapshot. + +### New L2 Constructs + +When creating a new L2 construct (or new construct library) it is important to ensure you have a good +coverage base from which future contributions can build on. + +Some general rules to follow are: + +- **1 test with all default values** +One test for each L2 that only populates the required properties. For a Lambda Function this would look like: + +```ts +new lambda.Function(this, 'Handler', { + code, + handler, + runtime, +}); +``` + +- **1 test with all values provided** +One test for each L2 that populates non-default properties. Some of this will come down to judgement, but this should +be based on major functionality. For example, when testing a Lambda Function there are 37 (*at the time of this writing) different +input parameters. Some of these can be tested together and don't represent large pieces of functionality, +while others do. + +For example, the test for a Lambda Function might look like this. For most of these properties we are probably fine +testing them together and just testing one of their values. For example we don't gain much by testing a bunch of +different `memorySize` settings, as long as we test that we can `set` the memorySize then we should be good. + +```ts +new lambda.Function(this, 'Handler', { + code, + handler, + runtime, + architecture, + description, + environment, + environmentEncryption, + functionName, + initialPolicy, + insightsVersion, + layers, + maxEventAge, + memorySize, + reservedConcurrentExecutions, + retryAttempts, + role, + timeout, + tracing, +}); +``` + +Other parameters might represent larger pieces of functionality and might create other resources for us or configure +integrations with other services. For these it might make sense to split them out into separate tests so it is easier +to reason about them. + +A couple of examples would be +(you could also mix in different configurations of the above parameters with each of these): + +_testing filesystems_ +```ts +new lambda.Function(this, 'Handler', { + filesystem, +}); +``` + +_testing event sources_ +```ts +new lambda.Function(this, 'Handler', { + events, +}); +``` + +_testing VPCs_ +```ts +new lambda.Function(this, 'Handler', { + securityGroups, + vpc, + vpcSubnets, +}); +``` + +### Existing L2 Constructs + +Updating an existing L2 Construct could consist of: + +1. **Adding coverage for a new (or previously uncovered) CloudFormation property.** +In this case you would want to either add this new property to an existing integration test or create a new +integration test. A new integration test is preferred for larger update (e.g. adding VPC connectivity, etc). + +2. **Updating functionality for an existing property.** +In this case you should first check if you are already covered by an existing integration test. If not, then you would follow the +same process as adding new coverage. + +3. **Changing functionality that affects asset bundling** +Some constructs deal with asset bundling (i.e. `aws-lambda-nodejs`, `aws-lambda-python`, etc). There are some updates that may not +touch any CloudFormation property, but instead change the way that code is bundled. While these types of changes may not require +a change to an integration test, you need to make sure that the integration tests and assertions are rerun. + +An example of this would be making a change to the way `aws-lambda-nodejs` bundles Lambda code. A couple of things could go wrong that would +only be caught by rerunning the integration tests. + +1. The bundling commands are only running when performing a real synth (not part of unit tests). Running the integration test confirms +that the actual bundling was not broken. +2. When deploying Lambda Functions, CloudFormation will only update the Function configuration with the new code, +but it will not validate that the Lambda function can be invoked. Because of this, it is important to rerun the integration test +to deploy the Lambda Function _and_ then rerun the assertions to ensure that the function can still be invoked. + +### Assertions +...Coming soon...