From e36fa6f060f65a83085a927370ae6392c3c98473 Mon Sep 17 00:00:00 2001 From: Steffen Neubauer Date: Tue, 10 Dec 2024 12:16:19 +0100 Subject: [PATCH] refactor: Use named parameters instead of positional in `loadAndValidateYaml` --- core/src/config/base.ts | 27 +++++++------ .../kubernetes/kubernetes-type/common.ts | 2 +- core/test/unit/src/config/base.ts | 38 ++++++++++++++++--- core/test/unit/src/config/validation.ts | 6 ++- core/test/unit/src/template-string.ts | 10 ++--- plugins/pulumi/src/helpers.ts | 10 ++--- 6 files changed, 64 insertions(+), 29 deletions(-) diff --git a/core/src/config/base.ts b/core/src/config/base.ts index 1b9c4b11bd..597869e96a 100644 --- a/core/src/config/base.ts +++ b/core/src/config/base.ts @@ -126,12 +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, - filename: string | undefined, - version: DocumentOptions["version"] = "1.2" -): Promise { +export async function loadAndValidateYaml({ + content, + sourceDescription, + filename, + version = "1.2", +}: { + content: string + sourceDescription: string + filename: string | undefined + version?: DocumentOptions["version"] +}): Promise { try { return Array.from(parseAllDocuments(content, { merge: true, strict: false, version }) || []).map((doc) => { if (doc.errors.length > 0) { @@ -200,11 +205,11 @@ export async function validateRawConfig({ projectRoot: string allowInvalid?: boolean }) { - let rawSpecs = await loadAndValidateYaml( - rawConfig, - `${basename(configPath)} in directory ${dirname(configPath)}`, - 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/plugins/kubernetes/kubernetes-type/common.ts b/core/src/plugins/kubernetes/kubernetes-type/common.ts index e0a29ef51c..e455ac5cf3 100644 --- a/core/src/plugins/kubernetes/kubernetes-type/common.ts +++ b/core/src/plugins/kubernetes/kubernetes-type/common.ts @@ -461,7 +461,7 @@ async function parseKubernetesManifests( ): 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") + 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 2aa7efe624..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", "bar/foo.yaml") + 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", "bar/foo.yaml") + 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", "bar/foo.yaml") + 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", "bar/foo.yaml") + 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", "bar/foo.yaml", "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", "bar/foo.yaml"), + () => + 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 907af96f38..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", "bar/foo.yaml") + 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 diff --git a/core/test/unit/src/template-string.ts b/core/test/unit/src/template-string.ts index fea3c72a48..d833944b8e 100644 --- a/core/test/unit/src/template-string.ts +++ b/core/test/unit/src/template-string.ts @@ -317,16 +317,16 @@ describe("resolveTemplateString", () => { it("if available, should include yaml context in error message", async () => { const command = "${resol${part}ed}" - const yamlDoc = await loadAndValidateYaml( - dedent` + const yamlDoc = await loadAndValidateYaml({ + content: dedent` name: test, kind: Build spec: command: '${command}' `, - "test", - "bar/foo.yaml" - ) + sourceDescription: "test", + filename: "bar/foo.yaml", + }) void expectError( () => resolveTemplateString({ diff --git a/plugins/pulumi/src/helpers.ts b/plugins/pulumi/src/helpers.ts index 99a8fcf4e8..4ee6824bbf 100644 --- a/plugins/pulumi/src/helpers.ts +++ b/plugins/pulumi/src/helpers.ts @@ -262,11 +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)}`, - 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) {