Skip to content

Commit

Permalink
fix: Improve GuAutoScalingGroup's support of a multi-app stack
Browse files Browse the repository at this point in the history
For consistency with other constructs, suffix the `id` of `GuAutoScalingGroup` with the `app`.
This ensures the logicalId is unique both on the auto-generated path and the `existingLogicalId` path.

Also ensure the app tag is added to the construct.
  • Loading branch information
akash1810 committed Apr 8, 2021
1 parent cf15c2f commit ad9aea6
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 3 deletions.
42 changes: 41 additions & 1 deletion src/constructs/autoscaling/asg.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
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 { ApplicationProtocol } from "@aws-cdk/aws-elasticloadbalancingv2";
import { Stack } from "@aws-cdk/core";
import { Stage } from "../../constants";
import { simpleGuStackForTesting } from "../../utils/test";
import { TrackingTag } from "../../constants/library-info";
import type { SynthedStack } from "../../utils/test";
import { alphabeticalTags, simpleGuStackForTesting } from "../../utils/test";
import { GuSecurityGroup } from "../ec2";
import { GuApplicationTargetGroup } from "../loadbalancing";
import type { GuAutoScalingGroupProps } from "./asg";
Expand All @@ -32,6 +34,44 @@ describe("The GuAutoScalingGroup", () => {
app: "testing",
};

test("Uses the AppIdentity to create the logicalId and tag the resource", () => {
const stack = simpleGuStackForTesting();
new GuAutoScalingGroup(stack, "MyAutoScalingGroup", defaultProps);

expect(stack).toHaveResourceOfTypeAndLogicalId(
"AWS::AutoScaling::AutoScalingGroup",
/MyAutoScalingGroupTesting[A-Z0-9]+/
);

expect(stack).toHaveResource("AWS::AutoScaling::AutoScalingGroup", {
Tags: alphabeticalTags([
{
Key: "App",
PropagateAtLaunch: true,
Value: "testing",
},
{
Key: "Stack",
PropagateAtLaunch: true,
Value: "test-stack",
},
{
Key: "Stage",
PropagateAtLaunch: true,
Value: {
Ref: "Stage",
},
},
{
Key: "Name",
PropagateAtLaunch: true,
Value: "Test/MyAutoScalingGroupTesting",
},
{ ...TrackingTag, PropagateAtLaunch: true },
]),
});
});

test("adds the AMI parameter if no imageId prop provided", () => {
const stack = simpleGuStackForTesting();

Expand Down
6 changes: 4 additions & 2 deletions src/constructs/autoscaling/asg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { ApplicationTargetGroup } from "@aws-cdk/aws-elasticloadbalancingv2
import { Stage } from "../../constants";
import type { GuStack } from "../core";
import { GuAmiParameter, GuInstanceTypeParameter } from "../core";
import type { AppIdentity } from "../core/identity";
import { AppIdentity } from "../core/identity";
import { GuHttpsEgressSecurityGroup } from "../ec2";

// Since we want to override the types of what gets passed in for the below props,
Expand Down Expand Up @@ -104,7 +104,7 @@ export class GuAutoScalingGroup extends AutoScalingGroup {
securityGroup: GuHttpsEgressSecurityGroup.forVpc(scope, props),
};

super(scope, id, mergedProps);
super(scope, AppIdentity.suffixText(props, id), mergedProps);

mergedProps.targetGroup && this.attachToApplicationTargetGroup(mergedProps.targetGroup);

Expand All @@ -117,5 +117,7 @@ export class GuAutoScalingGroup extends AutoScalingGroup {
cfnAsg.addDeletionOverride("UpdatePolicy");

if (mergedProps.overrideId) cfnAsg.overrideLogicalId(id);

AppIdentity.taggedConstruct(props, this);
}
}
1 change: 1 addition & 0 deletions src/utils/test/alphabetical-tags.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
interface Tag {
Key: string;
Value: unknown;
PropagateAtLaunch?: boolean;
}

export function alphabeticalTags(tags: Tag[]): Tag[] {
Expand Down

0 comments on commit ad9aea6

Please sign in to comment.