Skip to content

Commit

Permalink
chore: update unit tests and assertions to support feature flag expir…
Browse files Browse the repository at this point in the history
…y in CDKv2 (#13217)

In CDKv2, we will be expiring the feature flags - 
'@aws-cdk/aws-ecr-assets:dockerIgnoreSupport' and 
'@aws-cdk/aws-s3:grantWriteWithoutAcl'.

Tests across the CDK code base explicitly or implicitly are verifying
the behaviour of these modules that are affected by these feature flags. 

Update the unit tests appropriately so that they explicitly depend on
one of the two behaviours, or test both.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Niranjan Jayakar authored Feb 24, 2021
1 parent 6318e26 commit 6d6aa0c
Show file tree
Hide file tree
Showing 25 changed files with 124 additions and 95 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-codepipeline-actions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"devDependencies": {
"@aws-cdk/assert": "0.0.0",
"@aws-cdk/aws-cloudtrail": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@types/lodash": "^4.14.168",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import * as lambda from '@aws-cdk/aws-lambda';
import * as s3 from '@aws-cdk/aws-s3';
import * as sns from '@aws-cdk/aws-sns';
import { App, Aws, Lazy, SecretValue, Stack, Token } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag';
import * as cpactions from '../../lib';

/* eslint-disable quote-props */

const s3GrantWriteCtx = { '@aws-cdk/aws-s3:grantWriteWithoutAcl': true };
const s3GrantWriteCtx = { [cxapi.S3_GRANT_WRITE_WITHOUT_ACL]: true };

describe('', () => {
describe('Lambda invoke Action', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import '@aws-cdk/assert/jest';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as s3 from '@aws-cdk/aws-s3';
import { App, Duration, SecretValue, Stack } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag';
import * as cpactions from '../../lib';

Expand Down Expand Up @@ -48,7 +49,7 @@ describe('', () => {

});

testFutureBehavior('grant the pipeline correct access to the target bucket', { '@aws-cdk/aws-s3:grantWriteWithoutAcl': true }, App, (app) => {
testFutureBehavior('grant the pipeline correct access to the target bucket', { [cxapi.S3_GRANT_WRITE_WITHOUT_ACL]: true }, App, (app) => {
const stack = new Stack(app);
minimalPipeline(stack);

Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ec2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/assert": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-ec2/test/volume.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior, testLegacyBehavior } from 'cdk-build-tools/lib/feature-flag';
import {
AmazonLinuxGeneration,
Expand Down Expand Up @@ -575,7 +576,7 @@ describe('volume', () => {

});

testFutureBehavior('with future flag aws-kms:defaultKeyPolicies', { '@aws-cdk/aws-kms:defaultKeyPolicies': true }, cdk.App, (app) => {
testFutureBehavior('with future flag aws-kms:defaultKeyPolicies', { [cxapi.KMS_DEFAULT_KEY_POLICIES]: true }, cdk.App, (app) => {
// GIVEN
const stack = new cdk.Stack(app);
const role = new Role(stack, 'Role', { assumedBy: new AccountRootPrincipal() });
Expand Down
68 changes: 37 additions & 31 deletions packages/@aws-cdk/aws-ecr-assets/test/image-asset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,19 @@ import * as iam from '@aws-cdk/aws-iam';
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import { App, DefaultStackSynthesizer, IgnoreMode, Lazy, LegacyStackSynthesizer, Stack, Stage } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag';
import { DockerImageAsset } from '../lib';

/* eslint-disable quote-props */

const DEMO_IMAGE_ASSET_HASH = 'b2c69bfbfe983b634456574587443159b3b7258849856a118ad3d2772238f1a5';


let app: App;
let stack: Stack;
beforeEach(() => {
app = new App({
context: {
'@aws-cdk/aws-ecr-assets:dockerIgnoreSupport': true,
},
});
stack = new Stack(app, 'Stack');
});
const flags = { [cxapi.DOCKER_IGNORE_SUPPORT]: true };

describe('image asset', () => {
test('test instantiating Asset Image', () => {
testFutureBehavior('test instantiating Asset Image', flags, App, (app) => {
// WHEN
const stack = new Stack(app);
new DockerImageAsset(stack, 'Image', {
directory: path.join(__dirname, 'demo-image'),
});
Expand All @@ -47,8 +39,9 @@ describe('image asset', () => {

});

test('with build args', () => {
testFutureBehavior('with build args', flags, App, (app) => {
// WHEN
const stack = new Stack(app);
new DockerImageAsset(stack, 'Image', {
directory: path.join(__dirname, 'demo-image'),
buildArgs: {
Expand All @@ -62,8 +55,9 @@ describe('image asset', () => {

});

test('with target', () => {
testFutureBehavior('with target', flags, App, (app) => {
// WHEN
const stack = new Stack(app);
new DockerImageAsset(stack, 'Image', {
directory: path.join(__dirname, 'demo-image'),
buildArgs: {
Expand All @@ -78,8 +72,9 @@ describe('image asset', () => {

});

test('with file', () => {
testFutureBehavior('with file', flags, App, (app) => {
// GIVEN
const stack = new Stack(app);
const directoryPath = path.join(__dirname, 'demo-image-custom-docker-file');
// WHEN
new DockerImageAsset(stack, 'Image', {
Expand All @@ -93,8 +88,9 @@ describe('image asset', () => {

});

test('asset.repository.grantPull can be used to grant a principal permissions to use the image', () => {
testFutureBehavior('asset.repository.grantPull can be used to grant a principal permissions to use the image', flags, App, (app) => {
// GIVEN
const stack = new Stack(app);
const user = new iam.User(stack, 'MyUser');
const asset = new DockerImageAsset(stack, 'Image', {
directory: path.join(__dirname, 'demo-image'),
Expand Down Expand Up @@ -155,6 +151,7 @@ describe('image asset', () => {
});

test('fails if the directory does not exist', () => {
const stack = new Stack();
// THEN
expect(() => {
new DockerImageAsset(stack, 'MyAsset', {
Expand All @@ -165,6 +162,7 @@ describe('image asset', () => {
});

test('fails if the directory does not contain a Dockerfile', () => {
const stack = new Stack();
// THEN
expect(() => {
new DockerImageAsset(stack, 'Asset', {
Expand All @@ -175,6 +173,7 @@ describe('image asset', () => {
});

test('fails if the file does not exist', () => {
const stack = new Stack();
// THEN
expect(() => {
new DockerImageAsset(stack, 'Asset', {
Expand All @@ -185,7 +184,8 @@ describe('image asset', () => {

});

test('docker directory is staged if asset staging is enabled', () => {
testFutureBehavior('docker directory is staged if asset staging is enabled', flags, App, (app) => {
const stack = new Stack(app);
const image = new DockerImageAsset(stack, 'MyAsset', {
directory: path.join(__dirname, 'demo-image'),
});
Expand All @@ -197,15 +197,16 @@ describe('image asset', () => {

});

test('docker directory is staged without files specified in .dockerignore', () => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore();
testFutureBehavior('docker directory is staged without files specified in .dockerignore', flags, App, (app) => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(app);
});

test('docker directory is staged without files specified in .dockerignore with IgnoreMode.GLOB', () => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(IgnoreMode.GLOB);
testFutureBehavior('docker directory is staged without files specified in .dockerignore with IgnoreMode.GLOB', flags, App, (app) => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(app, IgnoreMode.GLOB);
});

test('docker directory is staged with whitelisted files specified in .dockerignore', () => {
testFutureBehavior('docker directory is staged with whitelisted files specified in .dockerignore', flags, App, (app) => {
const stack = new Stack(app);
const image = new DockerImageAsset(stack, 'MyAsset', {
directory: path.join(__dirname, 'whitelisted-image'),
});
Expand All @@ -227,16 +228,17 @@ describe('image asset', () => {

});

test('docker directory is staged without files specified in exclude option', () => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption();
testFutureBehavior('docker directory is staged without files specified in exclude option', flags, App, (app) => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(app);
});

test('docker directory is staged without files specified in exclude option with IgnoreMode.GLOB', () => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(IgnoreMode.GLOB);
testFutureBehavior('docker directory is staged without files specified in exclude option with IgnoreMode.GLOB', flags, App, (app) => {
testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(app, IgnoreMode.GLOB);
});

test('fails if using tokens in build args keys or values', () => {
// GIVEN
const stack = new Stack();
const token = Lazy.string({ produce: () => 'foo' });
const expected = /Cannot use tokens in keys or values of "buildArgs" since they are needed before deployment/;

Expand All @@ -256,6 +258,7 @@ describe('image asset', () => {

test('fails if using token as repositoryName', () => {
// GIVEN
const stack = new Stack();
const token = Lazy.string({ produce: () => 'foo' });

// THEN
Expand All @@ -267,8 +270,9 @@ describe('image asset', () => {

});

test('docker build options are included in the asset id', () => {
testFutureBehavior('docker build options are included in the asset id', flags, App, (app) => {
// GIVEN
const stack = new Stack(app);
const directory = path.join(__dirname, 'demo-image-custom-docker-file');

const asset1 = new DockerImageAsset(stack, 'Asset1', { directory });
Expand All @@ -290,7 +294,8 @@ describe('image asset', () => {
});
});

function testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(ignoreMode?: IgnoreMode) {
function testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(app: App, ignoreMode?: IgnoreMode) {
const stack = new Stack(app);
const image = new DockerImageAsset(stack, 'MyAsset', {
ignoreMode,
directory: path.join(__dirname, 'dockerignore-image'),
Expand All @@ -309,7 +314,8 @@ function testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(ignoreMo

}

function testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(ignoreMode?: IgnoreMode) {
function testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(app: App, ignoreMode?: IgnoreMode) {
const stack = new Stack(app);
const image = new DockerImageAsset(stack, 'MyAsset', {
directory: path.join(__dirname, 'dockerignore-image'),
exclude: ['subdirectory'],
Expand All @@ -328,7 +334,7 @@ function testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(ignoreM

}

test('nested assemblies share assets: legacy synth edition', () => {
testFutureBehavior('nested assemblies share assets: legacy synth edition', flags, App, (app) => {
// GIVEN
const stack1 = new Stack(new Stage(app, 'Stage1'), 'Stack', { synthesizer: new LegacyStackSynthesizer() });
const stack2 = new Stack(new Stage(app, 'Stage2'), 'Stack', { synthesizer: new LegacyStackSynthesizer() });
Expand All @@ -354,7 +360,7 @@ test('nested assemblies share assets: legacy synth edition', () => {
}
});

test('nested assemblies share assets: default synth edition', () => {
testFutureBehavior('nested assemblies share assets: default synth edition', flags, App, (app) => {
// GIVEN
const stack1 = new Stack(new Stage(app, 'Stage1'), 'Stack', { synthesizer: new DefaultStackSynthesizer() });
const stack2 = new Stack(new Stage(app, 'Stage2'), 'Stack', { synthesizer: new DefaultStackSynthesizer() });
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-ecr-assets/test/integ.assets-docker.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import * as path from 'path';
import * as iam from '@aws-cdk/aws-iam';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import * as assets from '../lib';

const app = new cdk.App({
context: {
'@aws-cdk/aws-ecr-assets:dockerIgnoreSupport': true,
[cxapi.DOCKER_IGNORE_SUPPORT]: true,
},
});
const stack = new cdk.Stack(app, 'integ-assets-docker');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as path from 'path';
import * as iam from '@aws-cdk/aws-iam';
import { App, CfnOutput, NestedStack, NestedStackProps, Stack, StackProps } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import * as ecr_assets from '../lib';

Expand Down Expand Up @@ -29,7 +30,7 @@ class TheParentStack extends Stack {

const app = new App({
context: {
'@aws-cdk/aws-ecr-assets:dockerIgnoreSupport': true,
[cxapi.DOCKER_IGNORE_SUPPORT]: true,
},
});
new TheParentStack(app, 'nested-stacks-docker');
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ecs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"devDependencies": {
"@aws-cdk/assert": "0.0.0",
"@aws-cdk/aws-s3-deployment": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@types/nodeunit": "^0.0.31",
"@types/proxyquire": "^1.3.28",
"cdk-build-tools": "0.0.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-ecs/test/container-definition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as s3 from '@aws-cdk/aws-s3';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import * as ssm from '@aws-cdk/aws-ssm';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag';
import * as ecs from '../lib';

Expand Down Expand Up @@ -1643,7 +1644,7 @@ describe('container definition', () => {
});
});

testFutureBehavior('can use a DockerImageAsset directly for a container image', { '@aws-cdk/aws-ecr-assets:dockerIgnoreSupport': true }, cdk.App, (app) => {
testFutureBehavior('can use a DockerImageAsset directly for a container image', { [cxapi.DOCKER_IGNORE_SUPPORT]: true }, cdk.App, (app) => {
// GIVEN
const stack = new cdk.Stack(app, 'Stack');
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as iam from '@aws-cdk/aws-iam';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import * as ssm from '@aws-cdk/aws-ssm';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag';
import * as ecs from '../../lib';

Expand Down Expand Up @@ -585,7 +586,7 @@ describe('ec2 task definition', () => {

});

testFutureBehavior('correctly sets containers from asset using default props', { '@aws-cdk/aws-ecr-assets:dockerIgnoreSupport': true }, cdk.App, (app) => {
testFutureBehavior('correctly sets containers from asset using default props', { [cxapi.DOCKER_IGNORE_SUPPORT]: true }, cdk.App, (app) => {
// GIVEN
const stack = new cdk.Stack(app, 'Stack');

Expand Down
Loading

0 comments on commit 6d6aa0c

Please sign in to comment.