Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cfn-include): cfn-include fails in monocdk #11595

Merged
merged 7 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/aws-cdk/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Following are the steps involved in running these tests:
- Download the previous version tarball from npm and extract the integration tests.
- Export a `FRAMWORK_VERSION` env variable based on the caller, and execute the integration tests of the previous version.

7. Our integration tests now run and have knowledge of which framework version they should [install](./test/integ/cli/cdk-helpers.ts#L74).
7. Our integration tests now run and have knowledge of which framework version they should [install](./test/integ/helpers/cdk.ts#L74).

That "basically" it, hope it makes sense...

Expand Down
6 changes: 3 additions & 3 deletions packages/aws-cdk/test/integ/cli/bootstrapping.integtest.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from 'fs';
import * as path from 'path';
import { randomString, withDefaultFixture } from './cdk-helpers';
import { integTest } from './test-helpers';
import { randomString, withDefaultFixture } from '../helpers/cdk';
import { integTest } from '../helpers/test-helpers';

jest.setTimeout(600_000);

Expand Down Expand Up @@ -242,4 +242,4 @@ integTest('add tags, left alone on re-bootstrap', withDefaultFixture(async (fixt
expect(response.Stacks?.[0].Tags).toEqual([
{ Key: 'Foo', Value: 'Bar' },
]);
}));
}));
6 changes: 3 additions & 3 deletions packages/aws-cdk/test/integ/cli/cli.integtest.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { promises as fs } from 'fs';
import * as os from 'os';
import * as path from 'path';
import { retry, sleep } from './aws-helpers';
import { cloneDirectory, shell, withDefaultFixture } from './cdk-helpers';
import { integTest } from './test-helpers';
import { retry, sleep } from '../helpers/aws';
import { cloneDirectory, shell, withDefaultFixture } from '../helpers/cdk';
import { integTest } from '../helpers/test-helpers';

jest.setTimeout(600 * 1000);

Expand Down
15 changes: 2 additions & 13 deletions packages/aws-cdk/test/integ/cli/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,5 @@ echo '~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~'

cd $scriptdir

# Install these dependencies that the tests (written in Jest) need.
# Only if we're not running from the repo, because if we are the
# dependencies have already been installed by the containing 'aws-cdk' package's
# package.json.
if ! npx --no-install jest --version; then
echo 'Looks like we need to install jest first. Hold on.' >& 2
npm install --prefix . jest jest-junit aws-sdk
fi

