From ad34c61cb9fadd856110950133669ca9b70bc5de Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 11:54:02 -0500 Subject: [PATCH 01/49] Sort imports --- src/lib/filter.ts | 167 ++++++++++++++++++---------------------------- 1 file changed, 64 insertions(+), 103 deletions(-) diff --git a/src/lib/filter.ts b/src/lib/filter.ts index aa21bc1de..749a10513 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -1,39 +1,40 @@ +/* eslint-disable complexity */ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors import { AdmissionRequest, Binding, Operation } from "./types"; import { - carriesIgnoredNamespace, + carriedAnnotations, + carriedLabels, carriedName, - definedEvent, - declaredOperation, - definedName, - definedGroup, + carriedNamespace, + carriesIgnoredNamespace, declaredGroup, - definedVersion, + declaredKind, + declaredOperation, declaredVersion, + definedAnnotations, + definedEvent, + definedGroup, definedKind, - declaredKind, - definedNamespaces, - carriedNamespace, definedLabels, - carriedLabels, - definedAnnotations, - carriedAnnotations, - definedNamespaceRegexes, + definedName, definedNameRegex, + definedNamespaceRegexes, + definedNamespaces, + definedVersion, misboundDeleteWithDeletionTimestamp, - mismatchedDeletionTimestamp, mismatchedAnnotations, + mismatchedDeletionTimestamp, + mismatchedEvent, + mismatchedGroup, + mismatchedKind, mismatchedLabels, mismatchedName, mismatchedNameRegex, mismatchedNamespace, mismatchedNamespaceRegex, - mismatchedEvent, - mismatchedGroup, mismatchedVersion, - mismatchedKind, unbindableNamespaces, uncarryableNamespace, } from "./adjudicators"; @@ -54,90 +55,50 @@ export function shouldSkipRequest( const prefix = "Ignoring Admission Callback:"; const obj = req.operation === Operation.DELETE ? req.oldObject : req.object; - // prettier-ignore - return ( - misboundDeleteWithDeletionTimestamp(binding) ? - `${prefix} Cannot use deletionTimestamp filter on a DELETE operation.` : - - mismatchedDeletionTimestamp(binding, obj) ? - `${prefix} Binding defines deletionTimestamp but Object does not carry it.` : - - mismatchedEvent(binding, req) ? - ( - `${prefix} Binding defines event '${definedEvent(binding)}' but ` + - `Request declares '${declaredOperation(req)}'.` - ) : - - mismatchedName(binding, obj) ? - `${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(obj)}'.` : - - mismatchedGroup(binding, req) ? - ( - `${prefix} Binding defines group '${definedGroup(binding)}' but ` + - `Request declares '${declaredGroup(req)}'.` - ) : - - mismatchedVersion(binding, req) ? - ( - `${prefix} Binding defines version '${definedVersion(binding)}' but ` + - `Request declares '${declaredVersion(req)}'.` - ) : - - mismatchedKind(binding, req) ? - ( - `${prefix} Binding defines kind '${definedKind(binding)}' but ` + - `Request declares '${declaredKind(req)}'.` - ) : - - unbindableNamespaces(capabilityNamespaces, binding) ? - ( - `${prefix} Binding defines namespaces ${JSON.stringify(definedNamespaces(binding))} ` + - `but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.` - ) : - - uncarryableNamespace(capabilityNamespaces, obj) ? - ( - `${prefix} Object carries namespace '${carriedNamespace(obj)}' ` + - `but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.` - ) : - - mismatchedNamespace(binding, obj) ? - ( - `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' ` + - `but Object carries '${carriedNamespace(obj)}'.` - ) : - - mismatchedLabels(binding, obj) ? - ( - `${prefix} Binding defines labels '${JSON.stringify(definedLabels(binding))}' ` + - `but Object carries '${JSON.stringify(carriedLabels(obj))}'.` - ) : - - mismatchedAnnotations(binding, obj) ? - ( - `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' ` + - `but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` - ) : - - mismatchedNamespaceRegex(binding, obj) ? - ( - `${prefix} Binding defines namespace regexes ` + - `'${JSON.stringify(definedNamespaceRegexes(binding))}' ` + - `but Object carries '${carriedNamespace(obj)}'.` - ) : - - mismatchedNameRegex(binding, obj) ? - ( - `${prefix} Binding defines name regex '${definedNameRegex(binding)}' ` + - `but Object carries '${carriedName(obj)}'.` - ) : - - carriesIgnoredNamespace(ignoredNamespaces, obj) ? - ( - `${prefix} Object carries namespace '${carriedNamespace(obj)}' ` + - `but ignored namespaces include '${JSON.stringify(ignoredNamespaces)}'.` - ) : - - "" - ); + const admissionFilterMessage = misboundDeleteWithDeletionTimestamp(binding) + ? `${prefix} Cannot use deletionTimestamp filter on a DELETE operation.` + : mismatchedDeletionTimestamp(binding, obj) + ? `${prefix} Binding defines deletionTimestamp but Object does not carry it.` + : mismatchedEvent(binding, req) + ? `${prefix} Binding defines event '${definedEvent(binding)}' but ` + + `Request declares '${declaredOperation(req)}'.` + : mismatchedName(binding, obj) + ? `${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(obj)}'.` + : mismatchedGroup(binding, req) + ? `${prefix} Binding defines group '${definedGroup(binding)}' but ` + + `Request declares '${declaredGroup(req)}'.` + : mismatchedVersion(binding, req) + ? `${prefix} Binding defines version '${definedVersion(binding)}' but ` + + `Request declares '${declaredVersion(req)}'.` + : mismatchedKind(binding, req) + ? `${prefix} Binding defines kind '${definedKind(binding)}' but ` + + `Request declares '${declaredKind(req)}'.` + : unbindableNamespaces(capabilityNamespaces, binding) + ? `${prefix} Binding defines namespaces ${JSON.stringify(definedNamespaces(binding))} ` + + `but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.` + : uncarryableNamespace(capabilityNamespaces, obj) + ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' ` + + `but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.` + : mismatchedNamespace(binding, obj) + ? `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' ` + + `but Object carries '${carriedNamespace(obj)}'.` + : mismatchedLabels(binding, obj) + ? `${prefix} Binding defines labels '${JSON.stringify(definedLabels(binding))}' ` + + `but Object carries '${JSON.stringify(carriedLabels(obj))}'.` + : mismatchedAnnotations(binding, obj) + ? `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' ` + + `but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` + : mismatchedNamespaceRegex(binding, obj) + ? `${prefix} Binding defines namespace regexes ` + + `'${JSON.stringify(definedNamespaceRegexes(binding))}' ` + + `but Object carries '${carriedNamespace(obj)}'.` + : mismatchedNameRegex(binding, obj) + ? `${prefix} Binding defines name regex '${definedNameRegex(binding)}' ` + + `but Object carries '${carriedName(obj)}'.` + : carriesIgnoredNamespace(ignoredNamespaces, obj) + ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' ` + + `but ignored namespaces include '${JSON.stringify(ignoredNamespaces)}'.` + : ""; + + return admissionFilterMessage; } From 04bc01a5150ff93a60b0158200b8acdb73b7f956 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 14:01:08 -0500 Subject: [PATCH 02/49] Create wrapper to translate adjudicators into filters --- src/lib/adjudicatorsFilterWrapper.ts | 14 + src/lib/filter.ts | 38 ++ src/lib/filterChain.test.ts | 713 +++++++++++++++++++++++++++ 3 files changed, 765 insertions(+) create mode 100644 src/lib/adjudicatorsFilterWrapper.ts create mode 100644 src/lib/filterChain.test.ts diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts new file mode 100644 index 000000000..1216bfc8e --- /dev/null +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -0,0 +1,14 @@ +import { carriedName, definedNameRegex, mismatchedNameRegex } from "./adjudicators"; +import { AdmissionRequest, Binding } from "./types"; + +//TODO: Dupe'd declaration +type FilterParams = { binding: Binding; request: AdmissionRequest }; +const prefix = "Ignoring Admission Callback:"; + +export const mismatchedNameRegexFilter = (data: FilterParams): string => { + const result = mismatchedNameRegex(data.binding, data.request.operation) + ? `${prefix} Binding defines name regex '${definedNameRegex(data.binding)}' ` + + `but Object carries '${carriedName(data.request.operation)}'.` + : ""; + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index 749a10513..f7c005cf2 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -38,6 +38,7 @@ import { unbindableNamespaces, uncarryableNamespace, } from "./adjudicators"; +import { mismatchedNameRegexFilter } from "./adjudicatorsFilterWrapper"; /** * shouldSkipRequest determines if a request should be skipped based on the binding filters. @@ -102,3 +103,40 @@ export function shouldSkipRequest( return admissionFilterMessage; } + +//TODO: Dupe'd declaration +type FilterParams = { binding: Binding; request: AdmissionRequest }; +interface Filter { + (data: FilterParams): string; +} + +export class FilterChain { + private filters: Filter[] = []; + + public addFilter(filter: Filter): FilterChain { + this.filters.push(filter); + return this; + } + public execute(data: FilterParams): string { + return this.filters.reduce((result, filter) => { + // The result of each filter is passed as a new concatenated string + return filter(data); + }, ""); + } +} + +export const shouldSkipRequestWithFilterChain = ( + binding: Binding, + req: AdmissionRequest, + // capabilityNamespaces: string[], + // ignoredNamespaces?: string[], +): string => { + // const obj = req.operation === Operation.DELETE ? req.oldObject : req.object; + + const filterChain = new FilterChain(); + + filterChain.addFilter(mismatchedNameRegexFilter); + + const admissionFilterMessage = filterChain.execute({ binding, request: req }); + return admissionFilterMessage; +}; diff --git a/src/lib/filterChain.test.ts b/src/lib/filterChain.test.ts new file mode 100644 index 000000000..04ddd56da --- /dev/null +++ b/src/lib/filterChain.test.ts @@ -0,0 +1,713 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2023-Present The Pepr Authors + +import { expect, test, describe } from "@jest/globals"; +import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; +import * as fc from "fast-check"; +import { CreatePod, DeletePod } from "../fixtures/loader"; +import { shouldSkipRequest, shouldSkipRequestWithFilterChain } from "./filter"; +import { AdmissionRequest, Binding, Event } from "./types"; + +export const callback = () => undefined; + +export const podKind = modelToGroupVersionKind(kind.Pod.name); + +describe("Fuzzing shouldSkipRequest", () => { + test("should handle random inputs without crashing", () => { + fc.assert( + fc.property( + fc.record({ + event: fc.constantFrom("CREATE", "UPDATE", "DELETE", "ANY"), + kind: fc.record({ + group: fc.string(), + version: fc.string(), + kind: fc.string(), + }), + filters: fc.record({ + name: fc.string(), + namespaces: fc.array(fc.string()), + labels: fc.dictionary(fc.string(), fc.string()), + annotations: fc.dictionary(fc.string(), fc.string()), + deletionTimestamp: fc.boolean(), + }), + }), + fc.record({ + operation: fc.string(), + uid: fc.string(), + name: fc.string(), + namespace: fc.string(), + kind: fc.record({ + group: fc.string(), + version: fc.string(), + kind: fc.string(), + }), + object: fc.record({ + metadata: fc.record({ + deletionTimestamp: fc.option(fc.date()), + }), + }), + }), + fc.array(fc.string()), + (binding, req, capabilityNamespaces) => { + expect(() => + shouldSkipRequest(binding as Binding, req as AdmissionRequest, capabilityNamespaces), + ).not.toThrow(); + }, + ), + { numRuns: 100 }, + ); + }); +}); + +describe("Property-Based Testing shouldSkipRequest", () => { + test("should only skip requests that do not match the binding criteria", () => { + fc.assert( + fc.property( + fc.record({ + event: fc.constantFrom("CREATE", "UPDATE", "DELETE", "ANY"), + kind: fc.record({ + group: fc.string(), + version: fc.string(), + kind: fc.string(), + }), + filters: fc.record({ + name: fc.string(), + namespaces: fc.array(fc.string()), + labels: fc.dictionary(fc.string(), fc.string()), + annotations: fc.dictionary(fc.string(), fc.string()), + deletionTimestamp: fc.boolean(), + }), + }), + fc.record({ + operation: fc.string(), + uid: fc.string(), + name: fc.string(), + namespace: fc.string(), + kind: fc.record({ + group: fc.string(), + version: fc.string(), + kind: fc.string(), + }), + object: fc.record({ + metadata: fc.record({ + deletionTimestamp: fc.option(fc.date()), + }), + }), + }), + fc.array(fc.string()), + (binding, req, capabilityNamespaces) => { + const shouldSkip = shouldSkipRequest(binding as Binding, req as AdmissionRequest, capabilityNamespaces); + expect(typeof shouldSkip).toBe("string"); + }, + ), + { numRuns: 100 }, + ); + }); +}); + +test("create: should reject when regex name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + ); +}); + +test("create: should not reject when regex name does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("delete: should reject when regex name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + ); +}); + +test("delete: should not reject when regex name does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("create: should not reject when regex namespace does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^helm"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("create: should reject when regex namespace does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^argo"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, + ); +}); + +test("delete: should reject when regex namespace does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^argo"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, + ); +}); + +test("delete: should not reject when regex namespace does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^helm"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("delete: should reject when name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "bleh", + namespaces: [], + regexNamespaces: [], + regexName: "^not-cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name '.*' but Object carries '.*'./, + ); +}); + +test("should reject when kind does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: { + group: "", + version: "v1", + kind: "Nope", + }, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines kind '.*' but Request declares '.*'./, + ); +}); + +test("should reject when group does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: { + group: "Nope", + version: "v1", + kind: "Pod", + }, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines group '.*' but Request declares '.*'./, + ); +}); + +test("should reject when version does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: { + group: "", + version: "Nope", + kind: "Pod", + }, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines version '.*' but Request declares '.*'./, + ); +}); + +test("should allow when group, version, and kind match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should allow when kind match and others are empty", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: { + group: "", + version: "", + kind: "Pod", + }, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should reject when the capability namespace does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, ["bleh", "bleh2"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but namespaces allowed by Capability are '.*'./, + ); +}); + +test("should reject when namespace does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: ["bleh"], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines namespaces '.*' but Object carries '.*'./, + ); +}); + +test("should allow when namespace is match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: ["helm-releasename", "unicorn", "things"], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should reject when label does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: { + foo: "bar", + }, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines labels '.*' but Object carries '.*'./, + ); +}); + +test("should allow when label is match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + deletionTimestamp: false, + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: { + foo: "bar", + test: "test1", + }, + annotations: {}, + }, + callback, + }; + + const pod = CreatePod(); + pod.object.metadata = pod.object.metadata || {}; + pod.object.metadata.labels = { + foo: "bar", + test: "test1", + test2: "test2", + }; + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should reject when annotation does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: { + foo: "bar", + }, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + const pod = CreatePod(); + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines annotations '.*' but Object carries '.*'./, + ); +}); + +test("should allow when annotation is match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: {}, + annotations: { + foo: "bar", + test: "test1", + }, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }, + callback, + }; + + const pod = CreatePod(); + pod.object.metadata = pod.object.metadata || {}; + pod.object.metadata.annotations = { + foo: "bar", + test: "test1", + test2: "test2", + }; + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should use `oldObject` when the operation is `DELETE`", () => { + const binding = { + model: kind.Pod, + event: Event.Delete, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + deletionTimestamp: false, + labels: { + "test-op": "delete", + }, + annotations: {}, + }, + callback, + }; + + const pod = DeletePod(); + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should allow when deletionTimestamp is present on pod", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: {}, + regexNamespaces: [], + regexName: "", + annotations: { + foo: "bar", + test: "test1", + }, + deletionTimestamp: true, + }, + callback, + }; + + const pod = CreatePod(); + pod.object.metadata = pod.object.metadata || {}; + pod.object.metadata!.deletionTimestamp = new Date("2021-09-01T00:00:00Z"); + pod.object.metadata.annotations = { + foo: "bar", + test: "test1", + test2: "test2", + }; + + expect(shouldSkipRequest(binding, pod, [])).toBe(""); +}); + +test("should reject when deletionTimestamp is not present on pod", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + labels: {}, + regexNamespaces: [], + regexName: "", + annotations: { + foo: "bar", + test: "test1", + }, + deletionTimestamp: true, + }, + callback, + }; + + const pod = CreatePod(); + pod.object.metadata = pod.object.metadata || {}; + pod.object.metadata.annotations = { + foo: "bar", + test: "test1", + test2: "test2", + }; + + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, + ); +}); + +test.only("filterchain: should reject when regex name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + ); +}); From 4fb04809a5d16593fe597fb64142701f58ee2dcb Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 14:06:34 -0500 Subject: [PATCH 03/49] Use filterchain function in tests --- src/lib/filterChain.test.ts | 80 ++++++++++++++----------------------- 1 file changed, 31 insertions(+), 49 deletions(-) diff --git a/src/lib/filterChain.test.ts b/src/lib/filterChain.test.ts index 04ddd56da..13f20bc66 100644 --- a/src/lib/filterChain.test.ts +++ b/src/lib/filterChain.test.ts @@ -5,7 +5,7 @@ import { expect, test, describe } from "@jest/globals"; import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; import * as fc from "fast-check"; import { CreatePod, DeletePod } from "../fixtures/loader"; -import { shouldSkipRequest, shouldSkipRequestWithFilterChain } from "./filter"; +import { shouldSkipRequestWithFilterChain } from "./filter"; import { AdmissionRequest, Binding, Event } from "./types"; export const callback = () => undefined; @@ -50,7 +50,7 @@ describe("Fuzzing shouldSkipRequest", () => { fc.array(fc.string()), (binding, req, capabilityNamespaces) => { expect(() => - shouldSkipRequest(binding as Binding, req as AdmissionRequest, capabilityNamespaces), + shouldSkipRequestWithFilterChain(binding as Binding, req as AdmissionRequest, capabilityNamespaces), ).not.toThrow(); }, ), @@ -96,7 +96,11 @@ describe("Property-Based Testing shouldSkipRequest", () => { }), fc.array(fc.string()), (binding, req, capabilityNamespaces) => { - const shouldSkip = shouldSkipRequest(binding as Binding, req as AdmissionRequest, capabilityNamespaces); + const shouldSkip = shouldSkipRequestWithFilterChain( + binding as Binding, + req as AdmissionRequest, + capabilityNamespaces, + ); expect(typeof shouldSkip).toBe("string"); }, ), @@ -122,7 +126,7 @@ test("create: should reject when regex name does not match", () => { callback, }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, ); }); @@ -144,7 +148,7 @@ test("create: should not reject when regex name does match", () => { callback, }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("delete: should reject when regex name does not match", () => { @@ -164,7 +168,7 @@ test("delete: should reject when regex name does not match", () => { callback, }; const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, ); }); @@ -186,7 +190,7 @@ test("delete: should not reject when regex name does match", () => { callback, }; const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("create: should not reject when regex namespace does match", () => { @@ -206,7 +210,7 @@ test("create: should not reject when regex namespace does match", () => { callback, }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("create: should reject when regex namespace does not match", () => { @@ -226,7 +230,7 @@ test("create: should reject when regex namespace does not match", () => { callback, }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, ); }); @@ -248,7 +252,7 @@ test("delete: should reject when regex namespace does not match", () => { callback, }; const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, ); }); @@ -270,7 +274,7 @@ test("delete: should not reject when regex namespace does match", () => { callback, }; const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("delete: should reject when name does not match", () => { @@ -290,7 +294,7 @@ test("delete: should reject when name does not match", () => { callback, }; const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines name '.*' but Object carries '.*'./, ); }); @@ -317,7 +321,7 @@ test("should reject when kind does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines kind '.*' but Request declares '.*'./, ); }); @@ -344,7 +348,7 @@ test("should reject when group does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines group '.*' but Request declares '.*'./, ); }); @@ -371,7 +375,7 @@ test("should reject when version does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines version '.*' but Request declares '.*'./, ); }); @@ -394,7 +398,7 @@ test("should allow when group, version, and kind match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should allow when kind match and others are empty", () => { @@ -419,7 +423,7 @@ test("should allow when kind match and others are empty", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should reject when the capability namespace does not match", () => { @@ -440,7 +444,7 @@ test("should reject when the capability namespace does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, ["bleh", "bleh2"])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, ["bleh", "bleh2"])).toMatch( /Ignoring Admission Callback: Object carries namespace '.*' but namespaces allowed by Capability are '.*'./, ); }); @@ -463,7 +467,7 @@ test("should reject when namespace does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines namespaces '.*' but Object carries '.*'./, ); }); @@ -486,7 +490,7 @@ test("should allow when namespace is match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should reject when label does not match", () => { @@ -509,7 +513,7 @@ test("should reject when label does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines labels '.*' but Object carries '.*'./, ); }); @@ -542,7 +546,7 @@ test("should allow when label is match", () => { test2: "test2", }; - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should reject when annotation does not match", () => { @@ -565,7 +569,7 @@ test("should reject when annotation does not match", () => { }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines annotations '.*' but Object carries '.*'./, ); }); @@ -598,7 +602,7 @@ test("should allow when annotation is match", () => { test2: "test2", }; - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should use `oldObject` when the operation is `DELETE`", () => { @@ -622,7 +626,7 @@ test("should use `oldObject` when the operation is `DELETE`", () => { const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should allow when deletionTimestamp is present on pod", () => { @@ -654,7 +658,7 @@ test("should allow when deletionTimestamp is present on pod", () => { test2: "test2", }; - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); test("should reject when deletionTimestamp is not present on pod", () => { @@ -685,29 +689,7 @@ test("should reject when deletionTimestamp is not present on pod", () => { test2: "test2", }; - expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, - ); -}); - -test.only("filterchain: should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, ); }); From 0bed13297ee6aeb6bea9e793ea1c1bd967fe423e Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 14:08:36 -0500 Subject: [PATCH 04/49] Use it() instead of test() --- src/lib/filterChain.test.ts | 54 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/lib/filterChain.test.ts b/src/lib/filterChain.test.ts index 13f20bc66..6d811c912 100644 --- a/src/lib/filterChain.test.ts +++ b/src/lib/filterChain.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -import { expect, test, describe } from "@jest/globals"; +import { expect, describe, it } from "@jest/globals"; import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; import * as fc from "fast-check"; import { CreatePod, DeletePod } from "../fixtures/loader"; @@ -13,7 +13,7 @@ export const callback = () => undefined; export const podKind = modelToGroupVersionKind(kind.Pod.name); describe("Fuzzing shouldSkipRequest", () => { - test("should handle random inputs without crashing", () => { + it("should handle random inputs without crashing", () => { fc.assert( fc.property( fc.record({ @@ -60,7 +60,7 @@ describe("Fuzzing shouldSkipRequest", () => { }); describe("Property-Based Testing shouldSkipRequest", () => { - test("should only skip requests that do not match the binding criteria", () => { + it("should only skip requests that do not match the binding criteria", () => { fc.assert( fc.property( fc.record({ @@ -109,7 +109,7 @@ describe("Property-Based Testing shouldSkipRequest", () => { }); }); -test("create: should reject when regex name does not match", () => { +it("create: should reject when regex name does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -131,7 +131,7 @@ test("create: should reject when regex name does not match", () => { ); }); -test("create: should not reject when regex name does match", () => { +it("create: should not reject when regex name does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -151,7 +151,7 @@ test("create: should not reject when regex name does match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("delete: should reject when regex name does not match", () => { +it("delete: should reject when regex name does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -173,7 +173,7 @@ test("delete: should reject when regex name does not match", () => { ); }); -test("delete: should not reject when regex name does match", () => { +it("delete: should not reject when regex name does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -193,7 +193,7 @@ test("delete: should not reject when regex name does match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("create: should not reject when regex namespace does match", () => { +it("create: should not reject when regex namespace does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -213,7 +213,7 @@ test("create: should not reject when regex namespace does match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("create: should reject when regex namespace does not match", () => { +it("create: should reject when regex namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -235,7 +235,7 @@ test("create: should reject when regex namespace does not match", () => { ); }); -test("delete: should reject when regex namespace does not match", () => { +it("delete: should reject when regex namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -257,7 +257,7 @@ test("delete: should reject when regex namespace does not match", () => { ); }); -test("delete: should not reject when regex namespace does match", () => { +it("delete: should not reject when regex namespace does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -277,7 +277,7 @@ test("delete: should not reject when regex namespace does match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("delete: should reject when name does not match", () => { +it("delete: should reject when name does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -299,7 +299,7 @@ test("delete: should reject when name does not match", () => { ); }); -test("should reject when kind does not match", () => { +it("should reject when kind does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -326,7 +326,7 @@ test("should reject when kind does not match", () => { ); }); -test("should reject when group does not match", () => { +it("should reject when group does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -353,7 +353,7 @@ test("should reject when group does not match", () => { ); }); -test("should reject when version does not match", () => { +it("should reject when version does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -380,7 +380,7 @@ test("should reject when version does not match", () => { ); }); -test("should allow when group, version, and kind match", () => { +it("should allow when group, version, and kind match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -401,7 +401,7 @@ test("should allow when group, version, and kind match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should allow when kind match and others are empty", () => { +it("should allow when kind match and others are empty", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -426,7 +426,7 @@ test("should allow when kind match and others are empty", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should reject when the capability namespace does not match", () => { +it("should reject when the capability namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -449,7 +449,7 @@ test("should reject when the capability namespace does not match", () => { ); }); -test("should reject when namespace does not match", () => { +it("should reject when namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -472,7 +472,7 @@ test("should reject when namespace does not match", () => { ); }); -test("should allow when namespace is match", () => { +it("should allow when namespace is match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -493,7 +493,7 @@ test("should allow when namespace is match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should reject when label does not match", () => { +it("should reject when label does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -518,7 +518,7 @@ test("should reject when label does not match", () => { ); }); -test("should allow when label is match", () => { +it("should allow when label is match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -549,7 +549,7 @@ test("should allow when label is match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should reject when annotation does not match", () => { +it("should reject when annotation does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -574,7 +574,7 @@ test("should reject when annotation does not match", () => { ); }); -test("should allow when annotation is match", () => { +it("should allow when annotation is match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -605,7 +605,7 @@ test("should allow when annotation is match", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should use `oldObject` when the operation is `DELETE`", () => { +it("should use `oldObject` when the operation is `DELETE`", () => { const binding = { model: kind.Pod, event: Event.Delete, @@ -629,7 +629,7 @@ test("should use `oldObject` when the operation is `DELETE`", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should allow when deletionTimestamp is present on pod", () => { +it("should allow when deletionTimestamp is present on pod", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -661,7 +661,7 @@ test("should allow when deletionTimestamp is present on pod", () => { expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); }); -test("should reject when deletionTimestamp is not present on pod", () => { +it("should reject when deletionTimestamp is not present on pod", () => { const binding = { model: kind.Pod, event: Event.Any, From a5da31310b3b0bd923ce7d686d41199633c04b74 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 14:23:05 -0500 Subject: [PATCH 05/49] Support namespace regex filter --- src/lib/adjudicatorsFilterWrapper.ts | 23 ++++++++++++++++++++--- src/lib/filter.ts | 3 ++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 1216bfc8e..aeb886e53 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -1,14 +1,31 @@ -import { carriedName, definedNameRegex, mismatchedNameRegex } from "./adjudicators"; -import { AdmissionRequest, Binding } from "./types"; +import { + carriedName, + carriedNamespace, + definedNameRegex, + definedNamespaceRegexes, + mismatchedNameRegex, + mismatchedNamespaceRegex, +} from "./adjudicators"; +import { AdmissionRequest, Binding, Operation } from "./types"; //TODO: Dupe'd declaration type FilterParams = { binding: Binding; request: AdmissionRequest }; const prefix = "Ignoring Admission Callback:"; export const mismatchedNameRegexFilter = (data: FilterParams): string => { - const result = mismatchedNameRegex(data.binding, data.request.operation) + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedNameRegex(data.binding, obj) ? `${prefix} Binding defines name regex '${definedNameRegex(data.binding)}' ` + `but Object carries '${carriedName(data.request.operation)}'.` : ""; return result; }; + +export const mismatchedNamespaceRegexFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedNamespaceRegex(data.binding, obj) + ? `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(data.binding))}' but Object carries '${carriedNamespace(obj)}'.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index f7c005cf2..56cd02f7f 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -38,7 +38,7 @@ import { unbindableNamespaces, uncarryableNamespace, } from "./adjudicators"; -import { mismatchedNameRegexFilter } from "./adjudicatorsFilterWrapper"; +import { mismatchedNameRegexFilter, mismatchedNamespaceRegexFilter } from "./adjudicatorsFilterWrapper"; /** * shouldSkipRequest determines if a request should be skipped based on the binding filters. @@ -136,6 +136,7 @@ export const shouldSkipRequestWithFilterChain = ( const filterChain = new FilterChain(); filterChain.addFilter(mismatchedNameRegexFilter); + filterChain.addFilter(mismatchedNamespaceRegexFilter); const admissionFilterMessage = filterChain.execute({ binding, request: req }); return admissionFilterMessage; From 531175b3958485b647fbe45644c0804a6d8d48d7 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 14:50:35 -0500 Subject: [PATCH 06/49] support namespace test cases with filter chain --- src/lib/adjudicatorsFilterWrapper.ts | 26 +++++++++++++++++++++++--- src/lib/filter.ts | 18 +++++++++++++----- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index aeb886e53..d76bff593 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -3,20 +3,22 @@ import { carriedNamespace, definedNameRegex, definedNamespaceRegexes, + definedNamespaces, mismatchedNameRegex, + mismatchedNamespace, mismatchedNamespaceRegex, + uncarryableNamespace, } from "./adjudicators"; import { AdmissionRequest, Binding, Operation } from "./types"; //TODO: Dupe'd declaration -type FilterParams = { binding: Binding; request: AdmissionRequest }; +type FilterParams = { binding: Binding; request: AdmissionRequest; capabilityNamespaces: string[] }; const prefix = "Ignoring Admission Callback:"; export const mismatchedNameRegexFilter = (data: FilterParams): string => { const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; const result = mismatchedNameRegex(data.binding, obj) - ? `${prefix} Binding defines name regex '${definedNameRegex(data.binding)}' ` + - `but Object carries '${carriedName(data.request.operation)}'.` + ? `${prefix} Binding defines name regex '${definedNameRegex(data.binding)}' but Object carries '${carriedName(data.request.operation)}'.` : ""; return result; }; @@ -29,3 +31,21 @@ export const mismatchedNamespaceRegexFilter = (data: FilterParams): string => { return result; }; + +export const mismatchedNamespaceFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedNamespace(data.binding, obj) + ? `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(data.binding))}' but Object carries '${carriedNamespace(obj)}'.` + : ""; + + return result; +}; + +export const uncarryableNamespaceFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = uncarryableNamespace(data.capabilityNamespaces, obj) + ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(data.capabilityNamespaces)}'.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index 56cd02f7f..635a53802 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -38,7 +38,12 @@ import { unbindableNamespaces, uncarryableNamespace, } from "./adjudicators"; -import { mismatchedNameRegexFilter, mismatchedNamespaceRegexFilter } from "./adjudicatorsFilterWrapper"; +import { + mismatchedNameRegexFilter, + mismatchedNamespaceFilter, + mismatchedNamespaceRegexFilter, + uncarryableNamespaceFilter, +} from "./adjudicatorsFilterWrapper"; /** * shouldSkipRequest determines if a request should be skipped based on the binding filters. @@ -105,7 +110,7 @@ export function shouldSkipRequest( } //TODO: Dupe'd declaration -type FilterParams = { binding: Binding; request: AdmissionRequest }; +type FilterParams = { binding: Binding; request: AdmissionRequest; capabilityNamespaces: string[] }; interface Filter { (data: FilterParams): string; } @@ -119,8 +124,9 @@ export class FilterChain { } public execute(data: FilterParams): string { return this.filters.reduce((result, filter) => { + result += filter(data); // The result of each filter is passed as a new concatenated string - return filter(data); + return result; }, ""); } } @@ -128,7 +134,7 @@ export class FilterChain { export const shouldSkipRequestWithFilterChain = ( binding: Binding, req: AdmissionRequest, - // capabilityNamespaces: string[], + capabilityNamespaces: string[], // ignoredNamespaces?: string[], ): string => { // const obj = req.operation === Operation.DELETE ? req.oldObject : req.object; @@ -136,8 +142,10 @@ export const shouldSkipRequestWithFilterChain = ( const filterChain = new FilterChain(); filterChain.addFilter(mismatchedNameRegexFilter); + filterChain.addFilter(mismatchedNamespaceFilter); filterChain.addFilter(mismatchedNamespaceRegexFilter); + filterChain.addFilter(uncarryableNamespaceFilter); - const admissionFilterMessage = filterChain.execute({ binding, request: req }); + const admissionFilterMessage = filterChain.execute({ binding, request: req, capabilityNamespaces }); return admissionFilterMessage; }; From dc85c20dd2afb91a849979ac3d2da64828827ff9 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 15:13:05 -0500 Subject: [PATCH 07/49] Support deletion timestamp tests --- src/lib/adjudicatorsFilterWrapper.ts | 10 ++++++++++ src/lib/filter.ts | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index d76bff593..8559c0b13 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -4,6 +4,7 @@ import { definedNameRegex, definedNamespaceRegexes, definedNamespaces, + mismatchedDeletionTimestamp, mismatchedNameRegex, mismatchedNamespace, mismatchedNamespaceRegex, @@ -49,3 +50,12 @@ export const uncarryableNamespaceFilter = (data: FilterParams): string => { return result; }; + +export const mismatchedDeletionTimestampFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedDeletionTimestamp(data.binding, obj) + ? `${prefix} Binding defines deletionTimestamp but Object does not carry it.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index 635a53802..bd8b84773 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -39,6 +39,7 @@ import { uncarryableNamespace, } from "./adjudicators"; import { + mismatchedDeletionTimestampFilter, mismatchedNameRegexFilter, mismatchedNamespaceFilter, mismatchedNamespaceRegexFilter, @@ -145,6 +146,7 @@ export const shouldSkipRequestWithFilterChain = ( filterChain.addFilter(mismatchedNamespaceFilter); filterChain.addFilter(mismatchedNamespaceRegexFilter); filterChain.addFilter(uncarryableNamespaceFilter); + filterChain.addFilter(mismatchedDeletionTimestampFilter); const admissionFilterMessage = filterChain.execute({ binding, request: req, capabilityNamespaces }); return admissionFilterMessage; From e8398e7e1254707374882568738dc11d0da00a8b Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 15:17:30 -0500 Subject: [PATCH 08/49] Support label and annotation test cases --- src/lib/adjudicatorsFilterWrapper.ts | 22 ++++++++++++++++++++++ src/lib/filter.ts | 4 ++++ 2 files changed, 26 insertions(+) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 8559c0b13..3d01e7d1d 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -1,10 +1,14 @@ import { + carriedAnnotations, carriedName, carriedNamespace, + definedAnnotations, definedNameRegex, definedNamespaceRegexes, definedNamespaces, + mismatchedAnnotations, mismatchedDeletionTimestamp, + mismatchedLabels, mismatchedNameRegex, mismatchedNamespace, mismatchedNamespaceRegex, @@ -59,3 +63,21 @@ export const mismatchedDeletionTimestampFilter = (data: FilterParams): string => return result; }; + +export const mismatchedAnnotationsFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedAnnotations(data.binding, obj) + ? `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(data.binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` + : ""; + + return result; +}; + +export const mismatchedLabelsFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedLabels(data.binding, obj) + ? `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(data.binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index bd8b84773..d9470e1e7 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -39,7 +39,9 @@ import { uncarryableNamespace, } from "./adjudicators"; import { + mismatchedAnnotationsFilter, mismatchedDeletionTimestampFilter, + mismatchedLabelsFilter, mismatchedNameRegexFilter, mismatchedNamespaceFilter, mismatchedNamespaceRegexFilter, @@ -147,6 +149,8 @@ export const shouldSkipRequestWithFilterChain = ( filterChain.addFilter(mismatchedNamespaceRegexFilter); filterChain.addFilter(uncarryableNamespaceFilter); filterChain.addFilter(mismatchedDeletionTimestampFilter); + filterChain.addFilter(mismatchedAnnotationsFilter); + filterChain.addFilter(mismatchedLabelsFilter); const admissionFilterMessage = filterChain.execute({ binding, request: req, capabilityNamespaces }); return admissionFilterMessage; From 2fe80ec1ecfda6a19dfdec4dcbbe21e5a4450f93 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 15:20:49 -0500 Subject: [PATCH 09/49] Support kind filter tests --- src/lib/adjudicatorsFilterWrapper.ts | 11 +++++++++++ src/lib/filter.ts | 2 ++ 2 files changed, 13 insertions(+) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 3d01e7d1d..bd2037fa7 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -2,12 +2,15 @@ import { carriedAnnotations, carriedName, carriedNamespace, + declaredKind, definedAnnotations, + definedKind, definedNameRegex, definedNamespaceRegexes, definedNamespaces, mismatchedAnnotations, mismatchedDeletionTimestamp, + mismatchedKind, mismatchedLabels, mismatchedNameRegex, mismatchedNamespace, @@ -81,3 +84,11 @@ export const mismatchedLabelsFilter = (data: FilterParams): string => { return result; }; + +export const mismatchedKindFilter = (data: FilterParams): string => { + const result = mismatchedKind(data.binding, data.request) + ? `${prefix} Binding defines kind '${definedKind(data.binding)}' but Request declares '${declaredKind(data.request)}'.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index d9470e1e7..d736c99b9 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -41,6 +41,7 @@ import { import { mismatchedAnnotationsFilter, mismatchedDeletionTimestampFilter, + mismatchedKindFilter, mismatchedLabelsFilter, mismatchedNameRegexFilter, mismatchedNamespaceFilter, @@ -151,6 +152,7 @@ export const shouldSkipRequestWithFilterChain = ( filterChain.addFilter(mismatchedDeletionTimestampFilter); filterChain.addFilter(mismatchedAnnotationsFilter); filterChain.addFilter(mismatchedLabelsFilter); + filterChain.addFilter(mismatchedKindFilter); const admissionFilterMessage = filterChain.execute({ binding, request: req, capabilityNamespaces }); return admissionFilterMessage; From 231efee653e197d5d539c476d27f47bc9985355a Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 15:24:04 -0500 Subject: [PATCH 10/49] Support group and version filters --- src/lib/adjudicatorsFilterWrapper.ts | 18 ++++++++++++++++++ src/lib/filter.ts | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index bd2037fa7..c4ad25445 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -10,11 +10,13 @@ import { definedNamespaces, mismatchedAnnotations, mismatchedDeletionTimestamp, + mismatchedGroup, mismatchedKind, mismatchedLabels, mismatchedNameRegex, mismatchedNamespace, mismatchedNamespaceRegex, + mismatchedVersion, uncarryableNamespace, } from "./adjudicators"; import { AdmissionRequest, Binding, Operation } from "./types"; @@ -92,3 +94,19 @@ export const mismatchedKindFilter = (data: FilterParams): string => { return result; }; + +export const mismatchedGroupFilter = (data: FilterParams): string => { + const result = mismatchedGroup(data.binding, data.request) + ? `${prefix} Binding defines group '${definedKind(data.binding)}' but Request declares '${declaredKind(data.request)}'.` + : ""; + + return result; +}; + +export const mismatchedVersionFilter = (data: FilterParams): string => { + const result = mismatchedVersion(data.binding, data.request) + ? `${prefix} Binding defines version '${definedKind(data.binding)}' but Request declares '${declaredKind(data.request)}'.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index d736c99b9..18cf6dfa4 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -41,11 +41,13 @@ import { import { mismatchedAnnotationsFilter, mismatchedDeletionTimestampFilter, + mismatchedGroupFilter, mismatchedKindFilter, mismatchedLabelsFilter, mismatchedNameRegexFilter, mismatchedNamespaceFilter, mismatchedNamespaceRegexFilter, + mismatchedVersionFilter, uncarryableNamespaceFilter, } from "./adjudicatorsFilterWrapper"; @@ -153,6 +155,8 @@ export const shouldSkipRequestWithFilterChain = ( filterChain.addFilter(mismatchedAnnotationsFilter); filterChain.addFilter(mismatchedLabelsFilter); filterChain.addFilter(mismatchedKindFilter); + filterChain.addFilter(mismatchedGroupFilter); + filterChain.addFilter(mismatchedVersionFilter); const admissionFilterMessage = filterChain.execute({ binding, request: req, capabilityNamespaces }); return admissionFilterMessage; From b1e72f9665241cd71e07f577d79ac07712c2f046 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 16:07:03 -0500 Subject: [PATCH 11/49] Add support for ignored namespaces on filter chain --- src/lib/adjudicatorsFilterWrapper.ts | 27 ++++++++- src/lib/filter.ts | 20 ++++++- src/lib/filterChain.test.ts | 83 ++++++++++++++++++++++++++++ src/lib/mutate-processor.ts | 4 +- src/lib/validate-processor.ts | 4 +- 5 files changed, 130 insertions(+), 8 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index c4ad25445..3018b4282 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -2,9 +2,11 @@ import { carriedAnnotations, carriedName, carriedNamespace, + carriesIgnoredNamespace, declaredKind, definedAnnotations, definedKind, + definedName, definedNameRegex, definedNamespaceRegexes, definedNamespaces, @@ -13,6 +15,7 @@ import { mismatchedGroup, mismatchedKind, mismatchedLabels, + mismatchedName, mismatchedNameRegex, mismatchedNamespace, mismatchedNamespaceRegex, @@ -22,9 +25,22 @@ import { import { AdmissionRequest, Binding, Operation } from "./types"; //TODO: Dupe'd declaration -type FilterParams = { binding: Binding; request: AdmissionRequest; capabilityNamespaces: string[] }; +type FilterParams = { + binding: Binding; + request: AdmissionRequest; + capabilityNamespaces: string[]; + ignoredNamespaces?: string[]; +}; const prefix = "Ignoring Admission Callback:"; +export const mismatchedNameFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = mismatchedName(data.binding, obj) + ? `${prefix} Binding defines name '${definedName(data.binding)}' but Object carries '${carriedName(obj)}'.` + : ""; + return result; +}; + export const mismatchedNameRegexFilter = (data: FilterParams): string => { const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; const result = mismatchedNameRegex(data.binding, obj) @@ -110,3 +126,12 @@ export const mismatchedVersionFilter = (data: FilterParams): string => { return result; }; + +export const carriesIgnoredNamespacesFilter = (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + const result = carriesIgnoredNamespace(data.ignoredNamespaces, obj) + ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(data.ignoredNamespaces)}'.` + : ""; + + return result; +}; diff --git a/src/lib/filter.ts b/src/lib/filter.ts index 18cf6dfa4..a82627438 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -39,11 +39,13 @@ import { uncarryableNamespace, } from "./adjudicators"; import { + carriesIgnoredNamespacesFilter, mismatchedAnnotationsFilter, mismatchedDeletionTimestampFilter, mismatchedGroupFilter, mismatchedKindFilter, mismatchedLabelsFilter, + mismatchedNameFilter, mismatchedNameRegexFilter, mismatchedNamespaceFilter, mismatchedNamespaceRegexFilter, @@ -116,7 +118,12 @@ export function shouldSkipRequest( } //TODO: Dupe'd declaration -type FilterParams = { binding: Binding; request: AdmissionRequest; capabilityNamespaces: string[] }; +type FilterParams = { + binding: Binding; + request: AdmissionRequest; + capabilityNamespaces: string[]; + ignoredNamespaces?: string[]; +}; interface Filter { (data: FilterParams): string; } @@ -141,7 +148,7 @@ export const shouldSkipRequestWithFilterChain = ( binding: Binding, req: AdmissionRequest, capabilityNamespaces: string[], - // ignoredNamespaces?: string[], + ignoredNamespaces?: string[], ): string => { // const obj = req.operation === Operation.DELETE ? req.oldObject : req.object; @@ -157,7 +164,14 @@ export const shouldSkipRequestWithFilterChain = ( filterChain.addFilter(mismatchedKindFilter); filterChain.addFilter(mismatchedGroupFilter); filterChain.addFilter(mismatchedVersionFilter); + filterChain.addFilter(mismatchedNameFilter); + filterChain.addFilter(carriesIgnoredNamespacesFilter); - const admissionFilterMessage = filterChain.execute({ binding, request: req, capabilityNamespaces }); + const admissionFilterMessage = filterChain.execute({ + binding, + request: req, + capabilityNamespaces, + ignoredNamespaces, + }); return admissionFilterMessage; }; diff --git a/src/lib/filterChain.test.ts b/src/lib/filterChain.test.ts index 6d811c912..1bac0bac0 100644 --- a/src/lib/filterChain.test.ts +++ b/src/lib/filterChain.test.ts @@ -693,3 +693,86 @@ it("should reject when deletionTimestamp is not present on pod", () => { /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, ); }); + +it("create: should not reject when namespace is not ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch(""); +}); +it("create: should reject when namespace is ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequestWithFilterChain(binding, pod, [], ["helm-releasename"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + ); +}); + +it("delete: should reject when namespace is ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequestWithFilterChain(binding, pod, [], ["helm-releasename"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + ); +}); + +it("delete: should not reject when namespace is not ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch(""); +}); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index eef1f0ead..c5023c952 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -6,7 +6,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; import { Errors } from "./errors"; -import { shouldSkipRequest } from "./filter"; +import { shouldSkipRequestWithFilterChain } from "./filter"; import { MutateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; @@ -50,7 +50,7 @@ export async function mutateProcessor( } // 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 = shouldSkipRequestWithFilterChain(action, req, namespaces, config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); continue; diff --git a/src/lib/validate-processor.ts b/src/lib/validate-processor.ts index c41fa0608..20953a80e 100644 --- a/src/lib/validate-processor.ts +++ b/src/lib/validate-processor.ts @@ -4,7 +4,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; -import { shouldSkipRequest } from "./filter"; +import { shouldSkipRequestWithFilterChain } from "./filter"; import { ValidateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; @@ -44,7 +44,7 @@ export async function validateProcessor( }; // 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 = shouldSkipRequestWithFilterChain(action, req, namespaces, config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); continue; From 6e2f724e51b9239e58a0602669a13d17a6656a83 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 16:21:03 -0500 Subject: [PATCH 12/49] Replace shouldSkipRequest() with FilterChain implementation --- src/lib/filter.test.ts | 137 ++++-- src/lib/filter.ts | 110 +---- src/lib/filterChain.test.ts | 778 ---------------------------------- src/lib/mutate-processor.ts | 4 +- src/lib/validate-processor.ts | 4 +- 5 files changed, 123 insertions(+), 910 deletions(-) delete mode 100644 src/lib/filterChain.test.ts diff --git a/src/lib/filter.test.ts b/src/lib/filter.test.ts index 6e714dd37..e2469b585 100644 --- a/src/lib/filter.test.ts +++ b/src/lib/filter.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -import { expect, test, describe } from "@jest/globals"; +import { expect, describe, it } from "@jest/globals"; import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; import * as fc from "fast-check"; import { CreatePod, DeletePod } from "../fixtures/loader"; @@ -13,7 +13,7 @@ export const callback = () => undefined; export const podKind = modelToGroupVersionKind(kind.Pod.name); describe("Fuzzing shouldSkipRequest", () => { - test("should handle random inputs without crashing", () => { + it("should handle random inputs without crashing", () => { fc.assert( fc.property( fc.record({ @@ -60,7 +60,7 @@ describe("Fuzzing shouldSkipRequest", () => { }); describe("Property-Based Testing shouldSkipRequest", () => { - test("should only skip requests that do not match the binding criteria", () => { + it("should only skip requests that do not match the binding criteria", () => { fc.assert( fc.property( fc.record({ @@ -105,7 +105,7 @@ describe("Property-Based Testing shouldSkipRequest", () => { }); }); -test("create: should reject when regex name does not match", () => { +it("create: should reject when regex name does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -127,7 +127,7 @@ test("create: should reject when regex name does not match", () => { ); }); -test("create: should not reject when regex name does match", () => { +it("create: should not reject when regex name does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -147,7 +147,7 @@ test("create: should not reject when regex name does match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("delete: should reject when regex name does not match", () => { +it("delete: should reject when regex name does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -169,7 +169,7 @@ test("delete: should reject when regex name does not match", () => { ); }); -test("delete: should not reject when regex name does match", () => { +it("delete: should not reject when regex name does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -189,7 +189,7 @@ test("delete: should not reject when regex name does match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("create: should not reject when regex namespace does match", () => { +it("create: should not reject when regex namespace does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -209,7 +209,7 @@ test("create: should not reject when regex namespace does match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("create: should reject when regex namespace does not match", () => { +it("create: should reject when regex namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -231,7 +231,7 @@ test("create: should reject when regex namespace does not match", () => { ); }); -test("delete: should reject when regex namespace does not match", () => { +it("delete: should reject when regex namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -253,7 +253,7 @@ test("delete: should reject when regex namespace does not match", () => { ); }); -test("delete: should not reject when regex namespace does match", () => { +it("delete: should not reject when regex namespace does match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -273,7 +273,7 @@ test("delete: should not reject when regex namespace does match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("delete: should reject when name does not match", () => { +it("delete: should reject when name does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -295,7 +295,7 @@ test("delete: should reject when name does not match", () => { ); }); -test("should reject when kind does not match", () => { +it("should reject when kind does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -322,7 +322,7 @@ test("should reject when kind does not match", () => { ); }); -test("should reject when group does not match", () => { +it("should reject when group does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -349,7 +349,7 @@ test("should reject when group does not match", () => { ); }); -test("should reject when version does not match", () => { +it("should reject when version does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -376,7 +376,7 @@ test("should reject when version does not match", () => { ); }); -test("should allow when group, version, and kind match", () => { +it("should allow when group, version, and kind match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -397,7 +397,7 @@ test("should allow when group, version, and kind match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should allow when kind match and others are empty", () => { +it("should allow when kind match and others are empty", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -422,7 +422,7 @@ test("should allow when kind match and others are empty", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should reject when the capability namespace does not match", () => { +it("should reject when the capability namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -445,7 +445,7 @@ test("should reject when the capability namespace does not match", () => { ); }); -test("should reject when namespace does not match", () => { +it("should reject when namespace does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -468,7 +468,7 @@ test("should reject when namespace does not match", () => { ); }); -test("should allow when namespace is match", () => { +it("should allow when namespace is match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -489,7 +489,7 @@ test("should allow when namespace is match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should reject when label does not match", () => { +it("should reject when label does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -514,7 +514,7 @@ test("should reject when label does not match", () => { ); }); -test("should allow when label is match", () => { +it("should allow when label is match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -545,7 +545,7 @@ test("should allow when label is match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should reject when annotation does not match", () => { +it("should reject when annotation does not match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -570,7 +570,7 @@ test("should reject when annotation does not match", () => { ); }); -test("should allow when annotation is match", () => { +it("should allow when annotation is match", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -601,7 +601,7 @@ test("should allow when annotation is match", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should use `oldObject` when the operation is `DELETE`", () => { +it("should use `oldObject` when the operation is `DELETE`", () => { const binding = { model: kind.Pod, event: Event.Delete, @@ -625,7 +625,7 @@ test("should use `oldObject` when the operation is `DELETE`", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should allow when deletionTimestamp is present on pod", () => { +it("should allow when deletionTimestamp is present on pod", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -657,7 +657,7 @@ test("should allow when deletionTimestamp is present on pod", () => { expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); -test("should reject when deletionTimestamp is not present on pod", () => { +it("should reject when deletionTimestamp is not present on pod", () => { const binding = { model: kind.Pod, event: Event.Any, @@ -689,3 +689,86 @@ test("should reject when deletionTimestamp is not present on pod", () => { /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, ); }); + +it("create: should not reject when namespace is not ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch(""); +}); +it("create: should reject when namespace is ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + ); +}); + +it("delete: should reject when namespace is ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + ); +}); + +it("delete: should not reject when namespace is not ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch(""); +}); diff --git a/src/lib/filter.ts b/src/lib/filter.ts index a82627438..7fcbfd5fe 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -2,42 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -import { AdmissionRequest, Binding, Operation } from "./types"; -import { - carriedAnnotations, - carriedLabels, - carriedName, - carriedNamespace, - carriesIgnoredNamespace, - declaredGroup, - declaredKind, - declaredOperation, - declaredVersion, - definedAnnotations, - definedEvent, - definedGroup, - definedKind, - definedLabels, - definedName, - definedNameRegex, - definedNamespaceRegexes, - definedNamespaces, - definedVersion, - misboundDeleteWithDeletionTimestamp, - mismatchedAnnotations, - mismatchedDeletionTimestamp, - mismatchedEvent, - mismatchedGroup, - mismatchedKind, - mismatchedLabels, - mismatchedName, - mismatchedNameRegex, - mismatchedNamespace, - mismatchedNamespaceRegex, - mismatchedVersion, - unbindableNamespaces, - uncarryableNamespace, -} from "./adjudicators"; +import { AdmissionRequest, Binding } from "./types"; import { carriesIgnoredNamespacesFilter, mismatchedAnnotationsFilter, @@ -53,70 +18,6 @@ import { uncarryableNamespaceFilter, } from "./adjudicatorsFilterWrapper"; -/** - * shouldSkipRequest determines if a request should be skipped based on the binding filters. - * - * @param binding the action binding - * @param req the incoming request - * @returns - */ -export function shouldSkipRequest( - binding: Binding, - req: AdmissionRequest, - capabilityNamespaces: string[], - ignoredNamespaces?: string[], -): string { - const prefix = "Ignoring Admission Callback:"; - const obj = req.operation === Operation.DELETE ? req.oldObject : req.object; - - const admissionFilterMessage = misboundDeleteWithDeletionTimestamp(binding) - ? `${prefix} Cannot use deletionTimestamp filter on a DELETE operation.` - : mismatchedDeletionTimestamp(binding, obj) - ? `${prefix} Binding defines deletionTimestamp but Object does not carry it.` - : mismatchedEvent(binding, req) - ? `${prefix} Binding defines event '${definedEvent(binding)}' but ` + - `Request declares '${declaredOperation(req)}'.` - : mismatchedName(binding, obj) - ? `${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(obj)}'.` - : mismatchedGroup(binding, req) - ? `${prefix} Binding defines group '${definedGroup(binding)}' but ` + - `Request declares '${declaredGroup(req)}'.` - : mismatchedVersion(binding, req) - ? `${prefix} Binding defines version '${definedVersion(binding)}' but ` + - `Request declares '${declaredVersion(req)}'.` - : mismatchedKind(binding, req) - ? `${prefix} Binding defines kind '${definedKind(binding)}' but ` + - `Request declares '${declaredKind(req)}'.` - : unbindableNamespaces(capabilityNamespaces, binding) - ? `${prefix} Binding defines namespaces ${JSON.stringify(definedNamespaces(binding))} ` + - `but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.` - : uncarryableNamespace(capabilityNamespaces, obj) - ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' ` + - `but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.` - : mismatchedNamespace(binding, obj) - ? `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' ` + - `but Object carries '${carriedNamespace(obj)}'.` - : mismatchedLabels(binding, obj) - ? `${prefix} Binding defines labels '${JSON.stringify(definedLabels(binding))}' ` + - `but Object carries '${JSON.stringify(carriedLabels(obj))}'.` - : mismatchedAnnotations(binding, obj) - ? `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' ` + - `but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` - : mismatchedNamespaceRegex(binding, obj) - ? `${prefix} Binding defines namespace regexes ` + - `'${JSON.stringify(definedNamespaceRegexes(binding))}' ` + - `but Object carries '${carriedNamespace(obj)}'.` - : mismatchedNameRegex(binding, obj) - ? `${prefix} Binding defines name regex '${definedNameRegex(binding)}' ` + - `but Object carries '${carriedName(obj)}'.` - : carriesIgnoredNamespace(ignoredNamespaces, obj) - ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' ` + - `but ignored namespaces include '${JSON.stringify(ignoredNamespaces)}'.` - : ""; - - return admissionFilterMessage; -} - //TODO: Dupe'd declaration type FilterParams = { binding: Binding; @@ -144,7 +45,14 @@ export class FilterChain { } } -export const shouldSkipRequestWithFilterChain = ( +/** + * shouldSkipRequest determines if a request should be skipped based on the binding filters. + * + * @param binding the action binding + * @param req the incoming request + * @returns + */ +export const shouldSkipRequest = ( binding: Binding, req: AdmissionRequest, capabilityNamespaces: string[], diff --git a/src/lib/filterChain.test.ts b/src/lib/filterChain.test.ts deleted file mode 100644 index 1bac0bac0..000000000 --- a/src/lib/filterChain.test.ts +++ /dev/null @@ -1,778 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2023-Present The Pepr Authors - -import { expect, describe, it } from "@jest/globals"; -import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; -import * as fc from "fast-check"; -import { CreatePod, DeletePod } from "../fixtures/loader"; -import { shouldSkipRequestWithFilterChain } from "./filter"; -import { AdmissionRequest, Binding, Event } from "./types"; - -export const callback = () => undefined; - -export const podKind = modelToGroupVersionKind(kind.Pod.name); - -describe("Fuzzing shouldSkipRequest", () => { - it("should handle random inputs without crashing", () => { - fc.assert( - fc.property( - fc.record({ - event: fc.constantFrom("CREATE", "UPDATE", "DELETE", "ANY"), - kind: fc.record({ - group: fc.string(), - version: fc.string(), - kind: fc.string(), - }), - filters: fc.record({ - name: fc.string(), - namespaces: fc.array(fc.string()), - labels: fc.dictionary(fc.string(), fc.string()), - annotations: fc.dictionary(fc.string(), fc.string()), - deletionTimestamp: fc.boolean(), - }), - }), - fc.record({ - operation: fc.string(), - uid: fc.string(), - name: fc.string(), - namespace: fc.string(), - kind: fc.record({ - group: fc.string(), - version: fc.string(), - kind: fc.string(), - }), - object: fc.record({ - metadata: fc.record({ - deletionTimestamp: fc.option(fc.date()), - }), - }), - }), - fc.array(fc.string()), - (binding, req, capabilityNamespaces) => { - expect(() => - shouldSkipRequestWithFilterChain(binding as Binding, req as AdmissionRequest, capabilityNamespaces), - ).not.toThrow(); - }, - ), - { numRuns: 100 }, - ); - }); -}); - -describe("Property-Based Testing shouldSkipRequest", () => { - it("should only skip requests that do not match the binding criteria", () => { - fc.assert( - fc.property( - fc.record({ - event: fc.constantFrom("CREATE", "UPDATE", "DELETE", "ANY"), - kind: fc.record({ - group: fc.string(), - version: fc.string(), - kind: fc.string(), - }), - filters: fc.record({ - name: fc.string(), - namespaces: fc.array(fc.string()), - labels: fc.dictionary(fc.string(), fc.string()), - annotations: fc.dictionary(fc.string(), fc.string()), - deletionTimestamp: fc.boolean(), - }), - }), - fc.record({ - operation: fc.string(), - uid: fc.string(), - name: fc.string(), - namespace: fc.string(), - kind: fc.record({ - group: fc.string(), - version: fc.string(), - kind: fc.string(), - }), - object: fc.record({ - metadata: fc.record({ - deletionTimestamp: fc.option(fc.date()), - }), - }), - }), - fc.array(fc.string()), - (binding, req, capabilityNamespaces) => { - const shouldSkip = shouldSkipRequestWithFilterChain( - binding as Binding, - req as AdmissionRequest, - capabilityNamespaces, - ); - expect(typeof shouldSkip).toBe("string"); - }, - ), - { numRuns: 100 }, - ); - }); -}); - -it("create: should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, - ); -}); - -it("create: should not reject when regex name does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("delete: should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, - ); -}); - -it("delete: should not reject when regex name does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("create: should not reject when regex namespace does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^helm"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("create: should reject when regex namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, - ); -}); - -it("delete: should reject when regex namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, - ); -}); - -it("delete: should not reject when regex namespace does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^helm"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("delete: should reject when name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "bleh", - namespaces: [], - regexNamespaces: [], - regexName: "^not-cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name '.*' but Object carries '.*'./, - ); -}); - -it("should reject when kind does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: { - group: "", - version: "v1", - kind: "Nope", - }, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines kind '.*' but Request declares '.*'./, - ); -}); - -it("should reject when group does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: { - group: "Nope", - version: "v1", - kind: "Pod", - }, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines group '.*' but Request declares '.*'./, - ); -}); - -it("should reject when version does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: { - group: "", - version: "Nope", - kind: "Pod", - }, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines version '.*' but Request declares '.*'./, - ); -}); - -it("should allow when group, version, and kind match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should allow when kind match and others are empty", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: { - group: "", - version: "", - kind: "Pod", - }, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should reject when the capability namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, ["bleh", "bleh2"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but namespaces allowed by Capability are '.*'./, - ); -}); - -it("should reject when namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: ["bleh"], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespaces '.*' but Object carries '.*'./, - ); -}); - -it("should allow when namespace is match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: ["helm-releasename", "unicorn", "things"], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should reject when label does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: { - foo: "bar", - }, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines labels '.*' but Object carries '.*'./, - ); -}); - -it("should allow when label is match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - deletionTimestamp: false, - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: { - foo: "bar", - test: "test1", - }, - annotations: {}, - }, - callback, - }; - - const pod = CreatePod(); - pod.object.metadata = pod.object.metadata || {}; - pod.object.metadata.labels = { - foo: "bar", - test: "test1", - test2: "test2", - }; - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should reject when annotation does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: { - foo: "bar", - }, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - const pod = CreatePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines annotations '.*' but Object carries '.*'./, - ); -}); - -it("should allow when annotation is match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: { - foo: "bar", - test: "test1", - }, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; - - const pod = CreatePod(); - pod.object.metadata = pod.object.metadata || {}; - pod.object.metadata.annotations = { - foo: "bar", - test: "test1", - test2: "test2", - }; - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should use `oldObject` when the operation is `DELETE`", () => { - const binding = { - model: kind.Pod, - event: Event.Delete, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - deletionTimestamp: false, - labels: { - "test-op": "delete", - }, - annotations: {}, - }, - callback, - }; - - const pod = DeletePod(); - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should allow when deletionTimestamp is present on pod", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - regexNamespaces: [], - regexName: "", - annotations: { - foo: "bar", - test: "test1", - }, - deletionTimestamp: true, - }, - callback, - }; - - const pod = CreatePod(); - pod.object.metadata = pod.object.metadata || {}; - pod.object.metadata!.deletionTimestamp = new Date("2021-09-01T00:00:00Z"); - pod.object.metadata.annotations = { - foo: "bar", - test: "test1", - test2: "test2", - }; - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toBe(""); -}); - -it("should reject when deletionTimestamp is not present on pod", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - regexNamespaces: [], - regexName: "", - annotations: { - foo: "bar", - test: "test1", - }, - deletionTimestamp: true, - }, - callback, - }; - - const pod = CreatePod(); - pod.object.metadata = pod.object.metadata || {}; - pod.object.metadata.annotations = { - foo: "bar", - test: "test1", - test2: "test2", - }; - - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, - ); -}); - -it("create: should not reject when namespace is not ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch(""); -}); -it("create: should reject when namespace is ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [], ["helm-releasename"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, - ); -}); - -it("delete: should reject when namespace is ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [], ["helm-releasename"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, - ); -}); - -it("delete: should not reject when namespace is not ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequestWithFilterChain(binding, pod, [])).toMatch(""); -}); diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index c5023c952..eef1f0ead 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -6,7 +6,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; import { Errors } from "./errors"; -import { shouldSkipRequestWithFilterChain } from "./filter"; +import { shouldSkipRequest } from "./filter"; import { MutateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; @@ -50,7 +50,7 @@ export async function mutateProcessor( } // Continue to the next action without doing anything if this one should be skipped - const shouldSkip = shouldSkipRequestWithFilterChain(action, req, namespaces, config?.alwaysIgnore?.namespaces); + const shouldSkip = shouldSkipRequest(action, req, namespaces, config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); continue; diff --git a/src/lib/validate-processor.ts b/src/lib/validate-processor.ts index 20953a80e..c41fa0608 100644 --- a/src/lib/validate-processor.ts +++ b/src/lib/validate-processor.ts @@ -4,7 +4,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; -import { shouldSkipRequestWithFilterChain } from "./filter"; +import { shouldSkipRequest } from "./filter"; import { ValidateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; @@ -44,7 +44,7 @@ export async function validateProcessor( }; // Continue to the next action without doing anything if this one should be skipped - const shouldSkip = shouldSkipRequestWithFilterChain(action, req, namespaces, config?.alwaysIgnore?.namespaces); + const shouldSkip = shouldSkipRequest(action, req, namespaces, config?.alwaysIgnore?.namespaces); if (shouldSkip !== "") { Log.debug(shouldSkip); continue; From 343ffdf8489b404e7ad56db297c3cd38512db0b0 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Thu, 24 Oct 2024 17:09:49 -0500 Subject: [PATCH 13/49] Initial attempt to consolidate filter-generation logic --- src/lib/adjudicatorsFilterWrapper.ts | 171 +++++++++++++-------------- 1 file changed, 84 insertions(+), 87 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 3018b4282..c9fbf6768 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -1,3 +1,4 @@ +import { KubernetesObject } from "kubernetes-fluent-client"; import { carriedAnnotations, carriedName, @@ -24,114 +25,110 @@ import { } from "./adjudicators"; import { AdmissionRequest, Binding, Operation } from "./types"; -//TODO: Dupe'd declaration type FilterParams = { binding: Binding; request: AdmissionRequest; capabilityNamespaces: string[]; ignoredNamespaces?: string[]; }; -const prefix = "Ignoring Admission Callback:"; - -export const mismatchedNameFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedName(data.binding, obj) - ? `${prefix} Binding defines name '${definedName(data.binding)}' but Object carries '${carriedName(obj)}'.` - : ""; - return result; -}; - -export const mismatchedNameRegexFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedNameRegex(data.binding, obj) - ? `${prefix} Binding defines name regex '${definedNameRegex(data.binding)}' but Object carries '${carriedName(data.request.operation)}'.` - : ""; - return result; -}; - -export const mismatchedNamespaceRegexFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedNamespaceRegex(data.binding, obj) - ? `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(data.binding))}' but Object carries '${carriedNamespace(obj)}'.` - : ""; - - return result; -}; -export const mismatchedNamespaceFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedNamespace(data.binding, obj) - ? `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(data.binding))}' but Object carries '${carriedNamespace(obj)}'.` - : ""; +const prefix = "Ignoring Admission Callback:"; - return result; +const createBindingObjectFilter = ( + mismatchCheck: (data: Binding, obj?: KubernetesObject) => boolean, + logMessage: (data: Binding, obj?: KubernetesObject) => string, +) => { + return (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + return mismatchCheck(data.binding, obj) ? logMessage(data.binding, obj) : ""; + }; }; -export const uncarryableNamespaceFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = uncarryableNamespace(data.capabilityNamespaces, obj) - ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(data.capabilityNamespaces)}'.` - : ""; - - return result; +const createBindingRequestFilter = ( + mismatchCheck: (data: Binding, obj: AdmissionRequest) => boolean, + logMessage: (data: Binding, obj: AdmissionRequest) => string, +) => { + return (data: FilterParams): string => { + return mismatchCheck(data.binding, data.request) ? logMessage(data.binding, data.request) : ""; + }; }; -export const mismatchedDeletionTimestampFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedDeletionTimestamp(data.binding, obj) - ? `${prefix} Binding defines deletionTimestamp but Object does not carry it.` - : ""; - - return result; +const createArrayObjectFilter = ( + mismatchCheck: (data: string[], obj?: KubernetesObject) => boolean, + logMessage: (data: string[], obj?: KubernetesObject) => string, +) => { + return (data: FilterParams): string => { + const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; + return mismatchCheck(data.capabilityNamespaces, obj) ? logMessage(data.capabilityNamespaces, obj) : ""; + }; }; -export const mismatchedAnnotationsFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedAnnotations(data.binding, obj) - ? `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(data.binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` - : ""; - - return result; -}; +export const mismatchedNameFilter = createBindingObjectFilter( + mismatchedName, + (data, obj) => `${prefix} data.binding defines name '${definedName(data)}' but Object carries '${carriedName(obj)}'.`, +); -export const mismatchedLabelsFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = mismatchedLabels(data.binding, obj) - ? `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(data.binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.` - : ""; +export const mismatchedNameRegexFilter = createBindingObjectFilter( + mismatchedNameRegex, + (data, obj) => + `${prefix} data.binding defines name regex '${definedNameRegex(data)}' but Object carries '${carriedName(obj)}'.`, +); - return result; -}; +export const mismatchedNamespaceRegexFilter = createBindingObjectFilter( + mismatchedNamespaceRegex, + (data, obj) => + `${prefix} data.binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(data))}' but Object carries '${carriedNamespace(obj)}'.`, +); -export const mismatchedKindFilter = (data: FilterParams): string => { - const result = mismatchedKind(data.binding, data.request) - ? `${prefix} Binding defines kind '${definedKind(data.binding)}' but Request declares '${declaredKind(data.request)}'.` - : ""; +export const mismatchedNamespaceFilter = createBindingObjectFilter( + mismatchedNamespace, + (data, obj) => + `${prefix} data.binding defines namespace regexes '${JSON.stringify(definedNamespaces(data))}' but Object carries '${carriedNamespace(obj)}'.`, +); - return result; -}; +export const mismatchedAnnotationsFilter = createBindingObjectFilter( + mismatchedAnnotations, + (data, obj) => + `${prefix} data.binding defines annotations '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, +); -export const mismatchedGroupFilter = (data: FilterParams): string => { - const result = mismatchedGroup(data.binding, data.request) - ? `${prefix} Binding defines group '${definedKind(data.binding)}' but Request declares '${declaredKind(data.request)}'.` - : ""; +export const mismatchedLabelsFilter = createBindingObjectFilter( + mismatchedLabels, + (data, obj) => + `${prefix} data.binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, +); - return result; -}; +export const mismatchedDeletionTimestampFilter = createBindingObjectFilter( + mismatchedDeletionTimestamp, + (data, obj) => + `${prefix} data.binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, +); +export const mismatchedKindFilter = createBindingRequestFilter( + mismatchedKind, + (data, request) => + `${prefix} data.binding defines kind '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, +); -export const mismatchedVersionFilter = (data: FilterParams): string => { - const result = mismatchedVersion(data.binding, data.request) - ? `${prefix} Binding defines version '${definedKind(data.binding)}' but Request declares '${declaredKind(data.request)}'.` - : ""; +export const mismatchedGroupFilter = createBindingObjectFilter( + mismatchedGroup, + (data, request) => + `${prefix} data.binding defines group '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, +); - return result; -}; +export const mismatchedVersionFilter = createBindingRequestFilter( + mismatchedVersion, + (data, request) => + `${prefix} data.binding defines version '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, +); -export const carriesIgnoredNamespacesFilter = (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - const result = carriesIgnoredNamespace(data.ignoredNamespaces, obj) - ? `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(data.ignoredNamespaces)}'.` - : ""; +export const carriesIgnoredNamespacesFilter = createArrayObjectFilter( + carriesIgnoredNamespace, + (data, obj) => + `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(data)}'.`, +); - return result; -}; +export const uncarryableNamespaceFilter = createArrayObjectFilter( + uncarryableNamespace, + (data, obj) => + `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(data)}'.`, +); From adb2bf3d52c7b14243caf10672ceef4f7178e333 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 09:51:00 -0500 Subject: [PATCH 14/49] Fix log message format --- src/lib/adjudicatorsFilterWrapper.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index c9fbf6768..e6e74d242 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -65,60 +65,60 @@ const createArrayObjectFilter = ( export const mismatchedNameFilter = createBindingObjectFilter( mismatchedName, - (data, obj) => `${prefix} data.binding defines name '${definedName(data)}' but Object carries '${carriedName(obj)}'.`, + (data, obj) => `${prefix} Binding defines name '${definedName(data)}' but Object carries '${carriedName(obj)}'.`, ); export const mismatchedNameRegexFilter = createBindingObjectFilter( mismatchedNameRegex, (data, obj) => - `${prefix} data.binding defines name regex '${definedNameRegex(data)}' but Object carries '${carriedName(obj)}'.`, + `${prefix} Binding defines name regex '${definedNameRegex(data)}' but Object carries '${carriedName(obj)}'.`, ); export const mismatchedNamespaceRegexFilter = createBindingObjectFilter( mismatchedNamespaceRegex, (data, obj) => - `${prefix} data.binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(data))}' but Object carries '${carriedNamespace(obj)}'.`, + `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(data))}' but Object carries '${carriedNamespace(obj)}'.`, ); export const mismatchedNamespaceFilter = createBindingObjectFilter( mismatchedNamespace, (data, obj) => - `${prefix} data.binding defines namespace regexes '${JSON.stringify(definedNamespaces(data))}' but Object carries '${carriedNamespace(obj)}'.`, + `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaces(data))}' but Object carries '${carriedNamespace(obj)}'.`, ); export const mismatchedAnnotationsFilter = createBindingObjectFilter( mismatchedAnnotations, (data, obj) => - `${prefix} data.binding defines annotations '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, + `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, ); export const mismatchedLabelsFilter = createBindingObjectFilter( mismatchedLabels, (data, obj) => - `${prefix} data.binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, + `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, ); export const mismatchedDeletionTimestampFilter = createBindingObjectFilter( mismatchedDeletionTimestamp, (data, obj) => - `${prefix} data.binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, + `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, ); export const mismatchedKindFilter = createBindingRequestFilter( mismatchedKind, (data, request) => - `${prefix} data.binding defines kind '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, + `${prefix} Binding defines kind '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, ); export const mismatchedGroupFilter = createBindingObjectFilter( mismatchedGroup, (data, request) => - `${prefix} data.binding defines group '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, + `${prefix} Binding defines group '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, ); export const mismatchedVersionFilter = createBindingRequestFilter( mismatchedVersion, (data, request) => - `${prefix} data.binding defines version '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, + `${prefix} Binding defines version '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, ); export const carriesIgnoredNamespacesFilter = createArrayObjectFilter( From a9cce0f65246319df4abf42fb10e0a7fdc4e0ec5 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 10:04:10 -0500 Subject: [PATCH 15/49] Review log messags & fix tests --- src/lib/adjudicatorsFilterWrapper.ts | 25 +++++++++++++++++++++---- src/lib/filter.ts | 2 ++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index e6e74d242..28ca76849 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -63,6 +63,17 @@ const createArrayObjectFilter = ( }; }; +const createArrayBindingFilter = ( + mismatchCheck: (data: string[], obj?: Binding) => boolean, + logMessage: (data: string[], obj?: Binding) => string, +) => { + return (data: FilterParams): string => { + return mismatchCheck(data.capabilityNamespaces, data.binding) + ? logMessage(data.capabilityNamespaces, data.binding) + : ""; + }; +}; + export const mismatchedNameFilter = createBindingObjectFilter( mismatchedName, (data, obj) => `${prefix} Binding defines name '${definedName(data)}' but Object carries '${carriedName(obj)}'.`, @@ -83,7 +94,7 @@ export const mismatchedNamespaceRegexFilter = createBindingObjectFilter( export const mismatchedNamespaceFilter = createBindingObjectFilter( mismatchedNamespace, (data, obj) => - `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaces(data))}' but Object carries '${carriedNamespace(obj)}'.`, + `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(data))}' but Object carries '${carriedNamespace(obj)}'.`, ); export const mismatchedAnnotationsFilter = createBindingObjectFilter( @@ -100,8 +111,8 @@ export const mismatchedLabelsFilter = createBindingObjectFilter( export const mismatchedDeletionTimestampFilter = createBindingObjectFilter( mismatchedDeletionTimestamp, - (data, obj) => - `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + (data, obj) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, ); export const mismatchedKindFilter = createBindingRequestFilter( mismatchedKind, @@ -130,5 +141,11 @@ export const carriesIgnoredNamespacesFilter = createArrayObjectFilter( export const uncarryableNamespaceFilter = createArrayObjectFilter( uncarryableNamespace, (data, obj) => - `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(data)}'.`, + `${prefix} Object carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(data)}'.`, +); + +export const unbindableNamespacesFilter = createArrayBindingFilter( + uncarryableNamespace, + (data, obj) => + `${prefix} Binding carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(data)}'.`, ); diff --git a/src/lib/filter.ts b/src/lib/filter.ts index 7fcbfd5fe..03e939446 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter.ts @@ -15,6 +15,7 @@ import { mismatchedNamespaceFilter, mismatchedNamespaceRegexFilter, mismatchedVersionFilter, + unbindableNamespacesFilter, uncarryableNamespaceFilter, } from "./adjudicatorsFilterWrapper"; @@ -74,6 +75,7 @@ export const shouldSkipRequest = ( filterChain.addFilter(mismatchedVersionFilter); filterChain.addFilter(mismatchedNameFilter); filterChain.addFilter(carriesIgnoredNamespacesFilter); + filterChain.addFilter(unbindableNamespacesFilter); const admissionFilterMessage = filterChain.execute({ binding, From 662f7a2ce9d8629342330633c8c4da648832b206 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 10:10:49 -0500 Subject: [PATCH 16/49] Support ignore array operations --- src/lib/adjudicatorsFilterWrapper.ts | 172 +++++++++++++-------------- 1 file changed, 86 insertions(+), 86 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 28ca76849..9fd2281e5 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -1,4 +1,3 @@ -import { KubernetesObject } from "kubernetes-fluent-client"; import { carriedAnnotations, carriedName, @@ -34,118 +33,119 @@ type FilterParams = { const prefix = "Ignoring Admission Callback:"; -const createBindingObjectFilter = ( - mismatchCheck: (data: Binding, obj?: KubernetesObject) => boolean, - logMessage: (data: Binding, obj?: KubernetesObject) => string, +const createFilter = ( + dataSelector: (data: FilterParams) => T1, + objectSelector: (data: FilterParams) => T2, + mismatchCheck: (data: T1, obj?: T2) => boolean, + logMessage: (data: T1, obj?: T2) => string, ) => { return (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - return mismatchCheck(data.binding, obj) ? logMessage(data.binding, obj) : ""; + const dataValue = dataSelector(data); + const objectValue = objectSelector(data); + return mismatchCheck(dataValue, objectValue) ? logMessage(dataValue, objectValue) : ""; }; }; -const createBindingRequestFilter = ( - mismatchCheck: (data: Binding, obj: AdmissionRequest) => boolean, - logMessage: (data: Binding, obj: AdmissionRequest) => string, -) => { - return (data: FilterParams): string => { - return mismatchCheck(data.binding, data.request) ? logMessage(data.binding, data.request) : ""; - }; -}; - -const createArrayObjectFilter = ( - mismatchCheck: (data: string[], obj?: KubernetesObject) => boolean, - logMessage: (data: string[], obj?: KubernetesObject) => string, -) => { - return (data: FilterParams): string => { - const obj = data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; - return mismatchCheck(data.capabilityNamespaces, obj) ? logMessage(data.capabilityNamespaces, obj) : ""; - }; -}; - -const createArrayBindingFilter = ( - mismatchCheck: (data: string[], obj?: Binding) => boolean, - logMessage: (data: string[], obj?: Binding) => string, -) => { - return (data: FilterParams): string => { - return mismatchCheck(data.capabilityNamespaces, data.binding) - ? logMessage(data.capabilityNamespaces, data.binding) - : ""; - }; -}; - -export const mismatchedNameFilter = createBindingObjectFilter( - mismatchedName, - (data, obj) => `${prefix} Binding defines name '${definedName(data)}' but Object carries '${carriedName(obj)}'.`, +export const mismatchedNameFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedName(binding, obj), + (binding, obj) => + `${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(obj)}'.`, ); -export const mismatchedNameRegexFilter = createBindingObjectFilter( - mismatchedNameRegex, - (data, obj) => - `${prefix} Binding defines name regex '${definedNameRegex(data)}' but Object carries '${carriedName(obj)}'.`, +export const mismatchedNameRegexFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedNameRegex(binding, obj), + (binding, obj) => + `${prefix} Binding defines name regex '${definedNameRegex(binding)}' but Object carries '${carriedName(obj)}'.`, ); -export const mismatchedNamespaceRegexFilter = createBindingObjectFilter( - mismatchedNamespaceRegex, - (data, obj) => - `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(data))}' but Object carries '${carriedNamespace(obj)}'.`, +export const mismatchedNamespaceRegexFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedNamespaceRegex(binding, obj), + (binding, obj) => + `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(binding))}' but Object carries '${carriedNamespace(obj)}'.`, ); -export const mismatchedNamespaceFilter = createBindingObjectFilter( - mismatchedNamespace, - (data, obj) => - `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(data))}' but Object carries '${carriedNamespace(obj)}'.`, +export const mismatchedNamespaceFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedNamespace(binding, obj), + (binding, obj) => + `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' but Object carries '${carriedNamespace(obj)}'.`, ); -export const mismatchedAnnotationsFilter = createBindingObjectFilter( - mismatchedAnnotations, - (data, obj) => - `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, +export const mismatchedAnnotationsFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedAnnotations(binding, obj), + (binding, obj) => + `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, ); -export const mismatchedLabelsFilter = createBindingObjectFilter( - mismatchedLabels, - (data, obj) => - `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(data))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, +export const mismatchedLabelsFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedLabels(binding, obj), + (binding, obj) => + `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, ); -export const mismatchedDeletionTimestampFilter = createBindingObjectFilter( - mismatchedDeletionTimestamp, +export const mismatchedDeletionTimestampFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedDeletionTimestamp(binding, obj), // eslint-disable-next-line @typescript-eslint/no-unused-vars - (data, obj) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, + (binding, obj) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, ); -export const mismatchedKindFilter = createBindingRequestFilter( - mismatchedKind, - (data, request) => - `${prefix} Binding defines kind '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, + +export const mismatchedKindFilter = createFilter( + data => data.binding, + data => data.request, + (binding, request) => mismatchedKind(binding, request), + (binding, request) => + `${prefix} Binding defines kind '${definedKind(binding)}' but Request declares '${declaredKind(request)}'.`, ); -export const mismatchedGroupFilter = createBindingObjectFilter( - mismatchedGroup, - (data, request) => - `${prefix} Binding defines group '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, +export const mismatchedGroupFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, obj) => mismatchedGroup(binding, obj), + (binding, obj) => + `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(obj)}'.`, ); -export const mismatchedVersionFilter = createBindingRequestFilter( - mismatchedVersion, - (data, request) => - `${prefix} Binding defines version '${definedKind(data)}' but Request declares '${declaredKind(request)}'.`, +export const mismatchedVersionFilter = createFilter( + data => data.binding, + data => data.request, + (binding, request) => mismatchedVersion(binding, request), + (binding, request) => + `${prefix} Binding defines version '${definedKind(binding)}' but Request declares '${declaredKind(request)}'.`, ); -export const carriesIgnoredNamespacesFilter = createArrayObjectFilter( - carriesIgnoredNamespace, - (data, obj) => - `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(data)}'.`, +export const carriesIgnoredNamespacesFilter = createFilter( + data => data.ignoredNamespaces, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (ignoreArray, obj) => carriesIgnoredNamespace(ignoreArray, obj), + (ignoreArray, obj) => + `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(ignoreArray)}'.`, ); -export const uncarryableNamespaceFilter = createArrayObjectFilter( - uncarryableNamespace, - (data, obj) => - `${prefix} Object carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(data)}'.`, +export const uncarryableNamespaceFilter = createFilter( + data => data.capabilityNamespaces, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (capabilityNamespaces, obj) => uncarryableNamespace(capabilityNamespaces, obj), + (capabilityNamespaces, obj) => + `${prefix} Object carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`, ); -export const unbindableNamespacesFilter = createArrayBindingFilter( - uncarryableNamespace, - (data, obj) => - `${prefix} Binding carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(data)}'.`, +export const unbindableNamespacesFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (capabilityNamespaces, binding) => uncarryableNamespace(capabilityNamespaces, binding), + (capabilityNamespaces, binding) => + `${prefix} Binding carries namespace '${carriedNamespace(binding)}' but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`, ); From a37790ecfd4f689d0b899231a024692fa401e01a Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 10:53:04 -0500 Subject: [PATCH 17/49] Rename obj to kubernetesObject --- src/lib/adjudicatorsFilterWrapper.ts | 65 +++++++++++++--------------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 9fd2281e5..4a439ddec 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -1,15 +1,11 @@ +import { KubernetesObject } from "kubernetes-fluent-client"; import { - carriedAnnotations, carriedName, carriedNamespace, carriesIgnoredNamespace, declaredKind, - definedAnnotations, definedKind, definedName, - definedNameRegex, - definedNamespaceRegexes, - definedNamespaces, mismatchedAnnotations, mismatchedDeletionTimestamp, mismatchedGroup, @@ -32,12 +28,14 @@ type FilterParams = { }; const prefix = "Ignoring Admission Callback:"; +const bindingKubernetesObjectLogMessage = (subject: string, binding: Binding, kubernetesObject?: KubernetesObject) => + `${prefix} Binding defines ${subject} '${definedName(binding)}' but Object carries '${carriedName(kubernetesObject)}'.`; const createFilter = ( dataSelector: (data: FilterParams) => T1, objectSelector: (data: FilterParams) => T2, - mismatchCheck: (data: T1, obj?: T2) => boolean, - logMessage: (data: T1, obj?: T2) => string, + mismatchCheck: (data: T1, kubernetesObject?: T2) => boolean, + logMessage: (data: T1, kubernetesObject?: T2) => string, ) => { return (data: FilterParams): string => { const dataValue = dataSelector(data); @@ -49,57 +47,51 @@ const createFilter = ( export const mismatchedNameFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedName(binding, obj), - (binding, obj) => - `${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(obj)}'.`, + (binding, kubernetesObject) => mismatchedName(binding, kubernetesObject), + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("name", binding, kubernetesObject), ); export const mismatchedNameRegexFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedNameRegex(binding, obj), - (binding, obj) => - `${prefix} Binding defines name regex '${definedNameRegex(binding)}' but Object carries '${carriedName(obj)}'.`, + (binding, kubernetesObject) => mismatchedNameRegex(binding, kubernetesObject), + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("name regex", binding, kubernetesObject), ); export const mismatchedNamespaceRegexFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedNamespaceRegex(binding, obj), - (binding, obj) => - `${prefix} Binding defines namespace regexes '${JSON.stringify(definedNamespaceRegexes(binding))}' but Object carries '${carriedNamespace(obj)}'.`, + (binding, kubernetesObject) => mismatchedNamespaceRegex(binding, kubernetesObject), + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("namespace regexes", binding, kubernetesObject), ); export const mismatchedNamespaceFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedNamespace(binding, obj), - (binding, obj) => - `${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' but Object carries '${carriedNamespace(obj)}'.`, + (binding, kubernetesObject) => mismatchedNamespace(binding, kubernetesObject), + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("namespaces", binding, kubernetesObject), ); export const mismatchedAnnotationsFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedAnnotations(binding, obj), - (binding, obj) => - `${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, + (binding, kubernetesObject) => mismatchedAnnotations(binding, kubernetesObject), + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("annotations", binding, kubernetesObject), ); export const mismatchedLabelsFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedLabels(binding, obj), - (binding, obj) => - `${prefix} Binding defines labels '${JSON.stringify(definedAnnotations(binding))}' but Object carries '${JSON.stringify(carriedAnnotations(obj))}'.`, + (binding, kubernetesObject) => mismatchedLabels(binding, kubernetesObject), + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("labels", binding, kubernetesObject), ); export const mismatchedDeletionTimestampFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedDeletionTimestamp(binding, obj), + (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), // eslint-disable-next-line @typescript-eslint/no-unused-vars - (binding, obj) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, + (binding, kubernetesObject) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, ); export const mismatchedKindFilter = createFilter( @@ -113,9 +105,10 @@ export const mismatchedKindFilter = createFilter( export const mismatchedGroupFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, obj) => mismatchedGroup(binding, obj), - (binding, obj) => - `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(obj)}'.`, + (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), + // (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("group", binding, kubernetesObject) + (binding, kubernetesObject) => + `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(kubernetesObject)}'.`, ); export const mismatchedVersionFilter = createFilter( @@ -129,17 +122,17 @@ export const mismatchedVersionFilter = createFilter( export const carriesIgnoredNamespacesFilter = createFilter( data => data.ignoredNamespaces, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (ignoreArray, obj) => carriesIgnoredNamespace(ignoreArray, obj), - (ignoreArray, obj) => - `${prefix} Object carries namespace '${carriedNamespace(obj)}' but ignored namespaces include '${JSON.stringify(ignoreArray)}'.`, + (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), + (ignoreArray, kubernetesObject) => + `${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' but ignored namespaces include '${JSON.stringify(ignoreArray)}'.`, ); export const uncarryableNamespaceFilter = createFilter( data => data.capabilityNamespaces, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (capabilityNamespaces, obj) => uncarryableNamespace(capabilityNamespaces, obj), - (capabilityNamespaces, obj) => - `${prefix} Object carries namespace '${carriedNamespace(obj)}' but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`, + (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), + (capabilityNamespaces, kubernetesObject) => + `${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`, ); export const unbindableNamespacesFilter = createFilter( From 6fed3fb315f3245ea42248d8757183a049721ba4 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 11:03:31 -0500 Subject: [PATCH 18/49] Add more standardized log messages --- src/lib/adjudicatorsFilterWrapper.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index 4a439ddec..e13f944b9 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -31,6 +31,12 @@ const prefix = "Ignoring Admission Callback:"; const bindingKubernetesObjectLogMessage = (subject: string, binding: Binding, kubernetesObject?: KubernetesObject) => `${prefix} Binding defines ${subject} '${definedName(binding)}' but Object carries '${carriedName(kubernetesObject)}'.`; +const bindingAdmissionRequestLogMessage = (subject: string, binding: Binding, request?: AdmissionRequest) => + `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; + +const arrayKubernetesObjectLogMessage = (subject: string, array?: string[], kubernetesObject?: KubernetesObject) => + `${prefix} Object carries ${subject} '${carriedNamespace(kubernetesObject)}' but ignored ${subject}s include '${JSON.stringify(array)}'.`; + const createFilter = ( dataSelector: (data: FilterParams) => T1, objectSelector: (data: FilterParams) => T2, @@ -98,15 +104,13 @@ export const mismatchedKindFilter = createFilter( data => data.binding, data => data.request, (binding, request) => mismatchedKind(binding, request), - (binding, request) => - `${prefix} Binding defines kind '${definedKind(binding)}' but Request declares '${declaredKind(request)}'.`, + (binding, request) => bindingAdmissionRequestLogMessage("kind", binding, request), ); export const mismatchedGroupFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), - // (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("group", binding, kubernetesObject) (binding, kubernetesObject) => `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(kubernetesObject)}'.`, ); @@ -115,16 +119,14 @@ export const mismatchedVersionFilter = createFilter( data => data.binding, data => data.request, (binding, request) => mismatchedVersion(binding, request), - (binding, request) => - `${prefix} Binding defines version '${definedKind(binding)}' but Request declares '${declaredKind(request)}'.`, + (binding, request) => bindingAdmissionRequestLogMessage("version", binding, request), ); export const carriesIgnoredNamespacesFilter = createFilter( data => data.ignoredNamespaces, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), - (ignoreArray, kubernetesObject) => - `${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' but ignored namespaces include '${JSON.stringify(ignoreArray)}'.`, + (ignoreArray, kubernetesObject) => arrayKubernetesObjectLogMessage("namespace", ignoreArray, kubernetesObject), ); export const uncarryableNamespaceFilter = createFilter( From 23337da0a41697d49819c1e4c797569d170fa980 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 11:12:38 -0500 Subject: [PATCH 19/49] Apply standard log message to filters with shared contents --- src/lib/adjudicatorsFilterWrapper.ts | 51 ++++++++++++++++------------ 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index e13f944b9..d198fbf80 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -34,9 +34,16 @@ const bindingKubernetesObjectLogMessage = (subject: string, binding: Binding, ku const bindingAdmissionRequestLogMessage = (subject: string, binding: Binding, request?: AdmissionRequest) => `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; -const arrayKubernetesObjectLogMessage = (subject: string, array?: string[], kubernetesObject?: KubernetesObject) => +const ignoreArrayKubernetesObjectLogMessage = ( + subject: string, + array?: string[], + kubernetesObject?: KubernetesObject, +) => `${prefix} Object carries ${subject} '${carriedNamespace(kubernetesObject)}' but ignored ${subject}s include '${JSON.stringify(array)}'.`; +const arrayKubernetesObjectLogMessage = (subject: string, array?: string[], kubernetesObject?: KubernetesObject) => + `${prefix} Object carries ${subject} '${carriedNamespace(kubernetesObject)}' but ${subject}s allowed by Capability are '${JSON.stringify(array)}'.`; + const createFilter = ( dataSelector: (data: FilterParams) => T1, objectSelector: (data: FilterParams) => T2, @@ -92,14 +99,6 @@ export const mismatchedLabelsFilter = createFilter( (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("labels", binding, kubernetesObject), ); -export const mismatchedDeletionTimestampFilter = createFilter( - data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), - // eslint-disable-next-line @typescript-eslint/no-unused-vars - (binding, kubernetesObject) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, -); - export const mismatchedKindFilter = createFilter( data => data.binding, data => data.request, @@ -107,14 +106,6 @@ export const mismatchedKindFilter = createFilter( (binding, request) => bindingAdmissionRequestLogMessage("kind", binding, request), ); -export const mismatchedGroupFilter = createFilter( - data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), - (binding, kubernetesObject) => - `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(kubernetesObject)}'.`, -); - export const mismatchedVersionFilter = createFilter( data => data.binding, data => data.request, @@ -126,7 +117,7 @@ export const carriesIgnoredNamespacesFilter = createFilter( data => data.ignoredNamespaces, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), - (ignoreArray, kubernetesObject) => arrayKubernetesObjectLogMessage("namespace", ignoreArray, kubernetesObject), + (ignoreArray, kubernetesObject) => ignoreArrayKubernetesObjectLogMessage("namespace", ignoreArray, kubernetesObject), ); export const uncarryableNamespaceFilter = createFilter( @@ -134,13 +125,29 @@ export const uncarryableNamespaceFilter = createFilter( data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), (capabilityNamespaces, kubernetesObject) => - `${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`, + arrayKubernetesObjectLogMessage("namespace", capabilityNamespaces, kubernetesObject), ); export const unbindableNamespacesFilter = createFilter( data => data.binding, data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), - (capabilityNamespaces, binding) => uncarryableNamespace(capabilityNamespaces, binding), - (capabilityNamespaces, binding) => - `${prefix} Binding carries namespace '${carriedNamespace(binding)}' but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`, + (binding, request) => uncarryableNamespace(binding, request), + (binding, request) => + `${prefix} Binding carries namespace '${carriedNamespace(request)}' but namespaces allowed by Capability are '${JSON.stringify(binding)}'.`, +); + +export const mismatchedGroupFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), + (binding, kubernetesObject) => + `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(kubernetesObject)}'.`, +); + +export const mismatchedDeletionTimestampFilter = createFilter( + data => data.binding, + data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), + // eslint-disable-next-line @typescript-eslint/no-unused-vars + (binding, kubernetesObject) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, ); From 7456b5d2946a8acbda05e18f85bb157409773b29 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 11:19:09 -0500 Subject: [PATCH 20/49] Extract request selection logic to shared function --- src/lib/adjudicatorsFilterWrapper.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/adjudicatorsFilterWrapper.ts index d198fbf80..68861b3ed 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/adjudicatorsFilterWrapper.ts @@ -44,6 +44,10 @@ const ignoreArrayKubernetesObjectLogMessage = ( const arrayKubernetesObjectLogMessage = (subject: string, array?: string[], kubernetesObject?: KubernetesObject) => `${prefix} Object carries ${subject} '${carriedNamespace(kubernetesObject)}' but ${subject}s allowed by Capability are '${JSON.stringify(array)}'.`; +const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { + return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; +}; + const createFilter = ( dataSelector: (data: FilterParams) => T1, objectSelector: (data: FilterParams) => T2, @@ -59,42 +63,42 @@ const createFilter = ( export const mismatchedNameFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedName(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("name", binding, kubernetesObject), ); export const mismatchedNameRegexFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedNameRegex(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("name regex", binding, kubernetesObject), ); export const mismatchedNamespaceRegexFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedNamespaceRegex(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("namespace regexes", binding, kubernetesObject), ); export const mismatchedNamespaceFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedNamespace(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("namespaces", binding, kubernetesObject), ); export const mismatchedAnnotationsFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedAnnotations(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("annotations", binding, kubernetesObject), ); export const mismatchedLabelsFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedLabels(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("labels", binding, kubernetesObject), ); @@ -115,14 +119,14 @@ export const mismatchedVersionFilter = createFilter( export const carriesIgnoredNamespacesFilter = createFilter( data => data.ignoredNamespaces, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), (ignoreArray, kubernetesObject) => ignoreArrayKubernetesObjectLogMessage("namespace", ignoreArray, kubernetesObject), ); export const uncarryableNamespaceFilter = createFilter( data => data.capabilityNamespaces, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), (capabilityNamespaces, kubernetesObject) => arrayKubernetesObjectLogMessage("namespace", capabilityNamespaces, kubernetesObject), @@ -130,7 +134,7 @@ export const uncarryableNamespaceFilter = createFilter( export const unbindableNamespacesFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, request) => uncarryableNamespace(binding, request), (binding, request) => `${prefix} Binding carries namespace '${carriedNamespace(request)}' but namespaces allowed by Capability are '${JSON.stringify(binding)}'.`, @@ -138,7 +142,7 @@ export const unbindableNamespacesFilter = createFilter( export const mismatchedGroupFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), (binding, kubernetesObject) => `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(kubernetesObject)}'.`, @@ -146,7 +150,7 @@ export const mismatchedGroupFilter = createFilter( export const mismatchedDeletionTimestampFilter = createFilter( data => data.binding, - data => (data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object), + data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), // eslint-disable-next-line @typescript-eslint/no-unused-vars (binding, kubernetesObject) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, From 4d1ff599b567f555299fe144f2b4323266a370b8 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:28:20 -0500 Subject: [PATCH 21/49] Move filtering code to 'filter' directory --- src/lib/{ => filter}/adjudicators.test.ts | 2 +- src/lib/{ => filter}/adjudicators.ts | 4 +- .../{ => filter}/adjudicatorsFilterWrapper.ts | 37 ++++++++++--------- src/lib/{ => filter}/filter.test.ts | 4 +- src/lib/{ => filter}/filter.ts | 2 +- src/lib/helpers.ts | 2 +- src/lib/mutate-processor.ts | 2 +- src/lib/validate-processor.ts | 2 +- 8 files changed, 30 insertions(+), 25 deletions(-) rename src/lib/{ => filter}/adjudicators.test.ts (99%) rename src/lib/{ => filter}/adjudicators.ts (99%) rename src/lib/{ => filter}/adjudicatorsFilterWrapper.ts (78%) rename src/lib/{ => filter}/filter.test.ts (99%) rename src/lib/{ => filter}/filter.ts (97%) diff --git a/src/lib/adjudicators.test.ts b/src/lib/filter/adjudicators.test.ts similarity index 99% rename from src/lib/adjudicators.test.ts rename to src/lib/filter/adjudicators.test.ts index 5817d1b3a..10284c93f 100644 --- a/src/lib/adjudicators.test.ts +++ b/src/lib/filter/adjudicators.test.ts @@ -4,7 +4,7 @@ import { expect, describe, it } from "@jest/globals"; import * as sut from "./adjudicators"; import { KubernetesObject } from "kubernetes-fluent-client"; -import { AdmissionRequest, Binding, DeepPartial, Event, Operation } from "./types"; +import { AdmissionRequest, Binding, DeepPartial, Event, Operation } from "../types"; describe("definesDeletionTimestamp", () => { //[ Binding, result ] diff --git a/src/lib/adjudicators.ts b/src/lib/filter/adjudicators.ts similarity index 99% rename from src/lib/adjudicators.ts rename to src/lib/filter/adjudicators.ts index 36df1fcb6..35fc2c5f6 100644 --- a/src/lib/adjudicators.ts +++ b/src/lib/filter/adjudicators.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -import { Event, Operation } from "./types"; +import { Event, Operation } from "../types"; import { __, allPass, @@ -57,6 +57,8 @@ export const carriesLabels = pipe(carriedLabels, equals({}), not); /* Binding collectors */ + +//TODO: Defnined can go?? export const definesDeletionTimestamp = pipe(binding => binding?.filters?.deletionTimestamp, defaultTo(false)); export const ignoresDeletionTimestamp = complement(definesDeletionTimestamp); diff --git a/src/lib/adjudicatorsFilterWrapper.ts b/src/lib/filter/adjudicatorsFilterWrapper.ts similarity index 78% rename from src/lib/adjudicatorsFilterWrapper.ts rename to src/lib/filter/adjudicatorsFilterWrapper.ts index 68861b3ed..1e4b4e8a3 100644 --- a/src/lib/adjudicatorsFilterWrapper.ts +++ b/src/lib/filter/adjudicatorsFilterWrapper.ts @@ -18,7 +18,7 @@ import { mismatchedVersion, uncarryableNamespace, } from "./adjudicators"; -import { AdmissionRequest, Binding, Operation } from "./types"; +import { AdmissionRequest, Binding, Operation } from "../types"; type FilterParams = { binding: Binding; @@ -27,37 +27,40 @@ type FilterParams = { ignoredNamespaces?: string[]; }; +type FilterInput = Binding | KubernetesObject | AdmissionRequest | string[] | undefined; + const prefix = "Ignoring Admission Callback:"; -const bindingKubernetesObjectLogMessage = (subject: string, binding: Binding, kubernetesObject?: KubernetesObject) => - `${prefix} Binding defines ${subject} '${definedName(binding)}' but Object carries '${carriedName(kubernetesObject)}'.`; -const bindingAdmissionRequestLogMessage = (subject: string, binding: Binding, request?: AdmissionRequest) => +const bindingKubernetesObjectLogMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => + `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + +const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput, request?: FilterInput) => `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; const ignoreArrayKubernetesObjectLogMessage = ( subject: string, - array?: string[], - kubernetesObject?: KubernetesObject, + filterCriteria?: FilterInput, + filterInput?: FilterInput, ) => - `${prefix} Object carries ${subject} '${carriedNamespace(kubernetesObject)}' but ignored ${subject}s include '${JSON.stringify(array)}'.`; + `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ignored ${subject}s include '${JSON.stringify(filterCriteria)}'.`; -const arrayKubernetesObjectLogMessage = (subject: string, array?: string[], kubernetesObject?: KubernetesObject) => - `${prefix} Object carries ${subject} '${carriedNamespace(kubernetesObject)}' but ${subject}s allowed by Capability are '${JSON.stringify(array)}'.`; +const arrayKubernetesObjectLogMessage = (subject: string, filterCriteria?: FilterInput, filterInput?: FilterInput) => + `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; }; -const createFilter = ( - dataSelector: (data: FilterParams) => T1, - objectSelector: (data: FilterParams) => T2, - mismatchCheck: (data: T1, kubernetesObject?: T2) => boolean, - logMessage: (data: T1, kubernetesObject?: T2) => string, +const createFilter = ( + filterInputSelector: (data: FilterParams) => FilterInput, + filterCriteriaSelector: (data: FilterParams) => FilterInput, + mismatchCheck: (filterInput: FilterInput, filterCriteria?: FilterInput) => boolean, + logMessage: (filterInput: FilterInput, filterCriteria?: FilterInput) => string, ) => { return (data: FilterParams): string => { - const dataValue = dataSelector(data); - const objectValue = objectSelector(data); - return mismatchCheck(dataValue, objectValue) ? logMessage(dataValue, objectValue) : ""; + const filterInput = filterInputSelector(data); + const filterCriteria = filterCriteriaSelector(data); + return mismatchCheck(filterInput, filterCriteria) ? logMessage(filterInput, filterCriteria) : ""; }; }; diff --git a/src/lib/filter.test.ts b/src/lib/filter/filter.test.ts similarity index 99% rename from src/lib/filter.test.ts rename to src/lib/filter/filter.test.ts index e2469b585..bc60ad734 100644 --- a/src/lib/filter.test.ts +++ b/src/lib/filter/filter.test.ts @@ -4,9 +4,9 @@ import { expect, describe, it } from "@jest/globals"; import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; import * as fc from "fast-check"; -import { CreatePod, DeletePod } from "../fixtures/loader"; +import { CreatePod, DeletePod } from "../../fixtures/loader"; import { shouldSkipRequest } from "./filter"; -import { AdmissionRequest, Binding, Event } from "./types"; +import { AdmissionRequest, Binding, Event } from "../types"; export const callback = () => undefined; diff --git a/src/lib/filter.ts b/src/lib/filter/filter.ts similarity index 97% rename from src/lib/filter.ts rename to src/lib/filter/filter.ts index 03e939446..95df302f4 100644 --- a/src/lib/filter.ts +++ b/src/lib/filter/filter.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors -import { AdmissionRequest, Binding } from "./types"; +import { AdmissionRequest, Binding } from "../types"; import { carriesIgnoredNamespacesFilter, mismatchedAnnotationsFilter, diff --git a/src/lib/helpers.ts b/src/lib/helpers.ts index b9d68c7b7..1bd580f6c 100644 --- a/src/lib/helpers.ts +++ b/src/lib/helpers.ts @@ -28,7 +28,7 @@ import { mismatchedNamespaceRegex, unbindableNamespaces, uncarryableNamespace, -} from "./adjudicators"; +} from "./filter/adjudicators"; export function matchesRegex(pattern: string, testString: string): boolean { // edge-case diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index eef1f0ead..e1ff31f7b 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -6,7 +6,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; import { Errors } from "./errors"; -import { shouldSkipRequest } from "./filter"; +import { shouldSkipRequest } from "./filter/filter"; import { MutateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; diff --git a/src/lib/validate-processor.ts b/src/lib/validate-processor.ts index c41fa0608..611fa6411 100644 --- a/src/lib/validate-processor.ts +++ b/src/lib/validate-processor.ts @@ -4,7 +4,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; -import { shouldSkipRequest } from "./filter"; +import { shouldSkipRequest } from "./filter/filter"; import { ValidateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; From 0ffdd806d3ba644e8a564a38bf64d61ad1d74ff4 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:31:20 -0500 Subject: [PATCH 22/49] Move FilterChain to own file --- src/lib/filter/filter.ts | 57 +++++++++-------------------------- src/lib/filter/filterChain.ts | 29 ++++++++++++++++++ 2 files changed, 44 insertions(+), 42 deletions(-) create mode 100644 src/lib/filter/filterChain.ts diff --git a/src/lib/filter/filter.ts b/src/lib/filter/filter.ts index 95df302f4..06288b33d 100644 --- a/src/lib/filter/filter.ts +++ b/src/lib/filter/filter.ts @@ -18,33 +18,7 @@ import { unbindableNamespacesFilter, uncarryableNamespaceFilter, } from "./adjudicatorsFilterWrapper"; - -//TODO: Dupe'd declaration -type FilterParams = { - binding: Binding; - request: AdmissionRequest; - capabilityNamespaces: string[]; - ignoredNamespaces?: string[]; -}; -interface Filter { - (data: FilterParams): string; -} - -export class FilterChain { - private filters: Filter[] = []; - - public addFilter(filter: Filter): FilterChain { - this.filters.push(filter); - return this; - } - public execute(data: FilterParams): string { - return this.filters.reduce((result, filter) => { - result += filter(data); - // The result of each filter is passed as a new concatenated string - return result; - }, ""); - } -} +import { FilterChain } from "./filterChain"; /** * shouldSkipRequest determines if a request should be skipped based on the binding filters. @@ -59,23 +33,22 @@ export const shouldSkipRequest = ( capabilityNamespaces: string[], ignoredNamespaces?: string[], ): string => { - // const obj = req.operation === Operation.DELETE ? req.oldObject : req.object; - const filterChain = new FilterChain(); - filterChain.addFilter(mismatchedNameRegexFilter); - filterChain.addFilter(mismatchedNamespaceFilter); - filterChain.addFilter(mismatchedNamespaceRegexFilter); - filterChain.addFilter(uncarryableNamespaceFilter); - filterChain.addFilter(mismatchedDeletionTimestampFilter); - filterChain.addFilter(mismatchedAnnotationsFilter); - filterChain.addFilter(mismatchedLabelsFilter); - filterChain.addFilter(mismatchedKindFilter); - filterChain.addFilter(mismatchedGroupFilter); - filterChain.addFilter(mismatchedVersionFilter); - filterChain.addFilter(mismatchedNameFilter); - filterChain.addFilter(carriesIgnoredNamespacesFilter); - filterChain.addFilter(unbindableNamespacesFilter); + filterChain + .addFilter(mismatchedNameRegexFilter) + .addFilter(mismatchedNamespaceFilter) + .addFilter(mismatchedNamespaceRegexFilter) + .addFilter(uncarryableNamespaceFilter) + .addFilter(mismatchedDeletionTimestampFilter) + .addFilter(mismatchedAnnotationsFilter) + .addFilter(mismatchedLabelsFilter) + .addFilter(mismatchedKindFilter) + .addFilter(mismatchedGroupFilter) + .addFilter(mismatchedVersionFilter) + .addFilter(mismatchedNameFilter) + .addFilter(carriesIgnoredNamespacesFilter) + .addFilter(unbindableNamespacesFilter); const admissionFilterMessage = filterChain.execute({ binding, diff --git a/src/lib/filter/filterChain.ts b/src/lib/filter/filterChain.ts new file mode 100644 index 000000000..d2062f3a8 --- /dev/null +++ b/src/lib/filter/filterChain.ts @@ -0,0 +1,29 @@ +import { AdmissionRequest, Binding } from "../types"; + +//TODO: Dupe'd declaration +type FilterParams = { + binding: Binding; + request: AdmissionRequest; + capabilityNamespaces: string[]; + ignoredNamespaces?: string[]; +}; + +interface Filter { + (data: FilterParams): string; +} + +export class FilterChain { + private filters: Filter[] = []; + + public addFilter(filter: Filter): FilterChain { + this.filters.push(filter); + return this; + } + public execute(data: FilterParams): string { + return this.filters.reduce((result, filter) => { + result += filter(data); + // The result of each filter is passed as a new concatenated string + return result; + }, ""); + } +} From 3584e68b591e39f4577db0acb04e13834c186923 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:34:46 -0500 Subject: [PATCH 23/49] Rename file that only exports one function --- src/lib/filter/filter.test.ts | 2 +- src/lib/filter/{filter.ts => shouldSkipRequest.ts} | 0 src/lib/mutate-processor.ts | 2 +- src/lib/validate-processor.ts | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename src/lib/filter/{filter.ts => shouldSkipRequest.ts} (100%) diff --git a/src/lib/filter/filter.test.ts b/src/lib/filter/filter.test.ts index bc60ad734..d416de3de 100644 --- a/src/lib/filter/filter.test.ts +++ b/src/lib/filter/filter.test.ts @@ -5,7 +5,7 @@ import { expect, describe, it } from "@jest/globals"; import { kind, modelToGroupVersionKind } from "kubernetes-fluent-client"; import * as fc from "fast-check"; import { CreatePod, DeletePod } from "../../fixtures/loader"; -import { shouldSkipRequest } from "./filter"; +import { shouldSkipRequest } from "./shouldSkipRequest"; import { AdmissionRequest, Binding, Event } from "../types"; export const callback = () => undefined; diff --git a/src/lib/filter/filter.ts b/src/lib/filter/shouldSkipRequest.ts similarity index 100% rename from src/lib/filter/filter.ts rename to src/lib/filter/shouldSkipRequest.ts diff --git a/src/lib/mutate-processor.ts b/src/lib/mutate-processor.ts index e1ff31f7b..7f3b5b47f 100644 --- a/src/lib/mutate-processor.ts +++ b/src/lib/mutate-processor.ts @@ -6,7 +6,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; import { Errors } from "./errors"; -import { shouldSkipRequest } from "./filter/filter"; +import { shouldSkipRequest } from "./filter/shouldSkipRequest"; import { MutateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; diff --git a/src/lib/validate-processor.ts b/src/lib/validate-processor.ts index 611fa6411..43571a330 100644 --- a/src/lib/validate-processor.ts +++ b/src/lib/validate-processor.ts @@ -4,7 +4,7 @@ import { kind } from "kubernetes-fluent-client"; import { Capability } from "./capability"; -import { shouldSkipRequest } from "./filter/filter"; +import { shouldSkipRequest } from "./filter/shouldSkipRequest"; import { ValidateResponse } from "./k8s"; import { AdmissionRequest } from "./types"; import Log from "./logger"; From 91896e82a24db372b7721e16444ee39b1b4c27f6 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:44:43 -0500 Subject: [PATCH 24/49] Support multiple logging flows in bindingKubernetesObject Log --- src/lib/filter/adjudicators.ts | 1 - src/lib/filter/adjudicatorsFilterWrapper.ts | 32 ++++------------ src/lib/filter/logMessages.ts | 38 +++++++++++++++++++ ...lter.test.ts => shouldSkipRequest.test.ts} | 0 4 files changed, 46 insertions(+), 25 deletions(-) create mode 100644 src/lib/filter/logMessages.ts rename src/lib/filter/{filter.test.ts => shouldSkipRequest.test.ts} (100%) diff --git a/src/lib/filter/adjudicators.ts b/src/lib/filter/adjudicators.ts index 35fc2c5f6..0c79d8b3b 100644 --- a/src/lib/filter/adjudicators.ts +++ b/src/lib/filter/adjudicators.ts @@ -58,7 +58,6 @@ export const carriesLabels = pipe(carriedLabels, equals({}), not); Binding collectors */ -//TODO: Defnined can go?? export const definesDeletionTimestamp = pipe(binding => binding?.filters?.deletionTimestamp, defaultTo(false)); export const ignoresDeletionTimestamp = complement(definesDeletionTimestamp); diff --git a/src/lib/filter/adjudicatorsFilterWrapper.ts b/src/lib/filter/adjudicatorsFilterWrapper.ts index 1e4b4e8a3..870d6ae7f 100644 --- a/src/lib/filter/adjudicatorsFilterWrapper.ts +++ b/src/lib/filter/adjudicatorsFilterWrapper.ts @@ -1,11 +1,7 @@ import { KubernetesObject } from "kubernetes-fluent-client"; import { - carriedName, carriedNamespace, carriesIgnoredNamespace, - declaredKind, - definedKind, - definedName, mismatchedAnnotations, mismatchedDeletionTimestamp, mismatchedGroup, @@ -19,6 +15,12 @@ import { uncarryableNamespace, } from "./adjudicators"; import { AdmissionRequest, Binding, Operation } from "../types"; +import { + arrayKubernetesObjectLogMessage, + bindingAdmissionRequestLogMessage, + bindingKubernetesObjectLogMessage, + ignoreArrayKubernetesObjectLogMessage, +} from "./logMessages"; type FilterParams = { binding: Binding; @@ -29,24 +31,6 @@ type FilterParams = { type FilterInput = Binding | KubernetesObject | AdmissionRequest | string[] | undefined; -const prefix = "Ignoring Admission Callback:"; - -const bindingKubernetesObjectLogMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => - `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; - -const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput, request?: FilterInput) => - `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; - -const ignoreArrayKubernetesObjectLogMessage = ( - subject: string, - filterCriteria?: FilterInput, - filterInput?: FilterInput, -) => - `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ignored ${subject}s include '${JSON.stringify(filterCriteria)}'.`; - -const arrayKubernetesObjectLogMessage = (subject: string, filterCriteria?: FilterInput, filterInput?: FilterInput) => - `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; - const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; }; @@ -147,10 +131,10 @@ export const mismatchedGroupFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), - (binding, kubernetesObject) => - `${prefix} Binding defines group '${definedKind(binding)}' but Request declares '${declaredKind(kubernetesObject)}'.`, + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("group", binding, kubernetesObject), ); +const prefix = "Ignoring Admission Callback:"; export const mismatchedDeletionTimestampFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts new file mode 100644 index 000000000..a5c97020e --- /dev/null +++ b/src/lib/filter/logMessages.ts @@ -0,0 +1,38 @@ +import { KubernetesObject } from "kubernetes-fluent-client"; +import { AdmissionRequest, Binding } from "../types"; +import { carriedName, carriedNamespace, definedName } from "./adjudicators"; + +type FilterInput = Binding | KubernetesObject | AdmissionRequest | string[] | undefined; + +const prefix = "Ignoring Admission Callback:"; + +export const bindingKubernetesObjectLogMessage = ( + subject: string, + filterInput: FilterInput, + filterCriteria?: FilterInput, +): string => { + let logMessage = ""; + if (subject === "group") { + logMessage = `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + } else { + logMessage = `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + } + return logMessage; +}; + +export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput, request?: FilterInput) => + `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; + +export const ignoreArrayKubernetesObjectLogMessage = ( + subject: string, + filterCriteria?: FilterInput, + filterInput?: FilterInput, +) => + `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ignored ${subject}s include '${JSON.stringify(filterCriteria)}'.`; + +export const arrayKubernetesObjectLogMessage = ( + subject: string, + filterCriteria?: FilterInput, + filterInput?: FilterInput, +) => + `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; diff --git a/src/lib/filter/filter.test.ts b/src/lib/filter/shouldSkipRequest.test.ts similarity index 100% rename from src/lib/filter/filter.test.ts rename to src/lib/filter/shouldSkipRequest.test.ts From 7abc3d07b43c4e74a706808860094b22f76b12e7 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:46:48 -0500 Subject: [PATCH 25/49] Rename file for clarity --- .../{adjudicatorsFilterWrapper.ts => filterRulesWithLogs.ts} | 0 src/lib/filter/shouldSkipRequest.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename src/lib/filter/{adjudicatorsFilterWrapper.ts => filterRulesWithLogs.ts} (100%) diff --git a/src/lib/filter/adjudicatorsFilterWrapper.ts b/src/lib/filter/filterRulesWithLogs.ts similarity index 100% rename from src/lib/filter/adjudicatorsFilterWrapper.ts rename to src/lib/filter/filterRulesWithLogs.ts diff --git a/src/lib/filter/shouldSkipRequest.ts b/src/lib/filter/shouldSkipRequest.ts index 06288b33d..f30e89110 100644 --- a/src/lib/filter/shouldSkipRequest.ts +++ b/src/lib/filter/shouldSkipRequest.ts @@ -17,7 +17,7 @@ import { mismatchedVersionFilter, unbindableNamespacesFilter, uncarryableNamespaceFilter, -} from "./adjudicatorsFilterWrapper"; +} from "./filterRulesWithLogs"; import { FilterChain } from "./filterChain"; /** From e55ee47c74b924cdfda5490db7d0ac1c712d22cf Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:48:31 -0500 Subject: [PATCH 26/49] Move filter types to types --- .../{filterRulesWithLogs.ts => filtersWithLogs.ts} | 11 +---------- src/lib/filter/logMessages.ts | 5 +---- src/lib/filter/shouldSkipRequest.ts | 2 +- src/lib/types.ts | 9 +++++++++ 4 files changed, 12 insertions(+), 15 deletions(-) rename src/lib/filter/{filterRulesWithLogs.ts => filtersWithLogs.ts} (94%) diff --git a/src/lib/filter/filterRulesWithLogs.ts b/src/lib/filter/filtersWithLogs.ts similarity index 94% rename from src/lib/filter/filterRulesWithLogs.ts rename to src/lib/filter/filtersWithLogs.ts index 870d6ae7f..2f988cf98 100644 --- a/src/lib/filter/filterRulesWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -14,7 +14,7 @@ import { mismatchedVersion, uncarryableNamespace, } from "./adjudicators"; -import { AdmissionRequest, Binding, Operation } from "../types"; +import { FilterInput, FilterParams, Operation } from "../types"; import { arrayKubernetesObjectLogMessage, bindingAdmissionRequestLogMessage, @@ -22,15 +22,6 @@ import { ignoreArrayKubernetesObjectLogMessage, } from "./logMessages"; -type FilterParams = { - binding: Binding; - request: AdmissionRequest; - capabilityNamespaces: string[]; - ignoredNamespaces?: string[]; -}; - -type FilterInput = Binding | KubernetesObject | AdmissionRequest | string[] | undefined; - const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; }; diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index a5c97020e..41502eeea 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -1,9 +1,6 @@ -import { KubernetesObject } from "kubernetes-fluent-client"; -import { AdmissionRequest, Binding } from "../types"; +import { FilterInput } from "../types"; import { carriedName, carriedNamespace, definedName } from "./adjudicators"; -type FilterInput = Binding | KubernetesObject | AdmissionRequest | string[] | undefined; - const prefix = "Ignoring Admission Callback:"; export const bindingKubernetesObjectLogMessage = ( diff --git a/src/lib/filter/shouldSkipRequest.ts b/src/lib/filter/shouldSkipRequest.ts index f30e89110..85d12c40d 100644 --- a/src/lib/filter/shouldSkipRequest.ts +++ b/src/lib/filter/shouldSkipRequest.ts @@ -17,7 +17,7 @@ import { mismatchedVersionFilter, unbindableNamespacesFilter, uncarryableNamespaceFilter, -} from "./filterRulesWithLogs"; +} from "./filtersWithLogs"; import { FilterChain } from "./filterChain"; /** diff --git a/src/lib/types.ts b/src/lib/types.ts index fed2b078a..6e12764a9 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -375,3 +375,12 @@ export interface GroupVersionResource { readonly version: string; readonly resource: string; } + +export type FilterParams = { + binding: Binding; + request: AdmissionRequest; + capabilityNamespaces: string[]; + ignoredNamespaces?: string[]; +}; + +export type FilterInput = Binding | KubernetesObject | AdmissionRequest | string[] | undefined; From 093dc0c3d6c9b58f54167fa6b3f42acdb4ddae82 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:57:05 -0500 Subject: [PATCH 27/49] Support all logging with defined helper functions --- src/lib/filter/filtersWithLogs.ts | 16 ++++++---------- src/lib/filter/logMessages.ts | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index 2f988cf98..40463b956 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -1,6 +1,5 @@ import { KubernetesObject } from "kubernetes-fluent-client"; import { - carriedNamespace, carriesIgnoredNamespace, mismatchedAnnotations, mismatchedDeletionTimestamp, @@ -27,15 +26,15 @@ const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined = }; const createFilter = ( - filterInputSelector: (data: FilterParams) => FilterInput, - filterCriteriaSelector: (data: FilterParams) => FilterInput, - mismatchCheck: (filterInput: FilterInput, filterCriteria?: FilterInput) => boolean, + filterInputSelector: (data: FilterParams) => FilterInput, // FilterInput is unvalidated + filterCriteriaSelector: (data: FilterParams) => FilterInput, // FilterCriteria adjudicates FilterInput + filterCheck: (filterInput: FilterInput, filterCriteria?: FilterInput) => boolean, // Adjudicates FilterInput based upon FilterCriteria logMessage: (filterInput: FilterInput, filterCriteria?: FilterInput) => string, ) => { return (data: FilterParams): string => { const filterInput = filterInputSelector(data); const filterCriteria = filterCriteriaSelector(data); - return mismatchCheck(filterInput, filterCriteria) ? logMessage(filterInput, filterCriteria) : ""; + return filterCheck(filterInput, filterCriteria) ? logMessage(filterInput, filterCriteria) : ""; }; }; @@ -114,8 +113,7 @@ export const unbindableNamespacesFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, request) => uncarryableNamespace(binding, request), - (binding, request) => - `${prefix} Binding carries namespace '${carriedNamespace(request)}' but namespaces allowed by Capability are '${JSON.stringify(binding)}'.`, + (binding, request) => bindingAdmissionRequestLogMessage("namespace", binding, request), ); export const mismatchedGroupFilter = createFilter( @@ -125,11 +123,9 @@ export const mismatchedGroupFilter = createFilter( (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("group", binding, kubernetesObject), ); -const prefix = "Ignoring Admission Callback:"; export const mismatchedDeletionTimestampFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), - // eslint-disable-next-line @typescript-eslint/no-unused-vars - (binding, kubernetesObject) => `${prefix} Binding defines deletionTimestamp but Object does not carry it.`, + (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("deletionTimestamp", binding, kubernetesObject), ); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index 41502eeea..92bc71d92 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -11,14 +11,24 @@ export const bindingKubernetesObjectLogMessage = ( let logMessage = ""; if (subject === "group") { logMessage = `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + } else if (subject === "deletionTimestamp") { + logMessage = `${prefix} Binding defines ${subject} but Object does not carry it.`; } else { logMessage = `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; } return logMessage; }; -export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput, request?: FilterInput) => - `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; +export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput, request?: FilterInput) => { + let logMessage = ""; + + if (subject === "namespace") { + logMessage = `${prefix} Binding carries ${subject} '${definedName(binding)}' but namespaces allowed by Capability are '${carriedName(request)}'.`; + } else { + logMessage = `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; + } + return logMessage; +}; export const ignoreArrayKubernetesObjectLogMessage = ( subject: string, From fc273b06f9017775bb944875452988cf23161010 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:57:48 -0500 Subject: [PATCH 28/49] Remove complexity suppression --- src/lib/filter/shouldSkipRequest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/filter/shouldSkipRequest.ts b/src/lib/filter/shouldSkipRequest.ts index 85d12c40d..c10aadc4e 100644 --- a/src/lib/filter/shouldSkipRequest.ts +++ b/src/lib/filter/shouldSkipRequest.ts @@ -1,4 +1,3 @@ -/* eslint-disable complexity */ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors From cde50947820a8e0dbf30c2e4c78c2fdcbf5e5804 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 14:59:04 -0500 Subject: [PATCH 29/49] Remove TODO --- src/lib/filter/filterChain.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/lib/filter/filterChain.ts b/src/lib/filter/filterChain.ts index d2062f3a8..d69c099dd 100644 --- a/src/lib/filter/filterChain.ts +++ b/src/lib/filter/filterChain.ts @@ -1,12 +1,4 @@ -import { AdmissionRequest, Binding } from "../types"; - -//TODO: Dupe'd declaration -type FilterParams = { - binding: Binding; - request: AdmissionRequest; - capabilityNamespaces: string[]; - ignoredNamespaces?: string[]; -}; +import { FilterParams } from "../types"; interface Filter { (data: FilterParams): string; From 541cebb9827dbcf0c707741b89b198fad364fa71 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 15:05:19 -0500 Subject: [PATCH 30/49] Reorganize tests --- src/lib/filter/shouldSkipRequest.test.ts | 527 ++++++++++++----------- 1 file changed, 265 insertions(+), 262 deletions(-) diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index d416de3de..d97ab5aa7 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -105,194 +105,280 @@ describe("Property-Based Testing shouldSkipRequest", () => { }); }); -it("create: should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, - ); -}); +describe("when a pod is created", () => { + it("should reject when regex name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + ); + }); -it("create: should not reject when regex name does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); -}); + it("should not reject when regex name does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); + }); -it("delete: should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, - ); -}); + it("should not reject when regex namespace does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^helm"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); + }); -it("delete: should not reject when regex name does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); + it("should reject when regex namespace does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^argo"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, + ); + }); + it("should not reject when namespace is not ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch(""); + }); + it("should reject when namespace is ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + ); + }); }); -it("create: should not reject when regex namespace does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^helm"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); -}); +describe("when a pod is deleted", () => { + it("should reject when regex name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + ); + }); -it("create: should reject when regex namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, - ); -}); + it("should not reject when regex name does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); + }); -it("delete: should reject when regex namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, - ); -}); + it("should reject when regex namespace does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^argo"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, + ); + }); -it("delete: should not reject when regex namespace does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^helm"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toBe(""); -}); + it("should not reject when regex namespace does match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: ["^helm"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toBe(""); + }); -it("delete: should reject when name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "bleh", - namespaces: [], - regexNamespaces: [], - regexName: "^not-cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name '.*' but Object carries '.*'./, - ); + it("should reject when name does not match", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "bleh", + namespaces: [], + regexNamespaces: [], + regexName: "^not-cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name '.*' but Object carries '.*'./, + ); + }); + + it("should reject when namespace is ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( + /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + ); + }); + + it("should not reject when namespace is not ignored", () => { + const binding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }, + callback, + }; + const pod = DeletePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch(""); + }); }); it("should reject when kind does not match", () => { @@ -689,86 +775,3 @@ it("should reject when deletionTimestamp is not present on pod", () => { /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, ); }); - -it("create: should not reject when namespace is not ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch(""); -}); -it("create: should reject when namespace is ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, - ); -}); - -it("delete: should reject when namespace is ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, - ); -}); - -it("delete: should not reject when namespace is not ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; - const pod = DeletePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch(""); -}); From 0b09f22c920cd1f800bae01f9097ccc85ee649af Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 15:09:21 -0500 Subject: [PATCH 31/49] Create default test inputs --- src/lib/filter/shouldSkipRequest.test.ts | 39 ++++++++++++------------ 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index d97ab5aa7..426f279ef 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -12,7 +12,7 @@ export const callback = () => undefined; export const podKind = modelToGroupVersionKind(kind.Pod.name); -describe("Fuzzing shouldSkipRequest", () => { +describe("when fuzzing shouldSkipRequest", () => { it("should handle random inputs without crashing", () => { fc.assert( fc.property( @@ -57,9 +57,7 @@ describe("Fuzzing shouldSkipRequest", () => { { numRuns: 100 }, ); }); -}); -describe("Property-Based Testing shouldSkipRequest", () => { it("should only skip requests that do not match the binding criteria", () => { fc.assert( fc.property( @@ -105,25 +103,28 @@ describe("Property-Based Testing shouldSkipRequest", () => { }); }); +describe("when checking specific properties of shouldSkipRequest()", () => {}); + describe("when a pod is created", () => { + const defaultFilters = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }; + const defaultBinding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: defaultFilters, + callback, + }; it("should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, - }; const pod = CreatePod(); - expect(shouldSkipRequest(binding, pod, [])).toMatch( + expect(shouldSkipRequest(defaultBinding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, ); }); From 535d4500bbddf94438007ab90f1d4244770274cb Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 15:17:52 -0500 Subject: [PATCH 32/49] Consolidate test setup --- src/lib/filter/shouldSkipRequest.test.ts | 274 ++++++++++------------- 1 file changed, 124 insertions(+), 150 deletions(-) diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index 426f279ef..4ce4ae56d 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -130,101 +130,77 @@ describe("when a pod is created", () => { }); it("should not reject when regex name does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + const binding = { ...defaultBinding, filters: mismatchedFilter }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should not reject when regex namespace does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^helm"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: ["^helm"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + + const binding = { ...defaultBinding, filters: mismatchedFilter }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when regex namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: ["^argo"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + const binding = { ...defaultBinding, filters: mismatchedFilter }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, ); }); it("should not reject when namespace is not ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + const binding = { ...defaultBinding, mismatchedFilter }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch(""); }); it("should reject when namespace is ignored", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + const binding = { ...defaultBinding, mismatchedFilter }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, @@ -233,22 +209,33 @@ describe("when a pod is created", () => { }); describe("when a pod is deleted", () => { + const defaultFilters = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }; + const defaultBinding = { + model: kind.Pod, + event: Event.Any, + kind: podKind, + filters: defaultFilters, + callback, + }; it("should reject when regex name does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^default$", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + const binding = { ...defaultBinding, filters: mismatchedFilter }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, @@ -256,40 +243,33 @@ describe("when a pod is deleted", () => { }); it("should not reject when regex name does match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "^cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, }; + const binding = { ...defaultBinding, filters: mismatchedFilter }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when regex namespace does not match", () => { + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: ["^argo"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + ...defaultBinding, + filters: mismatchedFilter, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( @@ -298,40 +278,36 @@ describe("when a pod is deleted", () => { }); it("should not reject when regex namespace does match", () => { + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: ["^helm"], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: ["^helm"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + ...defaultBinding, + filters: mismatchedFilter, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when name does not match", () => { + const mismatchedFilter = { + name: "bleh", + namespaces: [], + regexNamespaces: [], + regexName: "^not-cool", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "bleh", - namespaces: [], - regexNamespaces: [], - regexName: "^not-cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + ...defaultBinding, + filters: mismatchedFilter, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( @@ -340,20 +316,18 @@ describe("when a pod is deleted", () => { }); it("should reject when namespace is ignored", () => { + const mismatchedFilter = { + name: "", + namespaces: [], + regexNamespaces: [], + regexName: "", + labels: {}, + annotations: {}, + deletionTimestamp: false, + }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, - callback, + ...defaultBinding, + mismatchedFilter, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( From 69f5a45d4c20a1a174f93a3571561a957183ae4b Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Fri, 25 Oct 2024 15:45:01 -0500 Subject: [PATCH 33/49] Consolidate more test setup --- src/lib/filter/shouldSkipRequest.test.ts | 511 +++++++---------------- 1 file changed, 149 insertions(+), 362 deletions(-) diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index 4ce4ae56d..d579e54ee 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -12,6 +12,23 @@ export const callback = () => undefined; export const podKind = modelToGroupVersionKind(kind.Pod.name); +const defaultFilters = { + annotations: {}, + deletionTimestamp: false, + labels: {}, + name: "", + namespaces: [], + regexName: "^default$", + regexNamespaces: [], +}; +const defaultBinding = { + callback, + event: Event.Any, + filters: defaultFilters, + kind: podKind, + model: kind.Pod, +}; + describe("when fuzzing shouldSkipRequest", () => { it("should handle random inputs without crashing", () => { fc.assert( @@ -106,22 +123,6 @@ describe("when fuzzing shouldSkipRequest", () => { describe("when checking specific properties of shouldSkipRequest()", () => {}); describe("when a pod is created", () => { - const defaultFilters = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const defaultBinding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: defaultFilters, - callback, - }; it("should reject when regex name does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(defaultBinding, pod, [])).toMatch( @@ -130,77 +131,41 @@ describe("when a pod is created", () => { }); it("should not reject when regex name does match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const binding = { ...defaultBinding, filters: mismatchedFilter }; + const filters = { ...defaultFilters, regexName: "^cool" }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should not reject when regex namespace does match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], + const filters = { + ...defaultFilters, regexNamespaces: ["^helm"], regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, }; - const binding = { ...defaultBinding, filters: mismatchedFilter }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when regex namespace does not match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const binding = { ...defaultBinding, filters: mismatchedFilter }; + const filters = { ...defaultFilters, regexNamespaces: ["^argo"] }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, ); }); it("should not reject when namespace is not ignored", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const binding = { ...defaultBinding, mismatchedFilter }; + const filters = { ...defaultFilters, regexName: "" }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch(""); }); it("should reject when namespace is ignored", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const binding = { ...defaultBinding, mismatchedFilter }; + const filters = { ...defaultFilters, regexName: "" }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, @@ -209,33 +174,9 @@ describe("when a pod is created", () => { }); describe("when a pod is deleted", () => { - const defaultFilters = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const defaultBinding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: defaultFilters, - callback, - }; it("should reject when regex name does not match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^default$", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const binding = { ...defaultBinding, filters: mismatchedFilter }; + const filters = { ...defaultFilters, regexName: "^default$" }; + const binding = { ...defaultBinding, filters }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, @@ -243,33 +184,17 @@ describe("when a pod is deleted", () => { }); it("should not reject when regex name does match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "^cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; - const binding = { ...defaultBinding, filters: mismatchedFilter }; + const filters = { ...defaultFilters, regexName: "^cool" }; + const binding = { ...defaultBinding, filters }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when regex namespace does not match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: ["^argo"], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; + const filters = { ...defaultFilters, regexNamespaces: ["^argo"] }; const binding = { ...defaultBinding, - filters: mismatchedFilter, + filters, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( @@ -278,9 +203,8 @@ describe("when a pod is deleted", () => { }); it("should not reject when regex namespace does match", () => { - const mismatchedFilter = { - name: "", - namespaces: [], + const filters = { + ...defaultFilters, regexNamespaces: ["^helm"], regexName: "", labels: {}, @@ -289,25 +213,17 @@ describe("when a pod is deleted", () => { }; const binding = { ...defaultBinding, - filters: mismatchedFilter, + filters, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when name does not match", () => { - const mismatchedFilter = { - name: "bleh", - namespaces: [], - regexNamespaces: [], - regexName: "^not-cool", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; + const filters = { ...defaultFilters, name: "bleh", regexName: "^not-cool" }; const binding = { ...defaultBinding, - filters: mismatchedFilter, + filters, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( @@ -316,18 +232,10 @@ describe("when a pod is deleted", () => { }); it("should reject when namespace is ignored", () => { - const mismatchedFilter = { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }; + const filters = { ...defaultFilters, namespaces: [] }; const binding = { ...defaultBinding, - mismatchedFilter, + filters, }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( @@ -336,19 +244,10 @@ describe("when a pod is deleted", () => { }); it("should not reject when namespace is not ignored", () => { + const filters = { ...defaultFilters, regexName: "" }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, + ...defaultBinding, + filters, callback, }; const pod = DeletePod(); @@ -357,23 +256,15 @@ describe("when a pod is deleted", () => { }); it("should reject when kind does not match", () => { + const filters = { ...defaultFilters, regexName: "" }; const binding = { - model: kind.Pod, - event: Event.Any, + ...defaultBinding, kind: { group: "", version: "v1", kind: "Nope", }, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: {}, - annotations: {}, - deletionTimestamp: false, - }, + filters, callback, }; const pod = CreatePod(); @@ -384,23 +275,15 @@ it("should reject when kind does not match", () => { }); it("should reject when group does not match", () => { + const filters = { ...defaultFilters, regexName: "" }; const binding = { - model: kind.Pod, - event: Event.Any, + ...defaultBinding, kind: { group: "Nope", version: "v1", kind: "Pod", }, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, + filters, callback, }; const pod = CreatePod(); @@ -411,23 +294,15 @@ it("should reject when group does not match", () => { }); it("should reject when version does not match", () => { + const filters = { ...defaultFilters, regexName: "" }; const binding = { - model: kind.Pod, - event: Event.Any, + ...defaultBinding, kind: { group: "", version: "Nope", kind: "Pod", }, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, + filters, callback, }; const pod = CreatePod(); @@ -438,66 +313,27 @@ it("should reject when version does not match", () => { }); it("should allow when group, version, and kind match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; + const filters = { ...defaultFilters, regexName: "" }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should allow when kind match and others are empty", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: { - group: "", - version: "", - kind: "Pod", - }, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; + const filters = { ...defaultFilters, regexName: "" }; + + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toBe(""); }); it("should reject when the capability namespace does not match", () => { + const filters = { ...defaultFilters }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -507,21 +343,8 @@ it("should reject when the capability namespace does not match", () => { }); it("should reject when namespace does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: ["bleh"], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, - }; + const filters = { ...defaultFilters, namespaces: ["bleh"] }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( @@ -530,20 +353,18 @@ it("should reject when namespace does not match", () => { }); it("should allow when namespace is match", () => { + const filters = { + ...defaultFilters, + namespaces: ["helm-releasename", "unicorn", "things"], + labels: {}, + annotations: {}, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }; const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: ["helm-releasename", "unicorn", "things"], - labels: {}, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", - }, - callback, + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -551,22 +372,15 @@ it("should allow when namespace is match", () => { }); it("should reject when label does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: { - foo: "bar", - }, - annotations: {}, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", + const filters = { + ...defaultFilters, + labels: { + foo: "bar", }, - callback, + }; + const binding = { + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -576,23 +390,18 @@ it("should reject when label does not match", () => { }); it("should allow when label is match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - deletionTimestamp: false, - namespaces: [], - regexNamespaces: [], - regexName: "", - labels: { - foo: "bar", - test: "test1", - }, - annotations: {}, + const filters = { + ...defaultFilters, + regexName: "", + labels: { + foo: "bar", + test: "test1", }, - callback, + annotations: {}, + }; + const binding = { + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -607,22 +416,15 @@ it("should allow when label is match", () => { }); it("should reject when annotation does not match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: { - foo: "bar", - }, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", + const filters = { + ...defaultFilters, + annotations: { + foo: "bar", }, - callback, + }; + const binding = { + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -632,23 +434,21 @@ it("should reject when annotation does not match", () => { }); it("should allow when annotation is match", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - annotations: { - foo: "bar", - test: "test1", - }, - deletionTimestamp: false, - regexNamespaces: [], - regexName: "", + const filters = { + name: "", + namespaces: [], + labels: {}, + annotations: { + foo: "bar", + test: "test1", }, - callback, + deletionTimestamp: false, + regexNamespaces: [], + regexName: "", + }; + const binding = { + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -663,22 +463,19 @@ it("should allow when annotation is match", () => { }); it("should use `oldObject` when the operation is `DELETE`", () => { - const binding = { - model: kind.Pod, - event: Event.Delete, - kind: podKind, - filters: { - name: "", - namespaces: [], - regexNamespaces: [], - regexName: "", - deletionTimestamp: false, - labels: { - "test-op": "delete", - }, - annotations: {}, + const filters = { + ...defaultFilters, + regexNamespaces: [], + regexName: "", + deletionTimestamp: false, + labels: { + "test-op": "delete", }, - callback, + annotations: {}, + }; + const binding = { + ...defaultBinding, + filters, }; const pod = DeletePod(); @@ -687,23 +484,21 @@ it("should use `oldObject` when the operation is `DELETE`", () => { }); it("should allow when deletionTimestamp is present on pod", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - regexNamespaces: [], - regexName: "", - annotations: { - foo: "bar", - test: "test1", - }, - deletionTimestamp: true, + const filters = { + name: "", + namespaces: [], + labels: {}, + regexNamespaces: [], + regexName: "", + annotations: { + foo: "bar", + test: "test1", }, - callback, + deletionTimestamp: true, + }; + const binding = { + ...defaultBinding, + filters, }; const pod = CreatePod(); @@ -719,24 +514,16 @@ it("should allow when deletionTimestamp is present on pod", () => { }); it("should reject when deletionTimestamp is not present on pod", () => { - const binding = { - model: kind.Pod, - event: Event.Any, - kind: podKind, - filters: { - name: "", - namespaces: [], - labels: {}, - regexNamespaces: [], - regexName: "", - annotations: { - foo: "bar", - test: "test1", - }, - deletionTimestamp: true, + const filters = { + ...defaultFilters, + regexName: "", + annotations: { + foo: "bar", + test: "test1", }, - callback, + deletionTimestamp: true, }; + const binding = { ...defaultBinding, filters }; const pod = CreatePod(); pod.object.metadata = pod.object.metadata || {}; From 28fda9fada19c140c464e70ff3659e79e5348109 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Mon, 28 Oct 2024 15:18:28 -0500 Subject: [PATCH 34/49] Note some TODO items --- src/lib/filter/filterChain.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/filter/filterChain.ts b/src/lib/filter/filterChain.ts index d69c099dd..93cdd50de 100644 --- a/src/lib/filter/filterChain.ts +++ b/src/lib/filter/filterChain.ts @@ -12,6 +12,8 @@ export class FilterChain { return this; } public execute(data: FilterParams): string { + //TODO: Break on first "failure"? + //TODO: Dev flag for detailed logging? Check some envar? return this.filters.reduce((result, filter) => { result += filter(data); // The result of each filter is passed as a new concatenated string From 007baf4c48398b07b9b48d643ba498dda4f89857 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 11:26:10 -0500 Subject: [PATCH 35/49] Add missing filters --- src/lib/filter/filtersWithLogs.ts | 24 +++++++++++++++++++++++- src/lib/filter/shouldSkipRequest.ts | 26 +++++++++++++++----------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index 0a11a27dd..dbbfd5eb2 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -1,8 +1,12 @@ import { KubernetesObject } from "kubernetes-fluent-client"; import { carriesIgnoredNamespace, + declaredOperation, + definedEvent, + misboundDeleteWithDeletionTimestamp, mismatchedAnnotations, mismatchedDeletionTimestamp, + mismatchedEvent, mismatchedGroup, mismatchedKind, mismatchedLabels, @@ -95,7 +99,7 @@ export const mismatchedVersionFilter = createFilter( (binding, request) => bindingAdmissionRequestLogMessage("version", binding, request), ); -export const carriesIgnoredNamespacesFilter = createFilter( +export const carriesIgnoredNamespaceFilter = createFilter( data => data.ignoredNamespaces, data => getAdmissionRequest(data), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), @@ -130,3 +134,21 @@ export const mismatchedDeletionTimestampFilter = createFilter( (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("deletionTimestamp", binding, kubernetesObject), ); + +export const misboundDeleteWithDeletionTimestampFilter = createFilter( + data => data.binding, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + data => undefined, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + (binding, unused) => misboundDeleteWithDeletionTimestamp(binding), + // eslint-disable-next-line @typescript-eslint/no-unused-vars + (binding, unused) => `Cannot use deletionTimestamp filter on a DELETE operation.`, +); + +export const mismatchedEventFilter = createFilter( + data => data.binding, + data => data.request, + (binding, request) => mismatchedEvent(binding, request), + (binding, request) => + `Binding defines event '${definedEvent(binding)}' but Request declares '${declaredOperation(request)}'.`, +); diff --git a/src/lib/filter/shouldSkipRequest.ts b/src/lib/filter/shouldSkipRequest.ts index c10aadc4e..63f0cf0ef 100644 --- a/src/lib/filter/shouldSkipRequest.ts +++ b/src/lib/filter/shouldSkipRequest.ts @@ -3,9 +3,11 @@ import { AdmissionRequest, Binding } from "../types"; import { - carriesIgnoredNamespacesFilter, + carriesIgnoredNamespaceFilter, + misboundDeleteWithDeletionTimestampFilter, mismatchedAnnotationsFilter, mismatchedDeletionTimestampFilter, + mismatchedEventFilter, mismatchedGroupFilter, mismatchedKindFilter, mismatchedLabelsFilter, @@ -35,19 +37,21 @@ export const shouldSkipRequest = ( const filterChain = new FilterChain(); filterChain - .addFilter(mismatchedNameRegexFilter) - .addFilter(mismatchedNamespaceFilter) - .addFilter(mismatchedNamespaceRegexFilter) - .addFilter(uncarryableNamespaceFilter) + .addFilter(misboundDeleteWithDeletionTimestampFilter) .addFilter(mismatchedDeletionTimestampFilter) - .addFilter(mismatchedAnnotationsFilter) - .addFilter(mismatchedLabelsFilter) - .addFilter(mismatchedKindFilter) + .addFilter(mismatchedEventFilter) + .addFilter(mismatchedNameFilter) .addFilter(mismatchedGroupFilter) .addFilter(mismatchedVersionFilter) - .addFilter(mismatchedNameFilter) - .addFilter(carriesIgnoredNamespacesFilter) - .addFilter(unbindableNamespacesFilter); + .addFilter(mismatchedKindFilter) + .addFilter(unbindableNamespacesFilter) + .addFilter(uncarryableNamespaceFilter) + .addFilter(mismatchedNamespaceFilter) + .addFilter(mismatchedLabelsFilter) + .addFilter(mismatchedAnnotationsFilter) + .addFilter(mismatchedNamespaceRegexFilter) + .addFilter(mismatchedNameRegexFilter) + .addFilter(carriesIgnoredNamespaceFilter); const admissionFilterMessage = filterChain.execute({ binding, From 632a84ab0b5549139a54495eacfc4a3f16cf0eb2 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:12:31 -0500 Subject: [PATCH 36/49] Ignore .DS_Store everywhere --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 2bd8d1c73..55f8e93fd 100644 --- a/.gitignore +++ b/.gitignore @@ -25,7 +25,7 @@ pepr-test-module pepr-upgrade-test *.tgz coverage/ -src/.DS_Store +.DS_Store # Test binary, build with `go test -c` *.test From 3903fee481a8b1dc4484a7b6d8387608c3157352 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:12:57 -0500 Subject: [PATCH 37/49] Stop filtering after first failure --- src/lib/filter/filterChain.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/lib/filter/filterChain.ts b/src/lib/filter/filterChain.ts index 93cdd50de..0272e9ab3 100644 --- a/src/lib/filter/filterChain.ts +++ b/src/lib/filter/filterChain.ts @@ -12,12 +12,13 @@ export class FilterChain { return this; } public execute(data: FilterParams): string { - //TODO: Break on first "failure"? - //TODO: Dev flag for detailed logging? Check some envar? - return this.filters.reduce((result, filter) => { + let result = ""; + for (const filter of this.filters) { result += filter(data); - // The result of each filter is passed as a new concatenated string - return result; - }, ""); + if (result !== "") { + break; + } + } + return result; } } From db498822f023d9c17a44d3bb89ef220ada8beecb Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:13:20 -0500 Subject: [PATCH 38/49] Refactor log message generation --- src/lib/filter/filtersWithLogs.ts | 12 ++--- src/lib/filter/logMessages.ts | 64 ++++++++++++++---------- src/lib/filter/shouldSkipRequest.test.ts | 2 +- 3 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index dbbfd5eb2..f1f56d740 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -1,8 +1,6 @@ import { KubernetesObject } from "kubernetes-fluent-client"; import { carriesIgnoredNamespace, - declaredOperation, - definedEvent, misboundDeleteWithDeletionTimestamp, mismatchedAnnotations, mismatchedDeletionTimestamp, @@ -23,7 +21,7 @@ import { arrayKubernetesObjectLogMessage, bindingAdmissionRequestLogMessage, bindingKubernetesObjectLogMessage, - ignoreArrayKubernetesObjectLogMessage, + bindingLogMessage, } from "./logMessages"; const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { @@ -103,7 +101,8 @@ export const carriesIgnoredNamespaceFilter = createFilter( data => data.ignoredNamespaces, data => getAdmissionRequest(data), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), - (ignoreArray, kubernetesObject) => ignoreArrayKubernetesObjectLogMessage("namespace", ignoreArray, kubernetesObject), + (ignoreArray, kubernetesObject) => + arrayKubernetesObjectLogMessage("ignored namespaces", ignoreArray, kubernetesObject), ); export const uncarryableNamespaceFilter = createFilter( @@ -142,13 +141,12 @@ export const misboundDeleteWithDeletionTimestampFilter = createFilter( // eslint-disable-next-line @typescript-eslint/no-unused-vars (binding, unused) => misboundDeleteWithDeletionTimestamp(binding), // eslint-disable-next-line @typescript-eslint/no-unused-vars - (binding, unused) => `Cannot use deletionTimestamp filter on a DELETE operation.`, + (binding, unused) => bindingLogMessage("deletionTimestamp"), ); export const mismatchedEventFilter = createFilter( data => data.binding, data => data.request, (binding, request) => mismatchedEvent(binding, request), - (binding, request) => - `Binding defines event '${definedEvent(binding)}' but Request declares '${declaredOperation(request)}'.`, + (binding, request) => bindingAdmissionRequestLogMessage("event", binding, request), ); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index 92bc71d92..5ab0b460c 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -1,5 +1,5 @@ import { FilterInput } from "../types"; -import { carriedName, carriedNamespace, definedName } from "./adjudicators"; +import { carriedName, carriedNamespace, definedGroup, definedKind, definedName } from "./adjudicators"; const prefix = "Ignoring Admission Callback:"; @@ -8,38 +8,50 @@ export const bindingKubernetesObjectLogMessage = ( filterInput: FilterInput, filterCriteria?: FilterInput, ): string => { - let logMessage = ""; - if (subject === "group") { - logMessage = `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; - } else if (subject === "deletionTimestamp") { - logMessage = `${prefix} Binding defines ${subject} but Object does not carry it.`; - } else { - logMessage = `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + switch (subject) { + case "group": + return `${prefix} Binding defines ${subject} '${definedGroup(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + case "deletionTimestamp": + return `${prefix} Binding defines ${subject} but Object does not carry it.`; + default: + return `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; } - return logMessage; }; -export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput, request?: FilterInput) => { - let logMessage = ""; - - if (subject === "namespace") { - logMessage = `${prefix} Binding carries ${subject} '${definedName(binding)}' but namespaces allowed by Capability are '${carriedName(request)}'.`; - } else { - logMessage = `${prefix} Binding defines ${subject} '${definedName(binding)}' but Request declares '${carriedName(request)}'.`; +export const bindingAdmissionRequestLogMessage = ( + subject: string, + binding: FilterInput, + request?: FilterInput, +): string => { + switch (subject) { + case "namespace": + return `${prefix} Binding defines ${subject} '${carriedNamespace(binding)}' but Request declares '${carriedNamespace(request)}'.`; + case "name": + case "version": + case "kind": + return `${prefix} Binding defines ${subject} '${carriedName(binding)}' but Request declares '${carriedName(request)}'.`; + default: + return `${prefix} Binding defines ${subject} '${definedKind(binding)}' but Request does not declare it.`; } - return logMessage; }; -export const ignoreArrayKubernetesObjectLogMessage = ( - subject: string, - filterCriteria?: FilterInput, - filterInput?: FilterInput, -) => - `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ignored ${subject}s include '${JSON.stringify(filterCriteria)}'.`; - export const arrayKubernetesObjectLogMessage = ( subject: string, filterCriteria?: FilterInput, filterInput?: FilterInput, -) => - `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; +): string => { + let logMessage: string = ""; + switch (subject) { + case "namespace": + logMessage = `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; + break; + case "ignored namespaces": + logMessage = `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; + break; + default: + break; + } + return logMessage; +}; + +export const bindingLogMessage = (subject: string) => `${prefix} Cannot use ${subject} filter on a DELETE operation.`; diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index d579e54ee..ebfd837dc 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -232,7 +232,7 @@ describe("when a pod is deleted", () => { }); it("should reject when namespace is ignored", () => { - const filters = { ...defaultFilters, namespaces: [] }; + const filters = { ...defaultFilters, regexName: "", namespaces: [] }; const binding = { ...defaultBinding, filters, From 3155e439ceeea25f7e39c13bdbcd56f5c24ccd66 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:15:48 -0500 Subject: [PATCH 39/49] Create skeleton for new unit tests --- src/lib/filter/shouldSkipRequest.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index ebfd837dc..780ad0ac3 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -537,3 +537,8 @@ it("should reject when deletionTimestamp is not present on pod", () => { /Ignoring Admission Callback: Binding defines deletionTimestamp but Object does not carry it./, ); }); + +describe("when multiple filtuers are triggered", () => { + it("should display the failure message for the first matching filter", () => {}); + it("should NOT display the failure message for the second matching filter", () => {}); +}); From fce8eee2d13f775ccf0e7b748aa37f00d0f63f8f Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:25:47 -0500 Subject: [PATCH 40/49] Move some functions to use a common logger --- src/lib/filter/filtersWithLogs.ts | 25 ++++++++++++------------- src/lib/filter/logMessages.ts | 16 ++++++++-------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index f1f56d740..62714d3a1 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -20,7 +20,7 @@ import { FilterInput, FilterParams } from "../types"; import { arrayKubernetesObjectLogMessage, bindingAdmissionRequestLogMessage, - bindingKubernetesObjectLogMessage, + commonLogMessage, bindingLogMessage, } from "./logMessages"; @@ -45,64 +45,63 @@ export const mismatchedNameFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedName(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("name", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("name", binding, kubernetesObject), ); export const mismatchedNameRegexFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedNameRegex(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("name regex", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("name regex", binding, kubernetesObject), ); export const mismatchedNamespaceRegexFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedNamespaceRegex(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("namespace regexes", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("namespace regexes", binding, kubernetesObject), ); export const mismatchedNamespaceFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedNamespace(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("namespaces", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("namespaces", binding, kubernetesObject), ); export const mismatchedAnnotationsFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedAnnotations(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("annotations", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("annotations", binding, kubernetesObject), ); export const mismatchedLabelsFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedLabels(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("labels", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("labels", binding, kubernetesObject), ); export const mismatchedKindFilter = createFilter( data => data.binding, data => data.request, (binding, request) => mismatchedKind(binding, request), - (binding, request) => bindingAdmissionRequestLogMessage("kind", binding, request), + (binding, request) => commonLogMessage("kind", binding, request), ); export const mismatchedVersionFilter = createFilter( data => data.binding, data => data.request, (binding, request) => mismatchedVersion(binding, request), - (binding, request) => bindingAdmissionRequestLogMessage("version", binding, request), + (binding, request) => commonLogMessage("version", binding, request), ); export const carriesIgnoredNamespaceFilter = createFilter( data => data.ignoredNamespaces, data => getAdmissionRequest(data), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), - (ignoreArray, kubernetesObject) => - arrayKubernetesObjectLogMessage("ignored namespaces", ignoreArray, kubernetesObject), + (ignoreArray, kubernetesObject) => commonLogMessage("ignored namespaces", ignoreArray, kubernetesObject), ); export const uncarryableNamespaceFilter = createFilter( @@ -124,14 +123,14 @@ export const mismatchedGroupFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedGroup(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("group", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("group", binding, kubernetesObject), ); export const mismatchedDeletionTimestampFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, kubernetesObject) => mismatchedDeletionTimestamp(binding, kubernetesObject), - (binding, kubernetesObject) => bindingKubernetesObjectLogMessage("deletionTimestamp", binding, kubernetesObject), + (binding, kubernetesObject) => commonLogMessage("deletionTimestamp", binding, kubernetesObject), ); export const misboundDeleteWithDeletionTimestampFilter = createFilter( diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index 5ab0b460c..cdd140800 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -3,16 +3,19 @@ import { carriedName, carriedNamespace, definedGroup, definedKind, definedName } const prefix = "Ignoring Admission Callback:"; -export const bindingKubernetesObjectLogMessage = ( - subject: string, - filterInput: FilterInput, - filterCriteria?: FilterInput, -): string => { +export const commonLogMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput): string => { switch (subject) { case "group": return `${prefix} Binding defines ${subject} '${definedGroup(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; case "deletionTimestamp": return `${prefix} Binding defines ${subject} but Object does not carry it.`; + case "version": + case "kind": + return `${prefix} Binding defines ${subject} '${carriedName(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + case "ignored namespaces": + return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; + case "namespace": + return `${prefix} Binding defines ${subject} '${carriedNamespace(filterInput)}' but Request declares '${carriedNamespace(filterCriteria)}'.`; default: return `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; } @@ -24,10 +27,7 @@ export const bindingAdmissionRequestLogMessage = ( request?: FilterInput, ): string => { switch (subject) { - case "namespace": - return `${prefix} Binding defines ${subject} '${carriedNamespace(binding)}' but Request declares '${carriedNamespace(request)}'.`; case "name": - case "version": case "kind": return `${prefix} Binding defines ${subject} '${carriedName(binding)}' but Request declares '${carriedName(request)}'.`; default: From 75dd70f32c1624ad5af3a5550856d1567d1576de Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:43:34 -0500 Subject: [PATCH 41/49] Remove dangling logging logic --- src/lib/filter/filtersWithLogs.ts | 14 ++++-------- src/lib/filter/logMessages.ts | 37 +++++++++---------------------- 2 files changed, 15 insertions(+), 36 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index 62714d3a1..9f7c5774c 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -17,12 +17,7 @@ import { } from "./adjudicators"; import { Operation } from "../mutate-types"; import { FilterInput, FilterParams } from "../types"; -import { - arrayKubernetesObjectLogMessage, - bindingAdmissionRequestLogMessage, - commonLogMessage, - bindingLogMessage, -} from "./logMessages"; +import { arrayKubernetesObjectLogMessage, commonLogMessage } from "./logMessages"; const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; @@ -116,7 +111,7 @@ export const unbindableNamespacesFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, request) => uncarryableNamespace(binding, request), - (binding, request) => bindingAdmissionRequestLogMessage("namespace", binding, request), + (binding, request) => commonLogMessage("namespace", binding, request), ); export const mismatchedGroupFilter = createFilter( @@ -140,12 +135,11 @@ export const misboundDeleteWithDeletionTimestampFilter = createFilter( // eslint-disable-next-line @typescript-eslint/no-unused-vars (binding, unused) => misboundDeleteWithDeletionTimestamp(binding), // eslint-disable-next-line @typescript-eslint/no-unused-vars - (binding, unused) => bindingLogMessage("deletionTimestamp"), + (binding, unused) => commonLogMessage("deletionTimestamp", undefined, undefined), ); - export const mismatchedEventFilter = createFilter( data => data.binding, data => data.request, (binding, request) => mismatchedEvent(binding, request), - (binding, request) => bindingAdmissionRequestLogMessage("event", binding, request), + (binding, request) => commonLogMessage("event", binding, request), ); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index cdd140800..7e165f128 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -8,7 +8,7 @@ export const commonLogMessage = (subject: string, filterInput: FilterInput, filt case "group": return `${prefix} Binding defines ${subject} '${definedGroup(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; case "deletionTimestamp": - return `${prefix} Binding defines ${subject} but Object does not carry it.`; + return getDeletionTimestampLogMessage(filterInput, filterCriteria); case "version": case "kind": return `${prefix} Binding defines ${subject} '${carriedName(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; @@ -16,23 +16,21 @@ export const commonLogMessage = (subject: string, filterInput: FilterInput, filt return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; case "namespace": return `${prefix} Binding defines ${subject} '${carriedNamespace(filterInput)}' but Request declares '${carriedNamespace(filterCriteria)}'.`; + case "event": + return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request does not declare it.`; default: return `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; } }; -export const bindingAdmissionRequestLogMessage = ( - subject: string, - binding: FilterInput, - request?: FilterInput, -): string => { - switch (subject) { - case "name": - case "kind": - return `${prefix} Binding defines ${subject} '${carriedName(binding)}' but Request declares '${carriedName(request)}'.`; - default: - return `${prefix} Binding defines ${subject} '${definedKind(binding)}' but Request does not declare it.`; +const getDeletionTimestampLogMessage = (filterInput: FilterInput, filterCriteria: FilterInput) => { + if (filterInput === undefined && filterCriteria === undefined) { + return `${prefix} Cannot use deletionTimestamp filter on a DELETE operation.`; } + return `${prefix} Binding defines deletionTimestamp but Object does not carry it.`; +}; +export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput): string => { + return `${prefix} Binding defines ${subject} '${definedKind(binding)}' but Request does not declare it.`; }; export const arrayKubernetesObjectLogMessage = ( @@ -40,18 +38,5 @@ export const arrayKubernetesObjectLogMessage = ( filterCriteria?: FilterInput, filterInput?: FilterInput, ): string => { - let logMessage: string = ""; - switch (subject) { - case "namespace": - logMessage = `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; - break; - case "ignored namespaces": - logMessage = `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; - break; - default: - break; - } - return logMessage; + return `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; }; - -export const bindingLogMessage = (subject: string) => `${prefix} Cannot use ${subject} filter on a DELETE operation.`; From d8476c9b42adda53fc23c1b6ff5f95572ab51442 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 12:47:16 -0500 Subject: [PATCH 42/49] Set functions to take input, then criteria --- src/lib/filter/filtersWithLogs.ts | 4 ++-- src/lib/filter/logMessages.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index 9f7c5774c..8cf8657ba 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -96,7 +96,7 @@ export const carriesIgnoredNamespaceFilter = createFilter( data => data.ignoredNamespaces, data => getAdmissionRequest(data), (ignoreArray, kubernetesObject) => carriesIgnoredNamespace(ignoreArray, kubernetesObject), - (ignoreArray, kubernetesObject) => commonLogMessage("ignored namespaces", ignoreArray, kubernetesObject), + (ignoreArray, kubernetesObject) => commonLogMessage("ignored namespaces", kubernetesObject, ignoreArray), ); export const uncarryableNamespaceFilter = createFilter( @@ -104,7 +104,7 @@ export const uncarryableNamespaceFilter = createFilter( data => getAdmissionRequest(data), (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), (capabilityNamespaces, kubernetesObject) => - arrayKubernetesObjectLogMessage("namespace", capabilityNamespaces, kubernetesObject), + arrayKubernetesObjectLogMessage("namespace", kubernetesObject, capabilityNamespaces), ); export const unbindableNamespacesFilter = createFilter( diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index 7e165f128..eb278a5fc 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -35,8 +35,8 @@ export const bindingAdmissionRequestLogMessage = (subject: string, binding: Filt export const arrayKubernetesObjectLogMessage = ( subject: string, - filterCriteria?: FilterInput, filterInput?: FilterInput, + filterCriteria?: FilterInput, ): string => { return `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; }; From 0d2a90682a782c8e7f54e48d114705141126245c Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 13:45:24 -0500 Subject: [PATCH 43/49] Rename adjudicator variables for clarity --- src/lib/filter/adjudicators.ts | 62 +++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/src/lib/filter/adjudicators.ts b/src/lib/filter/adjudicators.ts index 2a756919d..399458d6b 100644 --- a/src/lib/filter/adjudicators.ts +++ b/src/lib/filter/adjudicators.ts @@ -23,7 +23,7 @@ import { /* Naming scheme: - AdmissionRequest - "declares" / "neglects" - - KubernetesObject - "carries" / "missing" + - KuberneteskubernetesObjectect - "carries" / "missing" - Binding - "defines" / "ignores" */ @@ -37,22 +37,25 @@ export const declaredKind = pipe(request => request?.kind?.kind, defaultTo("")); export const declaredUid = pipe(request => request?.uid, defaultTo("")); /* - KubernetesObject collectors + KuberneteskubernetesObjectect collectors */ -export const carriesDeletionTimestamp = pipe(obj => !!obj.metadata?.deletionTimestamp, defaultTo(false)); +export const carriesDeletionTimestamp = pipe( + kubernetesObject => !!kubernetesObject.metadata?.deletionTimestamp, + defaultTo(false), +); export const missingDeletionTimestamp = complement(carriesDeletionTimestamp); -export const carriedName = pipe(obj => obj?.metadata?.name, defaultTo("")); +export const carriedName = pipe(kubernetesObject => kubernetesObject?.metadata?.name, defaultTo("")); export const carriesName = pipe(carriedName, equals(""), not); export const missingName = complement(carriesName); -export const carriedNamespace = pipe(obj => obj?.metadata?.namespace, defaultTo("")); +export const carriedNamespace = pipe(kubernetesObject => kubernetesObject?.metadata?.namespace, defaultTo("")); export const carriesNamespace = pipe(carriedNamespace, equals(""), not); -export const carriedAnnotations = pipe(obj => obj?.metadata?.annotations, defaultTo({})); +export const carriedAnnotations = pipe(kubernetesObject => kubernetesObject?.metadata?.annotations, defaultTo({})); export const carriesAnnotations = pipe(carriedAnnotations, equals({}), not); -export const carriedLabels = pipe(obj => obj?.metadata?.labels, defaultTo({})); +export const carriedLabels = pipe(kubernetesObject => kubernetesObject?.metadata?.labels, defaultTo({})); export const carriesLabels = pipe(carriedLabels, equals({}), not); /* @@ -114,7 +117,7 @@ export const definedCallback = pipe(binding => { null ); }); -export const definedCallbackName = pipe(definedCallback, defaultTo({ name: "" }), cb => cb.name); +export const definedCallbackName = pipe(definedCallback, defaultTo({ name: "" }), callback => callback.name); /* post-collection comparitors @@ -126,32 +129,32 @@ export const mismatchedDeletionTimestamp = allPass([ export const mismatchedName = allPass([ pipe(nthArg(0), definesName), - pipe((bnd, obj) => definedName(bnd) !== carriedName(obj)), + pipe((binding, kubernetesObject) => definedName(binding) !== carriedName(kubernetesObject)), ]); export const mismatchedNameRegex = allPass([ pipe(nthArg(0), definesNameRegex), - pipe((bnd, obj) => new RegExp(definedNameRegex(bnd)).test(carriedName(obj)), not), + pipe((binding, kubernetesObject) => new RegExp(definedNameRegex(binding)).test(carriedName(kubernetesObject)), not), ]); export const bindsToKind = curry( - allPass([pipe(nthArg(0), definedKind, equals(""), not), pipe((bnd, knd) => definedKind(bnd) === knd)]), + allPass([pipe(nthArg(0), definedKind, equals(""), not), pipe((binding, kind) => definedKind(binding) === kind)]), ); export const bindsToNamespace = curry(pipe(bindsToKind(__, "Namespace"))); export const misboundNamespace = allPass([bindsToNamespace, definesNamespaces]); export const mismatchedNamespace = allPass([ pipe(nthArg(0), definesNamespaces), - pipe((bnd, obj) => definedNamespaces(bnd).includes(carriedNamespace(obj)), not), + pipe((binding, kubernetesObject) => definedNamespaces(binding).includes(carriedNamespace(kubernetesObject)), not), ]); export const mismatchedNamespaceRegex = allPass([ pipe(nthArg(0), definesNamespaceRegexes), - pipe((bnd, obj) => + pipe((binding, kubernetesObject) => pipe( - any((rex: string) => new RegExp(rex).test(carriedNamespace(obj))), + any((regEx: string) => new RegExp(regEx).test(carriedNamespace(kubernetesObject))), not, - )(definedNamespaceRegexes(bnd)), + )(definedNamespaceRegexes(binding)), ), ]); @@ -160,18 +163,18 @@ export const metasMismatch = pipe( const result = { defined, carried, unalike: {} }; result.unalike = Object.entries(result.defined) - .map(([key, val]) => { + .map(([key, value]) => { const keyMissing = !Object.hasOwn(result.carried, key); - const noValue = !val; + const noValue = !value; const valMissing = !result.carried[key]; const valDiffers = result.carried[key] !== result.defined[key]; // prettier-ignore return ( - keyMissing ? { [key]: val } : + keyMissing ? { [key]: value } : noValue ? {} : - valMissing ? { [key]: val } : - valDiffers ? { [key]: val } : + valMissing ? { [key]: value } : + valDiffers ? { [key]: value } : {} ) }) @@ -184,38 +187,43 @@ export const metasMismatch = pipe( export const mismatchedAnnotations = allPass([ pipe(nthArg(0), definesAnnotations), - pipe((bnd, obj) => metasMismatch(definedAnnotations(bnd), carriedAnnotations(obj))), + pipe((binding, kubernetesObject) => metasMismatch(definedAnnotations(binding), carriedAnnotations(kubernetesObject))), ]); export const mismatchedLabels = allPass([ pipe(nthArg(0), definesLabels), - pipe((bnd, obj) => metasMismatch(definedLabels(bnd), carriedLabels(obj))), + pipe((binding, kubernetesObject) => metasMismatch(definedLabels(binding), carriedLabels(kubernetesObject))), ]); export const uncarryableNamespace = allPass([ pipe(nthArg(0), length, gt(__, 0)), pipe(nthArg(1), carriesNamespace), - pipe((nss, obj) => nss.includes(carriedNamespace(obj)), not), + pipe((namespaceSelector, kubernetesObject) => namespaceSelector.includes(carriedNamespace(kubernetesObject)), not), ]); export const carriesIgnoredNamespace = allPass([ pipe(nthArg(0), length, gt(__, 0)), pipe(nthArg(1), carriesNamespace), - pipe((nss, obj) => nss.includes(carriedNamespace(obj))), + pipe((namespaceSelector, kubernetesObject) => namespaceSelector.includes(carriedNamespace(kubernetesObject))), ]); export const unbindableNamespaces = allPass([ pipe(nthArg(0), length, gt(__, 0)), pipe(nthArg(1), definesNamespaces), - pipe((nss, bnd) => difference(definedNamespaces(bnd), nss), length, equals(0), not), + pipe( + (namespaceSelector, binding) => difference(definedNamespaces(binding), namespaceSelector), + length, + equals(0), + not, + ), ]); export const misboundDeleteWithDeletionTimestamp = allPass([definesDelete, definesDeletionTimestamp]); export const operationMatchesEvent = anyPass([ pipe(nthArg(1), equals(Event.Any)), - pipe((op, evt) => op === evt), - pipe((op, evt) => (op ? evt.includes(op) : false)), + pipe((operation, event) => operation === event), + pipe((operation, event) => (operation ? event.includes(operation) : false)), ]); export const mismatchedEvent = pipe( From b000c5408697338f009129fcdd62e526cc2961ac Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 14:27:30 -0500 Subject: [PATCH 44/49] Fix issue with namespace logging --- src/lib/filter/filtersWithLogs.ts | 21 ++++++++++---------- src/lib/filter/logMessages.ts | 32 ++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index 8cf8657ba..e19c8c284 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -17,7 +17,7 @@ import { } from "./adjudicators"; import { Operation } from "../mutate-types"; import { FilterInput, FilterParams } from "../types"; -import { arrayKubernetesObjectLogMessage, commonLogMessage } from "./logMessages"; +import { arrayKubernetesObjectLogMessage, bindingAdmissionRequestLogMessage, commonLogMessage } from "./logMessages"; const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; @@ -99,19 +99,12 @@ export const carriesIgnoredNamespaceFilter = createFilter( (ignoreArray, kubernetesObject) => commonLogMessage("ignored namespaces", kubernetesObject, ignoreArray), ); -export const uncarryableNamespaceFilter = createFilter( - data => data.capabilityNamespaces, - data => getAdmissionRequest(data), - (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), - (capabilityNamespaces, kubernetesObject) => - arrayKubernetesObjectLogMessage("namespace", kubernetesObject, capabilityNamespaces), -); - export const unbindableNamespacesFilter = createFilter( data => data.binding, data => getAdmissionRequest(data), (binding, request) => uncarryableNamespace(binding, request), - (binding, request) => commonLogMessage("namespace", binding, request), + // eslint-disable-next-line @typescript-eslint/no-unused-vars + (binding, request) => bindingAdmissionRequestLogMessage("namespaces", binding), ); export const mismatchedGroupFilter = createFilter( @@ -143,3 +136,11 @@ export const mismatchedEventFilter = createFilter( (binding, request) => mismatchedEvent(binding, request), (binding, request) => commonLogMessage("event", binding, request), ); + +export const uncarryableNamespaceFilter = createFilter( + data => data.capabilityNamespaces, + data => getAdmissionRequest(data), + (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), + (capabilityNamespaces, kubernetesObject) => + arrayKubernetesObjectLogMessage("namespace", kubernetesObject, capabilityNamespaces), +); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index eb278a5fc..f9c8c0031 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -1,5 +1,16 @@ +/* eslint-disable complexity */ import { FilterInput } from "../types"; -import { carriedName, carriedNamespace, definedGroup, definedKind, definedName } from "./adjudicators"; +import { + carriedName, + carriedNamespace, + definedAnnotations, + definedGroup, + definedKind, + definedLabels, + definedName, + definedNameRegex, + definedVersion, +} from "./adjudicators"; const prefix = "Ignoring Admission Callback:"; @@ -10,16 +21,27 @@ export const commonLogMessage = (subject: string, filterInput: FilterInput, filt case "deletionTimestamp": return getDeletionTimestampLogMessage(filterInput, filterCriteria); case "version": + return `${prefix} Binding defines ${subject} '${definedVersion(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; case "kind": - return `${prefix} Binding defines ${subject} '${carriedName(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; case "ignored namespaces": return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; - case "namespace": - return `${prefix} Binding defines ${subject} '${carriedNamespace(filterInput)}' but Request declares '${carriedNamespace(filterCriteria)}'.`; + case "namespaces": + return `${prefix} Binding defines ${subject} '${carriedNamespace(filterInput)}' but Object carries '${carriedNamespace(filterCriteria)}'.`; case "event": return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request does not declare it.`; - default: + case "annotations": + return `${prefix} Binding defines ${subject} '${definedAnnotations(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + case "labels": + return `${prefix} Binding defines ${subject} '${definedLabels(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + case "name": return `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + case "name regex": + return `${prefix} Binding defines ${subject} '${definedNameRegex(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + case "namespace regexes": + return `${prefix} Binding defines ${subject} '${definedNameRegex(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + default: + return `An undefined logging condition occurred. Filter input was '${definedName(filterInput)}' and Filter criteria was '${carriedName(filterCriteria)}`; } }; From b649d195f449ea6c0412a2cac5ea1b4deb19a1d5 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 14:53:40 -0500 Subject: [PATCH 45/49] Add tests to ensure filter chain stops after a failure --- src/lib/filter/logMessages.ts | 5 ++-- src/lib/filter/shouldSkipRequest.test.ts | 29 +++++++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index f9c8c0031..b7534ea99 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -9,6 +9,7 @@ import { definedLabels, definedName, definedNameRegex, + definedNamespaces, definedVersion, } from "./adjudicators"; @@ -27,7 +28,7 @@ export const commonLogMessage = (subject: string, filterInput: FilterInput, filt case "ignored namespaces": return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; case "namespaces": - return `${prefix} Binding defines ${subject} '${carriedNamespace(filterInput)}' but Object carries '${carriedNamespace(filterCriteria)}'.`; + return `${prefix} Binding defines ${subject} '${definedNamespaces(filterInput)}' but Object carries '${carriedNamespace(filterCriteria)}'.`; case "event": return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request does not declare it.`; case "annotations": @@ -41,7 +42,7 @@ export const commonLogMessage = (subject: string, filterInput: FilterInput, filt case "namespace regexes": return `${prefix} Binding defines ${subject} '${definedNameRegex(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; default: - return `An undefined logging condition occurred. Filter input was '${definedName(filterInput)}' and Filter criteria was '${carriedName(filterCriteria)}`; + return `${prefix} An undefined logging condition occurred. Filter input was '${definedName(filterInput)}' and Filter criteria was '${carriedName(filterCriteria)}`; } }; diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index 780ad0ac3..c44025e7f 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -538,7 +538,30 @@ it("should reject when deletionTimestamp is not present on pod", () => { ); }); -describe("when multiple filtuers are triggered", () => { - it("should display the failure message for the first matching filter", () => {}); - it("should NOT display the failure message for the second matching filter", () => {}); +describe("when multiple filters are triggered", () => { + const filters = { + ...defaultFilters, + regexName: "asdf", + name: "not-a-match", + namespaces: ["not-allowed", "also-not-matching"], + }; + const binding = { ...defaultBinding, filters }; + it("should display the failure message for the first matching filter", () => { + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).toMatch( + /Ignoring Admission Callback: Binding defines name 'not-a-match' but Object carries '.*'/, + ); + }); + it("should NOT display the failure message for the second matching filter", () => { + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).not.toMatch( + /Ignoring Admission Callback: Binding defines namespaces 'not-allowed,also-not-matching' but Object carries '.*'./, + ); + }); + it("should NOT display the failure message for the third matching filter", () => { + const pod = CreatePod(); + expect(shouldSkipRequest(binding, pod, [])).not.toMatch( + /Ignoring Admission Callback: Binding defines name regex 'asdf' but Object carries '.*./, + ); + }); }); From 6453783e8b8b9945c7f4adc9b19cc0c484245cc8 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 15:10:41 -0500 Subject: [PATCH 46/49] Unify all logging under commonLogMessage --- src/lib/filter/filtersWithLogs.ts | 4 +-- src/lib/filter/logMessages.ts | 60 +++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index e19c8c284..a9975a4a1 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -17,7 +17,7 @@ import { } from "./adjudicators"; import { Operation } from "../mutate-types"; import { FilterInput, FilterParams } from "../types"; -import { arrayKubernetesObjectLogMessage, bindingAdmissionRequestLogMessage, commonLogMessage } from "./logMessages"; +import { bindingAdmissionRequestLogMessage, commonLogMessage } from "./logMessages"; const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; @@ -142,5 +142,5 @@ export const uncarryableNamespaceFilter = createFilter( data => getAdmissionRequest(data), (capabilityNamespaces, kubernetesObject) => uncarryableNamespace(capabilityNamespaces, kubernetesObject), (capabilityNamespaces, kubernetesObject) => - arrayKubernetesObjectLogMessage("namespace", kubernetesObject, capabilityNamespaces), + commonLogMessage("namespace array", kubernetesObject, capabilityNamespaces), ); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index b7534ea99..c91775279 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -16,36 +16,74 @@ import { const prefix = "Ignoring Admission Callback:"; export const commonLogMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput): string => { + switch (subject) { + case "annotations": + case "deletionTimestamp": + case "labels": + case "name regex": + case "name": + case "namespace array": + case "namespace regexes": + case "namespaces": + return bindingKubernetesObjectMessages(subject, filterInput, filterCriteria); + case "event": + case "group": + case "kind": + case "version": + return bindingAdmissionRequestMessages(subject, filterInput, filterCriteria); + case "ignored namespaces": + return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; + default: + return getUndefinedLoggingConditionMessage(subject, filterInput, filterCriteria); + } +}; + +const bindingAdmissionRequestMessages = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => { switch (subject) { case "group": return `${prefix} Binding defines ${subject} '${definedGroup(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; - case "deletionTimestamp": - return getDeletionTimestampLogMessage(filterInput, filterCriteria); + case "event": + return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request does not declare it.`; case "version": return `${prefix} Binding defines ${subject} '${definedVersion(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; case "kind": return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; - case "ignored namespaces": - return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; + default: + return getUndefinedLoggingConditionMessage(subject, filterInput, filterCriteria); + } +}; + +const bindingKubernetesObjectMessages = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => { + switch (subject) { case "namespaces": return `${prefix} Binding defines ${subject} '${definedNamespaces(filterInput)}' but Object carries '${carriedNamespace(filterCriteria)}'.`; - case "event": - return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request does not declare it.`; case "annotations": return `${prefix} Binding defines ${subject} '${definedAnnotations(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; case "labels": return `${prefix} Binding defines ${subject} '${definedLabels(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; case "name": return `${prefix} Binding defines ${subject} '${definedName(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + case "namespace array": + return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but namespaces allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; case "name regex": return `${prefix} Binding defines ${subject} '${definedNameRegex(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; case "namespace regexes": return `${prefix} Binding defines ${subject} '${definedNameRegex(filterInput)}' but Object carries '${carriedName(filterCriteria)}'.`; + case "deletionTimestamp": + return getDeletionTimestampLogMessage(filterInput, filterCriteria); default: - return `${prefix} An undefined logging condition occurred. Filter input was '${definedName(filterInput)}' and Filter criteria was '${carriedName(filterCriteria)}`; + return getUndefinedLoggingConditionMessage(subject, filterInput, filterCriteria); } }; +const getUndefinedLoggingConditionMessage = ( + subject: string, + filterInput: FilterInput, + filterCriteria: FilterInput, +) => { + return `${prefix} An undefined logging condition occurred. Filter input was '${definedName(filterInput)}' and Filter criteria was '${carriedName(filterCriteria)}`; +}; + const getDeletionTimestampLogMessage = (filterInput: FilterInput, filterCriteria: FilterInput) => { if (filterInput === undefined && filterCriteria === undefined) { return `${prefix} Cannot use deletionTimestamp filter on a DELETE operation.`; @@ -55,11 +93,3 @@ const getDeletionTimestampLogMessage = (filterInput: FilterInput, filterCriteria export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput): string => { return `${prefix} Binding defines ${subject} '${definedKind(binding)}' but Request does not declare it.`; }; - -export const arrayKubernetesObjectLogMessage = ( - subject: string, - filterInput?: FilterInput, - filterCriteria?: FilterInput, -): string => { - return `${prefix} Object carries ${subject} '${carriedNamespace(filterInput)}' but ${subject}s allowed by Capability are '${JSON.stringify(filterCriteria)}'.`; -}; From 4cc76a71ed6a782203486e0702f394546c67a8cc Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 15:23:42 -0500 Subject: [PATCH 47/49] Present common log message in a less-complex way --- src/lib/filter/logMessages.ts | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index c91775279..4aec75356 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -1,4 +1,3 @@ -/* eslint-disable complexity */ import { FilterInput } from "../types"; import { carriedName, @@ -15,26 +14,27 @@ import { const prefix = "Ignoring Admission Callback:"; +const bindingKubernetesObjectCases = [ + "annotations", + "deletionTimestamp", + "labels", + "name regex", + "name", + "namespace array", + "namespace regexes", + "namespaces", +]; +const bindingAdmissionRequestCases = ["event", "group", "kind", "version"]; + export const commonLogMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput): string => { - switch (subject) { - case "annotations": - case "deletionTimestamp": - case "labels": - case "name regex": - case "name": - case "namespace array": - case "namespace regexes": - case "namespaces": - return bindingKubernetesObjectMessages(subject, filterInput, filterCriteria); - case "event": - case "group": - case "kind": - case "version": - return bindingAdmissionRequestMessages(subject, filterInput, filterCriteria); - case "ignored namespaces": - return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; - default: - return getUndefinedLoggingConditionMessage(subject, filterInput, filterCriteria); + if (bindingKubernetesObjectCases.includes(subject)) { + return bindingKubernetesObjectMessages(subject, filterInput, filterCriteria); + } else if (bindingAdmissionRequestCases.includes(subject)) { + return bindingAdmissionRequestMessages(subject, filterInput, filterCriteria); + } else if (subject === "ignored namespaces") { + return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; + } else { + return getUndefinedLoggingConditionMessage(subject, filterInput, filterCriteria); } }; From f1d94ffd616e6a9784f835085f1434cf86d65b56 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 15:39:17 -0500 Subject: [PATCH 48/49] Add test for unsupported subject types --- src/lib/filter/filtersWithLogs.ts | 4 ++-- src/lib/filter/logMessages.test.ts | 9 +++++++++ src/lib/filter/logMessages.ts | 13 +++++-------- 3 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 src/lib/filter/logMessages.test.ts diff --git a/src/lib/filter/filtersWithLogs.ts b/src/lib/filter/filtersWithLogs.ts index a9975a4a1..0426a63d3 100644 --- a/src/lib/filter/filtersWithLogs.ts +++ b/src/lib/filter/filtersWithLogs.ts @@ -17,7 +17,7 @@ import { } from "./adjudicators"; import { Operation } from "../mutate-types"; import { FilterInput, FilterParams } from "../types"; -import { bindingAdmissionRequestLogMessage, commonLogMessage } from "./logMessages"; +import { commonLogMessage } from "./logMessages"; const getAdmissionRequest = (data: FilterParams): KubernetesObject | undefined => { return data.request.operation === Operation.DELETE ? data.request.oldObject : data.request.object; @@ -104,7 +104,7 @@ export const unbindableNamespacesFilter = createFilter( data => getAdmissionRequest(data), (binding, request) => uncarryableNamespace(binding, request), // eslint-disable-next-line @typescript-eslint/no-unused-vars - (binding, request) => bindingAdmissionRequestLogMessage("namespaces", binding), + (binding, request) => commonLogMessage("namespaces", binding), ); export const mismatchedGroupFilter = createFilter( diff --git a/src/lib/filter/logMessages.test.ts b/src/lib/filter/logMessages.test.ts new file mode 100644 index 000000000..eb1d691f6 --- /dev/null +++ b/src/lib/filter/logMessages.test.ts @@ -0,0 +1,9 @@ +import { expect, it } from "@jest/globals"; +import { commonLogMessage } from "./logMessages"; + +it("should display the default log message when the subject is unsupported", () => { + const result = commonLogMessage("not-supported", ["some input"], ["some criteria"]); + expect(result).toMatch( + /Ignoring Admission Callback: An undefined logging condition occurred. Filter input was '.*' and Filter criteria was '.*'/, + ); +}); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index 4aec75356..eff30100d 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -28,9 +28,9 @@ const bindingAdmissionRequestCases = ["event", "group", "kind", "version"]; export const commonLogMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput): string => { if (bindingKubernetesObjectCases.includes(subject)) { - return bindingKubernetesObjectMessages(subject, filterInput, filterCriteria); + return getBindingKubernetesObjectMessage(subject, filterInput, filterCriteria); } else if (bindingAdmissionRequestCases.includes(subject)) { - return bindingAdmissionRequestMessages(subject, filterInput, filterCriteria); + return getBindingAdmissionRequestMessage(subject, filterInput, filterCriteria); } else if (subject === "ignored namespaces") { return `${prefix} Object carries namespace '${carriedNamespace(filterInput)}' but ${subject} include '${JSON.stringify(filterCriteria)}'.`; } else { @@ -38,7 +38,7 @@ export const commonLogMessage = (subject: string, filterInput: FilterInput, filt } }; -const bindingAdmissionRequestMessages = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => { +const getBindingAdmissionRequestMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => { switch (subject) { case "group": return `${prefix} Binding defines ${subject} '${definedGroup(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; @@ -53,7 +53,7 @@ const bindingAdmissionRequestMessages = (subject: string, filterInput: FilterInp } }; -const bindingKubernetesObjectMessages = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => { +const getBindingKubernetesObjectMessage = (subject: string, filterInput: FilterInput, filterCriteria?: FilterInput) => { switch (subject) { case "namespaces": return `${prefix} Binding defines ${subject} '${definedNamespaces(filterInput)}' but Object carries '${carriedNamespace(filterCriteria)}'.`; @@ -81,7 +81,7 @@ const getUndefinedLoggingConditionMessage = ( filterInput: FilterInput, filterCriteria: FilterInput, ) => { - return `${prefix} An undefined logging condition occurred. Filter input was '${definedName(filterInput)}' and Filter criteria was '${carriedName(filterCriteria)}`; + return `${prefix} An undefined logging condition occurred. Filter input was '${JSON.stringify(filterInput)}' and Filter criteria was '${JSON.stringify(filterCriteria)}'`; }; const getDeletionTimestampLogMessage = (filterInput: FilterInput, filterCriteria: FilterInput) => { @@ -90,6 +90,3 @@ const getDeletionTimestampLogMessage = (filterInput: FilterInput, filterCriteria } return `${prefix} Binding defines deletionTimestamp but Object does not carry it.`; }; -export const bindingAdmissionRequestLogMessage = (subject: string, binding: FilterInput): string => { - return `${prefix} Binding defines ${subject} '${definedKind(binding)}' but Request does not declare it.`; -}; From 048e70728a8b6cef6b6c19e5e2344d5a81a3b501 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Tue, 29 Oct 2024 15:42:53 -0500 Subject: [PATCH 49/49] Ensure that at least one character is logged in regex matcher --- src/lib/filter/adjudicators.ts | 2 ++ src/lib/filter/logMessages.test.ts | 2 +- src/lib/filter/logMessages.ts | 6 +++-- src/lib/filter/shouldSkipRequest.test.ts | 32 ++++++++++++------------ 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/lib/filter/adjudicators.ts b/src/lib/filter/adjudicators.ts index 399458d6b..116c932d1 100644 --- a/src/lib/filter/adjudicators.ts +++ b/src/lib/filter/adjudicators.ts @@ -45,6 +45,8 @@ export const carriesDeletionTimestamp = pipe( ); export const missingDeletionTimestamp = complement(carriesDeletionTimestamp); +export const carriedKind = pipe(kubernetesObject => kubernetesObject?.metadata?.kind, defaultTo("not set")); +export const carriedVersion = pipe(kubernetesObject => kubernetesObject?.metadata?.version, defaultTo("not set")); export const carriedName = pipe(kubernetesObject => kubernetesObject?.metadata?.name, defaultTo("")); export const carriesName = pipe(carriedName, equals(""), not); export const missingName = complement(carriesName); diff --git a/src/lib/filter/logMessages.test.ts b/src/lib/filter/logMessages.test.ts index eb1d691f6..47e1e2f24 100644 --- a/src/lib/filter/logMessages.test.ts +++ b/src/lib/filter/logMessages.test.ts @@ -4,6 +4,6 @@ import { commonLogMessage } from "./logMessages"; it("should display the default log message when the subject is unsupported", () => { const result = commonLogMessage("not-supported", ["some input"], ["some criteria"]); expect(result).toMatch( - /Ignoring Admission Callback: An undefined logging condition occurred. Filter input was '.*' and Filter criteria was '.*'/, + /Ignoring Admission Callback: An undefined logging condition occurred. Filter input was '.+' and Filter criteria was '.+'/, ); }); diff --git a/src/lib/filter/logMessages.ts b/src/lib/filter/logMessages.ts index eff30100d..d2f2096a8 100644 --- a/src/lib/filter/logMessages.ts +++ b/src/lib/filter/logMessages.ts @@ -1,7 +1,9 @@ import { FilterInput } from "../types"; import { + carriedKind, carriedName, carriedNamespace, + carriedVersion, definedAnnotations, definedGroup, definedKind, @@ -45,9 +47,9 @@ const getBindingAdmissionRequestMessage = (subject: string, filterInput: FilterI case "event": return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request does not declare it.`; case "version": - return `${prefix} Binding defines ${subject} '${definedVersion(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + return `${prefix} Binding defines ${subject} '${definedVersion(filterInput)}' but Request declares '${carriedVersion(filterCriteria)}'.`; case "kind": - return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request declares '${carriedName(filterCriteria)}'.`; + return `${prefix} Binding defines ${subject} '${definedKind(filterInput)}' but Request declares '${carriedKind(filterCriteria)}'.`; default: return getUndefinedLoggingConditionMessage(subject, filterInput, filterCriteria); } diff --git a/src/lib/filter/shouldSkipRequest.test.ts b/src/lib/filter/shouldSkipRequest.test.ts index c44025e7f..d06e3eaaa 100644 --- a/src/lib/filter/shouldSkipRequest.test.ts +++ b/src/lib/filter/shouldSkipRequest.test.ts @@ -126,7 +126,7 @@ describe("when a pod is created", () => { it("should reject when regex name does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(defaultBinding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines name regex '.+' but Object carries '.+'./, ); }); @@ -154,7 +154,7 @@ describe("when a pod is created", () => { const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines namespace regexes '.+' but Object carries '.+'./, ); }); it("should not reject when namespace is not ignored", () => { @@ -168,7 +168,7 @@ describe("when a pod is created", () => { const binding = { ...defaultBinding, filters }; const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + /Ignoring Admission Callback: Object carries namespace '.+' but ignored namespaces include '.+'./, ); }); }); @@ -179,7 +179,7 @@ describe("when a pod is deleted", () => { const binding = { ...defaultBinding, filters }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name regex '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines name regex '.+' but Object carries '.+'./, ); }); @@ -198,7 +198,7 @@ describe("when a pod is deleted", () => { }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespace regexes '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines namespace regexes '.+' but Object carries '.+'./, ); }); @@ -227,7 +227,7 @@ describe("when a pod is deleted", () => { }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines name '.+' but Object carries '.+'./, ); }); @@ -239,7 +239,7 @@ describe("when a pod is deleted", () => { }; const pod = DeletePod(); expect(shouldSkipRequest(binding, pod, [], ["helm-releasename"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but ignored namespaces include '.*'./, + /Ignoring Admission Callback: Object carries namespace '.+' but ignored namespaces include '.+'./, ); }); @@ -270,7 +270,7 @@ it("should reject when kind does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines kind '.*' but Request declares '.*'./, + /Ignoring Admission Callback: Binding defines kind '.+' but Request declares 'not set'./, ); }); @@ -289,7 +289,7 @@ it("should reject when group does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines group '.*' but Request declares '.*'./, + /Ignoring Admission Callback: Binding defines group '.+' but Request declares '.+'./, ); }); @@ -308,7 +308,7 @@ it("should reject when version does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines version '.*' but Request declares '.*'./, + /Ignoring Admission Callback: Binding defines version '.+' but Request declares '.+'./, ); }); @@ -338,7 +338,7 @@ it("should reject when the capability namespace does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, ["bleh", "bleh2"])).toMatch( - /Ignoring Admission Callback: Object carries namespace '.*' but namespaces allowed by Capability are '.*'./, + /Ignoring Admission Callback: Object carries namespace '.+' but namespaces allowed by Capability are '.+'./, ); }); @@ -348,7 +348,7 @@ it("should reject when namespace does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines namespaces '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines namespaces '.+' but Object carries '.+'./, ); }); @@ -385,7 +385,7 @@ it("should reject when label does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines labels '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines labels '.+' but Object carries '.+'./, ); }); @@ -429,7 +429,7 @@ it("should reject when annotation does not match", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines annotations '.*' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines annotations '.+' but Object carries '.+'./, ); }); @@ -549,13 +549,13 @@ describe("when multiple filters are triggered", () => { it("should display the failure message for the first matching filter", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).toMatch( - /Ignoring Admission Callback: Binding defines name 'not-a-match' but Object carries '.*'/, + /Ignoring Admission Callback: Binding defines name 'not-a-match' but Object carries '.+'./, ); }); it("should NOT display the failure message for the second matching filter", () => { const pod = CreatePod(); expect(shouldSkipRequest(binding, pod, [])).not.toMatch( - /Ignoring Admission Callback: Binding defines namespaces 'not-allowed,also-not-matching' but Object carries '.*'./, + /Ignoring Admission Callback: Binding defines namespaces 'not-allowed,also-not-matching' but Object carries '.+'./, ); }); it("should NOT display the failure message for the third matching filter", () => {