diff --git a/core/src/config/template-contexts/base.ts b/core/src/config/template-contexts/base.ts index 7ef0eb1213..4c4d9da4d9 100644 --- a/core/src/config/template-contexts/base.ts +++ b/core/src/config/template-contexts/base.ts @@ -25,6 +25,10 @@ export type ContextKey = ContextKeySegment[] export interface ContextResolveOpts { // Allow templates to be partially resolved (used to defer runtime template resolution, for example) allowPartial?: boolean + // This is kept for backwards compatibility of rendering kubernetes manifests + // TODO(0.14): Do not allow the use of template strings in kubernetes manifest files + // TODO(0.14): Remove legacyAllowPartial + legacyAllowPartial?: boolean // Allow partial resolution for values that originate from a special context that always returns CONTEXT_RESOLVE_KEY_AVAILABLE_LATER. // This is used for module resolution and can be removed whenever we remove support for modules. allowPartialContext?: boolean diff --git a/core/src/plugins/kubernetes/kubernetes-type/common.ts b/core/src/plugins/kubernetes/kubernetes-type/common.ts index e455ac5cf3..08a47035de 100644 --- a/core/src/plugins/kubernetes/kubernetes-type/common.ts +++ b/core/src/plugins/kubernetes/kubernetes-type/common.ts @@ -428,7 +428,17 @@ async function readFileManifests( const absPath = resolve(manifestPath, path) log.debug(`Reading manifest for ${action.longDescription()} from path ${absPath}`) const str = (await readFile(absPath)).toString() - const resolved = ctx.resolveTemplateStrings(str, { allowPartial: true, unescape: true }) + + // TODO(0.14): Do not resolve template strings in unparsed YAML and remove legacyAllowPartial + // First of all, evaluating template strings can result in invalid YAML that fails to parse, because the result of the + // template expressions will be interpreted by the YAML parser later. + // Then also, the use of `legacyAllowPartial: true` is quite unfortunate here, because users will not notice + // if they reference variables that do not exist. + const resolved = ctx.resolveTemplateStrings(str, { + legacyAllowPartial: true, + unescape: true, + }) + const manifests = await parseKubernetesManifests( resolved, `${basename(absPath)} in directory ${dirname(absPath)} (specified in ${action.longDescription()})`, diff --git a/core/src/template-string/ast.ts b/core/src/template-string/ast.ts index 0c33ed155f..54abec49fe 100644 --- a/core/src/template-string/ast.ts +++ b/core/src/template-string/ast.ts @@ -86,7 +86,8 @@ function* astVisitAll(e: TemplateExpression, source: ConfigSource): TemplateExpr } export abstract class TemplateExpression { - constructor(public readonly loc: Location) {} + public abstract readonly rawText: string + public abstract readonly loc: Location; *visitAll(source: ConfigSource): TemplateExpressionGenerator { yield* astVisitAll(this, source) @@ -97,7 +98,8 @@ export abstract class TemplateExpression { export class IdentifierExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, // in the template expression ${foo.123}, 123 is a valid identifier expression and is treated as a number. public readonly identifier: string | number ) { @@ -106,7 +108,7 @@ export class IdentifierExpression extends TemplateExpression { message: `identifier arg for IdentifierExpression must be a string or number. Got: ${typeof identifier}`, }) } - super(loc) + super() } override evaluate(): string | number { @@ -116,10 +118,11 @@ export class IdentifierExpression extends TemplateExpression { export class LiteralExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly literal: TemplatePrimitive ) { - super(loc) + super() } override evaluate(): TemplatePrimitive { @@ -129,12 +132,13 @@ export class LiteralExpression extends TemplateExpression { export class ArrayLiteralExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, // an ArrayLiteralExpression consists of several template expressions, // for example other literal expressions and context lookup expressions. public readonly literal: TemplateExpression[] ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { @@ -153,10 +157,11 @@ export class ArrayLiteralExpression extends TemplateExpression { export abstract class UnaryExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly innerExpression: TemplateExpression ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult { @@ -204,12 +209,13 @@ export class NotExpression extends UnaryExpression { export abstract class LogicalExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly operator: string, public readonly left: TemplateExpression, public readonly right: TemplateExpression ) { - super(loc) + super() } } @@ -293,12 +299,13 @@ export class LogicalAndExpression extends LogicalExpression { export abstract class BinaryExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly operator: string, public readonly left: TemplateExpression, public readonly right: TemplateExpression ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { @@ -467,16 +474,24 @@ export class GreaterThanExpression extends BinaryExpressionOnNumbers { export class FormatStringExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly innerExpression: TemplateExpression, public readonly isOptional: boolean ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { const result = this.innerExpression.evaluate({ ...args, + opts: { + ...args.opts, + // nested expressions should never be partially resolved in legacy mode + // Otherwise, weird problems can happen if we e.g. partially resolve a sub-expression and pass it to a parameter of a function call. + // TODO(0.14): remove legacyAllowPartial + legacyAllowPartial: false, + }, optional: args.optional || this.isOptional, }) @@ -490,66 +505,68 @@ export class FormatStringExpression extends TemplateExpression { } } -export class ElseBlockExpression extends TemplateExpression { - override evaluate(): never { - // See also `buildConditionalTree` in `parser.pegjs` - throw new InternalError({ - message: `{else} block expression should not end up in the final AST`, - }) - } -} - -export class EndIfBlockExpression extends TemplateExpression { - override evaluate(): never { - // See also `buildConditionalTree` in `parser.pegjs` - throw new InternalError({ - message: `{endif} block expression should not end up in the final AST`, - }) - } -} +export abstract class AbstractBlockExpression extends TemplateExpression { + // this is a method because visitAll is confused if multiple properties contain the same expression + protected abstract getExpressions(): TemplateExpression[] -export class IfBlockExpression extends TemplateExpression { - constructor( - loc: Location, - public readonly condition: TemplateExpression, - public ifTrue: TemplateExpression | undefined, - public ifFalse: TemplateExpression | undefined - ) { - super(loc) + override get rawText(): string { + return this.getExpressions() + .map((expr) => expr.rawText || "") + .join("") } - override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { - const condition = this.condition.evaluate(args) - - if (typeof condition === "symbol") { - return condition - } + override get loc(): Location { + const expressions = this.getExpressions() + const first = expressions[0]! + const last = expressions[expressions.length - 1]! - // For backwards compatibility, we do allow if block expressions without endif block in some cases. - // For a stand-alone if-block expression, we evaluate the condition and return the result. - if (this.ifTrue === undefined) { - return condition + return { + start: first.loc.start, + end: last.loc.end, + source: first.loc.source, } - - const evaluated = isTruthy(condition) ? this.ifTrue?.evaluate(args) : this.ifFalse?.evaluate(args) - - return evaluated } } -export class StringConcatExpression extends TemplateExpression { +export class BlockExpression extends AbstractBlockExpression { public readonly expressions: TemplateExpression[] - constructor(loc: Location, ...expressions: TemplateExpression[]) { - super(loc) + constructor(...expressions: TemplateExpression[]) { + super() + if (expressions.length === 0) { + throw new InternalError({ + message: "Compound expression must consist of at least one expression", + }) + } this.expressions = expressions } - override evaluate(args: EvaluateArgs): TemplateEvaluationResult { - let result: string = "" + protected override getExpressions(): TemplateExpression[] { + return this.expressions + } + override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { + const legacyAllowPartial = args.opts.legacyAllowPartial + + let result: string = "" for (const expr of this.expressions) { - const r = expr.evaluate(args) + const r = expr.evaluate({ + ...args, + // in legacyAllowPartial mode, all template expressions are optional + optional: args.optional || legacyAllowPartial, + }) + + // For legacy allow partial mode, other format string expressions might be evaluated, and + // we produce a string even when a key hasn't been found. + // This is extremely evil, but unfortunately needs to be kept for backwards bug-compatibility. + // TODO(0.14): please remove legacyAllowPartial + if (legacyAllowPartial && typeof r === "symbol") { + result += expr.rawText + continue + } else if (this.expressions.length === 1) { + // if we evaluate a single expression we are allowed to evaluate to something other than a string + return r + } if (typeof r === "symbol") { return r @@ -567,12 +584,86 @@ export class StringConcatExpression extends TemplateExpression { } } +export class IfBlockExpression extends AbstractBlockExpression { + // `buildConditionalTree` in parser.pegjs will fill these in + public consequent: TemplateExpression | undefined + public else: ElseBlockExpression | undefined + public alternate: TemplateExpression | undefined + public endIf: EndIfBlockExpression | undefined + + constructor(public readonly condition: FormatStringExpression) { + if (!(condition instanceof FormatStringExpression)) { + throw new InternalError({ + message: "expected if block condition to be FormatStringExpression", + }) + } + super() + } + + protected override getExpressions(): TemplateExpression[] { + return [this.condition, this.consequent, this.else, this.alternate, this.endIf].filter( + (e): e is TemplateExpression => !!e + ) + } + + override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { + const condition = this.condition.evaluate(args) + + if (typeof condition === "symbol") { + return condition + } + + // For backwards compatibility, we do allow if block expressions without endif block in some cases. + // For a stand-alone if-block expression, we evaluate the condition and return the result. + if (this.consequent === undefined) { + return condition + } + + const evaluated = isTruthy(condition) ? this.consequent?.evaluate(args) : this.alternate?.evaluate(args) + + return evaluated + } +} + +export class ElseBlockExpression extends TemplateExpression { + constructor( + public readonly rawText: string, + public readonly loc: Location + ) { + super() + } + + override evaluate(): never { + // See also `ast.IfBlockExpression` and `buildConditionalTree` in `parser.pegjs` + throw new InternalError({ + message: `{else} block expression should never be evaluated`, + }) + } +} + +export class EndIfBlockExpression extends TemplateExpression { + constructor( + public readonly rawText: string, + public readonly loc: Location + ) { + super() + } + + override evaluate(): never { + // See also `ast.IfBlockExpression` and `buildConditionalTree` in `parser.pegjs` + throw new InternalError({ + message: `{endif} block expression should should never be evaluated`, + }) + } +} + export class MemberExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly innerExpression: TemplateExpression ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult { @@ -596,10 +687,11 @@ export class MemberExpression extends TemplateExpression { export class ContextLookupExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly keyPath: (IdentifierExpression | MemberExpression)[] ) { - super(loc) + super() } override evaluate({ @@ -669,11 +761,12 @@ export class ContextLookupExpression extends TemplateExpression { export class FunctionCallExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly functionName: IdentifierExpression, public readonly args: TemplateExpression[] ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { @@ -765,12 +858,13 @@ export class FunctionCallExpression extends TemplateExpression { export class TernaryExpression extends TemplateExpression { constructor( - loc: Location, + public readonly rawText: string, + public readonly loc: Location, public readonly condition: TemplateExpression, - public readonly ifTrue: TemplateExpression, - public readonly ifFalse: TemplateExpression + public readonly consequent: TemplateExpression, + public readonly alternate: TemplateExpression ) { - super(loc) + super() } override evaluate(args: EvaluateArgs): TemplateEvaluationResult> { @@ -786,8 +880,8 @@ export class TernaryExpression extends TemplateExpression { // evaluate ternary expression const evaluationResult = !isNotFound(conditionResult) && isTruthy(conditionResult) - ? this.ifTrue.evaluate(args) - : this.ifFalse.evaluate(args) + ? this.consequent.evaluate(args) + : this.alternate.evaluate(args) return evaluationResult } diff --git a/core/src/template-string/parser.pegjs b/core/src/template-string/parser.pegjs index e57d248f11..6f830fde5e 100644 --- a/core/src/template-string/parser.pegjs +++ b/core/src/template-string/parser.pegjs @@ -43,27 +43,27 @@ switch (operator) { case "==": - return new ast.EqualExpression(location(), operator, left, right) + return new ast.EqualExpression(text(), location(), operator, left, right) case "!=": - return new ast.NotEqualExpression(location(), operator, left, right) + return new ast.NotEqualExpression(text(), location(), operator, left, right) case "<=": - return new ast.LessThanEqualExpression(location(), operator, left, right) + return new ast.LessThanEqualExpression(text(), location(), operator, left, right) case ">=": - return new ast.GreaterThanEqualExpression(location(), operator, left, right) + return new ast.GreaterThanEqualExpression(text(), location(), operator, left, right) case "<": - return new ast.LessThanExpression(location(), operator, left, right) + return new ast.LessThanExpression(text(), location(), operator, left, right) case ">": - return new ast.GreaterThanExpression(location(), operator, left, right) + return new ast.GreaterThanExpression(text(), location(), operator, left, right) case "+": - return new ast.AddExpression(location(), operator, left, right) + return new ast.AddExpression(text(), location(), operator, left, right) case "-": - return new ast.SubtractExpression(location(), operator, left, right) + return new ast.SubtractExpression(text(), location(), operator, left, right) case "*": - return new ast.MultiplyExpression(location(), operator, left, right) + return new ast.MultiplyExpression(text(), location(), operator, left, right) case "/": - return new ast.DivideExpression(location(), operator, left, right) + return new ast.DivideExpression(text(), location(), operator, left, right) case "%": - return new ast.ModuloExpression(location(), operator, left, right) + return new ast.ModuloExpression(text(), location(), operator, left, right) default: throw new TemplateStringError({ message: `Unrecognized logical operator: ${operator}`, loc: location() }) } @@ -82,9 +82,9 @@ switch (operator) { case "&&": - return new ast.LogicalAndExpression(location(), operator, left, right) + return new ast.LogicalAndExpression(text(), location(), operator, left, right) case "||": - return new ast.LogicalOrExpression(location(), operator, left, right) + return new ast.LogicalOrExpression(text(), location(), operator, left, right) default: throw new TemplateStringError({ message: `Unrecognized logical operator: ${operator}`, loc: location() }) } @@ -101,8 +101,8 @@ let rootExpressions = [] let currentCondition = undefined - let ifTrue = [] - let ifFalse = [] + let consequent = [] + let alternate = [] let nestingLevel = 0 let encounteredElse = false const pushElement = (e) => { @@ -110,9 +110,9 @@ return rootExpressions.push(e) } if (encounteredElse) { - ifFalse.push(e) + alternate.push(e) } else { - ifTrue.push(e) + consequent.push(e) } } for (const e of elements) { @@ -132,6 +132,7 @@ } if (currentCondition && nestingLevel === 0) { + currentCondition.else = e encounteredElse = true } else { pushElement(e) @@ -141,9 +142,9 @@ throw new TemplateStringError({ message: "Found ${endif} block without a preceding ${if...} block.", loc: location() }) } if (nestingLevel === 0) { - currentCondition.ifTrue = buildConditionalTree(...ifTrue) - currentCondition.ifFalse = buildConditionalTree(...ifFalse) - currentCondition.loc.end = e.loc.end + currentCondition.consequent = buildConditionalTree(...consequent) + currentCondition.alternate = buildConditionalTree(...alternate) + currentCondition.endIf = e rootExpressions.push(currentCondition) currentCondition = undefined } else { @@ -158,20 +159,18 @@ // For backwards compatibility we allow unclosed ${if } block statements in some cases, // (Namely if there are no other expressions on the same nesting level) // e.g. '${if "foo"}' evaluates to "foo" - if (currentCondition && (rootExpressions.length > 0 || ifTrue.length > 0)) { + if (currentCondition && (rootExpressions.length > 0 || consequent.length > 0)) { throw new TemplateStringError({ message: "Missing ${endif} after ${if ...} block.", loc: location() }) } if (rootExpressions.length === 0) { + if (!currentCondition) { + return new ast.LiteralExpression(text(), location(), "") + } // if we have a current condition, we return the unclosed if statement - // if it's undefined this is equivalent to empty string literal expression - return currentCondition || new ast.LiteralExpression(location(), "") - } - - if (rootExpressions.length === 1) { - return rootExpressions[0] + rootExpressions.push(currentCondition) } - return new ast.StringConcatExpression(location(), ...rootExpressions) + return new ast.BlockExpression(...rootExpressions) } function isBlockExpression(e) { @@ -185,11 +184,6 @@ Start = elements:TemplateStrings { - // If there is only one format string, return it's result directly without wrapping in StringConcatExpression - if (elements.length === 1 && !(isBlockExpression(elements[0]))) { - return elements[0] - } - return buildConditionalTree(...elements) } @@ -204,15 +198,15 @@ TemplateStrings } / UnclosedFormatString / $(.+) { - return [new ast.LiteralExpression(location(), text())] + return [new ast.LiteralExpression(text(), location(), text())] } FormatString = EscapeStart (!FormatEndWithOptional SourceCharacter)* FormatEndWithOptional { if (options.unescape) { - return new ast.LiteralExpression(location(), text().slice(1)) + return new ast.LiteralExpression(text(), location(), text().slice(1)) } else { - return new ast.LiteralExpression(location(), text()) + return new ast.LiteralExpression(text(), location(), text()) } } / FormatStart op:BlockOperator FormatEnd { @@ -220,9 +214,9 @@ FormatString // We instantiate expressions here to get the correct locations for constructing good error messages switch (op) { case "else": - return new ast.ElseBlockExpression(location()) + return new ast.ElseBlockExpression(text(), location()) case "endif": - return new ast.EndIfBlockExpression(location()) + return new ast.EndIfBlockExpression(text(), location()) default: throw new TemplateStringError({ message: `Unrecognized block operator: ${op}`, loc: location() }) } @@ -230,24 +224,22 @@ FormatString / pre:FormatStartWithEscape blockOperator:(ExpressionBlockOperator __)* e:Expression end:FormatEndWithOptional { if (pre[0] === escapePrefix) { if (options.unescape) { - return new ast.LiteralExpression(location(), text().slice(1)) + return new ast.LiteralExpression(text(), location(), text().slice(1)) } else { - return new ast.LiteralExpression(location(), text()) + return new ast.LiteralExpression(text(), location(), text()) } } const isOptional = end[1] === optionalSuffix - const expression = new ast.FormatStringExpression(location(), e, isOptional) + const expression = new ast.FormatStringExpression(text(), location(), e, isOptional, text()) if (blockOperator && blockOperator.length > 0) { if (isOptional) { throw new TemplateStringError({ message: "Cannot specify optional suffix in if-block.", loc: location() }) } - // ifTrue and ifFalse will be filled in by `buildConditionalTree` - const ifTrue = undefined - const ifFalse = undefined - return new ast.IfBlockExpression(location(), expression, ifTrue, ifFalse, isOptional) + // consequent, alternate, else and endif expressions will be filled in by `buildConditionalTree` + return new ast.IfBlockExpression(expression) } return expression @@ -286,10 +278,10 @@ ExpressionBlockOperator = "if" Prefix - = !FormatStartWithEscape (. ! FormatStartWithEscape)* . { return new ast.LiteralExpression(location(), text()) } + = !FormatStartWithEscape (. ! FormatStartWithEscape)* . { return new ast.LiteralExpression(text(), location(), text()) } Suffix - = !FormatEnd (. ! FormatEnd)* . { return new ast.LiteralExpression(location(), text()) } + = !FormatEnd (. ! FormatEnd)* . { return new ast.LiteralExpression(text(), location(), text()) } // ---- expressions ----- // Reduced and adapted from: https://github.com/pegjs/pegjs/blob/master/examples/javascript.pegjs @@ -297,7 +289,7 @@ MemberExpression = head:Identifier tail:( "[" __ e:Expression __ "]" { - return new ast.MemberExpression(location(), e) + return new ast.MemberExpression(text(), location(), e) } / "." e:Identifier { return e @@ -309,7 +301,7 @@ MemberExpression CallExpression = functionName:Identifier __ args:Arguments { - return new ast.FunctionCallExpression(location(), functionName, args) + return new ast.FunctionCallExpression(text(), location(), functionName, args) } Arguments @@ -327,7 +319,7 @@ ArgumentListTail ArrayLiteral = v:_ArrayLiteral { - return new ast.ArrayLiteralExpression(location(), v) + return new ast.ArrayLiteralExpression(text(), location(), v) } _ArrayLiteral = "[" __ elision:(Elision __)? "]" { @@ -358,18 +350,18 @@ Elision PrimaryExpression = v:NonStringLiteral { - return new ast.LiteralExpression(location(), v) + return new ast.LiteralExpression(text(), location(), v) } / v:StringLiteral { // Do not parse empty strings. if (v === "") { - return new ast.LiteralExpression(location(), "") + return new ast.LiteralExpression(text(), location(), "") } // Allow nested template strings in literals const parsed = parseNested(v) if (typeof parsed === "string") { // The nested string did not contain template expressions, so it is a literal expression - return new ast.LiteralExpression(location(), parsed) + return new ast.LiteralExpression(text(), location(), parsed) } // v contained a template expression return parsed @@ -377,7 +369,7 @@ PrimaryExpression / ArrayLiteral / CallExpression / key:MemberExpression { - return new ast.ContextLookupExpression(location(), key) + return new ast.ContextLookupExpression(text(), location(), key) } / "(" __ e:Expression __ ")" { return e @@ -388,9 +380,9 @@ UnaryExpression / operator:UnaryOperator __ argument:UnaryExpression { switch (operator) { case "typeof": - return new ast.TypeofExpression(location(), argument) + return new ast.TypeofExpression(text(), location(), argument) case "!": - return new ast.NotExpression(location(), argument) + return new ast.NotExpression(text(), location(), argument) default: throw new TemplateStringError({ message: `Unrecognized unary operator: ${operator}`, loc: location() }) } @@ -402,7 +394,7 @@ UnaryOperator ContainsExpression = iterable:UnaryExpression __ ContainsOperator __ element:UnaryExpression { - return new ast.ContainsExpression(location(), "contains", iterable, element) + return new ast.ContainsExpression(text(), location(), "contains", iterable, element) } / UnaryExpression @@ -470,7 +462,7 @@ ConditionalExpression "?" __ consequent:Expression __ ":" __ alternate:Expression { - return new ast.TernaryExpression(location(), test, consequent, alternate) + return new ast.TernaryExpression(text(), location(), test, consequent, alternate) } / LogicalORExpression @@ -515,7 +507,7 @@ SingleLineComment = "//" (!LineTerminator SourceCharacter)* Identifier - = !ReservedWord name:IdentifierName { return new ast.IdentifierExpression(location(), name) } + = !ReservedWord name:IdentifierName { return new ast.IdentifierExpression(text(), location(), name) } IdentifierName "identifier" = head:IdentifierStart tail:IdentifierPart* { diff --git a/core/src/util/fs.ts b/core/src/util/fs.ts index 1cf6490c29..f8df3d8a3e 100644 --- a/core/src/util/fs.ts +++ b/core/src/util/fs.ts @@ -253,14 +253,14 @@ export async function makeTempDir({ }: { git?: boolean; initialCommit?: boolean } = {}): Promise { const tmpDir = await tmp.dir({ unsafeCleanup: true }) // Fully resolve path so that we don't get path mismatches in tests - const tmpPath = await realpath(tmpDir.path) + tmpDir.path = await realpath(tmpDir.path) if (git) { - await exec("git", ["init", "--initial-branch=main"], { cwd: tmpPath }) + await exec("git", ["init", "--initial-branch=main"], { cwd: tmpDir.path }) if (initialCommit) { - await writeFile(join(tmpPath, "foo"), "bar") - await exec("git", ["add", "."], { cwd: tmpPath }) - await exec("git", ["commit", "-m", "first commit"], { cwd: tmpPath }) + await writeFile(join(tmpDir.path, "foo"), "bar") + await exec("git", ["add", "."], { cwd: tmpDir.path }) + await exec("git", ["commit", "-m", "first commit"], { cwd: tmpDir.path }) } } diff --git a/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/garden.yml b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/garden.yml new file mode 100644 index 0000000000..f0f9e2cc5c --- /dev/null +++ b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/garden.yml @@ -0,0 +1,25 @@ + +kind: Deploy +type: kubernetes +name: legacypartial-ifblock-doesnotexist +spec: + files: + - ./k8s/ifblock-doesnotexist.yaml + +--- + +kind: Deploy +type: kubernetes +name: legacypartial-ifblock-true +spec: + files: + - ./k8s/ifblock-true.yaml + +--- + +kind: Deploy +type: kubernetes +name: legacypartial-ifblock-false +spec: + files: + - ./k8s/ifblock-false.yaml diff --git a/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-doesnotexist.yaml b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-doesnotexist.yaml new file mode 100644 index 0000000000..1c0873843d --- /dev/null +++ b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-doesnotexist.yaml @@ -0,0 +1,15 @@ +${if var.doesNotExist} +apiVersion: v1 +kind: Service +metadata: + name: silly-demo +spec: + type: ClusterIP + ports: + - port: 9090 + targetPort: 9090 + protocol: TCP + name: http + selector: + app: silly-demo +${endif} diff --git a/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-false.yaml b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-false.yaml new file mode 100644 index 0000000000..c576a925f9 --- /dev/null +++ b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-false.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Service +metadata: +${if false} + name: evaluated wrong branch +${else} + name: it-${"partially"}-resolves-${var.doesNotExist}-and-$${unescapes} +${endif} diff --git a/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-true.yaml b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-true.yaml new file mode 100644 index 0000000000..b276ab9bdd --- /dev/null +++ b/core/test/data/test-projects/kubernetes-type/legacyAllowPartial/k8s/ifblock-true.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Service +metadata: +${if true} + name: it-${"partially"}-resolves-${var.doesNotExist}-and-$${unescapes} +${else} + name: evaluated wrong branch +${endif} diff --git a/core/test/integ/src/plugins/kubernetes/kubernetes-type/common.ts b/core/test/integ/src/plugins/kubernetes/kubernetes-type/common.ts index 99cbecab44..09a867012f 100644 --- a/core/test/integ/src/plugins/kubernetes/kubernetes-type/common.ts +++ b/core/test/integ/src/plugins/kubernetes/kubernetes-type/common.ts @@ -44,6 +44,63 @@ describe("getManifests", () => { let api: KubeApi const defaultNamespace = "foobar" + context("legacyAllowPartial", () => { + let action: Resolved + + before(async () => { + garden = await getKubernetesTestGarden() + const provider = (await garden.resolveProvider({ + log: garden.log, + name: "local-kubernetes", + })) as KubernetesProvider + ctx = await garden.getPluginContext({ provider, templateContext: undefined, events: undefined }) + api = await KubeApi.factory(garden.log, ctx, provider) + }) + + beforeEach(async () => { + graph = await garden.getConfigGraph({ + log: garden.log, + emit: false, + }) + }) + + it("crashes with yaml syntax error if an if block references variable that does not exist", async () => { + action = await garden.resolveAction({ + action: cloneDeep(graph.getDeploy("legacypartial-ifblock-doesnotexist")), + log: garden.log, + graph, + }) + + await expectError(() => getManifests({ ctx, api, action, log: garden.log, defaultNamespace }), { + contains: ["could not parse ifblock-doesnotexist.yaml in directory ", "as valid yaml"], + }) + }) + + it("partially resolves the consequent branch of ${if true} block", async () => { + action = await garden.resolveAction({ + action: cloneDeep(graph.getDeploy("legacypartial-ifblock-true")), + log: garden.log, + graph, + }) + + const result = await getManifests({ ctx, api, action, log: garden.log, defaultNamespace }) + expect(result.length).to.eq(2) // due to metadata configmap + expect(result[0].metadata.name).to.eq("it-partially-resolves-${var.doesNotExist}-and-${unescapes}") + }) + + it("partially resolves the alternate branch of ${if false} block", async () => { + action = await garden.resolveAction({ + action: cloneDeep(graph.getDeploy("legacypartial-ifblock-false")), + log: garden.log, + graph, + }) + + const result = await getManifests({ ctx, api, action, log: garden.log, defaultNamespace }) + expect(result.length).to.eq(2) // due to metadata configmap + expect(result[0].metadata.name).to.eq("it-partially-resolves-${var.doesNotExist}-and-${unescapes}") + }) + }) + context("duplicates", () => { let action: Resolved diff --git a/core/test/unit/src/template-string.ts b/core/test/unit/src/template-string.ts index b0783953a7..72bfadf2ba 100644 --- a/core/test/unit/src/template-string.ts +++ b/core/test/unit/src/template-string.ts @@ -1200,6 +1200,120 @@ describe("resolveTemplateString", () => { expect(res).to.equal("foo-bar") }) }) + context("legacyAllowPartial=true", () => { + it("partially resolves template expressions when 'b' is missing in the context", () => { + const res = resolveTemplateString({ + string: "${a}-${b}", + context: new GenericContext({ a: "foo" }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res).to.equal("foo-${b}") + }) + + it("partially resolves template expressions when 'a' is missing in the context", () => { + const res = resolveTemplateString({ + string: "${a}-${b}", + context: new GenericContext({ b: "foo" }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res).to.equal("${a}-foo") + }) + + it("fully resolves template expressions when 'a' is missing in the context when evaluating a conditional expression", () => { + const res = resolveTemplateString({ + string: "${a || b}-${c}", + context: new GenericContext({ b: 123, c: "foo" }), + contextOpts: { + legacyAllowPartial: true, + }, + }) + expect(res).to.equal("123-foo") + }) + + it("resolves template expressions when the context is fully available", () => { + const res = resolveTemplateString({ + string: "${a}-${b}", + context: new GenericContext({ a: "foo", b: "bar" }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res).to.equal("foo-bar") + }) + + it("partially resolves template expressions in the presence of an if expression", () => { + const res = resolveTemplateString({ + string: "${a}-${b}-${if c}exists${else}missing${endif}", + context: new GenericContext({ a: "foo", b: "bar" }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res).to.equal("foo-bar-${if c}exists${else}missing${endif}") + }) + + it("does not resolve template expressions inside unresolved if expression", () => { + // this is different in behaviour compared to prior versions of Garden; + // This is unlikely to be a breaking change for users, because unresolved if statements likely result in YAML errors when resolving template strings in kubernetes manifests. + const res = resolveTemplateString({ + string: "${if c}${a}${else}${b}${endif}", + context: new GenericContext({ a: "foo" }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res).to.equal("${if c}${a}${else}${b}${endif}") + }) + + it("partially resolves to consequent branch of if expression if conditional is true", () => { + const res1 = resolveTemplateString({ + string: "${if c}${a}${else}${b}${endif}", + context: new GenericContext({ a: "foo", c: true }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res1).to.equal("foo") + const res2 = resolveTemplateString({ + string: "${if c}${a}${else}${b}${endif}", + context: new GenericContext({ c: true }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res2).to.equal("${a}") + }) + + it("partially resolves to alternate branch of if expression if conditional is false", () => { + const res1 = resolveTemplateString({ + string: "${if c}${a}${else}${b}${endif}", + context: new GenericContext({ b: "foo", c: false }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res1).to.equal("foo") + const res2 = resolveTemplateString({ + string: "${if c}${a}${else}${b}${endif}", + context: new GenericContext({ c: false }), + contextOpts: { legacyAllowPartial: true }, + }) + expect(res2).to.equal("${b}") + }) + + it("should not partially resolve sub-expressions in a format string expression to avoid crashes", () => { + const testCases = [ + "${var.exists[var.doesNotExist]}", + '${var.exists["${var.doesNotExist}"]}', + "${jsonEncode(var.doesNotExist)}", + "${var.one + var.twoDoesNotExist}", + ] + + for (const templateString of testCases) { + const result = resolveTemplateString({ + string: templateString, + contextOpts: { + legacyAllowPartial: true, + }, + context: new GenericContext({ var: { exists: {}, one: 1 } }), + }) + // it should not partially resolve, i.e. resolve back to input + const expectation = templateString + expect(result).to.eq( + expectation, + `Template "${templateString}" did not resolve to expected value ${JSON.stringify(expectation)}` + ) + } + }) + }) }) context("contains operator", () => {