diff --git a/core/cli/package.json b/core/cli/package.json index 5db3ca7dd..e06de7090 100644 --- a/core/cli/package.json +++ b/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", diff --git a/core/cli/src/plugin/options.ts b/core/cli/src/plugin/options.ts index 5a035e650..7fb521189 100644 --- a/core/cli/src/plugin/options.ts +++ b/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) + 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) + // 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) diff --git a/core/cli/test/options.test.ts b/core/cli/test/options.test.ts index 9555a0d56..f7875a273 100644 --- a/core/cli/test/options.test.ts +++ b/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).value.rcFile?.options.hooks[0].Test.baz).toEqual('bar') + expect((plugin as Valid).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 () => { diff --git a/package-lock.json b/package-lock.json index 7a19ab249..5031d4ed2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -72,6 +72,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", @@ -35040,6 +35041,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" }, @@ -35365,7 +35367,7 @@ }, "devDependencies": { "@dotcom-tool-kit/config": "^1.1.1", - "@relmify/jest-serializer-strip-ansi": "^1.0.2", + "@relmify/jest-serializer-strip-ansi": "1.0.2", "@types/npmcli__map-workspaces": "^3.0.4", "@types/pluralize": "^0.0.33" }, diff --git a/plugins/containerised-app-with-assets/.toolkitrc.yml b/plugins/containerised-app-with-assets/.toolkitrc.yml index dfbe09880..7f980e0c3 100644 --- a/plugins/containerised-app-with-assets/.toolkitrc.yml +++ b/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 diff --git a/plugins/containerised-app/.toolkitrc.yml b/plugins/containerised-app/.toolkitrc.yml index 8cf23caa4..8d8c3a169 100644 --- a/plugins/containerised-app/.toolkitrc.yml +++ b/plugins/containerised-app/.toolkitrc.yml @@ -18,8 +18,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': - DockerAuthCloudsmith - DockerBuild @@ -27,22 +26,13 @@ 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' optionsSchema: ./schema version: 2 diff --git a/plugins/containerised-app/package.json b/plugins/containerised-app/package.json index 061759f33..3a71cc5a8 100644 --- a/plugins/containerised-app/package.json +++ b/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" } diff --git a/plugins/containerised-app/readme.md b/plugins/containerised-app/readme.md index 9d9925232..b849242ac 100644 --- a/plugins/containerised-app/readme.md +++ b/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` | `["ft-com-test-eu"]` | +| `hakoStagingEnvironments` | the set of Hako environments to deploy to in the deploy:staging command | `Array` | `["ft-com-test-eu"]` | +| `hakoProductionEnvironments` | the set of Hako environments to deploy to in the deploy:production command | `Array` | `["ft-com-prod-eu"]` | _(\*) Required._ diff --git a/plugins/containerised-app/schema.js b/plugins/containerised-app/schema.js index b7976baee..a5200c6ba 100644 --- a/plugins/containerised-app/schema.js +++ b/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.` + }) diff --git a/plugins/hako/readme.md b/plugins/hako/readme.md index a79732937..a0e4316b9 100644 --- a/plugins/hako/readme.md +++ b/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` | | _(\*) Required._ diff --git a/plugins/hako/src/tasks/deploy.ts b/plugins/hako/src/tasks/deploy.ts index 8012bbdc4..5daa33e96 100644 --- a/plugins/hako/src/tasks/deploy.ts +++ b/plugins/hako/src/tasks/deploy.ts @@ -10,8 +10,23 @@ import { createHash } from 'node:crypto' const hakoImageName = 'docker.packages.ft.com/financial-times-internal-releases/hako-cli:0.2.6-beta' -const HakoEnvironmentNames = z.enum(['ft-com-prod-eu', 'ft-com-prod-us', 'ft-com-test-eu']) -type HakoEnvironmentNames = (typeof HakoEnvironmentNames.options)[number] +export const HakoEnvironmentName = z.string().transform((val, ctx) => { + const match = val.match(/-(prod|test)-(eu|us)$/) + if (!match) { + ctx.addIssue({ + code: z.ZodIssueCode.invalid_string, + validation: 'regex', + message: 'Hako environment name must end with a stage and region, e.g., -prod-eu' + }) + return z.NEVER + } + return { + name: val, + stage: match[1], + region: match[2] + } +}) +export type HakoEnvironment = z.output const HakoDeploySchema = z .object({ @@ -19,34 +34,32 @@ const HakoDeploySchema = z .boolean() .default(false) .describe('whether to deploy as a temporary review app, used for code review'), - environments: z.array(HakoEnvironmentNames).describe('the Hako environments to deploy an image to') + environments: z.array(HakoEnvironmentName).describe('the Hako environments to deploy an image to') }) .describe('Deploy to ECS via the Hako CLI') -const hakoEnvironments: Record = { - 'ft-com-prod-eu': 'eu-west-1', - 'ft-com-prod-us': 'us-east-1', - 'ft-com-test-eu': 'eu-west-1' +const hakoRegions: Record = { + eu: 'eu-west-1', + us: 'us-east-1' } -const hakoDomains: Record = { - 'ft-com-prod-eu': 'ft-com-prod.ftweb.tech', - 'ft-com-prod-us': 'ft-com-prod.ftweb.tech', - 'ft-com-test-eu': 'ft-com-test.ftweb.tech' +const hakoDomains: Record = { + prod: 'ft-com-prod.ftweb.tech', + test: 'ft-com-test.ftweb.tech' } export { HakoDeploySchema as schema } interface DeploymentOptions { awsCredentials: CIState['awsCredentials'] - environment: HakoEnvironmentNames + environment: HakoEnvironment name: string tag: string } export default class HakoDeploy extends Task<{ task: typeof HakoDeploySchema }> { async deployApp({ awsCredentials, environment, name, tag }: DeploymentOptions): Promise { - this.logger.info(`Deploying image "${name}" with tag "${tag}" to environment "${environment}"`) - const awsRegion = hakoEnvironments[environment] + this.logger.info(`Deploying image "${name}" with tag "${tag}" to environment "${environment.name}"`) + const awsRegion = hakoRegions[environment.region] const commandArgs = [ 'run', '--interactive', @@ -70,9 +83,9 @@ export default class HakoDeploy extends Task<{ task: typeof HakoDeploySchema }> '--app', name, // NOTE: the app name MUST match the image name (for now) '--env', - environment + environment.name ] - const domain = hakoDomains[environment] + const domain = hakoDomains[environment.stage] if (this.options.asReviewApp) { if (!process.env.CIRCLE_BRANCH) { throw new Error( @@ -91,7 +104,7 @@ export default class HakoDeploy extends Task<{ task: typeof HakoDeploySchema }> // Because we can't mount volumes in Docker images on CircleCI we have to // pass the hako config via STDIN if (this.options.asReviewApp) { - const hakoConfigPath = join(process.cwd(), 'hako-config', 'apps', name, environment, 'app.yaml') + const hakoConfigPath = join(process.cwd(), 'hako-config', 'apps', name, environment.name, 'app.yaml') try { const hakoConfig = await readFile(hakoConfigPath, 'utf-8') child.stdin.setDefaultEncoding('utf-8') diff --git a/plugins/monorepo/package.json b/plugins/monorepo/package.json index c9a51dacd..74424bd14 100644 --- a/plugins/monorepo/package.json +++ b/plugins/monorepo/package.json @@ -40,7 +40,7 @@ }, "devDependencies": { "@dotcom-tool-kit/config": "^1.1.1", - "@relmify/jest-serializer-strip-ansi": "^1.0.2", + "@relmify/jest-serializer-strip-ansi": "1.0.2", "@types/npmcli__map-workspaces": "^3.0.4", "@types/pluralize": "^0.0.33" }