feat: withdeletiontimestamp filter (#1026)

## Description Adds a new filter `.WithDeletionTimestamp` to aid Operator developers as they use `ownerRefs`. Decide if `.HasDeletionTimestamp` needs to be done. Needs: - [x] e2e test (defenseunicorns/pepr-excellent-examples#61) - [x] docs ## Related Issue Fixes #1018 Fixes: #1027  Relates to # ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request) followed --------- Signed-off-by: Case Wylie <[email protected]> Co-authored-by: Barrett <[email protected]>
defenseunicorns · Aug 30, 2024 · a5ae3cc · a5ae3cc
1 parent 8cd1cb1
commit a5ae3cc
Show file tree

Hide file tree

Showing 8 changed files with 180 additions and 4 deletions.
diff --git a/codecov.yaml b/codecov.yaml
@@ -12,8 +12,8 @@ coverage:
     project:
       default:
         target: auto
-        threshold: 10% 
+        threshold: 30% 
     patch: # Settings for PR patches
       default:
         target: auto
-        threshold: 10% # Adjust as needed
+        threshold: 30% # Adjust as needed
diff --git a/docs/030_user-guide/130_filters.md b/docs/030_user-guide/130_filters.md
@@ -0,0 +1,33 @@
+# Pepr Filters
+
+Filters are functions that take a `AdmissionReview` or Watch event and return a boolean. They are used to filter out resources that do not meet certain criteria. Filters are used in the package to filter out resources that are not relevant to the user-defined admission or watch process.
+
+```ts
+When(a.ConfigMap)
+  // This limits the action to only act on new resources.
+  .IsCreated()
+  // Namespace filter
+  .InNamespace("webapp")
+  // Name filter
+  .WithName("example-1")
+  // Label filter
+  .WithLabel("app", "webapp")
+  .WithLabel("env", "prod")
+  .Mutate(request => {
+    request
+      .SetLabel("pepr", "was-here")
+      .SetAnnotation("pepr.dev", "annotations-work-too");
+  });
+```
+
+
+## `Filters`
+
+- `.WithName("name")`: Filters resources by name.
+- `.InNamespace("namespace")`: Filters resources by namespace.
+- `.WithLabel("key", "value")`: Filters resources by label. (Can be multiple)
+- `.WithDeletionTimestamp()`: Filters resources that have a deletion timestamp. 
+
+Notes: 
+- `WithDeletionTimestamp()` is does not work on Delete through the `Mutate` or `Validate` methods because the Kubernetes Admission Process does not fire the DELETE event with a deletion timestamp on the resource. 
+- `WithDeletionTimestamp()` _will_ match on an Update event during Admission (`Mutate` or `Validate`) when pending-deletion permitted changes (like removing a finalizer) occur.
diff --git a/src/lib/capability.ts b/src/lib/capability.ts
@@ -198,12 +198,13 @@ export class Capability implements CapabilityExport {
         namespaces: [],
         labels: {},
         annotations: {},
+        deletionTimestamp: false,
       },
     };
 
     const bindings = this.#bindings;
     const prefix = `${this.#name}: ${model.name}`;
