diff --git a/src/pepr/operator/controllers/istio/injection.ts b/src/pepr/operator/controllers/istio/injection.ts index 4ee2971ce..92147894a 100644 --- a/src/pepr/operator/controllers/istio/injection.ts +++ b/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); } } diff --git a/src/pepr/operator/controllers/keycloak/authservice/authservice.ts b/src/pepr/operator/controllers/keycloak/authservice/authservice.ts index ca52198d2..0d6af015d 100644 --- a/src/pepr/operator/controllers/keycloak/authservice/authservice.ts +++ b/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) { // 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); } diff --git a/src/pepr/operator/controllers/keycloak/authservice/config.ts b/src/pepr/operator/controllers/keycloak/authservice/config.ts index 958359347..f33dc3c91 100644 --- a/src/pepr/operator/controllers/keycloak/authservice/config.ts +++ b/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 { 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); diff --git a/src/pepr/operator/controllers/keycloak/client-sync.ts b/src/pepr/operator/controllers/keycloak/client-sync.ts index 3ecc9bd28..f92dea26f 100644 --- a/src/pepr/operator/controllers/keycloak/client-sync.ts +++ b/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`); } diff --git a/src/pepr/operator/crd/generated/exemption-v1alpha1.ts b/src/pepr/operator/crd/generated/exemption-v1alpha1.ts index 7c22b5a1a..66a737f58 100644 --- a/src/pepr/operator/crd/generated/exemption-v1alpha1.ts +++ b/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; } diff --git a/src/pepr/operator/crd/generated/package-v1alpha1.ts b/src/pepr/operator/crd/generated/package-v1alpha1.ts index 1696c9f8d..1bed25c65 100644 --- a/src/pepr/operator/crd/generated/package-v1alpha1.ts +++ b/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", } diff --git a/src/pepr/operator/crd/sources/package/v1alpha1.ts b/src/pepr/operator/crd/sources/package/v1alpha1.ts index 3b36df0a6..bac9b73c6 100644 --- a/src/pepr/operator/crd/sources/package/v1alpha1.ts +++ b/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: { diff --git a/src/pepr/operator/index.ts b/src/pepr/operator/index.ts index 23c7510ee..9f34252cf 100644 --- a/src/pepr/operator/index.ts +++ b/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); diff --git a/src/pepr/operator/reconcilers/index.ts b/src/pepr/operator/reconcilers/index.ts index 6466c84d7..3c219e889 100644 --- a/src/pepr/operator/reconcilers/index.ts +++ b/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`); diff --git a/src/pepr/operator/reconcilers/package-reconciler.ts b/src/pepr/operator/reconcilers/package-reconciler.ts index e60e39150..a74a1ab38 100644 --- a/src/pepr/operator/reconcilers/package-reconciler.ts +++ b/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`); +} diff --git a/src/pepr/tasks.yaml b/src/pepr/tasks.yaml index 44db43117..c6baaabaf 100644 --- a/src/pepr/tasks.yaml +++ b/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"