From 84a80b946931182dcf127eb372f2eaf6129ce457 Mon Sep 17 00:00:00 2001
From: Bryan Frimin <bryan@getprobo.com>
Date: Tue, 14 Oct 2025 15:39:20 +0200
Subject: [PATCH] Fix cert provisioner clearing valid challenges on transient
 errors

Signed-off-by: Bryan Frimin <bryan@getprobo.com>
---
 pkg/certmanager/provisioner.go | 59 ++++++++++++++++++++++++++--------
 pkg/certmanager/renewer.go     |  5 +--
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/pkg/certmanager/provisioner.go b/pkg/certmanager/provisioner.go
index aecfef3d6..bbcbe2c73 100644
--- a/pkg/certmanager/provisioner.go
+++ b/pkg/certmanager/provisioner.go
@@ -17,6 +17,7 @@ package certmanager
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/getprobo/probo/pkg/coredata"
@@ -115,6 +116,21 @@ func (p *Provisioner) checkPendingDomains(ctx context.Context) error {
 	)
 }
 
+func isFatalChallengeError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	errStr := strings.ToLower(err.Error())
+
+	return (strings.Contains(errStr, "invalid") &&
+		(strings.Contains(errStr, "challenge") ||
+			strings.Contains(errStr, "authorization") ||
+			strings.Contains(errStr, "order"))) ||
+		strings.Contains(errStr, "authorization must be pending") ||
+		strings.Contains(errStr, "expired")
+}
+
 func (p *Provisioner) handleStaleProvisioningAttempts(ctx context.Context, conn pg.Conn) error {
 	var domains coredata.CustomDomains
 	if err := domains.ListStaleProvisioningDomains(ctx, conn, coredata.NewNoScope()); err != nil {
@@ -291,35 +307,50 @@ func (p *Provisioner) provisionDomainCertificate(
 			return nil
 		}
 
+		if isFatalChallengeError(err) {
+			p.logger.InfoCtx(
+				ctx,
+				"fatal challenge error, resetting domain to retry with fresh challenge",
+				log.String("domain", domain.Domain),
+				log.Int("retry_count", fullDomain.SSLRetryCount),
+			)
+
+			fullDomain.HTTPChallengeToken = nil
+			fullDomain.HTTPChallengeKeyAuth = nil
+			fullDomain.HTTPChallengeURL = nil
+			fullDomain.HTTPOrderURL = nil
+			fullDomain.SSLStatus = coredata.CustomDomainSSLStatusPending
+
+			if updateErr := fullDomain.Update(ctx, conn, coredata.NewNoScope(), p.encryptionKey); updateErr != nil {
+				p.logger.ErrorCtx(
+					ctx,
+					"cannot reset domain for retry",
+					log.String("domain", domain.Domain),
+					log.Error(updateErr),
+				)
+				return updateErr
+			}
+
+			return nil
+		}
+
 		p.logger.InfoCtx(
 			ctx,
-			"resetting domain to retry with fresh challenge",
+			"transient error, keeping existing challenge for retry",
 			log.String("domain", domain.Domain),
 			log.Int("retry_count", fullDomain.SSLRetryCount),
 		)
 
-		fullDomain.HTTPChallengeToken = nil
-		fullDomain.HTTPChallengeKeyAuth = nil
-		fullDomain.HTTPChallengeURL = nil
-		fullDomain.HTTPOrderURL = nil
-		fullDomain.SSLStatus = coredata.CustomDomainSSLStatusPending
-
 		if updateErr := fullDomain.Update(ctx, conn, coredata.NewNoScope(), p.encryptionKey); updateErr != nil {
 			p.logger.ErrorCtx(
 				ctx,
-				"cannot reset domain for retry",
+				"cannot update domain retry tracking",
 				log.String("domain", domain.Domain),
 				log.Error(updateErr),
 			)
 			return updateErr
 		}
 
-		p.logger.InfoCtx(
-			ctx,
-			"domain reset to pending, will retry with new challenge on next cycle",
-			log.String("domain", domain.Domain),
-		)
-
 		return nil
 	}
 
diff --git a/pkg/certmanager/renewer.go b/pkg/certmanager/renewer.go
index 50c535f51..edd66d2a1 100644
--- a/pkg/certmanager/renewer.go
+++ b/pkg/certmanager/renewer.go
@@ -208,7 +208,7 @@ func (r *Renewer) renewDomain(ctx context.Context, conn pg.Conn, domain *coredat
 				return updateErr
 			}
 
-			return nil
+			return fmt.Errorf("domain marked as failed after %d retry attempts: %w", maxRetries, err)
 		}
 
 		// Update retry tracking but keep domain ACTIVE for next renewal cycle
@@ -229,7 +229,8 @@ func (r *Renewer) renewDomain(ctx context.Context, conn pg.Conn, domain *coredat
 			log.Int("retry_count", lockedDomain.SSLRetryCount),
 		)
 
-		return nil
+		// Return the original error so caller knows renewal failed
+		return fmt.Errorf("renewal failed, will retry: %w", err)
 	}
 
 	r.logger.InfoCtx(