From fc1ff087df73cfcb5c52478bfaf14e7312bb36a0 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Mon, 16 Dec 2024 11:59:06 -0600 Subject: [PATCH 1/3] Extract rule validation to separate function --- src/lib/assets/webhooks.ts | 82 +++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/src/lib/assets/webhooks.ts b/src/lib/assets/webhooks.ts index 3a3187340..dac085321 100644 --- a/src/lib/assets/webhooks.ts +++ b/src/lib/assets/webhooks.ts @@ -11,6 +11,7 @@ import { concat, equals, uniqWith } from "ramda"; import { Assets } from "."; import { Event } from "../enums"; +import { Binding } from "../types"; const peprIgnoreLabel: V1LabelSelectorRequirement = { key: "pepr.dev", @@ -20,53 +21,60 @@ const peprIgnoreLabel: V1LabelSelectorRequirement = { const peprIgnoreNamespaces: string[] = ["kube-system", "pepr-system"]; -export async function generateWebhookRules(assets: Assets, isMutateWebhook: boolean): Promise { - const { config, capabilities } = assets; - const rules: V1RuleWithOperations[] = []; +const validateRule = (binding: Binding, isMutateWebhook: boolean): V1RuleWithOperations | undefined => { + const { event, kind, isMutate, isValidate } = binding; - // Iterate through the capabilities and generate the rules - for (const capability of capabilities) { - console.info(`Module ${config.uuid} has capability: ${capability.name}`); + // If the module doesn't have a callback for the event, skip it + if (isMutateWebhook && !isMutate) { + return undefined; + } - // Read the bindings and generate the rules - for (const binding of capability.bindings) { - const { event, kind, isMutate, isValidate } = binding; + if (!isMutateWebhook && !isValidate) { + return undefined; + } - // If the module doesn't have a callback for the event, skip it - if (isMutateWebhook && !isMutate) { - continue; - } + const operations: string[] = []; - if (!isMutateWebhook && !isValidate) { - continue; - } + // CreateOrUpdate is a Pepr-specific event that is translated to Create and Update + if (event === Event.CREATE_OR_UPDATE) { + operations.push(Event.CREATE, Event.UPDATE); + } else { + operations.push(event); + } - const operations: string[] = []; + // Use the plural property if it exists, otherwise use lowercase kind + s + const resource = kind.plural || `${kind.kind.toLowerCase()}s`; - // CreateOrUpdate is a Pepr-specific event that is translated to Create and Update - if (event === Event.CREATE_OR_UPDATE) { - operations.push(Event.CREATE, Event.UPDATE); - } else { - operations.push(event); - } + const ruleObject = { + apiGroups: [kind.group], + apiVersions: [kind.version || "*"], + operations, + resources: [resource], + }; - // Use the plural property if it exists, otherwise use lowercase kind + s - const resource = kind.plural || `${kind.kind.toLowerCase()}s`; + // If the resource is pods, add ephemeralcontainers as well + if (resource === "pods") { + ruleObject.resources.push("pods/ephemeralcontainers"); + } - const ruleObject = { - apiGroups: [kind.group], - apiVersions: [kind.version || "*"], - operations, - resources: [resource], - }; + // Add the rule to the rules array + return ruleObject; +}; - // If the resource is pods, add ephemeralcontainers as well - if (resource === "pods") { - ruleObject.resources.push("pods/ephemeralcontainers"); - } +export async function generateWebhookRules(assets: Assets, isMutateWebhook: boolean): Promise { + const rules: V1RuleWithOperations[] = []; + const { config, capabilities } = assets; - // Add the rule to the rules array - rules.push(ruleObject); + // Iterate through the capabilities and generate the rules + for (const capability of capabilities) { + console.info(`Module ${config.uuid} has capability: ${capability.name}`); + + // Read the bindings and generate the rules + for (const binding of capability.bindings) { + const validatedRule = validateRule(binding, isMutateWebhook); + if (validatedRule) { + rules.push(validatedRule); + } } } From d7111a080498f4e3648561af7cce5585ff2db75a Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Mon, 16 Dec 2024 12:04:53 -0600 Subject: [PATCH 2/3] Use map() & filter() instead of nested loops --- src/lib/assets/webhooks.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/lib/assets/webhooks.ts b/src/lib/assets/webhooks.ts index dac085321..93fa1a9c5 100644 --- a/src/lib/assets/webhooks.ts +++ b/src/lib/assets/webhooks.ts @@ -62,23 +62,16 @@ const validateRule = (binding: Binding, isMutateWebhook: boolean): V1RuleWithOpe }; export async function generateWebhookRules(assets: Assets, isMutateWebhook: boolean): Promise { - const rules: V1RuleWithOperations[] = []; const { config, capabilities } = assets; - // Iterate through the capabilities and generate the rules - for (const capability of capabilities) { + const rules = capabilities.flatMap(capability => { console.info(`Module ${config.uuid} has capability: ${capability.name}`); - // Read the bindings and generate the rules - for (const binding of capability.bindings) { - const validatedRule = validateRule(binding, isMutateWebhook); - if (validatedRule) { - rules.push(validatedRule); - } - } - } + return capability.bindings + .map(binding => validateRule(binding, isMutateWebhook)) + .filter((rule): rule is V1RuleWithOperations => !!rule); + }); - // Return the rules with duplicates removed return uniqWith(equals, rules); } From 5afda9917b9cb9314094a1f98de534d4970e0603 Mon Sep 17 00:00:00 2001 From: Sam Mayer Date: Mon, 16 Dec 2024 12:11:22 -0600 Subject: [PATCH 3/3] Simplify validation logic --- src/lib/assets/webhooks.ts | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/lib/assets/webhooks.ts b/src/lib/assets/webhooks.ts index 93fa1a9c5..4ac1025d7 100644 --- a/src/lib/assets/webhooks.ts +++ b/src/lib/assets/webhooks.ts @@ -24,40 +24,24 @@ const peprIgnoreNamespaces: string[] = ["kube-system", "pepr-system"]; const validateRule = (binding: Binding, isMutateWebhook: boolean): V1RuleWithOperations | undefined => { const { event, kind, isMutate, isValidate } = binding; - // If the module doesn't have a callback for the event, skip it - if (isMutateWebhook && !isMutate) { + // Skip invalid bindings based on webhook type + if ((isMutateWebhook && !isMutate) || (!isMutateWebhook && !isValidate)) { return undefined; } - if (!isMutateWebhook && !isValidate) { - return undefined; - } - - const operations: string[] = []; - - // CreateOrUpdate is a Pepr-specific event that is translated to Create and Update - if (event === Event.CREATE_OR_UPDATE) { - operations.push(Event.CREATE, Event.UPDATE); - } else { - operations.push(event); - } + // Translate event to operations + const operations = event === Event.CREATE_OR_UPDATE ? [Event.CREATE, Event.UPDATE] : [event]; // Use the plural property if it exists, otherwise use lowercase kind + s const resource = kind.plural || `${kind.kind.toLowerCase()}s`; - const ruleObject = { + const ruleObject: V1RuleWithOperations = { apiGroups: [kind.group], apiVersions: [kind.version || "*"], operations, - resources: [resource], + resources: [resource, ...(resource === "pods" ? ["pods/ephemeralcontainers"] : [])], }; - // If the resource is pods, add ephemeralcontainers as well - if (resource === "pods") { - ruleObject.resources.push("pods/ephemeralcontainers"); - } - - // Add the rule to the rules array return ruleObject; };