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

feat(ecs): ContainerImage.fromDockerImageAsset #6093

Merged
merged 5 commits into from
Feb 5, 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
20 changes: 14 additions & 6 deletions packages/@aws-cdk/aws-ecr-assets/lib/image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import * as fs from 'fs';
import * as minimatch from 'minimatch';
import * as path from 'path';

export interface DockerImageAssetProps extends assets.FingerprintOptions {
/**
* The directory where the Dockerfile is stored
*/
readonly directory: string;

/**
* Options for DockerImageAsset
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can be more descriptive as to when users should use DockerImageAssetOptions vs DockerImageAssetProps? (we've had users who have been confused with ContainerDefinitionOptions vs ContainerDefinitionProps in the past)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure there is something super smart to write here... Maybe something like "includes all the optional configuration" (without the mandatory one)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update in a subsequent commit. Let me know what you think.

*/
export interface DockerImageAssetOptions extends assets.FingerprintOptions {
/**
* ECR repository name
*
Expand Down Expand Up @@ -51,6 +49,16 @@ export interface DockerImageAssetProps extends assets.FingerprintOptions {
readonly file?: string;
}

/**
* Props for DockerImageAssets
*/
export interface DockerImageAssetProps extends DockerImageAssetOptions {
/**
* The directory where the Dockerfile is stored
*/
readonly directory: string;
}

/**
* An asset that represents a Docker image.
*
Expand Down
7 changes: 1 addition & 6 deletions packages/@aws-cdk/aws-ecr-assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,5 @@
"bundledDependencies": [
"minimatch"
],
"stability": "experimental",
"awslint": {
"exclude": [
"docs-public-apis:@aws-cdk/aws-ecr-assets.DockerImageAssetProps"
]
}
"stability": "experimental"
}
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-ecs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ obtained from either DockerHub or from ECR repositories, or built directly from
to start. If no tag is provided, "latest" is assumed.
* `ecs.ContainerImage.fromAsset('./image')`: build and upload an
image directly from a `Dockerfile` in your source directory.
* `ecs.ContainerImage.fromDockerImageAsset(asset)`: uses an existing
`@aws-cdk/aws-ecr-assets.DockerImageAsset` as a container image.

### Environment variables

Expand Down
22 changes: 21 additions & 1 deletion packages/@aws-cdk/aws-ecs/lib/container-image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,33 @@ export abstract class ContainerImage {
}

/**
* Reference an image that's constructed directly from sources on disk
* Reference an image that's constructed directly from sources on disk.
*
* If you already have a `DockerImageAsset` instance, you can use the
* `ContainerImage.fromDockerImageAsset` method instead.
*
* @param directory The directory containing the Dockerfile
*/
public static fromAsset(directory: string, props: AssetImageProps = {}) {
return new AssetImage(directory, props);
}

/**
* Use an existing `DockerImageAsset` for this container image.
*
* @param asset The `DockerImageAsset` to use for this container definition.
*/
public static fromDockerImageAsset(asset: DockerImageAsset): ContainerImage {
return {
bind(_scope: cdk.Construct, containerDefinition: ContainerDefinition): ContainerImageConfig {
asset.repository.grantPull(containerDefinition.taskDefinition.obtainExecutionRole());
return {
imageName: asset.imageUri
};
}
};
}

/**
* Called when the image is used by a ContainerDefinition
*/
Expand All @@ -51,6 +70,7 @@ export interface ContainerImageConfig {
readonly repositoryCredentials?: CfnTaskDefinition.RepositoryCredentialsProperty;
}

import { DockerImageAsset } from '@aws-cdk/aws-ecr-assets';
import { AssetImage, AssetImageProps } from './images/asset-image';
import { EcrImage } from './images/ecr';
import { RepositoryImage, RepositoryImageProps } from './images/repository';
30 changes: 4 additions & 26 deletions packages/@aws-cdk/aws-ecs/lib/images/asset-image.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,12 @@
import { DockerImageAsset } from '@aws-cdk/aws-ecr-assets';
import { DockerImageAsset, DockerImageAssetOptions } from '@aws-cdk/aws-ecr-assets';
import * as cdk from '@aws-cdk/core';
import { ContainerDefinition } from '../container-definition';
import { ContainerImage, ContainerImageConfig } from '../container-image';

/**
* The properties for building an AssetImage.
*/
export interface AssetImageProps {
/**
* The arguments to pass to the `docker build` command
*
* @default none
*/
readonly buildArgs?: { [key: string]: string };

/**
* Docker target to build to
*
* @default none
*/
readonly target?: string;

/**
* Path to the Dockerfile (relative to the directory).
*
* @default 'Dockerfile'
*/
readonly file?: string;

export interface AssetImageProps extends DockerImageAssetOptions {
}

/**
Expand All @@ -46,10 +25,9 @@ export class AssetImage extends ContainerImage {
public bind(scope: cdk.Construct, containerDefinition: ContainerDefinition): ContainerImageConfig {
const asset = new DockerImageAsset(scope, 'AssetImage', {
directory: this.directory,
buildArgs: this.props.buildArgs,
target: this.props.target,
file: this.props.file,
...this.props,
});

asset.repository.grantPull(containerDefinition.taskDefinition.obtainExecutionRole());

return {
Expand Down
99 changes: 98 additions & 1 deletion packages/@aws-cdk/aws-ecs/test/test.container-definition.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { expect, haveResource, haveResourceLike, InspectionFailure } from '@aws-cdk/assert';
import * as ecr_assets from '@aws-cdk/aws-ecr-assets';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import * as ssm from '@aws-cdk/aws-ssm';
import * as cdk from '@aws-cdk/core';
import { Test } from 'nodeunit';
import * as path from 'path';
import * as ecs from '../lib';

export = {
Expand Down Expand Up @@ -1293,5 +1295,100 @@ export = {
test.done();
}
},
// render extra hosts test

'can use a DockerImageAsset directly for a container image'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
const asset = new ecr_assets.DockerImageAsset(stack, 'MyDockerImage', {
directory: path.join(__dirname, 'demo-image')
});

// WHEN
taskDefinition.addContainer('default', {
image: ecs.ContainerImage.fromDockerImageAsset(asset),
memoryLimitMiB: 1024
});

// THEN
expect(stack).to(haveResource('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
Essential: true,
Image: {
"Fn::Join": [
"",
[
{ Ref: "AWS::AccountId" },
".dkr.ecr.",
{ Ref: "AWS::Region" },
".",
{ Ref: "AWS::URLSuffix" },
"/aws-cdk/assets:baa2d6eb2a17c75424df631c8c70ff39f2d5f3bee8b9e1a109ee24ca17300540"
]
]
},
Memory: 1024,
Name: "default"
}
]
}));
expect(stack).to(haveResource('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: [
"ecr:BatchCheckLayerAvailability",
"ecr:GetDownloadUrlForLayer",
"ecr:BatchGetImage"
],
Effect: "Allow",
Resource: {
"Fn::Join": [
"",
[ "arn:", { Ref: "AWS::Partition" }, ":ecr:", { Ref: "AWS::Region" }, ":", { Ref: "AWS::AccountId" }, ":repository/aws-cdk/assets" ]
]
}
},
{
Action: "ecr:GetAuthorizationToken",
Effect: "Allow",
Resource: "*"
}
],
Version: "2012-10-17"
}
}));
test.done();
},

'docker image asset options can be used when using container image'(test: Test) {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'MyStack');
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

// WHEN
taskDefinition.addContainer('default', {
memoryLimitMiB: 1024,
image: ecs.ContainerImage.fromAsset(path.join(__dirname, 'demo-image'), {
file: 'index.py', // just because it's there already
target: 'build-target'
})
});

// THEN
const asm = app.synth();
test.deepEqual(asm.getStackArtifact(stack.artifactId).assets[0], {
repositoryName: 'aws-cdk/assets',
imageTag: 'f9014d1df7c8f5a5e7abaf18eb5bc895e82f8b06eeed6f75a40cf1bc2a78955a',
id: 'f9014d1df7c8f5a5e7abaf18eb5bc895e82f8b06eeed6f75a40cf1bc2a78955a',
packaging: 'container-image',
path: 'asset.f9014d1df7c8f5a5e7abaf18eb5bc895e82f8b06eeed6f75a40cf1bc2a78955a',
sourceHash: 'f9014d1df7c8f5a5e7abaf18eb5bc895e82f8b06eeed6f75a40cf1bc2a78955a',
target: 'build-target',
file: 'index.py'
});
test.done();
}
};