From 9b2a3a1873fa081761c0959d7bf15893273a670a Mon Sep 17 00:00:00 2001 From: Konni Hartmann Date: Tue, 23 Oct 2018 02:01:13 +0200 Subject: [PATCH] netcup: make unmarshalling of api-responses more leniant. (#685) --- providers/dns/netcup/client.go | 229 +++++++------ providers/dns/netcup/client_test.go | 490 +++++++++++++++++++++------- providers/dns/netcup/netcup.go | 93 ++++-- providers/dns/netcup/netcup_test.go | 111 ++++++- 4 files changed, 661 insertions(+), 262 deletions(-) diff --git a/providers/dns/netcup/client.go b/providers/dns/netcup/client.go index f30bd7f1..17a2ae96 100644 --- a/providers/dns/netcup/client.go +++ b/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"` - 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 ResponseData `json:"responsedata,omitempty"` + 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 json.RawMessage `json:"responsedata,omitempty"` } -// LogoutResponseMsg similar to ResponseMsg -// allows empty ResponseData field whilst unmarshaling -type LogoutResponseMsg struct { - 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"` +func (r *ResponseMsg) Error() string { + return fmt.Sprintf("an error occurred during the action %s: [Status=%s, StatusCode=%d, ShortMessage=%s, LongMessage=%s]", + r.Action, r.Status, r.StatusCode, r.ShortMessage, r.LongMessage) } -// ResponseData to enable correct unmarshaling of ResponseMsg -type ResponseData struct { +// LoginResponse response to login action. +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 - - 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 + return 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 - - 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 + return 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 { - return nil, fmt.Errorf("API request failed with HTTP Status code %d", resp.StatusCode) + if err = checkResponse(resp); err != nil { + 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 -// 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 + if responseData != nil { + err = json.Unmarshal(respMsg.ResponseData, responseData) + if err != nil { + return fmt.Errorf("%v: unmarshaling %T error: %v: %s", + respMsg, responseData, err, string(respMsg.ResponseData)) } } - 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 CreateTxtRecord(hostname, value string, ttl int) DNSRecord { - return DNSRecord{ - ID: 0, - Hostname: hostname, - RecordType: "TXT", - Priority: "", - Destination: value, - DeleteRecord: false, - State: "", - TTL: ttl, +func checkResponse(resp *http.Response) error { + if resp.StatusCode > 299 { + if resp.Body == nil { + return 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 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 } diff --git a/providers/dns/netcup/client_test.go b/providers/dns/netcup/client_test.go index 6fcf78cc..dd40cccd 100644 --- a/providers/dns/netcup/client_test.go +++ b/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) - } - }) - } -} diff --git a/providers/dns/netcup/netcup.go b/providers/dns/netcup/netcup.go index 983b71e5..0dc59f96 100644 --- a/providers/dns/netcup/netcup.go +++ b/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), - PollingInterval: env.GetOrDefaultSecond("NETCUP_POLLING_INTERVAL", acme.DefaultPollingInterval), + PropagationTimeout: env.GetOrDefaultSecond("NETCUP_PROPAGATION_TIMEOUT", 120*time.Second), + 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 == "" { - return nil, fmt.Errorf("netcup: netcup credentials missing") + client, err := NewClient(config.Customer, config.Key, config.Password) + 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) - record := CreateTxtRecord(hostname, value, d.config.TTL) - - err = d.client.UpdateDNSRecord(sessionID, acme.UnFqdn(zone), record) - if err != nil { - if errLogout := d.client.Logout(sessionID); errLogout != nil { - return fmt.Errorf("netcup: failed to add TXT-Record: %v; %v", err, errLogout) + defer func() { + err = d.client.Logout(sessionID) + if err != nil { + log.Print("netcup: %v", err) } + }() + + 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 { - fqdn, value, _ := acme.DNS01Record(domainname, keyAuth) +func (d *DNSProvider) CleanUp(domainName, token, keyAuth string) error { + 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, + } +} diff --git a/providers/dns/netcup/netcup_test.go b/providers/dns/netcup/netcup_test.go index d7c06294..bb62bfa6 100644 --- a/providers/dns/netcup/netcup_test.go +++ b/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")