From 9509928453997cbc3ed90da528f4733ed0a78f23 Mon Sep 17 00:00:00 2001
From: Sridhar Gaddam <sgaddam@redhat.com>
Date: Sat, 1 Nov 2025 01:07:22 +0530
Subject: [PATCH] Prevent pruning IstioRevisions with unknown InUse status

Currently, the pruning logic would delete IstioRevisions when their
InUse condition status was ConditionUnknown. This could occur when
there are any transient API server errors while listing pods/namespaces.
This fix implements a fail-safe approach by pruning only revisions that
are explicitly confirmed to be not in use (Status == ConditionFalse).
Revisions with ConditionTrue (inUse) or ConditionUnknown are now preserved,
preventing accidental deletion of active control planes.

Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com>
---
 pkg/revision/prune.go      | 10 ++++++++--
 pkg/revision/prune_test.go | 30 ++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/pkg/revision/prune.go b/pkg/revision/prune.go
index 9e8c0e73a..355e570d1 100644
--- a/pkg/revision/prune.go
+++ b/pkg/revision/prune.go
@@ -46,11 +46,17 @@ func PruneInactive(ctx context.Context, cl client.Client, ownerUID types.UID, ac
 			continue
 		}
 		inUseCondition := rev.Status.GetCondition(v1.IstioRevisionConditionInUse)
-		inUse := inUseCondition.Status == metav1.ConditionTrue
-		if inUse {
+
+		// Only prune revisions that are confirmed to be not in use (i.e., ConditionFalse).
+		// Skip revisions that are in use (ConditionTrue) or whose usage status is unknown (ConditionUnknown).
+		if inUseCondition.Status == metav1.ConditionTrue {
 			log.V(2).Info("IstioRevision is in use", "IstioRevision", rev.Name)
 			continue
 		}
+		if inUseCondition.Status != metav1.ConditionFalse {
+			log.V(2).Info("IstioRevision usage status is unknown, skipping pruning", "IstioRevision", rev.Name)
+			continue
+		}
 
 		pruneTimestamp := inUseCondition.LastTransitionTime.Time.Add(gracePeriod)
 		expired := pruneTimestamp.Before(time.Now())
diff --git a/pkg/revision/prune_test.go b/pkg/revision/prune_test.go
index 79956d28f..34bbdea3b 100644
--- a/pkg/revision/prune_test.go
+++ b/pkg/revision/prune_test.go
@@ -172,6 +172,29 @@ func TestPruneInactive(t *testing.T) {
 			expectDeletion:     true,
 			expectRequeueAfter: ptr.Of((v1.DefaultRevisionDeletionGracePeriodSeconds - 25) * time.Second),
 		},
+		{
+			name:           "preserves non-active IstioRevision with unknown usage status",
+			revName:        istioName + "-non-active",
+			ownerReference: ownedByIstio,
+			inUseCondition: &v1.IstioRevisionCondition{
+				Type:               v1.IstioRevisionConditionInUse,
+				Status:             metav1.ConditionUnknown,
+				Reason:             v1.IstioRevisionReasonUsageCheckFailed,
+				Message:            "failed to determine if revision is in use",
+				LastTransitionTime: oneMinuteAgo,
+			},
+			expectDeletion:     false,
+			expectRequeueAfter: nil,
+		},
+		{
+			name:           "preserves non-active IstioRevision with missing InUse condition",
+			revName:        istioName + "-non-active",
+			ownerReference: ownedByIstio,
+			// No InUse condition is set to simulate GetCondition returning ConditionUnknown
+			inUseCondition:     nil,
+			expectDeletion:     false,
+			expectRequeueAfter: nil,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -191,9 +214,12 @@ func TestPruneInactive(t *testing.T) {
 					Name:            tc.revName,
 					OwnerReferences: []metav1.OwnerReference{tc.ownerReference},
 				},
-				Status: v1.IstioRevisionStatus{
+			}
+
+			if tc.inUseCondition != nil {
+				rev.Status = v1.IstioRevisionStatus{
 					Conditions: []v1.IstioRevisionCondition{*tc.inUseCondition},
-				},
+				}
 			}
 
 			initObjs := []client.Object{istio, rev}