-    const commonChain = { WithLabel, WithAnnotation, Mutate, Validate, Watch, Reconcile };
+    const commonChain = { WithLabel, WithAnnotation, WithDeletionTimestamp, Mutate, Validate, Watch, Reconcile };
     const isNotEmpty = (value: object) => Object.keys(value).length > 0;
     const log = (message: string, cbString: string) => {
       const filteredObj = pickBy(isNotEmpty, binding.filters);
@@ -277,6 +278,12 @@ export class Capability implements CapabilityExport {
       return { ...commonChain, WithName };
     }
 
+    function WithDeletionTimestamp(): BindingFilter<T> {
+      Log.debug("Add deletionTimestamp filter");
+      binding.filters.deletionTimestamp = true;
+      return commonChain;
+    }
+
     function WithName(name: string): BindingFilter<T> {
       Log.debug(`Add name filter ${name}`, prefix);
       binding.filters.name = name;
@@ -301,6 +308,7 @@ export class Capability implements CapabilityExport {
         ...commonChain,
         InNamespace,
         WithName,
+        WithDeletionTimestamp,
       };
     }
 

diff --git a/src/lib/filter.test.ts b/src/lib/filter.test.ts
@@ -29,6 +29,7 @@ describe("Fuzzing shouldSkipRequest", () => {
             namespaces: fc.array(fc.string()),
             labels: fc.dictionary(fc.string(), fc.string()),
             annotations: fc.dictionary(fc.string(), fc.string()),
+            deletionTimestamp: fc.boolean(),
           }),
         }),
         fc.record({
@@ -41,6 +42,11 @@ describe("Fuzzing shouldSkipRequest", () => {
             version: fc.string(),
             kind: fc.string(),
           }),
+          object: fc.record({
+            metadata: fc.record({
+              deletionTimestamp: fc.option(fc.date()),
+            }),
+          }),
         }),
         fc.array(fc.string()),
         (binding, req, capabilityNamespaces) => {
@@ -69,6 +75,7 @@ describe("Property-Based Testing shouldSkipRequest", () => {
             namespaces: fc.array(fc.string()),
             labels: fc.dictionary(fc.string(), fc.string()),
             annotations: fc.dictionary(fc.string(), fc.string()),
+            deletionTimestamp: fc.boolean(),
           }),
         }),
         fc.record({
@@ -81,6 +88,11 @@ describe("Property-Based Testing shouldSkipRequest", () => {
             version: fc.string(),
             kind: fc.string(),
           }),
+          object: fc.record({
+            metadata: fc.record({
+              deletionTimestamp: fc.option(fc.date()),
+            }),
+          }),
         }),
         fc.array(fc.string()),
         (binding, req, capabilityNamespaces) => {
@@ -103,6 +115,7 @@ test("should reject when name does not match", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -121,6 +134,7 @@ test("should reject when kind does not match", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -139,6 +153,7 @@ test("should reject when group does not match", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -161,6 +176,7 @@ test("should reject when version does not match", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -179,6 +195,7 @@ test("should allow when group, version, and kind match", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -201,6 +218,7 @@ test("should allow when kind match and others are empty", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -219,6 +237,7 @@ test("should reject when teh capability namespace does not match", () => {
       namespaces: [],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -237,6 +256,7 @@ test("should reject when namespace does not match", () => {
       namespaces: ["bleh"],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -255,6 +275,7 @@ test("should allow when namespace is match", () => {
       namespaces: ["default", "unicorn", "things"],
       labels: {},
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -275,6 +296,7 @@ test("should reject when label does not match", () => {
         foo: "bar",
       },
       annotations: {},
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -290,7 +312,7 @@ test("should allow when label is match", () => {
     kind: podKind,
     filters: {
       name: "",
-
+      deletionTimestamp: false,
       namespaces: [],
       labels: {
         foo: "bar",
@@ -324,6 +346,7 @@ test("should reject when annotation does not match", () => {
       annotations: {
         foo: "bar",
       },
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -345,6 +368,7 @@ test("should allow when annotation is match", () => {
         foo: "bar",
         test: "test1",
       },
+      deletionTimestamp: false,
     },
     callback,
   };
@@ -368,6 +392,7 @@ test("should use `oldObject` when the operation is `DELETE`", () => {
     filters: {
       name: "",
       namespaces: [],
+      deletionTimestamp: false,
       labels: {
         "app.kubernetes.io/name": "cool-name-podinfo",
       },
@@ -382,3 +407,62 @@ test("should use `oldObject` when the operation is `DELETE`", () => {
 
   expect(shouldSkipRequest(binding, pod, [])).toBe(false);
 });
+
+test("should skip processing when deletionTimestamp is not present on pod", () => {
+  const binding = {
+    model: kind.Pod,
+    event: Event.Any,
+    kind: podKind,
+    filters: {
+      name: "",
+      namespaces: [],
+      labels: {},
+      annotations: {
+        foo: "bar",
+        test: "test1",
+      },
+      deletionTimestamp: true,
+    },
+    callback,
+  };
+
+  const pod = CreatePod();
+  pod.object.metadata = pod.object.metadata || {};
+  pod.object.metadata.annotations = {
+    foo: "bar",
+    test: "test1",
+    test2: "test2",
+  };
+
+  expect(shouldSkipRequest(binding, pod, [])).toBe(true);
+});
+
+test("should processing when deletionTimestamp is not present on pod", () => {
+  const binding = {
+    model: kind.Pod,
+    event: Event.Any,
+    kind: podKind,
+    filters: {
+      name: "",
+      namespaces: [],
+      labels: {},
+      annotations: {
+        foo: "bar",
+        test: "test1",
+      },
+      deletionTimestamp: true,
+    },
+    callback,
+  };
+
+  const pod = CreatePod();
+  pod.object.metadata = pod.object.metadata || {};
+  pod.object.metadata!.deletionTimestamp = new Date("2021-09-01T00:00:00Z");
+  pod.object.metadata.annotations = {
+    foo: "bar",
+    test: "test1",
+    test2: "test2",
+  };
+
+  expect(shouldSkipRequest(binding, pod, [])).toBe(false);
+});
diff --git a/src/lib/filter.ts b/src/lib/filter.ts
@@ -22,6 +22,15 @@ export function shouldSkipRequest(binding: Binding, req: AdmissionRequest, capab
   const { metadata } = srcObject || {};
   const combinedNamespaces = [...namespaces, ...capabilityNamespaces];
 
+  // Delete bindings do not work through admission with DeletionTimestamp
+  if (binding.event.includes(Event.Delete) && binding.filters?.deletionTimestamp) {
+    return true;
+  }
+
+  // Test for deletionTimestamp
+  if (binding.filters?.deletionTimestamp && !req.object.metadata?.deletionTimestamp) {
+    return true;
+  }
   // Test for matching operation
   if (!binding.event.includes(operation) && !binding.event.includes(Event.Any)) {
     return true;

diff --git a/src/lib/helpers.test.ts b/src/lib/helpers.test.ts
@@ -1056,6 +1056,40 @@ describe("filterMatcher", () => {
     expect(result).toEqual("Ignoring Watch Callback: Cannot use a namespace filter in a namespace object.");
   });
 
+  test("return deletionTimestamp error when there is no deletionTimestamp in the object", () => {
+    const binding = {
+      filters: { deletionTimestamp: true },
+    };
+    const obj = {
+      metadata: {},
+    };
+    const capabilityNamespaces: string[] = [];
+    const result = filterNoMatchReason(
+      binding as unknown as Partial<Binding>,
+      obj as unknown as Partial<KubernetesObject>,
+      capabilityNamespaces,
+    );
+    expect(result).toEqual("Ignoring Watch Callback: Object does not have a deletion timestamp.");
+  });
+
+  test("return no deletionTimestamp error when there is a deletionTimestamp in the object", () => {
+    const binding = {
+      filters: { deletionTimestamp: true },
+    };
+    const obj = {
+      metadata: {
+        deletionTimestamp: "2021-01-01T00:00:00Z",
+      },
+    };
+    const capabilityNamespaces: string[] = [];
+    const result = filterNoMatchReason(
+      binding as unknown as Partial<Binding>,
+      obj as unknown as Partial<KubernetesObject>,
+      capabilityNamespaces,
+    );
+    expect(result).not.toEqual("Ignoring Watch Callback: Object does not have a deletion timestamp.");
+  });
+
   test("returns label overlap error when there is no overlap between binding and object labels", () => {
     const binding = {
       filters: { labels: { key: "value" } },

diff --git a/src/lib/helpers.ts b/src/lib/helpers.ts
@@ -73,6 +73,11 @@ export function filterNoMatchReason(
   obj: Partial<KubernetesObject>,
   capabilityNamespaces: string[],
 ): string {
+  // binding deletionTimestamp filter and object deletionTimestamp dont match
+  if (binding.filters?.deletionTimestamp && !obj.metadata?.deletionTimestamp) {
+    return `Ignoring Watch Callback: Object does not have a deletion timestamp.`;
+  }
+
   // binding kind is namespace with a InNamespace filter
   if (binding.kind && binding.kind.kind === "Namespace" && binding.filters && binding.filters.namespaces.length !== 0) {
     return `Ignoring Watch Callback: Cannot use a namespace filter in a namespace object.`;

diff --git a/src/lib/types.ts b/src/lib/types.ts
@@ -94,6 +94,7 @@ export type Binding = {
     namespaces: string[];
     labels: Record<string, string>;
     annotations: Record<string, string>;
+    deletionTimestamp: boolean;
   };
   readonly mutateCallback?: MutateAction<GenericClass, InstanceType<GenericClass>>;
   readonly validateCallback?: ValidateAction<GenericClass, InstanceType<GenericClass>>;
@@ -137,6 +138,8 @@ export type BindingFilter<T extends GenericClass> = CommonActionChain<T> & {
    * @param value
    */
   WithAnnotation: (key: string, value?: string) => BindingFilter<T>;
+  /** Only apply the action if the resource has a deletionTimestamp. */
+  WithDeletionTimestamp: () => BindingFilter<T>;
 };
 
 export type BindingWithName<T extends GenericClass> = BindingFilter<T> & {