fix: Add controller finalizer when policy is active state only

[jvanz]

Description

This changes the controller reconciliation loop to ensure that the finalizer used by the controller will be added in the policy resource only when it is in active state. Therefore, after the kubewarden-controller uninstall, the policies can be removed with no issues.

In other words, when the policy is in:

• Scheduled status: no Finalizer (new), no {Validating,Mutating}WebhookConfiguration. Can be safely deleted. Will be garbage-collected if the CRD is removed.
• Active status: has an associated {Validating,Mutating}WebhookConfiguration, must not be garbage-collected away or we break the cluster. Therefore, it also has a Finalizer (as it already had prior to this change).

Tests

Beyond the e2e tests,I have a bash script that simulate helm charts uninstall to see if this is working as expected:

Click to see the testing script

#!/bin/bash
set -e

# Colors for output
RED='\033[0;31m'
GREEN='\033[0;32m'
YELLOW='\033[1;33m'
BLUE='\033[0;34m'
NC='\033[0m' # No Color

log() {
    echo -e "${BLUE}==>${NC} $1"
}

success() {
    echo -e "${GREEN}✓${NC} $1"
}

error() {
    echo -e "${RED}✗${NC} $1"
}

warn() {
    echo -e "${YELLOW}!${NC} $1"
}

# Configuration
CLUSTER_NAME="${CLUSTER_NAME:-kubewarden-test}"
NAMESPACE="${NAMESPACE:-kubewarden}"
CONTROLLER_IMAGE="ghcr.io/kubewarden/adm-controller/controller:dev"
AUDIT_SCANNER_IMAGE="ghcr.io/kubewarden/adm-controller/audit-scanner:dev"
POLICY_SERVER_IMAGE="ghcr.io/kubewarden/adm-controller/policy-server:dev"

# Check if cluster exists
if kind get clusters 2>/dev/null | grep -q "^${CLUSTER_NAME}$"; then
    warn "Cluster ${CLUSTER_NAME} already exists"
    read -p "Delete and recreate? (y/N) " -n 1 -r
    echo
    if [[ $REPLY =~ ^[Yy]$ ]]; then
        log "Deleting existing cluster..."
        kind delete cluster --name "${CLUSTER_NAME}"
    else
        log "Using existing cluster"
        REUSE_CLUSTER=true
    fi
fi

# Create cluster if needed
if [[ "$REUSE_CLUSTER" != "true" ]]; then
    log "Creating kind cluster: ${CLUSTER_NAME}"
    kind create cluster --name "${CLUSTER_NAME}"
    success "Cluster created"
fi

# Set kubectl context
kubectl config use-context "kind-${CLUSTER_NAME}"

# Load images into cluster
log "Loading images into kind cluster..."
kind load docker-image "${CONTROLLER_IMAGE}" --name "${CLUSTER_NAME}"
kind load docker-image "${AUDIT_SCANNER_IMAGE}" --name "${CLUSTER_NAME}"
kind load docker-image "${POLICY_SERVER_IMAGE}" --name "${CLUSTER_NAME}"
success "Images loaded"

# Create namespace
log "Creating namespace: ${NAMESPACE}"
kubectl create namespace "${NAMESPACE}" --dry-run=client -o yaml | kubectl apply -f -
kubectl label namespace "${NAMESPACE}" \
    pod-security.kubernetes.io/enforce=restricted \
    pod-security.kubernetes.io/enforce-version=latest \
    --overwrite

# Install CRDs chart
log "Installing kubewarden-crds chart..."
helm upgrade --install kubewarden-crds \
    ./charts/kubewarden-crds \
    --namespace "${NAMESPACE}" \
    --wait
success "kubewarden-crds installed"

# Install controller chart
log "Installing kubewarden-controller chart..."
helm upgrade --install kubewarden-controller \
    ./charts/kubewarden-controller \
    --namespace "${NAMESPACE}" \
    --set image.tag=dev \
    --set auditScanner.image.tag=dev \
    --set logLevel=debug \
    --wait \
    --timeout 5m
success "kubewarden-controller installed"

# Wait for controller to be ready
log "Waiting for controller deployment..."
kubectl wait --for=condition=available --timeout=300s \
    deployment/kubewarden-controller -n "${NAMESPACE}"
success "Controller is ready"

# Create a PolicyServer
log "Creating PolicyServer..."
cat <<EOF | kubectl apply -f -
apiVersion: policies.kubewarden.io/v1
kind: PolicyServer
metadata:
  name: default
  namespace: ${NAMESPACE}
