Fix cert provisioner clearing valid challenges on transient errors

Signed-off-by: Bryan Frimin <bryan@getprobo.com>

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,9 +307,10 @@ func (p *Provisioner) provisionDomainCertificate(
 			return nil
 		}
 
+		if isFatalChallengeError(err) {
 			p.logger.InfoCtx(
 				ctx,
-			"resetting domain to retry with fresh challenge",
+				"fatal challenge error, resetting domain to retry with fresh challenge",
 				log.String("domain", domain.Domain),
 				log.Int("retry_count", fullDomain.SSLRetryCount),
 			)
@@ -314,12 +331,26 @@ func (p *Provisioner) provisionDomainCertificate(
 				return updateErr
 			}
 
+			return nil
+		}
+
 		p.logger.InfoCtx(
 			ctx,
-			"domain reset to pending, will retry with new challenge on next cycle",
+			"transient error, keeping existing challenge for retry",
 			log.String("domain", domain.Domain),
+			log.Int("retry_count", fullDomain.SSLRetryCount),
 		)
 
+		if updateErr := fullDomain.Update(ctx, conn, coredata.NewNoScope(), p.encryptionKey); updateErr != nil {
+			p.logger.ErrorCtx(
+				ctx,
+				"cannot update domain retry tracking",
+				log.String("domain", domain.Domain),
+				log.Error(updateErr),
+			)
+			return updateErr
+		}
+
 		return nil
 	}
 

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(