dnsimple: fix challenge. (#666)

providers/dns/dnsimple/dnsimple.go

@@ -78,7 +78,7 @@ func NewDNSProviderConfig(config *Config) (*DNSProvider, error) {
 		client.BaseURL = config.BaseURL
 	}
 
-	return &DNSProvider{client: client}, nil
+	return &DNSProvider{client: client, config: config}, nil
 }
 
 // Present creates a TXT record to fulfill the dns-01 challenge.
@@ -87,18 +87,18 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 
 	zoneName, err := d.getHostedZone(domain)
 	if err != nil {
-		return err
+		return fmt.Errorf("dnsimple: %v", err)
 	}
 
 	accountID, err := d.getAccountID()
 	if err != nil {
-		return err
+		return fmt.Errorf("dnsimple: %v", err)
 	}
 
-	recordAttributes := d.newTxtRecord(zoneName, fqdn, value, d.config.TTL)
+	recordAttributes := newTxtRecord(zoneName, fqdn, value, d.config.TTL)
 	_, err = d.client.Zones.CreateRecord(accountID, zoneName, recordAttributes)
 	if err != nil {
-		return fmt.Errorf("API call failed: %v", err)
+		return fmt.Errorf("dnsimple: API call failed: %v", err)
 	}
 
 	return nil
@@ -110,22 +110,23 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error {
 
 	records, err := d.findTxtRecords(domain, fqdn)
 	if err != nil {
-		return err
+		return fmt.Errorf("dnsimple: %v", err)
 	}
 
 	accountID, err := d.getAccountID()
 	if err != nil {
-		return err
+		return fmt.Errorf("dnsimple: %v", err)
 	}
 
+	var lastErr error
 	for _, rec := range records {
 		_, err := d.client.Zones.DeleteRecord(accountID, rec.ZoneID, rec.ID)
 		if err != nil {
-			return err
+			lastErr = fmt.Errorf("dnsimple: %v", err)
 		}
 	}
 
-	return nil
+	return lastErr
 }
 
 // Timeout returns the timeout and interval to use when checking for DNS propagation.
@@ -177,7 +178,7 @@ func (d *DNSProvider) findTxtRecords(domain, fqdn string) ([]dnsimple.ZoneRecord
 		return nil, err
 	}
 
