From 92ec7c36a7b894537388150ac74769a1e8856402 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Wed, 15 Jan 2025 14:00:44 -0600 Subject: [PATCH] chore: consolidate file parsing implementation (#1663) ## Description This PR provides a fix to feedback on #1642 ## Related Issue Relates to #1640 ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Other (security config, docs update, etc) ## Checklist before merging - [x] Unit, [Journey](https://github.com/defenseunicorns/pepr/tree/main/journey), [E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples), [docs](https://github.com/defenseunicorns/pepr/tree/main/docs), [adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or updated as needed - [x] [Contributor Guide Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request) followed --- integration/cli/build.noembed.test.ts | 2 +- integration/cli/build.nonconflict.test.ts | 26 ++++++--- integration/cli/build.registryinfo.test.ts | 8 +-- integration/helpers/resource.test.ts | 14 ++--- integration/helpers/resource.ts | 64 ++++++---------------- 5 files changed, 48 insertions(+), 66 deletions(-) diff --git a/integration/cli/build.noembed.test.ts b/integration/cli/build.noembed.test.ts index cefdeb34d..e364f1ae2 100644 --- a/integration/cli/build.noembed.test.ts +++ b/integration/cli/build.noembed.test.ts @@ -51,7 +51,7 @@ describe("build", () => { expect(build.stderr.join("").trim()).toContain("Error: Cannot find module"); expect(build.stdout.join("").trim()).toContain(""); - packageJson = await resource.oneFromFile(`${testModule}/package.json`); + packageJson = await resource.resourcesFromFile(`${testModule}/package.json`); uuid = packageJson.pepr.uuid; }, time.toMs("1m"), diff --git a/integration/cli/build.nonconflict.test.ts b/integration/cli/build.nonconflict.test.ts index 764c35ebd..63cab02cc 100644 --- a/integration/cli/build.nonconflict.test.ts +++ b/integration/cli/build.nonconflict.test.ts @@ -69,7 +69,7 @@ describe("build", () => { expect(build.stderr.join("").trim()).toBe(""); expect(build.stdout.join("").trim()).toContain("K8s resource for the module saved"); - packageJson = await resource.oneFromFile(`${testModule}/package.json`); + packageJson = await resource.resourcesFromFile(`${testModule}/package.json`); uuid = packageJson.pepr.uuid; }, time.toMs("1m"), @@ -93,7 +93,9 @@ describe("build", () => { }); it("--custom-image, works", async () => { - const moduleYaml = await resource.manyFromFile(`${outputDir}/pepr-module-${uuid}.yaml`); + const moduleYaml = await resource.resourcesFromFile( + `${outputDir}/pepr-module-${uuid}.yaml`, + ); { const admission = resource.select(moduleYaml, kind.Deployment, `pepr-${uuid}`); const admissionImage = getDepConImg(admission, "server"); @@ -104,13 +106,15 @@ describe("build", () => { expect(watcherImage).toBe(customImage); } - const zarfYaml = await resource.oneFromFile(`${outputDir}/zarf.yaml`); + const zarfYaml = await resource.resourcesFromFile(`${outputDir}/zarf.yaml`); { const componentImage = zarfYaml.components.at(0).images.at(0); expect(componentImage).toBe(customImage); } - const valuesYaml = await resource.oneFromFile(`${outputDir}/${uuid}-chart/values.yaml`); + const valuesYaml = await resource.resourcesFromFile( + `${outputDir}/${uuid}-chart/values.yaml`, + ); { const admissionImage = valuesYaml.admission.image; expect(admissionImage).toBe(customImage); @@ -121,7 +125,9 @@ describe("build", () => { }); it("--timeout, works", async () => { - const moduleYaml = await resource.manyFromFile(`${outputDir}/pepr-module-${uuid}.yaml`); + const moduleYaml = await resource.resourcesFromFile( + `${outputDir}/pepr-module-${uuid}.yaml`, + ); { const mwc = resource.select( moduleYaml, @@ -145,7 +151,9 @@ describe("build", () => { expect(webhook.timeoutSeconds).toBe(timeout); } - const valuesYaml = await resource.oneFromFile(`${outputDir}/${uuid}-chart/values.yaml`); + const valuesYaml = await resource.resourcesFromFile( + `${outputDir}/${uuid}-chart/values.yaml`, + ); expect(valuesYaml.admission.webhookTimeout).toBe(timeout); }); @@ -156,7 +164,9 @@ describe("build", () => { ); }; - const moduleYaml = await resource.manyFromFile(`${outputDir}/pepr-module-${uuid}.yaml`); + const moduleYaml = await resource.resourcesFromFile( + `${outputDir}/pepr-module-${uuid}.yaml`, + ); const admission = resource.select(moduleYaml, kind.Deployment, `pepr-${uuid}`); const admissionSecrets = getDepImgPull(admission); expect(admissionSecrets).toEqual([withPullSecret]); @@ -174,7 +184,7 @@ describe("build", () => { localPath: `${uuid}-chart`, }; - const zarfYaml = await resource.oneFromFile(`${outputDir}/zarf.yaml`); + const zarfYaml = await resource.resourcesFromFile(`${outputDir}/zarf.yaml`); const component = zarfYaml.components .filter((component: { name: string }) => component.name === "module") .at(0); diff --git a/integration/cli/build.registryinfo.test.ts b/integration/cli/build.registryinfo.test.ts index ae03947c7..e192e84f7 100644 --- a/integration/cli/build.registryinfo.test.ts +++ b/integration/cli/build.registryinfo.test.ts @@ -59,7 +59,7 @@ describe("build", () => { expect(build.stderr.join("").trim()).toBe(""); expect(build.stdout.join("").trim()).toContain("K8s resource for the module saved"); - packageJson = await resource.oneFromFile(`${testModule}/package.json`); + packageJson = await resource.resourcesFromFile(`${testModule}/package.json`); uuid = packageJson.pepr.uuid; }, time.toMs("1m"), @@ -69,7 +69,7 @@ describe("build", () => { const image = `${registryInfo}/custom-pepr-controller:0.0.0-development`; { - const moduleYaml = await resource.manyFromFile( + const moduleYaml = await resource.resourcesFromFile( `${testModule}/dist/pepr-module-${uuid}.yaml`, ); const admission = resource.select(moduleYaml, kind.Deployment, `pepr-${uuid}`); @@ -81,12 +81,12 @@ describe("build", () => { expect(watcherImage).toBe(image); } { - const zarfYaml = await resource.oneFromFile(`${testModule}/dist/zarf.yaml`); + const zarfYaml = await resource.resourcesFromFile(`${testModule}/dist/zarf.yaml`); const componentImage = zarfYaml.components.at(0).images.at(0); expect(componentImage).toBe(image); } { - const valuesYaml = await resource.oneFromFile( + const valuesYaml = await resource.resourcesFromFile( `${testModule}/dist/${uuid}-chart/values.yaml`, ); const admissionImage = valuesYaml.admission.image; diff --git a/integration/helpers/resource.test.ts b/integration/helpers/resource.test.ts index 787737b7a..4330a8543 100644 --- a/integration/helpers/resource.test.ts +++ b/integration/helpers/resource.test.ts @@ -17,7 +17,7 @@ beforeAll(async () => { await workdir.recreate(); }); -describe("oneFromFile", () => { +describe("resourcesFromFile", () => { it("can load one resource from .json file", async () => { const oneJson = `${workdir.path()}/one.json`; await fs.writeFile( @@ -28,7 +28,7 @@ describe("oneFromFile", () => { } `, ); - const result = await sut.oneFromFile(oneJson); + const result = await sut.resourcesFromFile(oneJson); expect(result.one).toBe("json"); }); @@ -41,12 +41,12 @@ describe("oneFromFile", () => { one: yaml `, ); - const result = await sut.oneFromFile(oneYaml); + const result = await sut.resourcesFromFile(oneYaml); expect(result.one).toBe("yaml"); }); }); -describe("manyFromFile", () => { +describe("resourcesFromFile", () => { it("can load many resources from .json file", async () => { const manyJson = `${workdir.path()}/many.json`; await fs.writeFile( @@ -65,7 +65,7 @@ describe("manyFromFile", () => { ] `, ); - const result = await sut.manyFromFile(manyJson); + const result = await sut.resourcesFromFile(manyJson); expect(result.at(0).one).toBe("json"); expect(result.at(1).two).toBe("json"); expect(result.at(2).three).toBe("json"); @@ -84,7 +84,7 @@ describe("manyFromFile", () => { three: yaml `, ); - const result = await sut.manyFromFile(manyYaml); + const result = await sut.resourcesFromFile(manyYaml); expect(result.at(0).one).toBe("yaml"); expect(result.at(1).two).toBe("yaml"); expect(result.at(2).three).toBe("yaml"); @@ -115,7 +115,7 @@ describe("select", () => { fake: news `, ); - const many = await sut.manyFromFile(manyYaml); + const many = await sut.resourcesFromFile(manyYaml); const sec = sut.select(many, kind.Secret, "sec"); const cm = sut.select(many, kind.ConfigMap, "cm"); diff --git a/integration/helpers/resource.ts b/integration/helpers/resource.ts index 50eaadcf7..876590508 100644 --- a/integration/helpers/resource.ts +++ b/integration/helpers/resource.ts @@ -1,65 +1,37 @@ import { readFile } from "node:fs/promises"; import { kind, KubernetesObject } from "kubernetes-fluent-client"; -import { parseDocument, parseAllDocuments } from "yaml"; +import { parseAllDocuments } from "yaml"; /** - * Read one resource from file, rehydrated as a JS object + * Read resources from a file and return them as JS objects. * - * @param path Path to file holding one JSON (*.json) object / YAML (*.yaml) document - * @returns JS object + * @param path Path to the file (supports JSON (*.json) or YAML (*.yaml)) + * @returns JS object or array of JS objects. */ /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ -export async function oneFromFile(path: string): Promise { - const ext = path.split(".").at(-1); +export async function resourcesFromFile(path: string): Promise { + const extension = path.split(".").at(-1); - let ret: object; - switch (ext) { - case "json": { - const all = JSON.parse(await readFile(path, { encoding: "utf8" })); - ret = Array.isArray(all) ? all.at(0) : all; - break; - } - - case "yaml": - ret = parseDocument(await readFile(path, { encoding: "utf8" })).contents!.toJSON(); - break; - - default: - throw `oops: don't recognize file of type ".${ext}"`; - } - - return ret; -} + let result: object | object[]; + const content = await readFile(path, { encoding: "utf8" }); -/** - * Read many resources from file, rehydrated as an array of JS objects - * - * @param path Path to file holding an array of JSON (*.json) objects / multiple concatinated YAML (*.yaml) documents - * @returns Array of JS objects - */ -/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ -export async function manyFromFile(path: string): Promise { - const ext = path.split(".").at(-1); - - let ret: object[]; - switch (ext) { + switch (extension) { case "json": { - const all = JSON.parse(await readFile(path, { encoding: "utf8" })); - ret = Array.isArray(all) ? all : [all]; + const parsed = JSON.parse(content); + result = Array.isArray(parsed) ? parsed : [parsed]; break; } - - case "yaml": - ret = parseAllDocuments(await readFile(path, { encoding: "utf8" })).map(yamlDoc => - yamlDoc.contents!.toJSON(), - ); + case "yaml": { + const documents = parseAllDocuments(content).map(doc => doc.contents!.toJSON()); + result = documents.length === 1 ? documents[0] : documents; break; - + } default: - throw `oops: don't recognize file of type ".${ext}"`; + throw new Error(`Unsupported file type ".${extension}"`); } - return ret; + // If the result is an array with one element, return the single element + return Array.isArray(result) && result.length === 1 ? result[0] : result; } /**