Repository: xenolf/lego
Pull request #588 (2018-06-26 14:07:59)

Route53: make provider timeouts configurable
👋 I'm looking to [configure the timeout](https://github.com/vancluever/terraform-provider-acme/issues/55) for the Route53 provider, as I'm hitting the error @ 
https://github.com/xenolf/lego/blob/master/acme/utils.go#L15

```
[*********] Time limit exceeded. Last error: NS ns-71.awsdns-10.net. did not return the expected TXT record
```
This provides a way to pass in a different configuration for the provider, rather than using the hardcoded values. This keeps the current defaults.

Mongey: 👋 @ldez, is this something you'd consider merging ?
ianks: I am also experiencing timeouts with gcloud

```
acme: Error 400 - urn:ietf:params:acme:error:dns - DNS problem: NXDOMAIN looking up TXT
```

Would this patch help with that?
ldez: @ianks it's not related: `NXDOMAIN` means Non eXistant Domain.
Check your DNS and fush your DNS cache.


Patch:
diff --git a/providers/dns/route53/route53.go b/providers/dns/route53/route53.go
index adc15401..d7cc4c71 100644
--- a/providers/dns/route53/route53.go
+++ b/providers/dns/route53/route53.go
@@ -3,6 +3,7 @@
 package route53
 
 import (
+	"errors"
 	"fmt"
 	"math/rand"
 	"os"
@@ -17,15 +18,30 @@ import (
 	"github.com/xenolf/lego/acme"
 )
 
-const (
-	maxRetries = 5
-	route53TTL = 10
-)
+// Config is used to configure the creation of the DNSProvider
+type Config struct {
+	MaxRetries         int
+	TTL                int
+	PropagationTimeout time.Duration
+	PollingInterval    time.Duration
+	HostedZoneID       string
+}
+
+// NewDefaultConfig returns a default configuration for the DNSProvider
+func NewDefaultConfig() *Config {
+	return &Config{
+		MaxRetries:         5,
+		TTL:                10,
+		PropagationTimeout: time.Minute * 2,
+		PollingInterval:    time.Second * 4,
+		HostedZoneID:       os.Getenv("AWS_HOSTED_ZONE_ID"),
+	}
+}
 
 // DNSProvider implements the acme.ChallengeProvider interface
 type DNSProvider struct {
-	client       *route53.Route53
-	hostedZoneID string
+	client *route53.Route53
+	config *Config
 }
 
 // customRetryer implements the client.Retryer interface by composing the
@@ -65,35 +81,49 @@ func (d customRetryer) RetryRules(r *request.Request) time.Duration {
 //
 // See also: https://github.com/aws/aws-sdk-go/wiki/configuring-sdk
 func NewDNSProvider() (*DNSProvider, error) {
-	hostedZoneID := os.Getenv("AWS_HOSTED_ZONE_ID")
+	return NewDNSProviderConfig(NewDefaultConfig())
+}
+
+// NewDNSProviderConfig takes a given config ans returns a custom configured
+// DNSProvider instance
+func NewDNSProviderConfig(config *Config) (*DNSProvider, error) {
+	if config == nil {
+		return nil, errors.New("the configuration of the Route53 DNS provider is nil")
+	}
 
 	r := customRetryer{}
-	r.NumMaxRetries = maxRetries
-	config := request.WithRetryer(aws.NewConfig(), r)
-	session, err := session.NewSessionWithOptions(session.Options{Config: *config})
+	r.NumMaxRetries = config.MaxRetries
+	sessionCfg := request.WithRetryer(aws.NewConfig(), r)
+	session, err := session.NewSessionWithOptions(session.Options{Config: *sessionCfg})
 	if err != nil {
 		return nil, err
 	}
 	client := route53.New(session)
 
 	return &DNSProvider{
-		client:       client,
-		hostedZoneID: hostedZoneID,
+		client: client,
+		config: config,
 	}, nil
 }
 
+// Timeout returns the timeout and interval to use when checking for DNS
+// propagation.
+func (r *DNSProvider) Timeout() (timeout, interval time.Duration) {
+	return r.config.PropagationTimeout, r.config.PollingInterval
+}
+
 // Present creates a TXT record using the specified parameters
 func (r *DNSProvider) Present(domain, token, keyAuth string) error {
 	fqdn, value, _ := acme.DNS01Record(domain, keyAuth)
 	value = `"` + value + `"`
-	return r.changeRecord("UPSERT", fqdn, value, route53TTL)
+	return r.changeRecord("UPSERT", fqdn, value, r.config.TTL)
 }
 
 // CleanUp removes the TXT record matching the specified parameters
 func (r *DNSProvider) CleanUp(domain, token, keyAuth string) error {
 	fqdn, value, _ := acme.DNS01Record(domain, keyAuth)
 	value = `"` + value + `"`
-	return r.changeRecord("DELETE", fqdn, value, route53TTL)
+	return r.changeRecord("DELETE", fqdn, value, r.config.TTL)
 }
 
 func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error {
@@ -123,7 +153,7 @@ func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error {
 
 	statusID := resp.ChangeInfo.Id
 
-	return acme.WaitFor(120*time.Second, 4*time.Second, func() (bool, error) {
+	return acme.WaitFor(r.config.PropagationTimeout, r.config.PollingInterval, func() (bool, error) {
 		reqParams := &route53.GetChangeInput{
 			Id: statusID,
 		}
@@ -139,8 +169,8 @@ func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error {
 }
 
 func (r *DNSProvider) getHostedZoneID(fqdn string) (string, error) {
-	if r.hostedZoneID != "" {
-		return r.hostedZoneID, nil
+	if r.config.HostedZoneID != "" {
+		return r.config.HostedZoneID, nil
 	}
 
 	authZone, err := acme.FindZoneByFqdn(fqdn, acme.RecursiveNameservers)


Test patch:
diff --git a/providers/dns/route53/route53_test.go b/providers/dns/route53/route53_test.go
index 00bbe02e..6036082d 100644
--- a/providers/dns/route53/route53_test.go
+++ b/providers/dns/route53/route53_test.go
@@ -42,7 +42,8 @@ func makeRoute53Provider(ts *httptest.Server) *DNSProvider {
 	}
 
 	client := route53.New(session.New(config))
-	return &DNSProvider{client: client}
+	cfg := NewDefaultConfig()
+	return &DNSProvider{client: client, config: cfg}
 }
 
 func TestCredentialsFromEnv(t *testing.T) {