fix(godaddy): Handle missing Retry-After header gracefully

provider/godaddy/client.go

@@ -28,6 +28,7 @@ import (
 	"strconv"
 	"time"
 
+	log "github.com/sirupsen/logrus"
 	"golang.org/x/time/rate"
 
 	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
@@ -41,6 +42,11 @@ var (
 	ErrAPIDown = errors.New("godaddy: the GoDaddy API is down")
 )
 
+// error codes
+const (
+	ErrCodeQuotaExceeded = "QUOTA_EXCEEDED"
+)
+
 // APIError error
 type APIError struct {
 	Code    string
@@ -129,7 +135,13 @@ func NewClient(useOTE bool, apiKey, apiSecret string) (*Client, error) {
 
 	// Get and check the configuration
 	if err := client.validate(); err != nil {
-		return nil, err
+		var apiErr *APIError
+		// Quota Exceeded errors are limited to the endpoint being called. Other endpoints are not affected when we hit
+		// the quota limit on the endpoint used for validation. We can safely ignore this error.
+		// Quota limits on other endpoints will be logged by their respective calls.
+		if ok := errors.As(err, &apiErr); ok && apiErr.Code != ErrCodeQuotaExceeded {
+			return nil, err
+		}
 	}
 	return &client, nil
 }
@@ -140,56 +152,56 @@ func NewClient(useOTE bool, apiKey, apiSecret string) (*Client, error) {
 
 // Get is a wrapper for the GET method
 func (c *Client) Get(url string, resType interface{}) error {
-	return c.CallAPI("GET", url, nil, resType, true)
+	return c.CallAPI("GET", url, nil, resType)
 }
 
 // Patch is a wrapper for the PATCH method
 func (c *Client) Patch(url string, reqBody, resType interface{}) error {
-	return c.CallAPI("PATCH", url, reqBody, resType, true)
+	return c.CallAPI("PATCH", url, reqBody, resType)
 }
 
 // Post is a wrapper for the POST method
 func (c *Client) Post(url string, reqBody, resType interface{}) error {
-	return c.CallAPI("POST", url, reqBody, resType, true)
+	return c.CallAPI("POST", url, reqBody, resType)
 }
 
 // Put is a wrapper for the PUT method
 func (c *Client) Put(url string, reqBody, resType interface{}) error {
-	return c.CallAPI("PUT", url, reqBody, resType, true)
+	return c.CallAPI("PUT", url, reqBody, resType)
 }
 
 // Delete is a wrapper for the DELETE method
 func (c *Client) Delete(url string, resType interface{}) error {
-	return c.CallAPI("DELETE", url, nil, resType, true)
+	return c.CallAPI("DELETE", url, nil, resType)
 }
 
 // GetWithContext is a wrapper for the GET method
 func (c *Client) GetWithContext(ctx context.Context, url string, resType interface{}) error {
-	return c.CallAPIWithContext(ctx, "GET", url, nil, resType, true)
+	return c.CallAPIWithContext(ctx, "GET", url, nil, resType)
 }
 
 // PatchWithContext is a wrapper for the PATCH method
 func (c *Client) PatchWithContext(ctx context.Context, url string, reqBody, resType interface{}) error {
-	return c.CallAPIWithContext(ctx, "PATCH", url, reqBody, resType, true)
+	return c.CallAPIWithContext(ctx, "PATCH", url, reqBody, resType)
 }
 
 // PostWithContext is a wrapper for the POST method
 func (c *Client) PostWithContext(ctx context.Context, url string, reqBody, resType interface{}) error {
-	return c.CallAPIWithContext(ctx, "POST", url, reqBody, resType, true)
+	return c.CallAPIWithContext(ctx, "POST", url, reqBody, resType)
 }
 
 // PutWithContext is a wrapper for the PUT method
 func (c *Client) PutWithContext(ctx context.Context, url string, reqBody, resType interface{}) error {
-	return c.CallAPIWithContext(ctx, "PUT", url, reqBody, resType, true)
+	return c.CallAPIWithContext(ctx, "PUT", url, reqBody, resType)
 }
 
 // DeleteWithContext is a wrapper for the DELETE method
 func (c *Client) DeleteWithContext(ctx context.Context, url string, resType interface{}) error {
-	return c.CallAPIWithContext(ctx, "DELETE", url, nil, resType, true)
+	return c.CallAPIWithContext(ctx, "DELETE", url, nil, resType)
 }
 
 // NewRequest returns a new HTTP request
-func (c *Client) NewRequest(method, path string, reqBody interface{}, needAuth bool) (*http.Request, error) {
+func (c *Client) NewRequest(method, path string, reqBody interface{}) (*http.Request, error) {
 	var body []byte
 	var err error
 
@@ -228,9 +240,16 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 
 	c.Ratelimiter.Wait(req.Context())
 	resp, err := c.Client.Do(req)
+	if err != nil {
+		return nil, err
+	}
 	// In case of several clients behind NAT we still can hit rate limit
-	for i := 1; i < 3 && err == nil && resp.StatusCode == 429; i++ {
-		retryAfter, _ := strconv.ParseInt(resp.Header.Get("Retry-After"), 10, 0)
+	for i := 1; i < 3 && resp != nil && resp.StatusCode == 429; i++ {
+		retryAfter, err := strconv.ParseInt(resp.Header.Get("Retry-After"), 10, 0)
+		if err != nil {
+			log.Error("Rate-limited response did not contain a valid Retry-After header, quota likely exceeded")
+			break
+		}
 
 		jitter := rand.Int63n(retryAfter)
 		retryAfterSec := retryAfter + jitter/2
@@ -240,9 +259,9 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 
 		c.Ratelimiter.Wait(req.Context())
 		resp, err = c.Client.Do(req)
-	}
-	if err != nil {
-		return nil, err
+		if err != nil {
+			return nil, fmt.Errorf("doing request after waiting for retry after: %w", err)
+		}
 	}
 	if c.Logger != nil {
 		c.Logger.LogResponse(resp)
@@ -268,8 +287,8 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 //
 // If everything went fine, unmarshall response into resType and return nil
 // otherwise, return the error
-func (c *Client) CallAPI(method, path string, reqBody, resType interface{}, needAuth bool) error {
-	return c.CallAPIWithContext(context.Background(), method, path, reqBody, resType, needAuth)
+func (c *Client) CallAPI(method, path string, reqBody, resType interface{}) error {
+	return c.CallAPIWithContext(context.Background(), method, path, reqBody, resType)
 }
 
 // CallAPIWithContext is the lowest level call helper. If needAuth is true,
@@ -292,8 +311,8 @@ func (c *Client) CallAPI(method, path string, reqBody, resType interface{}, need
 //
 // If everything went fine, unmarshall response into resType and return nil
 // otherwise, return the error
-func (c *Client) CallAPIWithContext(ctx context.Context, method, path string, reqBody, resType interface{}, needAuth bool) error {
-	req, err := c.NewRequest(method, path, reqBody, needAuth)
+func (c *Client) CallAPIWithContext(ctx context.Context, method, path string, reqBody, resType interface{}) error {
+	req, err := c.NewRequest(method, path, reqBody)
 	if err != nil {
 		return err
 	}

provider/godaddy/client_test.go

@@ -0,0 +1,72 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package godaddy
+
+import (
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"golang.org/x/time/rate"
+)
+
+// Tests that
+func TestClient_DoWhenQuotaExceeded(t *testing.T) {
+	assert := assert.New(t)
+
+	// Mock server to return 429 with a JSON payload
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusTooManyRequests)
+		_, err := w.Write([]byte(`{"code": "QUOTA_EXCEEDED", "message": "rate limit exceeded"}`))
+		if err != nil {
+			t.Fatalf("Failed to write response: %v", err)
+		}
+	}))
+	defer mockServer.Close()
+
+	client := Client{
+		APIKey:      "",
+		APISecret:   "",
+		APIEndPoint: mockServer.URL,
+		Client:      &http.Client{},
+		// Add one token every second
+		Ratelimiter: rate.NewLimiter(rate.Every(time.Second), 60),
+		Timeout:     DefaultTimeout,
+	}
+
+	req, err := client.NewRequest("GET", "/v1/domains/example.net/records", nil)
+	if err != nil {
+		t.Fatalf("Failed to create request: %v", err)
+	}
+
+	resp, err := client.Do(req)
+	assert.Nil(err, "A CODE_EXCEEDED response should not return an error")
+	assert.Equal(http.StatusTooManyRequests, resp.StatusCode, "Expected a 429 response")
+
+	respContents := GDErrorResponse{}
+	err = client.UnmarshalResponse(resp, &respContents)
+	if assert.NotNil(err) {
+		var apiErr *APIError
+		errors.As(err, &apiErr)
+		assert.Equal("QUOTA_EXCEEDED", apiErr.Code)
+		assert.Equal("rate limit exceeded", apiErr.Message)
+	}
+}