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

chore(lambda-nodejs): prepare code to reduce merge conflicts when deprecated APIs are stripped #13738

Merged
merged 17 commits into from
Mar 24, 2021
Merged
29 changes: 22 additions & 7 deletions packages/@aws-cdk/aws-ecr-assets/lib/image-asset.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
import * as fs from 'fs';
import * as path from 'path';
import * as assets from '@aws-cdk/assets';
import * as ecr from '@aws-cdk/aws-ecr';
import { Annotations, FeatureFlags, IgnoreMode, Stack, Token } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';

// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
// eslint-disable-next-line
import { FingerprintOptions, IAsset, Staging } from '@aws-cdk/assets';
// 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
import { Construct as CoreConstruct } from '@aws-cdk/core';

/**
* Options for DockerImageAsset
*/
export interface DockerImageAssetOptions extends assets.FingerprintOptions {
export interface DockerImageAssetOptions extends FingerprintOptions {
/**
* ECR repository name
*
Expand Down Expand Up @@ -69,7 +71,7 @@ export interface DockerImageAssetProps extends DockerImageAssetOptions {
*
* The image will be created in build time and uploaded to an ECR repository.
*/
export class DockerImageAsset extends CoreConstruct implements assets.IAsset {
export class DockerImageAsset extends CoreConstruct implements IAsset {
/**
* The full URI of the image (including a tag). Use this reference to pull
* the asset.
Expand All @@ -81,8 +83,21 @@ export class DockerImageAsset extends CoreConstruct implements assets.IAsset {
*/
public repository: ecr.IRepository;

/**
* A hash of the source of this asset, which is available at construction time. As this is a plain
* string, it can be used in construct IDs in order to enforce creation of a new resource when
* the content hash has changed.
* @deprecated use assetHash
*/
public readonly sourceHash: string;

/**
* A hash of this asset, which is available at construction time. As this is a plain string, it
* can be used in construct IDs in order to enforce creation of a new resource when the content
* hash has changed.
*/
public readonly assetHash: string;

constructor(scope: Construct, id: string, props: DockerImageAssetProps) {
super(scope, id);

Expand Down Expand Up @@ -141,7 +156,7 @@ export class DockerImageAsset extends CoreConstruct implements assets.IAsset {
// deletion of the ECR repository the app used).
extraHash.version = '1.21.0';

const staging = new assets.Staging(this, 'Staging', {
const staging = new Staging(this, 'Staging', {
...props,
exclude,
ignoreMode,
Expand All @@ -151,16 +166,16 @@ export class DockerImageAsset extends CoreConstruct implements assets.IAsset {
: JSON.stringify(extraHash),
});

this.sourceHash = staging.sourceHash;
this.sourceHash = staging.assetHash;
this.assetHash = staging.assetHash;

const stack = Stack.of(this);
const location = stack.synthesizer.addDockerImageAsset({
directoryName: staging.relativeStagedPath(stack),
dockerBuildArgs: props.buildArgs,
dockerBuildTarget: props.target,
dockerFile: props.file,
repositoryName: props.repositoryName,
sourceHash: staging.sourceHash,
sourceHash: staging.assetHash,
});

this.repository = ecr.Repository.fromRepositoryName(this, 'Repository', location.repositoryName);
Expand Down
62 changes: 31 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 @@ -192,8 +192,8 @@ describe('image asset', () => {

const session = app.synth();

expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'index.py'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'index.py'))).toBeDefined();

});

Expand All @@ -214,16 +214,16 @@ describe('image asset', () => {
const session = app.synth();

// Only the files exempted above should be included.
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, '.dockerignore'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'index.py'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'foobar.txt'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'subdirectory'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'subdirectory', 'baz.txt'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'node_modules'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'node_modules', 'one'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'node_modules', 'some_dep'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'node_modules', 'some_dep', 'file'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, '.dockerignore'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'index.py'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'foobar.txt'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'subdirectory'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'subdirectory', 'baz.txt'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'node_modules'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'node_modules', 'one'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'node_modules', 'some_dep'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'node_modules', 'some_dep', 'file'))).toBeDefined();


});
Expand Down Expand Up @@ -283,13 +283,13 @@ describe('image asset', () => {
const asset6 = new DockerImageAsset(stack, 'Asset6', { directory, extraHash: 'random-extra' });
const asset7 = new DockerImageAsset(stack, 'Asset7', { directory, repositoryName: 'foo' });

expect(asset1.sourceHash).toEqual('ab01ecd4419f59e1ec0ac9e57a60dbb653be68a29af0223fa8cb24b4b747bc73');
expect(asset2.sourceHash).toEqual('7fb12f6148098e3f5c56c788a865d2af689125ead403b795fe6a262ec34384b3');
expect(asset3.sourceHash).toEqual('fc3b6d802ba198ba2ee55079dbef27682bcd1288d5849eb5bbd5cd69038359b3');
expect(asset4.sourceHash).toEqual('30439ea6dfeb4ddfd9175097286895c78393ef52a78c68f92db08abc4513cad6');
expect(asset5.sourceHash).toEqual('5775170880e26ba31799745241b90d4340c674bb3b1c01d758e416ee3f1c386f');
expect(asset6.sourceHash).toEqual('ba82fd351a4d3e3f5c5d948b9948e7e829badc3da90f97e00bb7724afbeacfd4');
expect(asset7.sourceHash).toEqual('26ec194928431cab6ec5af24ea9f01af2cf7b20e361128b07b2a7405d2951f95');
expect(asset1.assetHash).toEqual('ab01ecd4419f59e1ec0ac9e57a60dbb653be68a29af0223fa8cb24b4b747bc73');
expect(asset2.assetHash).toEqual('7fb12f6148098e3f5c56c788a865d2af689125ead403b795fe6a262ec34384b3');
expect(asset3.assetHash).toEqual('fc3b6d802ba198ba2ee55079dbef27682bcd1288d5849eb5bbd5cd69038359b3');
expect(asset4.assetHash).toEqual('30439ea6dfeb4ddfd9175097286895c78393ef52a78c68f92db08abc4513cad6');
expect(asset5.assetHash).toEqual('5775170880e26ba31799745241b90d4340c674bb3b1c01d758e416ee3f1c386f');
expect(asset6.assetHash).toEqual('ba82fd351a4d3e3f5c5d948b9948e7e829badc3da90f97e00bb7724afbeacfd4');
expect(asset7.assetHash).toEqual('26ec194928431cab6ec5af24ea9f01af2cf7b20e361128b07b2a7405d2951f95');

});
});
Expand All @@ -304,12 +304,12 @@ function testDockerDirectoryIsStagedWithoutFilesSpecifiedInDockerignore(app: App
const session = app.synth();

// .dockerignore itself should be included in output to be processed during docker build
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, '.dockerignore'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'index.py'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'foobar.txt'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'subdirectory'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'subdirectory', 'baz.txt'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, '.dockerignore'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'index.py'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'foobar.txt'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'subdirectory'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'subdirectory', 'baz.txt'))).toBeDefined();


}
Expand All @@ -324,12 +324,12 @@ function testDockerDirectoryIsStagedWithoutFilesSpecifiedInExcludeOption(app: Ap

const session = app.synth();

expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, '.dockerignore'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'index.py'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'foobar.txt'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'subdirectory'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.sourceHash}`, 'subdirectory', 'baz.txt'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, '.dockerignore'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'Dockerfile'))).toBeDefined();
expect(fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'index.py'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'foobar.txt'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'subdirectory'))).toBeDefined();
expect(!fs.existsSync(path.join(session.directory, `asset.${image.assetHash}`, 'subdirectory', 'baz.txt'))).toBeDefined();


}
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-events-targets/lib/event-bus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@ export class EventBus implements events.IRuleTarget {
this.role = props.role;
}

bind(rule: events.IRule, id?: string): events.RuleTargetConfig {
bind(rule: events.IRule, _id?: string): events.RuleTargetConfig {
if (this.role) {
this.role.addToPrincipalPolicy(this.putEventStatement());
}
const role = this.role ?? singletonEventRole(rule, [this.putEventStatement()]);
return {
id: id ?? '',
arn: this.eventBus.eventBusArn,
role,
};
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class Bundling implements cdk.BundlingOptions {
private static runsLocally?: boolean;

// Core bundling options
public readonly image: cdk.BundlingDockerImage;
public readonly image: cdk.DockerImage;
public readonly command: string[];
public readonly environment?: { [key: string]: string };
public readonly workingDirectory: string;
Expand Down Expand Up @@ -78,14 +78,14 @@ export class Bundling implements cdk.BundlingOptions {
// Docker bundling
const shouldBuildImage = props.forceDockerBundling || !Bundling.runsLocally;
this.image = shouldBuildImage
? props.dockerImage ?? cdk.BundlingDockerImage.fromAsset(path.join(__dirname, '../lib'), {
? props.dockerImage ?? cdk.DockerImage.fromBuild(path.join(__dirname, '../lib'), {
buildArgs: {
...props.buildArgs ?? {},
IMAGE: props.runtime.bundlingDockerImage.image,
ESBUILD_VERSION: props.esbuildVersion ?? ESBUILD_VERSION,
},
})
: cdk.BundlingDockerImage.fromRegistry('dummy'); // Do not build if we don't need to
: cdk.DockerImage.fromRegistry('dummy'); // Do not build if we don't need to

const bundlingCommand = this.createBundlingCommand(cdk.AssetStaging.BUNDLING_INPUT_DIR, cdk.AssetStaging.BUNDLING_OUTPUT_DIR);
this.command = ['bash', '-c', bundlingCommand];
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BundlingDockerImage } from '@aws-cdk/core';
import { DockerImage } from '@aws-cdk/core';

/**
* Bundling options
Expand Down Expand Up @@ -200,7 +200,7 @@ export interface BundlingOptions {
*
* @default - use the Docker image provided by @aws-cdk/aws-lambda-nodejs
*/
readonly dockerImage?: BundlingDockerImage;
readonly dockerImage?: DockerImage;

/**
* Command hooks
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
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 os from 'os';
import * as path from 'path';
import { Code, Runtime } from '@aws-cdk/aws-lambda';
import { AssetHashType, BundlingDockerImage } from '@aws-cdk/core';
import { AssetHashType, DockerImage } from '@aws-cdk/core';
import { version as delayVersion } from 'delay/package.json';
import { Bundling } from '../lib/bundling';
import { LogLevel } from '../lib/types';
Expand All @@ -11,7 +11,7 @@ import * as util from '../lib/util';
jest.mock('@aws-cdk/aws-lambda');

// Mock BundlingDockerImage.fromAsset() to avoid building the image
let fromAssetMock = jest.spyOn(BundlingDockerImage, 'fromAsset');
let fromAssetMock = jest.spyOn(DockerImage, 'fromBuild');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: adapt the comment above

let getEsBuildVersionMock = jest.spyOn(util, 'getEsBuildVersion');
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -290,7 +290,7 @@ test('Custom bundling docker image', () => {
entry,
depsLockFilePath,
runtime: Runtime.NODEJS_12_X,
dockerImage: BundlingDockerImage.fromRegistry('my-custom-image'),
dockerImage: DockerImage.fromRegistry('my-custom-image'),
forceDockerBundling: true,
});

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda-python/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export function bundle(options: BundlingOptions): lambda.Code {
// copy Dockerfile to workdir
fs.copyFileSync(path.join(__dirname, dockerfile), path.join(stagedir, dockerfile));

const image = cdk.BundlingDockerImage.fromAsset(stagedir, {
const image = cdk.DockerImage.fromBuild(stagedir, {
buildArgs: {
IMAGE: runtime.bundlingDockerImage.image,
},
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda/lib/runtime.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BundlingDockerImage } from '@aws-cdk/core';
import { BundlingDockerImage, DockerImage } from '@aws-cdk/core';

export interface LambdaRuntimeProps {
/**
Expand Down Expand Up @@ -218,7 +218,7 @@ export class Runtime {
this.supportsInlineCode = !!props.supportsInlineCode;
this.family = family;
const imageName = props.bundlingDockerImage ?? `amazon/aws-sam-cli-build-image-${name}`;
this.bundlingDockerImage = BundlingDockerImage.fromRegistry(imageName);
this.bundlingDockerImage = DockerImage.fromRegistry(imageName);
this.supportsCodeGuruProfiling = props.supportsCodeGuruProfiling ?? false;

Runtime.ALL.push(this);
Expand Down
6 changes: 4 additions & 2 deletions packages/@aws-cdk/aws-s3-assets/lib/asset.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as path from 'path';
import * as assets from '@aws-cdk/assets';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
Expand All @@ -8,11 +7,14 @@ import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import { toSymlinkFollow } from './compat';

// 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
import { CopyOptions } from '@aws-cdk/assets';
// 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
import { Construct as CoreConstruct } from '@aws-cdk/core';

export interface AssetOptions extends assets.CopyOptions, cdk.AssetOptions {
export interface AssetOptions extends CopyOptions, cdk.AssetOptions {
/**
* A list of principals that should be able to read this asset from S3.
* You can use `asset.grantRead(principal)` to grant read permissions later.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as path from 'path';
import * as iam from '@aws-cdk/aws-iam';
import { App, BundlingDockerImage, Stack, StackProps } from '@aws-cdk/core';
import { App, DockerImage, Stack, StackProps } from '@aws-cdk/core';
import { Construct } from 'constructs';
import * as assets from '../lib';

Expand All @@ -12,7 +12,7 @@ class TestStack extends Stack {
const asset = new assets.Asset(this, 'BundledAsset', {
path: path.join(__dirname, 'markdown-asset'), // /asset-input and working directory in the container
bundling: {
image: BundlingDockerImage.fromAsset(path.join(__dirname, 'alpine-markdown')), // Build an image
image: DockerImage.fromBuild(path.join(__dirname, 'alpine-markdown')), // Build an image
command: [
'sh', '-c', `
markdown index.md > /asset-output/index.html
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1829,7 +1829,7 @@ export class Bucket extends BucketBase {
private enableAutoDeleteObjects() {
const provider = CustomResourceProvider.getOrCreateProvider(this, AUTO_DELETE_OBJECTS_RESOURCE_TYPE, {
codeDirectory: path.join(__dirname, 'auto-delete-objects-handler'),
runtime: CustomResourceProviderRuntime.NODEJS_12,
runtime: CustomResourceProviderRuntime.NODEJS_12_X,
description: `Lambda function for auto-deleting objects in ${this.bucketName} S3 bucket.`,
});

Expand Down
45 changes: 44 additions & 1 deletion packages/@aws-cdk/core/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class BundlingDockerImage {
* @param image the image name
*/
public static fromRegistry(image: string) {
return new BundlingDockerImage(image);
return new DockerImage(image);
}

/**
Expand Down Expand Up @@ -278,6 +278,49 @@ export class DockerImage extends BundlingDockerImage {
public static fromBuild(path: string, options: DockerBuildOptions = {}) {
return BundlingDockerImage.fromAsset(path, options);
}

/**
* Reference an image on DockerHub or another online registry.
*
* @param image the image name
*/
public static fromRegistry(image: string) {
return new DockerImage(image);
}

/** @param image The Docker image */
constructor(public readonly image: string, _imageHash?: string) {
super(image, _imageHash);
}

/**
* Provides a stable representation of this image for JSON serialization.
*
* @return The overridden image name if set or image hash name in that order
*/
public toJSON() {
return super.toJSON();
}

/**
* Runs a Docker image
*/
public run(options: DockerRunOptions = {}) {
return super.run(options);
}

/**
* Copies a file or directory out of the Docker image to the local filesystem.
*
* If `outputPath` is omitted the destination path is a temporary directory.
*
* @param imagePath the path in the Docker image
* @param outputPath the destination path for the copy operation
* @returns the destination path
*/
public cp(imagePath: string, outputPath?: string): string {
return super.cp(imagePath, outputPath);
}
}

/**
Expand Down
Loading