Skip to content

Commit

Permalink
chore: fix circular dependency between types and mutate-request (#1332)
Browse files Browse the repository at this point in the history
## Description
We would like to remove the circular dependency between types.ts and
mutate-request.ts
...

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

## Related Issue

Fixes #
<!-- or -->
Relates to #

## 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
- [ ] 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
- [ ] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Co-authored-by: Sam Mayer <[email protected]>
  • Loading branch information
schaeferka and samayer12 authored Oct 25, 2024
1 parent 2b0d353 commit 97c2845
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 100 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ jobs:
run: |
npx madge --circular --ts-config tsconfig.json --extensions ts,js src/ > tmp.log || true # Force exit 0 for post-processing
tail -n +4 tmp.log > circular-deps.log
if [ $(wc -l < circular-deps.log) -gt 18 ]; then
echo "circular-deps.log has more than 18 circular dependencies."
if [ $(wc -l < circular-deps.log) -gt 17 ]; then
echo "circular-deps.log has more than 17 circular dependencies."
wc -l circular-deps.log
exit 1
else
echo "circular-deps.log has 18 or fewer circular dependencies."
echo "circular-deps.log has 17 or fewer circular dependencies."
exit 0
fi
3 changes: 2 additions & 1 deletion src/lib/adjudicators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
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, Event } from "./types";
import { DeepPartial, Operation } from "./mutate-types";

