Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add typing to templates.ts #1602

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/030_user-guide/120_customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Below are the available configurations through `package.json`.
| Field | Description | Example Values |
|------------------|----------------------------------------|---------------------------------|
| `uuid` | Unique identifier for the module | `hub-operator` |
| `onError` | Behavior of the webhook failure policy | `reject`, `ignore` |
| `onError` | Behavior of the webhook failure policy | `audit`, `ignore`, `reject` |
| `webhookTimeout` | Webhook timeout in seconds | `1` - `30` |
| `customLabels` | Custom labels for namespaces | `{namespace: {}}` |
| `alwaysIgnore` | Conditions to always ignore | `{namespaces: []}` |
Expand Down
9 changes: 9 additions & 0 deletions src/cli/init/enums.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export enum RbacMode {
SCOPED = "scoped",
ADMIN = "admin",
}
export enum OnError {
AUDIT = "audit",
IGNORE = "ignore",
REJECT = "reject",
}
5 changes: 3 additions & 2 deletions src/cli/init/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
} from "./templates";
import { createDir, sanitizeName, write } from "./utils";
import { confirm, PromptOptions, walkthrough } from "./walkthrough";
import { ErrorList, Errors } from "../../lib/errors";
import { ErrorList } from "../../lib/errors";
import { OnError } from "./enums";

