Skip to content

Commit 3e832ba

Browse files
author
Tarun Belani
committed
Addressed review comments
1 parent 4c9a6c8 commit 3e832ba

14 files changed

+103
-44
lines changed
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"exclude": [
3-
"props-no-arn-refs:@aws-cdk/aws-imagebuilder-alpha.InfrastructureConfigurationProps.ec2InstanceHostResourceGroupArn"
3+
"props-no-arn-refs:@aws-cdk/aws-imagebuilder-alpha.InfrastructureConfigurationProps.ec2InstanceHostResourceGroupArn",
4+
"no-unused-type:@aws-cdk/aws-imagebuilder-alpha.ContainerType"
45
]
56
}

packages/@aws-cdk/aws-imagebuilder-alpha/lib/container-recipe.ts

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ const CONTAINER_RECIPE_SYMBOL = Symbol.for('@aws-cdk/aws-imagebuilder-alpha.Cont
2222
*/
2323
const LATEST_VERSION = 'x.x.x';
2424

25+
/**
26+
* The default version to use in the container recipe. When the recipe is updated, the `x` will be incremented off from
27+
* the latest recipe version that exists.
28+
*
29+
* @see https://docs.aws.amazon.com/imagebuilder/latest/userguide/create-image-recipes.html
30+
*/
31+
const DEFAULT_RECIPE_VERSION = '1.0.x';
32+
2533
/**
2634
* An EC2 Image Builder Container Recipe.
2735
*/
@@ -143,6 +151,16 @@ export interface ContainerRecipeProps {
143151
readonly tags?: { [key: string]: string };
144152
}
145153

154+
/**
155+
* The type of the container being used in the container recipe
156+
*/
157+
export enum ContainerType {
158+
/**
159+
* Indicates the container recipe uses a Docker container
160+
*/
161+
DOCKER = 'DOCKER',
162+
}
163+
146164
/**
147165
* The rendered Dockerfile value, for use in CloudFormation.
148166
* - For inline dockerfiles, dockerfileTemplateData is the Dockerfile template text
@@ -397,17 +415,13 @@ export class ContainerRecipe extends ContainerRecipeBase {
397415
id: string,
398416
attrs: ContainerRecipeAttributes,
399417
): IContainerRecipe {
400-
if (attrs.containerRecipeArn && (attrs.containerRecipeName || attrs.containerRecipeVersion)) {
418+
if (!attrs.containerRecipeArn && !attrs.containerRecipeName) {
401419
throw new cdk.ValidationError(
402-
'a containerRecipeName and containerRecipeVersion cannot be provided when a containerRecipeArn is provided',
420+
'either either containerRecipeArn or containerRecipeName must be provided to import a container recipe',
403421
scope,
404422
);
405423
}
406424

407-
if (!attrs.containerRecipeArn && !attrs.containerRecipeName) {
408-
throw new cdk.ValidationError('either containerRecipeArn or containerRecipeName is required', scope);
409-
}
410-
411425
const containerRecipeArn =
412426
attrs.containerRecipeArn ??
413427
cdk.Stack.of(scope).formatArn({
@@ -481,14 +495,17 @@ export class ContainerRecipe extends ContainerRecipeBase {
481495

482496
this.validateContainerRecipeName();
483497

484-
this.addInstanceBlockDevices(...(props.instanceBlockDevices ?? []));
498+
this.addInstanceBlockDevice(...(props.instanceBlockDevices ?? []));
485499

486500
const components: CfnContainerRecipe.ComponentConfigurationProperty[] | undefined = props.components?.map(
487501
(component) => ({
488502
componentArn: component.component.componentArn,
489-
...(Object.keys(component.parameters ?? {}).length && {
490-
parameters: Object.entries(component.parameters!).map(
491-
([key, value]): CfnContainerRecipe.ComponentParameterProperty => ({ name: key, value: value.value }),
503+
...(component.parameters && {
504+
parameters: Object.entries(component.parameters).map(
505+
([name, param]): CfnContainerRecipe.ComponentParameterProperty => ({
506+
name,
507+
value: param.value,
508+
}),
492509
),
493510
}),
494511
}),
@@ -500,13 +517,13 @@ export class ContainerRecipe extends ContainerRecipeBase {
500517
'FROM {{{ imagebuilder:parentImage }}}\n{{{ imagebuilder:environments }}}\n{{{ imagebuilder:components }}}',
501518
);
502519

503-
const containerRecipeVersion = props.containerRecipeVersion ?? '1.0.x';
520+
const containerRecipeVersion = props.containerRecipeVersion ?? DEFAULT_RECIPE_VERSION;
504521
const containerRecipe = new CfnContainerRecipe(this, 'Resource', {
505522
name: this.physicalName,
506523
version: containerRecipeVersion,
507524
description: props.description,
508525
parentImage: props.baseImage.image,
509-
containerType: 'DOCKER',
526+
containerType: ContainerType.DOCKER,
510527
targetRepository: {
511528
repositoryName: props.targetRepository.repositoryName,
512529
service: props.targetRepository.service,
@@ -535,15 +552,15 @@ export class ContainerRecipe extends ContainerRecipeBase {
535552
*
536553
* @param instanceBlockDevices - The list of block devices to attach
537554
*/
538-
public addInstanceBlockDevices(...instanceBlockDevices: ec2.BlockDevice[]): void {
555+
public addInstanceBlockDevice(...instanceBlockDevices: ec2.BlockDevice[]): void {
539556
this.instanceBlockDevices.push(...instanceBlockDevices);
540557
}
541558

542559
/**
543560
* Renders the block devices provided as input to the construct, into the block device mapping structure that
544561
* CfnContainerRecipe expects to receive.
545562
*
546-
* This is rendered at synthesis time, as users can add additional block devices with `addInstanceBlockDevices`, after
563+
* This is rendered at synthesis time, as users can add additional block devices with `addInstanceBlockDevice`, after
547564
* the construct has been instantiated.
548565
*
549566
* @private
@@ -576,6 +593,13 @@ export class ContainerRecipe extends ContainerRecipeBase {
576593
return blockDevices.length ? blockDevices : undefined;
577594
}
578595

596+
/**
597+
* Generates the instance configuration property into the `InstanceConfiguration` type in the CloudFormation L1
598+
* definition.
599+
*
600+
* @param props The props passed as input to the construct
601+
* @private
602+
*/
579603
private buildInstanceConfiguration(
580604
props: ContainerRecipeProps,
581605
): CfnContainerRecipe.InstanceConfigurationProperty | undefined {

packages/@aws-cdk/aws-imagebuilder-alpha/test/base-image.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as ecr from 'aws-cdk-lib/aws-ecr';
33
import * as ssm from 'aws-cdk-lib/aws-ssm';
44
import { BaseContainerImage, ContainerInstanceImage } from '../lib';
55

6-
describe('Base Image', () => {
6+
describe('Base Container Image', () => {
77
let app: cdk.App;
88
let stack: cdk.Stack;
99

@@ -36,6 +36,16 @@ describe('Base Image', () => {
3636
const baseImage = BaseContainerImage.fromString('base-image');
3737
expect(baseImage.image).toEqual('base-image');
3838
});
39+
});
40+
41+
describe('Container Instance Image', () => {
42+
let app: cdk.App;
43+
let stack: cdk.Stack;
44+
45+
beforeEach(() => {
46+
app = new cdk.App();
47+
stack = new cdk.Stack(app, 'Stack', { env: { region: 'us-east-1', account: '123456789012' } });
48+
});
3949

4050
test('should return the correct container instance image for an AMI ID', () => {
4151
const containerInstanceImage = ContainerInstanceImage.fromAmiId('ami-12345678');

packages/@aws-cdk/aws-imagebuilder-alpha/test/container-recipe.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ CMD ["echo", "Hello, world!"]
199199
},
200200
});
201201

202-
containerRecipe.addInstanceBlockDevices(
202+
containerRecipe.addInstanceBlockDevice(
203203
{
204204
deviceName: '/dev/sda2',
205205
volume: ec2.BlockDeviceVolume.ebs(75, {
@@ -601,14 +601,14 @@ CMD ["echo", "Hello, world!"]
601601
});
602602
});
603603

604-
test('throws a validation error when an containerRecipeArn and containerRecipeName are provided when importing by attributes', () => {
604+
test('does not throw a validation error when a containerRecipeArn and containerRecipeName are provided when importing by attributes', () => {
605605
expect(() =>
606606
ContainerRecipe.fromContainerRecipeAttributes(stack, 'ContainerRecipe', {
607607
containerRecipeArn:
608608
'arn:aws:imagebuilder:us-east-1:123456789012:container-recipe/imported-container-recipe/x.x.x',
609609
containerRecipeName: 'imported-container-recipe',
610610
}),
611-
).toThrow(cdk.ValidationError);
611+
).not.toThrow(cdk.ValidationError);
612612
});
613613

614614
test('throws a validation error when neither a containerRecipeArn or containerRecipeName are provided when importing by attributes', () => {

packages/@aws-cdk/aws-imagebuilder-alpha/test/integ.all-parameters.container-recipe.js.snapshot/aws-cdk-imagebuilder-container-recipe-all-parameters.assets.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-imagebuilder-alpha/test/integ.all-parameters.container-recipe.js.snapshot/aws-cdk-imagebuilder-container-recipe-all-parameters.template.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,16 @@
9797
}
9898
}
9999
},
100+
"Outputs": {
101+
"ContainerRecipeVersion": {
102+
"Value": {
103+
"Fn::GetAtt": [
104+
"ContainerRecipe8A7CC9ED",
105+
"Version"
106+
]
107+
}
108+
}
109+
},
100110
"Parameters": {
101111
"BootstrapVersion": {
102112
"Type": "AWS::SSM::Parameter::Value<String>",

packages/@aws-cdk/aws-imagebuilder-alpha/test/integ.all-parameters.container-recipe.js.snapshot/manifest.json

Lines changed: 7 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)