fix: idempotent record delete on 404 and surface API error messages

internal/poweradmin/client.go

@@ -28,6 +28,16 @@ type apiResponse struct {
 	Message string          `json:"message"`
 }
 
+// apiErrorf builds an error from a non-success PowerAdmin API response,
+// preferring the structured "message" field over the raw body when present.
+func apiErrorf(op string, status int, body []byte) error {
+	var resp apiResponse
+	if err := json.Unmarshal(body, &resp); err == nil && resp.Message != "" {
+		return fmt.Errorf("%s: PowerAdmin API returned HTTP %d: %s", op, status, resp.Message)
+	}
+	return fmt.Errorf("%s: PowerAdmin API returned HTTP %d: %s", op, status, string(body))
+}
+
 // v2ZonesData wraps the v2 zones response where data is {"zones": [...]}.
 type v2ZonesData struct {
 	Zones []Zone `json:"zones"`
@@ -103,7 +113,7 @@ func (c *client) GetZones(ctx context.Context) ([]Zone, error) {
 		return nil, err
 	}
 	if status != http.StatusOK {
-		return nil, fmt.Errorf("PowerAdmin API returned HTTP %d: %s", status, string(body))
+		return nil, apiErrorf("list zones", status, body)
 	}
 
 	var resp apiResponse
@@ -148,7 +158,7 @@ func (c *client) ListTXTRecords(ctx context.Context, zoneID int) ([]Record, erro
 		return nil, err
 	}
 	if status != http.StatusOK {
-		return nil, fmt.Errorf("PowerAdmin API returned HTTP %d: %s", status, string(body))
+		return nil, apiErrorf("list TXT records", status, body)
 	}
 
 	var resp apiResponse
@@ -196,7 +206,7 @@ func (c *client) CreateTXTRecord(ctx context.Context, zoneID int, name, content
 		return nil, err
 	}
 	if status != http.StatusCreated && status != http.StatusOK {
-		return nil, fmt.Errorf("PowerAdmin API returned HTTP %d: %s", status, string(body))
+		return nil, apiErrorf("create TXT record", status, body)
 	}
 
 	var resp apiResponse
@@ -237,8 +247,13 @@ func (c *client) DeleteRecord(ctx context.Context, zoneID int, recordID int) err
 	if err != nil {
 		return err
 	}
+	// A missing record means it is already gone; treat as success so ACME
+	// CleanUp stays idempotent across cert-manager retries.
+	if status == http.StatusNotFound {
+		return nil
+	}
 	if status != http.StatusNoContent && status != http.StatusOK {
-		return fmt.Errorf("PowerAdmin API returned HTTP %d: %s", status, string(body))
+		return apiErrorf("delete record", status, body)
 	}
 	return nil
 }

internal/poweradmin/client_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
@@ -458,14 +459,31 @@ func TestListTXTRecords_EmptyZone(t *testing.T) {
 
 func TestDeleteRecord_HTTPErrors(t *testing.T) {
 	handler := func(w http.ResponseWriter, r *http.Request) {
-		w.WriteHeader(http.StatusNotFound)
-		_, _ = w.Write([]byte(`{"error":"not found"}`))
+		w.WriteHeader(http.StatusInternalServerError)
+		_, _ = w.Write([]byte(`{"success":false,"message":"Failed to delete record"}`))
 	}
 
 	_, client := setupTestServerWithVersion(t, handler, "v2")
 	err := client.DeleteRecord(context.Background(), 1, 999)
 	if err == nil {
-		t.Error("expected error for 404 response on delete")
+		t.Fatal("expected error for 500 response on delete")
+	}
+	if !strings.Contains(err.Error(), "Failed to delete record") {
+		t.Errorf("expected error to surface the API message, got: %v", err)
+	}
+}
+
+// A 404 on delete means the record is already gone; ACME CleanUp must stay
+// idempotent across cert-manager retries, so this is treated as success.
+func TestDeleteRecord_NotFoundIsIdempotent(t *testing.T) {
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		_, _ = w.Write([]byte(`{"success":false,"message":"Record not found in this zone"}`))
+	}
+
+	_, client := setupTestServerWithVersion(t, handler, "v2")
+	if err := client.DeleteRecord(context.Background(), 1, 999); err != nil {
+		t.Errorf("expected nil (idempotent) for 404 on delete, got: %v", err)
 	}
 }