Skip to content

Commit

Permalink
chore: support contributing to the AWS CDK using alternative containe…
Browse files Browse the repository at this point in the history
…r clients (#23855)

The AWS CDK uses `docker` for a feature subset related to building assets.
For users, we already support the `CDK_DOCKER` env variable to change the container client at runtime.
However contributing to the AWS CDK currently still requires a contributor to have `docker` installed.

This PR extends the support for the `CDK_DOCKER` env variable to contributor tooling.
For example one can now run:

```console
CDK_DOCKER=finch ./scripts/foreach.sh "yarn build"
CDK_DOCKER=finch ./scripts/foreach.sh "yarn test"
```

When using `finch`, the following packages do still fail tests, due to known feature gaps:

```csv
@aws-cdk/aws-ec2
@aws-cdk/aws-lambda
@aws-cdk/aws-lambda-go
@aws-cdk/aws-lambda-python
@aws-cdk/aws-lambda-nodejs
@aws-cdk/aws-s3-deployment
```

---

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn 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*
  • Loading branch information
mrgrain committed Jan 30, 2023
1 parent d9ce580 commit a6fa5b8
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ cp ${script_dir}/test_index.py $PWD
cp ${script_dir}/Dockerfile $PWD

NOTIFICATIONS_RESOURCE_TEST_NO_DOCKER=${NOTIFICATIONS_RESOURCE_TEST_NO_DOCKER:-""}
DOCKER_CMD=${CDK_DOCKER:-docker}

if [ -z ${NOTIFICATIONS_RESOURCE_TEST_NO_DOCKER} ]; then
# this will run our tests inside the right environment
docker build .
$DOCKER_CMD build .
else
python test_index.py
fi
30 changes: 16 additions & 14 deletions packages/@aws-cdk/core/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import * as path from 'path';
import * as sinon from 'sinon';
import { DockerImage, FileSystem } from '../lib';

const dockerCmd = process.env.CDK_DOCKER ?? 'docker';

describe('bundling', () => {
afterEach(() => {
sinon.restore();
Expand Down Expand Up @@ -33,7 +35,7 @@ describe('bundling', () => {
user: 'user:group',
});

expect(spawnSyncStub.calledWith('docker', [
expect(spawnSyncStub.calledWith(dockerCmd, [
'run', '--rm',
'-u', 'user:group',
'-v', '/host-path:/container-path:delegated',
Expand Down Expand Up @@ -74,13 +76,13 @@ describe('bundling', () => {
})).digest('hex');
const tag = `cdk-${tagHash}`;

expect(spawnSyncStub.firstCall.calledWith('docker', [
expect(spawnSyncStub.firstCall.calledWith(dockerCmd, [
'build', '-t', tag,
'--build-arg', 'TEST_ARG=cdk-test',
'docker-path',
])).toEqual(true);

expect(spawnSyncStub.secondCall.calledWith('docker', [
expect(spawnSyncStub.secondCall.calledWith(dockerCmd, [
'run', '--rm',
tag,
])).toEqual(true);
Expand Down Expand Up @@ -110,13 +112,13 @@ describe('bundling', () => {
})).digest('hex');
const tag = `cdk-${tagHash}`;

expect(spawnSyncStub.firstCall.calledWith('docker', [
expect(spawnSyncStub.firstCall.calledWith(dockerCmd, [
'build', '-t', tag,
'--platform', platform,
'docker-path',
])).toEqual(true);

expect(spawnSyncStub.secondCall.calledWith('docker', [
expect(spawnSyncStub.secondCall.calledWith(dockerCmd, [
'run', '--rm',
tag,
])).toEqual(true);
Expand Down Expand Up @@ -146,13 +148,13 @@ describe('bundling', () => {
})).digest('hex');
const tag = `cdk-${tagHash}`;

expect(spawnSyncStub.firstCall.calledWith('docker', [
expect(spawnSyncStub.firstCall.calledWith(dockerCmd, [
'build', '-t', tag,
'--target', targetStage,
'docker-path',
])).toEqual(true);

expect(spawnSyncStub.secondCall.calledWith('docker', [
expect(spawnSyncStub.secondCall.calledWith(dockerCmd, [
'run', '--rm',
tag,
])).toEqual(true);
Expand Down Expand Up @@ -281,7 +283,7 @@ describe('bundling', () => {
user: 'user:group',
});

expect(spawnSyncStub.calledWith('docker', [
expect(spawnSyncStub.calledWith(dockerCmd, [
'run', '--rm',
'-u', 'user:group',
'-v', '/host-path:/container-path:delegated',
Expand Down Expand Up @@ -392,7 +394,7 @@ describe('bundling', () => {
user: 'user:group',
});

expect(spawnSyncStub.calledWith('docker', [
expect(spawnSyncStub.calledWith(dockerCmd, [
'run', '--rm',
'--security-opt', 'no-new-privileges',
'-u', 'user:group',
Expand Down Expand Up @@ -427,7 +429,7 @@ describe('bundling', () => {
user: 'user:group',
});

expect(spawnSyncStub.calledWith('docker', [
expect(spawnSyncStub.calledWith(dockerCmd, [
'run', '--rm',
'--network', 'host',
'-u', 'user:group',
Expand Down Expand Up @@ -464,7 +466,7 @@ describe('bundling', () => {
// nevertheless what we want to check here is that the command was built correctly and triggered
};

expect(spawnSyncStub.calledWith('docker', [
expect(spawnSyncStub.calledWith(dockerCmd, [
'run', '--rm',
'-u', 'user:group',
'--volumes-from', 'foo',
Expand Down Expand Up @@ -507,7 +509,7 @@ describe('bundling', () => {
});

// THEN
expect(spawnSyncStub.secondCall.calledWith('docker', [
expect(spawnSyncStub.secondCall.calledWith(dockerCmd, [
'run', '--rm',
'-u', 'user:group',
'-v', '/host-path:/container-path:z,delegated',
Expand Down Expand Up @@ -548,7 +550,7 @@ describe('bundling', () => {
});

// THEN
expect(spawnSyncStub.secondCall.calledWith('docker', [
expect(spawnSyncStub.secondCall.calledWith(dockerCmd, [
'run', '--rm',
'-u', 'user:group',
'-v', '/host-path:/container-path:delegated',
Expand Down Expand Up @@ -589,7 +591,7 @@ describe('bundling', () => {
});

// THEN
expect(spawnSyncStub.secondCall.calledWith('docker', [
expect(spawnSyncStub.secondCall.calledWith(dockerCmd, [
'run', '--rm',
'-u', 'user:group',
'-v', '/host-path:/container-path:delegated',
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export interface IntegRunnerOptions {
*
* @default - no additional environment variables
*/
readonly env?: { [name: string]: string },
readonly env?: { [name: string]: string | undefined },

/**
* tmp cdk.out directory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker
profile: request.profile,
env: {
AWS_REGION: request.region,
CDK_DOCKER: process.env.CDK_DOCKER,
},
showOutput: verbosity >= 2,
}, testInfo.destructiveChanges);
Expand Down
13 changes: 11 additions & 2 deletions packages/cdk-cli-wrapper/lib/cdk-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,15 @@ export interface SynthFastOptions {
readonly env?: { [name: string]: string }
}

/**
* Additional environment variables to set
* in the execution environment that will be running
* the cdk commands
*/
export interface Environment {
[key: string]: string | undefined
}

/**
* AWS CDK client that provides an API to programatically execute the CDK CLI
* by wrapping calls to exec the CLI
Expand All @@ -86,7 +95,7 @@ export interface CdkCliWrapperOptions {
*
* @default - no additional env vars
*/
readonly env?: { [key: string]: string },
readonly env?: Environment,

/**
* The path to the cdk executable
Expand All @@ -109,7 +118,7 @@ export interface CdkCliWrapperOptions {
*/
export class CdkCliWrapper implements ICdk {
private readonly directory: string;
private readonly env?: { [key: string]: string };
private readonly env?: Environment;
private readonly cdk: string;
private readonly showOutput: boolean;

Expand Down
52 changes: 42 additions & 10 deletions scripts/check-build-prerequisites.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,51 @@ else
wrong_version
fi

# Container Client
container_client=${CDK_DOCKER:-docker}
docker_min="19.03.0"
finch_min="0.3.0"

# [Docker >= 19.03]
app="docker"
app_min="19.03.0"
check_which $app $app_min
if [ "$container_client" == "docker" ]; then
check_which $container_client $docker_min

# Make sure docker is running
echo -e "Checking if docker is running... \c"
docker_running=$(docker ps)
if [ $? -eq 0 ]
then
echo "Ok"
# Make sure docker is running
echo -e "Checking if Docker is running... \c"
docker_running=$(docker ps)
if [ $? -eq 0 ]
then
echo "Ok"
else
die "Docker is not running"
fi

# [finch >= 0.3.0]
elif [ "$container_client" == "finch" ]; then
check_which $container_client $finch_min

# Make sure docker is running
echo -e "Checking if finch is running... \c"
finch_running=$($container_client ps)
if [ $? -eq 0 ]
then
echo "Ok"
else
die "Finch is not running"
fi
else
die "Docker is not running"
echo "⚠️ Dependency warning: Unknown container client detected. You have set \"CDK_DOCKER=$CDK_DOCKER\"."
check_which $container_client "(unknown version requirement)"
echo "While any docker compatible client can be used as a drop-in replacement, support for \"$CDK_DOCKER\" is unknown."
echo "Proceed with caution."
echo -e "Checking if $container_client is running... \c"
client_running=$($container_client ps)
if [ $? -eq 0 ]
then
echo "Ok"
else
die "$CDK_DOCKER is not running"
fi
fi

# [.NET == 3.1.x, == 5.x]
Expand Down
2 changes: 1 addition & 1 deletion tools/@aws-cdk/node-bundle/src/api/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export interface BundleProps {
*
* @default "inline"
*/
readonly sourcemap?: 'linked' | 'inline' | 'external' | 'both';
readonly sourcemap?: 'linked' | 'inline' | 'external' | 'both';

/**
* Minifies the bundled code.
Expand Down

0 comments on commit a6fa5b8

Please sign in to comment.