diff --git a/core/src/config/base.ts b/core/src/config/base.ts index d539931ab4..597869e96a 100644 --- a/core/src/config/base.ts +++ b/core/src/config/base.ts @@ -54,6 +54,7 @@ export type ObjectPath = (string | number)[] export interface YamlDocumentWithSource extends Document { source: string + filename: string | undefined } export function getEffectiveConfigFileLocation(actionConfig: BaseActionConfig): string { @@ -125,11 +126,17 @@ export const allConfigKinds = ["Module", "Workflow", "Project", configTemplateKi * @param sourceDescription - A description of the location of the yaml file, e.g. "bar.yaml at directory /foo/". * @param version - YAML standard version. Defaults to "1.2" */ -export async function loadAndValidateYaml( - content: string, - sourceDescription: string, - version: DocumentOptions["version"] = "1.2" -): Promise<YamlDocumentWithSource[]> { +export async function loadAndValidateYaml({ + content, + sourceDescription, + filename, + version = "1.2", +}: { + content: string + sourceDescription: string + filename: string | undefined + version?: DocumentOptions["version"] +}): Promise<YamlDocumentWithSource[]> { try { return Array.from(parseAllDocuments(content, { merge: true, strict: false, version }) || []).map((doc) => { if (doc.errors.length > 0) { @@ -142,6 +149,7 @@ export async function loadAndValidateYaml( const docWithSource = doc as YamlDocumentWithSource docWithSource.source = content + docWithSource.filename = filename return docWithSource }) @@ -197,7 +205,11 @@ export async function validateRawConfig({ projectRoot: string allowInvalid?: boolean }) { - let rawSpecs = await loadAndValidateYaml(rawConfig, `${basename(configPath)} in directory ${dirname(configPath)}`) + let rawSpecs = await loadAndValidateYaml({ + content: rawConfig, + sourceDescription: `${basename(configPath)} in directory ${dirname(configPath)}`, + filename: configPath, + }) // Ignore empty resources rawSpecs = rawSpecs.filter(Boolean) diff --git a/core/src/config/validation.ts b/core/src/config/validation.ts index 0442268955..6c37b3bd32 100644 --- a/core/src/config/validation.ts +++ b/core/src/config/validation.ts @@ -11,7 +11,7 @@ import { ConfigurationError } from "../exceptions.js" import { relative } from "path" import { uuidv4 } from "../util/random.js" import type { BaseGardenResource, ObjectPath, YamlDocumentWithSource } from "./base.js" -import type { ParsedNode, Range } from "yaml" +import type { ParsedNode } from "yaml" import { padEnd } from "lodash-es" import { styles } from "../logger/styles.js" @@ -130,8 +130,6 @@ export function validateSchema<T>( return result.value } - const yamlDoc = source?.yamlDoc - const rawYaml = yamlDoc?.source const yamlBasePath = source?.path || [] const errorDetails = error.details.map((e) => { @@ -140,15 +138,16 @@ export function validateSchema<T>( ? improveZodValidationErrorMessage(e, yamlBasePath) : improveJoiValidationErrorMessage(e, schema, yamlBasePath) - const node = yamlDoc?.getIn([...yamlBasePath, ...e.path], true) as ParsedNode | undefined - const range = node?.range - - if (rawYaml && yamlDoc?.contents && range) { - try { - e.message = addYamlContext({ rawYaml, range, message: e.message }) - } catch { - // ignore - } + try { + e.message = addYamlContext({ + source: { + ...source, + path: [...yamlBasePath, ...e.path], + }, + message: e.message, + }) + } catch { + // ignore } return e @@ -218,7 +217,25 @@ function improveZodValidationErrorMessage(item: Joi.ValidationErrorItem, yamlBas } } -function addYamlContext({ rawYaml, range, message }: { rawYaml: string; range: Range; message: string }): string { +export function addYamlContext({ + source: { yamlDoc, path }, + message, +}: { + source: ConfigSource + message: string +}): string { + if (!yamlDoc) { + return message + } + + const node = yamlDoc.getIn(path, true) as ParsedNode | undefined + const range = node?.range + const rawYaml = yamlDoc.source + + if (!node || !range || !rawYaml) { + return message + } + // Get one line before the error location start, and the line including the error location end const toStart = rawYaml.slice(0, range[0]) const lineNumber = toStart.split("\n").length + 1 @@ -256,5 +273,5 @@ function addYamlContext({ rawYaml, range, message }: { rawYaml: string; range: R const errorLineOffset = range[0] - errorLineStart + linePrefixLength + 2 const marker = styles.error("-".repeat(errorLineOffset)) + styles.error.bold("^") - return `${snippet}\n${marker}\n${styles.error.bold(message)}` + return `${yamlDoc.filename ? `${styles.secondary(`${yamlDoc.filename}:${lineNumber - (snippetLines - 1)}`)}\n` : ""}${snippet}\n${marker}\n${styles.error.bold(message)}` } diff --git a/core/src/exceptions.ts b/core/src/exceptions.ts index 8c97c9631e..2a53bae1b0 100644 --- a/core/src/exceptions.ts +++ b/core/src/exceptions.ts @@ -18,7 +18,7 @@ import dns from "node:dns" import { styles } from "./logger/styles.js" import type { ExecaError } from "execa" import type { Location } from "./template-string/ast.js" -import type { ConfigSource } from "./config/validation.js" +import { addYamlContext, type ConfigSource } from "./config/validation.js" // Unfortunately, NodeJS does not provide a list of all error codes, so we have to maintain this list manually. // See https://nodejs.org/docs/latest-v18.x/api/dns.html#error-codes @@ -314,14 +314,25 @@ export class TemplateStringError extends GardenError { originalMessage: string constructor(params: GardenErrorParams & { loc: Location; yamlSource: ConfigSource }) { - const path = params.yamlSource.path - const pathDescription = path.length > 0 ? ` at path ${styles.accent(path.join("."))}` : "" - const prefix = `Invalid template string (${styles.accent( - truncate(params.loc.source.rawTemplateString, { length: 200 }).replace(/\n/g, "\\n") - )})${pathDescription}: ` - const message = params.message.startsWith(prefix) ? params.message : prefix + params.message - - super({ ...params, message }) + let enriched: string = params.message + try { + // TODO: Use Location information from parser to point to the specific part + enriched = addYamlContext({ source: params.yamlSource, message: params.message }) + } catch { + // ignore + } + + if (enriched === params.message) { + const { path } = params.yamlSource + + const pathDescription = path.length > 0 ? ` at path ${styles.accent(path.join("."))}` : "" + const prefix = `Invalid template string (${styles.accent( + truncate(params.loc.source.rawTemplateString, { length: 200 }).replace(/\n/g, "\\n") + )})${pathDescription}: ` + enriched = params.message.startsWith(prefix) ? params.message : prefix + params.message + } + + super({ ...params, message: enriched }) this.loc = params.loc this.originalMessage = params.message } diff --git a/core/src/plugins/kubernetes/kubernetes-type/common.ts b/core/src/plugins/kubernetes/kubernetes-type/common.ts index b4b01e797c..e455ac5cf3 100644 --- a/core/src/plugins/kubernetes/kubernetes-type/common.ts +++ b/core/src/plugins/kubernetes/kubernetes-type/common.ts @@ -368,7 +368,11 @@ async function readKustomizeManifests( log, args: ["build", kustomizePath, ...extraArgs], }) - const manifests = await parseKubernetesManifests(kustomizeOutput, `kustomize output of ${action.longDescription()}`) + const manifests = await parseKubernetesManifests( + kustomizeOutput, + `kustomize output of ${action.longDescription()}`, + undefined + ) return manifests.map((manifest, index) => ({ declaration: { type: "kustomize", @@ -427,7 +431,8 @@ async function readFileManifests( const resolved = ctx.resolveTemplateStrings(str, { allowPartial: true, unescape: true }) const manifests = await parseKubernetesManifests( resolved, - `${basename(absPath)} in directory ${dirname(absPath)} (specified in ${action.longDescription()})` + `${basename(absPath)} in directory ${dirname(absPath)} (specified in ${action.longDescription()})`, + absPath ) return manifests.map((manifest, index) => ({ declaration: { @@ -449,10 +454,14 @@ async function readFileManifests( * @param str raw string containing Kubernetes manifests in YAML format * @param sourceDescription description of where the YAML string comes from, e.g. "foo.yaml in directory /bar" */ -async function parseKubernetesManifests(str: string, sourceDescription: string): Promise<KubernetesResource[]> { +async function parseKubernetesManifests( + str: string, + sourceDescription: string, + filename: string | undefined +): Promise<KubernetesResource[]> { // parse yaml with version 1.1 by default, as Kubernetes still uses this version. // See also https://github.com/kubernetes/kubernetes/issues/34146 - const docs = await loadAndValidateYaml(str, sourceDescription, "1.1") + const docs = await loadAndValidateYaml({ content: str, sourceDescription, filename, version: "1.1" }) // TODO: We should use schema validation to make sure that apiVersion, kind and metadata are always defined as required by the type. const manifests = docs.map((d) => d.toJS()) as KubernetesResource[] diff --git a/core/test/unit/src/config/base.ts b/core/test/unit/src/config/base.ts index 8b40095981..770cda5b8d 100644 --- a/core/test/unit/src/config/base.ts +++ b/core/test/unit/src/config/base.ts @@ -610,7 +610,11 @@ describe("loadAndValidateYaml", () => { name: foo ` - const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + const yamlDocs = await loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + }) expect(yamlDocs).to.have.length(1) expect(yamlDocs[0].source).to.equal(yaml) @@ -633,7 +637,11 @@ describe("loadAndValidateYaml", () => { name: doc3 ` - const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + const yamlDocs = await loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + }) expect(yamlDocs).to.have.length(3) @@ -662,7 +670,11 @@ describe("loadAndValidateYaml", () => { newYamlOctalNumber: 0o777 ` - const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + const yamlDocs = await loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + }) expect(yamlDocs).to.have.length(1) expect(yamlDocs[0].source).to.equal(yaml) @@ -684,7 +696,11 @@ describe("loadAndValidateYaml", () => { newYamlOctalNumber: 0o777 ` - const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + const yamlDocs = await loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + }) expect(yamlDocs).to.have.length(1) expect(yamlDocs[0].source).to.equal(yaml) @@ -704,7 +720,12 @@ describe("loadAndValidateYaml", () => { ` // we use the version parameter to force the yaml 1.1 standard - const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "1.1") + const yamlDocs = await loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + version: "1.1", + }) expect(yamlDocs).to.have.length(1) expect(yamlDocs[0].source).to.equal(yaml) @@ -720,7 +741,12 @@ describe("loadAndValidateYaml", () => { ` await expectError( - () => loadAndValidateYaml(yaml, "foo.yaml in directory bar"), + () => + loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + }), (err) => { expect(err.message).to.eql(dedent` Could not parse foo.yaml in directory bar as valid YAML: YAMLException: unidentified alias "bar" (1:10) diff --git a/core/test/unit/src/config/validation.ts b/core/test/unit/src/config/validation.ts index e480081b5b..cf33bc47ae 100644 --- a/core/test/unit/src/config/validation.ts +++ b/core/test/unit/src/config/validation.ts @@ -306,7 +306,11 @@ describe("validateSchema", () => { name: bar ` - const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + const yamlDocs = await loadAndValidateYaml({ + content: yaml, + sourceDescription: "foo.yaml in directory bar", + filename: "bar/foo.yaml", + }) const yamlDoc = yamlDocs[1] // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -324,6 +328,7 @@ describe("validateSchema", () => { expect(stripAnsi(err.message)).to.equal(dedent` Validation error: + bar/foo.yaml:10 ... 9 | spec: 10 | foo: 456 @@ -347,7 +352,8 @@ describe("validateSchema", () => { ` const yamlDoc = parseDocument(yaml) as YamlDocumentWithSource - yamlDoc["source"] = yaml + yamlDoc.source = yaml + yamlDoc.filename = "foo/bar.yaml" // eslint-disable-next-line @typescript-eslint/no-explicit-any const config: any = { @@ -364,6 +370,7 @@ describe("validateSchema", () => { expect(stripAnsi(err.message)).to.equal(dedent` Validation error: + foo/bar.yaml:4 ... 3 | spec: 4 | foo: 123 diff --git a/core/test/unit/src/template-string.ts b/core/test/unit/src/template-string.ts index d9859d915c..b0783953a7 100644 --- a/core/test/unit/src/template-string.ts +++ b/core/test/unit/src/template-string.ts @@ -26,6 +26,7 @@ import { UnresolvableValue, visitAll, } from "../../../src/template-string/static-analysis.js" +import { loadAndValidateYaml } from "../../../src/config/base.js" describe("resolveTemplateString", () => { it("should return a non-templated string unchanged", () => { @@ -368,6 +369,41 @@ describe("resolveTemplateString", () => { }) }) + it("if available, should include yaml context in error message", async () => { + const command = "${resol${part}ed}" + const yamlDoc = await loadAndValidateYaml({ + content: dedent` + name: test, + kind: Build + spec: + command: '${command}' + `, + sourceDescription: "test", + filename: "bar/foo.yaml", + }) + void expectError( + () => + resolveTemplateString({ + string: command, + context: new GenericContext({}), + source: { + yamlDoc: yamlDoc[0], + path: ["spec", "command"], + }, + }), + { + contains: dedent` + bar/foo.yaml:4 + ... + 3 | spec: + 4 | command: '\${resol\${part}ed} + ----------------^ + unable to parse as valid template string. + `, + } + ) + }) + it("should handle a single-quoted string", () => { const res = resolveTemplateString({ string: "${'foo'}", context: new GenericContext({}) }) expect(res).to.equal("foo") diff --git a/plugins/pulumi/src/helpers.ts b/plugins/pulumi/src/helpers.ts index 47286c3344..4ee6824bbf 100644 --- a/plugins/pulumi/src/helpers.ts +++ b/plugins/pulumi/src/helpers.ts @@ -262,10 +262,11 @@ export async function applyConfig(params: PulumiParams & { previewDirPath?: stri let stackConfigFileExists: boolean try { const fileData = await readFile(stackConfigPath) - const stackConfigDocs = await loadAndValidateYaml( - fileData.toString(), - `${basename(stackConfigPath)} in directory ${dirname(stackConfigPath)}` - ) + const stackConfigDocs = await loadAndValidateYaml({ + content: fileData.toString(), + sourceDescription: `${basename(stackConfigPath)} in directory ${dirname(stackConfigPath)}`, + filename: stackConfigPath, + }) stackConfig = stackConfigDocs[0].toJS() stackConfigFileExists = true } catch (err) {