Skip to content

Commit

Permalink
fix: prevent crash due to unresolved alias in yaml
Browse files Browse the repository at this point in the history
Fixes #5186
  • Loading branch information
stefreak committed Oct 9, 2023
1 parent 0ba6d82 commit 6b1166a
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 3 deletions.
5 changes: 5 additions & 0 deletions core/src/config/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ export async function loadAndValidateYaml(content: string, path: string): Promis
if (doc.errors.length > 0) {
throw doc.errors[0]
}
// 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()

doc.source = content
return doc
})
Expand Down
15 changes: 12 additions & 3 deletions core/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,10 +464,19 @@ function parseKubernetesManifests(str: string): KubernetesResource[] {
}
}

// 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[]
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}`,
})
}

return expandListManifests(manifests)
// 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[])
}

/**
Expand Down
41 changes: 41 additions & 0 deletions core/test/unit/src/config/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
noTemplateFields,
validateRawConfig,
configTemplateKind,
loadAndValidateYaml,
} from "../../../../src/config/base"
import { resolve, join } from "path"
import { expectError, getDataDir, getDefaultProjectConfig } from "../../../helpers"
Expand All @@ -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")
Expand Down Expand Up @@ -591,3 +593,42 @@ 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")

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("should throw ConfigurationError if yaml contains reference to undefined alias", async () => {
const yaml = dedent`
foo: *bar
`

await expectError(
() => loadAndValidateYaml(yaml, "/foo"),
(err) => {
expect(err.message).to.include("ReferenceError: Unresolved alias")
expect(err.message).to.include("bar")
}
)
})
})

0 comments on commit 6b1166a

Please sign in to comment.