chore: remove circular dependencies in src/lib/assets/ (defenseunic…

…orns#1652) ## Description This PR removes circular dependencies. End to End Test:  (See [Pepr Excellent Examples](https://github.com/defenseunicorns/pepr-excellent-examples)) ## Related Issue Fixes defenseunicorns#1632 ## 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 --------- Co-authored-by: Case Wylie <[email protected]>
tamirazrab · Jan 17, 2025 · d4eef2b · d4eef2b
1 parent ae48978
commit d4eef2b
Show file tree

Hide file tree

Showing 11 changed files with 205 additions and 205 deletions.
diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml
@@ -41,11 +41,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 10 ]; then
-            echo "circular-deps.log has more than 10 circular dependencies."
+          if [ $(wc -l < circular-deps.log) -gt 9 ]; then
+            echo "circular-deps.log has more than 9 circular dependencies."
             wc -l circular-deps.log
             exit 1
           else
-            echo "circular-deps.log has 10 or fewer circular dependencies."
+            echo "circular-deps.log has 9 or fewer circular dependencies."
             exit 0
           fi
diff --git a/src/cli/build.helpers.ts b/src/cli/build.helpers.ts
@@ -8,6 +8,9 @@ import { BuildOptions, BuildResult, context, BuildContext } from "esbuild";
 import { Assets } from "../lib/assets/assets";
 import { resolve } from "path";
 import { promises as fs } from "fs";
