CPP-2497 Support setting custom Hako environments in containerised-app plugins

core/cli/package.json

@@ -21,6 +21,7 @@
   },
   "devDependencies": {
     "@jest/globals": "^29.7.0",
+    "@relmify/jest-serializer-strip-ansi": "1.0.2",
     "@types/lodash": "^4.17.16",
     "@types/pluralize": "^0.0.33",
     "globby": "^10.0.2",

core/cli/src/plugin/options.ts

@@ -33,7 +33,16 @@ export const validatePluginOptions = async (
     if (schemaEntrypoint) {
       const schema = await importSchemaEntryPoint({ plugin, modulePath: schemaEntrypoint })
       if (!schema.valid) {
-        invalidOptions.push([id, new z.ZodError([])])
+        invalidOptions.push([
+          id,
+          new z.ZodError([
+            {
+              message: schema.reasons.join('\n'),
+              code: z.ZodIssueCode.custom,
+              path: []
+            }
+          ])
+        ])
         continue
       }
       pluginSchema = schema.value
@@ -109,12 +118,13 @@ export const substituteOptionTags = (plugin: Plugin, config: ValidPluginsConfig)
     if (Array.isArray(node)) {
       return reduceValidated(node.map((item, i) => deeplySubstitute(item, [...path, i])))
     } else if (node && typeof node === 'object') {
-      const entries = Object.entries(node)
+      const entries: [string, unknown][] = Object.entries(node)
       const substituted: Validated<[string, unknown]>[] = []
       for (const entry of entries) {
         const subbedEntry = reduceValidated(
-          // allow both keys and (string) values to be substituted by options
-          entry.map((val) => {
+          // allow both keys and values to be substituted by options
+          entry.map((val, i) => {
+            const isKey = i === 0
             if (typeof val === 'string' && val.startsWith(toolKitOptionIdent)) {
               // check the tag path each time so that we can have a separate
               // error for each incorrect use of the tag
@@ -126,26 +136,23 @@ export const substituteOptionTags = (plugin: Plugin, config: ValidPluginsConfig)
                 // identifier
                 const optionPath = val.slice(toolKitOptionIdent.length)
                 const resolvedOption = resolveOptionPath(optionPath)
-                if (typeof resolvedOption === 'string') {
-                  return valid(resolvedOption)
-                } else {
+                if (isKey && typeof resolvedOption !== 'string') {
                   return invalid([
-                    `Option '${optionPath}' referenced at path '${path.join(
+                    `Option '${optionPath}' for the key at path '${path.join(
                       '.'
                     )}' does not resolve to a string (resolved to ${resolvedOption})`
                   ])
+                } else {
+                  return valid(resolvedOption)
                 }
               }
             } else {
               return valid(val)
             }
           })
-        )
+        ) as Validated<[string, unknown]>
         if (!subbedEntry.valid) {
-          /* eslint-disable-next-line @typescript-eslint/no-explicit-any --
-           * Invalid objects don't need to match the inner type
-           **/
-          substituted.push(subbedEntry as Validated<any>)
+          substituted.push(subbedEntry)
           continue
         }
 
@@ -167,10 +174,9 @@ export const substituteOptionTags = (plugin: Plugin, config: ValidPluginsConfig)
             if (subbedValues.valid) {
               substituted.push(...Object.entries(subbedValues.value as object).map((v) => valid(v)))
             } else {
-              /* eslint-disable-next-line @typescript-eslint/no-explicit-any --
-               * Invalid objects don't need to match the inner type
-               **/
-              substituted.push(subbedValues as Validated<any>)
+              // safe to cast as invalid objects don't need to match the inner
+              // type
+              substituted.push(subbedValues as Validated<[string, unknown]>)
             }
           }
         } else {
@@ -195,7 +201,9 @@ export const substituteOptionTags = (plugin: Plugin, config: ValidPluginsConfig)
   }
   if (plugin.rcFile) {
     plugin.rcFile = deeplySubstitute(plugin.rcFile, []).unwrap(
-      'cannot reference plugin options when specifying options'
+      `error when subsituting options (i.e., resolving ${styles.code('!toolkit/option')} and ${styles.code(
+        '!toolkit/if-defined'
+      )} tags)`
     ) as RCFile
   }
   config.resolutionTrackers.substitutedPlugins.add(plugin.id)

core/cli/test/options.test.ts

@@ -5,34 +5,44 @@ import * as fs from 'node:fs/promises'
 import type { Valid } from '@dotcom-tool-kit/validated'
 import type { Plugin } from '@dotcom-tool-kit/plugin'
 
+import { stripAnsi } from '@relmify/jest-serializer-strip-ansi'
 import winston, { type Logger } from 'winston'
+import * as YAML from 'yaml'
 
 const logger = winston as unknown as Logger
 
 jest.mock('node:fs/promises')
 const mockedFs = jest.mocked(fs)
 
+expect.addSnapshotSerializer(stripAnsi)
+
 // convince text editors (well, nvim) to highlight strings as YAML
 const yaml = (str) => str
 
 describe('option substitution', () => {
-  it('should substitute option tag with option value', async () => {
+  it.each([
+    { type: 'string', value: 'bar' },
+    { type: 'number', value: 137 },
+    { type: 'boolean', value: true },
+    { type: 'array', value: ['apple', 'pear'] },
+    { type: 'object', value: { apple: 1, pear: true } }
+  ])('should substitute option tag with $type value', async ({ value }) => {
     mockedFs.readFile.mockResolvedValueOnce(
-      yaml(`
+      `
 options:
   plugins:
     test:
-      foo: bar
+      foo: ${YAML.stringify(value, { collectionStyle: 'flow' })}
   hooks:
     - Test:
         baz: !toolkit/option 'test.foo'
-`)
+`
     )
 
     const config = await loadConfig(logger, { validate: false, root: process.cwd() })
     const plugin = config.plugins['app root']
     expect(plugin.valid).toBe(true)
-    expect((plugin as Valid<Plugin>).value.rcFile?.options.hooks[0].Test.baz).toEqual('bar')
+    expect((plugin as Valid<Plugin>).value.rcFile?.options.hooks[0].Test.baz).toEqual(value)
   })
 
   it('should substitute defined tag with value when defined', async () => {
@@ -102,9 +112,39 @@ options:
 `)
     )
 
-    expect(loadConfig(logger, { validate: false, root: process.cwd() })).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"cannot reference plugin options when specifying options"`
+    expect.assertions(1)
+    try {
+      await loadConfig(logger, { validate: false, root: process.cwd() })
+    } catch (error) {
+      expect(error.details).toMatchInlineSnapshot(
+        `"YAML tag referencing options used at path 'options.plugins.test.foo'"`
+      )
+    }
+  })
+
+  it('should disallow option tag with non-string value in key position', async () => {
+    mockedFs.readFile.mockResolvedValueOnce(
+      yaml(`
+options:
+  plugins:
+    test:
+      foo:
+        - apple
+        - pear
+  hooks:
+    - Test:
+        !toolkit/option 'test.foo': baz
+`)
     )
+
+    expect.assertions(1)
+    try {
+      await loadConfig(logger, { validate: false, root: process.cwd() })
+    } catch (error) {
+      expect(error.details).toMatchInlineSnapshot(
+        `"Option 'test.foo' for the key at path 'options.hooks.0.Test' does not resolve to a string (resolved to apple,pear)"`
+      )
+    }
   })
 
   it('should print multiple invalid tags in error', async () => {

plugins/containerised-app-with-assets/.toolkitrc.yml

@@ -22,8 +22,7 @@ commands:
         roleArn: !toolkit/option '@dotcom-tool-kit/containerised-app.awsRoleArnStaging'
     - HakoDeploy:
         asReviewApp: true
-        environments:
-          - ft-com-test-eu
+        environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoReviewEnvironments'
   'deploy:staging':
     - Webpack:
         envName: production
@@ -34,21 +33,12 @@ commands:
     - AwsAssumeRole:
         roleArn: !toolkit/option '@dotcom-tool-kit/containerised-app.awsRoleArnStaging'
     - HakoDeploy:
-        environments:
-          - ft-com-test-eu
+        environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoStagingEnvironments'
   'deploy:production':
     - DockerAuthCloudsmith
     - AwsAssumeRole:
         roleArn: !toolkit/option '@dotcom-tool-kit/containerised-app.awsRoleArnProduction'
-    # HACK: the `environments` property under `HakoDeploy` gets fully overridden by the `environments`
-    # property under the !toolkit/if-defined if multiregion is true. We'll refactor this later when
-    # we decide what new YAML tags we need
     - HakoDeploy:
-        environments:
-          - ft-com-prod-eu
-        !toolkit/if-defined '@dotcom-tool-kit/containerised-app.multiregion':
-          environments:
-            - ft-com-prod-eu
-            - ft-com-prod-us
+        environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoProductionEnvironments'
 
 version: 2

plugins/containerised-app/.toolkitrc.yml

@@ -18,31 +18,21 @@ commands:
         roleArn: !toolkit/option '@dotcom-tool-kit/containerised-app.awsRoleArnStaging'
     - HakoDeploy:
         asReviewApp: true
-        environments:
-          - ft-com-test-eu
+        environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoReviewEnvironments'
   'deploy:staging':
     - DockerAuthCloudsmith
     - DockerBuild
     - DockerPush
     - AwsAssumeRole:
         roleArn: !toolkit/option '@dotcom-tool-kit/containerised-app.awsRoleArnStaging'
     - HakoDeploy:
-        environments:
-          - ft-com-test-eu
+        environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoStagingEnvironments'
   'deploy:production':
     - DockerAuthCloudsmith
     - AwsAssumeRole:
         roleArn: !toolkit/option '@dotcom-tool-kit/containerised-app.awsRoleArnProduction'
-    # HACK: the `environments` property under `HakoDeploy` gets fully overridden by the `environments`
-    # property under the !toolkit/if-defined if multiregion is true. We'll refactor this later when
-    # we decide what new YAML tags we need
     - HakoDeploy:
-        environments:
-          - ft-com-prod-eu
-        !toolkit/if-defined '@dotcom-tool-kit/containerised-app.multiregion':
-          environments:
-            - ft-com-prod-eu
-            - ft-com-prod-us
+        environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoProductionEnvironments'
 
 optionsSchema: ./schema
 version: 2

plugins/containerised-app/package.json

@@ -32,6 +32,7 @@
     "@dotcom-tool-kit/docker": "^0.4.2",
     "@dotcom-tool-kit/doppler": "^2.2.2",
     "@dotcom-tool-kit/hako": "^0.1.12",
+    "@dotcom-tool-kit/logger": "^4.2.1",
     "@dotcom-tool-kit/node": "^4.3.3",
     "zod": "^3.24.4"
   }

plugins/containerised-app/readme.md

@@ -49,11 +49,13 @@ See the relevant documentation for further options:
 
 ### `@dotcom-tool-kit/containerised-app`
 
-| Property                        | Description                                                   | Type                                             |
-| :------------------------------ | :------------------------------------------------------------ | :----------------------------------------------- |
-| **`awsRoleArnStaging`** (\*)    | the ARN of an IAM role to assume when deploying to staging    | `string` (_regex: `/^arn:aws:iam::\d+:role\//`_) |
-| **`awsRoleArnProduction`** (\*) | the ARN of an IAM role to assume when deploying to production | `string` (_regex: `/^arn:aws:iam::\d+:role\//`_) |
-| `multiregion`                   | Whether the app is deployed across both EU and US regions     | `true`                                           |
+| Property                        | Description                                                                | Type                                             | Default              |
+| :------------------------------ | :------------------------------------------------------------------------- | :----------------------------------------------- | :------------------- |
+| **`awsRoleArnStaging`** (\*)    | the ARN of an IAM role to assume when deploying to staging                 | `string` (_regex: `/^arn:aws:iam::\d+:role\//`_) |                      |
+| **`awsRoleArnProduction`** (\*) | the ARN of an IAM role to assume when deploying to production              | `string` (_regex: `/^arn:aws:iam::\d+:role\//`_) |                      |
+| `hakoReviewEnvironments`        | the set of Hako environments to deploy to in the deploy:review command     | `Array<string>`                                  | `["ft-com-test-eu"]` |
+| `hakoStagingEnvironments`       | the set of Hako environments to deploy to in the deploy:staging command    | `Array<string>`                                  | `["ft-com-test-eu"]` |
+| `hakoProductionEnvironments`    | the set of Hako environments to deploy to in the deploy:production command | `Array<string>`                                  | `["ft-com-prod-eu"]` |
 
 _(\*) Required._
 <!-- end autogenerated docs -->

plugins/containerised-app/schema.js

@@ -1,17 +1,43 @@
 const z = require('zod')
+const { HakoEnvironmentName } = require('@dotcom-tool-kit/hako/lib/tasks/deploy')
+const { styles } = require('@dotcom-tool-kit/logger')
 
-module.exports = z.object({
-  awsRoleArnStaging: z
-    .string()
-    .regex(/^arn:aws:iam::\d+:role\//, 'Role ARN must be a full IAM role ARN including account number')
-    .describe('the ARN of an IAM role to assume when deploying to staging'),
-  awsRoleArnProduction: z
-    .string()
-    .regex(/^arn:aws:iam::\d+:role\//, 'Role ARN must be a full IAM role ARN including account number')
-    .describe('the ARN of an IAM role to assume when deploying to production'),
-  // HACK: must be either `true` or `undefined` because of the way !toolkit/if-defined works
-  multiregion: z
-    .literal(true)
-    .optional()
-    .describe('Whether the app is deployed across both EU and US regions')
-})
+// We don't want to transform the environment yet as the value will get
+// substituted into the HakoDeploy task options where it will then get
+// transformed. The transform isn't idempotent so will result in a parse error
+// if applied twice.
+const HakoEnvironmentNameInner = HakoEnvironmentName.innerType()
+
+module.exports = z
+  .object({
+    awsRoleArnStaging: z
+      .string()
+      .regex(/^arn:aws:iam::\d+:role\//, 'Role ARN must be a full IAM role ARN including account number')
+      .describe('the ARN of an IAM role to assume when deploying to staging'),
+    awsRoleArnProduction: z
+      .string()
+      .regex(/^arn:aws:iam::\d+:role\//, 'Role ARN must be a full IAM role ARN including account number')
+      .describe('the ARN of an IAM role to assume when deploying to production'),
+    hakoReviewEnvironments: z
+      .array(HakoEnvironmentNameInner)
+      .default(['ft-com-test-eu'])
+      .describe('the set of Hako environments to deploy to in the deploy:review command'),
+    hakoStagingEnvironments: z
+      .array(HakoEnvironmentNameInner)
+      .default(['ft-com-test-eu'])
+      .describe('the set of Hako environments to deploy to in the deploy:staging command'),
+    hakoProductionEnvironments: z
+      .array(HakoEnvironmentNameInner)
+      .default(['ft-com-prod-eu'])
+      .describe('the set of Hako environments to deploy to in the deploy:production command')
+  })
+  .passthrough()
+  .refine((options) => !('multiregion' in options), {
+    message: `the option ${styles.code('multiregion')} has been replaced by ${styles.code(
+      'hakoReviewEnvironments'
+    )}, ${styles.code('hakoStagingEnvironments')}, and ${styles.code(
+      'hakoProductionEnvironments'
+    )}. set ${styles.code('hakoProductionEnvironments')} to ${styles.code(
+      '[ft-com-prod-eu, ft-com-prod-us]'
+    )} for equivalent behaviour.`
+  })

plugins/hako/readme.md

@@ -23,10 +23,10 @@ plugins:
 Deploy to ECS via the Hako CLI
 #### Task options
 
-| Property                | Description                                                       | Type                                                              | Default |
-| :---------------------- | :---------------------------------------------------------------- | :---------------------------------------------------------------- | :------ |
-| `asReviewApp`           | whether to deploy as a temporary review app, used for code review | `boolean`                                                         | `false` |
-| **`environments`** (\*) | the Hako environments to deploy an image to                       | `Array<'ft-com-prod-eu' \| 'ft-com-prod-us' \| 'ft-com-test-eu'>` |         |
+| Property                | Description                                                       | Type            | Default |
+| :---------------------- | :---------------------------------------------------------------- | :-------------- | :------ |
+| `asReviewApp`           | whether to deploy as a temporary review app, used for code review | `boolean`       | `false` |
+| **`environments`** (\*) | the Hako environments to deploy an image to                       | `Array<string>` |         |
 
 _(\*) Required._
 <!-- end autogenerated docs -->