fix: validate secrets before action resolution (#6822)

* fix: validate secrets before action resolution

 Validate missing secrets when instantiating a `ResolveActionTask` , rather than when adding configs.
 This would ensure we only get the error when missing secrets would actually cause a failure for the given command, while still failing relatively early even if the action happens to use another action’s outputs.

* test: ensure that `ResolveActionTask` constructor throws correct error if action references missing secrets when user is not logged in

* test: ensure that `scanAndAddConfigs` does not throw if an action references missing secrets

The missing secrets check was moved to `ResolveActionTask` constructor.

* test: ensure that `scanAndAddConfigs` does not throw if an action references missing secrets

The missing secrets check was moved to `ResolveActionTask` constructor.

* test: ensure that `scanAndAddConfigs` does not throw if a module references missing secrets

* test: ensure that `scanAndAddConfigs` does not throw if a workflow references missing secrets

* test: remove unused directory

* test: rename test projects

* test: create surrounding context for missing secrets in "scanAndAddConfig" tests suite

* fix: fail-fast before workflow execution if any step references missing secrets

* test: restore the state of the shared test data after the secrets test

* test: add assertions for Garden state

* Not logged in
* Has no secrets

* refactor: extract helper function to compose error message

* chore: fix lint error

* refactor: convert positional args to param object in `throwOnMissingSecretKeys`

* refactor: convert positional args to param object in `detectMissingSecretKeys`

* refactor: extract function to create error message footer

* chore: make function `throwOnMissingSecretKeys` aware of the login state

* test: fix test to expect an error

* chore: more informative error message footer on missing secrets

The error message depends on the login status to be less confusing.

* test: ensure that `ResolveActionTask` constructor throws correct error if module references missing secrets

* fix: skip `${secrets.*}` references evaluation in `ModuleResolver`

Some secrets might be missing and not resolvable now,
because the missing secrets check was moved to `ResolveActionTask` constructor.

* chore: add a link to the secrets guide in the error message

* improvement: better error message if the secrets have not been fetched

* test: re-work negative tests a bit

core/src/commands/workflow.ts

@@ -34,6 +34,8 @@ import { getCustomCommands } from "./custom.js"
 import { getBuiltinCommands } from "./commands.js"
 import { styles } from "../logger/styles.js"
 import { deepEvaluate } from "../template/evaluate.js"
+import { throwOnMissingSecretKeys } from "../config/secrets.js"
+import { RemoteSourceConfigContext } from "../config/template-contexts/project.js"
 
 const { ensureDir, writeFile } = fsExtra
 
@@ -79,6 +81,15 @@ export class WorkflowCommand extends Command<Args, {}> {
     // Prepare any configured files before continuing
     const workflow = await garden.getWorkflowConfig(args.workflow)
 
+    throwOnMissingSecretKeys({
+      configs: [workflow],
+      context: new RemoteSourceConfigContext(garden, garden.variables),
+      secrets: garden.secrets,
+      prefix: workflow.kind,
+      isLoggedIn: garden.isLoggedIn(),
+      log,
+    })
+
     // Merge any workflow-level environment variables into process.env.
     for (const [key, value] of Object.entries(toEnvVars(workflow.envVars))) {
       process.env[key] = value

core/src/config/secrets.ts

@@ -17,31 +17,33 @@ import difference from "lodash-es/difference.js"
 import { ConfigurationError } from "../exceptions.js"
 import { CONTEXT_RESOLVE_KEY_NOT_FOUND } from "../template/ast.js"
 
-/**
- * Gathers secret references in configs and throws an error if one or more referenced secrets isn't present (or has
- * blank values) in the provided secrets map.
- *
- * Prefix should be e.g. "Module" or "Provider" (used when generating error messages).
- */
-export function throwOnMissingSecretKeys(
-  configs: ObjectWithName[],
-  context: ConfigContext,
-  secrets: StringMap,
-  prefix: string,
-  log?: Log
-) {
-  const allMissing: [string, ContextKeySegment[]][] = [] // [[key, missing keys]]
-  for (const config of configs) {
-    const missing = detectMissingSecretKeys(config, context, secrets)
-    if (missing.length > 0) {
-      allMissing.push([config.name, missing])
-    }
+const secretsGuideLink = "https://cloud.docs.garden.io/features/secrets"
+
+function getMessageFooter({ loadedKeys, isLoggedIn }: { loadedKeys: string[]; isLoggedIn: boolean }) {
+  if (!isLoggedIn) {
+    return `You are not logged in. Log in to get access to Secrets in Garden Cloud. See also ${secretsGuideLink}`
   }
 
-  if (allMissing.length === 0) {
-    return
+  if (loadedKeys.length === 0) {
+    return deline`
+      Note: You can manage secrets in Garden Cloud. No secrets have been defined for the current project and environment. See also ${secretsGuideLink}
+    `
+  } else {
+    return `Secret keys with loaded values: ${loadedKeys.join(", ")}`
   }
+}
 
+function composeErrorMessage({
+  allMissing,
+  secrets,
+  prefix,
+  isLoggedIn,
+}: {
+  allMissing: [string, ContextKeySegment[]][]
+  secrets: StringMap
+  prefix: string
+  isLoggedIn: boolean
+}): string {
   const descriptions = allMissing.map(([key, missing]) => `${prefix} ${key}: ${missing.join(", ")}`)
   /**
    * Secret keys with empty values should have resulted in an error by this point, but we filter on keys with
@@ -50,37 +52,73 @@ export function throwOnMissingSecretKeys(
   const loadedKeys = Object.entries(secrets)
     .filter(([_key, value]) => value)
     .map(([key, _value]) => key)
-  let footer: string
-  if (loadedKeys.length === 0) {
-    footer = deline`
-      Note: No secrets have been loaded. If you have defined secrets for the current project and environment in Garden
-      Cloud, this may indicate a problem with your configuration.
-    `
-  } else {
-    footer = `Secret keys with loaded values: ${loadedKeys.join(", ")}`
-  }
+
+  const footer = getMessageFooter({ loadedKeys, isLoggedIn })
+
   const errMsg = dedent`
     The following secret names were referenced in configuration, but are missing from the secrets loaded remotely:
 
     ${descriptions.join("\n\n")}
 
     ${footer}
   `
+  return errMsg
+}
+
+/**
+ * Gathers secret references in configs and throws an error if one or more referenced secrets isn't present (or has
+ * blank values) in the provided secrets map.
+ *
+ * Prefix should be e.g. "Module" or "Provider" (used when generating error messages).
+ */
+export function throwOnMissingSecretKeys({
+  configs,
+  context,
+  secrets,
+  prefix,
+  isLoggedIn,
+  log,
+}: {
+  configs: ObjectWithName[]
+  context: ConfigContext
+  secrets: StringMap
+  prefix: string
+  isLoggedIn: boolean
+  log?: Log
+}) {
+  const allMissing: [string, ContextKeySegment[]][] = [] // [[key, missing keys]]
+  for (const config of configs) {
+    const missing = detectMissingSecretKeys({ obj: config, context, secrets })
+    if (missing.length > 0) {
+      allMissing.push([config.name, missing])
+    }
+  }
+
+  if (allMissing.length === 0) {
+    return
+  }
+
+  const errMsg = composeErrorMessage({ allMissing, secrets, prefix, isLoggedIn })
   if (log) {
     log.silly(() => errMsg)
   }
+
   throw new ConfigurationError({ message: errMsg })
 }
 
 /**
  * Collects template references to secrets in obj, and returns an array of any secret keys referenced in it that
  * aren't present (or have blank values) in the provided secrets map.
  */
-export function detectMissingSecretKeys(
-  obj: ObjectWithName,
-  context: ConfigContext,
+export function detectMissingSecretKeys({
+  obj,
+  context,
+  secrets,
+}: {
+  obj: ObjectWithName
+  context: ConfigContext
   secrets: StringMap
-): ContextKeySegment[] {
+}): ContextKeySegment[] {
   const requiredKeys: ContextKeySegment[] = []
   const generator = getContextLookupReferences(
     visitAll({

core/src/garden.ts

@@ -805,13 +805,14 @@ export class Garden {
         providerNames = getNames(rawConfigs)
       }
 
-      throwOnMissingSecretKeys(
-        rawConfigs,
-        new RemoteSourceConfigContext(this, this.variables),
-        this.secrets,
-        "Provider",
-        log
-      )
+      throwOnMissingSecretKeys({
+        configs: rawConfigs,
+        context: new RemoteSourceConfigContext(this, this.variables),
+        secrets: this.secrets,
+        prefix: "Provider",
+        isLoggedIn: this.isLoggedIn(),
+        log,
+      })
 
       // As an optimization, we return immediately if all requested providers are already resolved
       const alreadyResolvedProviders = providerNames.map((name) => this.resolvedProviders[name]).filter(Boolean)
@@ -1440,16 +1441,6 @@ export class Garden {
       )
       const groupedResources = groupBy(allResources, "kind")
 
-      for (const [kind, configs] of Object.entries(groupedResources)) {
-        throwOnMissingSecretKeys(
-          configs,
-          new RemoteSourceConfigContext(this, this.variables),
-          this.secrets,
-          kind,
-          this.log
-        )
-      }
-
       let rawModuleConfigs = [...((groupedResources.Module as ModuleConfig[]) || [])]
       const rawWorkflowConfigs = (groupedResources.Workflow as WorkflowConfig[]) || []
       const rawConfigTemplateResources = (groupedResources[configTemplateKind] as ConfigTemplateResource[]) || []

core/src/resolve-module.ts

@@ -1236,17 +1236,17 @@ function partiallyEvaluateModule<Input extends ParsedTemplate>(config: Input, co
   /**
    * Returns false if the unresolved template value contains runtime references, in order to skip resolving it at this point.
    */
-  const skipRuntimeReferences = (value: UnresolvedTemplateValue) => {
+  const skipRuntimeAndSecretsReferences = (value: UnresolvedTemplateValue) => {
     if (
       someReferences({
         value,
         context,
         opts: {},
         onlyEssential: true,
-        matcher: (ref) => ref.keyPath[0] === "runtime",
+        matcher: (ref) => ref.keyPath[0] === "runtime" || ref.keyPath[0] === "secrets",
       })
     ) {
-      return false // do not evaluate runtime references
+      return false // do not evaluate runtime and secrets references
     }
 
     return true
@@ -1262,7 +1262,7 @@ function partiallyEvaluateModule<Input extends ParsedTemplate>(config: Input, co
         keepEscapingInTemplateStrings: true,
       },
     },
-    skipRuntimeReferences
+    skipRuntimeAndSecretsReferences
   )
 
   // any leftover unresolved template values are now turned back into the raw form

core/src/tasks/resolve-action.ts

@@ -6,7 +6,13 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import type { ActionTaskStatusParams, BaseTask, ValidResultType, ActionTaskProcessParams } from "./base.js"
+import type {
+  ActionTaskStatusParams,
+  BaseTask,
+  ValidResultType,
+  ActionTaskProcessParams,
+  BaseActionTaskParams,
+} from "./base.js"
 import { BaseActionTask } from "./base.js"
 import { Profile } from "../util/profiling.js"
 import type {
@@ -31,6 +37,8 @@ import { describeActionConfig } from "../actions/base.js"
 import { InputContext } from "../config/template-contexts/input.js"
 import { VariablesContext } from "../config/template-contexts/variables.js"
 import type { GroupConfig } from "../config/group.js"
+import { throwOnMissingSecretKeys } from "../config/secrets.js"
+import { RemoteSourceConfigContext } from "../config/template-contexts/project.js"
 
 export interface ResolveActionResults<T extends Action> extends ValidResultType {
   state: ActionState
@@ -48,6 +56,18 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
   override readonly executeConcurrencyLimit = 10
   override readonly statusConcurrencyLimit = 10
 
+  constructor(params: BaseActionTaskParams<T>) {
+    super(params)
+    throwOnMissingSecretKeys({
+      configs: [this.action.getConfig()],
+      context: new RemoteSourceConfigContext(params.garden, params.garden.variables),
+      secrets: params.garden.secrets,
+      prefix: this.action.kind,
+      isLoggedIn: params.garden.isLoggedIn(),
+      log: this.log,
+    })
+  }
+
   getDescription() {
     return `resolve ${this.action.longDescription()}`
   }

core/test/data/missing-secrets/action/garden.yml

@@ -0,0 +1,8 @@
+apiVersion: garden.io/v1
+kind: Project
+name: test-project-missing-secrets-in-action
+environments:
+  - name: local
+providers:
+  - name: local-kubernetes
+    environments: [ local ]

core/test/data/missing-secrets/action/run.garden.yml

@@ -0,0 +1,6 @@
+kind: Run
+name: run-with-missing-secrets
+type: exec
+description: This should not fail while config scan
+spec:
+  command: [ "echo", "${secrets.missing}" ]

core/test/data/missing-secrets/module/garden.yml

@@ -1,5 +1,5 @@
 kind: Project
-name: test-project-missing-secrets
+name: test-project-missing-secrets-in-module
 environments:
   - name: local
 providers:
@@ -8,4 +8,4 @@ providers:
   - name: test-plugin-b
     environments: [local]
 variables:
-  some: variable
+  some: variable

core/test/data/missing-secrets/workflow/garden.yml

@@ -1,5 +1,5 @@
 kind: Project
-name: test-project-missing-secrets
+name: test-project-missing-secrets-in-workflow
 environments:
   - name: local
 providers:
@@ -15,4 +15,4 @@ variables:
 kind: Workflow
 name: test-workflow
 steps:
-  - command: [deploy, "${secrets.missing}"]
+  - command: [deploy, "${secrets.missing}"]