diff --git a/core/src/config/base.ts b/core/src/config/base.ts index 940f4a86e75..4d09fe44dac 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,11 +93,11 @@ 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/". */ -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: any) => { if (doc.errors.length > 0) { throw doc.errors[0] } @@ -115,13 +115,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} `, @@ -161,7 +161,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 21b80f9de76..10782d50f7c 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,7 @@ 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", @@ -446,37 +446,15 @@ async function readFileManifests( * @throws ConfigurationError on parser errors. * @param str raw string containing Kubernetes manifests in YAML format */ -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]}`, - }) - } - } - - let manifests: any[] - try { - manifests = docs.map((d) => d.toJS()) - } catch (error) { - // toJS sometimes throws errors that are not caught by the above error check. - // See also https://github.com/eemeli/yaml/issues/497 - throw new ConfigurationError({ - message: `Failed to parse Kubernetes manifest: ${error}`, - }) - } +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. - return expandListManifests(manifests as KubernetesResource[]) + const manifests = docs.map((d) => d.toJS()) as KubernetesResource[] + + return expandListManifests(manifests) } /** diff --git a/core/test/unit/src/config/base.ts b/core/test/unit/src/config/base.ts index 9e5c3215e73..629b2c44dc1 100644 --- a/core/test/unit/src/config/base.ts +++ b/core/test/unit/src/config/base.ts @@ -604,7 +604,7 @@ describe("loadAndValidateYaml", () => { name: foo ` - const yamlDocs = await loadAndValidateYaml(yaml, "/foo") + const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar") expect(yamlDocs).to.have.length(1) expect(yamlDocs[0].source).to.equal(yaml) @@ -618,16 +618,111 @@ describe("loadAndValidateYaml", () => { }) }) + 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"), + () => loadAndValidateYaml(yaml, "foo.yaml in directory bar"), (err) => { - expect(err.message).to.include("ReferenceError: Unresolved alias") - expect(err.message).to.include("bar") + 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 b1241b3348a..b027f4b31a6 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/plugins/pulumi/src/helpers.ts b/plugins/pulumi/src/helpers.ts index 0501b0468ac..6d98475c5a2 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,7 @@ 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}`)