spec:
  image: ${POLICY_SERVER_IMAGE}
  replicas: 1
  serviceAccountName: kubewarden-controller
EOF

# Wait for PolicyServer deployment
log "Waiting for PolicyServer deployment..."
kubectl wait --for=condition=available --timeout=300s \
    deployment/policy-server-default -n "${NAMESPACE}" 2>/dev/null || true
success "PolicyServer deployed"

# Create a sample ClusterAdmissionPolicy
log "Creating sample ClusterAdmissionPolicy..."
cat <<EOF | kubectl apply -f -
apiVersion: policies.kubewarden.io/v1
kind: ClusterAdmissionPolicy
metadata:
  name: privileged-pods
spec:
  module: registry://ghcr.io/kubewarden/policies/pod-privileged:v0.2.5
  rules:
    - apiGroups: [""]
      apiVersions: ["v1"]
      resources: ["pods"]
      operations: ["CREATE", "UPDATE"]
  mutating: false
  policyServer: default
EOF
success "Policy created"

# Show initial state
echo ""
log "Initial state:"
echo ""

echo -e "${BLUE}Policies:${NC}"
kubectl get clusteradmissionpolicies -o custom-columns=\
NAME:.metadata.name,\
STATUS:.status.policyStatus,\
FINALIZERS:.metadata.finalizers

echo ""
echo -e "${BLUE}PolicyServers:${NC}"
kubectl get policyservers -n "${NAMESPACE}"

echo ""
echo -e "${BLUE}ValidatingWebhookConfigurations:${NC}"
kubectl get validatingwebhookconfigurations -l app.kubernetes.io/part-of=kubewarden

echo ""
warn "Press Enter to uninstall kubewarden-controller chart..."
read

# Uninstall controller chart
log "Uninstalling kubewarden-controller chart..."
helm uninstall kubewarden-controller -n "${NAMESPACE}" --wait --timeout 2m
success "kubewarden-controller uninstalled"

# Check policy state after controller uninstall
echo ""
log "State after controller uninstall:"
echo ""

