review

Author: ldez

providers/dns/pdns/internal/client.go

@@ -30,10 +30,11 @@ type Client struct {
 }
 
 // NewClient creates a new Client.
-func NewClient(host *url.URL, serverName string, apiKey string) *Client {
+func NewClient(host *url.URL, serverName string, apiVersion int, apiKey string) *Client {
 	return &Client{
 		serverName: serverName,
 		apiKey:     apiKey,
+		apiVersion: apiVersion,
 		Host:       host,
 		HTTPClient: &http.Client{Timeout: 5 * time.Second},
 	}
@@ -43,10 +44,6 @@ func (c *Client) APIVersion() int {
 	return c.apiVersion
 }
 
-func (c *Client) SetCustomAPIVersion(version int) {
-	c.apiVersion = version
-}
-
 func (c *Client) SetAPIVersion(ctx context.Context) error {
 	var err error
 

providers/dns/pdns/internal/client_test.go

@@ -57,7 +57,7 @@ func setupTest(t *testing.T, method, pattern string, status int, file string) *C
 
 	serverURL, _ := url.Parse(server.URL)
 
-	client := NewClient(serverURL, "server", "secret")
+	client := NewClient(serverURL, "server", 0, "secret")
 	client.HTTPClient = server.Client()
 
 	return client
@@ -151,8 +151,7 @@ func TestClient_joinPath(t *testing.T) {
 			host, err := url.Parse(test.baseURL)
 			require.NoError(t, err)
 
-			client := NewClient(host, "test", "secret")
-			client.apiVersion = test.apiVersion
+			client := NewClient(host, "test", test.apiVersion, "secret")
 
 			endpoint := client.joinPath(test.uri)
 

providers/dns/pdns/pdns.go

@@ -23,7 +23,7 @@ const (
 	EnvAPIURL = envNamespace + "API_URL"
 
 	EnvTTL                = envNamespace + "TTL"
-	EnvCustomAPIVersion   = envNamespace + "CUSTOM_API_VERSION"
+	EnvAPIVersion         = envNamespace + "API_VERSION"
 	EnvPropagationTimeout = envNamespace + "PROPAGATION_TIMEOUT"
 	EnvPollingInterval    = envNamespace + "POLLING_INTERVAL"
 	EnvHTTPTimeout        = envNamespace + "HTTP_TIMEOUT"
@@ -35,21 +35,21 @@ type Config struct {
 	APIKey             string
 	Host               *url.URL
 	ServerName         string
+	APIVersion         int
 	PropagationTimeout time.Duration
 	PollingInterval    time.Duration
 	TTL                int
-	CustomAPIVersion   int
 	HTTPClient         *http.Client
 }
 
 // NewDefaultConfig returns a default configuration for the DNSProvider.
 func NewDefaultConfig() *Config {
 	return &Config{
+		ServerName:         env.GetOrDefaultString(EnvServerName, "localhost"),
+		APIVersion:         env.GetOrDefaultInt(EnvAPIVersion, 0),
 		TTL:                env.GetOrDefaultInt(EnvTTL, dns01.DefaultTTL),
-		CustomAPIVersion:   env.GetOrDefaultInt(EnvCustomAPIVersion, 0),
 		PropagationTimeout: env.GetOrDefaultSecond(EnvPropagationTimeout, 120*time.Second),
 		PollingInterval:    env.GetOrDefaultSecond(EnvPollingInterval, 2*time.Second),
-		ServerName:         env.GetOrDefaultString(EnvServerName, "localhost"),
 		HTTPClient: &http.Client{
 			Timeout: env.GetOrDefaultSecond(EnvHTTPTimeout, 30*time.Second),
 		},
@@ -97,12 +97,9 @@ func NewDNSProviderConfig(config *Config) (*DNSProvider, error) {
 		return nil, errors.New("pdns: API URL missing")
 	}
 
-	client := internal.NewClient(config.Host, config.ServerName, config.APIKey)
+	client := internal.NewClient(config.Host, config.ServerName, config.APIVersion, config.APIKey)
 
-	if config.CustomAPIVersion > 0 {
-		client.SetCustomAPIVersion(config.CustomAPIVersion)
-		log.Infof("pdns: using custom API version %d", config.CustomAPIVersion)
-	} else {
+	if config.APIVersion <= 0 {
 		err := client.SetAPIVersion(context.Background())
 		if err != nil {
 			log.Warnf("pdns: failed to get API version %v", err)

providers/dns/pdns/pdns.toml

@@ -18,20 +18,20 @@ Tested and confirmed to work with PowerDNS authoritative server 3.4.8 and 4.0.1.
 PowerDNS Notes:
 - PowerDNS API does not currently support SSL, therefore you should take care to ensure that traffic between lego and the PowerDNS API is over a trusted network, VPN etc.
 - In order to have the SOA serial automatically increment each time the `_acme-challenge` record is added/modified via the API, set `SOA-EDIT-API` to `INCEPTION-INCREMENT` for the zone in the `domainmetadata` table
-- Some PowerDNS servers doesn't have root API endpoints enabled and API version autodetection will not work. In that case version number can be defined using `PDNS_CUSTOM_API_VERSION`.
+- Some PowerDNS servers doesn't have root API endpoints enabled and API version autodetection will not work. In that case version number can be defined using `PDNS_API_VERSION`.
 '''
 
 [Configuration]
   [Configuration.Credentials]
     PDNS_API_KEY = "API key"
     PDNS_API_URL = "API URL"
   [Configuration.Additional]
+    PDNS_SERVER_NAME = "Name of the server in the URL, 'localhost' by default"
+    PDNS_API_VERSION = "Skip API version autodetection and use the provided version number."
     PDNS_POLLING_INTERVAL = "Time between DNS propagation check"
     PDNS_PROPAGATION_TIMEOUT = "Maximum waiting time for DNS propagation"
     PDNS_TTL = "The TTL of the TXT record used for the DNS challenge"
     PDNS_HTTP_TIMEOUT = "API request timeout"
-    PDNS_SERVER_NAME = "Name of the server in the URL, 'localhost' by default"
-    PDNS_CUSTOM_API_VERSION = "Skip API version autodetection and use custom version number."
 
 [Links]
   API = "https://doc.powerdns.com/md/httpapi/README/"

providers/dns/pdns/pdns_test.go

@@ -85,31 +85,22 @@ func TestNewDNSProviderConfig(t *testing.T) {
 		{
 			desc:   "success",
 			apiKey: "123",
-			host: func() *url.URL {
-				u, _ := url.Parse("http://example.com")
-				return u
-			}(),
+			host:   mustParse("http://example.com"),
 		},
 		{
 			desc:             "success custom API version",
 			apiKey:           "123",
 			customAPIVersion: 1,
-			host: func() *url.URL {
-				u, _ := url.Parse("http://example.com")
-				return u
-			}(),
+			host:             mustParse("http://example.com"),
 		},
 		{
 			desc:     "missing credentials",
 			expected: "pdns: API key missing",
 		},
 		{
-			desc:   "missing API key",
-			apiKey: "",
-			host: func() *url.URL {
-				u, _ := url.Parse("http://example.com")
-				return u
-			}(),
+			desc:     "missing API key",
+			apiKey:   "",
+			host:     mustParse("http://example.com"),
 			expected: "pdns: API key missing",
 		},
 		{
@@ -124,7 +115,7 @@ func TestNewDNSProviderConfig(t *testing.T) {
 			config := NewDefaultConfig()
 			config.APIKey = test.apiKey
 			config.Host = test.host
-			config.CustomAPIVersion = test.customAPIVersion
+			config.APIVersion = test.customAPIVersion
 
 			p, err := NewDNSProviderConfig(config)
 
@@ -154,3 +145,11 @@ func TestLivePresentAndCleanup(t *testing.T) {
 	err = provider.CleanUp(envTest.GetDomain(), "", "123d==")
 	require.NoError(t, err)
 }
+
+func mustParse(rawURL string) *url.URL {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		panic(err)
+	}
+	return u
+}