Review DNS providers. (#565)

* refactor: review DNS providers.

platform/config/env/env.go

@@ -0,0 +1,27 @@
+package env
+
+import (
+	"fmt"
+	"os"
+	"strings"
+)
+
+// Get environment variables
+func Get(names ...string) (map[string]string, error) {
+	values := map[string]string{}
+
+	var missingEnvVars []string
+	for _, envVar := range names {
+		value := os.Getenv(envVar)
+		if value == "" {
+			missingEnvVars = append(missingEnvVars, envVar)
+		}
+		values[envVar] = value
+	}
+
+	if len(missingEnvVars) > 0 {
+		return nil, fmt.Errorf("some credentials information are missing: %s", strings.Join(missingEnvVars, ","))
+	}
+
+	return values, nil
+}

providers/dns/auroradns/auroradns.go

@@ -9,6 +9,7 @@ import (
 	"github.com/edeckers/auroradnsclient/records"
 	"github.com/edeckers/auroradnsclient/zones"
 	"github.com/xenolf/lego/acme"
+	"github.com/xenolf/lego/platform/config/env"
 )
 
 // DNSProvider describes a provider for AuroraDNS
@@ -22,20 +23,23 @@ type DNSProvider struct {
 // Credentials must be passed in the environment variables: AURORA_USER_ID
 // and AURORA_KEY.
 func NewDNSProvider() (*DNSProvider, error) {
-	userID := os.Getenv("AURORA_USER_ID")
-	key := os.Getenv("AURORA_KEY")
+	values, err := env.Get("AURORA_USER_ID", "AURORA_KEY")
+	if err != nil {
+		return nil, fmt.Errorf("AuroraDNS: %v", err)
+	}
 
 	endpoint := os.Getenv("AURORA_ENDPOINT")
-	if endpoint == "" {
-		endpoint = "https://api.auroradns.eu"
-	}
 
-	return NewDNSProviderCredentials(endpoint, userID, key)
+	return NewDNSProviderCredentials(endpoint, values["AURORA_USER_ID"], values["AURORA_KEY"])
 }
 
 // NewDNSProviderCredentials uses the supplied credentials to return a
 // DNSProvider instance configured for AuroraDNS.
 func NewDNSProviderCredentials(baseURL string, userID string, key string) (*DNSProvider, error) {
+	if baseURL == "" {
+		baseURL = "https://api.auroradns.eu"
+	}
+
 	client, err := auroradnsclient.NewAuroraDNSClient(baseURL, userID, key)
 	if err != nil {
 		return nil, err
@@ -69,7 +73,7 @@ func (provider *DNSProvider) Present(domain, token, keyAuth string) error {
 
 	authZone, err := acme.FindZoneByFqdn(acme.ToFqdn(domain), acme.RecursiveNameservers)
 	if err != nil {
-		return fmt.Errorf("Could not determine zone for domain: '%s'. %s", domain, err)
+		return fmt.Errorf("could not determine zone for domain: '%s'. %s", domain, err)
 	}
 
 	// 1. Aurora will happily create the TXT record when it is provided a fqdn,

providers/dns/auroradns/auroradns_test.go

@@ -6,6 +6,9 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var fakeAuroraDNSUserID = "asdf1234"
@@ -26,28 +29,13 @@ func TestAuroraDNSPresent(t *testing.T) {
 
 		requestReceived = true
 
-		if got, want := r.Method, "POST"; got != want {
-			t.Errorf("Expected method to be '%s' but got '%s'", want, got)
-		}
-
-		if got, want := r.URL.Path, "/zones/c56a4180-65aa-42ec-a945-5fd21dec0538/records"; got != want {
-			t.Errorf("Expected path to be '%s' but got '%s'", want, got)
-		}
-
-		if got, want := r.Header.Get("Content-Type"), "application/json"; got != want {
-			t.Errorf("Expected Content-Type to be '%s' but got '%s'", want, got)
-		}
+		assert.Equal(t, http.MethodPost, r.Method, "method")
+		assert.Equal(t, "/zones/c56a4180-65aa-42ec-a945-5fd21dec0538/records", r.URL.Path, "Path")
+		assert.Equal(t, "application/json", r.Header.Get("Content-Type"), "Content-Type")
 
 		reqBody, err := ioutil.ReadAll(r.Body)
-		if err != nil {
-			t.Fatalf("Error reading request body: %v", err)
-		}
-
-		if got, want := string(reqBody),
-			`{"type":"TXT","name":"_acme-challenge","content":"w6uP8Tcg6K2QR905Rms8iXTlksL6OD1KOWBxTK7wxPI","ttl":300}`; got != want {
-
-			t.Errorf("Expected body data to be: `%s` but got `%s`", want, got)
-		}
+		require.NoError(t, err, "reading request body")
+		assert.Equal(t, `{"type":"TXT","name":"_acme-challenge","content":"w6uP8Tcg6K2QR905Rms8iXTlksL6OD1KOWBxTK7wxPI","ttl":300}`, string(reqBody))
 
 		w.WriteHeader(http.StatusCreated)
 		fmt.Fprintf(w, `{
@@ -61,22 +49,13 @@ func TestAuroraDNSPresent(t *testing.T) {
 	defer mock.Close()
 
 	auroraProvider, err := NewDNSProviderCredentials(mock.URL, fakeAuroraDNSUserID, fakeAuroraDNSKey)
-	if auroraProvider == nil {
-		t.Fatal("Expected non-nil AuroraDNS provider, but was nil")
-	}
-
-	if err != nil {
-		t.Fatalf("Expected no error creating provider, but got: %v", err)
-	}
+	require.NoError(t, err)
+	require.NotNil(t, auroraProvider)
 
 	err = auroraProvider.Present("example.com", "", "foobar")
-	if err != nil {
-		t.Fatalf("Expected no error creating TXT record, but got: %v", err)
-	}
+	require.NoError(t, err, "fail to create TXT record")
 
-	if !requestReceived {
-		t.Error("Expected request to be received by mock backend, but it wasn't")
-	}
+	assert.True(t, requestReceived, "Expected request to be received by mock backend, but it wasn't")
 }
 
 func TestAuroraDNSCleanUp(t *testing.T) {
@@ -105,44 +84,24 @@ func TestAuroraDNSCleanUp(t *testing.T) {
 
 		requestReceived = true
 
-		if got, want := r.Method, "DELETE"; got != want {
-			t.Errorf("Expected method to be '%s' but got '%s'", want, got)
-		}
-
-		if got, want := r.URL.Path,
-			"/zones/c56a4180-65aa-42ec-a945-5fd21dec0538/records/ec56a4180-65aa-42ec-a945-5fd21dec0538"; got != want {
-			t.Errorf("Expected path to be '%s' but got '%s'", want, got)
-		}
-
-		if got, want := r.Header.Get("Content-Type"), "application/json"; got != want {
-			t.Errorf("Expected Content-Type to be '%s' but got '%s'", want, got)
-		}
+		assert.Equal(t, http.MethodDelete, r.Method, "method")
+		assert.Equal(t, "/zones/c56a4180-65aa-42ec-a945-5fd21dec0538/records/ec56a4180-65aa-42ec-a945-5fd21dec0538", r.URL.Path, "Path")
+		assert.Equal(t, "application/json", r.Header.Get("Content-Type"), "Content-Type")
 
 		w.WriteHeader(http.StatusCreated)
 		fmt.Fprintf(w, `{}`)
 	}))
 	defer mock.Close()
 
 	auroraProvider, err := NewDNSProviderCredentials(mock.URL, fakeAuroraDNSUserID, fakeAuroraDNSKey)
-	if auroraProvider == nil {
-		t.Fatal("Expected non-nil AuroraDNS provider, but was nil")
-	}
-
-	if err != nil {
-		t.Fatalf("Expected no error creating provider, but got: %v", err)
-	}
+	require.NoError(t, err)
+	require.NotNil(t, auroraProvider)
 
 	err = auroraProvider.Present("example.com", "", "foobar")
-	if err != nil {
-		t.Fatalf("Expected no error creating TXT record, but got: %v", err)
-	}
+	require.NoError(t, err, "fail to create TXT record")
 
 	err = auroraProvider.CleanUp("example.com", "", "foobar")
-	if err != nil {
-		t.Fatalf("Expected no error removing TXT record, but got: %v", err)
-	}
+	require.NoError(t, err, "fail to remove TXT record")
 
-	if !requestReceived {
-		t.Error("Expected request to be received by mock backend, but it wasn't")
-	}
+	assert.True(t, requestReceived, "Expected request to be received by mock backend, but it wasn't")
 }

providers/dns/azure/azure.go

@@ -5,8 +5,8 @@ package azure
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"os"
 	"strings"
 	"time"
 
@@ -16,6 +16,7 @@ import (
 	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/xenolf/lego/acme"
+	"github.com/xenolf/lego/platform/config/env"
 )
 
 // DNSProvider is an implementation of the acme.ChallengeProvider interface
@@ -32,25 +33,25 @@ type DNSProvider struct {
 // Credentials must be passed in the environment variables: AZURE_CLIENT_ID,
 // AZURE_CLIENT_SECRET, AZURE_SUBSCRIPTION_ID, AZURE_TENANT_ID, AZURE_RESOURCE_GROUP
 func NewDNSProvider() (*DNSProvider, error) {
-	clientID := os.Getenv("AZURE_CLIENT_ID")
-	clientSecret := os.Getenv("AZURE_CLIENT_SECRET")
-	subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID")
-	tenantID := os.Getenv("AZURE_TENANT_ID")
-	resourceGroup := os.Getenv("AZURE_RESOURCE_GROUP")
-	return NewDNSProviderCredentials(clientID, clientSecret, subscriptionID, tenantID, resourceGroup)
+	values, err := env.Get("AZURE_CLIENT_ID", "AZURE_CLIENT_SECRET", "AZURE_SUBSCRIPTION_ID", "AZURE_TENANT_ID", "AZURE_RESOURCE_GROUP")
+	if err != nil {
+		return nil, fmt.Errorf("Azure: %v", err)
+	}
+
+	return NewDNSProviderCredentials(
+		values["AZURE_CLIENT_ID"],
+		values["AZURE_CLIENT_SECRET"],
+		values["AZURE_SUBSCRIPTION_ID"],
+		values["AZURE_TENANT_ID"],
+		values["AZURE_RESOURCE_GROUP"],
+	)
 }
 
 // NewDNSProviderCredentials uses the supplied credentials to return a
 // DNSProvider instance configured for azure.
 func NewDNSProviderCredentials(clientID, clientSecret, subscriptionID, tenantID, resourceGroup string) (*DNSProvider, error) {
 	if clientID == "" || clientSecret == "" || subscriptionID == "" || tenantID == "" || resourceGroup == "" {
-		var missingEnvVars []string
-		for _, envVar := range []string{"AZURE_CLIENT_ID", "AZURE_CLIENT_SECRET", "AZURE_SUBSCRIPTION_ID", "AZURE_TENANT_ID", "AZURE_RESOURCE_GROUP"} {
-			if os.Getenv(envVar) == "" {
-				missingEnvVars = append(missingEnvVars, envVar)
-			}
-		}
-		return nil, fmt.Errorf("Azure configuration missing: %s", strings.Join(missingEnvVars, ","))
+		return nil, errors.New("Azure: some credentials information are missing")
 	}
 
 	return &DNSProvider{

providers/dns/azure/azure_test.go

@@ -30,7 +30,7 @@ func init() {
 	}
 }
 
-func restoreAzureEnv() {
+func restoreEnv() {
 	os.Setenv("AZURE_CLIENT_ID", azureClientID)
 	os.Setenv("AZURE_SUBSCRIPTION_ID", azureSubscriptionID)
 }
@@ -39,27 +39,32 @@ func TestNewDNSProviderValid(t *testing.T) {
 	if !azureLiveTest {
 		t.Skip("skipping live test (requires credentials)")
 	}
+
+	defer restoreEnv()
 	os.Setenv("AZURE_CLIENT_ID", "")
+
 	_, err := NewDNSProviderCredentials(azureClientID, azureClientSecret, azureSubscriptionID, azureTenantID, azureResourceGroup)
 	assert.NoError(t, err)
-	restoreAzureEnv()
 }
 
 func TestNewDNSProviderValidEnv(t *testing.T) {
 	if !azureLiveTest {
 		t.Skip("skipping live test (requires credentials)")
 	}
+
+	defer restoreEnv()
 	os.Setenv("AZURE_CLIENT_ID", "other")
+
 	_, err := NewDNSProvider()
 	assert.NoError(t, err)
-	restoreAzureEnv()
 }
 
 func TestNewDNSProviderMissingCredErr(t *testing.T) {
+	defer restoreEnv()
 	os.Setenv("AZURE_SUBSCRIPTION_ID", "")
+
 	_, err := NewDNSProvider()
-	assert.EqualError(t, err, "Azure configuration missing: AZURE_CLIENT_ID,AZURE_CLIENT_SECRET,AZURE_SUBSCRIPTION_ID,AZURE_TENANT_ID,AZURE_RESOURCE_GROUP")
-	restoreAzureEnv()
+	assert.EqualError(t, err, "Azure: some credentials information are missing: AZURE_CLIENT_ID,AZURE_CLIENT_SECRET,AZURE_SUBSCRIPTION_ID,AZURE_TENANT_ID,AZURE_RESOURCE_GROUP")
 }
 
 func TestLiveAzurePresent(t *testing.T) {

providers/dns/bluecat/bluecat.go

@@ -6,16 +6,15 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"net/http"
-	"os"
 	"regexp"
 	"strconv"
 	"strings"
 	"time"
 
-	"io/ioutil"
-
 	"github.com/xenolf/lego/acme"
+	"github.com/xenolf/lego/platform/config/env"
 )
 
 const bluecatURLTemplate = "%s/Services/REST/v1"
@@ -51,29 +50,42 @@ type DNSProvider struct {
 // and external DNS View Name must be passed in BLUECAT_CONFIG_NAME and
 // BLUECAT_DNS_VIEW
 func NewDNSProvider() (*DNSProvider, error) {
-	server := os.Getenv("BLUECAT_SERVER_URL")
-	userName := os.Getenv("BLUECAT_USER_NAME")
-	password := os.Getenv("BLUECAT_PASSWORD")
-	configName := os.Getenv("BLUECAT_CONFIG_NAME")
-	dnsView := os.Getenv("BLUECAT_DNS_VIEW")
-	httpClient := http.Client{Timeout: 30 * time.Second}
-	return NewDNSProviderCredentials(server, userName, password, configName, dnsView, httpClient)
+	values, err := env.Get("BLUECAT_SERVER_URL", "BLUECAT_USER_NAME", "BLUECAT_CONFIG_NAME", "BLUECAT_CONFIG_NAME", "BLUECAT_DNS_VIEW")
+	if err != nil {
+		return nil, fmt.Errorf("BlueCat: %v", err)
+	}
+
+	httpClient := &http.Client{Timeout: 30 * time.Second}
+
+	return NewDNSProviderCredentials(
+		values["BLUECAT_SERVER_URL"],
+		values["BLUECAT_USER_NAME"],
+		values["BLUECAT_PASSWORD"],
+		values["BLUECAT_CONFIG_NAME"],
+		values["BLUECAT_DNS_VIEW"],
+		httpClient,
+	)
 }
 
 // NewDNSProviderCredentials uses the supplied credentials to return a
 // DNSProvider instance configured for Bluecat DNS.
-func NewDNSProviderCredentials(server, userName, password, configName, dnsView string, httpClient http.Client) (*DNSProvider, error) {
+func NewDNSProviderCredentials(server, userName, password, configName, dnsView string, httpClient *http.Client) (*DNSProvider, error) {
 	if server == "" || userName == "" || password == "" || configName == "" || dnsView == "" {
 		return nil, fmt.Errorf("Bluecat credentials missing")
 	}
 
+	client := http.DefaultClient
+	if httpClient != nil {
+		client = httpClient
+	}
+
 	return &DNSProvider{
 		baseURL:    fmt.Sprintf(bluecatURLTemplate, server),
 		userName:   userName,
 		password:   password,
 		configName: configName,
 		dnsView:    dnsView,
-		httpClient: http.DefaultClient,
+		httpClient: client,
 	}, nil
 }