echo -e "${BLUE}Policies (checking finalizers):${NC}"
POLICIES=$(kubectl get clusteradmissionpolicies -o json)
if echo "$POLICIES" | jq -e '.items | length > 0' >/dev/null 2>&1; then
    echo "$POLICIES" | jq -r '.items[] |
        "\(.metadata.name):
          Status: \(.status.policyStatus // "unknown")
          Finalizers: \(.metadata.finalizers // [] | join(", ") | if . == "" then "NONE" else . end)"'

    # Check if any policies have finalizers
    HAS_FINALIZERS=$(echo "$POLICIES" | jq -r '.items[] | select(.metadata.finalizers != null and (.metadata.finalizers | length > 0)) | .metadata.name')
    if [ -z "$HAS_FINALIZERS" ]; then
        success "No policies have finalizers (expected behavior)"
    else
        error "Policies still have finalizers:"
        echo "$HAS_FINALIZERS"
    fi
else
    warn "No policies found"
fi

echo ""
echo -e "${BLUE}ValidatingWebhookConfigurations:${NC}"
WEBHOOKS=$(kubectl get validatingwebhookconfigurations -l app.kubernetes.io/part-of=kubewarden --no-headers 2>/dev/null | wc -l)
if [ "$WEBHOOKS" -eq 0 ]; then
    success "No webhooks remaining (expected behavior)"
else
    error "Webhooks still exist:"
    kubectl get validatingwebhookconfigurations -l app.kubernetes.io/part-of=kubewarden
fi

echo ""
warn "Press Enter to uninstall kubewarden-crds chart (this should NOT hang)..."
read

# Uninstall CRDs chart (this should complete quickly)
log "Uninstalling kubewarden-crds chart..."
START_TIME=$(date +%s)
if timeout 10s helm uninstall kubewarden-crds -n "${NAMESPACE}" --wait; then
    END_TIME=$(date +%s)
    DURATION=$((END_TIME - START_TIME))
    success "kubewarden-crds uninstalled in ${DURATION} seconds (expected: < 5s)"
else
    error "CRD uninstall timed out or failed (indicates finalizer blocking)"
fi

# Verify policies are deleted
echo ""
REMAINING_POLICIES=$(kubectl get clusteradmissionpolicies --no-headers 2>/dev/null | wc -l)
if [ "$REMAINING_POLICIES" -eq 0 ]; then
    success "All policies deleted"
else
    error "${REMAINING_POLICIES} policies still remain"
    kubectl get clusteradmissionpolicies
fi

echo ""
log "Test complete!"
echo ""
echo -e "${GREEN}Summary:${NC}"
echo "  - Controller chart uninstalled successfully"
echo "  - Policies should have NO finalizers after controller uninstall"
echo "  - Webhooks should be removed"
echo "  - CRD chart uninstall should complete in < 5 seconds"

echo ""
warn "Keep cluster for inspection? (y/N)"
read -n 1 -r
echo
if [[ ! $REPLY =~ ^[Yy]$ ]]; then
    log "Deleting cluster..."
    kind delete cluster --name "${CLUSTER_NAME}"
    success "Cluster deleted"
else
    echo ""
    echo -e "${BLUE}Cluster preserved. To inspect:${NC}"
    echo "  kubectl config use-context kind-${CLUSTER_NAME}"
    echo ""
    echo -e "${BLUE}To delete later:${NC}"
    echo "  kind delete cluster --name ${CLUSTER_NAME}"
fi

[Copilot]

Pull request overview

This PR changes policy finalizer handling so controller-managed finalizers are no longer added by admission webhooks at creation time, and are instead added during reconciliation once policy activation can proceed. It also adds Helm pre-delete cleanup logic and tests around the finalizer lifecycle.

Changes:

• Removed finalizer injection from policy and policy group defaulters, with unit test expectations updated.
• Added finalizer add/remove logic to the policy reconciler around active/unavailable policy server states.
• Added a Helm pre-delete hook fallback and new controller integration tests for finalizer lifecycle behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/controller/policy_subreconciler.go	Moves finalizer management into policy reconciliation.
internal/controller/policy_finalizer_test.go	Adds integration-style tests for finalizer lifecycle scenarios.
charts/kubewarden-controller/templates/pre-delete-hook.yaml	Expands uninstall hook to delete PolicyServers and patch remaining policy finalizers.
api/policies/v1/clusteradmissionpolicygroup_webhook.go	Stops adding finalizer during cluster policy group defaulting.
api/policies/v1/clusteradmissionpolicygroup_webhook_test.go	Updates defaulter test to expect no finalizer.
api/policies/v1/clusteradmissionpolicy_webhook.go	Stops adding finalizer during cluster policy defaulting.
api/policies/v1/clusteradmissionpolicy_webhook_test.go	Updates defaulter test to expect no finalizer.
api/policies/v1/admissionpolicygroup_webhook.go	Stops adding finalizer during namespaced policy group defaulting.
api/policies/v1/admissionpolicygroup_webhook_test.go	Updates defaulter test to expect no finalizer.
api/policies/v1/admissionpolicy_webhook.go	Stops adding finalizer during namespaced policy defaulting.
api/policies/v1/admissionpolicy_webhook_test.go	Updates defaulter test to expect no finalizer.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

[viccuad]

Done in a6aebc2.

[viccuad]

Done in a6aebc2, which tackles this and the previous copilot review comment.

[viccuad]

Done in 87dc788.

[viccuad]

This is valid.
In addition, this new block in reconcilePolicyServerUnavailable() is the same as we already have in reconcilePolicyDeletion(), we can extract it to a function.

[viccuad]

This commit on my fork addresses this and also DRYes 2 functions into one: viccuad@817f51b

Please review, I'm happy pushing it to this PR's branch.

[flavio]

I like this refactor, do you want to push a new PR?

[viccuad]

Pushed the commit here

[viccuad]

This can never succeed, as we add a special integrationTestFinalizer in the policy factories: https://github.com/kubewarden/adm-controller/blob/finalizer-removal/api/policies/v1/factories.go#L225-L234

This is the reason why the e2e tests fail on CI.

[viccuad]

Pushed 5401119 that fixes the e2e test errors.

[flavio]

It looks good. We need to address the failures and the some of the comments from copilot, as highlighted by Victor.

[viccuad]

Tackled the opened comments, ready for another review. The e2e tests pass locally for me.

edit: the e2e tests passed on my fork (same commits) https://github.com/viccuad/adm-controller/actions/runs/26567167995/job/78264737960

[jvanz]

LGTM. I cannot "approve" my own PR. But I'm fine with the latest changes from Victor.