From bae7428c089eedaed1d01fcd13edaabcaf856693 Mon Sep 17 00:00:00 2001
From: Jan Broer <janeczku@yahoo.de>
Date: Wed, 3 Feb 2016 07:04:12 +0100
Subject: [PATCH] Fixes issues with the Present() method of Route53 provider: -
 InvalidTXTRDATA error when creating TXT record (closes #94) - Present()
 should poll and wait until the status of the record change becomes INSYNC
 (thanks @oov)

Adds a retry/timeout utility function to dns_challenge.go that may be used in other places
---
 acme/dns_challenge.go              | 20 ++++++++++++++++++++
 acme/dns_challenge_route53.go      | 20 ++++++++++++++++++--
 acme/dns_challenge_route53_test.go | 14 ++++++++++++--
 3 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/acme/dns_challenge.go b/acme/dns_challenge.go
index bca8d09c..f34fcccc 100644
--- a/acme/dns_challenge.go
+++ b/acme/dns_challenge.go
@@ -123,3 +123,23 @@ func unFqdn(name string) string {
 	}
 	return name
 }
+
+// waitFor polls the given function 'f', once per second, up to 'timeout' seconds.
+func waitFor(timeout int, f func() (bool, error)) error {
+	start := time.Now().Second()
+	for {
+		time.Sleep(1 * time.Second)
+
+		if delta := time.Now().Second() - start; delta >= timeout {
+			return fmt.Errorf("Time limit exceeded (%d seconds)", delta)
+		}
+
+		stop, err := f()
+		if err != nil {
+			return err
+		}
+		if stop {
+			return nil
+		}
+	}
+}
diff --git a/acme/dns_challenge_route53.go b/acme/dns_challenge_route53.go
index ba6f3e99..28ed0259 100644
--- a/acme/dns_challenge_route53.go
+++ b/acme/dns_challenge_route53.go
@@ -43,12 +43,14 @@ func NewDNSProviderRoute53(awsAccessKey, awsSecretKey, awsRegionName string) (*D
 // Present creates a TXT record using the specified parameters
 func (r *DNSProviderRoute53) Present(domain, token, keyAuth string) error {
 	fqdn, value, ttl := DNS01Record(domain, keyAuth)
+	value = `"` + value + `"`
 	return r.changeRecord("UPSERT", fqdn, value, ttl)
 }
 
 // CleanUp removes the TXT record matching the specified parameters
 func (r *DNSProviderRoute53) CleanUp(domain, token, keyAuth string) error {
 	fqdn, value, ttl := DNS01Record(domain, keyAuth)
+	value = `"` + value + `"`
 	return r.changeRecord("DELETE", fqdn, value, ttl)
 }
 
@@ -61,8 +63,21 @@ func (r *DNSProviderRoute53) changeRecord(action, fqdn, value string, ttl int) e
 	update := route53.Change{action, recordSet}
 	changes := []route53.Change{update}
 	req := route53.ChangeResourceRecordSetsRequest{Comment: "Created by Lego", Changes: changes}
-	_, err = r.client.ChangeResourceRecordSets(hostedZoneID, &req)
-	return err
+	resp, err := r.client.ChangeResourceRecordSets(hostedZoneID, &req)
+	if err != nil {
+		return err
+	}
+
+	return waitFor(90, func() (bool, error) {
+		status, err := r.client.GetChange(resp.ChangeInfo.ID)
+		if err != nil {
+			return false, err
+		}
+		if status == "INSYNC" {
+			return true, nil
+		}
+		return false, nil
+	})
 }
 
 func (r *DNSProviderRoute53) getHostedZoneID(fqdn string) (string, error) {
@@ -108,6 +123,7 @@ func newTXTRecordSet(fqdn, value string, ttl int) route53.ResourceRecordSet {
 		Records: []string{value},
 		TTL:     ttl,
 	}
+
 }
 
 // Route53 API has pretty strict rate limits (5req/s globally per account)
diff --git a/acme/dns_challenge_route53_test.go b/acme/dns_challenge_route53_test.go
index 3ab573af..eb551d03 100644
--- a/acme/dns_challenge_route53_test.go
+++ b/acme/dns_challenge_route53_test.go
@@ -51,9 +51,19 @@ var ListHostedZonesAnswer = `<?xml version="1.0" encoding="utf-8"?>
     <MaxItems>100</MaxItems>
 </ListHostedZonesResponse>`
 
+var GetChangeAnswer = `<?xml version="1.0" encoding="UTF-8"?>
+<GetChangeResponse xmlns="https://route53.amazonaws.com/doc/2013-04-01/">
+   <ChangeInfo>
+      <Id>/change/asdf</Id>
+      <Status>INSYNC</Status>
+      <SubmittedAt>2016-02-03T01:36:41.958Z</SubmittedAt>
+   </ChangeInfo>
+</GetChangeResponse>`
+
 var serverResponseMap = testutil.ResponseMap{
 	"/2013-04-01/hostedzone/":                      testutil.Response{200, nil, ListHostedZonesAnswer},
 	"/2013-04-01/hostedzone/Z2K123214213123/rrset": testutil.Response{200, nil, ChangeResourceRecordSetsAnswer},
+	"/2013-04-01/change/asdf":                      testutil.Response{200, nil, GetChangeAnswer},
 }
 
 func init() {
@@ -112,7 +122,7 @@ func TestRoute53Present(t *testing.T) {
 	assert := assert.New(t)
 	testServer := makeRoute53TestServer()
 	provider := makeRoute53Provider(testServer)
-	testServer.ResponseMap(2, serverResponseMap)
+	testServer.ResponseMap(3, serverResponseMap)
 
 	domain := "example.com"
 	keyAuth := "123456d=="
@@ -120,7 +130,7 @@ func TestRoute53Present(t *testing.T) {
 	err := provider.Present(domain, "", keyAuth)
 	assert.NoError(err, "Expected Present to return no error")
 
-	httpReqs := testServer.WaitRequests(2)
+	httpReqs := testServer.WaitRequests(3)
 	httpReq := httpReqs[1]
 
 	assert.Equal("/2013-04-01/hostedzone/Z2K123214213123/rrset", httpReq.URL.Path,