Skip to content

Commit

Permalink
fix(workflows): error referencing undefined vars in workflow templates
Browse files Browse the repository at this point in the history
Fixes #2235
  • Loading branch information
edvald authored and thsig committed Feb 5, 2021
1 parent 1d01ed6 commit 7591cb1
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 24 deletions.
11 changes: 0 additions & 11 deletions core/src/config/config-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,16 +504,6 @@ export class WorkflowConfigContext extends EnvironmentConfigContext {
)
public variables: DeepPrimitiveMap

@schema(
joiStringMap(joi.string().description("The secret's value."))
.description("A map of all secrets for this project in the current environment.")
.meta({
internal: true,
keyPlaceholder: "<secret-name>",
})
)
public secrets: PrimitiveMap

constructor(garden: Garden) {
super({
projectName: garden.projectName,
Expand All @@ -527,7 +517,6 @@ export class WorkflowConfigContext extends EnvironmentConfigContext {

const fullEnvName = garden.namespace ? `${garden.namespace}.${garden.environmentName}` : garden.environmentName
this.environment = new EnvironmentContext(this, garden.environmentName, fullEnvName, garden.namespace)
this.project = new ProjectContext(this, garden.projectName)
}
}

Expand Down
35 changes: 29 additions & 6 deletions core/src/config/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { getCoreCommands } from "../commands/commands"
import { CommandGroup } from "../commands/base"
import { EnvironmentConfig, getNamespace } from "./project"
import { globalOptions } from "../cli/params"
import { isTruthy } from "../util/util"
import { isTruthy, omitUndefined } from "../util/util"
import { parseCliArgs, pickCommand } from "../cli/helpers"

export interface WorkflowConfig {
Expand Down Expand Up @@ -306,24 +306,47 @@ export interface WorkflowConfigMap {
export function resolveWorkflowConfig(garden: Garden, config: WorkflowConfig) {
const log = garden.log
const context = new WorkflowConfigContext(garden)

log.silly(`Resolving template strings for workflow ${config.name}`)
let resolvedConfig = {
...resolveTemplateStrings(omit(config, "name", "triggers"), context, { allowPartial: true }),

const partialConfig = {
// Don't allow templating in names and triggers
...omit(config, "name", "triggers"),
// Defer resolution of step commands and scripts (the dummy script will be overwritten again below)
steps: config.steps.map((s) => ({ ...s, command: undefined, script: "echo" })),
}

let resolvedPartialConfig: WorkflowConfig = {
...resolveTemplateStrings(partialConfig, context),
name: config.name,
}

if (config.triggers) {
resolvedConfig["triggers"] = config.triggers
resolvedPartialConfig.triggers = config.triggers
}

log.silly(`Validating config for workflow ${config.name}`)

resolvedConfig = <WorkflowConfig>validateWithPath({
config: resolvedConfig,
resolvedPartialConfig = validateWithPath({
config: resolvedPartialConfig,
configType: "workflow",
schema: workflowConfigSchema(),
path: config.path,
projectRoot: garden.projectRoot,
})

// Re-add the deferred step commands and scripts
const resolvedConfig = {
...resolvedPartialConfig,
steps: resolvedPartialConfig.steps.map((s, i) =>
omitUndefined({
...omit(s, "command", "script"),
command: config.steps[i].command,
script: config.steps[i].script,
})
),
}

validateSteps(resolvedConfig)
validateTriggers(resolvedConfig, garden.environmentConfigs)
populateNamespaceForTriggers(resolvedConfig, garden.environmentConfigs)
Expand Down
43 changes: 36 additions & 7 deletions core/test/unit/src/config/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe("resolveWorkflowConfig", () => {
before(async () => {
garden = await makeTestGardenA()
garden["secrets"] = { foo: "bar", bar: "baz", baz: "banana" }
garden["variables"] = { foo: "baz" }
garden["variables"] = { foo: "baz", skip: false }
})

it("should pass through a canonical workflow config", async () => {
Expand Down Expand Up @@ -72,15 +72,44 @@ describe("resolveWorkflowConfig", () => {
path: "/tmp/foo",
description: "Secret: ${secrets.foo}, var: ${variables.foo}",
steps: [
{ description: "Deploy the stack", command: ["deploy"], skip: false, when: "onSuccess" },
{ command: ["test"], skip: false, when: "onSuccess" },
{
description: "Deploy the stack",
command: ["deploy"],
skip: ("${var.skip}" as unknown) as boolean,
when: "onSuccess",
},
],
}

expect(resolveWorkflowConfig(garden, config)).to.eql({
...config,
description: `Secret: bar, var: baz`,
})
const resolved = resolveWorkflowConfig(garden, config)

expect(resolved.description).to.equal("Secret: bar, var: baz")
expect(resolved.steps[0].skip).to.equal(false)
})

it("should not resolve template strings in step commands and scripts", async () => {
const config: WorkflowConfig = {
...defaults,
apiVersion: DEFAULT_API_VERSION,
kind: "Workflow",
name: "workflow-a",
path: "/tmp/foo",
description: "foo",
steps: [
{
description: "Deploy the stack",
command: ["deploy", "${var.foo}"],
skip: false,
when: "onSuccess",
},
{ script: "echo ${var.foo}", skip: false, when: "onSuccess" },
],
}

const resolved = resolveWorkflowConfig(garden, config)

expect(resolved.steps[0].command).to.eql(config.steps[0].command)
expect(resolved.steps[1].script).to.eql(config.steps[1].script)
})

it("should not resolve template strings in trigger specs or in the workflow name", async () => {
Expand Down

0 comments on commit 7591cb1

Please sign in to comment.