Skip to content

Commit

Permalink
chore: reduce complexity of helpers.ts (#1575)
Browse files Browse the repository at this point in the history
## Description

This PR addresses complexity in `helpers.ts`

End to End Test:  <!-- if applicable -->  
(See [Pepr Excellent
Examples](https://github.com/defenseunicorns/pepr-excellent-examples))

## Related Issue

Fixes #1534

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging
- [x] Unit,
[Journey](https://github.com/defenseunicorns/pepr/tree/main/journey),
[E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples),
[docs](https://github.com/defenseunicorns/pepr/tree/main/docs),
[adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or
updated as needed
- [x] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed
  • Loading branch information
samayer12 authored Dec 13, 2024
1 parent f618e2b commit 34f4e50
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 156 deletions.
108 changes: 108 additions & 0 deletions src/lib/filter/filterNoMatchReason.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { KubernetesObject } from "kubernetes-fluent-client";
import {
mismatchedDeletionTimestamp,
mismatchedName,
definedName,
carriedName,
misboundNamespace,
mismatchedLabels,
definedLabels,
carriedLabels,
mismatchedAnnotations,
definedAnnotations,
carriedAnnotations,
uncarryableNamespace,
carriedNamespace,
unbindableNamespaces,
definedNamespaces,
mismatchedNamespace,
mismatchedNamespaceRegex,
definedNamespaceRegexes,
mismatchedNameRegex,
definedNameRegex,
carriesIgnoredNamespace,
missingCarriableNamespace,
} from "./adjudicators/adjudicators";
import { Binding } from "../types";

/**
* Decide to run callback after the event comes back from API Server
**/

export function filterNoMatchReason(
binding: Binding,
kubernetesObject: Partial<KubernetesObject>,
capabilityNamespaces: string[],
ignoredNamespaces?: string[],
): string {
const prefix = "Ignoring Watch Callback:";

// prettier-ignore
return (
mismatchedDeletionTimestamp(binding, kubernetesObject) ?
`${prefix} Binding defines deletionTimestamp but Object does not carry it.` :

mismatchedName(binding, kubernetesObject) ?
`${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(kubernetesObject)}'.` :

misboundNamespace(binding) ?
`${prefix} Cannot use namespace filter on a namespace object.` :

mismatchedLabels(binding, kubernetesObject) ?
(
`${prefix} Binding defines labels '${JSON.stringify(definedLabels(binding))}' ` +
`but Object carries '${JSON.stringify(carriedLabels(kubernetesObject))}'.`
) :

mismatchedAnnotations(binding, kubernetesObject) ?
(
`${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' ` +
`but Object carries '${JSON.stringify(carriedAnnotations(kubernetesObject))}'.`
) :

uncarryableNamespace(capabilityNamespaces, kubernetesObject) ?
(
`${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' ` +
`but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`
) :

unbindableNamespaces(capabilityNamespaces, binding) ?
(
`${prefix} Binding defines namespaces ${JSON.stringify(definedNamespaces(binding))} ` +
`but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`
) :

mismatchedNamespace(binding, kubernetesObject) ?
(
`${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' ` +
`but Object carries '${carriedNamespace(kubernetesObject)}'.`
) :

mismatchedNamespaceRegex(binding, kubernetesObject) ?
(
`${prefix} Binding defines namespace regexes ` +
`'${JSON.stringify(definedNamespaceRegexes(binding))}' ` +
`but Object carries '${carriedNamespace(kubernetesObject)}'.`
) :

mismatchedNameRegex(binding, kubernetesObject) ?
(
`${prefix} Binding defines name regex '${definedNameRegex(binding)}' ` +
`but Object carries '${carriedName(kubernetesObject)}'.`
) :

carriesIgnoredNamespace(ignoredNamespaces, kubernetesObject) ?
(
`${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' ` +
`but ignored namespaces include '${JSON.stringify(ignoredNamespaces)}'.`
) :

missingCarriableNamespace(capabilityNamespaces, kubernetesObject) ?
(
`${prefix} Object does not carry a namespace ` +
`but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`
) :

""
);
}
36 changes: 10 additions & 26 deletions src/lib/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@
import { Binding, CapabilityExport } from "./types";
import { Event } from "./enums";
import {
addVerbIfNotExists,
bindingAndCapabilityNSConflict,
createRBACMap,
filterNoMatchReason,
dedent,
generateWatchNamespaceError,
hasAnyOverlap,
Expand All @@ -22,6 +20,7 @@ import {
validateCapabilityNames,
ValidationError,
} from "./helpers";
import { filterNoMatchReason } from "./filter/filterNoMatchReason";
import { sanitizeResourceName } from "../sdk/sdk";
import * as fc from "fast-check";
import { expect, describe, jest, beforeEach, afterEach, it } from "@jest/globals";
Expand Down Expand Up @@ -358,20 +357,6 @@ describe("createRBACMap", () => {
});
});

describe("addVerbIfNotExists", () => {
it("should add a verb if it does not exist in the array", () => {
const verbs = ["get", "list"];
addVerbIfNotExists(verbs, "watch");
expect(verbs).toEqual(["get", "list", "watch"]);
});

it("should not add a verb if it already exists in the array", () => {
const verbs = ["get", "list", "watch"];
addVerbIfNotExists(verbs, "get");
expect(verbs).toEqual(["get", "list", "watch"]); // The array remains unchanged
});
});

describe("hasAnyOverlap", () => {
it("returns true for overlapping arrays", () => {
expect(hasAnyOverlap([1, 2, 3], [3, 4, 5])).toBe(true);
Expand Down Expand Up @@ -683,26 +668,25 @@ describe("namespaceComplianceValidator", () => {
});

describe("parseTimeout", () => {
const PREV = "a";
it("should return a number when a valid string number between 1 and 30 is provided", () => {
expect(parseTimeout("5", PREV)).toBe(5);
expect(parseTimeout("1", PREV)).toBe(1);
expect(parseTimeout("30", PREV)).toBe(30);
expect(parseTimeout("5")).toBe(5);
expect(parseTimeout("1")).toBe(1);
expect(parseTimeout("30")).toBe(30);
});

it("should throw an InvalidArgumentError for non-numeric strings", () => {
expect(() => parseTimeout("abc", PREV)).toThrow(Error);
expect(() => parseTimeout("", PREV)).toThrow(Error);
expect(() => parseTimeout("abc")).toThrow(Error);
expect(() => parseTimeout("")).toThrow(Error);
});

it("should throw an InvalidArgumentError for numbers outside the 1-30 range", () => {
expect(() => parseTimeout("0", PREV)).toThrow(Error);
expect(() => parseTimeout("31", PREV)).toThrow(Error);
expect(() => parseTimeout("0")).toThrow(Error);
expect(() => parseTimeout("31")).toThrow(Error);
});

it("should throw an InvalidArgumentError for numeric strings that represent floating point numbers", () => {
expect(() => parseTimeout("5.5", PREV)).toThrow(Error);
expect(() => parseTimeout("20.1", PREV)).toThrow(Error);
expect(() => parseTimeout("5.5")).toThrow(Error);
expect(() => parseTimeout("20.1")).toThrow(Error);
});
});

Expand Down
145 changes: 16 additions & 129 deletions src/lib/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,9 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

import { KubernetesObject } from "kubernetes-fluent-client";
import Log from "./telemetry/logger";
import { Binding, CapabilityExport } from "./types";
import { sanitizeResourceName } from "../sdk/sdk";
import {
carriedAnnotations,
carriedLabels,
carriedName,
carriedNamespace,
carriesIgnoredNamespace,
definedAnnotations,
definedLabels,
definedName,
definedNameRegex,
definedNamespaces,
definedNamespaceRegexes,
misboundNamespace,
mismatchedAnnotations,
mismatchedDeletionTimestamp,
mismatchedLabels,
mismatchedName,
mismatchedNameRegex,
mismatchedNamespace,
mismatchedNamespaceRegex,
missingCarriableNamespace,
unbindableNamespaces,
uncarryableNamespace,
} from "./filter/adjudicators/adjudicators";

export function matchesRegex(pattern: string, testString: string): boolean {
return new RegExp(pattern).test(testString);
Expand All @@ -55,100 +30,13 @@ export function validateHash(expectedHash: string): void {
}
}

export type RBACMap = {
type RBACMap = {
[key: string]: {
verbs: string[];
plural: string;
};
};

/**
* Decide to run callback after the event comes back from API Server
**/
export function filterNoMatchReason(
binding: Binding,
kubernetesObject: Partial<KubernetesObject>,
capabilityNamespaces: string[],
ignoredNamespaces?: string[],
): string {
const prefix = "Ignoring Watch Callback:";

// prettier-ignore
return (
mismatchedDeletionTimestamp(binding, kubernetesObject) ?
`${prefix} Binding defines deletionTimestamp but Object does not carry it.` :

mismatchedName(binding, kubernetesObject) ?
`${prefix} Binding defines name '${definedName(binding)}' but Object carries '${carriedName(kubernetesObject)}'.` :

misboundNamespace(binding) ?
`${prefix} Cannot use namespace filter on a namespace object.` :

mismatchedLabels(binding, kubernetesObject) ?
(
`${prefix} Binding defines labels '${JSON.stringify(definedLabels(binding))}' ` +
`but Object carries '${JSON.stringify(carriedLabels(kubernetesObject))}'.`
) :

mismatchedAnnotations(binding, kubernetesObject) ?
(
`${prefix} Binding defines annotations '${JSON.stringify(definedAnnotations(binding))}' ` +
`but Object carries '${JSON.stringify(carriedAnnotations(kubernetesObject))}'.`
) :

uncarryableNamespace(capabilityNamespaces, kubernetesObject) ?
(
`${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' ` +
`but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`
) :

unbindableNamespaces(capabilityNamespaces, binding) ?
(
`${prefix} Binding defines namespaces ${JSON.stringify(definedNamespaces(binding))} ` +
`but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`
) :

mismatchedNamespace(binding, kubernetesObject) ?
(
`${prefix} Binding defines namespaces '${JSON.stringify(definedNamespaces(binding))}' ` +
`but Object carries '${carriedNamespace(kubernetesObject)}'.`
) :

mismatchedNamespaceRegex(binding, kubernetesObject) ?
(
`${prefix} Binding defines namespace regexes ` +
`'${JSON.stringify(definedNamespaceRegexes(binding))}' ` +
`but Object carries '${carriedNamespace(kubernetesObject)}'.`
) :

mismatchedNameRegex(binding, kubernetesObject) ?
(
`${prefix} Binding defines name regex '${definedNameRegex(binding)}' ` +
`but Object carries '${carriedName(kubernetesObject)}'.`
) :

carriesIgnoredNamespace(ignoredNamespaces, kubernetesObject) ?
(
`${prefix} Object carries namespace '${carriedNamespace(kubernetesObject)}' ` +
`but ignored namespaces include '${JSON.stringify(ignoredNamespaces)}'.`
) :

missingCarriableNamespace(capabilityNamespaces, kubernetesObject) ?
(
`${prefix} Object does not carry a namespace ` +
`but namespaces allowed by Capability are '${JSON.stringify(capabilityNamespaces)}'.`
) :

""
);
}

export function addVerbIfNotExists(verbs: string[], verb: string): void {
if (!verbs.includes(verb)) {
verbs.push(verb);
}
}

export function createRBACMap(capabilities: CapabilityExport[]): RBACMap {
return capabilities.reduce((acc: RBACMap, capability: CapabilityExport) => {
capability.bindings.forEach(binding => {
Expand Down Expand Up @@ -256,13 +144,16 @@ export function namespaceComplianceValidator(capability: CapabilityExport, ignor
}

// Ensure that each regexNamespace matches a capabilityNamespace
matchRegexToCapababilityNamespace(bindingRegexNamespaces, capabilityNamespaces);
// ensure regexNamespaces do not match ignored ns
checkRegexNamespaces(bindingRegexNamespaces, ignoredNamespaces);
}

if (
bindingRegexNamespaces &&
bindingRegexNamespaces.length > 0 &&
capabilityNamespaces &&
capabilityNamespaces.length > 0
) {
const matchRegexToCapababilityNamespace = (
bindingRegexNamespaces: string[],
capabilityNamespaces: string[] | undefined,
): void => {
if (bindingRegexNamespaces.length > 0 && capabilityNamespaces && capabilityNamespaces.length > 0) {
for (const regexNamespace of bindingRegexNamespaces) {
let matches = false;
matches =
Expand All @@ -275,13 +166,10 @@ export function namespaceComplianceValidator(capability: CapabilityExport, ignor
}
}
}
// ensure regexNamespaces do not match ignored ns
if (
bindingRegexNamespaces &&
bindingRegexNamespaces.length > 0 &&
ignoredNamespaces &&
ignoredNamespaces.length > 0
) {
};

const checkRegexNamespaces = (bindingRegexNamespaces: string[], ignoredNamespaces: string[] | undefined): void => {
if (bindingRegexNamespaces.length > 0 && ignoredNamespaces && ignoredNamespaces.length > 0) {
for (const regexNamespace of bindingRegexNamespaces) {
const matchedNS = ignoredNamespaces.find(ignoredNS => matchesRegex(regexNamespace, ignoredNS));
if (matchedNS) {
Expand All @@ -291,7 +179,7 @@ export function namespaceComplianceValidator(capability: CapabilityExport, ignor
}
}
}
}
};

// check if secret is over the size limit
export function secretOverLimit(str: string): boolean {
Expand All @@ -302,8 +190,7 @@ export function secretOverLimit(str: string): boolean {
return sizeInBytes > oneMiBInBytes;
}

/* eslint-disable @typescript-eslint/no-unused-vars */
export const parseTimeout = (value: string, previous: unknown): number => {
export const parseTimeout = (value: string): number => {
const parsedValue = parseInt(value, 10);
const floatValue = parseFloat(value);
if (isNaN(parsedValue)) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/watch-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { K8s, KubernetesObject, WatchCfg, WatchEvent } from "kubernetes-fluent-client";
import { WatchPhase } from "kubernetes-fluent-client/dist/fluent/types";
import { Capability } from "./capability";
import { filterNoMatchReason } from "./helpers";
import { filterNoMatchReason } from "./filter/filterNoMatchReason";
import { removeFinalizer } from "./finalizer";
import Log from "./telemetry/logger";
import { Queue } from "./queue";
Expand Down

0 comments on commit 34f4e50

Please sign in to comment.