cloudns: fix TTL and status validation (#856)

providers/dns/cloudns/cloudns.go

@@ -27,7 +27,7 @@ func NewDefaultConfig() *Config {
 	return &Config{
 		PropagationTimeout: env.GetOrDefaultSecond("CLOUDNS_PROPAGATION_TIMEOUT", 120*time.Second),
 		PollingInterval:    env.GetOrDefaultSecond("CLOUDNS_POLLING_INTERVAL", 4*time.Second),
-		TTL:                env.GetOrDefaultInt("CLOUDNS_TTL", dns01.DefaultTTL),
+		TTL:                env.GetOrDefaultInt("CLOUDNS_TTL", 60),
 		HTTPClient: &http.Client{
 			Timeout: env.GetOrDefaultSecond("CLOUDNS_HTTP_TIMEOUT", 30*time.Second),
 		},
@@ -64,7 +64,7 @@ func NewDNSProviderConfig(config *Config) (*DNSProvider, error) {
 
 	client, err := internal.NewClient(config.AuthID, config.AuthPassword)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("ClouDNS: %v", err)
 	}
 
 	client.HTTPClient = config.HTTPClient
@@ -78,10 +78,15 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 
 	zone, err := d.client.GetZone(fqdn)
 	if err != nil {
-		return err
+		return fmt.Errorf("ClouDNS: %v", err)
 	}
 
-	return d.client.AddTxtRecord(zone.Name, fqdn, value, d.config.TTL)
+	err = d.client.AddTxtRecord(zone.Name, fqdn, value, d.config.TTL)
+	if err != nil {
+		return fmt.Errorf("ClouDNS: %v", err)
+	}
+
+	return nil
 }
 
 // CleanUp removes the TXT record matching the specified parameters.
@@ -90,15 +95,23 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error {
 
 	zone, err := d.client.GetZone(fqdn)
 	if err != nil {
-		return err
+		return fmt.Errorf("ClouDNS: %v", err)
 	}
 
 	record, err := d.client.FindTxtRecord(zone.Name, fqdn)
 	if err != nil {
-		return err
+		return fmt.Errorf("ClouDNS: %v", err)
 	}
 
-	return d.client.RemoveTxtRecord(record.ID, zone.Name)
+	if record == nil {
+		return nil
+	}
+
+	err = d.client.RemoveTxtRecord(record.ID, zone.Name)
+	if err != nil {
+		return fmt.Errorf("ClouDNS: %v", err)
+	}
+	return nil
 }
 
 // Timeout returns the timeout and interval to use when checking for DNS propagation.

providers/dns/cloudns/internal/client.go

@@ -2,6 +2,7 @@ package internal
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -14,6 +15,11 @@ import (
 
 const defaultBaseURL = "https://api.cloudns.net/dns/"
 
+type apiResponse struct {
+	Status            string `json:"status"`
+	StatusDescription string `json:"statusDescription"`
+}
+
 type Zone struct {
 	Name   string
 	Type   string
@@ -37,11 +43,11 @@ type TXTRecords map[string]TXTRecord
 // NewClient creates a ClouDNS client
 func NewClient(authID string, authPassword string) (*Client, error) {
 	if authID == "" {
-		return nil, fmt.Errorf("ClouDNS: credentials missing: authID")
+		return nil, fmt.Errorf("credentials missing: authID")
 	}
 
 	if authPassword == "" {
-		return nil, fmt.Errorf("ClouDNS: credentials missing: authPassword")
+		return nil, fmt.Errorf("credentials missing: authPassword")
 	}
 
 	baseURL, err := url.Parse(defaultBaseURL)
@@ -90,7 +96,7 @@ func (c *Client) GetZone(authFQDN string) (*Zone, error) {
 
 	if len(result) > 0 {
 		if err = json.Unmarshal(result, &zone); err != nil {
-			return nil, fmt.Errorf("ClouDNS: zone unmarshaling error: %v", err)
+			return nil, fmt.Errorf("zone unmarshaling error: %v", err)
 		}
 	}
 
@@ -98,7 +104,7 @@ func (c *Client) GetZone(authFQDN string) (*Zone, error) {
 		return &zone, nil
 	}
 
-	return nil, fmt.Errorf("ClouDNS: zone %s not found for authFQDN %s", authZoneName, authFQDN)
+	return nil, fmt.Errorf("zone %s not found for authFQDN %s", authZoneName, authFQDN)
 }
 
 // FindTxtRecord return the TXT record a zone ID and a FQDN
@@ -119,9 +125,14 @@ func (c *Client) FindTxtRecord(zoneName, fqdn string) (*TXTRecord, error) {
 		return nil, err
 	}
 
+	// the API returns [] when there is no records.
+	if string(result) == "[]" {
+		return nil, nil
+	}
+
 	var records TXTRecords
 	if err = json.Unmarshal(result, &records); err != nil {
-		return nil, fmt.Errorf("ClouDNS: TXT record unmarshaling error: %v", err)
+		return nil, fmt.Errorf("TXT record unmarshaling error: %v: %s", err, string(result))
 	}
 
 	for _, record := range records {
@@ -130,7 +141,7 @@ func (c *Client) FindTxtRecord(zoneName, fqdn string) (*TXTRecord, error) {
 		}
 	}
 
-	return nil, fmt.Errorf("ClouDNS: no existing record found for %q", fqdn)
+	return nil, nil
 }
 
 // AddTxtRecord add a TXT record
@@ -144,12 +155,25 @@ func (c *Client) AddTxtRecord(zoneName string, fqdn, value string, ttl int) erro
 	q.Add("domain-name", zoneName)
 	q.Add("host", host)
 	q.Add("record", value)
-	q.Add("ttl", strconv.Itoa(ttl))
+	q.Add("ttl", strconv.Itoa(ttlRounder(ttl)))
 	q.Add("record-type", "TXT")
 	reqURL.RawQuery = q.Encode()
 
-	_, err := c.doRequest(http.MethodPost, &reqURL)
+	raw, err := c.doRequest(http.MethodPost, &reqURL)
-	return err
+	if err != nil {
+		return err
+	}
+
+	resp := apiResponse{}
+	if err = json.Unmarshal(raw, &resp); err != nil {
+		return fmt.Errorf("apiResponse unmarshaling error: %v: %s", err, string(raw))
+	}
+
+	if resp.Status != "Success" {
+		return fmt.Errorf("fail to add TXT record: %s %s", resp.Status, resp.StatusDescription)
+	}
+
+	return nil
 }
 
 // RemoveTxtRecord remove a TXT record
@@ -162,8 +186,21 @@ func (c *Client) RemoveTxtRecord(recordID int, zoneName string) error {
 	q.Add("record-id", strconv.Itoa(recordID))
 	reqURL.RawQuery = q.Encode()
 
-	_, err := c.doRequest(http.MethodPost, &reqURL)
+	raw, err := c.doRequest(http.MethodPost, &reqURL)
-	return err
+	if err != nil {
+		return err
+	}
+
+	resp := apiResponse{}
+	if err = json.Unmarshal(raw, &resp); err != nil {
+		return fmt.Errorf("apiResponse unmarshaling error: %v: %s", err, string(raw))
+	}
+
+	if resp.Status != "Success" {
+		return fmt.Errorf("fail to add TXT record: %s %s", resp.Status, resp.StatusDescription)
+	}
+
+	return nil
 }
 
 func (c *Client) doRequest(method string, url *url.URL) (json.RawMessage, error) {
@@ -174,18 +211,18 @@ func (c *Client) doRequest(method string, url *url.URL) (json.RawMessage, error)
 
 	resp, err := c.HTTPClient.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("ClouDNS: %v", err)
+		return nil, err
 	}
 
 	defer resp.Body.Close()
 
 	content, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("ClouDNS: %s", toUnreadableBodyMessage(req, content))
+		return nil, errors.New(toUnreadableBodyMessage(req, content))
 	}
 
 	if resp.StatusCode != 200 {
-		return nil, fmt.Errorf("ClouDNS: invalid code (%v), error: %s", resp.StatusCode, content)
+		return nil, fmt.Errorf("invalid code (%v), error: %s", resp.StatusCode, content)
 	}
 	return content, nil
 }
@@ -198,7 +235,7 @@ func (c *Client) buildRequest(method string, url *url.URL) (*http.Request, error
 
 	req, err := http.NewRequest(method, url.String(), nil)
 	if err != nil {
-		return nil, fmt.Errorf("ClouDNS: invalid request: %v", err)
+		return nil, fmt.Errorf("invalid request: %v", err)
 	}
 
 	return req, nil
@@ -207,3 +244,28 @@ func (c *Client) buildRequest(method string, url *url.URL) (*http.Request, error
 func toUnreadableBodyMessage(req *http.Request, rawBody []byte) string {
 	return fmt.Sprintf("the request %s sent a response with a body which is an invalid format: %q", req.URL, string(rawBody))
 }
+
+// https://www.cloudns.net/wiki/article/58/
+// Available TTL's:
+// 60 = 1 minute
+// 300 = 5 minutes
+// 900 = 15 minutes
+// 1800 = 30 minutes
+// 3600 = 1 hour
+// 21600 = 6 hours
+// 43200 = 12 hours
+// 86400 = 1 day
+// 172800 = 2 days
+// 259200 = 3 days
+// 604800 = 1 week
+// 1209600 = 2 weeks
+// 2592000 = 1 month
+func ttlRounder(ttl int) int {
+	for _, validTTL := range []int{60, 300, 900, 1800, 3600, 21600, 43200, 86400, 172800, 259200, 604800, 1209600} {
+		if ttl <= validTTL {
+			return validTTL
+		}
+	}
+
+	return 2592000
+}

providers/dns/cloudns/internal/client_test.go

@@ -92,18 +92,37 @@ func TestClientFindTxtRecord(t *testing.T) {
 		expected    result
 	}{
 		{
-			desc:        "record found",
+			desc:     "record found",
-			authFQDN:    "_acme-challenge.foo.com.",
+			authFQDN: "_acme-challenge.foo.com.",
-			zoneName:    "foo.com",
+			zoneName: "foo.com",
-			apiResponse: []byte(`{"1":{"id":"1","type":"TXT","host":"_acme-challenge","record":"txtTXTtxtTXTtxtTXTtxtTXT","failover":"1","ttl":"30","status":1}}`),
+			apiResponse: []byte(`{
+  "5769228": {
+    "id": "5769228",
+    "type": "TXT",
+    "host": "_acme-challenge",
+    "record": "txtTXTtxtTXTtxtTXTtxtTXT",
+    "failover": "0",
+    "ttl": "3600",
+    "status": 1
+  },
+  "181805209": {
+    "id": "181805209",
+    "type": "TXT",
+    "host": "_github-challenge",
+    "record": "b66b8324b5",
+    "failover": "0",
+    "ttl": "300",
+    "status": 1
+  }
+}`),
 			expected: result{
 				txtRecord: &TXTRecord{
-					ID:       1,
+					ID:       5769228,
 					Type:     "TXT",
 					Host:     "_acme-challenge",
 					Record:   "txtTXTtxtTXTtxtTXTtxtTXT",
-					Failover: 1,
+					Failover: 0,
-					TTL:      30,
+					TTL:      3600,
 					Status:   1,
 				},
 			},
@@ -112,8 +131,8 @@ func TestClientFindTxtRecord(t *testing.T) {
 			desc:        "record not found",
 			authFQDN:    "_acme-challenge.foo.com.",
 			zoneName:    "test-zone",
-			apiResponse: []byte(``),
+			apiResponse: []byte(`[]`),
-			expected:    result{error: true},
+			expected:    result{txtRecord: nil},
 		},
 	}
 
@@ -138,13 +157,19 @@ func TestClientFindTxtRecord(t *testing.T) {
 }
 
 func TestClientAddTxtRecord(t *testing.T) {
+	type expected struct {
+		Query string
+		Error string
+	}
+
 	testCases := []struct {
-		desc     string
+		desc        string
-		zone     *Zone
+		zone        *Zone
-		authFQDN string
+		authFQDN    string
-		value    string
+		value       string
-		ttl      int
+		ttl         int
-		expected string
+		apiResponse []byte
+		expected    expected
 	}{
 		{
 			desc: "sub-zone",
@@ -154,10 +179,13 @@ func TestClientAddTxtRecord(t *testing.T) {
 				Zone:   "domain",
 				Status: "1",
 			},
-			authFQDN: "_acme-challenge.foo.bar.com.",
+			authFQDN:    "_acme-challenge.foo.bar.com.",
-			value:    "txtTXTtxtTXTtxtTXTtxtTXT",
+			value:       "txtTXTtxtTXTtxtTXTtxtTXT",
-			ttl:      60,
+			ttl:         60,
-			expected: `auth-id=myAuthID&auth-password=myAuthPassword&domain-name=bar.com&host=_acme-challenge.foo&record=txtTXTtxtTXTtxtTXTtxtTXT&record-type=TXT&ttl=60`,
+			apiResponse: []byte(`{"status":"Success","statusDescription":"The record was added successfully."}`),
+			expected: expected{
+				Query: `auth-id=myAuthID&auth-password=myAuthPassword&domain-name=bar.com&host=_acme-challenge.foo&record=txtTXTtxtTXTtxtTXTtxtTXT&record-type=TXT&ttl=60`,
+			},
 		},
 		{
 			desc: "main zone",
@@ -167,10 +195,30 @@ func TestClientAddTxtRecord(t *testing.T) {
 				Zone:   "domain",
 				Status: "1",
 			},
-			authFQDN: "_acme-challenge.bar.com.",
+			authFQDN:    "_acme-challenge.bar.com.",
-			value:    "TXTtxtTXTtxtTXTtxtTXTtxt",
+			value:       "TXTtxtTXTtxtTXTtxtTXTtxt",
-			ttl:      60,
+			ttl:         60,
-			expected: `auth-id=myAuthID&auth-password=myAuthPassword&domain-name=bar.com&host=_acme-challenge&record=TXTtxtTXTtxtTXTtxtTXTtxt&record-type=TXT&ttl=60`,
+			apiResponse: []byte(`{"status":"Success","statusDescription":"The record was added successfully."}`),
+			expected: expected{
+				Query: `auth-id=myAuthID&auth-password=myAuthPassword&domain-name=bar.com&host=_acme-challenge&record=TXTtxtTXTtxtTXTtxtTXTtxt&record-type=TXT&ttl=60`,
+			},
+		},
+		{
+			desc: "invalid status",
+			zone: &Zone{
+				Name:   "bar.com",
+				Type:   "master",
+				Zone:   "domain",
+				Status: "1",
+			},
+			authFQDN:    "_acme-challenge.bar.com.",
+			value:       "TXTtxtTXTtxtTXTtxtTXTtxt",
+			ttl:         120,
+			apiResponse: []byte(`{"status":"Failed","statusDescription":"Invalid TTL. Choose from the list of the values we support."}`),
+			expected: expected{
+				Query: `auth-id=myAuthID&auth-password=myAuthPassword&domain-name=bar.com&host=_acme-challenge&record=TXTtxtTXTtxtTXTtxtTXTtxt&record-type=TXT&ttl=300`,
+				Error: "fail to add TXT record: Failed Invalid TTL. Choose from the list of the values we support.",
+			},
 		},
 	}
 
@@ -178,9 +226,9 @@ func TestClientAddTxtRecord(t *testing.T) {
 		t.Run(test.desc, func(t *testing.T) {
 			server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 				assert.NotNil(t, req.URL.RawQuery)
-				assert.Equal(t, test.expected, req.URL.RawQuery)
+				assert.Equal(t, test.expected.Query, req.URL.RawQuery)
 
-				handlerMock(http.MethodPost, nil).ServeHTTP(rw, req)
+				handlerMock(http.MethodPost, test.apiResponse).ServeHTTP(rw, req)
 			}))
 
 			client, _ := NewClient("myAuthID", "myAuthPassword")
@@ -188,7 +236,12 @@ func TestClientAddTxtRecord(t *testing.T) {
 			client.BaseURL = mockBaseURL
 
 			err := client.AddTxtRecord(test.zone.Name, test.authFQDN, test.value, test.ttl)
-			require.NoError(t, err)
+
+			if test.expected.Error != "" {
+				require.EqualError(t, err, test.expected.Error)
+			} else {
+				require.NoError(t, err)
+			}
 		})
 	}
 }