From 036f4bc168f3f7137ef6dbadfcfe0bb1022f9eca Mon Sep 17 00:00:00 2001
From: 264nm <c.j.ward264nm@gmail.com>
Date: Fri, 18 Jul 2025 12:48:45 +1000
Subject: [PATCH] Fix: Add garbage collection to handle Approved-Unissued CSRs

---
 .../certificates/cleaner/cleaner.go           | 20 +++++++++++++++-
 .../certificates/cleaner/cleaner_test.go      | 24 +++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/pkg/controller/certificates/cleaner/cleaner.go b/pkg/controller/certificates/cleaner/cleaner.go
index 32c0830d1d580..f98461cba2974 100644
--- a/pkg/controller/certificates/cleaner/cleaner.go
+++ b/pkg/controller/certificates/cleaner/cleaner.go
@@ -107,7 +107,12 @@ func (ccc *CSRCleanerController) worker(ctx context.Context) {
 
 func (ccc *CSRCleanerController) handle(ctx context.Context, csr *capi.CertificateSigningRequest) error {
 	logger := klog.FromContext(ctx)
-	if isIssuedPastDeadline(logger, csr) || isDeniedPastDeadline(logger, csr) || isFailedPastDeadline(logger, csr) || isPendingPastDeadline(logger, csr) || isIssuedExpired(logger, csr) {
+	if isIssuedPastDeadline(logger, csr) ||
+	   isDeniedPastDeadline(logger, csr) ||
+	   isFailedPastDeadline(logger, csr) ||
+	   isPendingPastDeadline(logger, csr) ||
+	   isIssuedExpired(logger, csr) ||
+	   isApprovedUnissuedPastDeadline(logger, csr) {
 		if err := ccc.csrClient.Delete(ctx, csr.Name, metav1.DeleteOptions{}); err != nil {
 			return fmt.Errorf("unable to delete CSR %q: %v", csr.Name, err)
 		}
@@ -179,6 +184,19 @@ func isIssuedPastDeadline(logger klog.Logger, csr *capi.CertificateSigningReques
 	return false
 }
 
+// isApprovedUnissuedPastDeadline checks if the certificate has an Approved status but
+// no certificate has been issued, and the approval time has passed the deadline
+// that pending requests are maintained for.
+func isApprovedUnissuedPastDeadline(logger klog.Logger, csr *capi.CertificateSigningRequest) bool {
+	for _, c := range csr.Status.Conditions {
+		if c.Type == capi.CertificateApproved && !(isIssued(csr) && isOlderThan(c.LastUpdateTime, pendingExpiration)) {
+			logger.Info("Cleaning CSR as it is approved but unissued for more than pendingExpiration duration.", "csr", csr.Name, "pendingExpiration", pendingExpiration)
+			return true
+		}
+	}
+	return false
+}
+
 // isOlderThan checks that t is a non-zero and older than d from time.Now().
 func isOlderThan(t metav1.Time, d time.Duration) bool {
 	return !t.IsZero() && time.Since(t.Time) > d
diff --git a/pkg/controller/certificates/cleaner/cleaner_test.go b/pkg/controller/certificates/cleaner/cleaner_test.go
index 6faeeb7bdf0b8..bb7c3a7d172a0 100644
--- a/pkg/controller/certificates/cleaner/cleaner_test.go
+++ b/pkg/controller/certificates/cleaner/cleaner_test.go
@@ -171,6 +171,30 @@ func TestCleanerWithApprovedExpiredCSR(t *testing.T) {
 			[]capi.CertificateSigningRequestCondition{},
 			[]string{"delete"},
 		},
+		{
+			"delete approved unissued past deadline",
+			metav1.NewTime(time.Now().Add(-1 * time.Minute)),
+			nil,
+			[]capi.CertificateSigningRequestCondition{
+				{
+					Type:           capi.CertificateApproved,
+					LastUpdateTime: metav1.NewTime(time.Now().Add(-25 * time.Hour)),
+				},
+			},
+			[]string{"delete"},
+		},
+		{
+			"no delete approved unissued not past deadline",
+			metav1.NewTime(time.Now().Add(-1 * time.Minute)),
+			nil,
+			[]capi.CertificateSigningRequestCondition{
+				{
+					Type:           capi.CertificateApproved,
+					LastUpdateTime: metav1.NewTime(time.Now().Add(-5 * time.Hour)),
+				},
+			},
+			[]string{},
+		},
 		{
 			"no delete approved not passed deadline unexpired",
 			metav1.NewTime(time.Now().Add(-1 * time.Minute)),