godaddy: fix cleanup (#2270)

providers/dns/godaddy/godaddy.go

@@ -119,13 +119,13 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 
 	ctx := context.Background()
 
-	records, err := d.client.GetRecords(ctx, authZone, "TXT", subDomain)
+	existingRecords, err := d.client.GetRecords(ctx, authZone, "TXT", subDomain)
 	if err != nil {
 		return fmt.Errorf("godaddy: failed to get TXT records: %w", err)
 	}
 
 	var newRecords []internal.DNSRecord
-	for _, record := range records {
+	for _, record := range existingRecords {
 		if record.Data != "" {
 			newRecords = append(newRecords, record)
 		}
@@ -165,34 +165,28 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error {
 
 	ctx := context.Background()
 
-	records, err := d.client.GetRecords(ctx, authZone, "TXT", subDomain)
+	existingRecords, err := d.client.GetRecords(ctx, authZone, "TXT", subDomain)
-	if err != nil {
-		return fmt.Errorf("godaddy: failed to get TXT records: %w", err)
-	}
-
-	if len(records) == 0 {
-		return nil
-	}
-
-	allTxtRecords, err := d.client.GetRecords(ctx, authZone, "TXT", "")
 	if err != nil {
 		return fmt.Errorf("godaddy: failed to get all TXT records: %w", err)
 	}
 
-	var recordsKeep []internal.DNSRecord
+	var recordsToKeep []internal.DNSRecord
-	for _, record := range allTxtRecords {
+	for _, record := range existingRecords {
 		if record.Data != info.Value && record.Data != "" {
-			recordsKeep = append(recordsKeep, record)
+			recordsToKeep = append(recordsToKeep, record)
 		}
 	}
 
-	// GoDaddy API don't provide a way to delete a record, an "empty" record must be added.
+	if len(recordsToKeep) == 0 {
-	if len(recordsKeep) == 0 {
+		err = d.client.DeleteTxtRecords(ctx, authZone, subDomain)
-		emptyRecord := internal.DNSRecord{Name: "empty", Data: ""}
+		if err != nil {
-		recordsKeep = append(recordsKeep, emptyRecord)
+			return fmt.Errorf("godaddy: failed to delete TXT record: %w", err)
+		}
+
+		return nil
 	}
 
-	err = d.client.UpdateTxtRecords(ctx, recordsKeep, authZone, "")
+	err = d.client.UpdateTxtRecords(ctx, recordsToKeep, authZone, subDomain)
 	if err != nil {
 		return fmt.Errorf("godaddy: failed to remove TXT record: %w", err)
 	}

providers/dns/godaddy/internal/client.go

@@ -37,6 +37,8 @@ func NewClient(apiKey string, apiSecret string) *Client {
 	}
 }
 
+// GetRecords retrieves DNS Records for the specified Domain.
+// https://developer.godaddy.com/doc/endpoint/domains#/v1/recordGet
 func (c *Client) GetRecords(ctx context.Context, domainZone, rType, recordName string) ([]DNSRecord, error) {
 	endpoint := c.baseURL.JoinPath("v1", "domains", domainZone, "records", rType, recordName)
 
@@ -54,6 +56,8 @@ func (c *Client) GetRecords(ctx context.Context, domainZone, rType, recordName s
 	return records, nil
 }
 
+// UpdateTxtRecords replaces all DNS Records for the specified Domain with the specified Type.
+// https://developer.godaddy.com/doc/endpoint/domains#/v1/recordReplaceType
 func (c *Client) UpdateTxtRecords(ctx context.Context, records []DNSRecord, domainZone, recordName string) error {
 	endpoint := c.baseURL.JoinPath("v1", "domains", domainZone, "records", "TXT", recordName)
 
@@ -65,6 +69,19 @@ func (c *Client) UpdateTxtRecords(ctx context.Context, records []DNSRecord, doma
 	return c.do(req, nil)
 }
 
+// DeleteTxtRecords deletes all DNS Records for the specified Domain with the specified Type and Name.
+// https://developer.godaddy.com/doc/endpoint/domains#/v1/recordDeleteTypeName
+func (c *Client) DeleteTxtRecords(ctx context.Context, domainZone, recordName string) error {
+	endpoint := c.baseURL.JoinPath("v1", "domains", domainZone, "records", "TXT", recordName)
+
+	req, err := newJSONRequest(ctx, http.MethodDelete, endpoint, nil)
+	if err != nil {
+		return err
+	}
+
+	return c.do(req, nil)
+}
+
 func (c *Client) do(req *http.Request, result any) error {
 	req.Header.Set(authorizationHeader, fmt.Sprintf("sso-key %s:%s", c.apiKey, c.apiSecret))
 
@@ -75,8 +92,8 @@ func (c *Client) do(req *http.Request, result any) error {
 
 	defer func() { _ = resp.Body.Close() }()
 
-	if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode/100 != 2 {
-		return errutils.NewUnexpectedResponseStatusCodeError(req, resp)
+		return parseError(req, resp)
 	}
 
 	if result == nil {
@@ -119,3 +136,15 @@ func newJSONRequest(ctx context.Context, method string, endpoint *url.URL, paylo
 
 	return req, nil
 }
+
+func parseError(req *http.Request, resp *http.Response) error {
+	raw, _ := io.ReadAll(resp.Body)
+
+	var errAPI APIError
+	err := json.Unmarshal(raw, &errAPI)
+	if err != nil {
+		return errutils.NewUnexpectedStatusCodeError(req, resp.StatusCode, raw)
+	}
+
+	return fmt.Errorf("[status code: %d] %w", resp.StatusCode, &errAPI)
+}

providers/dns/godaddy/internal/client_test.go

@@ -55,7 +55,7 @@ func TestClient_GetRecords_errors(t *testing.T) {
 	mux.HandleFunc("/v1/domains/example.com/records/TXT/", testHandler(http.MethodGet, http.StatusUnprocessableEntity, "errors.json"))
 
 	records, err := client.GetRecords(context.Background(), "example.com", "TXT", "")
-	require.Error(t, err)
+	require.EqualError(t, err, "[status code: 422] INVALID_BODY: Request body doesn't fulfill schema, see details in `fields`")
 	assert.Nil(t, records)
 }
 
@@ -104,7 +104,25 @@ func TestClient_UpdateTxtRecords_errors(t *testing.T) {
 	}
 
 	err := client.UpdateTxtRecords(context.Background(), records, "example.com", "lego")
-	require.Error(t, err)
+	require.EqualError(t, err, "[status code: 422] INVALID_BODY: Request body doesn't fulfill schema, see details in `fields`")
+}
+
+func TestClient_DeleteTxtRecords(t *testing.T) {
+	client, mux := setupTest(t)
+
+	mux.HandleFunc("/v1/domains/example.com/records/TXT/foo", testHandler(http.MethodDelete, http.StatusNoContent, ""))
+
+	err := client.DeleteTxtRecords(context.Background(), "example.com", "foo")
+	require.NoError(t, err)
+}
+
+func TestClient_DeleteTxtRecords_errors(t *testing.T) {
+	client, mux := setupTest(t)
+
+	mux.HandleFunc("/v1/domains/example.com/records/TXT/foo", testHandler(http.MethodDelete, http.StatusConflict, "error-extended.json"))
+
+	err := client.DeleteTxtRecords(context.Background(), "example.com", "foo")
+	require.EqualError(t, err, "[status code: 409] ACCESS_DENIED: Authenticated user is not allowed access [test: content (path=/foo) (pathRelated=/bar)]")
 }
 
 func testHandler(method string, statusCode int, filename string) http.HandlerFunc {

providers/dns/godaddy/internal/fixtures/error-extended.json

@@ -0,0 +1,12 @@
+{
+  "code": "ACCESS_DENIED",
+  "fields": [
+    {
+      "code": "test",
+      "message": "content",
+      "path": "/foo",
+      "pathRelated": "/bar"
+    }
+  ],
+  "message": "Authenticated user is not allowed access"
+}

providers/dns/godaddy/internal/types.go

@@ -1,5 +1,7 @@
 package internal
 
+import "fmt"
+
 // DNSRecord a DNS record.
 type DNSRecord struct {
 	Name string `json:"name,omitempty"`
@@ -13,3 +15,42 @@ type DNSRecord struct {
 	Service  string `json:"service,omitempty"`
 	Weight   int    `json:"weight,omitempty"`
 }
+
+type APIError struct {
+	Code    string  `json:"code,omitempty"`
+	Fields  []Field `json:"fields,omitempty"`
+	Message string  `json:"message,omitempty"`
+}
+
+func (a APIError) Error() string {
+	msg := fmt.Sprintf("%s: %s", a.Code, a.Message)
+
+	for _, field := range a.Fields {
+		msg += " " + field.String()
+	}
+
+	return msg
+}
+
+type Field struct {
+	Code        string `json:"code,omitempty"`
+	Message     string `json:"message,omitempty"`
+	Path        string `json:"path,omitempty"`
+	PathRelated string `json:"pathRelated,omitempty"`
+}
+
+func (f Field) String() string {
+	msg := fmt.Sprintf("[%s: %s", f.Code, f.Message)
+
+	if f.Path != "" {
+		msg += fmt.Sprintf(" (path=%s)", f.Path)
+	}
+
+	if f.PathRelated != "" {
+		msg += fmt.Sprintf(" (pathRelated=%s)", f.PathRelated)
+	}
+
+	msg += "]"
+
+	return msg
+}