From a01cd04a015fbcce343ebcef68ea92bd0ca5b3b4 Mon Sep 17 00:00:00 2001 From: Jacob Winch Date: Wed, 15 Sep 2021 13:55:19 +0100 Subject: [PATCH] feat!: Make instanceType a mandatory prop for EC2-based infrastructure (#790) * feat!: Make instanceType a mandatory prop BREAKING CHANGE: The following constructs and patterns now require an instanceType prop: * GuAutoScalingGroup * GuEc2App * GuPlayApp * GuNodeApp Previously these constructs and patterns would automatically create a CloudFormation parameter for the instance type. This parameter has now been removed and instance type must be provided via the constructor's props. For example: ```typescript new GuEc2App(stack, { applicationPort: GuApplicationPorts.Node, app: "test-gu-ec2-app", access: { scope: AccessScope.PUBLIC }, instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), //This is now required monitoringConfiguration: { noMonitoring: true }, userData: "#!/bin/dev foobarbaz", certificateProps: getCertificateProps(), }); ``` --- src/constructs/autoscaling/asg.test.ts | 33 ++++----------- src/constructs/autoscaling/asg.ts | 7 +--- src/constructs/autoscaling/user-data.test.ts | 4 +- src/constructs/core/parameters/ec2.test.ts | 40 ------------------- src/constructs/core/parameters/ec2.ts | 11 ----- .../__snapshots__/ec2-app.test.ts.snap | 18 +-------- src/patterns/ec2-app.test.ts | 29 ++++++++++++-- src/patterns/ec2-app.ts | 6 ++- 8 files changed, 45 insertions(+), 103 deletions(-) delete mode 100644 src/constructs/core/parameters/ec2.test.ts diff --git a/src/constructs/autoscaling/asg.test.ts b/src/constructs/autoscaling/asg.test.ts index 03fe6a82f4..7c23b895bb 100644 --- a/src/constructs/autoscaling/asg.test.ts +++ b/src/constructs/autoscaling/asg.test.ts @@ -1,7 +1,7 @@ import "@aws-cdk/assert/jest"; import "../../utils/test/jest"; import { SynthUtils } from "@aws-cdk/assert/lib/synth-utils"; -import { InstanceType, UserData, Vpc } from "@aws-cdk/aws-ec2"; +import { InstanceClass, InstanceSize, InstanceType, UserData, Vpc } from "@aws-cdk/aws-ec2"; import { ApplicationProtocol } from "@aws-cdk/aws-elasticloadbalancingv2"; import { Stack } from "@aws-cdk/core"; import { Stage } from "../../constants"; @@ -28,6 +28,7 @@ describe("The GuAutoScalingGroup", () => { const defaultProps: GuAutoScalingGroupProps = { ...app, vpc, + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), userData: UserData.custom(["#!/bin/bash", "service some-dependency start", "service my-app start"].join("\n")), stageDependentProps: { [Stage.CODE]: { @@ -81,35 +82,11 @@ describe("The GuAutoScalingGroup", () => { }); }); - test("adds the instanceType parameter if none provided", () => { - const stack = simpleGuStackForTesting(); - - new GuAutoScalingGroup(stack, "AutoscalingGroup", defaultProps); - - const json = SynthUtils.toCloudFormation(stack) as SynthedStack; - - expect(json.Parameters["InstanceTypeTesting"]).toEqual({ - Type: "String", - Description: "EC2 Instance Type for the app testing", - Default: "t3.small", - }); - - expect(stack).toHaveResource("AWS::AutoScaling::LaunchConfiguration", { - InstanceType: { - Ref: "InstanceTypeTesting", - }, - }); - }); - - test("does not create the instanceType parameter if value is provided", () => { + test("correctly sets up the instance type in the launch configuration", () => { const stack = simpleGuStackForTesting(); new GuAutoScalingGroup(stack, "AutoscalingGroup", { ...defaultProps, instanceType: new InstanceType("t3.small") }); - const json = SynthUtils.toCloudFormation(stack) as SynthedStack; - - expect(Object.keys(json.Parameters)).not.toContain(InstanceType); - expect(stack).toHaveResource("AWS::AutoScaling::LaunchConfiguration", { InstanceType: "t3.small", }); @@ -258,6 +235,7 @@ describe("The GuAutoScalingGroup", () => { new GuAutoScalingGroup(stack, "AutoscalingGroup", { app: "TestApp", userData: "SomeUserData", + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), vpc, }); @@ -284,6 +262,7 @@ describe("The GuAutoScalingGroup", () => { new GuAutoScalingGroup(stack, "AutoscalingGroup", { app: "TestApp", userData: "UserData", + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), vpc, role: new GuInstanceRole(stack, { app: "TestApp", @@ -329,6 +308,7 @@ describe("The GuAutoScalingGroup", () => { new GuAutoScalingGroup(stack, "AutoscalingGroup", { app: "TestApp", userData: "SomeUserData", + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), vpc, }); @@ -353,6 +333,7 @@ describe("The GuAutoScalingGroup", () => { new GuAutoScalingGroup(stack, "AutoscalingGroup", { app: "TestApp", userData: "SomeUserData", + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), vpc, stageDependentProps: { [Stage.CODE]: { minimumInstances: 2 }, diff --git a/src/constructs/autoscaling/asg.ts b/src/constructs/autoscaling/asg.ts index a90cbf509f..a9426f3f4d 100644 --- a/src/constructs/autoscaling/asg.ts +++ b/src/constructs/autoscaling/asg.ts @@ -1,11 +1,11 @@ import { AutoScalingGroup } from "@aws-cdk/aws-autoscaling"; import type { AutoScalingGroupProps, CfnAutoScalingGroup } from "@aws-cdk/aws-autoscaling"; -import { InstanceType, OperatingSystemType, UserData } from "@aws-cdk/aws-ec2"; +import { OperatingSystemType, UserData } from "@aws-cdk/aws-ec2"; import type { ISecurityGroup, MachineImage, MachineImageConfig } from "@aws-cdk/aws-ec2"; import type { ApplicationTargetGroup } from "@aws-cdk/aws-elasticloadbalancingv2"; import { Stage } from "../../constants"; import { GuStatefulMigratableConstruct } from "../../utils/mixin"; -import { GuAmiParameter, GuInstanceTypeParameter } from "../core"; +import { GuAmiParameter } from "../core"; import type { GuStack } from "../core"; import { AppIdentity } from "../core/identity"; import type { GuMigratingResource } from "../core/migrating"; @@ -21,7 +21,6 @@ export interface GuAutoScalingGroupProps | "imageId" | "osType" | "machineImage" - | "instanceType" | "userData" | "minCapacity" | "maxCapacity" @@ -31,7 +30,6 @@ export interface GuAutoScalingGroupProps AppIdentity, GuMigratingResource { stageDependentProps?: GuStageDependentAsgProps; - instanceType?: InstanceType; imageId?: GuAmiParameter; machineImage?: MachineImage; userData: UserData | string; @@ -122,7 +120,6 @@ export class GuAutoScalingGroup extends GuStatefulMigratableConstruct(AutoScalin ...wireStageDependentProps(scope, props.stageDependentProps ?? defaultStageDependentProps), role: props.role ?? new GuInstanceRole(scope, { app: props.app }), machineImage: { getImage: getImage }, - instanceType: props.instanceType ?? new InstanceType(new GuInstanceTypeParameter(scope, props).valueAsString), userData, // Do not use the default AWS security group which allows egress on any port. // Favour HTTPS only egress rules by default. diff --git a/src/constructs/autoscaling/user-data.test.ts b/src/constructs/autoscaling/user-data.test.ts index 89aa617d2b..50ef3f67f6 100644 --- a/src/constructs/autoscaling/user-data.test.ts +++ b/src/constructs/autoscaling/user-data.test.ts @@ -1,5 +1,5 @@ import "@aws-cdk/assert/jest"; -import { Vpc } from "@aws-cdk/aws-ec2"; +import { InstanceClass, InstanceSize, InstanceType, Vpc } from "@aws-cdk/aws-ec2"; import { Stack } from "@aws-cdk/core"; import { Stage } from "../../constants"; import { simpleGuStackForTesting } from "../../utils/test"; @@ -32,6 +32,7 @@ describe("GuUserData", () => { new GuAutoScalingGroup(stack, "AutoscalingGroup", { vpc, userData, + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), stageDependentProps: { [Stage.CODE]: { minimumInstances: 1, @@ -86,6 +87,7 @@ describe("GuUserData", () => { new GuAutoScalingGroup(stack, "AutoscalingGroup", { vpc, userData, + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), stageDependentProps: { [Stage.CODE]: { minimumInstances: 1, diff --git a/src/constructs/core/parameters/ec2.test.ts b/src/constructs/core/parameters/ec2.test.ts deleted file mode 100644 index 02db801cf5..0000000000 --- a/src/constructs/core/parameters/ec2.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -import "@aws-cdk/assert/jest"; -import { SynthUtils } from "@aws-cdk/assert/lib/synth-utils"; -import { simpleGuStackForTesting } from "../../../utils/test"; -import type { SynthedStack } from "../../../utils/test"; -import { GuInstanceTypeParameter } from "./ec2"; - -describe("The GuInstanceTypeParameter class", () => { - it("should combine default, override and prop values", () => { - const stack = simpleGuStackForTesting(); - - new GuInstanceTypeParameter(stack, { allowedValues: ["t3.small"], app: "testing" }); - - const json = SynthUtils.toCloudFormation(stack) as SynthedStack; - - expect(json.Parameters[`InstanceTypeTesting`]).toEqual({ - Type: "String", - Description: "EC2 Instance Type for the app testing", - Default: "t3.small", - AllowedValues: ["t3.small"], - }); - }); - - it("let's you override the default values", () => { - const stack = simpleGuStackForTesting(); - - new GuInstanceTypeParameter(stack, { - description: "This is a test", - default: 1, - app: "testing", - }); - - const json = SynthUtils.toCloudFormation(stack) as SynthedStack; - - expect(json.Parameters["InstanceTypeTesting"]).toEqual({ - Type: "String", - Description: "This is a test", - Default: 1, - }); - }); -}); diff --git a/src/constructs/core/parameters/ec2.ts b/src/constructs/core/parameters/ec2.ts index 3bdf830ff1..ffa561cb97 100644 --- a/src/constructs/core/parameters/ec2.ts +++ b/src/constructs/core/parameters/ec2.ts @@ -3,17 +3,6 @@ import type { GuStack } from "../stack"; import { GuParameter } from "./base"; import type { GuNoTypeParameterPropsWithAppIdentity } from "./base"; -export class GuInstanceTypeParameter extends GuParameter { - constructor(scope: GuStack, props: GuNoTypeParameterPropsWithAppIdentity) { - super(scope, AppIdentity.suffixText(props, "InstanceType"), { - type: "String", - description: `EC2 Instance Type for the app ${props.app}`, - default: "t3.small", - ...props, - }); - } -} - export class GuAmiParameter extends GuParameter { constructor(scope: GuStack, props: GuNoTypeParameterPropsWithAppIdentity) { super(scope, AppIdentity.suffixText(props, "AMI"), { diff --git a/src/patterns/__snapshots__/ec2-app.test.ts.snap b/src/patterns/__snapshots__/ec2-app.test.ts.snap index 4b0ed800b9..af6ea124f7 100644 --- a/src/patterns/__snapshots__/ec2-app.test.ts.snap +++ b/src/patterns/__snapshots__/ec2-app.test.ts.snap @@ -39,11 +39,6 @@ Object { "Description": "SSM parameter containing the S3 bucket name holding distribution artifacts", "Type": "AWS::SSM::Parameter::Value", }, - "InstanceTypeTestguec2app": Object { - "Default": "t3.small", - "Description": "EC2 Instance Type for the app test-gu-ec2-app", - "Type": "String", - }, "LoggingStreamName": Object { "Default": "/account/services/logging.stream.name", "Description": "SSM parameter containing the Name (not ARN) on the kinesis stream", @@ -171,9 +166,7 @@ Object { "ImageId": Object { "Ref": "AMITestguec2app", }, - "InstanceType": Object { - "Ref": "InstanceTypeTestguec2app", - }, + "InstanceType": "t4g.medium", "SecurityGroups": Array [ Object { "Fn::GetAtt": Array [ @@ -908,11 +901,6 @@ Object { "Description": "SSM parameter containing the S3 bucket name holding distribution artifacts", "Type": "AWS::SSM::Parameter::Value", }, - "InstanceTypeTestguec2app": Object { - "Default": "t3.small", - "Description": "EC2 Instance Type for the app test-gu-ec2-app", - "Type": "String", - }, "LoggingStreamName": Object { "Default": "/account/services/logging.stream.name", "Description": "SSM parameter containing the Name (not ARN) on the kinesis stream", @@ -1040,9 +1028,7 @@ Object { "ImageId": Object { "Ref": "AMITestguec2app", }, - "InstanceType": Object { - "Ref": "InstanceTypeTestguec2app", - }, + "InstanceType": "t4g.medium", "SecurityGroups": Array [ Object { "Fn::GetAtt": Array [ diff --git a/src/patterns/ec2-app.test.ts b/src/patterns/ec2-app.test.ts index 3823d4727b..039e2fb6cf 100644 --- a/src/patterns/ec2-app.test.ts +++ b/src/patterns/ec2-app.test.ts @@ -1,8 +1,7 @@ -import "../utils/test/jest"; -import "@aws-cdk/assert/jest"; import { SynthUtils } from "@aws-cdk/assert"; +import "@aws-cdk/assert/jest"; import { BlockDeviceVolume, EbsDeviceVolumeType } from "@aws-cdk/aws-autoscaling"; -import { Peer, Port, Vpc } from "@aws-cdk/aws-ec2"; +import { InstanceClass, InstanceSize, InstanceType, Peer, Port, Vpc } from "@aws-cdk/aws-ec2"; import type { CfnLoadBalancer } from "@aws-cdk/aws-elasticloadbalancingv2"; import { Stage } from "../constants"; import { TagKeys } from "../constants/tag-keys"; @@ -12,6 +11,7 @@ import { GuSecurityGroup } from "../constructs/ec2/security-groups"; import { GuDynamoDBWritePolicy } from "../constructs/iam"; import type { SynthedStack } from "../utils/test"; import { simpleGuStackForTesting } from "../utils/test"; +import "../utils/test/jest"; import type { GuEc2AppProps } from "./ec2-app"; import { AccessScope, GuApplicationPorts, GuEc2App, GuNodeApp, GuPlayApp } from "./ec2-app"; @@ -31,6 +31,7 @@ function simpleEc2AppForTesting(stack: GuStack, app: string, props: Partial