diff --git a/internal/poweradmin/client.go b/internal/poweradmin/client.go index 6d3b895..47d416b 100644 --- a/internal/poweradmin/client.go +++ b/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 } diff --git a/internal/poweradmin/client_test.go b/internal/poweradmin/client_test.go index b34f18f..3356f56 100644 --- a/internal/poweradmin/client_test.go +++ b/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) } }