fix(riff-raff-generator): Correctly add minInstancesInServiceParameters parameter

src/experimental/patterns/__snapshots__/ec2-app.test.ts.snap

@@ -26,6 +26,8 @@ exports[`The GuEc2AppExperimental pattern matches the snapshot 1`] = `
       "GuApplicationTargetGroup",
       "GuHttpsApplicationListener",
       "GuRiffRaffDeploymentIdParameter",
+      "GuAutoScalingRollingUpdateTimeoutExperimental",
+      "GuHorizontallyScalingDeploymentPropertiesExperimental",
     ],
     "gu:cdk:version": "TEST",
   },

src/experimental/patterns/ec2-app.ts

@@ -1,9 +1,11 @@
 import type { IAspect } from "aws-cdk-lib";
+import { Stack } from "aws-cdk-lib";
 import { Aspects, CfnParameter, Duration, Tags } from "aws-cdk-lib";
 import { CfnAutoScalingGroup, CfnScalingPolicy, ScalingProcess, UpdatePolicy } from "aws-cdk-lib/aws-autoscaling";
 import type { CfnPolicy } from "aws-cdk-lib/aws-iam";
 import { Effect, Policy, PolicyStatement } from "aws-cdk-lib/aws-iam";
 import type { IConstruct } from "constructs";
+import { Construct } from "constructs";
 import { MetadataKeys } from "../../constants";
 import { GuAutoScalingGroup } from "../../constructs/autoscaling";
 import type { GuStack } from "../../constructs/core";
@@ -42,12 +44,11 @@ export const RollingUpdateDurations: AutoScalingRollingUpdateDurations = {
  *
  * TODO Expose the healthcheck grace period as a property on {@link GuEc2App} and remove this `Aspect`.
  */
-export class GuAutoScalingRollingUpdateTimeoutExperimental implements IAspect {
-  public readonly stack: GuStack;
+export class GuAutoScalingRollingUpdateTimeoutExperimental extends Construct implements IAspect {
   private static instance: GuAutoScalingRollingUpdateTimeoutExperimental | undefined;
 
   private constructor(scope: GuStack) {
-    this.stack = scope;
+    super(scope, GuAutoScalingRollingUpdateTimeoutExperimental.name);
   }
 
   public static getInstance(stack: GuStack): GuAutoScalingRollingUpdateTimeoutExperimental {
@@ -112,13 +113,12 @@ export class GuAutoScalingRollingUpdateTimeoutExperimental implements IAspect {
  *
  * @see https://github.com/guardian/testing-asg-rolling-update
  */
-export class GuHorizontallyScalingDeploymentPropertiesExperimental implements IAspect {
-  public readonly stack: GuStack;
+export class GuHorizontallyScalingDeploymentPropertiesExperimental extends Construct implements IAspect {
   public readonly asgToParamMap: Map<string, CfnParameter>;
   private static instance: GuHorizontallyScalingDeploymentPropertiesExperimental | undefined;
 
   private constructor(scope: GuStack) {
-    this.stack = scope;
+    super(scope, GuHorizontallyScalingDeploymentPropertiesExperimental.name);
     this.asgToParamMap = new Map();
   }
 
@@ -131,7 +131,8 @@ export class GuHorizontallyScalingDeploymentPropertiesExperimental implements IA
   }
 
   public visit(construct: IConstruct) {
-    if (construct instanceof CfnScalingPolicy && construct.stack.stackName === this.stack.stackName) {
+    const stack = Stack.of(this);
+    if (construct instanceof CfnScalingPolicy && construct.stack === stack) {
       const { node } = construct;
       const { scopes, path } = node;
 
@@ -174,7 +175,7 @@ export class GuHorizontallyScalingDeploymentPropertiesExperimental implements IA
           const cfnParameterName = getAsgRollingUpdateCfnParameterName(autoScalingGroup);
           this.asgToParamMap.set(
             asgNodeId,
-            new CfnParameter(this.stack, cfnParameterName, {
+            new CfnParameter(stack, cfnParameterName, {
               type: "Number",
             }),
           );

src/riff-raff-yaml-file/deployments/update-parameters.ts

@@ -0,0 +1,23 @@
+import type { RiffRaffDeployment, RiffRaffDeploymentParameters, RiffRaffDeployments } from "../types";
+
+/**
+ * Mutate the parameters of a Riff-Raff deployment.
+ */
+export function updateDeploymentParameters(
+  deployments: RiffRaffDeployments,
+  deployment: RiffRaffDeployment,
+  additionalParameters: RiffRaffDeploymentParameters,
+) {
+  const currentDeployment = deployments.get(deployment.name);
+  if (!currentDeployment) {
+    throw new Error(`Unable to find deployment ${deployment.name}`);
+  }
+
+  deployments.set(deployment.name, {
+    ...currentDeployment,
+    parameters: {
+      ...currentDeployment.parameters,
+      ...additionalParameters,
+    },
+  });
+}

src/riff-raff-yaml-file/index.test.ts

@@ -1525,4 +1525,113 @@ describe("The RiffRaffYamlFile class", () => {
       "
     `);
   });
