fix(template): avoid premature disabled flag evaluation on actions

* Do early `disabled` flag evaluation only for actions with duplicate names.
* Deny the usage of `var` and `variables` contexts in early `disabled` flag evaluation.

The contexts `var` and `variables` contexts are not fully resolved at that stage.
That can lead to incorrectly resolved `disabled` flag values.

Co-authored-by: Steffen Neubauer <[email protected]>

core/src/garden.ts

@@ -1522,10 +1522,16 @@ export class Garden {
     }
 
     const context = new TemplatableConfigContext(this, config)
-    // Inject action's variables
-    context.variables = context.var = {
-      ...context.variables,
-      ...config.variables,
+    // Hack: deny variables contexts here, because those have not been fully resolved yet.
+    const deniedContexts = ["var", "variables"]
+    for (const deniedContext of deniedContexts) {
+      Object.defineProperty(context, "var", {
+        get: () => {
+          throw new ConfigurationError({
+            message: `If you have duplicate action names, the ${styles.accent("`disabled`")} flag cannot depend on the ${styles.accent(`\`${deniedContext}\``)} context.`,
+          })
+        },
+      })
     }
 
     return resolveTemplateString({
@@ -1543,10 +1549,11 @@ export class Garden {
     const key = actionReferenceToString(config)
     const existing = this.actionConfigs[config.kind][config.name]
 
-    // Resolve the actual values of the `disabled` flag
-    config.disabled = this.evaluateDisabledFlag(config)
-
     if (existing) {
+      // Resolve the actual values of the `disabled` flag
+      config.disabled = this.evaluateDisabledFlag(config)
+      existing.disabled = this.evaluateDisabledFlag(existing)
+
       if (actionIsDisabled(config, this.environmentName)) {
         this.log.silly(
           () => `Skipping action ${key} because it is disabled and another action with the same key exists`

core/test/data/test-projects/disabled-action-with-var-context/project.garden.yml

@@ -0,0 +1,10 @@
+apiVersion: garden.io/v1
+kind: Project
+name: disabled-action-with-var-context
+defaultEnvironment: local
+environments:
+  - name: local
+  - name: remote
+providers:
+  - name: exec
+    environments: [ local, remote ]

core/test/data/test-projects/disabled-action-with-var-context/runs.garden.yml

@@ -0,0 +1,19 @@
+kind: Run
+name: run-script
+type: exec
+var:
+  foo: bar
+disabled: "${var.foo == 'bar'}"
+spec:
+  command: ["sh", "-c", "echo 'Hello from local'"]
+
+---
+
+kind: Run
+name: run-script
+type: exec
+var:
+  foo: bar
+disabled: "${var.foo != 'bar'}"
+spec:
+  command: ["sh", "-c", "echo 'Hello from remote'"]

core/test/unit/src/garden.ts

@@ -3052,6 +3052,22 @@ describe("Garden", () => {
       expect(runScript.getConfig().spec.command).to.eql(["sh", "-c", "echo 'Hello from local'"])
     })
 
+    it("should deny variables context in disabled flag for actions with duplicate names", async () => {
+      const garden = await makeTestGarden(getDataDir("test-projects", "disabled-action-with-var-context"))
+
+      // There are 2 'run-script' actions defined in the project, one per environment.
+      await expectError(() => garden.getConfigGraph({ log: garden.log, emit: false }), {
+        contains: [
+          "If you have duplicate action names",
+          "the",
+          "disabled",
+          "flag cannot depend on the",
+          "variables",
+          "context",
+        ],
+      })
+    })
+
     it("should resolve actions from templated config templates", async () => {
       const garden = await makeTestGarden(getDataDir("test-projects", "config-templates-with-templating"))
       await garden.scanAndAddConfigs()