diff --git a/core/package.json b/core/package.json index bdac64af2e..027c36353e 100644 --- a/core/package.json +++ b/core/package.json @@ -162,7 +162,7 @@ "wrap-ansi": "^7.0.0", "write-file-atomic": "^5.0.0", "ws": "^8.12.1", - "yaml": "^2.2.2", + "yaml": "^2.3.2", "yaml-lint": "^1.2.4", "zod": "^3.20.6", "zod-to-json-schema": "^3.21.1" diff --git a/core/src/config/base.ts b/core/src/config/base.ts index 394da15503..d7b07f8d7a 100644 --- a/core/src/config/base.ts +++ b/core/src/config/base.ts @@ -26,7 +26,7 @@ import { emitNonRepeatableWarning } from "../warnings" import { ActionKind, actionKinds } from "../actions/types" import { mayContainTemplateString } from "../template-string/template-string" import { Log } from "../logger/log-entry" -import { Document, parseAllDocuments } from "yaml" +import { Document, DocumentOptions, parseAllDocuments } from "yaml" import { dedent, deline } from "../util/string" export const configTemplateKind = "ConfigTemplate" @@ -93,16 +93,28 @@ export const allConfigKinds = ["Module", "Workflow", "Project", configTemplateKi * content is not valid YAML. * * @param content - The contents of the file as a string. - * @param path - The path to the file. + * @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, path: string): Promise { +export async function loadAndValidateYaml( + content: string, + sourceDescription: string, + version: DocumentOptions["version"] = "1.2" +): Promise { try { - return Array.from(parseAllDocuments(content, { merge: true, strict: false }) || []).map((doc: any) => { + return Array.from(parseAllDocuments(content, { merge: true, strict: false, version }) || []).map((doc) => { if (doc.errors.length > 0) { throw doc.errors[0] } - doc.source = content - return doc + // Workaround: Call toJS might throw an error that is not listed in the errors above. + // See also https://github.com/eemeli/yaml/issues/497 + // We call this here to catch this error early and prevent crashes later on. + doc.toJS() + + const docWithSource = doc as YamlDocumentWithSource + docWithSource.source = content + + return docWithSource }) } catch (loadErr) { // We try to find the error using a YAML linter @@ -110,13 +122,13 @@ export async function loadAndValidateYaml(content: string, path: string): Promis await lint(content) } catch (linterErr) { throw new ConfigurationError({ - message: `Could not parse ${basename(path)} in directory ${path} as valid YAML: ${linterErr}`, + message: `Could not parse ${sourceDescription} as valid YAML: ${linterErr}`, }) } // ... but default to throwing a generic error, in case the error wasn't caught by yaml-lint. throw new ConfigurationError({ message: dedent` - Failed to load YAML from ${basename(path)} in directory ${path}. + Failed to load YAML from ${sourceDescription}. Linting the file did not yield any errors. This is all we know: ${loadErr} `, @@ -156,7 +168,7 @@ export async function validateRawConfig({ projectRoot: string allowInvalid?: boolean }) { - let rawSpecs = await loadAndValidateYaml(rawConfig, configPath) + let rawSpecs = await loadAndValidateYaml(rawConfig, `${basename(configPath)} in directory ${dirname(configPath)}`) // Ignore empty resources rawSpecs = rawSpecs.filter(Boolean) diff --git a/core/src/plugins/kubernetes/kubernetes-type/common.ts b/core/src/plugins/kubernetes/kubernetes-type/common.ts index 46997373c8..8c648ef0f4 100644 --- a/core/src/plugins/kubernetes/kubernetes-type/common.ts +++ b/core/src/plugins/kubernetes/kubernetes-type/common.ts @@ -6,7 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { join, resolve } from "path" +import { basename, dirname, join, resolve } from "path" import { pathExists, readFile } from "fs-extra" import { flatten, keyBy, set } from "lodash" @@ -29,8 +29,8 @@ import { V1ConfigMap } from "@kubernetes/client-node" import { glob } from "glob" import isGlob from "is-glob" import pFilter from "p-filter" -import { parseAllDocuments } from "yaml" import { kubectl } from "../kubectl" +import { loadAndValidateYaml } from "../../../config/base" /** * "DeployFile": Manifest has been read from one of the files declared in Garden Deploy `spec.files` @@ -369,7 +369,7 @@ async function readKustomizeManifests( log, args: ["build", kustomizePath, ...extraArgs], }) - const manifests = parseKubernetesManifests(kustomizeOutput) + const manifests = await parseKubernetesManifests(kustomizeOutput, `kustomize output of ${action.longDescription()}`) return manifests.map((manifest, index) => ({ declaration: { type: "kustomize", @@ -426,7 +426,10 @@ async function readFileManifests( 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 }) - const manifests = parseKubernetesManifests(resolved) + const manifests = await parseKubernetesManifests( + resolved, + `${basename(absPath)} in directory ${dirname(absPath)} (specified in ${action.longDescription()})` + ) return manifests.map((manifest, index) => ({ declaration: { type: "file", @@ -445,24 +448,12 @@ async function readFileManifests( * * @throws ConfigurationError on parser errors. * @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" */ -function parseKubernetesManifests(str: string): KubernetesResource[] { - const docs = Array.from( - parseAllDocuments(str, { - // Kubernetes uses the YAML 1.1 spec by default and not YAML 1.2, which is the default for most libraries. - // See also https://github.com/kubernetes/kubernetes/issues/34146 - schema: "yaml-1.1", - strict: false, - }) - ) - - for (const doc of docs) { - if (doc.errors.length > 0) { - throw new ConfigurationError({ - message: `Failed to parse Kubernetes manifest: ${doc.errors[0]}`, - }) - } - } +async function parseKubernetesManifests(str: string, sourceDescription: string): Promise { + // 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") // 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 9544205857..9e504d4675 100644 --- a/core/test/unit/src/config/base.ts +++ b/core/test/unit/src/config/base.ts @@ -15,6 +15,7 @@ import { noTemplateFields, validateRawConfig, configTemplateKind, + loadAndValidateYaml, } from "../../../../src/config/base" import { resolve, join } from "path" import { expectError, getDataDir, getDefaultProjectConfig } from "../../../helpers" @@ -25,6 +26,7 @@ import { getRootLogger } from "../../../../src/logger/logger" import { ConfigurationError } from "../../../../src/exceptions" import { resetNonRepeatableWarningHistory } from "../../../../src/warnings" import { omit } from "lodash" +import { dedent } from "../../../../src/util/string" const projectPathA = getDataDir("test-project-a") const modulePathA = resolve(projectPathA, "module-a") @@ -591,3 +593,136 @@ describe("findProjectConfig", async () => { }) }) }) + +describe("loadAndValidateYaml", () => { + it("should load and validate yaml and annotate every document with the source", async () => { + const yaml = dedent` + apiVersion: v1 + kind: Test + spec: + foo: bar + name: foo + ` + + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + + expect(yamlDocs).to.have.length(1) + expect(yamlDocs[0].source).to.equal(yaml) + expect(yamlDocs[0].toJS()).to.eql({ + apiVersion: "v1", + kind: "Test", + spec: { + foo: "bar", + }, + name: "foo", + }) + }) + + it("supports loading multiple documents", async () => { + const yaml = dedent` + name: doc1 + --- + name: doc2 + --- + name: doc3 + ` + + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + + expect(yamlDocs).to.have.length(3) + + // they all share the same source: + expect(yamlDocs[0].source).to.equal(yaml) + expect(yamlDocs[1].source).to.equal(yaml) + expect(yamlDocs[2].source).to.equal(yaml) + + expect(yamlDocs[0].toJS()).to.eql({ + name: "doc1", + }) + expect(yamlDocs[1].toJS()).to.eql({ + name: "doc2", + }) + expect(yamlDocs[2].toJS()).to.eql({ + name: "doc3", + }) + }) + + it("should use the yaml 1.2 standard by default for reading", async () => { + const yaml = dedent` + # yaml 1.2 will interpret this as decimal number 777 (in accordance to the standard) + oldYamlOctalNumber: 0777 + + # yaml 1.2 will interpret this as octal number 0o777 (in accordance to the standard) + newYamlOctalNumber: 0o777 + ` + + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + + expect(yamlDocs).to.have.length(1) + expect(yamlDocs[0].source).to.equal(yaml) + expect(yamlDocs[0].toJS()).to.eql({ + oldYamlOctalNumber: 777, + newYamlOctalNumber: 0o777, + }) + }) + + it("should allows using the 1.1 yaml standard with the '%YAML 1.1' directive", async () => { + const yaml = dedent` + %YAML 1.1 + --- + + # yaml 1.1 will interpret this as octal number 0o777 (in accordance to the standard) + oldYamlOctalNumber: 0777 + + # yaml 1.1 will interpret this as string (in accordance to the standard) + newYamlOctalNumber: 0o777 + ` + + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") + + expect(yamlDocs).to.have.length(1) + expect(yamlDocs[0].source).to.equal(yaml) + expect(yamlDocs[0].toJS()).to.eql({ + oldYamlOctalNumber: 0o777, + newYamlOctalNumber: "0o777", + }) + }) + + it("should allow using the 1.1 yaml standard using the version parameter", async () => { + const yaml = dedent` + # yaml 1.1 will interpret this as octal number 0o777 (in accordance to the standard) + oldYamlOctalNumber: 0777 + + # yaml 1.1 will interpret this as string (in accordance to the standard) + newYamlOctalNumber: 0o777 + ` + + // we use the version parameter to force the yaml 1.1 standard + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "1.1") + + expect(yamlDocs).to.have.length(1) + expect(yamlDocs[0].source).to.equal(yaml) + expect(yamlDocs[0].toJS()).to.eql({ + oldYamlOctalNumber: 0o777, + newYamlOctalNumber: "0o777", + }) + }) + + it("should throw ConfigurationError if yaml contains reference to undefined alias", async () => { + const yaml = dedent` + foo: *bar + ` + + await expectError( + () => loadAndValidateYaml(yaml, "foo.yaml in directory bar"), + (err) => { + expect(err.message).to.eql(dedent` + Could not parse foo.yaml in directory bar as valid YAML: YAMLException: unidentified alias "bar" (1:10) + + 1 | foo: *bar + --------------^ + `) + } + ) + }) +}) diff --git a/core/test/unit/src/config/validation.ts b/core/test/unit/src/config/validation.ts index b1241b3348..b027f4b31a 100644 --- a/core/test/unit/src/config/validation.ts +++ b/core/test/unit/src/config/validation.ts @@ -307,7 +307,7 @@ describe("validateSchema", () => { name: bar ` - const yamlDocs = await loadAndValidateYaml(yaml, "/foo") + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") const yamlDoc = yamlDocs[1] const config: any = { diff --git a/package-lock.json b/package-lock.json index 54be9637bf..a26a7e71af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3403,7 +3403,7 @@ "wrap-ansi": "^7.0.0", "write-file-atomic": "^5.0.0", "ws": "^8.12.1", - "yaml": "^2.2.2", + "yaml": "^2.3.2", "yaml-lint": "^1.2.4", "zod": "^3.20.6", "zod-to-json-schema": "^3.21.1" diff --git a/plugins/pulumi/src/helpers.ts b/plugins/pulumi/src/helpers.ts index 0501b0468a..22999dabd7 100644 --- a/plugins/pulumi/src/helpers.ts +++ b/plugins/pulumi/src/helpers.ts @@ -11,7 +11,7 @@ import { load } from "js-yaml" import stripAnsi from "strip-ansi" import chalk from "chalk" import { merge } from "json-merge-patch" -import { extname, join, resolve } from "path" +import { basename, dirname, extname, join, resolve } from "path" import { ensureDir, pathExists, readFile } from "fs-extra" import { ChildProcessError, @@ -257,7 +257,10 @@ export async function applyConfig(params: PulumiParams & { previewDirPath?: stri let stackConfigFileExists: boolean try { const fileData = await readFile(stackConfigPath) - stackConfig = (await loadAndValidateYaml(fileData.toString(), stackConfigPath))[0].toJS() + stackConfig = await loadAndValidateYaml( + fileData.toString(), + `${basename(stackConfigPath)} in directory ${dirname(stackConfigPath)}` + )[0].toJS() stackConfigFileExists = true } catch (err) { log.debug(`No pulumi stack configuration file for action ${action.name} found at ${stackConfigPath}`)