feat: add finalizer for UDS Package CRs (#953)

## Description

This PR moves our `onDelete` code into a finalizer, using Pepr's
`Finalize` functionality. Due to an [open Pepr feature
request](defenseunicorns/pepr#1316) this does
not update the Package CR status during deletion to avoid triggering the
finalization multiple times. In order to provide visibility into
deletion status and failure this adds eventing around the specific
cleanup operations as well as specific failure events in catch blocks.

This PR does also add a new status to the Package CR of `Removing`,
which was stubbed out for a future state where we may be able to use it
(pending addition of the Pepr feature to not delete the finalizer).

3 additional related QoL changes:
- await removal of SSO tokens from the store
- modify log message around istio injection to be more accurate based on
state
- ensure `addlicense` is run on generated CRD files post-generate

## Related Issue

Fixes #523

## 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

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed

---------

Co-authored-by: Rob Ferguson <[email protected]>

src/pepr/operator/controllers/istio/injection.ts

@@ -164,7 +164,8 @@ async function killPods(ns: string, enableInjection: boolean) {
     }
 
     for (const pod of group) {
-      log.info(`Deleting pod ${ns}/${pod.metadata?.name} to enable the istio sidecar`);
+      const action = enableInjection ? "enable" : "remove";
+      log.info(`Deleting pod ${ns}/${pod.metadata?.name} to ${action} the istio sidecar`);
       await K8s(kind.Pod).Delete(pod);
     }
   }

src/pepr/operator/controllers/keycloak/authservice/authservice.ts

@@ -9,10 +9,16 @@ import { Component, setupLogger } from "../../../../logger";
 import { UDSPackage } from "../../../crd";
 import { Client } from "../types";
 import { updatePolicy } from "./authorization-policy";
-import { getAuthserviceConfig, operatorConfig, updateAuthServiceSecret } from "./config";
+import {
+  getAuthserviceConfig,
+  operatorConfig,
+  setAuthserviceConfig,
+  updateAuthServiceSecret,
+} from "./config";
 import { Action, AuthServiceEvent, AuthserviceConfig, Chain } from "./types";
 
 export const log = setupLogger(Component.OPERATOR_AUTHSERVICE);
+let lock = false;
 
 export async function authservice(pkg: UDSPackage, clients: Map<string, Client>) {
   // Get the list of clients from the package
@@ -65,13 +71,37 @@ export async function reconcileAuthservice(
 
 // Write authservice config to secret (ensure the new function name is referenced)
 export async function updateConfig(event: AuthServiceEvent) {
-  // Parse existing authservice config
-  let config = await getAuthserviceConfig();
+  // Lock to prevent concurrent updates
+  if (lock) {
+    log.debug("Lock is set for config update, retrying...");
+    setTimeout(() => updateConfig(event), 0);
+    return;
+  }
 
-  // Update config based on event
-  config = buildConfig(config, event);
+  let config: AuthserviceConfig;
+
+  try {
+    log.debug("Locking config for update");
+    lock = true;
+
+    // build updated config based on event
+    config = await getAuthserviceConfig().then(config => {
+      return buildConfig(config, event);
+    });
+
+    // Update the in-memory config immediately
+    setAuthserviceConfig(config);
+  } catch (e) {
+    log.error("Failed to build in memory authservice secret for event", event, e);
+    throw e;
+  } finally {
+    // unlock config
+    log.debug("Unlocking config for update");
+    lock = false;
+  }
 
-  // Update the authservice secret using the new function
+  // apply the authservice secret
+  log.debug("Applying authservice secret");
   await updateAuthServiceSecret(config);
 }
 

src/pepr/operator/controllers/keycloak/authservice/config.ts

@@ -130,6 +130,15 @@ export function buildInitialSecret(): AuthserviceConfig {
   return config;
 }
 
+/**
+ * Sets the in memory configuration for Authservce.
+ *
+ * @param config - The configuration object for Authservice.
+ */
+export function setAuthserviceConfig(config: AuthserviceConfig) {
+  inMemorySecret = config;
+}
+
 /**
  * Retrieves the authservice configuration, either from the in-memory cache
  * or from the Kubernetes secret if not already cached.
@@ -176,9 +185,6 @@ export async function updateAuthServiceSecret(
   checksum = true,
 ): Promise<void> {
   return new Promise((resolve, reject) => {
-    // Update the in-memory secret immediately
-    inMemorySecret = authserviceConfig;
-
     // Add the package config and its resolve function to the pending packages map
     pendingPackages.set(authserviceConfig, { resolve, reject });
 
@@ -195,7 +201,7 @@ export async function updateAuthServiceSecret(
         );
 
         // Prepare the config to be written (assumes that all packages share the same secret)
-        const { base64EncodedConfig, hash } = encodeConfig(inMemorySecret!);
+        const { base64EncodedConfig, hash } = encodeConfig(authserviceConfig!);
 
         // Apply the authservice config secret
         lastSuccessfulSecret = await applySecret(base64EncodedConfig);

src/pepr/operator/controllers/keycloak/client-sync.ts

@@ -84,7 +84,7 @@ export async function purgeSSOClients(pkg: UDSPackage, newClients: string[] = []
     const token = Store.getItem(storeKey);
     if (token) {
       await apiCall({ clientId: ref }, "DELETE", token);
-      Store.removeItem(storeKey);
+      await Store.removeItemAndWait(storeKey);
     } else {
       log.warn(pkg.metadata, `Failed to remove client ${ref}, token not found`);
     }

src/pepr/operator/crd/generated/exemption-v1alpha1.ts

@@ -4,9 +4,7 @@
  */
 
 // This file is auto-generated by kubernetes-fluent-client, do not edit manually
-
 import { GenericKind, RegisterKind } from "kubernetes-fluent-client";
-
 export class Exemption extends GenericKind {
   spec?: Spec;
 }

src/pepr/operator/crd/generated/package-v1alpha1.ts

@@ -4,9 +4,7 @@
  */
 
 // This file is auto-generated by kubernetes-fluent-client, do not edit manually
-
 import { GenericKind, RegisterKind } from "kubernetes-fluent-client";
-
 export class Package extends GenericKind {
   spec?: Spec;
   status?: Status;
@@ -635,14 +633,14 @@ export interface Sso {
    * A template for the generated secret
    */
   secretTemplate?: { [key: string]: string };
-  /**
-   * Enables the standard OpenID Connect redirect based authentication with authorization code.
-   */
-  standardFlowEnabled?: boolean;
   /**
    * Enables the client credentials grant based authentication via OpenID Connect protocol.
    */
   serviceAccountsEnabled?: boolean;
+  /**
+   * Enables the standard OpenID Connect redirect based authentication with authorization code.
+   */
+  standardFlowEnabled?: boolean;
   /**
    * Allowed CORS origins. To permit all origins of Valid Redirect URIs, add '+'. This does
    * not include the '*' wildcard though. To permit all origins, explicitly add '*'.
@@ -716,6 +714,7 @@ export enum Phase {
   Failed = "Failed",
   Pending = "Pending",
   Ready = "Ready",
+  Removing = "Removing",
   Retrying = "Retrying",
 }
 

src/pepr/operator/crd/sources/package/v1alpha1.ts

@@ -482,7 +482,7 @@ export const v1alpha1: V1CustomResourceDefinitionVersion = {
               type: "integer",
             },
             phase: {
-              enum: ["Pending", "Ready", "Failed", "Retrying"],
+              enum: ["Pending", "Ready", "Failed", "Retrying", "Removing"],
               type: "string",
             },
             ssoClients: {

src/pepr/operator/index.ts

@@ -8,8 +8,6 @@ import { a } from "pepr";
 import { When } from "./common";
 
 // Controller imports
-import { cleanupNamespace } from "./controllers/istio/injection";
-import { purgeSSOClients } from "./controllers/keycloak/client-sync";
 import {
   initAPIServerCIDR,
   updateAPIServerCIDRFromEndpointSlice,
@@ -23,9 +21,8 @@ import { validator } from "./crd/validators/package-validator";
 // Reconciler imports
 import { UDSConfig } from "../config";
 import { Component, setupLogger } from "../logger";
-import { purgeAuthserviceClients } from "./controllers/keycloak/authservice/authservice";
 import { exemptValidator } from "./crd/validators/exempt-validator";
-import { packageReconciler } from "./reconcilers/package-reconciler";
+import { packageFinalizer, packageReconciler } from "./reconcilers/package-reconciler";
 
 // Export the operator capability for registration in the root pepr.ts
 export { operator } from "./common";
@@ -50,25 +47,15 @@ When(a.Service)
   .WithName("kubernetes")
   .Reconcile(updateAPIServerCIDRFromService);
 
-// Watch for changes to the UDSPackage CRD and cleanup the namespace mutations
-When(UDSPackage)
-  .IsDeleted()
-  .Watch(async pkg => {
-    // Cleanup the namespace
-    await cleanupNamespace(pkg);
-
-    // Remove any SSO clients
-    await purgeSSOClients(pkg, []);
-    await purgeAuthserviceClients(pkg, []);
-  });
-
 // Watch for changes to the UDSPackage CRD to enqueue a package for processing
 When(UDSPackage)
   .IsCreatedOrUpdated()
   // Advanced CR validation
   .Validate(validator)
   // Enqueue the package for processing
-  .Reconcile(packageReconciler);
+  .Reconcile(packageReconciler)
+  // Handle finalizer (deletions) for the package
+  .Finalize(packageFinalizer);
 
 // Watch for Exemptions and validate
 When(UDSExemption).IsCreatedOrUpdated().Validate(exemptValidator);

src/pepr/operator/reconcilers/index.ts

@@ -18,11 +18,13 @@ const log = setupLogger(Component.OPERATOR_RECONCILERS);
  * Checks if the CRD is pending or the current generation has been processed
  *
  * @param cr The custom resource to check
- * @returns true if the CRD is pending or the current generation has been processed
+ * @returns true if the CRD is removing, pending, or the current generation has already been processed
  */
 export function shouldSkip(cr: UDSPackage) {
   const isRetrying = cr.status?.phase === Phase.Retrying;
   const isPending = cr.status?.phase === Phase.Pending;
+  // Check for status removing OR a deletion timestamp present
+  const isRemoving = cr.status?.phase === Phase.Removing || cr.metadata?.deletionTimestamp;
   const isCurrentGeneration = cr.metadata?.generation === cr.status?.observedGeneration;
 
   // First check if the CR has been seen before and return false if it has not
@@ -38,6 +40,12 @@ export function shouldSkip(cr: UDSPackage) {
     return false;
   }
 
+  // If the CR is removing, it should be skipped
+  if (isRemoving) {
+    log.debug(cr, `Should skip? Yes, removing`);
+    return true;
+  }
+
   // This is the second time the CR has been seen, so check if it is pending or the current generation
   if (isPending || isCurrentGeneration) {
     log.trace(cr, `Should skip? Yes, pending or current generation and not first time seen`);

src/pepr/operator/reconcilers/package-reconciler.ts

@@ -6,10 +6,13 @@
 import { handleFailure, shouldSkip, updateStatus, writeEvent } from ".";
 import { UDSConfig } from "../../config";
 import { Component, setupLogger } from "../../logger";
-import { enableInjection } from "../controllers/istio/injection";
+import { cleanupNamespace, enableInjection } from "../controllers/istio/injection";
 import { istioResources } from "../controllers/istio/istio-resources";
-import { authservice } from "../controllers/keycloak/authservice/authservice";
-import { keycloak } from "../controllers/keycloak/client-sync";
+import {
+  authservice,
+  purgeAuthserviceClients,
+} from "../controllers/keycloak/authservice/authservice";
+import { keycloak, purgeSSOClients } from "../controllers/keycloak/client-sync";
 import { Client } from "../controllers/keycloak/types";
 import { podMonitor } from "../controllers/monitoring/pod-monitor";
 import { serviceMonitor } from "../controllers/monitoring/service-monitor";
@@ -107,3 +110,57 @@ export async function packageReconciler(pkg: UDSPackage) {
     void handleFailure(err, pkg);
   }
 }
+
+/**
+ * The finalizer is called when an update with a deletion timestamp happens.
+ * On completion the finalizer is removed from the Package CR.
+ * This function removes any SSO/Authservice clients and ensures that Istio Injection is restored to the original state.
+ *
+ * @param pkg the package to finalize
+ */
+export async function packageFinalizer(pkg: UDSPackage) {
+  log.debug(`Processing removal of package ${pkg.metadata?.namespace}/${pkg.metadata?.name}`);
+
+  // In order to avoid triggering a second call of this finalizer, we just write events for each removal piece
+  // This could be switched to updateStatus once https://github.com/defenseunicorns/pepr/issues/1316 is resolved
+  // await updateStatus(pkg, { phase: Phase.Removing });
+
+  try {
+    await writeEvent(pkg, {
+      message: `Restoring original istio injection status on namespace`,
+      reason: "RemovalInProgress",
+      type: "Normal",
+    });
+    // Cleanup the namespace
+    await cleanupNamespace(pkg);
+  } catch (e) {
+    log.debug(
+      `Restoration of istio injection status during finalizer failed for ${pkg.metadata?.namespace}/${pkg.metadata?.name}: ${e.message}`,
+    );
+    await writeEvent(pkg, {
+      message: `Restoration of istio injection status failed: ${e.message}`,
+      reason: "RemovalFailed",
+    });
+  }
+
+  try {
+    await writeEvent(pkg, {
+      message: `Removing SSO / AuthService clients for package`,
+      reason: "RemovalInProgress",
+      type: "Normal",
+    });
+    // Remove any SSO clients
+    await purgeSSOClients(pkg, []);
+    await purgeAuthserviceClients(pkg, []);
+  } catch (e) {
+    log.debug(
+      `Removal of SSO / AuthService clients during finalizer failed for ${pkg.metadata?.namespace}/${pkg.metadata?.name}: ${e.message}`,
+    );
+    await writeEvent(pkg, {
+      message: `Removal of SSO / AuthService clients failed: ${e.message}`,
+      reason: "RemovalFailed",
+    });
+  }
+
+  log.debug(`Package ${pkg.metadata?.namespace}/${pkg.metadata?.name} removed successfully`);
+}

src/pepr/tasks.yaml

@@ -15,6 +15,20 @@ tasks:
 
       - cmd: "npx kubernetes-fluent-client crd exemptions.uds.dev src/pepr/operator/crd/generated"
 
+      - description: "Add license headers to generated CRD files"
+        shell:
+          darwin: bash
+          linux: bash
+        cmd: |
+          # check for addlicense bin
+          if [ -x "$HOME/go/bin/addlicense" ]; then
+            echo "addlicense installed in $HOME/go/bin"
+          else
+            echo "Error: addlicense is not installed in $HOME/go/bin" >&2
+            exit 1
+          fi
+          $HOME/go/bin/addlicense -l "AGPL-3.0-or-later OR LicenseRef-Defense-Unicorns-Commercial" -s=only -v -c "Defense Unicorns" src/pepr/operator/crd/generated
+
       - task: gen-docs
 
       - cmd: "npx pepr format"