From f769014efe3f55f3db48ec3bd7e3e8c613683d69 Mon Sep 17 00:00:00 2001
From: Ivo Murrell <ivo.murrell@ft.com>
Date: Mon, 13 Mar 2023 19:13:36 +0000
Subject: [PATCH 1/2] fix(cli): validate options even when none are given for a
 plugin

Now that we've moved the setting of default options into the zod schema
validation, we need to make sure we're using zod to parse options for
plugins even when no options have been specified explicitly in your
.toolkitrc config. This is also important to ensure that plugins with
required options are having those set.

I've updated the configuration validation code to iterate over the
loaded plugins rather than the loaded options. This means we won't miss
any plugins that haven't had any options explicitly set. We won't miss
any options that aren't associated with a plugin as unused options were
already disallowed. Any options that are created implicitly like this
will be linked to the user's root configuration rather than a nested
.toolkitrc.yml file.
---
 core/cli/src/config.ts | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/core/cli/src/config.ts b/core/cli/src/config.ts
index e0c82db37..d3d284f15 100644
--- a/core/cli/src/config.ts
+++ b/core/cli/src/config.ts
@@ -137,23 +137,35 @@ export function validateConfig(config: ValidPluginsConfig, logger: Logger): Vali
   }
 
   const invalidOptions: InvalidOption[] = []
-  for (const [id, options] of Object.entries(config.options).filter((entry): entry is [
-    string,
-    PluginOptions
-  ] => {
-    const [, options] = entry
-    return !!options && !isConflict(options)
-  })) {
+  for (const [id, plugin] of Object.entries(config.plugins)) {
     const pluginId = id as keyof SchemaOptions
+    const pluginOptions = config.options[pluginId]
+    if (pluginOptions && isConflict(pluginOptions)) {
+      continue
+    }
+
     const pluginSchema = Schemas[pluginId]
     if (!pluginSchema) {
       logger.silly(`skipping validation of ${pluginId} plugin as no schema can be found`)
       continue
     }
-    const result = pluginSchema.safeParse(options.options)
+    const result = pluginSchema.safeParse(pluginOptions?.options ?? {})
     if (result.success) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      validConfig.options[pluginId]!.options = result.data
+      // Set up options entry for plugins that don't have options specified
+      // explicitly. They could still have default options that are set by zod.
+      if (!pluginOptions) {
+        // TypeScript struggles with this type as it sees one side as
+        // `Foo<a | b | c>` and the other as `Foo<a> | Foo<b> | Foo<c>` for
+        // some reason (something to do with the record indexing) and it can't
+        // unify them. But they are equivalent so let's force it with a cast.
+        config.options[pluginId] = {
+          options: result.data,
+          plugin: config.plugins['app root'],
+          forPlugin: plugin
+        } as any // eslint-disable-line @typescript-eslint/no-explicit-any
+      } else {
+        pluginOptions.options = result.data
+      }
     } else {
       invalidOptions.push([id, result.error])
     }

From d6c2a9b7dd0b6395277ba78b9b767bcee86a3284 Mon Sep 17 00:00:00 2001
From: Ivo Murrell <ivo.murrell@ft.com>
Date: Mon, 13 Mar 2023 21:56:05 +0000
Subject: [PATCH 2/2] test(cli): specify required options for tests

Now that zod checks that we have specified all required options we need
to add these to our test configurations so that they don't spuriously
fail.
---
 .../cli/test/files/conflict-resolution/.toolkitrc.yml | 11 +++++++++++
 core/cli/test/files/duplicate/.toolkitrc.yml          | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/core/cli/test/files/conflict-resolution/.toolkitrc.yml b/core/cli/test/files/conflict-resolution/.toolkitrc.yml
index 635b8414b..390ed216c 100644
--- a/core/cli/test/files/conflict-resolution/.toolkitrc.yml
+++ b/core/cli/test/files/conflict-resolution/.toolkitrc.yml
@@ -5,6 +5,17 @@ plugins:
   - '@dotcom-tool-kit/heroku' # for build:remote hook and build:local via npm plugin
 
 options:
+  '@dotcom-tool-kit/heroku':
+    pipeline: tool-kit-test
+    systemCode: tool-kit-test
+    scaling:
+      tool-kit-test:
+        web:
+          size: standard-1x
+          quantity: 1
+  '@dotcom-tool-kit/vault':
+    team: platforms
+    app: tool-kit-test
 
 hooks:
   build:local:
diff --git a/core/cli/test/files/duplicate/.toolkitrc.yml b/core/cli/test/files/duplicate/.toolkitrc.yml
index 01d98a12d..5cbb53077 100644
--- a/core/cli/test/files/duplicate/.toolkitrc.yml
+++ b/core/cli/test/files/duplicate/.toolkitrc.yml
@@ -3,5 +3,16 @@ plugins:
   - '@dotcom-tool-kit/heroku'
 
 options:
+  '@dotcom-tool-kit/heroku':
+    pipeline: tool-kit-test
+    systemCode: tool-kit-test
+    scaling:
+      tool-kit-test:
+        web:
+          size: standard-1x
+          quantity: 1
+  '@dotcom-tool-kit/vault':
+    team: platforms
+    app: tool-kit-test
 
 hooks: