Skip to content

Commit

Permalink
fix(template): avoid premature disabled flag evaluation on actions
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
vvagaytsev committed Sep 17, 2024
1 parent 5546b56 commit f279492
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 7 deletions.
21 changes: 14 additions & 7 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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`
Expand Down
Original file line number Diff line number Diff line change
@@ -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 ]
Original file line number Diff line number Diff line change
@@ -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'"]
16 changes: 16 additions & 0 deletions core/test/unit/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit f279492

Please sign in to comment.