From 15860a99238e467fc3743da993e506b4341d7279 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Fri, 22 Nov 2024 15:52:28 -0600 Subject: [PATCH 1/4] refactor: solve max-statements, complexity eslint warnings --- src/lib/controller/index.ts | 121 ++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/src/lib/controller/index.ts b/src/lib/controller/index.ts index 4f3660bf5..8302d39d7 100644 --- a/src/lib/controller/index.ts +++ b/src/lib/controller/index.ts @@ -18,6 +18,49 @@ import { ResponseItem, AdmissionRequest } from "../types"; if (!process.env.PEPR_NODE_WARNINGS) { process.removeAllListeners("warning"); } + +interface KubeAdmissionResponse { + apiVersion: string; + kind: string; + response: ValidateResponse[] | MutateResponse | ResponseItem; +} + +function karForMutate(mr: MutateResponse): KubeAdmissionResponse { + return { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: mr, + }; +} + +function karForValidate(ar: AdmissionRequest, vr: ValidateResponse[]): KubeAdmissionResponse { + const isAllowed = vr.filter(r => !r.allowed).length === 0; + + const resp: ValidateResponse = + vr.length === 0 + ? { + uid: ar.uid, + allowed: true, + status: { code: 200, message: "no in-scope validations -- allowed!" }, + } + : { + uid: vr[0].uid, + allowed: isAllowed, + status: { + code: isAllowed ? 200 : 422, + message: vr + .filter(rl => !rl.allowed) + .map(curr => curr.status?.message) + .join("; "), + }, + }; + return { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: resp, + }; +} + export class Controller { // Track whether the server is running #running = false; @@ -206,77 +249,37 @@ export class Controller { // Get the request from the body or create an empty request const request: AdmissionRequest = req.body?.request || ({} as AdmissionRequest); - // Run the before hook if it exists - this.#beforeHook && this.#beforeHook(request || {}); - - // Setup identifiers for logging - const name = request?.name ? `/${request.name}` : ""; - const namespace = request?.namespace || ""; - const gvk = request?.kind || { group: "", version: "", kind: "" }; - - const reqMetadata = { - uid: request.uid, - namespace, - name, + const { name, namespace, gvk } = { + name: request?.name ? `/${request.name}` : "", + namespace: request?.namespace || "", + gvk: request?.kind || { group: "", version: "", kind: "" }, }; + const reqMetadata = { uid: request.uid, namespace, name }; Log.info({ ...reqMetadata, gvk, operation: request.operation, admissionKind }, "Incoming request"); Log.debug({ ...reqMetadata, request }, "Incoming request body"); - // Process the request - let response: MutateResponse | ValidateResponse[]; + // Run the before hook if it exists + this.#beforeHook && this.#beforeHook(request || {}); - // Call mutate or validate based on the admission kind - if (admissionKind === "Mutate") { - response = await mutateProcessor(this.#config, this.#capabilities, request, reqMetadata); - } else { - response = await validateProcessor(this.#config, this.#capabilities, request, reqMetadata); - } + // Process the request + const response: MutateResponse | ValidateResponse[] = + admissionKind === "Mutate" + ? await mutateProcessor(this.#config, this.#capabilities, request, reqMetadata) + : await validateProcessor(this.#config, this.#capabilities, request, reqMetadata); // Run the after hook if it exists - const responseList: ValidateResponse[] | MutateResponse[] = Array.isArray(response) ? response : [response]; - responseList.map(res => { + [response].flat().map(res => { this.#afterHook && this.#afterHook(res); - // Log the response - Log.info({ ...reqMetadata, res }, "Check response"); }); - let kubeAdmissionResponse: ValidateResponse[] | MutateResponse | ResponseItem; - - if (admissionKind === "Mutate") { - kubeAdmissionResponse = response; - Log.debug({ ...reqMetadata, response }, "Outgoing response"); - res.send({ - apiVersion: "admission.k8s.io/v1", - kind: "AdmissionReview", - response: kubeAdmissionResponse, - }); - } else { - kubeAdmissionResponse = - responseList.length === 0 - ? { - uid: request.uid, - allowed: true, - status: { message: "no in-scope validations -- allowed!" }, - } - : { - uid: responseList[0].uid, - allowed: responseList.filter(r => !r.allowed).length === 0, - status: { - message: (responseList as ValidateResponse[]) - .filter(rl => !rl.allowed) - .map(curr => curr.status?.message) - .join("; "), - }, - }; - res.send({ - apiVersion: "admission.k8s.io/v1", - kind: "AdmissionReview", - response: kubeAdmissionResponse, - }); - } + const kar: KubeAdmissionResponse = + admissionKind === "Mutate" + ? karForMutate(response as MutateResponse) + : karForValidate(request, response as ValidateResponse[]); - Log.debug({ ...reqMetadata, kubeAdmissionResponse }, "Outgoing response"); + Log.debug({ ...reqMetadata, kar }, "Outgoing response"); + res.send(kar); this.#metricsCollector.observeEnd(startTime, admissionKind); } catch (err) { From ea8fddd7baa7ea47f8985741bce9bca5267d8b01 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Fri, 22 Nov 2024 18:22:38 -0600 Subject: [PATCH 2/4] fix: restore accidentaly deleted line --- src/lib/controller/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/controller/index.ts b/src/lib/controller/index.ts index 8302d39d7..7e13ecdc4 100644 --- a/src/lib/controller/index.ts +++ b/src/lib/controller/index.ts @@ -271,6 +271,7 @@ export class Controller { // Run the after hook if it exists [response].flat().map(res => { this.#afterHook && this.#afterHook(res); + Log.info({ ...reqMetadata, res }, "Check response"); }); const kar: KubeAdmissionResponse = From d995d9b9659fd93f38b99ed3e21e5432dbc83147 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Sun, 24 Nov 2024 21:01:22 -0600 Subject: [PATCH 3/4] wip: saving progress --- src/lib/controller/index.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/controller/index.ts b/src/lib/controller/index.ts index 7e13ecdc4..85f3d60f1 100644 --- a/src/lib/controller/index.ts +++ b/src/lib/controller/index.ts @@ -19,13 +19,13 @@ if (!process.env.PEPR_NODE_WARNINGS) { process.removeAllListeners("warning"); } -interface KubeAdmissionResponse { +interface KubeAdmissionReview { apiVersion: string; kind: string; response: ValidateResponse[] | MutateResponse | ResponseItem; } -function karForMutate(mr: MutateResponse): KubeAdmissionResponse { +function karForMutate(mr: MutateResponse): KubeAdmissionReview { return { apiVersion: "admission.k8s.io/v1", kind: "AdmissionReview", @@ -33,7 +33,7 @@ function karForMutate(mr: MutateResponse): KubeAdmissionResponse { }; } -function karForValidate(ar: AdmissionRequest, vr: ValidateResponse[]): KubeAdmissionResponse { +function karForValidate(ar: AdmissionRequest, vr: ValidateResponse[]): KubeAdmissionReview { const isAllowed = vr.filter(r => !r.allowed).length === 0; const resp: ValidateResponse = @@ -274,12 +274,12 @@ export class Controller { Log.info({ ...reqMetadata, res }, "Check response"); }); - const kar: KubeAdmissionResponse = + const kar: KubeAdmissionReview = admissionKind === "Mutate" ? karForMutate(response as MutateResponse) : karForValidate(request, response as ValidateResponse[]); - Log.debug({ ...reqMetadata, kar }, "Outgoing response"); + Log.debug({ ...reqMetadata, kubeAdmissionResponse: kar.response }, "Outgoing response"); res.send(kar); this.#metricsCollector.observeEnd(startTime, admissionKind); From 27242331e6183c9c457c182011518e8027689a25 Mon Sep 17 00:00:00 2001 From: Barrett LaFrance Date: Sun, 24 Nov 2024 22:08:43 -0600 Subject: [PATCH 4/4] test: add units for new helpers --- src/lib/controller/index.ts | 45 +---------- src/lib/controller/index.util.test.ts | 103 ++++++++++++++++++++++++++ src/lib/controller/index.util.ts | 47 ++++++++++++ 3 files changed, 152 insertions(+), 43 deletions(-) create mode 100644 src/lib/controller/index.util.test.ts create mode 100644 src/lib/controller/index.util.ts diff --git a/src/lib/controller/index.ts b/src/lib/controller/index.ts index 85f3d60f1..7d7594954 100644 --- a/src/lib/controller/index.ts +++ b/src/lib/controller/index.ts @@ -13,54 +13,13 @@ import { ModuleConfig, isWatchMode } from "../module"; import { mutateProcessor } from "../mutate-processor"; import { validateProcessor } from "../validate-processor"; import { StoreController } from "./store"; -import { ResponseItem, AdmissionRequest } from "../types"; +import { AdmissionRequest } from "../types"; +import { karForMutate, karForValidate, KubeAdmissionReview } from "./index.util"; if (!process.env.PEPR_NODE_WARNINGS) { process.removeAllListeners("warning"); } -interface KubeAdmissionReview { - apiVersion: string; - kind: string; - response: ValidateResponse[] | MutateResponse | ResponseItem; -} - -function karForMutate(mr: MutateResponse): KubeAdmissionReview { - return { - apiVersion: "admission.k8s.io/v1", - kind: "AdmissionReview", - response: mr, - }; -} - -function karForValidate(ar: AdmissionRequest, vr: ValidateResponse[]): KubeAdmissionReview { - const isAllowed = vr.filter(r => !r.allowed).length === 0; - - const resp: ValidateResponse = - vr.length === 0 - ? { - uid: ar.uid, - allowed: true, - status: { code: 200, message: "no in-scope validations -- allowed!" }, - } - : { - uid: vr[0].uid, - allowed: isAllowed, - status: { - code: isAllowed ? 200 : 422, - message: vr - .filter(rl => !rl.allowed) - .map(curr => curr.status?.message) - .join("; "), - }, - }; - return { - apiVersion: "admission.k8s.io/v1", - kind: "AdmissionReview", - response: resp, - }; -} - export class Controller { // Track whether the server is running #running = false; diff --git a/src/lib/controller/index.util.test.ts b/src/lib/controller/index.util.test.ts new file mode 100644 index 000000000..496b6ed19 --- /dev/null +++ b/src/lib/controller/index.util.test.ts @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2023-Present The Pepr Authors + +import { describe, it, expect } from "@jest/globals"; +import { MutateResponse, ValidateResponse } from "../k8s"; +import * as sut from "./index.util"; +import { AdmissionRequest } from "../types"; + +describe("karForMutate()", () => { + it("returns given MutateResponse wrapped in an KubeAdmissionReview", () => { + const mr: MutateResponse = { uid: "uid", allowed: true }; + const kar: sut.KubeAdmissionReview = { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: mr, + }; + const result = sut.karForMutate(mr); + expect(result).toEqual(kar); + }); +}); + +describe("karForValidate()", () => { + describe("given 0 ValidationResponse[]'s", () => { + it("returns KubeAdmissionReview with abbreviated success message", () => { + const ar = { uid: "uid" } as unknown as AdmissionRequest; + const vrs: ValidateResponse[] = []; + const resp = { + uid: ar.uid, + allowed: true, + status: { code: 200, message: "no in-scope validations -- allowed!" }, + }; + const kar: sut.KubeAdmissionReview = { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: resp, + }; + const result = sut.karForValidate(ar, vrs); + expect(result).toEqual(kar); + }); + }); + + describe("given 1-or-more ValidationResponse[]'s", () => { + it("returns KubeAdmissionReview with detailed success message", () => { + const ar = { uid: "uid" } as unknown as AdmissionRequest; + const vrs: ValidateResponse[] = [ + { + uid: "uid", + allowed: true, + status: { + code: 200, + message: "msg", + }, + }, + ]; + const resp = { + uid: ar.uid, + allowed: true, + status: { code: 200, message: "" }, + }; + const kar: sut.KubeAdmissionReview = { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: resp, + }; + const result = sut.karForValidate(ar, vrs); + expect(result).toEqual(kar); + }); + + it("returns KubeAdmissionReview with detailed failure message", () => { + const ar = { uid: "uid" } as unknown as AdmissionRequest; + const vrs: ValidateResponse[] = [ + { + uid: "uid", + allowed: false, + status: { + code: 422, + message: "mess", + }, + }, + { + uid: "uid", + allowed: false, + status: { + code: 422, + message: "age", + }, + }, + ]; + const resp = { + uid: ar.uid, + allowed: false, + status: { code: 422, message: "mess; age" }, + }; + const kar: sut.KubeAdmissionReview = { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: resp, + }; + const result = sut.karForValidate(ar, vrs); + expect(result).toEqual(kar); + }); + }); +}); diff --git a/src/lib/controller/index.util.ts b/src/lib/controller/index.util.ts new file mode 100644 index 000000000..cad73983a --- /dev/null +++ b/src/lib/controller/index.util.ts @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2023-Present The Pepr Authors + +import { MutateResponse, ValidateResponse } from "../k8s"; +import { ResponseItem, AdmissionRequest } from "../types"; + +export interface KubeAdmissionReview { + apiVersion: string; + kind: string; + response: ValidateResponse[] | MutateResponse | ResponseItem; +} + +export function karForMutate(mr: MutateResponse): KubeAdmissionReview { + return { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: mr, + }; +} + +export function karForValidate(ar: AdmissionRequest, vr: ValidateResponse[]): KubeAdmissionReview { + const isAllowed = vr.filter(r => !r.allowed).length === 0; + + const resp: ValidateResponse = + vr.length === 0 + ? { + uid: ar.uid, + allowed: true, + status: { code: 200, message: "no in-scope validations -- allowed!" }, + } + : { + uid: vr[0].uid, + allowed: isAllowed, + status: { + code: isAllowed ? 200 : 422, + message: vr + .filter(rl => !rl.allowed) + .map(curr => curr.status?.message) + .join("; "), + }, + }; + return { + apiVersion: "admission.k8s.io/v1", + kind: "AdmissionReview", + response: resp, + }; +}