export default function (program: RootCmd): void {
let response = {} as PromptOptions;
Expand All @@ -33,7 +34,7 @@
.option("--description <string>", "Explain the purpose of the new module.")
.option("--name <string>", "Set the name of the new module.")
.option("--skip-post-init", "Skip npm install, git init, and VSCode launch.")
.option(`--errorBehavior <${ErrorList.join("|")}>`, "Set an errorBehavior.", Errors.reject)
.option(`--errorBehavior <${ErrorList.join("|")}>`, "Set an errorBehavior.", OnError.REJECT)
.hook("preAction", async thisCommand => {
// TODO: Overrides for testing. Don't be so gross with Node CLI testing
// TODO: See pepr/#1140
Expand All @@ -46,7 +47,7 @@
Object.entries(response).map(([key, value]) => thisCommand.setOptionValue(key, value));
}
})
.action(async opts => {

Check warning on line 50 in src/cli/init/index.ts

View workflow job for this annotation

GitHub Actions / format

Async arrow function has too many statements (28). Maximum allowed is 20
const dirName = sanitizeName(response.name);
const packageJSON = genPkgJSON(response, pkgOverride);
const peprTS = genPeprTS();
Expand Down
34 changes: 32 additions & 2 deletions src/cli/init/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,38 @@ import settingsJSON from "../../templates/settings.json";
import tsConfigJSON from "../../templates/tsconfig.module.json";
import { sanitizeName } from "./utils";
import { InitOptions } from "../types";
import { V1PolicyRule as PolicyRule } from "@kubernetes/client-node";
import { OnError, RbacMode } from "./enums";

export const { dependencies, devDependencies, peerDependencies, scripts, version } = packageJSON;

export function genPkgJSON(opts: InitOptions, pgkVerOverride?: string) {
type peprPackageJSON = {
data: {
name: string;
version: string;
description: string;
keywords: string[];
engines: { node: string };
pepr: {
samayer12 marked this conversation as resolved.
Show resolved Hide resolved
uuid: string;
onError: OnError;
webhookTimeout: number;
customLabels: { namespace: Record<string, string> };
alwaysIgnore: { namespaces: string[] };
includedFiles: string[];
env: object;
rbac: PolicyRule[];
rbacMode: RbacMode;
};
scripts: { "k3d-setup": string };
dependencies: { pepr: string; undici: string };
devDependencies: { typescript: string };
};
path: string;
print: string;
};

export function genPkgJSON(opts: InitOptions, pgkVerOverride?: string): peprPackageJSON {
// Generate a random UUID for the module based on the module name
const uuid = uuidv5(opts.name, uuidv4());
// Generate a name for the module based on the module name
Expand Down Expand Up @@ -52,6 +80,8 @@ export function genPkgJSON(opts: InitOptions, pgkVerOverride?: string) {
},
includedFiles: [],
env: pgkVerOverride ? testEnv : {},
rbac: [],
rbacMode: RbacMode.SCOPED,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lack context here, but it seems like rbacMode is the equivalent to setting a default value here. In that case, I'm assuming we ought to default to the use of scoped instead of admin.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry, I missed this.

So, in short, in prod, without a doubt you are right, no reason for anyone to use cluster-admin, no way someone is going to need to control every single object in a cluster.

There is a more philosophical conversation to be had around starting a project (and Pepr's origin), figuring out how to do the Kubernetes calls that you need, then adding the appropriate RBAC after you figure everything out.

The intent of the project is that anyone can build an operator or controller. We increase the barrier of entry if we expect them to know RBAC at the beginning.

Currently, after initializing a new Pepr Module, the ServiceAccount is bound to a ClusterRole that has cluster-admin privs.

If we were going to change a default behavior there would need to be a longer convo, and warnings to potential users.

},
scripts: {
"k3d-setup": scripts["test:journey:k3d"],
Expand All @@ -72,7 +102,7 @@ export function genPkgJSON(opts: InitOptions, pgkVerOverride?: string) {
};
}

export function genPeprTS() {
export function genPeprTS(): { path: string; data: string } {
return {
path: "pepr.ts",
data: peprTS,
Expand Down
5 changes: 3 additions & 2 deletions src/cli/init/walkthrough.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
setName,
setErrorBehavior,
} from "./walkthrough";
import { OnError } from "./enums";

let consoleLog: jest.Spied<typeof console.log>;
let consoleError: jest.Spied<typeof console.error>;
Expand Down Expand Up @@ -41,7 +42,7 @@ describe("when processing input", () => {
const expected: PromptOptions = {
name: "My Test Module",
description: "A test module for Pepr",
errorBehavior: "reject",
errorBehavior: OnError.REJECT,
};

// Set values for the flag(s) under test by making a subset of (expected)
Expand Down Expand Up @@ -78,7 +79,7 @@ describe("when processing input", () => {
it("should prompt for errorBehavior when given invalid input", async () => {
const expected = { errorBehavior: "audit" };
prompts.inject(["audit"]);
const result = await setErrorBehavior("not-valid" as "reject"); // Type-Coercion forces invalid input
const result = await setErrorBehavior("not-valid" as OnError); // Type-Coercion forces invalid input
expect(result).toStrictEqual(expected);
});
});
Expand Down
15 changes: 7 additions & 8 deletions src/cli/init/walkthrough.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
import { promises as fs } from "fs";
import prompt, { Answers, PromptObject } from "prompts";

import { ErrorList, Errors } from "../../lib/errors";
import { eslint, gitignore, prettier, readme, tsConfig } from "./templates";
import { sanitizeName } from "./utils";
import { OnError } from "./enums";
import { ErrorList } from "../../lib/errors";

export type PromptOptions = {
name: string;
description: string;
errorBehavior: "audit" | "ignore" | "reject";
errorBehavior: OnError;
};

export type PartialPromptOptions = Partial<PromptOptions>;
Expand Down Expand Up @@ -70,30 +71,28 @@ async function setDescription(description?: string): Promise<Answers<string>> {
return prompt([askDescription]);
}

export async function setErrorBehavior(
errorBehavior?: "audit" | "ignore" | "reject",
): Promise<Answers<string>> {
export async function setErrorBehavior(errorBehavior?: OnError): Promise<Answers<string>> {
const askErrorBehavior: PromptObject = {
type: "select",
name: "errorBehavior",
message: "How do you want Pepr to handle errors encountered during K8s operations?",
choices: [
{
title: "Reject the operation",
value: Errors.reject,
value: OnError.REJECT,
description:
"In the event that Pepr is down or other module errors occur, the operation will not be allowed to continue. (Recommended for production.)",
},
{
title: "Ignore",
value: Errors.ignore,
value: OnError.IGNORE,
description:
"In the event that Pepr is down or other module errors occur, an entry will be generated in the Pepr Controller Log and the operation will be allowed to continue. (Recommended for development, not for production.)",
selected: true,
},
{
title: "Log an audit event",
value: Errors.audit,
value: OnError.AUDIT,
description:
"Pepr will continue processing and generate an entry in the Pepr Controller log as well as an audit event in the cluster.",
},
Expand Down
8 changes: 4 additions & 4 deletions src/lib/core/module.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { beforeEach, expect, jest, test, describe } from "@jest/globals";
import { clone } from "ramda";
import { Capability } from "./capability";
import { Schedule } from "./schedule";
import { Errors } from "../errors";
import { PackageJSON, PeprModule } from "./module";
import { CapabilityExport } from "../types";
import { OnError } from "../../cli/init/enums";

// Mock Controller
const startServerMock = jest.fn();
Expand Down Expand Up @@ -62,13 +62,13 @@ test("should reject invalid pepr onError conditions", () => {

test("should allow valid pepr onError conditions", () => {
const cfg = clone(packageJSON);
cfg.pepr.onError = Errors.audit;
cfg.pepr.onError = OnError.AUDIT;
expect(() => new PeprModule(cfg)).not.toThrow();

cfg.pepr.onError = Errors.ignore;
cfg.pepr.onError = OnError.IGNORE;
expect(() => new PeprModule(cfg)).not.toThrow();

cfg.pepr.onError = Errors.reject;
cfg.pepr.onError = OnError.REJECT;
expect(() => new PeprModule(cfg)).not.toThrow();
});

Expand Down
15 changes: 6 additions & 9 deletions src/lib/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import { expect, test, describe } from "@jest/globals";
import * as fc from "fast-check";
import { Errors, ErrorList, ValidateError } from "./errors";
import { ErrorList, ValidateError } from "./errors";
import { OnError } from "../cli/init/enums";

describe("ValidateError Fuzz Testing", () => {
test("should only accept predefined error values", () => {
Expand Down Expand Up @@ -59,17 +60,13 @@ describe("ValidateError Property-Based Testing", () => {
});

test("Errors object should have correct properties", () => {
expect(Errors).toEqual({
audit: "audit",
ignore: "ignore",
reject: "reject",
expect(OnError).toEqual({
AUDIT: "audit",
IGNORE: "ignore",
REJECT: "reject",
});
});

test("ErrorList should contain correct values", () => {
expect(ErrorList).toEqual(["audit", "ignore", "reject"]);
});

test("ValidateError should not throw an error for valid errors", () => {
expect(() => {
ValidateError("audit");
Expand Down
11 changes: 3 additions & 8 deletions src/lib/errors.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The Pepr Authors

export const Errors = {
audit: "audit",
ignore: "ignore",
reject: "reject",
};

export const ErrorList = Object.values(Errors);
import { OnError } from "../cli/init/enums";

export const ErrorList = Object.values(OnError) as string[];
/**
* Validate the error or throw an error
* @param error
*/
export function ValidateError(error = ""): void {
export function ValidateError(error: string = ""): void {
if (!ErrorList.includes(error)) {
throw new Error(`Invalid error: ${error}. Must be one of: ${ErrorList.join(", ")}`);
}
Expand Down
6 changes: 3 additions & 3 deletions src/lib/processors/mutate-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Event, Operation } from "../enums";
import { convertFromBase64Map, convertToBase64Map } from "../utils";
import { GenericClass, KubernetesObject } from "kubernetes-fluent-client";
import { MutateResponse } from "../k8s";
import { Errors } from "../errors";
import { OnError } from "../../cli/init/enums";

jest.mock("../utils");
const mockConvertFromBase64Map = jest.mocked(convertFromBase64Map);
Expand Down Expand Up @@ -243,7 +243,7 @@ describe("processRequest", () => {
);
const testBinding = { ...clone(defaultBinding), mutateCallback };
const testBindable = { ...clone(defaultBindable), binding: testBinding };
testBindable.config.onError = Errors.reject;
testBindable.config.onError = OnError.REJECT;
const testPeprMutateRequest = defaultPeprMutateRequest();
const testMutateResponse = clone(defaultMutateResponse);
const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`;
Expand All @@ -268,7 +268,7 @@ describe("processRequest", () => {
);
const testBinding = { ...clone(defaultBinding), mutateCallback };
const testBindable = { ...clone(defaultBindable), binding: testBinding };
testBindable.config.onError = Errors.audit;
testBindable.config.onError = OnError.AUDIT;
const testPeprMutateRequest = defaultPeprMutateRequest();
const testMutateResponse = clone(defaultMutateResponse);
const annote = `${defaultModuleConfig.uuid}.pepr.dev/${defaultBindable.name}`;
Expand Down
8 changes: 4 additions & 4 deletions src/lib/processors/mutate-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import { kind, KubernetesObject } from "kubernetes-fluent-client";
import { clone } from "ramda";

import { Capability } from "../core/capability";
import { Errors } from "../errors";
import { shouldSkipRequest } from "../filter/filter";
import { MutateResponse } from "../k8s";
import { AdmissionRequest, Binding } from "../types";
import Log from "../telemetry/logger";
import { ModuleConfig } from "../core/module";
import { PeprMutateRequest } from "../mutate-request";
import { base64Encode, convertFromBase64Map, convertToBase64Map } from "../utils";
import { OnError } from "../../cli/init/enums";

export interface Bindable {
req: AdmissionRequest;
Expand Down Expand Up @@ -117,11 +117,11 @@ export async function processRequest(
response.warnings.push(`Action failed: ${errorMessage}`);

switch (config.onError) {
case Errors.reject:
case OnError.REJECT:
response.result = "Pepr module configured to reject on error";
break;

case Errors.audit:
case OnError.AUDIT:
response.auditAnnotations = response.auditAnnotations || {};
response.auditAnnotations[Date.now()] = `Action failed: ${errorMessage}`;
break;
Expand Down Expand Up @@ -181,7 +181,7 @@ export async function mutateProcessor(

for (const bindable of bindables) {
({ wrapped, response } = await processRequest(bindable, wrapped, response));
if (config.onError === Errors.reject && response?.warnings!.length > 0) {
if (config.onError === OnError.REJECT && response?.warnings!.length > 0) {
return response;
}
}
Expand Down
Loading