+
+  it("Should include minInstancesInServiceParameters when GuEc2AppExperimental has a scaling policy (multiple stacks)", () => {
+    const app = new App({ outdir: "/tmp/cdk.out" });
+
+    class MyApplicationStack extends GuStack {
+      public readonly asg: GuAutoScalingGroup;
+
+      // eslint-disable-next-line custom-rules/valid-constructors -- unit testing
+      constructor(app: App, id: string, props: GuStackProps) {
+        super(app, id, props);
+
+        const appName = "my-app";
+
+        const { autoScalingGroup } = new GuEc2AppExperimental(this, {
+          app: appName,
+          instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MICRO),
+          access: { scope: AccessScope.PUBLIC },
+          userData: {
+            distributable: {
+              fileName: `${appName}.deb`,
+              executionStatement: `dpkg -i /${appName}/${appName}.deb`,
+            },
+          },
+          certificateProps: {
+            domainName: "rip.gu.com",
+          },
+          monitoringConfiguration: { noMonitoring: true },
+          instanceMetricGranularity: "5Minute",
+          scaling: {
+            minimumInstances: 1,
+          },
+          applicationPort: 9000,
+          imageRecipe: "arm64-bionic-java11-deploy-infrastructure",
+          buildIdentifier: "TEST",
+        });
+
+        new CfnScalingPolicy(autoScalingGroup, "ScaleOut", {
+          autoScalingGroupName: autoScalingGroup.autoScalingGroupName,
+          policyType: "SimpleScaling",
+          adjustmentType: "ChangeInCapacity",
+          scalingAdjustment: 1,
+        });
+
+        this.asg = autoScalingGroup;
+      }
+    }
+
+    new MyApplicationStack(app, "my-stack-CODE", {
+      stack: "test",
+      stage: "CODE",
+      env: { region: "eu-west-1" },
+    });
+
+    new MyApplicationStack(app, "my-stack-PROD", {
+      stack: "test",
+      stage: "PROD",
+      env: { region: "eu-west-1" },
+    });
+
+    // Ensure the Aspects are invoked...
+    app.synth();
+
+    // ...so that the CFN Parameters are added to the template, to then be processed by the `RiffRaffYamlFile`
+    const actual = new RiffRaffYamlFile(app).toYAML();
+
+    expect(actual).toMatchInlineSnapshot(`
+      "allowedStages:
+        - CODE
+        - PROD
+      deployments:
+        asg-upload-eu-west-1-test-my-app:
+          type: autoscaling
+          actions:
+            - uploadArtifacts
+          regions:
+            - eu-west-1
+          stacks:
+            - test
+          app: my-app
+          parameters:
+            bucketSsmLookup: true
+            prefixApp: true
+          contentDirectory: my-app
+        cfn-eu-west-1-test-my-application-stack:
+          type: cloud-formation
+          regions:
+            - eu-west-1
+          stacks:
+            - test
+          app: my-application-stack
+          contentDirectory: /tmp/cdk.out
+          parameters:
+            templateStagePaths:
+              CODE: my-stack-CODE.template.json
+              PROD: my-stack-PROD.template.json
+            amiParametersToTags:
+              AMIMyapp:
+                BuiltBy: amigo
+                AmigoStage: PROD
+                Recipe: arm64-bionic-java11-deploy-infrastructure
+                Encrypted: 'true'
+            minInstancesInServiceParameters:
+              MinInstancesInServiceFormyapp:
+                App: my-app
+          dependencies:
+            - asg-upload-eu-west-1-test-my-app
+      "
+    `);
+  });
 });

src/riff-raff-yaml-file/index.ts

@@ -1,6 +1,7 @@
 import { writeFileSync } from "fs";
 import path from "path";
 import type { App } from "aws-cdk-lib";
+import { Aspects } from "aws-cdk-lib";
 import { Token } from "aws-cdk-lib";
 import type { CfnAutoScalingGroup } from "aws-cdk-lib/aws-autoscaling";
 import { dump } from "js-yaml";
@@ -15,13 +16,15 @@ import {
   getMinInstancesInServiceParameters,
 } from "./deployments/cloudformation";
 import { updateLambdaDeployment, uploadLambdaArtifact } from "./deployments/lambda";
