From 8adc94246e45b0fb78151c5db7b540e35c79e1f2 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Mon, 9 Dec 2024 09:12:38 -0600 Subject: [PATCH 01/20] wip: initial --- src/lib/mutate-processor.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 941a95817..22554e718 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -14,6 +14,7 @@ import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; import { base64Encode, convertFromBase64Map, convertToBase64Map } from "./utils"; +//TODO export async function mutateProcessor( config: ModuleConfig, capabilities: Capability[], From b70802e35a755345e954c1c5558411863b289af8 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Mon, 9 Dec 2024 11:11:31 -0600 Subject: [PATCH 02/20] wip: saving progress --- src/lib/mutate-processor.test.ts | 103 +++++++++++++++++++++++++++++++ src/lib/mutate-processor.ts | 74 ++++++++++++---------- 2 files changed, 145 insertions(+), 32 deletions(-) create mode 100644 src/lib/mutate-processor.test.ts diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts new file mode 100644 index 000000000..90d5b2d2a --- /dev/null +++ b/src/lib/mutate-processor.test.ts @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2023-Present The Pepr Authors + +// import { beforeEach, describe, expect, it, jest } from "@jest/globals"; +// import { GroupVersionKind, kind, KubernetesObject } from "kubernetes-fluent-client"; +// import { AdmissionRequest, Binding, Filters } from "./types"; +// import { Event, Operation } from "./enums"; +// import { PeprValidateRequest } from "./validate-request"; +// import { clone } from "ramda"; +// import * as sut from "./mutate-processor"; + +// const testFilters: Filters = { +// annotations: {}, +// deletionTimestamp: false, +// labels: {}, +// name: "", +// namespaces: [], +// regexName: "^default$", +// regexNamespaces: [] as string[], +// }; + +// const testGroupVersionKind: GroupVersionKind = { +// kind: "some-kind", +// group: "some-group", +// }; + +// const testBinding: Binding = { +// event: Event.ANY, +// filters: testFilters, +// kind: testGroupVersionKind, +// model: kind.Pod, +// isFinalize: false, +// isMutate: false, +// isQueue: false, +// isValidate: false, +// isWatch: false, +// }; + +// export const testAdmissionRequest: AdmissionRequest = { +// uid: "some-uid", +// kind: { kind: "a-kind", group: "a-group" }, +// resource: { group: "some-group", version: "some-version", resource: "some-resource" }, +// operation: Operation.CONNECT, +// name: "some-name", +// userInfo: {}, +// object: {}, +// }; + +// export const testActionMetadata: Record = {}; + +// export const testPeprValidateRequest = (admissionRequest: AdmissionRequest) => +// new PeprValidateRequest(admissionRequest); + +// describe("processRequest", () => { +// let binding: Binding; +// let actionMetadata: Record; +// let peprValidateRequest: PeprValidateRequest; + +// beforeEach(() => { +// binding = clone(testBinding); +// actionMetadata = clone(testActionMetadata); +// peprValidateRequest = testPeprValidateRequest(testAdmissionRequest); +// }); + +// it("responds on successful validation action", async () => { +// const cbResult = { +// allowed: true, +// statusCode: 200, +// statusMessage: "yay", +// }; +// const callback = jest.fn().mockImplementation(() => cbResult) as Binding["validateCallback"]; +// binding = { ...clone(testBinding), validateCallback: callback }; + +// const result = await sut.processRequest(binding, actionMetadata, peprValidateRequest); + +// expect(result).toEqual({ +// uid: peprValidateRequest.Request.uid, +// allowed: cbResult.allowed, +// status: { +// code: cbResult.statusCode, +// message: cbResult.statusMessage, +// }, +// }); +// }); + +// it("responds on unsuccessful validation action", async () => { +// const callback = jest.fn().mockImplementation(() => { +// throw "oof"; +// }) as Binding["validateCallback"]; +// binding = { ...clone(testBinding), validateCallback: callback }; + +// const result = await sut.processRequest(binding, actionMetadata, peprValidateRequest); + +// expect(result).toEqual({ +// uid: peprValidateRequest.Request.uid, +// allowed: false, +// status: { +// code: 500, +// message: `Action failed with error: "oof"`, +// }, +// }); +// }); +// }); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 22554e718..f8ed685ca 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: 2023-Present The Pepr Authors import jsonPatch from "fast-json-patch"; -import { kind } from "kubernetes-fluent-client"; +import { kind, KubernetesObject } from "kubernetes-fluent-client"; import { Capability } from "./capability"; import { Errors } from "./errors"; @@ -14,14 +14,50 @@ import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; import { base64Encode, convertFromBase64Map, convertToBase64Map } from "./utils"; -//TODO +// Add annotations to the request to indicate that the capability started processing +// this will allow tracking of failed mutations that were permitted to continue +export function updateStatus( + req: AdmissionRequest, + config: ModuleConfig, + name: string, + wrapped: PeprMutateRequest, + status: string, +): PeprMutateRequest { + // Only update the status if the request is a CREATE or UPDATE (we don't use CONNECT) + if (req.operation === "DELETE") { + return wrapped; + } + + // + // Can this block be substituted for a call to wrapped.SetAnnotation()..? + // + const identifier = `${config.uuid}.pepr.dev/${name}`; + wrapped.Raw.metadata = wrapped.Raw.metadata || {}; + wrapped.Raw.metadata.annotations = wrapped.Raw.metadata.annotations || {}; + wrapped.Raw.metadata.annotations[identifier] = status; + + return wrapped; +} + +export function logMutateErrorMessage(e: Error): string { + try { + if (e.message && e.message !== "[object Object]") { + return e.message; + } else { + throw new Error("An error occurred in the mutate action."); + } + } catch (e) { + return "An error occurred with the mutate action."; + } +} + export async function mutateProcessor( config: ModuleConfig, capabilities: Capability[], req: AdmissionRequest, reqMetadata: Record, ): Promise { - const wrapped = new PeprMutateRequest(req); + let wrapped = new PeprMutateRequest(req); const response: MutateResponse = { uid: req.uid, warnings: [], @@ -61,21 +97,7 @@ export async function mutateProcessor( Log.info(actionMetadata, `Processing mutation action (${label})`); matchedAction = true; - // Add annotations to the request to indicate that the capability started processing - // this will allow tracking of failed mutations that were permitted to continue - const updateStatus = (status: string) => { - // Only update the status if the request is a CREATE or UPDATE (we don't use CONNECT) - if (req.operation === "DELETE") { - return; - } - - const identifier = `${config.uuid}.pepr.dev/${name}`; - wrapped.Raw.metadata = wrapped.Raw.metadata || {}; - wrapped.Raw.metadata.annotations = wrapped.Raw.metadata.annotations || {}; - wrapped.Raw.metadata.annotations[identifier] = status; - }; - - updateStatus("started"); + wrapped = updateStatus(req, config, name, wrapped, "started"); try { // Run the action @@ -85,9 +107,9 @@ export async function mutateProcessor( Log.info(actionMetadata, `Mutation action succeeded (${label})`); // Add annotations to the request to indicate that the capability succeeded - updateStatus("succeeded"); + wrapped = updateStatus(req, config, name, wrapped, "succeeded"); } catch (e) { - updateStatus("warning"); + wrapped = updateStatus(req, config, name, wrapped, "warning"); response.warnings = response.warnings || []; const errorMessage = logMutateErrorMessage(e); @@ -152,15 +174,3 @@ export async function mutateProcessor( return response; } - -const logMutateErrorMessage = (e: Error): string => { - try { - if (e.message && e.message !== "[object Object]") { - return e.message; - } else { - throw new Error("An error occurred in the mutate action."); - } - } catch (e) { - return "An error occurred with the mutate action."; - } -}; From 79901849b29c3966a55a3e57818afa33d35f1256 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Mon, 9 Dec 2024 11:23:41 -0600 Subject: [PATCH 03/20] wip: saving progress --- src/lib/mutate-processor.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index f8ed685ca..5185ee949 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -27,14 +27,7 @@ export function updateStatus( if (req.operation === "DELETE") { return wrapped; } - - // - // Can this block be substituted for a call to wrapped.SetAnnotation()..? - // - const identifier = `${config.uuid}.pepr.dev/${name}`; - wrapped.Raw.metadata = wrapped.Raw.metadata || {}; - wrapped.Raw.metadata.annotations = wrapped.Raw.metadata.annotations || {}; - wrapped.Raw.metadata.annotations[identifier] = status; + wrapped.SetAnnotation(`${config.uuid}.pepr.dev/${name}`, status); return wrapped; } From 874ed3184a274e20b4e77768c55e5359ae2e1b27 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Mon, 9 Dec 2024 13:07:25 -0600 Subject: [PATCH 04/20] wip: saving progress --- src/lib/mutate-processor.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 5185ee949..773e3dac3 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -17,14 +17,13 @@ import { base64Encode, convertFromBase64Map, convertToBase64Map } from "./utils" // Add annotations to the request to indicate that the capability started processing // this will allow tracking of failed mutations that were permitted to continue export function updateStatus( - req: AdmissionRequest, config: ModuleConfig, name: string, wrapped: PeprMutateRequest, status: string, ): PeprMutateRequest { // Only update the status if the request is a CREATE or UPDATE (we don't use CONNECT) - if (req.operation === "DELETE") { + if (wrapped.Request.operation === "DELETE") { return wrapped; } wrapped.SetAnnotation(`${config.uuid}.pepr.dev/${name}`, status); @@ -90,7 +89,7 @@ export async function mutateProcessor( Log.info(actionMetadata, `Processing mutation action (${label})`); matchedAction = true; - wrapped = updateStatus(req, config, name, wrapped, "started"); + wrapped = updateStatus(config, name, wrapped, "started"); try { // Run the action @@ -100,9 +99,9 @@ export async function mutateProcessor( Log.info(actionMetadata, `Mutation action succeeded (${label})`); // Add annotations to the request to indicate that the capability succeeded - wrapped = updateStatus(req, config, name, wrapped, "succeeded"); + wrapped = updateStatus(config, name, wrapped, "succeeded"); } catch (e) { - wrapped = updateStatus(req, config, name, wrapped, "warning"); + wrapped = updateStatus(config, name, wrapped, "warning"); response.warnings = response.warnings || []; const errorMessage = logMutateErrorMessage(e); From b56baa64b7adbd96fedb9ee79ad1a9477fee1e42 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Mon, 9 Dec 2024 13:28:48 -0600 Subject: [PATCH 05/20] wip: saving progress --- src/lib/mutate-processor.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index 90d5b2d2a..8edc50123 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -// import { beforeEach, describe, expect, it, jest } from "@jest/globals"; +import { describe, expect, it } from "@jest/globals"; // import { GroupVersionKind, kind, KubernetesObject } from "kubernetes-fluent-client"; // import { AdmissionRequest, Binding, Filters } from "./types"; // import { Event, Operation } from "./enums"; @@ -51,6 +51,18 @@ // export const testPeprValidateRequest = (admissionRequest: AdmissionRequest) => // new PeprValidateRequest(admissionRequest); +describe("updateStatus", () => { + it("does...", () => { + expect(true).toBe(true); + }); +}); + +describe("logMutateErrorMessage", () => { + it("does...", () => { + expect(true).toBe(true); + }); +}); + // describe("processRequest", () => { // let binding: Binding; // let actionMetadata: Record; From bd8a5ee1236a076d9909486572bfcd26b4b3521f Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Mon, 9 Dec 2024 20:43:46 -0600 Subject: [PATCH 06/20] wip: saving progress --- src/lib/mutate-processor.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 773e3dac3..74e3efafd 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -8,7 +8,7 @@ import { Capability } from "./capability"; import { Errors } from "./errors"; import { shouldSkipRequest } from "./filter/filter"; import { MutateResponse } from "./k8s"; -import { AdmissionRequest } from "./types"; +import { AdmissionRequest /*Binding*/ } from "./types"; import Log from "./telemetry/logger"; import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; @@ -72,20 +72,20 @@ export async function mutateProcessor( for (const { name, bindings, namespaces } of capabilities) { const actionMetadata = { ...reqMetadata, name }; - for (const action of bindings) { + for (const binding of bindings) { // Skip this action if it's not a mutate action - if (!action.mutateCallback) { + if (!binding.mutateCallback) { continue; } // Continue to the next action without doing anything if this one should be skipped - const shouldSkip = shouldSkipRequest(action, req, namespaces, config?.alwaysIgnore?.namespaces); + const shouldSkip = shouldSkipRequest(binding, req, namespaces, config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); continue; } - const label = action.mutateCallback.name; + const label = binding.mutateCallback.name; Log.info(actionMetadata, `Processing mutation action (${label})`); matchedAction = true; @@ -93,7 +93,7 @@ export async function mutateProcessor( try { // Run the action - await action.mutateCallback(wrapped); + await binding.mutateCallback(wrapped); // Log on success Log.info(actionMetadata, `Mutation action succeeded (${label})`); @@ -112,7 +112,6 @@ export async function mutateProcessor( switch (config.onError) { case Errors.reject: - Log.error(actionMetadata, `Action failed: ${errorMessage}`); response.result = "Pepr module configured to reject on error"; return response; From 86a5ce3a7bd36303268a1afa434e76e093716cf9 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 09:28:59 -0600 Subject: [PATCH 07/20] wip: saving progress --- src/lib/mutate-processor.ts | 95 +++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 74e3efafd..4163162c5 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -8,12 +8,19 @@ import { Capability } from "./capability"; import { Errors } from "./errors"; import { shouldSkipRequest } from "./filter/filter"; import { MutateResponse } from "./k8s"; -import { AdmissionRequest /*Binding*/ } from "./types"; +import { AdmissionRequest, Binding } from "./types"; import Log from "./telemetry/logger"; import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; import { base64Encode, convertFromBase64Map, convertToBase64Map } from "./utils"; +interface Bindable { + name: string; + namespaces: string[]; + binding: Binding; + actMeta: Record; +} + // Add annotations to the request to indicate that the capability started processing // this will allow tracking of failed mutations that were permitted to continue export function updateStatus( @@ -70,56 +77,62 @@ export async function mutateProcessor( Log.info(reqMetadata, `Processing request`); - for (const { name, bindings, namespaces } of capabilities) { - const actionMetadata = { ...reqMetadata, name }; - for (const binding of bindings) { - // Skip this action if it's not a mutate action - if (!binding.mutateCallback) { - continue; - } + const bindables: Bindable[] = capabilities.flatMap(c => + c.bindings.map(b => ({ + name: c.name, + namespaces: c.namespaces, + binding: b, + actMeta: { ...reqMetadata, name: c.name }, + })), + ); + + for (const { name, namespaces, binding, actMeta } of bindables) { + // Skip this action if it's not a mutate action + if (!binding.mutateCallback) { + continue; + } - // Continue to the next action without doing anything if this one should be skipped - const shouldSkip = shouldSkipRequest(binding, req, namespaces, config?.alwaysIgnore?.namespaces); - if (shouldSkip !== "") { - Log.debug(shouldSkip); - continue; - } + // Continue to the next action without doing anything if this one should be skipped + const shouldSkip = shouldSkipRequest(binding, req, namespaces, config?.alwaysIgnore?.namespaces); + if (shouldSkip !== "") { + Log.debug(shouldSkip); + continue; + } - const label = binding.mutateCallback.name; - Log.info(actionMetadata, `Processing mutation action (${label})`); - matchedAction = true; + const label = binding.mutateCallback.name; + Log.info(actMeta, `Processing mutation action (${label})`); + matchedAction = true; - wrapped = updateStatus(config, name, wrapped, "started"); + wrapped = updateStatus(config, name, wrapped, "started"); - try { - // Run the action - await binding.mutateCallback(wrapped); + try { + // Run the action + await binding.mutateCallback(wrapped); - // Log on success - Log.info(actionMetadata, `Mutation action succeeded (${label})`); + // Log on success + Log.info(actMeta, `Mutation action succeeded (${label})`); - // Add annotations to the request to indicate that the capability succeeded - wrapped = updateStatus(config, name, wrapped, "succeeded"); - } catch (e) { - wrapped = updateStatus(config, name, wrapped, "warning"); - response.warnings = response.warnings || []; + // Add annotations to the request to indicate that the capability succeeded + wrapped = updateStatus(config, name, wrapped, "succeeded"); + } catch (e) { + wrapped = updateStatus(config, name, wrapped, "warning"); + response.warnings = response.warnings || []; - const errorMessage = logMutateErrorMessage(e); + const errorMessage = logMutateErrorMessage(e); - // Log on failure - Log.error(actionMetadata, `Action failed: ${errorMessage}`); - response.warnings.push(`Action failed: ${errorMessage}`); + // Log on failure + Log.error(actMeta, `Action failed: ${errorMessage}`); + response.warnings.push(`Action failed: ${errorMessage}`); - switch (config.onError) { - case Errors.reject: - response.result = "Pepr module configured to reject on error"; - return response; + switch (config.onError) { + case Errors.reject: + response.result = "Pepr module configured to reject on error"; + return response; - case Errors.audit: - response.auditAnnotations = response.auditAnnotations || {}; - response.auditAnnotations[Date.now()] = `Action failed: ${errorMessage}`; - break; - } + case Errors.audit: + response.auditAnnotations = response.auditAnnotations || {}; + response.auditAnnotations[Date.now()] = `Action failed: ${errorMessage}`; + break; } } } From 6ce79549b7dd50ee0c1c677787237311d6024117 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 10:32:51 -0600 Subject: [PATCH 08/20] wip: saving progress --- src/lib/mutate-processor.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 4163162c5..f4a718b54 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -77,7 +77,7 @@ export async function mutateProcessor( Log.info(reqMetadata, `Processing request`); - const bindables: Bindable[] = capabilities.flatMap(c => + let bindables: Bindable[] = capabilities.flatMap(c => c.bindings.map(b => ({ name: c.name, namespaces: c.namespaces, @@ -86,20 +86,22 @@ export async function mutateProcessor( })), ); - for (const { name, namespaces, binding, actMeta } of bindables) { - // Skip this action if it's not a mutate action - if (!binding.mutateCallback) { - continue; + bindables = bindables.filter(b => { + if (!b.binding.mutateCallback) { + return false; } - // Continue to the next action without doing anything if this one should be skipped - const shouldSkip = shouldSkipRequest(binding, req, namespaces, config?.alwaysIgnore?.namespaces); + const shouldSkip = shouldSkipRequest(b.binding, req, b.namespaces, config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); - continue; + return false; } - const label = binding.mutateCallback.name; + return true; + }); + + for (const { name, binding, actMeta } of bindables) { + const label = binding.mutateCallback!.name; Log.info(actMeta, `Processing mutation action (${label})`); matchedAction = true; @@ -107,7 +109,7 @@ export async function mutateProcessor( try { // Run the action - await binding.mutateCallback(wrapped); + await binding.mutateCallback!(wrapped); // Log on success Log.info(actMeta, `Mutation action succeeded (${label})`); From 529213660bd0ac48758c8d878c2e079c3153f451 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 12:12:32 -0600 Subject: [PATCH 09/20] wip: saving progress --- src/lib/mutate-processor.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index f4a718b54..e26cfdd0f 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -15,6 +15,8 @@ import { PeprMutateRequest } from "./mutate-request"; import { base64Encode, convertFromBase64Map, convertToBase64Map } from "./utils"; interface Bindable { + req: AdmissionRequest; + config: ModuleConfig; name: string; namespaces: string[]; binding: Binding; @@ -63,9 +65,6 @@ export async function mutateProcessor( allowed: false, }; - // Track whether any capability matched the request - let matchedAction = false; - // Track data fields that should be skipped during decoding let skipDecode: string[] = []; @@ -79,6 +78,8 @@ export async function mutateProcessor( let bindables: Bindable[] = capabilities.flatMap(c => c.bindings.map(b => ({ + req, + config, name: c.name, namespaces: c.namespaces, binding: b, @@ -91,7 +92,7 @@ export async function mutateProcessor( return false; } - const shouldSkip = shouldSkipRequest(b.binding, req, b.namespaces, config?.alwaysIgnore?.namespaces); + const shouldSkip = shouldSkipRequest(b.binding, b.req, b.namespaces, b.config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); return false; @@ -103,7 +104,6 @@ export async function mutateProcessor( for (const { name, binding, actMeta } of bindables) { const label = binding.mutateCallback!.name; Log.info(actMeta, `Processing mutation action (${label})`); - matchedAction = true; wrapped = updateStatus(config, name, wrapped, "started"); @@ -143,7 +143,7 @@ export async function mutateProcessor( response.allowed = true; // If no capability matched the request, exit early - if (!matchedAction) { + if (bindables.length === 0) { Log.info(reqMetadata, `No matching actions found`); return response; } From a07179d9cd623b3fb05251701ecde7cf8d02a2e1 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 13:48:05 -0600 Subject: [PATCH 10/20] wip: saving progress --- src/lib/mutate-processor.ts | 113 ++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 45 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index e26cfdd0f..4e2483ad5 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -52,6 +52,55 @@ export function logMutateErrorMessage(e: Error): string { } } +async function processRequest( + bindable: Bindable, + wrapped: PeprMutateRequest, + response: MutateResponse, +): Promise<{ + wrapped: PeprMutateRequest; + response: MutateResponse; +}> { + const { binding, actMeta, name, config } = bindable; + + const label = binding.mutateCallback!.name; + Log.info(actMeta, `Processing mutation action (${label})`); + + wrapped = updateStatus(config, name, wrapped, "started"); + + try { + // Run the action + await binding.mutateCallback!(wrapped); + + // Log on success + Log.info(actMeta, `Mutation action succeeded (${label})`); + + // Add annotations to the request to indicate that the capability succeeded + wrapped = updateStatus(config, name, wrapped, "succeeded"); + } catch (e) { + wrapped = updateStatus(config, name, wrapped, "warning"); + response.warnings = response.warnings || []; + + const errorMessage = logMutateErrorMessage(e); + + // Log on failure + Log.error(actMeta, `Action failed: ${errorMessage}`); + response.warnings.push(`Action failed: ${errorMessage}`); + + switch (config.onError) { + case Errors.reject: + response.result = "Pepr module configured to reject on error"; + break; + + case Errors.audit: + response.auditAnnotations = response.auditAnnotations || {}; + response.auditAnnotations[Date.now()] = `Action failed: ${errorMessage}`; + break; + } + } + + return { wrapped, response }; +} + export async function mutateProcessor( config: ModuleConfig, capabilities: Capability[], @@ -59,7 +108,7 @@ export async function mutateProcessor( reqMetadata: Record, ): Promise { let wrapped = new PeprMutateRequest(req); - const response: MutateResponse = { + let response: MutateResponse = { uid: req.uid, warnings: [], allowed: false, @@ -76,23 +125,28 @@ export async function mutateProcessor( Log.info(reqMetadata, `Processing request`); - let bindables: Bindable[] = capabilities.flatMap(c => - c.bindings.map(b => ({ + let bindables: Bindable[] = capabilities.flatMap(capa => + capa.bindings.map(bind => ({ req, config, - name: c.name, - namespaces: c.namespaces, - binding: b, - actMeta: { ...reqMetadata, name: c.name }, + name: capa.name, + namespaces: capa.namespaces, + binding: bind, + actMeta: { ...reqMetadata, name: capa.name }, })), ); - bindables = bindables.filter(b => { - if (!b.binding.mutateCallback) { + bindables = bindables.filter(bind => { + if (!bind.binding.mutateCallback) { return false; } - const shouldSkip = shouldSkipRequest(b.binding, b.req, b.namespaces, b.config?.alwaysIgnore?.namespaces); + const shouldSkip = shouldSkipRequest( + bind.binding, + bind.req, + bind.namespaces, + bind.config?.alwaysIgnore?.namespaces, + ); if (shouldSkip !== "") { Log.debug(shouldSkip); return false; @@ -101,41 +155,10 @@ export async function mutateProcessor( return true; }); - for (const { name, binding, actMeta } of bindables) { - const label = binding.mutateCallback!.name; - Log.info(actMeta, `Processing mutation action (${label})`); - - wrapped = updateStatus(config, name, wrapped, "started"); - - try { - // Run the action - await binding.mutateCallback!(wrapped); - - // Log on success - Log.info(actMeta, `Mutation action succeeded (${label})`); - - // Add annotations to the request to indicate that the capability succeeded - wrapped = updateStatus(config, name, wrapped, "succeeded"); - } catch (e) { - wrapped = updateStatus(config, name, wrapped, "warning"); - response.warnings = response.warnings || []; - - const errorMessage = logMutateErrorMessage(e); - - // Log on failure - Log.error(actMeta, `Action failed: ${errorMessage}`); - response.warnings.push(`Action failed: ${errorMessage}`); - - switch (config.onError) { - case Errors.reject: - response.result = "Pepr module configured to reject on error"; - return response; - - case Errors.audit: - response.auditAnnotations = response.auditAnnotations || {}; - response.auditAnnotations[Date.now()] = `Action failed: ${errorMessage}`; - break; - } + for (const bindable of bindables) { + ({ wrapped, response } = await processRequest(bindable, wrapped, response)); + if (config.onError === Errors.reject) { + return response; } } From ab1f19ced24a780b553633f8d9992e0f4f947908 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 14:40:57 -0600 Subject: [PATCH 11/20] wip: saving progress --- src/lib/mutate-processor.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 4e2483ad5..895651dbd 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -23,6 +23,11 @@ interface Bindable { actMeta: Record; } +interface Result { + wrapped: PeprMutateRequest; + response: MutateResponse; +} + // Add annotations to the request to indicate that the capability started processing // this will allow tracking of failed mutations that were permitted to continue export function updateStatus( @@ -56,10 +61,7 @@ async function processRequest( bindable: Bindable, wrapped: PeprMutateRequest, response: MutateResponse, -): Promise<{ - wrapped: PeprMutateRequest; - response: MutateResponse; -}> { +): Promise { const { binding, actMeta, name, config } = bindable; const label = binding.mutateCallback!.name; @@ -157,7 +159,7 @@ export async function mutateProcessor( for (const bindable of bindables) { ({ wrapped, response } = await processRequest(bindable, wrapped, response)); - if (config.onError === Errors.reject) { + if (config.onError === Errors.reject && response?.warnings!.length > 0) { return response; } } From d047c76a0c8fa313b3e763eb0a9ee52eb8f2d977 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 15:39:51 -0600 Subject: [PATCH 12/20] wip: saving progress --- src/lib/mutate-processor.ts | 42 ++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 895651dbd..918547b82 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -3,6 +3,7 @@ import jsonPatch from "fast-json-patch"; import { kind, KubernetesObject } from "kubernetes-fluent-client"; +import { clone } from "ramda"; import { Capability } from "./capability"; import { Errors } from "./errors"; @@ -103,6 +104,29 @@ async function processRequest( return { wrapped, response }; } +function skipDecode(wrapped: PeprMutateRequest): string[] { + let skipped: string[] = []; + + const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; + if (isSecret) { + skipped = convertFromBase64Map(wrapped.Raw as unknown as kind.Secret); + } + + return skipped; +} + +function unskipRecode(wrapped: PeprMutateRequest, skipped: string[]): KubernetesObject { + const transformed = clone(wrapped.Raw); + + const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; + if (isSecret && skipped.length > 0) { + convertToBase64Map(transformed as unknown as kind.Secret, skipped); + } + + return transformed; +} + +/* eslint max-statements: ["warn", 25] */ export async function mutateProcessor( config: ModuleConfig, capabilities: Capability[], @@ -116,14 +140,8 @@ export async function mutateProcessor( allowed: false, }; - // Track data fields that should be skipped during decoding - let skipDecode: string[] = []; - - // If the resource is a secret, decode the data - const isSecret = req.kind.version === "v1" && req.kind.kind === "Secret"; - if (isSecret) { - skipDecode = convertFromBase64Map(wrapped.Raw as unknown as kind.Secret); - } + // Track base64-encoded data fields that should be skipped during decoding + const skipped = skipDecode(wrapped); Log.info(reqMetadata, `Processing request`); @@ -178,12 +196,8 @@ export async function mutateProcessor( return response; } - const transformed = wrapped.Raw; - - // Post-process the Secret requests to convert it back to the original format - if (isSecret) { - convertToBase64Map(transformed as unknown as kind.Secret, skipDecode); - } + // unskip base64-encoded data fields that were skipDecode'd + const transformed = unskipRecode(wrapped, skipped); // Compare the original request to the modified request to get the patches const patches = jsonPatch.compare(req.object, transformed); From f4802f5c8eae8bc2d4f52c0bc29d2f1c6f1378eb Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 16:22:17 -0600 Subject: [PATCH 13/20] test: gettin' coverage --- src/lib/mutate-processor.test.ts | 164 +++++++++++-------------------- 1 file changed, 60 insertions(+), 104 deletions(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index 8edc50123..58edb52c6 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -2,114 +2,70 @@ // SPDX-FileCopyrightText: 2023-Present The Pepr Authors import { describe, expect, it } from "@jest/globals"; -// import { GroupVersionKind, kind, KubernetesObject } from "kubernetes-fluent-client"; -// import { AdmissionRequest, Binding, Filters } from "./types"; -// import { Event, Operation } from "./enums"; -// import { PeprValidateRequest } from "./validate-request"; -// import { clone } from "ramda"; -// import * as sut from "./mutate-processor"; +import { clone } from "ramda"; +import { ModuleConfig } from "./module"; +import { PeprMutateRequest } from "./mutate-request"; +import * as sut from "./mutate-processor"; +import { AdmissionRequest } from "./types"; +import { Operation } from "./enums"; + +const defaultModuleConfig: ModuleConfig = { + uuid: "test-uuid", + alwaysIgnore: {}, +}; + +const defaultAdmissionRequest: AdmissionRequest = { + uid: "uid", + kind: { + kind: "kind", + group: "group", + }, + resource: { + group: "group", + version: "version", + resource: "resource", + }, + name: "", + object: {}, + operation: Operation.CREATE, + userInfo: {}, +}; + +const defaultPeprMutateRequest = (admissionRequest = defaultAdmissionRequest) => + new PeprMutateRequest(admissionRequest); -// const testFilters: Filters = { -// annotations: {}, -// deletionTimestamp: false, -// labels: {}, -// name: "", -// namespaces: [], -// regexName: "^default$", -// regexNamespaces: [] as string[], -// }; - -// const testGroupVersionKind: GroupVersionKind = { -// kind: "some-kind", -// group: "some-group", -// }; - -// const testBinding: Binding = { -// event: Event.ANY, -// filters: testFilters, -// kind: testGroupVersionKind, -// model: kind.Pod, -// isFinalize: false, -// isMutate: false, -// isQueue: false, -// isValidate: false, -// isWatch: false, -// }; - -// export const testAdmissionRequest: AdmissionRequest = { -// uid: "some-uid", -// kind: { kind: "a-kind", group: "a-group" }, -// resource: { group: "some-group", version: "some-version", resource: "some-resource" }, -// operation: Operation.CONNECT, -// name: "some-name", -// userInfo: {}, -// object: {}, -// }; - -// export const testActionMetadata: Record = {}; +describe("updateStatus", () => { + describe("when given non-delete request", () => { + it("adds status annotation to to-be-admitted resource", () => { + const name = "capa"; + const status = "test-status"; + const annote = `${defaultModuleConfig.uuid}.pepr.dev/${name}`; -// export const testPeprValidateRequest = (admissionRequest: AdmissionRequest) => -// new PeprValidateRequest(admissionRequest); + const result = sut.updateStatus(defaultModuleConfig, name, defaultPeprMutateRequest(), status); -describe("updateStatus", () => { - it("does...", () => { - expect(true).toBe(true); + expect(result.HasAnnotation(annote)).toBe(true); + expect(result.Raw.metadata?.annotations?.[annote]).toBe(status); + }); }); -}); -describe("logMutateErrorMessage", () => { - it("does...", () => { - expect(true).toBe(true); + describe("when given delete request", () => { + it("does not add status annotation to to-be-admitted resource", () => { + const testAdmissionRequest = { + ...clone(defaultAdmissionRequest), + operation: Operation.DELETE, + oldObject: {}, + }; + const name = "capa"; + const annote = `${defaultModuleConfig.uuid}.pepr.dev/${name}`; + + const result = sut.updateStatus( + defaultModuleConfig, + name, + defaultPeprMutateRequest(testAdmissionRequest), + "test-status", + ); + + expect(result.HasAnnotation(annote)).toBe(false); + }); }); }); - -// describe("processRequest", () => { -// let binding: Binding; -// let actionMetadata: Record; -// let peprValidateRequest: PeprValidateRequest; - -// beforeEach(() => { -// binding = clone(testBinding); -// actionMetadata = clone(testActionMetadata); -// peprValidateRequest = testPeprValidateRequest(testAdmissionRequest); -// }); - -// it("responds on successful validation action", async () => { -// const cbResult = { -// allowed: true, -// statusCode: 200, -// statusMessage: "yay", -// }; -// const callback = jest.fn().mockImplementation(() => cbResult) as Binding["validateCallback"]; -// binding = { ...clone(testBinding), validateCallback: callback }; - -// const result = await sut.processRequest(binding, actionMetadata, peprValidateRequest); - -// expect(result).toEqual({ -// uid: peprValidateRequest.Request.uid, -// allowed: cbResult.allowed, -// status: { -// code: cbResult.statusCode, -// message: cbResult.statusMessage, -// }, -// }); -// }); - -// it("responds on unsuccessful validation action", async () => { -// const callback = jest.fn().mockImplementation(() => { -// throw "oof"; -// }) as Binding["validateCallback"]; -// binding = { ...clone(testBinding), validateCallback: callback }; - -// const result = await sut.processRequest(binding, actionMetadata, peprValidateRequest); - -// expect(result).toEqual({ -// uid: peprValidateRequest.Request.uid, -// allowed: false, -// status: { -// code: 500, -// message: `Action failed with error: "oof"`, -// }, -// }); -// }); -// }); From 4a409acb04ed2001ace2c50a0bb5664c07264bf4 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 16:40:53 -0600 Subject: [PATCH 14/20] test: gettin' coverage --- src/lib/mutate-processor.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index 58edb52c6..5ee7e8b54 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -69,3 +69,15 @@ describe("updateStatus", () => { }); }); }); + +describe("logMutateErrorMessage", () => { + it.each([ + // error msg, result string + ["oof", "oof"], + ["", "An error occurred with the mutate action."], + ["[object Object]", "An error occurred with the mutate action."], + ])("given error '%s', returns '%s'", (err, res) => { + const result = sut.logMutateErrorMessage(new Error(err)); + expect(result).toBe(res); + }); +}); From 2a839657888d001b4d896192537c9375299fa2bb Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 17:13:21 -0600 Subject: [PATCH 15/20] test: gettin' coverage --- src/lib/mutate-processor.test.ts | 53 +++++++++++++++++++++++++++++++- src/lib/mutate-processor.ts | 46 +++++++++++++-------------- 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index 5ee7e8b54..334ba827a 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -1,13 +1,17 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -import { describe, expect, it } from "@jest/globals"; +import { beforeEach, describe, expect, it, jest } from "@jest/globals"; import { clone } from "ramda"; import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; import * as sut from "./mutate-processor"; import { AdmissionRequest } from "./types"; import { Operation } from "./enums"; +import { convertFromBase64Map } from "./utils"; + +jest.mock("./utils"); +const mockConvertFromBase64Map = jest.mocked(convertFromBase64Map); const defaultModuleConfig: ModuleConfig = { uuid: "test-uuid", @@ -19,6 +23,7 @@ const defaultAdmissionRequest: AdmissionRequest = { kind: { kind: "kind", group: "group", + version: "version", }, resource: { group: "group", @@ -34,6 +39,10 @@ const defaultAdmissionRequest: AdmissionRequest = { const defaultPeprMutateRequest = (admissionRequest = defaultAdmissionRequest) => new PeprMutateRequest(admissionRequest); +beforeEach(() => { + jest.resetAllMocks(); +}); + describe("updateStatus", () => { describe("when given non-delete request", () => { it("adds status annotation to to-be-admitted resource", () => { @@ -81,3 +90,45 @@ describe("logMutateErrorMessage", () => { expect(result).toBe(res); }); }); + +describe("skipDecode", () => { + const skips = ["convert", "From", "Base64", "Maps"]; + + beforeEach(() => { + mockConvertFromBase64Map.mockImplementation(() => skips); + }); + + it("returns skipped content with given a Secret", () => { + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "Secret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const skipped = sut.skipDecode(testPeprMutateRequest); + + expect(mockConvertFromBase64Map.mock.calls.length).toBe(1); + expect(skipped).toBe(skips); + }); + + it("returns nothing when given a non-Secret", () => { + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "NotASecret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const skipped = sut.skipDecode(testPeprMutateRequest); + + expect(mockConvertFromBase64Map.mock.calls.length).toBe(0); + expect(skipped).toEqual([]); + }); +}); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 918547b82..d29d1b86f 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -58,7 +58,29 @@ export function logMutateErrorMessage(e: Error): string { } } -async function processRequest( +export function skipDecode(wrapped: PeprMutateRequest): string[] { + let skipped: string[] = []; + + const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; + if (isSecret) { + skipped = convertFromBase64Map(wrapped.Raw as unknown as kind.Secret); + } + + return skipped; +} + +export function unskipRecode(wrapped: PeprMutateRequest, skipped: string[]): KubernetesObject { + const transformed = clone(wrapped.Raw); + + const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; + if (isSecret && skipped.length > 0) { + convertToBase64Map(transformed as unknown as kind.Secret, skipped); + } + + return transformed; +} + +export async function processRequest( bindable: Bindable, wrapped: PeprMutateRequest, response: MutateResponse, @@ -104,28 +126,6 @@ async function processRequest( return { wrapped, response }; } -function skipDecode(wrapped: PeprMutateRequest): string[] { - let skipped: string[] = []; - - const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; - if (isSecret) { - skipped = convertFromBase64Map(wrapped.Raw as unknown as kind.Secret); - } - - return skipped; -} - -function unskipRecode(wrapped: PeprMutateRequest, skipped: string[]): KubernetesObject { - const transformed = clone(wrapped.Raw); - - const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; - if (isSecret && skipped.length > 0) { - convertToBase64Map(transformed as unknown as kind.Secret, skipped); - } - - return transformed; -} - /* eslint max-statements: ["warn", 25] */ export async function mutateProcessor( config: ModuleConfig, From 1e16c2d02bcc359a444544a526faf9a40d20f1b7 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Tue, 10 Dec 2024 17:43:21 -0600 Subject: [PATCH 16/20] test: gettin' coverage --- src/lib/mutate-processor.test.ts | 70 ++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index 334ba827a..fc534ba6d 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -8,10 +8,11 @@ import { PeprMutateRequest } from "./mutate-request"; import * as sut from "./mutate-processor"; import { AdmissionRequest } from "./types"; import { Operation } from "./enums"; -import { convertFromBase64Map } from "./utils"; +import { convertFromBase64Map, convertToBase64Map } from "./utils"; jest.mock("./utils"); const mockConvertFromBase64Map = jest.mocked(convertFromBase64Map); +const mockConvertToBase64Map = jest.mocked(convertToBase64Map); const defaultModuleConfig: ModuleConfig = { uuid: "test-uuid", @@ -31,7 +32,11 @@ const defaultAdmissionRequest: AdmissionRequest = { resource: "resource", }, name: "", - object: {}, + object: { + metadata: { + name: "create-me", + }, + }, operation: Operation.CREATE, userInfo: {}, }; @@ -92,7 +97,7 @@ describe("logMutateErrorMessage", () => { }); describe("skipDecode", () => { - const skips = ["convert", "From", "Base64", "Maps"]; + const skips = ["convert", "From", "Base64", "Map"]; beforeEach(() => { mockConvertFromBase64Map.mockImplementation(() => skips); @@ -112,6 +117,7 @@ describe("skipDecode", () => { const skipped = sut.skipDecode(testPeprMutateRequest); expect(mockConvertFromBase64Map.mock.calls.length).toBe(1); + expect(mockConvertFromBase64Map.mock.calls[0].at(0)).toBe(testPeprMutateRequest.Raw); expect(skipped).toBe(skips); }); @@ -132,3 +138,61 @@ describe("skipDecode", () => { expect(skipped).toEqual([]); }); }); + +describe("unskipRecode", () => { + it("returns unchanged content when given non-secret", () => { + const skipped = ["convert", "To", "Base64", "Map"]; + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "NotASecret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const transformed = sut.unskipRecode(testPeprMutateRequest, skipped); + + expect(mockConvertToBase64Map.mock.calls.length).toBe(0); + expect(transformed).toEqual(testAdmissionRequest.object); + }); + + it("returns unchanged content when given a secret but no skips", () => { + const skipped: string[] = []; + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "Secret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const transformed = sut.unskipRecode(testPeprMutateRequest, skipped); + + expect(mockConvertToBase64Map.mock.calls.length).toBe(0); + expect(transformed).toEqual(testAdmissionRequest.object); + }); + + it("returns modified content when given a secret and skips", () => { + const skipped = ["convert", "To", "Base64", "Map"]; + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "Secret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const transformed = sut.unskipRecode(testPeprMutateRequest, skipped); + + expect(mockConvertToBase64Map.mock.calls.length).toBe(1); + expect(mockConvertToBase64Map.mock.calls[0].at(0)).toEqual(testPeprMutateRequest.Raw); + expect(mockConvertToBase64Map.mock.calls[0].at(1)).toBe(skipped); + expect(transformed).toEqual(testPeprMutateRequest.Raw); + }); +}); From 7b6fb13eaf31bfba651f00d922c24f9c300728b3 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Wed, 11 Dec 2024 15:33:32 -0600 Subject: [PATCH 17/20] test: gettin' coverage --- src/lib/mutate-processor.test.ts | 116 ++++++++++++++++++++++++++++++- src/lib/mutate-processor.ts | 4 +- 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index fc534ba6d..6e9ce081a 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -6,9 +6,12 @@ import { clone } from "ramda"; import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; import * as sut from "./mutate-processor"; -import { AdmissionRequest } from "./types"; -import { Operation } from "./enums"; +import { AdmissionRequest, Binding, MutateAction } from "./types"; +import { Event, Operation } from "./enums"; import { convertFromBase64Map, convertToBase64Map } from "./utils"; +import { GenericClass, KubernetesObject } from "kubernetes-fluent-client"; +import { MutateResponse } from "./k8s"; +import { Errors } from "./errors"; jest.mock("./utils"); const mockConvertFromBase64Map = jest.mocked(convertFromBase64Map); @@ -196,3 +199,112 @@ describe("unskipRecode", () => { expect(transformed).toEqual(testPeprMutateRequest.Raw); }); }); + +const defaultBinding: Binding = { + event: Event.CREATE, + model: {} as GenericClass, + kind: { + kind: "kind", + group: "group", + version: "version", + }, + filters: { + annotations: {}, + deletionTimestamp: false, + labels: {}, + name: "", + namespaces: [], + regexName: "", + regexNamespaces: [], + }, + mutateCallback: jest.fn() as jest.Mocked>, +}; + +const defaultBindable: sut.Bindable = { + req: defaultAdmissionRequest, + config: defaultModuleConfig, + name: "test-name", + namespaces: [], + binding: defaultBinding, + actMeta: {}, +}; + +const defaultMutateResponse: MutateResponse = { + uid: "default-uid", + allowed: true, +}; + +describe("processRequest", () => { + it("adds a status annotation on success", async () => { + const testPeprMutateRequest = defaultPeprMutateRequest(); + const testMutateResponse = clone(defaultMutateResponse); + const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`; + + const result = await sut.processRequest(defaultBindable, testPeprMutateRequest, testMutateResponse); + + expect(result).toEqual({ wrapped: testPeprMutateRequest, response: testMutateResponse }); + expect(result.wrapped.Raw.metadata?.annotations).toBeDefined(); + expect(result.wrapped.Raw.metadata!.annotations![annote]).toBe("succeeded"); + + expect(result.response.warnings).toBeUndefined(); + expect(result.response.result).toBeUndefined(); + expect(result.response.auditAnnotations).toBeUndefined(); + }); + + it("adds a status annotation, warning, and result on failure when Errors.reject", async () => { + const mutateCallback = (jest.fn() as jest.Mocked>).mockImplementation( + () => { + throw "oof"; + }, + ); + const testBinding = { ...clone(defaultBinding), mutateCallback }; + const testBindable = { ...clone(defaultBindable), binding: testBinding }; + testBindable.config.onError = Errors.reject; + const testPeprMutateRequest = defaultPeprMutateRequest(); + const testMutateResponse = clone(defaultMutateResponse); + const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`; + + const result = await sut.processRequest(testBindable, testPeprMutateRequest, testMutateResponse); + + expect(result).toEqual({ wrapped: testPeprMutateRequest, response: testMutateResponse }); + expect(result.wrapped.Raw.metadata?.annotations).toBeDefined(); + expect(result.wrapped.Raw.metadata!.annotations![annote]).toBe("warning"); + + expect(result.response.warnings).toHaveLength(1); + expect(result.response.warnings![0]).toBe("Action failed: An error occurred with the mutate action."); + expect(result.response.result).toBe("Pepr module configured to reject on error"); + expect(result.response.auditAnnotations).toBeUndefined(); + }); + + it("adds a status annotation, warning, and auditAnnotation on failure when Errors.audit", async () => { + const mutateCallback = (jest.fn() as jest.Mocked>).mockImplementation( + () => { + throw "oof"; + }, + ); + const testBinding = { ...clone(defaultBinding), mutateCallback }; + const testBindable = { ...clone(defaultBindable), binding: testBinding }; + testBindable.config.onError = Errors.audit; + const testPeprMutateRequest = defaultPeprMutateRequest(); + const testMutateResponse = clone(defaultMutateResponse); + const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`; + + const result = await sut.processRequest(testBindable, testPeprMutateRequest, testMutateResponse); + + expect(result).toEqual({ wrapped: testPeprMutateRequest, response: testMutateResponse }); + expect(result.wrapped.Raw.metadata?.annotations).toBeDefined(); + expect(result.wrapped.Raw.metadata!.annotations![annote]).toBe("warning"); + + expect(result.response.warnings).toHaveLength(1); + expect(result.response.warnings![0]).toBe("Action failed: An error occurred with the mutate action."); + expect(result.response.result).toBeUndefined(); + expect(result.response.auditAnnotations).toBeDefined(); + + const auditAnnotes = Object.entries(result.response.auditAnnotations!); + expect(auditAnnotes).toHaveLength(1); + + const [key, val] = auditAnnotes[0]; + expect(Date.now() - parseInt(key)).toBeLessThan(5); + expect(val).toBe("Action failed: An error occurred with the mutate action."); + }); +}); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index d29d1b86f..708946ab9 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -15,7 +15,7 @@ import { ModuleConfig } from "./module"; import { PeprMutateRequest } from "./mutate-request"; import { base64Encode, convertFromBase64Map, convertToBase64Map } from "./utils"; -interface Bindable { +export interface Bindable { req: AdmissionRequest; config: ModuleConfig; name: string; @@ -24,7 +24,7 @@ interface Bindable { actMeta: Record; } -interface Result { +export interface Result { wrapped: PeprMutateRequest; response: MutateResponse; } From 0e1ad8c3211c711953fa466c7a29b4ee4d4f2076 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Thu, 12 Dec 2024 10:53:17 -0600 Subject: [PATCH 18/20] test: gettin' coverage --- src/lib/mutate-processor.test.ts | 36 +++++++++----------------------- src/lib/mutate-processor.ts | 19 ++++++++++------- 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index 6e9ce081a..d10010ad9 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -99,14 +99,14 @@ describe("logMutateErrorMessage", () => { }); }); -describe("skipDecode", () => { +describe("decodeData", () => { const skips = ["convert", "From", "Base64", "Map"]; beforeEach(() => { mockConvertFromBase64Map.mockImplementation(() => skips); }); - it("returns skipped content with given a Secret", () => { + it("returns skips if required & given a Secret", () => { const testAdmissionRequest = { ...defaultAdmissionRequest, kind: { @@ -117,14 +117,15 @@ describe("skipDecode", () => { }; const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - const skipped = sut.skipDecode(testPeprMutateRequest); + const [skipped, wrapped] = sut.decodeData(testPeprMutateRequest); expect(mockConvertFromBase64Map.mock.calls.length).toBe(1); expect(mockConvertFromBase64Map.mock.calls[0].at(0)).toBe(testPeprMutateRequest.Raw); expect(skipped).toBe(skips); + expect(wrapped).toBe(testPeprMutateRequest); }); - it("returns nothing when given a non-Secret", () => { + it("returns no skips when given a non-Secret", () => { const testAdmissionRequest = { ...defaultAdmissionRequest, kind: { @@ -135,14 +136,15 @@ describe("skipDecode", () => { }; const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - const skipped = sut.skipDecode(testPeprMutateRequest); + const [skipped, wrapped] = sut.decodeData(testPeprMutateRequest); expect(mockConvertFromBase64Map.mock.calls.length).toBe(0); expect(skipped).toEqual([]); + expect(wrapped).toBe(testPeprMutateRequest); }); }); -describe("unskipRecode", () => { +describe("reencodeData", () => { it("returns unchanged content when given non-secret", () => { const skipped = ["convert", "To", "Base64", "Map"]; const testAdmissionRequest = { @@ -155,25 +157,7 @@ describe("unskipRecode", () => { }; const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - const transformed = sut.unskipRecode(testPeprMutateRequest, skipped); - - expect(mockConvertToBase64Map.mock.calls.length).toBe(0); - expect(transformed).toEqual(testAdmissionRequest.object); - }); - - it("returns unchanged content when given a secret but no skips", () => { - const skipped: string[] = []; - const testAdmissionRequest = { - ...defaultAdmissionRequest, - kind: { - kind: "Secret", - version: "v1", - group: "", - }, - }; - const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - - const transformed = sut.unskipRecode(testPeprMutateRequest, skipped); + const transformed = sut.reencodeData(testPeprMutateRequest, skipped); expect(mockConvertToBase64Map.mock.calls.length).toBe(0); expect(transformed).toEqual(testAdmissionRequest.object); @@ -191,7 +175,7 @@ describe("unskipRecode", () => { }; const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - const transformed = sut.unskipRecode(testPeprMutateRequest, skipped); + const transformed = sut.reencodeData(testPeprMutateRequest, skipped); expect(mockConvertToBase64Map.mock.calls.length).toBe(1); expect(mockConvertToBase64Map.mock.calls[0].at(0)).toEqual(testPeprMutateRequest.Raw); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index 708946ab9..ac3f0a247 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -58,22 +58,26 @@ export function logMutateErrorMessage(e: Error): string { } } -export function skipDecode(wrapped: PeprMutateRequest): string[] { +export function decodeData( + wrapped: PeprMutateRequest, +): [string[], PeprMutateRequest] { let skipped: string[] = []; const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; if (isSecret) { + // convertFromBase64Map modifies it's arg rather than returing a mod'ed copy (ye olde side-effect special, blerg) skipped = convertFromBase64Map(wrapped.Raw as unknown as kind.Secret); } - return skipped; + return [skipped, wrapped]; } -export function unskipRecode(wrapped: PeprMutateRequest, skipped: string[]): KubernetesObject { +export function reencodeData(wrapped: PeprMutateRequest, skipped: string[]): KubernetesObject { const transformed = clone(wrapped.Raw); const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; - if (isSecret && skipped.length > 0) { + if (isSecret) { + // convertToBase64Map modifies it's arg rather than returing a mod'ed copy (ye olde side-effect special, blerg) convertToBase64Map(transformed as unknown as kind.Secret, skipped); } @@ -140,8 +144,9 @@ export async function mutateProcessor( allowed: false, }; - // Track base64-encoded data fields that should be skipped during decoding - const skipped = skipDecode(wrapped); + // track base64-encoded data fields that should be decoded before / reencoded after processing + let skipped: string[] = []; + [skipped, wrapped] = decodeData(wrapped); Log.info(reqMetadata, `Processing request`); @@ -197,7 +202,7 @@ export async function mutateProcessor( } // unskip base64-encoded data fields that were skipDecode'd - const transformed = unskipRecode(wrapped, skipped); + const transformed = reencodeData(wrapped, skipped); // Compare the original request to the modified request to get the patches const patches = jsonPatch.compare(req.object, transformed); From f9c1c0e7393c1ac4f361e130f2c73d80f023da4c Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Thu, 12 Dec 2024 11:21:22 -0600 Subject: [PATCH 19/20] test: gettin' e2es working --- src/lib/mutate-processor.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index ac3f0a247..ccf9fb77e 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -137,16 +137,14 @@ export async function mutateProcessor( req: AdmissionRequest, reqMetadata: Record, ): Promise { - let wrapped = new PeprMutateRequest(req); let response: MutateResponse = { uid: req.uid, warnings: [], allowed: false, }; - // track base64-encoded data fields that should be decoded before / reencoded after processing - let skipped: string[] = []; - [skipped, wrapped] = decodeData(wrapped); + let wrapped = new PeprMutateRequest(req); + const skipped = decodeData(wrapped); Log.info(reqMetadata, `Processing request`); From 748842646bb719599df72633fe2d69f01dabe515 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Thu, 12 Dec 2024 11:40:12 -0600 Subject: [PATCH 20/20] test: gettin' e2es working --- src/lib/mutate-processor.test.ts | 4 ++-- src/lib/mutate-processor.ts | 15 ++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/lib/mutate-processor.test.ts b/src/lib/mutate-processor.test.ts index d10010ad9..30645cced 100644 --- a/src/lib/mutate-processor.test.ts +++ b/src/lib/mutate-processor.test.ts @@ -117,7 +117,7 @@ describe("decodeData", () => { }; const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - const [skipped, wrapped] = sut.decodeData(testPeprMutateRequest); + const { skipped, wrapped } = sut.decodeData(testPeprMutateRequest); expect(mockConvertFromBase64Map.mock.calls.length).toBe(1); expect(mockConvertFromBase64Map.mock.calls[0].at(0)).toBe(testPeprMutateRequest.Raw); @@ -136,7 +136,7 @@ describe("decodeData", () => { }; const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - const [skipped, wrapped] = sut.decodeData(testPeprMutateRequest); + const { skipped, wrapped } = sut.decodeData(testPeprMutateRequest); expect(mockConvertFromBase64Map.mock.calls.length).toBe(0); expect(skipped).toEqual([]); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index ccf9fb77e..6176a6cd6 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -58,9 +58,10 @@ export function logMutateErrorMessage(e: Error): string { } } -export function decodeData( - wrapped: PeprMutateRequest, -): [string[], PeprMutateRequest] { +export function decodeData(wrapped: PeprMutateRequest): { + skipped: string[]; + wrapped: PeprMutateRequest; +} { let skipped: string[] = []; const isSecret = wrapped.Request.kind.version === "v1" && wrapped.Request.kind.kind === "Secret"; @@ -69,7 +70,7 @@ export function decodeData( skipped = convertFromBase64Map(wrapped.Raw as unknown as kind.Secret); } - return [skipped, wrapped]; + return { skipped, wrapped }; } export function reencodeData(wrapped: PeprMutateRequest, skipped: string[]): KubernetesObject { @@ -143,8 +144,8 @@ export async function mutateProcessor( allowed: false, }; - let wrapped = new PeprMutateRequest(req); - const skipped = decodeData(wrapped); + const decoded = decodeData(new PeprMutateRequest(req)); + let wrapped = decoded.wrapped; Log.info(reqMetadata, `Processing request`); @@ -200,7 +201,7 @@ export async function mutateProcessor( } // unskip base64-encoded data fields that were skipDecode'd - const transformed = reencodeData(wrapped, skipped); + const transformed = reencodeData(wrapped, decoded.skipped); // Compare the original request to the modified request to get the patches const patches = jsonPatch.compare(req.object, transformed);