describe("definesDeletionTimestamp", () => {
//[ Binding, result ]
Expand Down
3 changes: 2 additions & 1 deletion src/lib/adjudicators.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

import { Event, Operation } from "./types";
import { Event } from "./types";
import { Operation } from "./mutate-types";
import {
__,
allPass,
Expand Down
3 changes: 2 additions & 1 deletion src/lib/capability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import { CapabilityCfg, FinalizeAction, MutateAction, ValidateAction, WatchLogAc
import { a } from "../lib";
import { V1Pod } from "@kubernetes/client-node";
import { expect, describe, jest, beforeEach, it } from "@jest/globals";
import { Operation } from "./mutate-types";
import { PeprMutateRequest } from "./mutate-request";
import { PeprValidateRequest } from "./validate-request";
import { Operation, AdmissionRequest } from "./types";
import { AdmissionRequest } from "./types";
import { WatchPhase } from "kubernetes-fluent-client/dist/fluent/types";
import { Event } from "./types";
import { GenericClass } from "kubernetes-fluent-client";
Expand Down
5 changes: 3 additions & 2 deletions src/lib/filter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

import { AdmissionRequest, Binding, Operation } from "./types";
import { AdmissionRequest, Binding } from "./types";
import { Operation } from "./mutate-types";
import {
carriesIgnoredNamespace,
carriedName,
Expand Down Expand Up @@ -56,7 +57,7 @@ export function shouldSkipRequest(

// prettier-ignore
return (
misboundDeleteWithDeletionTimestamp(binding) ?
misboundDeleteWithDeletionTimestamp(binding) ?
`${prefix} Cannot use deletionTimestamp filter on a DELETE operation.` :

mismatchedDeletionTimestamp(binding, obj) ?
Expand Down
3 changes: 2 additions & 1 deletion src/lib/finalizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { beforeEach, describe, expect, it, jest } from "@jest/globals";
import { addFinalizer, removeFinalizer } from "./finalizer";
import { KubernetesObject, K8s, GenericClass, RegisterKind } from "kubernetes-fluent-client";
import { K8sInit } from "kubernetes-fluent-client/dist/fluent/types";
import { AdmissionRequest, Operation } from "./types";
import { AdmissionRequest } from "./types";
import { Operation } from "./mutate-types";
import { PeprMutateRequest } from "./mutate-request";
import { Binding } from "./types";

Expand Down
4 changes: 2 additions & 2 deletions src/lib/finalizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

import { K8s, KubernetesObject, RegisterKind } from "kubernetes-fluent-client";
import Log from "./logger";
import { Binding, DeepPartial } from "./types";
import { Operation } from "./types";
import { Binding } from "./types";
import { Operation, DeepPartial } from "./mutate-types";
import { PeprMutateRequest } from "./mutate-request";

export function addFinalizer<K extends KubernetesObject>(request: PeprMutateRequest<K>) {
Expand Down
5 changes: 3 additions & 2 deletions src/lib/mutate-request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import { beforeEach, describe, expect, it } from "@jest/globals";
import { KubernetesObject } from "kubernetes-fluent-client";
import { Operation, AdmissionRequest } from "./types";
import { AdmissionRequest } from "./types";
import { Operation } from "./mutate-types";
import { PeprMutateRequest } from "./mutate-request";

describe("PeprMutateRequest", () => {
Expand Down Expand Up @@ -92,7 +93,7 @@ describe("PeprMutateRequest", () => {
object: undefined as unknown as KubernetesObject,
};

expect(() => new PeprMutateRequest(mockRequest)).toThrow("unable to load the request object into PeprRequest.RawP");
expect(() => new PeprMutateRequest(mockRequest)).toThrow("Unable to load the request object into PeprRequest.Raw");
});

it("should merge the provided object with the current resource", () => {
Expand Down
80 changes: 11 additions & 69 deletions src/lib/mutate-request.ts
Original file line number Diff line number Diff line change
@@ -1,54 +1,41 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

import { Operation, AdmissionRequest, DeepPartial } from "./mutate-types";
import { KubernetesObject } from "kubernetes-fluent-client";
import { clone, mergeDeepRight } from "ramda";
import { Operation, AdmissionRequest, DeepPartial } from "./types";
import { Logger } from "pino";
import { GenericClass } from "kubernetes-fluent-client";

/**
* The RequestWrapper class provides methods to modify Kubernetes objects in the context
* of a mutating webhook request.
*/
// MutateAction type for handling mutation callbacks
export type MutateAction<T extends GenericClass, K extends KubernetesObject = InstanceType<T>> = (
req: PeprMutateRequest<K>,
logger?: Logger,
) => Promise<void> | void | Promise<PeprMutateRequest<K>> | PeprMutateRequest<K>;

// PeprMutateRequest class for mutation request handling
export class PeprMutateRequest<T extends KubernetesObject> {
Raw: T;

#input: AdmissionRequest<T>;

get PermitSideEffects() {
return !this.#input.dryRun;
}

/**
* Indicates whether the request is a dry run.
* @returns true if the request is a dry run, false otherwise.
*/
get IsDryRun() {
return this.#input.dryRun;
}

/**
* Provides access to the old resource in the request if available.
* @returns The old Kubernetes resource object or null if not available.
*/
get OldResource() {
return this.#input.oldObject;
}

/**
* Provides access to the request object.
* @returns The request object containing the Kubernetes resource.
*/
get Request() {
return this.#input;
}

/**
* Creates a new instance of the action class.
* @param input - The request object containing the Kubernetes resource to modify.
*/
constructor(input: AdmissionRequest<T>) {
this.#input = input;

// If this is a DELETE operation, use the oldObject instead
if (input.operation.toUpperCase() === Operation.DELETE) {
this.Raw = clone(input.oldObject as T);
Expand All @@ -58,93 +45,48 @@ export class PeprMutateRequest<T extends KubernetesObject> {
}

if (!this.Raw) {
throw new Error("unable to load the request object into PeprRequest.RawP");
throw new Error("Unable to load the request object into PeprRequest.Raw");
}
}

/**
* Deep merges the provided object with the current resource.
*
* @param obj - The object to merge with the current resource.
*/
Merge = (obj: DeepPartial<T>) => {
this.Raw = mergeDeepRight(this.Raw, obj) as unknown as T;
};

/**
* Updates a label on the Kubernetes resource.
* @param key - The key of the label to update.
* @param value - The value of the label.
* @returns The current action instance for method chaining.
*/
SetLabel = (key: string, value: string) => {
const ref = this.Raw;

ref.metadata = ref.metadata ?? {};
ref.metadata.labels = ref.metadata.labels ?? {};
ref.metadata.labels[key] = value;

return this;
};

/**
* Updates an annotation on the Kubernetes resource.
* @param key - The key of the annotation to update.
* @param value - The value of the annotation.
* @returns The current action instance for method chaining.
*/
SetAnnotation = (key: string, value: string) => {
const ref = this.Raw;

ref.metadata = ref.metadata ?? {};
ref.metadata.annotations = ref.metadata.annotations ?? {};
ref.metadata.annotations[key] = value;

return this;
};

/**
* Removes a label from the Kubernetes resource.
* @param key - The key of the label to remove.
* @returns The current Action instance for method chaining.
*/
RemoveLabel = (key: string) => {
if (this.Raw.metadata?.labels?.[key]) {
delete this.Raw.metadata.labels[key];
}

return this;
};

/**
* Removes an annotation from the Kubernetes resource.
* @param key - The key of the annotation to remove.
* @returns The current Action instance for method chaining.
*/
RemoveAnnotation = (key: string) => {
if (this.Raw.metadata?.annotations?.[key]) {
delete this.Raw.metadata.annotations[key];
}

return this;
};

/**
* Check if a label exists on the Kubernetes resource.
*
* @param key the label key to check
* @returns
*/
HasLabel = (key: string) => {
return this.Raw.metadata?.labels?.[key] !== undefined;
};

/**
* Check if an annotation exists on the Kubernetes resource.
*
* @param key the annotation key to check
* @returns
*/
HasAnnotation = (key: string) => {
return this.Raw.metadata?.annotations?.[key] !== undefined;
};
Expand Down
50 changes: 50 additions & 0 deletions src/lib/mutate-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

import { KubernetesObject } from "kubernetes-fluent-client";
import { GroupVersionKind } from "kubernetes-fluent-client";

// Operation type for mutation operations
export enum Operation {
CREATE = "CREATE",
UPDATE = "UPDATE",
DELETE = "DELETE",
CONNECT = "CONNECT",
}

// AdmissionRequest interface for handling admission requests in mutation context
export interface AdmissionRequest<T = KubernetesObject> {
readonly uid: string;
readonly kind: GroupVersionKind;
readonly resource: GroupVersionResource;
readonly subResource?: string;
readonly requestKind?: GroupVersionKind;
readonly requestResource?: GroupVersionResource;
readonly requestSubResource?: string;
readonly name: string;
readonly namespace?: string;
readonly operation: Operation;
readonly userInfo: {
username?: string;
uid?: string;
groups?: string[];
extra?: { [key: string]: string[] };
};
readonly object: T;
readonly oldObject?: T;
readonly dryRun?: boolean;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
readonly options?: any;
}

// DeepPartial utility type for deep optional properties
export type DeepPartial<T> = {
[P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> : T[P];
};

// GroupVersionResource interface for resource identification
export interface GroupVersionResource {
readonly group: string;
readonly version: string;
readonly resource: string;
}
18 changes: 2 additions & 16 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,13 @@
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

import { GenericClass, GroupVersionKind, KubernetesObject } from "kubernetes-fluent-client";

import { Operation } from "./mutate-types";
import { WatchPhase } from "kubernetes-fluent-client/dist/fluent/types";

import { Logger } from "pino";
import { PeprMutateRequest } from "./mutate-request";
import { PeprValidateRequest } from "./validate-request";

import { Logger } from "pino";
import { V1PolicyRule as PolicyRule } from "@kubernetes/client-node";

export enum Operation {
CREATE = "CREATE",
UPDATE = "UPDATE",
DELETE = "DELETE",
CONNECT = "CONNECT",
}
/**
* Specifically for deploying images with a private registry
*/
Expand All @@ -41,12 +33,6 @@ export interface ResponseItem {
message: string;
};
}
/**
* Recursively make all properties in T optional.
*/
export type DeepPartial<T> = {
[P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> : T[P];
};

/**
* The type of Kubernetes mutating webhook event that the action is registered for.
Expand Down
3 changes: 2 additions & 1 deletion src/lib/validate-request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import { beforeEach, describe, expect, it } from "@jest/globals";
import { KubernetesObject } from "kubernetes-fluent-client";
import { ValidateActionResponse, AdmissionRequest, Operation } from "./types";
import { ValidateActionResponse, AdmissionRequest } from "./types";
import { Operation } from "./mutate-types";
import { PeprValidateRequest } from "./validate-request";
describe("PeprValidateRequest", () => {
let mockRequest: AdmissionRequest<KubernetesObject>;
Expand Down
3 changes: 2 additions & 1 deletion src/lib/validate-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import { KubernetesObject } from "kubernetes-fluent-client";

import { clone } from "ramda";
import { AdmissionRequest, Operation } from "./types";
import { AdmissionRequest } from "./types";
import { ValidateActionResponse } from "./types";
import { Operation } from "./mutate-types";

/**
* The RequestWrapper class provides methods to modify Kubernetes objects in the context
Expand Down

0 comments on commit 97c2845

Please sign in to comment.