+import { updateDeploymentParameters } from "./deployments/update-parameters";
 import { groupByClassNameStackRegionStage } from "./group-by";
 import type {
   GroupedCdkStacks,
   Region,
   RiffRaffDeployment,
   RiffRaffDeploymentName,
   RiffRaffDeploymentProps,
+  RiffRaffDeployments,
   RiffRaffYaml,
   StackTag,
   StageTag,
@@ -219,7 +222,7 @@ export class RiffRaffYamlFile {
 
     this.outdir = app.outdir;
 
-    const deployments = new Map<RiffRaffDeploymentName, RiffRaffDeploymentProps>();
+    const deployments: RiffRaffDeployments = new Map<RiffRaffDeploymentName, RiffRaffDeploymentProps>();
 
     const groupedStacks: GroupedCdkStacks = groupByClassNameStackRegionStage(this.allCdkStacks);
 
@@ -274,25 +277,29 @@ export class RiffRaffYamlFile {
             deployments.set(asgDeployment.name, asgDeployment.props);
           });
 
-          const amiParametersToTags = getAmiParameters(autoscalingGroups);
-
-          const minInServiceParamMap =
-            GuHorizontallyScalingDeploymentPropertiesExperimental.getInstance(stack).asgToParamMap;
-          const minInServiceAsgs = autoscalingGroups.filter((asg) => minInServiceParamMap.has(asg.node.id));
-          const minInstancesInServiceParameters = getMinInstancesInServiceParameters(minInServiceAsgs);
+          // only add the `amiParametersToTags` property if there are some
+          if (autoscalingGroups.length > 0) {
+            updateDeploymentParameters(deployments, cfnDeployment, {
+              amiParametersToTags: getAmiParameters(autoscalingGroups),
+            });
+          }
 
-          deployments.set(cfnDeployment.name, {
-            ...cfnDeployment.props,
-            parameters: {
-              ...cfnDeployment.props.parameters,
+          const maybeScalingDeploymentPropertiesAspect = Aspects.of(stack).all.find(
+            (_): _ is GuHorizontallyScalingDeploymentPropertiesExperimental =>
+              _ instanceof GuHorizontallyScalingDeploymentPropertiesExperimental,
+          );
 
-              // only add the `amiParametersToTags` property if there are some
-              ...(autoscalingGroups.length > 0 && { amiParametersToTags }),
+          if (maybeScalingDeploymentPropertiesAspect) {
+            const { asgToParamMap } = maybeScalingDeploymentPropertiesAspect;
+            const minInServiceAsgs = autoscalingGroups.filter((asg) => asgToParamMap.has(asg.node.id));
 
-              // only add the `minInstancesInServiceParameters` property if there are some
-              ...(minInServiceAsgs.length > 0 && { minInstancesInServiceParameters }),
-            },
-          });
+            // only add the `minInstancesInServiceParameters` property if there are some
+            if (minInServiceAsgs.length > 0) {
+              updateDeploymentParameters(deployments, cfnDeployment, {
+                minInstancesInServiceParameters: getMinInstancesInServiceParameters(minInServiceAsgs),
+              });
+            }
+          }
         });
       });
     });

src/riff-raff-yaml-file/types.ts

@@ -43,3 +43,5 @@ export type GroupedCdkStacks = Record<
   ClassName,
   Record<StackTag, Record<Region, Record<StageTag, CdkStacksDifferingOnlyByStage>>>
 >;
+
+export type RiffRaffDeployments = Map<RiffRaffDeploymentName, RiffRaffDeploymentProps>;

src/utils/singleton.ts

@@ -1,4 +1,5 @@
-import type { Stack } from "aws-cdk-lib/core/lib/stack";
+import { Stack } from "aws-cdk-lib";
+import type { Construct } from "constructs";
 import type { GuStack } from "../constructs/core";
 
 /**
@@ -35,7 +36,6 @@ import type { GuStack } from "../constructs/core";
  *
  * @see https://github.com/aws/aws-cdk/blob/0ea4b19afd639541e5f1d7c1783032ee480c307e/packages/%40aws-cdk/core/lib/private/refs.ts#L47-L50
  */
-export const isSingletonPresentInStack = (stack: GuStack, maybeSingletonInstance?: { stack: Stack }): boolean => {
-  // destructured `maybeSingletonInstance` to support `CfnElement`s (aka parameters) and `Resource`s, which do not share a type
-  return maybeSingletonInstance ? maybeSingletonInstance.stack.node === stack.node : false;
+export const isSingletonPresentInStack = (stack: GuStack, maybeSingletonInstance?: Construct): boolean => {
+  return maybeSingletonInstance ? Stack.of(maybeSingletonInstance) === stack : false;
 };