netcup: make unmarshalling of api-responses more leniant. (#685)

providers/dns/netcup/client.go

@@ -25,27 +25,27 @@ type Request struct {
 	Param  interface{} `json:"param"`
 }
 
-// LoginMsg as specified in netcup WSDL
+// LoginRequest as specified in netcup WSDL
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php#login
-type LoginMsg struct {
+type LoginRequest struct {
 	CustomerNumber  string `json:"customernumber"`
 	APIKey          string `json:"apikey"`
 	APIPassword     string `json:"apipassword"`
 	ClientRequestID string `json:"clientrequestid,omitempty"`
 }
 
-// LogoutMsg as specified in netcup WSDL
+// LogoutRequest as specified in netcup WSDL
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php#logout
-type LogoutMsg struct {
+type LogoutRequest struct {
 	CustomerNumber  string `json:"customernumber"`
 	APIKey          string `json:"apikey"`
 	APISessionID    string `json:"apisessionid"`
 	ClientRequestID string `json:"clientrequestid,omitempty"`
 }
 
-// UpdateDNSRecordsMsg as specified in netcup WSDL
+// UpdateDNSRecordsRequest as specified in netcup WSDL
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php#updateDnsRecords
-type UpdateDNSRecordsMsg struct {
+type UpdateDNSRecordsRequest struct {
 	DomainName      string       `json:"domainname"`
 	CustomerNumber  string       `json:"customernumber"`
 	APIKey          string       `json:"apikey"`
@@ -55,15 +55,15 @@ type UpdateDNSRecordsMsg struct {
 }
 
 // DNSRecordSet as specified in netcup WSDL
-// needed in UpdateDNSRecordsMsg
+// needed in UpdateDNSRecordsRequest
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php#Dnsrecordset
 type DNSRecordSet struct {
 	DNSRecords []DNSRecord `json:"dnsrecords"`
 }
 
-// InfoDNSRecordsMsg as specified in netcup WSDL
+// InfoDNSRecordsRequest as specified in netcup WSDL
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php#infoDnsRecords
-type InfoDNSRecordsMsg struct {
+type InfoDNSRecordsRequest struct {
 	DomainName      string `json:"domainname"`
 	CustomerNumber  string `json:"customernumber"`
 	APIKey          string `json:"apikey"`
@@ -87,33 +87,30 @@ type DNSRecord struct {
 // ResponseMsg as specified in netcup WSDL
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php#Responsemessage
 type ResponseMsg struct {
-	ServerRequestID string       `json:"serverrequestid"`
+	ServerRequestID string          `json:"serverrequestid"`
-	ClientRequestID string       `json:"clientrequestid,omitempty"`
+	ClientRequestID string          `json:"clientrequestid,omitempty"`
-	Action          string       `json:"action"`
+	Action          string          `json:"action"`
-	Status          string       `json:"status"`
+	Status          string          `json:"status"`
-	StatusCode      int          `json:"statuscode"`
+	StatusCode      int             `json:"statuscode"`
-	ShortMessage    string       `json:"shortmessage"`
+	ShortMessage    string          `json:"shortmessage"`
-	LongMessage     string       `json:"longmessage"`
+	LongMessage     string          `json:"longmessage"`
-	ResponseData    ResponseData `json:"responsedata,omitempty"`
+	ResponseData    json.RawMessage `json:"responsedata,omitempty"`
 }
 
-// LogoutResponseMsg similar to ResponseMsg
+func (r *ResponseMsg) Error() string {
-// allows empty ResponseData field whilst unmarshaling
+	return fmt.Sprintf("an error occurred during the action %s: [Status=%s, StatusCode=%d, ShortMessage=%s, LongMessage=%s]",
-type LogoutResponseMsg struct {
+		r.Action, r.Status, r.StatusCode, r.ShortMessage, r.LongMessage)
-	ServerRequestID string `json:"serverrequestid"`
-	ClientRequestID string `json:"clientrequestid,omitempty"`
-	Action          string `json:"action"`
-	Status          string `json:"status"`
-	StatusCode      int    `json:"statuscode"`
-	ShortMessage    string `json:"shortmessage"`
-	LongMessage     string `json:"longmessage"`
-	ResponseData    string `json:"responsedata,omitempty"`
 }
 
-// ResponseData to enable correct unmarshaling of ResponseMsg
+// LoginResponse response to login action.
-type ResponseData struct {
+type LoginResponse struct {
+	APISessionID string `json:"apisessionid"`
+}
+
+// InfoDNSRecordsResponse response to infoDnsRecords action.
+type InfoDNSRecordsResponse struct {
 	APISessionID string      `json:"apisessionid"`
-	DNSRecords   []DNSRecord `json:"dnsrecords"`
+	DNSRecords   []DNSRecord `json:"dnsrecords,omitempty"`
 }
 
 // Client netcup DNS client
@@ -126,7 +123,11 @@ type Client struct {
 }
 
 // NewClient creates a netcup DNS client
-func NewClient(customerNumber string, apiKey string, apiPassword string) *Client {
+func NewClient(customerNumber string, apiKey string, apiPassword string) (*Client, error) {
+	if customerNumber == "" || apiKey == "" || apiPassword == "" {
+		return nil, fmt.Errorf("credentials missing")
+	}
+
 	return &Client{
 		customerNumber: customerNumber,
 		apiKey:         apiKey,
@@ -135,7 +136,7 @@ func NewClient(customerNumber string, apiKey string, apiPassword string) *Client
 		HTTPClient: &http.Client{
 			Timeout: 10 * time.Second,
 		},
-	}
+	}, nil
 }
 
 // Login performs the login as specified by the netcup WSDL
@@ -144,7 +145,7 @@ func NewClient(customerNumber string, apiKey string, apiPassword string) *Client
 func (c *Client) Login() (string, error) {
 	payload := &Request{
 		Action: "login",
-		Param: &LoginMsg{
+		Param: &LoginRequest{
 			CustomerNumber:  c.customerNumber,
 			APIKey:          c.apiKey,
 			APIPassword:     c.apiPassword,
@@ -152,21 +153,13 @@ func (c *Client) Login() (string, error) {
 		},
 	}
 
-	response, err := c.sendRequest(payload)
+	var responseData LoginResponse
+	err := c.doRequest(payload, &responseData)
 	if err != nil {
-		return "", fmt.Errorf("error sending request to DNS-API, %v", err)
+		return "", fmt.Errorf("loging error: %v", err)
 	}
 
-	var r ResponseMsg
+	return responseData.APISessionID, nil
-
-	err = json.Unmarshal(response, &r)
-	if err != nil {
-		return "", fmt.Errorf("error decoding response of DNS-API, %v", err)
-	}
-	if r.Status != success {
-		return "", fmt.Errorf("error logging into DNS-API, %v", r.LongMessage)
-	}
-	return r.ResponseData.APISessionID, nil
 }
 
 // Logout performs the logout with the supplied sessionID as specified by the netcup WSDL
@@ -174,7 +167,7 @@ func (c *Client) Login() (string, error) {
 func (c *Client) Logout(sessionID string) error {
 	payload := &Request{
 		Action: "logout",
-		Param: &LogoutMsg{
+		Param: &LogoutRequest{
 			CustomerNumber:  c.customerNumber,
 			APIKey:          c.apiKey,
 			APISessionID:    sessionID,
@@ -182,54 +175,34 @@ func (c *Client) Logout(sessionID string) error {
 		},
 	}
 
-	response, err := c.sendRequest(payload)
+	err := c.doRequest(payload, nil)
 	if err != nil {
-		return fmt.Errorf("error logging out of DNS-API: %v", err)
+		return fmt.Errorf("logout error: %v", err)
 	}
 
-	var r LogoutResponseMsg
-
-	err = json.Unmarshal(response, &r)
-	if err != nil {
-		return fmt.Errorf("error logging out of DNS-API: %v", err)
-	}
-
-	if r.Status != success {
-		return fmt.Errorf("error logging out of DNS-API: %v", r.ShortMessage)
-	}
 	return nil
 }
 
 // UpdateDNSRecord performs an update of the DNSRecords as specified by the netcup WSDL
 // https://ccp.netcup.net/run/webservice/servers/endpoint.php
-func (c *Client) UpdateDNSRecord(sessionID, domainName string, record DNSRecord) error {
+func (c *Client) UpdateDNSRecord(sessionID, domainName string, records []DNSRecord) error {
 	payload := &Request{
 		Action: "updateDnsRecords",
-		Param: UpdateDNSRecordsMsg{
+		Param: UpdateDNSRecordsRequest{
 			DomainName:      domainName,
 			CustomerNumber:  c.customerNumber,
 			APIKey:          c.apiKey,
 			APISessionID:    sessionID,
 			ClientRequestID: "",
-			DNSRecordSet:    DNSRecordSet{DNSRecords: []DNSRecord{record}},
+			DNSRecordSet:    DNSRecordSet{DNSRecords: records},
 		},
 	}
 
-	response, err := c.sendRequest(payload)
+	err := c.doRequest(payload, nil)
 	if err != nil {
-		return err
+		return fmt.Errorf("error when sending the request: %v", err)
 	}
 
-	var r ResponseMsg
-
-	err = json.Unmarshal(response, &r)
-	if err != nil {
-		return err
-	}
-
-	if r.Status != success {
-		return fmt.Errorf("%s: %+v", r.ShortMessage, r)
-	}
 	return nil
 }
 
@@ -239,7 +212,7 @@ func (c *Client) UpdateDNSRecord(sessionID, domainName string, record DNSRecord)
 func (c *Client) GetDNSRecords(hostname, apiSessionID string) ([]DNSRecord, error) {
 	payload := &Request{
 		Action: "infoDnsRecords",
-		Param: InfoDNSRecordsMsg{
+		Param: InfoDNSRecordsRequest{
 			DomainName:      hostname,
 			CustomerNumber:  c.customerNumber,
 			APIKey:          c.apiKey,
@@ -248,82 +221,98 @@ func (c *Client) GetDNSRecords(hostname, apiSessionID string) ([]DNSRecord, erro
 		},
 	}
 
-	response, err := c.sendRequest(payload)
+	var responseData InfoDNSRecordsResponse
+	err := c.doRequest(payload, &responseData)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error when sending the request: %v", err)
 	}
 
-	var r ResponseMsg
+	return responseData.DNSRecords, nil
-
-	err = json.Unmarshal(response, &r)
-	if err != nil {
-		return nil, err
-	}
-
-	if r.Status != success {
-		return nil, fmt.Errorf("%s", r.ShortMessage)
-	}
-	return r.ResponseData.DNSRecords, nil
 
 }
 
-// sendRequest marshals given body to JSON, send the request to netcup API
+// doRequest marshals given body to JSON, send the request to netcup API
 // and returns body of response
-func (c *Client) sendRequest(payload interface{}) ([]byte, error) {
+func (c *Client) doRequest(payload interface{}, responseData interface{}) error {
 	body, err := json.Marshal(payload)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	req, err := http.NewRequest(http.MethodPost, c.BaseURL, bytes.NewReader(body))
 	if err != nil {
-		return nil, err
+		return err
 	}
-	req.Close = true
 
+	req.Close = true
 	req.Header.Set("content-type", "application/json")
 	req.Header.Set("User-Agent", acme.UserAgent)
 
 	resp, err := c.HTTPClient.Do(req)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	if resp.StatusCode > 299 {
+	if err = checkResponse(resp); err != nil {
-		return nil, fmt.Errorf("API request failed with HTTP Status code %d", resp.StatusCode)
+		return err
 	}
 
-	body, err = ioutil.ReadAll(resp.Body)
+	respMsg, err := decodeResponseMsg(resp)
 	if err != nil {
-		return nil, fmt.Errorf("read of response body failed, %v", err)
+		return err
 	}
-	defer resp.Body.Close()
 
-	return body, nil
+	if respMsg.Status != success {
-}
+		return respMsg
+	}
 
-// GetDNSRecordIdx searches a given array of DNSRecords for a given DNSRecord
+	if responseData != nil {
-// equivalence is determined by Destination and RecortType attributes
+		err = json.Unmarshal(respMsg.ResponseData, responseData)
-// returns index of given DNSRecord in given array of DNSRecords
+		if err != nil {
-func GetDNSRecordIdx(records []DNSRecord, record DNSRecord) (int, error) {
+			return fmt.Errorf("%v: unmarshaling %T error: %v: %s",
-	for index, element := range records {
+				respMsg, responseData, err, string(respMsg.ResponseData))
-		if record.Destination == element.Destination && record.RecordType == element.RecordType {
-			return index, nil
 		}
 	}
-	return -1, fmt.Errorf("no DNS Record found")
+
+	return nil
 }
 
-// CreateTxtRecord uses the supplied values to return a DNSRecord of type TXT for the dns-01 challenge
+func checkResponse(resp *http.Response) error {
-func CreateTxtRecord(hostname, value string, ttl int) DNSRecord {
+	if resp.StatusCode > 299 {
-	return DNSRecord{
+		if resp.Body == nil {
-		ID:           0,
+			return fmt.Errorf("response body is nil, status code=%d", resp.StatusCode)
-		Hostname:     hostname,
+		}
-		RecordType:   "TXT",
+
-		Priority:     "",
+		defer resp.Body.Close()
-		Destination:  value,
+
-		DeleteRecord: false,
+		raw, err := ioutil.ReadAll(resp.Body)
-		State:        "",
+		if err != nil {
-		TTL:          ttl,
+			return fmt.Errorf("unable to read body: status code=%d, error=%v", resp.StatusCode, err)
+		}
+
+		return fmt.Errorf("status code=%d: %s", resp.StatusCode, string(raw))
 	}
+
+	return nil
+}
+
+func decodeResponseMsg(resp *http.Response) (*ResponseMsg, error) {
+	if resp.Body == nil {
+		return nil, fmt.Errorf("response body is nil, status code=%d", resp.StatusCode)
+	}
+
+	defer resp.Body.Close()
+
+	raw, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("unable to read body: status code=%d, error=%v", resp.StatusCode, err)
+	}
+
+	var respMsg ResponseMsg
+	err = json.Unmarshal(raw, &respMsg)
+	if err != nil {
+		return nil, fmt.Errorf("unmarshaling %T error [status code=%d]: %v: %s", respMsg, resp.StatusCode, err, string(raw))
+	}
+
+	return &respMsg, nil
 }

providers/dns/netcup/client_test.go

@@ -2,6 +2,10 @@ package netcup
 
 import (
 	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -10,6 +14,371 @@ import (
 	"github.com/xenolf/lego/acme"
 )
 
+func setupClientTest() (*Client, *http.ServeMux, func()) {
+	handler := http.NewServeMux()
+	server := httptest.NewServer(handler)
+
+	client, err := NewClient("a", "b", "c")
+	if err != nil {
+		panic(err)
+	}
+	client.BaseURL = server.URL
+
+	return client, handler, server.Close
+}
+
+func TestClient_Login(t *testing.T) {
+	client, mux, tearDown := setupClientTest()
+	defer tearDown()
+
+	mux.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) {
+		raw, err := ioutil.ReadAll(req.Body)
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+
+		if string(raw) != `{"action":"login","param":{"customernumber":"a","apikey":"b","apipassword":"c"}}` {
+			http.Error(rw, fmt.Sprintf("invalid request body: %s", string(raw)), http.StatusBadRequest)
+		}
+
+		response := `
+		{
+		    "serverrequestid": "srv-request-id",
+		    "clientrequestid": "",
+		    "action": "login",
+		    "status": "success",
+		    "statuscode": 2000,
+		    "shortmessage": "Login successful",
+		    "longmessage": "Session has been created successful.",
+		    "responsedata": {
+		        "apisessionid": "api-session-id"
+		    }
+		}
+		`
+		_, err = rw.Write([]byte(response))
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+	})
+
+	sessionID, err := client.Login()
+	require.NoError(t, err)
+
+	assert.Equal(t, "api-session-id", sessionID)
+}
+
+func TestClient_Login_errors(t *testing.T) {
+	testCases := []struct {
+		desc    string
+		handler func(rw http.ResponseWriter, req *http.Request)
+	}{
+		{
+			desc: "HTTP error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				http.Error(rw, "error message", http.StatusInternalServerError)
+			},
+		},
+		{
+			desc: "API error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				response := `
+					{
+						"serverrequestid":"YxTr4EzdbJ101T211zR4yzUEMVE",
+						"clientrequestid":"",
+						"action":"login",
+						"status":"error",
+						"statuscode":4013,
+						"shortmessage":"Validation Error.",
+						"longmessage":"Message is empty.",
+						"responsedata":""
+					}`
+				_, err := rw.Write([]byte(response))
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusInternalServerError)
+				}
+			},
+		},
+		{
+			desc: "responsedata marshaling error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				response := `
+							{
+								"serverrequestid": "srv-request-id",
+								"clientrequestid": "",
+								"action": "login",
+								"status": "success",
+								"statuscode": 2000,
+								"shortmessage": "Login successful",
+								"longmessage": "Session has been created successful.",
+								"responsedata": ""
+							}`
+				_, err := rw.Write([]byte(response))
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusInternalServerError)
+				}
+			},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			client, mux, tearDown := setupClientTest()
+			defer tearDown()
+
+			mux.HandleFunc("/", test.handler)
+
+			sessionID, err := client.Login()
+			assert.Error(t, err)
+			assert.Equal(t, "", sessionID)
+		})
+	}
+}
+
+func TestClient_Logout(t *testing.T) {
+	client, mux, tearDown := setupClientTest()
+	defer tearDown()
+
+	mux.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) {
+		raw, err := ioutil.ReadAll(req.Body)
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+
+		if string(raw) != `{"action":"logout","param":{"customernumber":"a","apikey":"b","apisessionid":"session-id"}}` {
+			http.Error(rw, fmt.Sprintf("invalid request body: %s", string(raw)), http.StatusBadRequest)
+		}
+
+		response := `
+			{
+				"serverrequestid": "request-id",
+				"clientrequestid": "",
+				"action": "logout",
+				"status": "success",
+				"statuscode": 2000,
+				"shortmessage": "Logout successful",
+				"longmessage": "Session has been terminated successful.",
+				"responsedata": ""
+			}`
+		_, err = rw.Write([]byte(response))
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+	})
+
+	err := client.Logout("session-id")
+	require.NoError(t, err)
+}
+
+func TestClient_Logout_errors(t *testing.T) {
+	testCases := []struct {
+		desc    string
+		handler func(rw http.ResponseWriter, req *http.Request)
+	}{
+		{
+			desc: "HTTP error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				http.Error(rw, "error message", http.StatusInternalServerError)
+			},
+		},
+		{
+			desc: "API error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				response := `
+					{
+						"serverrequestid":"YxTr4EzdbJ101T211zR4yzUEMVE",
+						"clientrequestid":"",
+						"action":"logout",
+						"status":"error",
+						"statuscode":4013,
+						"shortmessage":"Validation Error.",
+						"longmessage":"Message is empty.",
+						"responsedata":""
+					}`
+				_, err := rw.Write([]byte(response))
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusInternalServerError)
+				}
+			},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			client, mux, tearDown := setupClientTest()
+			defer tearDown()
+
+			mux.HandleFunc("/", test.handler)
+
+			err := client.Logout("session-id")
+			require.Error(t, err)
+		})
+	}
+}
+
+func TestClient_GetDNSRecords(t *testing.T) {
+	client, mux, tearDown := setupClientTest()
+	defer tearDown()
+
+	mux.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) {
+		raw, err := ioutil.ReadAll(req.Body)
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+
+		if string(raw) != `{"action":"infoDnsRecords","param":{"domainname":"example.com","customernumber":"a","apikey":"b","apisessionid":"api-session-id"}}` {
+			http.Error(rw, fmt.Sprintf("invalid request body: %s", string(raw)), http.StatusBadRequest)
+		}
+
+		response := `
+			{
+			  "serverrequestid":"srv-request-id",
+			  "clientrequestid":"",
+			  "action":"infoDnsRecords",
+			  "status":"success",
+			  "statuscode":2000,
+			  "shortmessage":"Login successful",
+			  "longmessage":"Session has been created successful.",
+			  "responsedata":{
+			    "apisessionid":"api-session-id",
+			    "dnsrecords":[
+			      {
+			        "id":"1",
+			        "hostname":"example.com",
+			        "type":"TXT",
+			        "priority":"1",
+			        "destination":"bGVnbzE=",
+			        "state":"yes",
+			        "ttl":300
+			      },
+			      {
+			        "id":"2",
+			        "hostname":"example2.com",
+			        "type":"TXT",
+			        "priority":"1",
+			        "destination":"bGVnbw==",
+			        "state":"yes",
+			        "ttl":300
+			      }
+			    ]
+			  }
+			}`
+		_, err = rw.Write([]byte(response))
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+	})
+
+	expected := []DNSRecord{{
+		ID:           1,
+		Hostname:     "example.com",
+		RecordType:   "TXT",
+		Priority:     "1",
+		Destination:  "bGVnbzE=",
+		DeleteRecord: false,
+		State:        "yes",
+		TTL:          300,
+	}, {
+		ID:           2,
+		Hostname:     "example2.com",
+		RecordType:   "TXT",
+		Priority:     "1",
+		Destination:  "bGVnbw==",
+		DeleteRecord: false,
+		State:        "yes",
+		TTL:          300,
+	}}
+
+	records, err := client.GetDNSRecords("example.com", "api-session-id")
+	require.NoError(t, err)
+
+	assert.Equal(t, expected, records)
+}
+
+func TestClient_GetDNSRecords_errors(t *testing.T) {
+	testCases := []struct {
+		desc    string
+		handler func(rw http.ResponseWriter, req *http.Request)
+	}{
+		{
+			desc: "HTTP error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				http.Error(rw, "error message", http.StatusInternalServerError)
+			},
+		},
+		{
+			desc: "API error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				response := `
+					{
+						"serverrequestid":"YxTr4EzdbJ101T211zR4yzUEMVE",
+						"clientrequestid":"",
+						"action":"infoDnsRecords",
+						"status":"error",
+						"statuscode":4013,
+						"shortmessage":"Validation Error.",
+						"longmessage":"Message is empty.",
+						"responsedata":""
+					}`
+				_, err := rw.Write([]byte(response))
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusInternalServerError)
+				}
+			},
+		},
+		{
+			desc: "responsedata marshaling error",
+			handler: func(rw http.ResponseWriter, req *http.Request) {
+				raw, err := ioutil.ReadAll(req.Body)
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusInternalServerError)
+				}
+
+				if string(raw) != `{"action":"infoDnsRecords","param":{"domainname":"example.com","customernumber":"a","apikey":"b","apisessionid":"api-session-id"}}` {
+					http.Error(rw, fmt.Sprintf("invalid request body: %s", string(raw)), http.StatusBadRequest)
+				}
+
+				response := `
+			{
+			  "serverrequestid":"srv-request-id",
+			  "clientrequestid":"",
+			  "action":"infoDnsRecords",
+			  "status":"success",
+			  "statuscode":2000,
+			  "shortmessage":"Login successful",
+			  "longmessage":"Session has been created successful.",
+			  "responsedata":""
+			}`
+				_, err = rw.Write([]byte(response))
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusInternalServerError)
+				}
+			},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			client, mux, tearDown := setupClientTest()
+			defer tearDown()
+
+			mux.HandleFunc("/", test.handler)
+
+			records, err := client.GetDNSRecords("example.com", "api-session-id")
+			require.Error(t, err)
+			assert.Empty(t, records)
+		})
+	}
+}
+
 func TestLiveClientAuth(t *testing.T) {
 	if !envTest.IsLiveTest() {
 		t.Skip("skipping live test")
@@ -18,14 +387,15 @@ func TestLiveClientAuth(t *testing.T) {
 	// Setup
 	envTest.RestoreEnv()
 
-	client := NewClient(
+	client, err := NewClient(
 		envTest.GetValue("NETCUP_CUSTOMER_NUMBER"),
 		envTest.GetValue("NETCUP_API_KEY"),
 		envTest.GetValue("NETCUP_API_PASSWORD"))
+	require.NoError(t, err)
 
 	for i := 1; i < 4; i++ {
 		i := i
-		t.Run("Test"+string(i), func(t *testing.T) {
+		t.Run("Test_"+strconv.Itoa(i), func(t *testing.T) {
 			t.Parallel()
 
 			sessionID, err := client.Login()
@@ -46,10 +416,11 @@ func TestLiveClientGetDnsRecords(t *testing.T) {
 	// Setup
 	envTest.RestoreEnv()
 
-	client := NewClient(
+	client, err := NewClient(
 		envTest.GetValue("NETCUP_CUSTOMER_NUMBER"),
 		envTest.GetValue("NETCUP_API_KEY"),
 		envTest.GetValue("NETCUP_API_PASSWORD"))
+	require.NoError(t, err)
 
 	sessionID, err := client.Login()
 	require.NoError(t, err)
@@ -78,10 +449,11 @@ func TestLiveClientUpdateDnsRecord(t *testing.T) {
 	// Setup
 	envTest.RestoreEnv()
 
-	client := NewClient(
+	client, err := NewClient(
 		envTest.GetValue("NETCUP_CUSTOMER_NUMBER"),
 		envTest.GetValue("NETCUP_API_KEY"),
 		envTest.GetValue("NETCUP_API_PASSWORD"))
+	require.NoError(t, err)
 
 	sessionID, err := client.Login()
 	require.NoError(t, err)
@@ -93,18 +465,18 @@ func TestLiveClientUpdateDnsRecord(t *testing.T) {
 
 	hostname := strings.Replace(fqdn, "."+zone, "", 1)
 
-	record := CreateTxtRecord(hostname, "asdf5678", 120)
+	record := createTxtRecord(hostname, "asdf5678", 120)
 
 	// test
 	zone = acme.UnFqdn(zone)
 
-	err = client.UpdateDNSRecord(sessionID, zone, record)
+	err = client.UpdateDNSRecord(sessionID, zone, []DNSRecord{record})
 	require.NoError(t, err)
 
 	records, err := client.GetDNSRecords(zone, sessionID)
 	require.NoError(t, err)
 
-	recordIdx, err := GetDNSRecordIdx(records, record)
+	recordIdx, err := getDNSRecordIdx(records, record)
 	require.NoError(t, err)
 
 	assert.Equal(t, record.Hostname, records[recordIdx].Hostname)
@@ -115,111 +487,9 @@ func TestLiveClientUpdateDnsRecord(t *testing.T) {
 	records[recordIdx].DeleteRecord = true
 
 	// Tear down
-	err = client.UpdateDNSRecord(sessionID, envTest.GetDomain(), records[recordIdx])
+	err = client.UpdateDNSRecord(sessionID, envTest.GetDomain(), []DNSRecord{records[recordIdx]})
 	require.NoError(t, err, "Did not remove record! Please do so yourself.")
 
 	err = client.Logout(sessionID)
 	require.NoError(t, err)
 }
-
-func TestClientGetDnsRecordIdx(t *testing.T) {
-	records := []DNSRecord{
-		{
-			ID:           12345,
-			Hostname:     "asdf",
-			RecordType:   "TXT",
-			Priority:     "0",
-			Destination:  "randomtext",
-			DeleteRecord: false,
-			State:        "yes",
-		},
-		{
-			ID:           23456,
-			Hostname:     "@",
-			RecordType:   "A",
-			Priority:     "0",
-			Destination:  "127.0.0.1",
-			DeleteRecord: false,
-			State:        "yes",
-		},
-		{
-			ID:           34567,
-			Hostname:     "dfgh",
-			RecordType:   "CNAME",
-			Priority:     "0",
-			Destination:  "example.com",
-			DeleteRecord: false,
-			State:        "yes",
-		},
-		{
-			ID:           45678,
-			Hostname:     "fghj",
-			RecordType:   "MX",
-			Priority:     "10",
-			Destination:  "mail.example.com",
-			DeleteRecord: false,
-			State:        "yes",
-		},
-	}
-
-	testCases := []struct {
-		desc        string
-		record      DNSRecord
-		expectError bool
-	}{
-		{
-			desc: "simple",
-			record: DNSRecord{
-				ID:           12345,
-				Hostname:     "asdf",
-				RecordType:   "TXT",
-				Priority:     "0",
-				Destination:  "randomtext",
-				DeleteRecord: false,
-				State:        "yes",
-			},
-		},
-		{
-			desc: "wrong Destination",
-			record: DNSRecord{
-				ID:           12345,
-				Hostname:     "asdf",
-				RecordType:   "TXT",
-				Priority:     "0",
-				Destination:  "wrong",
-				DeleteRecord: false,
-				State:        "yes",
-			},
-			expectError: true,
-		},
-		{
-			desc: "record type CNAME",
-			record: DNSRecord{
-				ID:           12345,
-				Hostname:     "asdf",
-				RecordType:   "CNAME",
-				Priority:     "0",
-				Destination:  "randomtext",
-				DeleteRecord: false,
-				State:        "yes",
-			},
-			expectError: true,
-		},
-	}
-
-	for _, test := range testCases {
-		test := test
-		t.Run(test.desc, func(t *testing.T) {
-			t.Parallel()
-
-			idx, err := GetDNSRecordIdx(records, test.record)
-			if test.expectError {
-				assert.Error(t, err)
-				assert.Equal(t, -1, idx)
-			} else {
-				assert.NoError(t, err)
-				assert.Equal(t, records[idx], test.record)
-			}
-		})
-	}
-}

providers/dns/netcup/netcup.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/xenolf/lego/acme"
+	"github.com/xenolf/lego/log"
 	"github.com/xenolf/lego/platform/config/env"
 )
 
@@ -27,8 +28,8 @@ type Config struct {
 func NewDefaultConfig() *Config {
 	return &Config{
 		TTL:                env.GetOrDefaultInt("NETCUP_TTL", 120),
-		PropagationTimeout: env.GetOrDefaultSecond("NETCUP_PROPAGATION_TIMEOUT", acme.DefaultPropagationTimeout),
+		PropagationTimeout: env.GetOrDefaultSecond("NETCUP_PROPAGATION_TIMEOUT", 120*time.Second),
-		PollingInterval:    env.GetOrDefaultSecond("NETCUP_POLLING_INTERVAL", acme.DefaultPollingInterval),
+		PollingInterval:    env.GetOrDefaultSecond("NETCUP_POLLING_INTERVAL", 5*time.Second),
 		HTTPClient: &http.Client{
 			Timeout: env.GetOrDefaultSecond("NETCUP_HTTP_TIMEOUT", 10*time.Second),
 		},
@@ -76,11 +77,11 @@ func NewDNSProviderConfig(config *Config) (*DNSProvider, error) {
 		return nil, errors.New("netcup: the configuration of the DNS provider is nil")
 	}
 
-	if config.Customer == "" || config.Key == "" || config.Password == "" {
+	client, err := NewClient(config.Customer, config.Key, config.Password)
-		return nil, fmt.Errorf("netcup: netcup credentials missing")
+	if err != nil {
+		return nil, fmt.Errorf("netcup: %v", err)
 	}
 
-	client := NewClient(config.Customer, config.Key, config.Password)
 	client.HTTPClient = config.HTTPClient
 
 	return &DNSProvider{client: client, config: config}, nil
@@ -100,27 +101,37 @@ func (d *DNSProvider) Present(domainName, token, keyAuth string) error {
 		return fmt.Errorf("netcup: %v", err)
 	}
 
-	hostname := strings.Replace(fqdn, "."+zone, "", 1)
+	defer func() {
-	record := CreateTxtRecord(hostname, value, d.config.TTL)
+		err = d.client.Logout(sessionID)
-
+		if err != nil {
-	err = d.client.UpdateDNSRecord(sessionID, acme.UnFqdn(zone), record)
+			log.Print("netcup: %v", err)
-	if err != nil {
-		if errLogout := d.client.Logout(sessionID); errLogout != nil {
-			return fmt.Errorf("netcup: failed to add TXT-Record: %v; %v", err, errLogout)
 		}
+	}()
+
+	hostname := strings.Replace(fqdn, "."+zone, "", 1)
+	record := createTxtRecord(hostname, value, d.config.TTL)
+
+	zone = acme.UnFqdn(zone)
+
+	records, err := d.client.GetDNSRecords(zone, sessionID)
+	if err != nil {
+		// skip no existing records
+		log.Infof("no existing records, error ignored: %v", err)
+	}
+
+	records = append(records, record)
+
+	err = d.client.UpdateDNSRecord(sessionID, zone, records)
+	if err != nil {
 		return fmt.Errorf("netcup: failed to add TXT-Record: %v", err)
 	}
 
-	err = d.client.Logout(sessionID)
-	if err != nil {
-		return fmt.Errorf("netcup: %v", err)
-	}
 	return nil
 }
 
 // CleanUp removes the TXT record matching the specified parameters
-func (d *DNSProvider) CleanUp(domainname, token, keyAuth string) error {
+func (d *DNSProvider) CleanUp(domainName, token, keyAuth string) error {
-	fqdn, value, _ := acme.DNS01Record(domainname, keyAuth)
+	fqdn, value, _ := acme.DNS01Record(domainName, keyAuth)
 
 	zone, err := acme.FindZoneByFqdn(fqdn, acme.RecursiveNameservers)
 	if err != nil {
@@ -132,6 +143,13 @@ func (d *DNSProvider) CleanUp(domainname, token, keyAuth string) error {
 		return fmt.Errorf("netcup: %v", err)
 	}
 
+	defer func() {
+		err = d.client.Logout(sessionID)
+		if err != nil {
+			log.Print("netcup: %v", err)
+		}
+	}()
+
 	hostname := strings.Replace(fqdn, "."+zone, "", 1)
 
 	zone = acme.UnFqdn(zone)
@@ -141,27 +159,20 @@ func (d *DNSProvider) CleanUp(domainname, token, keyAuth string) error {
 		return fmt.Errorf("netcup: %v", err)
 	}
 
-	record := CreateTxtRecord(hostname, value, 0)
+	record := createTxtRecord(hostname, value, 0)
 
-	idx, err := GetDNSRecordIdx(records, record)
+	idx, err := getDNSRecordIdx(records, record)
 	if err != nil {
 		return fmt.Errorf("netcup: %v", err)
 	}
 
 	records[idx].DeleteRecord = true
 
-	err = d.client.UpdateDNSRecord(sessionID, zone, records[idx])
+	err = d.client.UpdateDNSRecord(sessionID, zone, []DNSRecord{records[idx]})
 	if err != nil {
-		if errLogout := d.client.Logout(sessionID); errLogout != nil {
-			return fmt.Errorf("netcup: %v; %v", err, errLogout)
-		}
 		return fmt.Errorf("netcup: %v", err)
 	}
 
-	err = d.client.Logout(sessionID)
-	if err != nil {
-		return fmt.Errorf("netcup: %v", err)
-	}
 	return nil
 }
 
@@ -170,3 +181,29 @@ func (d *DNSProvider) CleanUp(domainname, token, keyAuth string) error {
 func (d *DNSProvider) Timeout() (timeout, interval time.Duration) {
 	return d.config.PropagationTimeout, d.config.PollingInterval
 }
+
+// getDNSRecordIdx searches a given array of DNSRecords for a given DNSRecord
+// equivalence is determined by Destination and RecortType attributes
+// returns index of given DNSRecord in given array of DNSRecords
+func getDNSRecordIdx(records []DNSRecord, record DNSRecord) (int, error) {
+	for index, element := range records {
+		if record.Destination == element.Destination && record.RecordType == element.RecordType {
+			return index, nil
+		}
+	}
+	return -1, fmt.Errorf("no DNS Record found")
+}
+
+// createTxtRecord uses the supplied values to return a DNSRecord of type TXT for the dns-01 challenge
+func createTxtRecord(hostname, value string, ttl int) DNSRecord {
+	return DNSRecord{
+		ID:           0,
+		Hostname:     hostname,
+		RecordType:   "TXT",
+		Priority:     "",
+		Destination:  value,
+		DeleteRecord: false,
+		State:        "",
+		TTL:          ttl,
+	}
+}

providers/dns/netcup/netcup_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/xenolf/lego/acme"
 	"github.com/xenolf/lego/platform/tester"
@@ -104,28 +105,28 @@ func TestNewDNSProviderConfig(t *testing.T) {
 		},
 		{
 			desc:     "missing credentials",
-			expected: "netcup: netcup credentials missing",
+			expected: "netcup: credentials missing",
 		},
 		{
 			desc:     "missing customer",
 			customer: "",
 			key:      "B",
 			password: "C",
-			expected: "netcup: netcup credentials missing",
+			expected: "netcup: credentials missing",
 		},
 		{
 			desc:     "missing key",
 			customer: "A",
 			key:      "",
 			password: "C",
-			expected: "netcup: netcup credentials missing",
+			expected: "netcup: credentials missing",
 		},
 		{
 			desc:     "missing password",
 			customer: "A",
 			key:      "B",
 			password: "",
-			expected: "netcup: netcup credentials missing",
+			expected: "netcup: credentials missing",
 		},
 	}
 
@@ -150,6 +151,108 @@ func TestNewDNSProviderConfig(t *testing.T) {
 	}
 }
 
+func TestGetDNSRecordIdx(t *testing.T) {
+	records := []DNSRecord{
+		{
+			ID:           12345,
+			Hostname:     "asdf",
+			RecordType:   "TXT",
+			Priority:     "0",
+			Destination:  "randomtext",
+			DeleteRecord: false,
+			State:        "yes",
+		},
+		{
+			ID:           23456,
+			Hostname:     "@",
+			RecordType:   "A",
+			Priority:     "0",
+			Destination:  "127.0.0.1",
+			DeleteRecord: false,
+			State:        "yes",
+		},
+		{
+			ID:           34567,
+			Hostname:     "dfgh",
+			RecordType:   "CNAME",
+			Priority:     "0",
+			Destination:  "example.com",
+			DeleteRecord: false,
+			State:        "yes",
+		},
+		{
+			ID:           45678,
+			Hostname:     "fghj",
+			RecordType:   "MX",
+			Priority:     "10",
+			Destination:  "mail.example.com",
+			DeleteRecord: false,
+			State:        "yes",
+		},
+	}
+
+	testCases := []struct {
+		desc        string
+		record      DNSRecord
+		expectError bool
+	}{
+		{
+			desc: "simple",
+			record: DNSRecord{
+				ID:           12345,
+				Hostname:     "asdf",
+				RecordType:   "TXT",
+				Priority:     "0",
+				Destination:  "randomtext",
+				DeleteRecord: false,
+				State:        "yes",
+			},
+		},
+		{
+			desc: "wrong Destination",
+			record: DNSRecord{
+				ID:           12345,
+				Hostname:     "asdf",
+				RecordType:   "TXT",
+				Priority:     "0",
+				Destination:  "wrong",
+				DeleteRecord: false,
+				State:        "yes",
+			},
+			expectError: true,
+		},
+		{
+			desc: "record type CNAME",
+			record: DNSRecord{
+				ID:           12345,
+				Hostname:     "asdf",
+				RecordType:   "CNAME",
+				Priority:     "0",
+				Destination:  "randomtext",
+				DeleteRecord: false,
+				State:        "yes",
+			},
+			expectError: true,
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			idx, err := getDNSRecordIdx(records, test.record)
+			if test.expectError {
+				assert.Error(t, err)
+				assert.Equal(t, -1, idx)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, records[idx], test.record)
+			}
+		})
+	}
+}
+
 func TestLivePresentAndCleanup(t *testing.T) {
 	if !envTest.IsLiveTest() {
 		t.Skip("skipping live test")