Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow cross-referencing variables in the same scope #6814

Merged
merged 10 commits into from
Feb 3, 2025
5 changes: 5 additions & 0 deletions core/src/actions/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,11 @@ export abstract class ResolvedRuntimeAction<
this.executedDependencies = params.executedDependencies
this.resolvedDependencies = params.resolvedDependencies
this._staticOutputs = params.staticOutputs
this._config = {
...this._config,
// makes sure the variables show up in the `garden get config` command
variables: params.resolvedVariables,
}
this._config.spec = params.spec
this._config.internal.inputs = params.inputs
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/config/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ export function prepareProjectResource(log: Log, spec: any): ProjectConfig {
}

export function prepareModuleResource(spec: any, configPath: string, projectRoot: string): ModuleConfig {
spec.build = evaluate(spec.build, { context: new GenericContext({}), opts: {} }).resolved
spec.build = evaluate(spec.build, { context: new GenericContext("empty", {}), opts: {} }).resolved

const dependencies: BuildDependencyConfig[] = spec.build?.dependencies || []

Expand Down
41 changes: 26 additions & 15 deletions core/src/config/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ export function resolveProjectConfig({
context: ProjectConfigContext
}): ProjectConfig {
// Resolve template strings for non-environment-specific fields (apart from `sources`).
const { environments = [], name, sources = [], providers = [], outputs = [] } = config
const { variables = {}, environments = [], name, sources = [], providers = [], outputs = [] } = config

let globalConfig: any

Expand All @@ -479,7 +479,6 @@ export function resolveProjectConfig({
{
apiVersion: config.apiVersion,
varfile: config.varfile,
variables: config.variables,
environments: [],
sources: [],
},
Expand All @@ -505,6 +504,7 @@ export function resolveProjectConfig({
environments: [{ defaultNamespace: null, name: "fake-env-only-here-for-initial-load", variables: {} }],
providers: [],
sources: [],
variables: {},
// this makes sure that the output declaration shape is valid
outputs: serialiseUnresolvedTemplates(outputs),
},
Expand All @@ -519,6 +519,7 @@ export function resolveProjectConfig({
providers,
sources,
outputs,
variables,
}

config.defaultEnvironment = getDefaultEnvironmentName(defaultEnvironmentName, config)
Expand Down Expand Up @@ -632,20 +633,30 @@ export const pickEnvironment = profileAsync(async function _pickEnvironment({
// resolve project variables incl. varfiles
deepResolveContext("project", context.variables)

// @ts-expect-error todo: correct types for unresolved configs
const config = deepEvaluate(environmentConfig, {
context,
opts: {},
})
const config = deepEvaluate(
{
...environmentConfig,
// we leave variables unresolved, so we can cross-reference them
variables: {},
},
{
context,
opts: {},
}
)

environmentConfig = validateWithPath<EnvironmentConfig>({
config,
schema: environmentSchema(),
configType: `environment ${environment}`,
path: projectConfig.path,
projectRoot: projectConfig.path,
source,
})
environmentConfig = {
...validateWithPath<EnvironmentConfig>({
config,
schema: environmentSchema(),
configType: `environment ${environment}`,
path: projectConfig.path,
projectRoot: projectConfig.path,
source,
}),
// we leave variables unresolved, so we can cross-reference them
variables: environmentConfig.variables,
}

namespace = getNamespace(environmentConfig, namespace)

Expand Down
3 changes: 2 additions & 1 deletion core/src/config/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ export function getProviderTemplateReferences(config: UnresolvedProviderConfig,
visitAll({
value: config.unresolvedConfig,
}),
context
context,
{}
)
for (const finding of generator) {
const keyPath = finding.keyPath
Expand Down
6 changes: 4 additions & 2 deletions core/src/config/references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ export function* getActionTemplateReferences(
visitAll({
value: config as ObjectWithName,
}),
context
context,
{}
)

for (const finding of generator) {
Expand All @@ -190,7 +191,8 @@ export function getModuleTemplateReferences(config: ModuleConfig, context: Modul
visitAll({
value: config as ObjectWithName,
}),
context
context,
{}
)

for (const finding of generator) {
Expand Down
3 changes: 2 additions & 1 deletion core/src/config/secrets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ export function detectMissingSecretKeys(
visitAll({
value: obj,
}),
context
context,
{}
)
for (const finding of generator) {
const keyPath = finding.keyPath
Expand Down
50 changes: 45 additions & 5 deletions core/src/config/template-contexts/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,30 @@ export interface ContextResolveOpts {
*/
keepEscapingInTemplateStrings?: boolean

/**
* If true, the given context is final and contains everything needed to fully resolve the given templates.
*
* If false, this flag enables special behaviour of for some template values, like `ObjectSpreadLazyValue`,
* where we basically ignore unresolvable templates and resolve to whatever is available at the moment.
*
* This is currently used in the `VariableContext`, to allow for using some of the variables early during action processing,
* before we built the actual graph.
*
* @example
* kind: Build
* name: xy
* dependencies: ${var.dependencies} // <-- if false, we can resolve 'var.dependencies' despite the fact that 'actions' context is missing
* variables:
* $merge: ${actions.build.foo.vars} // <-- if true, the $merge operation fails if 'actions' context is missing
* dependencies: ["bar"]
*
* @warning If set to false, templates can lose information; Be careful when persisting the resolved values, because
* we may have lost some information even if evaluate returned `partial: false`.
*
* @default true
*/
isFinalContext?: boolean

// for detecting circular references
stack?: string[]
}
Expand Down Expand Up @@ -113,7 +137,7 @@ export abstract class ConfigContext {
private readonly _cache: Map<string, ContextResolveOutput>
private readonly _id: number

constructor(public readonly _description?: string) {
constructor(public readonly _description: string) {
this._id = globalConfigContextCounter++
this._cache = new Map()
if (!_description) {
Expand Down Expand Up @@ -191,6 +215,10 @@ export abstract class ConfigContext {
// Note: we're using classes here to be able to use decorators to describe each context node and key
@Profile()
export abstract class ContextWithSchema extends ConfigContext {
constructor(description: string = "") {
super(description)
}

static getSchema() {
const schemas = (<any>this)._schemas
return joi.object().keys(schemas).required()
Expand All @@ -214,7 +242,10 @@ export abstract class ContextWithSchema extends ConfigContext {
* A generic context that just wraps an object.
*/
export class GenericContext extends ConfigContext {
constructor(protected readonly data: ParsedTemplate | Collection<ParsedTemplate | ConfigContext>) {
constructor(
description: string,
protected readonly data: ParsedTemplate | Collection<ParsedTemplate | ConfigContext>
) {
if (data === undefined) {
throw new InternalError({
message: "Generic context may not be undefined.",
Expand All @@ -226,7 +257,7 @@ export class GenericContext extends ConfigContext {
"Generic context is useless when instantiated with just another context as parameter. Use the other context directly instead.",
})
}
super()
super(description)
}

protected override resolveImpl(params: ContextResolveParams): ContextResolveOutput {
Expand Down Expand Up @@ -269,7 +300,7 @@ export class EnvironmentContext extends ContextWithSchema {
*/
export class ErrorContext extends ConfigContext {
constructor(private readonly message: string) {
super()
super(`error`)
}

protected override resolveImpl({}): ContextResolveOutput {
Expand Down Expand Up @@ -325,7 +356,16 @@ export class LayeredContext extends ConfigContext {

constructor(description: string, ...layers: ConfigContext[]) {
super(description)
this.layers = layers
if (layers.length === 0) {
this.layers = [new GenericContext("empty", {})]
} else {
this.layers = layers
}
}

public addLayer(layer: ConfigContext) {
this.layers.push(layer)
this.clearCache()
}

override resolveImpl(args: ContextResolveParams): ContextResolveOutput {
Expand Down
6 changes: 3 additions & 3 deletions core/src/config/template-contexts/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ export class InputContext extends LayeredContext {
if (template) {
super(
"unresolved inputs (with best-effort schema defaults)",
new GenericContext(template.inputsSchemaDefaults),
new GenericContext(inputs || {})
new GenericContext("best-effort schema defaults", template.inputsSchemaDefaults),
new GenericContext("unresolved inputs", inputs || {})
)
} else {
super("fully resolved inputs", new GenericContext(inputs || {}))
super("fully resolved inputs", new GenericContext("fully resolved inputs", inputs || {}))
}
}
}
87 changes: 67 additions & 20 deletions core/src/config/template-contexts/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type { ActionConfig } from "../../actions/types.js"
import { isUnresolved } from "../../template/templated-strings.js"
import type { Varfile } from "../common.js"
import type { ActionConfigContext } from "./actions.js"
import { ActionSpecContext } from "./actions.js"
import type { CustomCommandContext } from "./custom-command.js"
import type { CommandResource } from "../command.js"
import type { GroupConfig } from "../group.js"
Expand All @@ -40,27 +41,55 @@ export class VariablesContext extends LayeredContext {
context,
variablePrecedence,
variableOverrides,
isFinalContext = true,
}: {
context: EnvironmentConfigContext | ProjectConfigContext | CustomCommandContext
variablePrecedence: (ParsedTemplate | undefined | null)[]
variableOverrides: DeepPrimitiveMap
/**
* @see ContextResolveOpts.isFinalContext
*/
isFinalContext?: boolean
}
) {
const layers: ConfigContext[] = [
// project config context has no variables yet. Use empty context as root instead then
"variables" in context ? context.variables : new GenericContext({}),
]

const entries = variablePrecedence.filter((tpl) => !isEmpty(tpl)).entries()

for (const [i, layer] of entries) {
const parent = layers[i]
layers.push(
new GenericContext(
// this ensures that a variable in any given context can refer to variables in the parent scope
capture(layer || {}, makeVariableRootContext(parent))
)
const layers: ConfigContext[] = []

let parent: ConfigContext | undefined

if ("variables" in context) {
// populate variables from root context
layers.push(context.variables)

// ensures that higher-precedence variables can refer to root vars
parent = makeVariableRootContext(`root variable context in ${description}`, context.variables)
}

const scopesOrderedByPrecedence = variablePrecedence.filter((tpl) => !isEmpty(tpl)).entries()

for (const [i, currentScope] of scopesOrderedByPrecedence) {
// add general context, to resolve inputs, action references, etc
const neededContext = new LayeredContext(`context for scope ${i} in ${description}`, context)

// capture the needed context, so variables can be resolved correctly
const capturedScope = new GenericContext(
`captured scope ${i} in ${description}`,
capture(currentScope, neededContext, { isFinalContext })
)
layers.push(capturedScope)

// NOTE: we now mutate the context we just captured
// this allowsat variables cross-referencing each other in the same scope, e.g. to reuse values across multiple variables
const variableRoot = makeVariableRootContext(`variable root for scope ${i} in ${description}`, capturedScope)
neededContext.addLayer(variableRoot)

// this ensures that a variable in any given context can refer to variables in the parent scope
// variables in the parent scope take precedence over cross-referencing for backwards-compatibility
if (parent) {
neededContext.addLayer(parent)
}

// make sure the lower precedence scope can access variables from this layer
parent = variableRoot
}

super(description, ...layers)
Expand All @@ -70,11 +99,23 @@ export class VariablesContext extends LayeredContext {
}
}

public static forTest(garden: Garden, ...vars: ParsedTemplate[]) {
public static forTest({
garden,
variablePrecedence,
isFinalContext,
}: {
garden: Garden
variablePrecedence: ParsedTemplate[]
/**
* @see ContextResolveOpts.isFinalContext
*/
isFinalContext?: boolean
}) {
return new this("test", {
context: garden.getProjectConfigContext(),
variablePrecedence: [...vars],
variablePrecedence,
variableOverrides: garden.variableOverrides,
isFinalContext,
})
}

Expand Down Expand Up @@ -152,7 +193,7 @@ export class VariablesContext extends LayeredContext {
public static async forAction(
garden: Garden,
config: ActionConfig,
context: ActionConfigContext,
context: ActionConfigContext | ActionSpecContext,
group?: GroupConfig
) {
const effectiveConfigFileLocation = getEffectiveConfigFileLocation(config)
Expand All @@ -170,6 +211,12 @@ export class VariablesContext extends LayeredContext {
variablePrecedence: [...groupVariables, ...actionVariables],
context,
variableOverrides: garden.variableOverrides,
/**
* If context is ActionConfigContext, we are still preprocessing and can't access dependency results.
*
* @see ContextResolveOpts.isFinalContext
*/
isFinalContext: context instanceof ActionSpecContext,
})
}

Expand Down Expand Up @@ -203,16 +250,16 @@ export class VariablesContext extends LayeredContext {
}
}

this.layers.push(new GenericContext(transformedOverrides))
this.layers.push(new GenericContext("variable overrides", transformedOverrides))
this.clearCache()
}
}

// helpers

function makeVariableRootContext(contents: ConfigContext) {
function makeVariableRootContext(description: string, contents: ConfigContext) {
// This makes the contents available under the keys `var` and `variables`
return new GenericContext({
return new GenericContext(description, {
var: contents,
variables: contents,
})
Expand Down
Loading