+import { generateAllYaml } from "../lib/assets/yaml/generateAllYaml";
+import { webhookConfigGenerator } from "../lib/assets/webhooks";
+import { generateZarfYamlGeneric } from "../lib/assets/yaml/generateZarfYaml";
 
 export type Reloader = (opts: BuildResult<BuildOptions>) => void | Promise<void>;
 /**
@@ -191,18 +194,18 @@ export async function generateYamlAndWriteToDisk(obj: {
   const yamlFile = `pepr-module-${uuid}.yaml`;
   const chartPath = `${uuid}-chart`;
   const yamlPath = resolve(outputDir, yamlFile);
-  const yaml = await assets.allYaml(imagePullSecret);
+  const yaml = await assets.allYaml(generateAllYaml, imagePullSecret);
   const zarfPath = resolve(outputDir, "zarf.yaml");
 
   let localZarf = "";
   if (zarf === "chart") {
-    localZarf = assets.zarfYamlChart(chartPath);
+    localZarf = assets.zarfYamlChart(generateZarfYamlGeneric, chartPath);
   } else {
-    localZarf = assets.zarfYaml(yamlFile);
+    localZarf = assets.zarfYaml(generateZarfYamlGeneric, yamlFile);
   }
   await fs.writeFile(yamlPath, yaml);
   await fs.writeFile(zarfPath, localZarf);
 
-  await assets.generateHelmChart(outputDir);
+  await assets.generateHelmChart(webhookConfigGenerator, outputDir);
   console.info(`✅ K8s resource for the module saved to ${yamlPath}`);
 }
diff --git a/src/cli/deploy.ts b/src/cli/deploy.ts
@@ -4,13 +4,13 @@
 import prompt from "prompts";
 
 import { Assets } from "../lib/assets/assets";
-import { buildModule } from "./build";
-import { RootCmd } from "./root";
-import { validateCapabilityNames } from "../lib/helpers";
 import { ImagePullSecret } from "../lib/types";
-import { sanitizeName } from "./init/utils";
-import { deployImagePullSecret } from "../lib/assets/deploy";
+import { RootCmd } from "./root";
+import { buildModule } from "./build";
+import { deployImagePullSecret, deployWebhook } from "../lib/assets/deploy";
 import { namespaceDeploymentsReady } from "../lib/deploymentChecks";
+import { sanitizeName } from "./init/utils";
+import { validateCapabilityNames } from "../lib/helpers";
 
 export interface ImagePullSecretDetails {
   pullSecret?: string;
@@ -132,7 +132,7 @@ export default function (program: RootCmd): void {
       webhook.image = opts.image ?? webhook.image;
 
       try {
-        await webhook.deploy(opts.force, builtModule.cfg.pepr.webhookTimeout ?? 10);
+        await webhook.deploy(deployWebhook, opts.force, builtModule.cfg.pepr.webhookTimeout ?? 10);
 
         // wait for capabilities to be loaded and test names
         validateCapabilityNames(webhook.capabilities);

diff --git a/src/cli/dev.ts b/src/cli/dev.ts
@@ -1,15 +1,16 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: 2023-Present The Pepr Authors
 
-import { ChildProcess, fork } from "child_process";
-import { promises as fs } from "fs";
 import prompt from "prompts";
-import { validateCapabilityNames } from "../lib/helpers";
 import { Assets } from "../lib/assets/assets";
-import { buildModule, loadModule } from "./build";
-import { RootCmd } from "./root";
+import { ChildProcess, fork } from "child_process";
 import { K8s, kind } from "kubernetes-fluent-client";
+import { RootCmd } from "./root";
 import { Store } from "../lib/k8s";
+import { buildModule, loadModule } from "./build";
+import { deployWebhook } from "../lib/assets/deploy";
+import { promises as fs } from "fs";
+import { validateCapabilityNames } from "../lib/helpers";
 export default function (program: RootCmd): void {
   program
     .command("dev")
@@ -59,7 +60,7 @@ export default function (program: RootCmd): void {
           console.info(`Running module ${path}`);
 
           // Deploy the webhook with a 30 second timeout for debugging, don't force
-          await webhook.deploy(false, 30);
+          await webhook.deploy(deployWebhook, false, 30);
 
           try {
             // wait for capabilities to be loaded and test names

diff --git a/src/lib/assets/assets.ts b/src/lib/assets/assets.ts
@@ -11,18 +11,21 @@ import {
   serviceMonitorTemplate,
   watcherDeployTemplate,
 } from "./helm";
+import {
+  V1Deployment,
+  V1MutatingWebhookConfiguration,
+  V1ValidatingWebhookConfiguration,
+} from "@kubernetes/client-node/dist/gen";
 import { createDirectoryIfNotExists } from "../filesystemService";
-import { deploy } from "./deploy";
+import { overridesFile } from "./yaml/overridesFile";
 import { getDeployment, getModuleSecret, getWatcher } from "./pods";
 import { helmLayout, createWebhookYaml, toYaml } from "./index";
 import { loadCapabilities } from "./loader";
 import { namespaceComplianceValidator, dedent } from "../helpers";
+import { promises as fs } from "fs";
 import { storeRole, storeRoleBinding, clusterRoleBinding, serviceAccount } from "./rbac";
 import { watcherService, service, tlsSecret, apiTokenSecret } from "./networking";
-import { webhookConfig } from "./webhooks";
-import { generateZarfYaml, generateZarfYamlChart, generateAllYaml, overridesFile } from "./yaml";
-import { promises as fs } from "fs";
-import { V1MutatingWebhookConfiguration, V1ValidatingWebhookConfiguration } from "@kubernetes/client-node/dist/gen";
+import { WebhookType } from "../enums";
 
 export class Assets {
   readonly name: string;
@@ -50,27 +53,41 @@ export class Assets {
     this.apiToken = crypto.randomBytes(32).toString("hex");
   }
 
-  deploy = async (force: boolean, webhookTimeout?: number): Promise<void> => {
+  async deploy(
+    deployFunction: (assets: Assets, force: boolean, webhookTimeout: number) => Promise<void>,
+    force: boolean,
+    webhookTimeout?: number,
+  ): Promise<void> {
     this.capabilities = await loadCapabilities(this.path);
-    await deploy(this, force, webhookTimeout);
-  };
 
-  zarfYaml = (path: string): string => generateZarfYaml(this.name, this.image, this.config, path);
+    const timeout = typeof webhookTimeout === "number" ? webhookTimeout : 10;
 
-  zarfYamlChart = (path: string): string => generateZarfYamlChart(this.name, this.image, this.config, path);
+    await deployFunction(this, force, timeout);
+  }
 
-  allYaml = async (imagePullSecret?: string): Promise<string> => {
+  zarfYaml = (
+    zarfYamlGenerator: (assets: Assets, path: string, type: "manifests" | "charts") => string,
+    path: string,
+  ): string => zarfYamlGenerator(this, path, "manifests");
+
+  zarfYamlChart = (
+    zarfYamlGenerator: (assets: Assets, path: string, type: "manifests" | "charts") => string,
+    path: string,
+  ): string => zarfYamlGenerator(this, path, "charts");
+
+  allYaml = async (
+    yamlGenerationFunction: (
+      assyts: Assets,
+      deployments: { default: V1Deployment; watch: V1Deployment | null },
+    ) => Promise<string>,
+    imagePullSecret?: string,
+  ): Promise<string> => {
     this.capabilities = await loadCapabilities(this.path);
     // give error if namespaces are not respected
     for (const capability of this.capabilities) {
       namespaceComplianceValidator(capability, this.alwaysIgnore?.namespaces);
     }
 
-    const webhooks = {
-      mutate: await webhookConfig(this, "mutate", this.config.webhookTimeout),
-      validate: await webhookConfig(this, "validate", this.config.webhookTimeout),
-    };
-
     const code = await fs.readFile(this.path);
 
     const moduleHash = crypto.createHash("sha256").update(code).digest("hex");
@@ -80,16 +97,7 @@ export class Assets {
       watch: getWatcher(this, moduleHash, this.buildTimestamp, imagePullSecret),
     };
 
-    const assetsInputs = {
-      apiToken: this.apiToken,
-      capabilities: this.capabilities,
-      config: this.config,
-      hash: moduleHash,
-      name: this.name,
-      path: this.path,
-      tls: this.tls,
-    };
-    return generateAllYaml(webhooks, deployments, assetsInputs);
+    return yamlGenerationFunction(this, deployments);
   };
 
   writeWebhookFiles = async (
@@ -111,7 +119,14 @@ export class Assets {
     }
   };
 
-  generateHelmChart = async (basePath: string): Promise<void> => {
+  generateHelmChart = async (
+    webhookGeneratorFunction: (
+      assets: Assets,
+      mutateOrValidate: WebhookType,
+      timeoutSeconds: number | undefined,
+    ) => Promise<V1MutatingWebhookConfiguration | V1ValidatingWebhookConfiguration | null>,
+    basePath: string,
+  ): Promise<void> => {
     const helm = helmLayout(basePath, this.config.uuid);
 
     try {
@@ -150,12 +165,12 @@ export class Assets {
       };
       await overridesFile(overrideData, helm.files.valuesYaml);
 
-      const [mutateWebhook, validateWebhook] = await Promise.all([
-        webhookConfig(this, "mutate", this.config.webhookTimeout),
-        webhookConfig(this, "validate", this.config.webhookTimeout),
-      ]);
+      const webhooks = {
+        mutate: await webhookGeneratorFunction(this, WebhookType.MUTATE, this.config.webhookTimeout),
+        validate: await webhookGeneratorFunction(this, WebhookType.VALIDATE, this.config.webhookTimeout),
+      };
 
-      await this.writeWebhookFiles(validateWebhook, mutateWebhook, helm);
+      await this.writeWebhookFiles(webhooks.validate, webhooks.mutate, helm);
 
       const watchDeployment = getWatcher(this, moduleHash, this.buildTimestamp);
       if (watchDeployment) {

diff --git a/src/lib/assets/deploy.ts b/src/lib/assets/deploy.ts
@@ -12,8 +12,9 @@ import { apiTokenSecret, service, tlsSecret, watcherService } from "./networking
 import { getDeployment, getModuleSecret, getNamespace, getWatcher } from "./pods";
 import { clusterRole, clusterRoleBinding, serviceAccount, storeRole, storeRoleBinding } from "./rbac";
 import { peprStoreCRD } from "./store";
-import { webhookConfig } from "./webhooks";
+import { webhookConfigGenerator } from "./webhooks";
 import { CapabilityExport, ImagePullSecret } from "../types";
+import { WebhookType } from "../enums";
 
 export async function deployImagePullSecret(imagePullSecret: ImagePullSecret, name: string): Promise<void> {
   try {
@@ -42,50 +43,51 @@ export async function deployImagePullSecret(imagePullSecret: ImagePullSecret, na
     Log.error(e);
   }
 }
-export async function deploy(assets: Assets, force: boolean, webhookTimeout?: number): Promise<void> {
-  Log.info("Establishing connection to Kubernetes");
 
-  const { name, host, path } = assets;
+async function handleWebhookConfiguration(
+  assets: Assets,
+  type: WebhookType,
+  webhookTimeout: number,
+  force: boolean,
+): Promise<void> {
+  const kindMap = {
+    mutate: kind.MutatingWebhookConfiguration,
+    validate: kind.ValidatingWebhookConfiguration,
+  };
+
+  const webhookConfig = await webhookConfigGenerator(assets, type, webhookTimeout);
+
+  if (webhookConfig) {
+    Log.info(`Applying ${type} webhook`);
+    await K8s(kindMap[type]).Apply(webhookConfig, { force });
+  } else {
+    Log.info(`${type.charAt(0).toUpperCase() + type.slice(1)} webhook not needed, removing if it exists`);
+    await K8s(kindMap[type]).Delete(assets.name);
+  }
+}
+
+export async function deployWebhook(assets: Assets, force: boolean, webhookTimeout: number): Promise<void> {
+  Log.info("Establishing connection to Kubernetes");
 
   Log.info("Applying pepr-system namespace");
   await K8s(kind.Namespace).Apply(getNamespace(assets.config.customLabels?.namespace));
 
   // Create the mutating webhook configuration if it is needed
-  const mutateWebhook = await webhookConfig(assets, "mutate", webhookTimeout);
-  if (mutateWebhook) {
-    Log.info("Applying mutating webhook");
-    await K8s(kind.MutatingWebhookConfiguration).Apply(mutateWebhook, { force });
-  } else {
-    Log.info("Mutating webhook not needed, removing if it exists");
-    await K8s(kind.MutatingWebhookConfiguration).Delete(name);
-  }
+  await handleWebhookConfiguration(assets, WebhookType.MUTATE, webhookTimeout, force);
 
   // Create the validating webhook configuration if it is needed
-  const validateWebhook = await webhookConfig(assets, "validate", webhookTimeout);
-  if (validateWebhook) {
-    Log.info("Applying validating webhook");
-    await K8s(kind.ValidatingWebhookConfiguration).Apply(validateWebhook, { force });
-  } else {
-    Log.info("Validating webhook not needed, removing if it exists");
-    await K8s(kind.ValidatingWebhookConfiguration).Delete(name);
-  }
+  await handleWebhookConfiguration(assets, WebhookType.VALIDATE, webhookTimeout, force);
 
   Log.info("Applying the Pepr Store CRD if it doesn't exist");
   await K8s(kind.CustomResourceDefinition).Apply(peprStoreCRD, { force });
 
-  // If a host is specified, we don't need to deploy the rest of the resources
-  if (host) {
-    return;
-  }
+  if (assets.host) return; // Skip resource deployment if a host is already specified
 
-  const code = await fs.readFile(path);
+  const code = await fs.readFile(assets.path);
+  if (!code.length) throw new Error("No code provided");
   const hash = crypto.createHash("sha256").update(code).digest("hex");
 
-  if (code.length < 1) {
-    throw new Error("No code provided");
-  }
-
-  await setupRBAC(name, assets.capabilities, force, assets.config);
+  await setupRBAC(assets.name, assets.capabilities, force, assets.config);
   await setupController(assets, code, hash, force);
   await setupWatcher(assets, hash, force);
 }

diff --git a/src/lib/assets/webhooks.ts b/src/lib/assets/webhooks.ts
@@ -10,7 +10,7 @@ import { kind } from "kubernetes-fluent-client";
 import { concat, equals, uniqWith } from "ramda";
 
 import { Assets } from "./assets";
-import { Event } from "../enums";
+import { Event, WebhookType } from "../enums";
 import { Binding } from "../types";
 
 export const peprIgnoreNamespaces: string[] = ["kube-system", "pepr-system"];
@@ -68,9 +68,9 @@ export async function generateWebhookRules(assets: Assets, isMutateWebhook: bool
   return uniqWith(equals, rules);
 }
 
-export async function webhookConfig(
+export async function webhookConfigGenerator(
   assets: Assets,
-  mutateOrValidate: "mutate" | "validate",
+  mutateOrValidate: WebhookType,
   timeoutSeconds = 10,
 ): Promise<kind.MutatingWebhookConfiguration | kind.ValidatingWebhookConfiguration | null> {
   const ignore: V1LabelSelectorRequirement[] = [];
@@ -106,7 +106,7 @@ export async function webhookConfig(
     };
   }
 
-  const isMutate = mutateOrValidate === "mutate";
+  const isMutate = mutateOrValidate === WebhookType.MUTATE;
   const rules = await generateWebhookRules(assets, isMutate);
 
   // If there are no rules, return null