From 64f8e0d225d617aef20accefd538a833564fe084 Mon Sep 17 00:00:00 2001
From: Chris Marchesi <cmarchesi@paybyphone.com>
Date: Tue, 14 Jun 2016 17:03:31 -0700
Subject: [PATCH] providers/dns/route53: Adjust DNS challenge TTL to 10 seconds

While more than likely never to come up in a real-world situation,
during renewal integration testing a value of 120 seconds has
proven to be too high (the old challenge record has not expired
by the time the new one is created).
---
 providers/dns/route53/route53.go              | 15 ++--
 .../dns/route53/route53_integration_test.go   | 70 +++++++++++++++++++
 2 files changed, 78 insertions(+), 7 deletions(-)
 create mode 100644 providers/dns/route53/route53_integration_test.go

diff --git a/providers/dns/route53/route53.go b/providers/dns/route53/route53.go
index f8a4bc2b..f3e53a8e 100644
--- a/providers/dns/route53/route53.go
+++ b/providers/dns/route53/route53.go
@@ -18,6 +18,7 @@ import (
 
 const (
 	maxRetries = 5
+	route53TTL = 10
 )
 
 // DNSProvider implements the acme.ChallengeProvider interface
@@ -69,20 +70,20 @@ func NewDNSProvider() (*DNSProvider, error) {
 
 // Present creates a TXT record using the specified parameters
 func (r *DNSProvider) Present(domain, token, keyAuth string) error {
-	fqdn, value, ttl := acme.DNS01Record(domain, keyAuth)
+	fqdn, value, _ := acme.DNS01Record(domain, keyAuth)
 	value = `"` + value + `"`
-	return r.changeRecord("UPSERT", fqdn, value, ttl)
+	return r.changeRecord("UPSERT", fqdn, value, route53TTL)
 }
 
 // CleanUp removes the TXT record matching the specified parameters
 func (r *DNSProvider) CleanUp(domain, token, keyAuth string) error {
-	fqdn, value, ttl := acme.DNS01Record(domain, keyAuth)
+	fqdn, value, _ := acme.DNS01Record(domain, keyAuth)
 	value = `"` + value + `"`
-	return r.changeRecord("DELETE", fqdn, value, ttl)
+	return r.changeRecord("DELETE", fqdn, value, route53TTL)
 }
 
 func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error {
-	hostedZoneID, err := r.getHostedZoneID(fqdn)
+	hostedZoneID, err := getHostedZoneID(fqdn, r.client)
 	if err != nil {
 		return fmt.Errorf("Failed to determine Route 53 hosted zone ID: %v", err)
 	}
@@ -123,7 +124,7 @@ func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error {
 	})
 }
 
-func (r *DNSProvider) getHostedZoneID(fqdn string) (string, error) {
+func getHostedZoneID(fqdn string, client *route53.Route53) (string, error) {
 	authZone, err := acme.FindZoneByFqdn(fqdn, acme.RecursiveNameservers)
 	if err != nil {
 		return "", err
@@ -133,7 +134,7 @@ func (r *DNSProvider) getHostedZoneID(fqdn string) (string, error) {
 	reqParams := &route53.ListHostedZonesByNameInput{
 		DNSName: aws.String(acme.UnFqdn(authZone)),
 	}
-	resp, err := r.client.ListHostedZonesByName(reqParams)
+	resp, err := client.ListHostedZonesByName(reqParams)
 	if err != nil {
 		return "", err
 	}
diff --git a/providers/dns/route53/route53_integration_test.go b/providers/dns/route53/route53_integration_test.go
new file mode 100644
index 00000000..64678906
--- /dev/null
+++ b/providers/dns/route53/route53_integration_test.go
@@ -0,0 +1,70 @@
+package route53
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/session"
+	"github.com/aws/aws-sdk-go/service/route53"
+)
+
+func TestRoute53TTL(t *testing.T) {
+
+	m, err := testGetAndPreCheck()
+	if err != nil {
+		t.Skip(err.Error())
+	}
+
+	provider, err := NewDNSProvider()
+	if err != nil {
+		t.Fatalf("Fatal: %s", err.Error())
+	}
+
+	err = provider.Present(m["route53Domain"], "foo", "bar")
+	if err != nil {
+		t.Fatalf("Fatal: %s", err.Error())
+	}
+	// we need a separate R53 client here as the one in the DNS provider is
+	// unexported.
+	fqdn := "_acme-challenge." + m["route53Domain"] + "."
+	svc := route53.New(session.New())
+	zoneID, err := getHostedZoneID(fqdn, svc)
+	if err != nil {
+		provider.CleanUp(m["route53Domain"], "foo", "bar")
+		t.Fatalf("Fatal: %s", err.Error())
+	}
+	params := &route53.ListResourceRecordSetsInput{
+		HostedZoneId: aws.String(zoneID),
+	}
+	resp, err := svc.ListResourceRecordSets(params)
+	if err != nil {
+		provider.CleanUp(m["route53Domain"], "foo", "bar")
+		t.Fatalf("Fatal: %s", err.Error())
+	}
+
+	for _, v := range resp.ResourceRecordSets {
+		if *v.Name == fqdn && *v.Type == "TXT" && *v.TTL == 10 {
+			provider.CleanUp(m["route53Domain"], "foo", "bar")
+			return
+		}
+	}
+	provider.CleanUp(m["route53Domain"], "foo", "bar")
+	t.Fatalf("Could not find a TXT record for _acme-challenge.%s with a TTL of 10", m["route53Domain"])
+}
+
+func testGetAndPreCheck() (map[string]string, error) {
+	m := map[string]string{
+		"route53Key":    os.Getenv("AWS_ACCESS_KEY_ID"),
+		"route53Secret": os.Getenv("AWS_SECRET_ACCESS_KEY"),
+		"route53Region": os.Getenv("AWS_REGION"),
+		"route53Domain": os.Getenv("R53_DOMAIN"),
+	}
+	for _, v := range m {
+		if v == "" {
+			return nil, fmt.Errorf("AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION, and R53_DOMAIN are needed to run this test")
+		}
+	}
+	return m, nil
+}