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(), }); ```
guardian · Sep 15, 2021 · a01cd04 · a01cd04
1 parent 7673a4a
commit a01cd04
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 103 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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
@@ -39,11 +39,6 @@ Object {
       "Description": "SSM parameter containing the S3 bucket name holding distribution artifacts",
       "Type": "AWS::SSM::Parameter::Value<String>",
     },
-    "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<String>",
     },
-    "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 [