diff --git a/src/lib/assets/defaultTestObjects.ts b/src/lib/assets/defaultTestObjects.ts index adec2f99..884c9a5e 100644 --- a/src/lib/assets/defaultTestObjects.ts +++ b/src/lib/assets/defaultTestObjects.ts @@ -1,8 +1,25 @@ -import { GenericClass } from "kubernetes-fluent-client"; +import { GenericClass, GroupVersionKind } from "kubernetes-fluent-client"; import { Event } from "../enums"; import { Binding, CapabilityExport } from "../types"; import { defaultFilters } from "../filter/adjudicators/defaultTestObjects"; import { V1PolicyRule as PolicyRule } from "@kubernetes/client-node"; +import { AdmissionRequest, GroupVersionResource } from "../types"; +import { Operation } from "../enums"; + +export const createMockAdmissionRequest = ( + kind: GroupVersionKind = { kind: "kind", group: "group", version: "version" }, + resource: GroupVersionResource = { group: "group", version: "version", resource: "resource" }, + object: { metadata: { name: string } } = { metadata: { name: "create-me" } }, + operation: Operation = Operation.CREATE, +): AdmissionRequest => ({ + uid: "uid", + kind, + resource, + name: "", + object, + operation, + userInfo: {}, +}); export const createMockRbacRule = ( apiGroups: string[] = ["pepr.dev"], diff --git a/src/lib/processors/decode-utils.test.ts b/src/lib/processors/decode-utils.test.ts new file mode 100644 index 00000000..6be065c2 --- /dev/null +++ b/src/lib/processors/decode-utils.test.ts @@ -0,0 +1,101 @@ +import { beforeEach, describe, expect, it, jest } from "@jest/globals"; +import { convertFromBase64Map, convertToBase64Map } from "../utils"; +import { PeprMutateRequest } from "../mutate-request"; +import { decodeData, reencodeData } from "./decode-utils"; +import { createMockAdmissionRequest } from "../assets/defaultTestObjects"; + +jest.mock("../utils"); + +const defaultAdmissionRequest = createMockAdmissionRequest(); + +const defaultPeprMutateRequest = (admissionRequest = defaultAdmissionRequest) => + new PeprMutateRequest(admissionRequest); + +const mockConvertToBase64Map = jest.mocked(convertToBase64Map); +const mockConvertFromBase64Map = jest.mocked(convertFromBase64Map); + +describe("decodeData", () => { + const skips = ["convert", "From", "Base64", "Map"]; + + beforeEach(() => { + mockConvertFromBase64Map.mockClear(); + mockConvertFromBase64Map.mockImplementation(() => skips); + }); + + it("returns skips if required & given a Secret", () => { + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "Secret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const { skipped, wrapped } = 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 no skips when given a non-Secret", () => { + const testAdmissionRequest = { + ...defaultAdmissionRequest, + kind: { + kind: "NotASecret", + version: "v1", + group: "", + }, + }; + const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); + + const { skipped, wrapped } = decodeData(testPeprMutateRequest); + + expect(mockConvertFromBase64Map.mock.calls.length).toBe(0); + expect(skipped).toEqual([]); + expect(wrapped).toBe(testPeprMutateRequest); + }); +}); + +describe("reencodeData", () => { + 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 = reencodeData(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 = reencodeData(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); + }); +}); diff --git a/src/lib/processors/decode-utils.ts b/src/lib/processors/decode-utils.ts new file mode 100644 index 00000000..54a6e8be --- /dev/null +++ b/src/lib/processors/decode-utils.ts @@ -0,0 +1,31 @@ +import { convertFromBase64Map, convertToBase64Map } from "../utils"; +import { kind, KubernetesObject } from "kubernetes-fluent-client"; +import { PeprMutateRequest } from "../mutate-request"; +import { clone } from "ramda"; + +export function decodeData(wrapped: PeprMutateRequest): { + skipped: string[]; + wrapped: 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, wrapped }; +} + +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) { + // 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); + } + + return transformed; +} diff --git a/src/lib/processors/mutate-processor.test.ts b/src/lib/processors/mutate-processor.test.ts index 06dc5e12..4967fe04 100644 --- a/src/lib/processors/mutate-processor.test.ts +++ b/src/lib/processors/mutate-processor.test.ts @@ -5,19 +5,52 @@ import { beforeEach, describe, expect, it, jest } from "@jest/globals"; import { clone } from "ramda"; import { ModuleConfig } from "../types"; import { PeprMutateRequest } from "../mutate-request"; -import * as sut from "./mutate-processor"; import { AdmissionRequest, Binding, MutateAction } from "../types"; import { Event, Operation } from "../enums"; -import { convertFromBase64Map, convertToBase64Map, base64Encode } from "../utils"; +import { base64Encode } from "../utils"; import { GenericClass, KubernetesObject } from "kubernetes-fluent-client"; import { MutateResponse } from "../k8s"; import { OnError } from "../../cli/init/enums"; -import { updateResponsePatchAndWarnings } from "./mutate-processor"; +import { + updateResponsePatchAndWarnings, + Bindable, + mutateProcessor, + updateStatus, + logMutateErrorMessage, + processRequest, +} from "./mutate-processor"; import { Operation as JSONPatchOperation } from "fast-json-patch"; +import { Capability } from "../core/capability"; +import { MeasureWebhookTimeout } from "../telemetry/webhookTimeouts"; +import { decodeData } from "./decode-utils"; + +jest.mock("./decode-utils", () => ({ + decodeData: jest.fn(), +})); + +jest.mock("../telemetry/logger", () => ({ + info: jest.fn(), + debug: jest.fn(), + error: jest.fn(), +})); + +jest.mock("../telemetry/metrics", () => ({ + metricsCollector: { + addCounter: jest.fn(), + incCounter: jest.fn(), + }, + MeasureWebhookTimeout: jest.fn(), +})); + +jest.mock("../telemetry/timeUtils", () => ({ + getNow: jest.fn(() => 1000), +})); + +jest.mock("../filter/filter", () => ({ + shouldSkipRequest: jest.fn(), +})); jest.mock("../utils"); -const mockConvertFromBase64Map = jest.mocked(convertFromBase64Map); -const mockConvertToBase64Map = jest.mocked(convertToBase64Map); const defaultModuleConfig: ModuleConfig = { uuid: "test-uuid", @@ -51,6 +84,10 @@ const defaultPeprMutateRequest = (admissionRequest = defaultAdmissionRequest) => beforeEach(() => { jest.resetAllMocks(); + (decodeData as jest.Mock).mockReturnValue({ + wrapped: { mockWrapped: true }, + skipped: [], + }); }); describe("updateStatus", () => { @@ -60,7 +97,7 @@ describe("updateStatus", () => { const status = "test-status"; const annote = `${defaultModuleConfig.uuid}.pepr.dev/${name}`; - const result = sut.updateStatus(defaultModuleConfig, name, defaultPeprMutateRequest(), status); + const result = updateStatus(defaultModuleConfig, name, defaultPeprMutateRequest(), status); expect(result.HasAnnotation(annote)).toBe(true); expect(result.Raw.metadata?.annotations?.[annote]).toBe(status); @@ -77,7 +114,7 @@ describe("updateStatus", () => { const name = "capa"; const annote = `${defaultModuleConfig.uuid}.pepr.dev/${name}`; - const result = sut.updateStatus( + const result = updateStatus( defaultModuleConfig, name, defaultPeprMutateRequest(testAdmissionRequest), @@ -96,96 +133,11 @@ describe("logMutateErrorMessage", () => { ["", "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)); + const result = logMutateErrorMessage(new Error(err)); expect(result).toBe(res); }); }); -describe("decodeData", () => { - const skips = ["convert", "From", "Base64", "Map"]; - - beforeEach(() => { - mockConvertFromBase64Map.mockImplementation(() => skips); - }); - - it("returns skips if required & given a Secret", () => { - const testAdmissionRequest = { - ...defaultAdmissionRequest, - kind: { - kind: "Secret", - version: "v1", - group: "", - }, - }; - const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - - 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 no skips when given a non-Secret", () => { - const testAdmissionRequest = { - ...defaultAdmissionRequest, - kind: { - kind: "NotASecret", - version: "v1", - group: "", - }, - }; - const testPeprMutateRequest = defaultPeprMutateRequest(testAdmissionRequest); - - const { skipped, wrapped } = sut.decodeData(testPeprMutateRequest); - - expect(mockConvertFromBase64Map.mock.calls.length).toBe(0); - expect(skipped).toEqual([]); - expect(wrapped).toBe(testPeprMutateRequest); - }); -}); - -describe("reencodeData", () => { - 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.reencodeData(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.reencodeData(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); - }); -}); - const defaultBinding: Binding = { event: Event.CREATE, model: {} as GenericClass, @@ -206,7 +158,7 @@ const defaultBinding: Binding = { mutateCallback: jest.fn() as jest.Mocked>, }; -const defaultBindable: sut.Bindable = { +const defaultBindable: Bindable = { req: defaultAdmissionRequest, config: defaultModuleConfig, name: "test-name", @@ -226,7 +178,7 @@ describe("processRequest", () => { const testMutateResponse = clone(defaultMutateResponse); const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`; - const result = await sut.processRequest(defaultBindable, testPeprMutateRequest, testMutateResponse); + const result = await processRequest(defaultBindable, testPeprMutateRequest, testMutateResponse); expect(result).toEqual({ wrapped: testPeprMutateRequest, response: testMutateResponse }); expect(result.wrapped.Raw.metadata?.annotations).toBeDefined(); @@ -250,7 +202,7 @@ describe("processRequest", () => { const testMutateResponse = clone(defaultMutateResponse); const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`; - const result = await sut.processRequest(testBindable, testPeprMutateRequest, testMutateResponse); + const result = await processRequest(testBindable, testPeprMutateRequest, testMutateResponse); expect(result).toEqual({ wrapped: testPeprMutateRequest, response: testMutateResponse }); expect(result.wrapped.Raw.metadata?.annotations).toBeDefined(); @@ -275,7 +227,7 @@ describe("processRequest", () => { const testMutateResponse = clone(defaultMutateResponse); const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`; - const result = await sut.processRequest(testBindable, testPeprMutateRequest, testMutateResponse); + const result = await processRequest(testBindable, testPeprMutateRequest, testMutateResponse); expect(result).toEqual({ wrapped: testPeprMutateRequest, response: testMutateResponse }); expect(result.wrapped.Raw.metadata?.annotations).toBeDefined(); @@ -312,3 +264,62 @@ describe("updateResponsePatchAndWarnings", () => { expect(localMutateResponse.warnings).not.toBeDefined(); }); }); + +describe("mutateProcessor", () => { + let config: ModuleConfig; + let capability: Capability; + beforeEach(() => { + jest.clearAllMocks(); + config = { + webhookTimeout: 11, + uuid: "some-uuid", + alwaysIgnore: {}, + }; + capability = new Capability({ + name: "test", + description: "test", + }); + (decodeData as jest.Mock).mockReturnValue({ + wrapped: { mockWrapped: true }, + skipped: [], + }); + }); + + it("should measure if a timeout occurred based on webhookTimeout", async () => { + const req = defaultAdmissionRequest; + const reqMetadata = {}; + + const spyStart = jest.spyOn(MeasureWebhookTimeout.prototype, "start"); + + await mutateProcessor(config, [capability], req, reqMetadata); + + expect(spyStart).toHaveBeenCalledWith(config.webhookTimeout); + spyStart.mockRestore(); + }); + + it("should stop the timer after processing", async () => { + const req = defaultAdmissionRequest; + const reqMetadata = {}; + + const spyStop = jest.spyOn(MeasureWebhookTimeout.prototype, "stop"); + + await mutateProcessor(config, [capability], req, reqMetadata); + + expect(spyStop).toHaveBeenCalled(); + spyStop.mockRestore(); + }); + + it("should call decodeData", async () => { + const req = defaultAdmissionRequest; + const reqMetadata = {}; + + (decodeData as jest.Mock).mockReturnValue({ + wrapped: { mockWrapped: true }, + skipped: [], + }); + + await mutateProcessor(config, [capability], req, reqMetadata); + + expect(decodeData).toHaveBeenCalled(); + }); +}); diff --git a/src/lib/processors/mutate-processor.ts b/src/lib/processors/mutate-processor.ts index 29a656f6..a35b55dc 100644 --- a/src/lib/processors/mutate-processor.ts +++ b/src/lib/processors/mutate-processor.ts @@ -2,8 +2,7 @@ // SPDX-FileCopyrightText: 2023-Present The Pepr Authors import jsonPatch from "fast-json-patch"; -import { kind, KubernetesObject } from "kubernetes-fluent-client"; -import { clone } from "ramda"; +import { KubernetesObject } from "kubernetes-fluent-client"; import { MeasureWebhookTimeout } from "../telemetry/webhookTimeouts"; import { Capability } from "../core/capability"; import { shouldSkipRequest } from "../filter/filter"; @@ -12,11 +11,13 @@ import { AdmissionRequest, Binding } from "../types"; import Log from "../telemetry/logger"; import { ModuleConfig } from "../types"; import { PeprMutateRequest } from "../mutate-request"; -import { base64Encode, convertFromBase64Map, convertToBase64Map } from "../utils"; +import { base64Encode } from "../utils"; import { OnError } from "../../cli/init/enums"; import { resolveIgnoreNamespaces } from "../assets/webhooks"; import { Operation } from "fast-json-patch"; import { WebhookType } from "../enums"; +import { decodeData, reencodeData } from "./decode-utils"; + export interface Bindable { req: AdmissionRequest; config: ModuleConfig; @@ -60,33 +61,6 @@ export function logMutateErrorMessage(e: Error): string { } } -export function decodeData(wrapped: PeprMutateRequest): { - skipped: string[]; - wrapped: 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, wrapped }; -} - -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) { - // 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); - } - - return transformed; -} - export async function processRequest( bindable: Bindable, wrapped: PeprMutateRequest,