fix: prevent crash due to unresolved alias in yaml (#5215)

* chore(deps): update yaml to 2.3.2

* fix: prevent crash due to unresolved alias in yaml

Fixes #5186

* improvement: use loadAndValidateYaml for both kubernetes manifests and
garden configs

test: add tests for loadAndValidateYaml

* improvement: eliminate explicit any in loadAndValidateYaml

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"

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,30 +93,42 @@ 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<YamlDocumentWithSource[]> {
+export async function loadAndValidateYaml(
+  content: string,
+  sourceDescription: string,
+  version: DocumentOptions["version"] = "1.2"
+): Promise<YamlDocumentWithSource[]> {
   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
     try {
       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)

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<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")
 
   // 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[]

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
+          --------------^
+        `)
+      }
+    )
+  })
+})

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 = {

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}`)