-	recordName := d.extractRecordName(fqdn, zoneName)
+	recordName := extractRecordName(fqdn, zoneName)
 
 	result, err := d.client.Zones.ListRecords(accountID, zoneName, &dnsimple.ZoneRecordListOptions{Name: recordName, Type: "TXT", ListOptions: dnsimple.ListOptions{}})
 	if err != nil {
@@ -187,8 +188,8 @@ func (d *DNSProvider) findTxtRecords(domain, fqdn string) ([]dnsimple.ZoneRecord
 	return result.Data, nil
 }
 
-func (d *DNSProvider) newTxtRecord(zoneName, fqdn, value string, ttl int) dnsimple.ZoneRecord {
+func newTxtRecord(zoneName, fqdn, value string, ttl int) dnsimple.ZoneRecord {
-	name := d.extractRecordName(fqdn, zoneName)
+	name := extractRecordName(fqdn, zoneName)
 
 	return dnsimple.ZoneRecord{
 		Type:    "TXT",
@@ -198,7 +199,7 @@ func (d *DNSProvider) newTxtRecord(zoneName, fqdn, value string, ttl int) dnsimp
 	}
 }
 
-func (d *DNSProvider) extractRecordName(fqdn, domain string) string {
+func extractRecordName(fqdn, domain string) string {
 	name := acme.UnFqdn(fqdn)
 	if idx := strings.Index(name, "."+domain); idx != -1 {
 		return name[:idx]

providers/dns/dnsimple/dnsimple_test.go

@@ -38,84 +38,125 @@ func restoreEnv() {
 	os.Setenv("DNSIMPLE_BASE_URL", dnsimpleBaseURL)
 }
 
-//
+func TestNewDNSProvider(t *testing.T) {
-// NewDNSProvider
+	testCases := []struct {
-//
+		desc      string
-
+		userAgent string
-func TestNewDNSProviderValid(t *testing.T) {
+		envVars   map[string]string
-	defer restoreEnv()
+		expected  string
-	os.Setenv("DNSIMPLE_OAUTH_TOKEN", "123")
+	}{
-
+		{
-	acme.UserAgent = "lego"
+			desc:      "success",
-
+			userAgent: "lego",
-	provider, err := NewDNSProvider()
+			envVars: map[string]string{
-
+				"DNSIMPLE_OAUTH_TOKEN": "my_token",
-	assert.NotNil(t, provider)
+			},
-	assert.Equal(t, "lego", provider.client.UserAgent)
+		},
-	assert.NoError(t, err)
+		{
+			desc: "success: base url",
+			envVars: map[string]string{
+				"DNSIMPLE_OAUTH_TOKEN": "my_token",
+				"DNSIMPLE_BASE_URL":    "https://api.dnsimple.test",
+			},
+		},
+		{
+			desc: "missing oauth token",
+			envVars: map[string]string{
+				"DNSIMPLE_OAUTH_TOKEN": "",
+			},
+			expected: "dnsimple: OAuth token is missing",
+		},
 	}
 
-func TestNewDNSProviderValidWithBaseUrl(t *testing.T) {
+	for _, test := range testCases {
+		t.Run(test.desc, func(t *testing.T) {
 			defer restoreEnv()
-	os.Setenv("DNSIMPLE_OAUTH_TOKEN", "123")
+			for key, value := range test.envVars {
-	os.Setenv("DNSIMPLE_BASE_URL", "https://api.dnsimple.test")
+				if len(value) == 0 {
-
+					os.Unsetenv(key)
-	provider, err := NewDNSProvider()
+				} else {
-
+					os.Setenv(key, value)
-	assert.NotNil(t, provider)
+				}
-	assert.NoError(t, err)
-
-	assert.Equal(t, provider.client.BaseURL, "https://api.dnsimple.test")
 			}
 
-func TestNewDNSProviderInvalidWithMissingOauthToken(t *testing.T) {
+			if test.userAgent != "" {
-	defer restoreEnv()
+				acme.UserAgent = test.userAgent
-	os.Setenv("DNSIMPLE_OAUTH_TOKEN", "")
-
-	provider, err := NewDNSProvider()
-
-	assert.Nil(t, provider)
-	assert.EqualError(t, err, "dnsimple: OAuth token is missing")
 			}
 
-//
+			p, err := NewDNSProvider()
-// NewDNSProviderCredentials
-//
 
-func TestNewDNSProviderCredentialsValid(t *testing.T) {
+			if len(test.expected) == 0 {
-	config := NewDefaultConfig()
-	config.AccessToken = "123"
-	config.BaseURL = ""
-
-	provider, err := NewDNSProviderConfig(config)
 				require.NoError(t, err)
-	require.NotNil(t, provider)
+				require.NotNil(t, p)
+				require.NotNil(t, p.config)
+				require.NotNil(t, p.client)
 
-	assert.Equal(t, "lego", provider.client.UserAgent)
+				baseURL := os.Getenv("DNSIMPLE_BASE_URL")
-	assert.NoError(t, err)
+				if baseURL != "" {
+					assert.Equal(t, baseURL, p.client.BaseURL)
 				}
 
-func TestNewDNSProviderCredentialsValidWithBaseUrl(t *testing.T) {
+				if test.userAgent != "" {
-	config := NewDefaultConfig()
+					assert.Equal(t, "lego", p.client.UserAgent)
-	config.AccessToken = "123"
+				}
-	config.BaseURL = "https://api.dnsimple.test"
 
-	provider, err := NewDNSProviderConfig(config)
+			} else {
+				require.EqualError(t, err, test.expected)
+			}
+		})
+	}
+}
+
+func TestNewDNSProviderConfig(t *testing.T) {
+	testCases := []struct {
+		desc        string
+		accessToken string
+		baseURL     string
+		expected    string
+	}{
+		{
+			desc:        "success",
+			accessToken: "my_token",
+			baseURL:     "",
+		},
+		{
+			desc:        "success: base url",
+			accessToken: "my_token",
+			baseURL:     "https://api.dnsimple.test",
+		},
+		{
+			desc:     "missing oauth token",
+			expected: "dnsimple: OAuth token is missing",
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.desc, func(t *testing.T) {
+			defer restoreEnv()
+			os.Unsetenv("DNSIMPLE_OAUTH_TOKEN")
+			os.Unsetenv("DNSIMPLE_BASE_URL")
+
+			config := NewDefaultConfig()
+			config.AccessToken = test.accessToken
+			config.BaseURL = test.baseURL
+
+			p, err := NewDNSProviderConfig(config)
+
+			if len(test.expected) == 0 {
 				require.NoError(t, err)
-	require.NotNil(t, provider)
+				require.NotNil(t, p)
+				require.NotNil(t, p.config)
+				require.NotNil(t, p.client)
 
-	assert.Equal(t, provider.client.BaseURL, "https://api.dnsimple.test")
+				if test.baseURL != "" {
+					assert.Equal(t, test.baseURL, p.client.BaseURL)
 				}
 
-func TestNewDNSProviderCredentialsInvalidWithMissingOauthToken(t *testing.T) {
+			} else {
-	config := NewDefaultConfig()
+				require.EqualError(t, err, test.expected)
-	config.AccessToken = ""
+			}
-	config.BaseURL = ""
+		})
-
+	}
-	provider, err := NewDNSProviderConfig(config)
-
-	assert.Nil(t, provider)
-	assert.EqualError(t, err, "dnsimple: OAuth token is missing")
 }
 
 //