# This must --runInBand because parallelism is arranged for inside the tests
# themselves and they must run in the same process in order to coordinate to
# make sure no 2 tests use the same region at the same time.
npx jest --runInBand --verbose "$@"
source ../common/jest-test.bash
invokeJest "$@"
15 changes: 15 additions & 0 deletions packages/aws-cdk/test/integ/common/jest-test.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
function invokeJest() {
# Install these dependencies that the tests (written in Jest) need.
# Only if we're not running from the repo, because if we are the
# dependencies have already been installed by the containing 'aws-cdk' package's
# package.json.
if ! npx --no-install jest --version; then
echo 'Looks like we need to install jest first. Hold on.' >& 2
npm install --prefix . jest jest-junit aws-sdk
fi

# This must --runInBand because parallelism is arranged for inside the tests
# themselves and they must run in the same process in order to coordinate to
# make sure no 2 tests use the same region at the same time.
npx jest --runInBand --verbose "$@"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as child_process from 'child_process';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { outputFromStack, AwsClients } from './aws-helpers';
import { outputFromStack, AwsClients } from './aws';
import { ResourcePool } from './resource-pool';
import { TestContext } from './test-helpers';

Expand Down Expand Up @@ -52,7 +52,7 @@ export function withCdkApp<A extends TestContext & AwsContext>(block: (context:
context.output.write(` Test directory: ${integTestDir}\n`);
context.output.write(` Region: ${context.aws.region}\n`);

await cloneDirectory(path.join(__dirname, 'app'), integTestDir, context.output);
await cloneDirectory(path.join(__dirname, '..', 'cli', 'app'), integTestDir, context.output);
const fixture = new TestFixture(
integTestDir,
stackNamePrefix,
Expand Down Expand Up @@ -92,6 +92,51 @@ export function withCdkApp<A extends TestContext & AwsContext>(block: (context:
};
}

export function withMonolithicCfnIncludeCdkApp<A extends TestContext>(block: (context: TestFixture) => Promise<void>) {
return async (context: A) => {
const uberPackage = process.env.UBERPACKAGE;
if (!uberPackage) {
throw new Error('The UBERPACKAGE environment variable is required for running this test!');
}

const randy = randomString();
const stackNamePrefix = `cdk-uber-cfn-include-${randy}`;
const integTestDir = path.join(os.tmpdir(), `cdk-uber-cfn-include-${randy}`);

context.output.write(` Stack prefix: ${stackNamePrefix}\n`);
context.output.write(` Test directory: ${integTestDir}\n`);

const awsClients = await AwsClients.default(context.output);
await cloneDirectory(path.join(__dirname, '..', 'uberpackage', 'cfn-include-app'), integTestDir, context.output);
const fixture = new TestFixture(
integTestDir,
stackNamePrefix,
context.output,
awsClients,
);

let success = true;
try {
let module = uberPackage;
if (FRAMEWORK_VERSION) {
module = `${module}@${FRAMEWORK_VERSION}`;
}
await fixture.shell(['npm', 'install', 'constructs', module]);

await block(fixture);
} catch (e) {
success = false;
throw e;
} finally {
if (process.env.INTEG_NO_CLEAN) {
process.stderr.write(`Left test directory in '${integTestDir}' ($INTEG_NO_CLEAN)\n`);
} else {
await fixture.dispose(success);
}
}
};
}

/**
* Default test fixture for most (all?) integ tests
*
Expand Down Expand Up @@ -370,10 +415,11 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
child.once('error', reject);

child.once('close', code => {
const output = (Buffer.concat(stdout).toString('utf-8') + Buffer.concat(stderr).toString('utf-8')).trim();
if (code === 0 || options.allowErrExit) {
resolve((Buffer.concat(stdout).toString('utf-8') + Buffer.concat(stderr).toString('utf-8')).trim());
resolve(output);
} else {
reject(new Error(`'${command.join(' ')}' exited with error code ${code}`));
reject(new Error(`'${command.join(' ')}' exited with error code ${code}. Output: \n${output}`));
}
});
});
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk/test/integ/run-against-dist.bash
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ npmws=/tmp/cdk-rundist
rm -rf $npmws
mkdir -p $npmws

set -x

# This script must create 1 or 2 traps, and the 'trap' command will replace
# the previous trap, so get some 'dynamic traps' mechanism in place
TRAPS=()
Expand Down Expand Up @@ -137,4 +139,4 @@ function prepare_python_packages() {

function pip() {
pip_ "$@"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!cfn-include-app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"app": "node cfn-include-app.js",
"versionReporting": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const path = require('path');

const uberPackage = process.env.UBERPACKAGE;
if (!uberPackage) {
throw new Error('The UBERPACKAGE environment variable is required for running this app!');
}

const cfn_inc = require(`${uberPackage}/cloudformation-include`);
const core = require(`${uberPackage}`);

const app = new core.App();
const stack = new core.Stack(app, 'Stack');
const cfnInclude = new cfn_inc.CfnInclude(stack, 'Template', {
templateFile: path.join(__dirname, 'example-template.json'),
});
const cfnBucket = cfnInclude.getResource('Bucket');
if (cfnBucket.bucketName !== 'my-example-bucket') {
throw new Error(`Expected bucketName to be 'my-example-bucket', got: '${cfnBucket.bucketName}'`);
}

app.synth();
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"Resources": {
"Bucket": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "my-example-bucket"
}
}
}
}
15 changes: 15 additions & 0 deletions packages/aws-cdk/test/integ/uberpackage/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module.exports = {
moduleFileExtensions: [
"js",
],
testMatch: [
"**/*.integtest.js",
],
testEnvironment: "node",
bail: 1,
verbose: true,
reporters: [
"default",
[ "jest-junit", { suiteName: "jest tests", outputDirectory: "coverage" } ]
]
};
14 changes: 14 additions & 0 deletions packages/aws-cdk/test/integ/uberpackage/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash

set -euo pipefail

scriptdir=$(cd $(dirname $0) && pwd)

echo '~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~'
echo 'UberCDK Integration Tests'
echo '~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~'

cd $scriptdir

source ../common/jest-test.bash
invokeJest "$@"
12 changes: 12 additions & 0 deletions packages/aws-cdk/test/integ/uberpackage/uberpackage.integtest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { withMonolithicCfnIncludeCdkApp } from '../helpers/cdk';
import { integTest } from '../helpers/test-helpers';

jest.setTimeout(600_000);

describe('uberpackage', () => {
integTest('works with cloudformation-include', withMonolithicCfnIncludeCdkApp(async (fixture) => {
fixture.log('Starting test of cfn-include with monolithic CDK');

await fixture.cdkSynth();
}));
});
4 changes: 2 additions & 2 deletions packages/aws-cdk/test/util/stack-monitor.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { StackActivityMonitor, IActivityPrinter, StackActivity } from '../../lib/api/util/cloudformation/stack-activity-monitor';
import { sleep } from '../integ/cli/aws-helpers';
import { sleep } from '../integ/helpers/aws';
import { MockSdk } from './mock-sdk';

let sdk: MockSdk;
Expand Down Expand Up @@ -167,4 +167,4 @@ async function waitForCondition(cb: () => boolean): Promise<void> {
while (!cb()) {
await sleep(10);
}
}
}
22 changes: 17 additions & 5 deletions tools/ubergen/bin/ubergen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ async function prepareSourceFiles(libraries: readonly LibraryReference[], packag
const indexStatements = new Array<string>();
for (const library of libraries) {
const libDir = path.join(LIB_ROOT, library.shortName);
await transformPackage(library, packageJson.jsii.targets, libDir, libraries);
await transformPackage(library, packageJson, libDir, libraries);

if (library.shortName === 'core') {
indexStatements.push(`export * from './${library.shortName}';`);
Expand All @@ -225,13 +225,13 @@ async function prepareSourceFiles(libraries: readonly LibraryReference[], packag

async function transformPackage(
library: LibraryReference,
config: PackageJson['jsii']['targets'],
uberPackageJson: PackageJson,
destination: string,
allLibraries: readonly LibraryReference[],
) {
await fs.mkdirp(destination);

await copyOrTransformFiles(library.root, destination, allLibraries);
await copyOrTransformFiles(library.root, destination, allLibraries, uberPackageJson);

await fs.writeFile(
path.join(destination, 'index.ts'),
Expand All @@ -240,6 +240,7 @@ async function transformPackage(
);

if (library.shortName !== 'core') {
const config = uberPackageJson.jsii.targets;
await fs.writeJson(
path.join(destination, '.jsiirc.json'),
{
Expand Down Expand Up @@ -291,7 +292,7 @@ function transformTargets(monoConfig: PackageJson['jsii']['targets'], targets: P
return result;
}

async function copyOrTransformFiles(from: string, to: string, libraries: readonly LibraryReference[]) {
async function copyOrTransformFiles(from: string, to: string, libraries: readonly LibraryReference[], uberPackageJson: PackageJson) {
const promises = (await fs.readdir(from)).map(async name => {
if (shouldIgnoreFile(name)) { return; }

Expand All @@ -308,14 +309,25 @@ async function copyOrTransformFiles(from: string, to: string, libraries: readonl
const stat = await fs.stat(source);
if (stat.isDirectory()) {
await fs.mkdirp(destination);
return copyOrTransformFiles(source, destination, libraries);
return copyOrTransformFiles(source, destination, libraries, uberPackageJson);
}
if (name.endsWith('.ts')) {
return fs.writeFile(
destination,
await rewriteImports(source, to, libraries),
{ encoding: 'utf8' },
);
} else if (name === 'cfn-types-2-classes.json') {
// This is a special file used by the cloudformation-include module that contains mappings
// of CFN resource types to the fully-qualified class names of the CDK L1 classes.
// We need to rewrite it to refer to the uberpackage instead of the individual packages
const cfnTypes2Classes: { [key: string]: string } = await fs.readJson(source);
for (const cfnType of Object.keys(cfnTypes2Classes)) {
const fqn = cfnTypes2Classes[cfnType];
// replace @aws-cdk/aws-<service> with <uber-package-name>/aws-<service>
cfnTypes2Classes[cfnType] = fqn.replace('@aws-cdk', uberPackageJson.name);
}
await fs.writeJson(destination, cfnTypes2Classes, { spaces: 2 });
} else {
return fs.copyFile(source, destination);
}
Expand Down