Repository: xenolf/lego
Pull request #740 (2018-12-21 21:06:02)

gcloud: fix for wildcard
the gcloud API returns values with quotes, this produce some errors for wildcards.

```
time="2018-11-01T11:46:41Z" level=error msg="Error obtaining certificate: acme: Error -> One or more domains had a problem:\n[qa.mydomain.com] error presenting token: googlecloud: googleapi: Error 409: The resource 'entity.change.additions[0].rrdata[1]' named '\"VuOLZS3tfLfrOswFtV3mayqOYaz5PvQK16nJdgMdMez\"' already exists, alreadyExists\n"
time="2018-11-01T11:46:41Z" level=error msg="Unable to obtain ACME certificate for domains \"*.qa.mydomain.com,qa.mydomain.com\" : unable to generate a certificate for the domains [*.qa.mydomain.com qa.mydomain.com]: acme: Error -> One or more domains had a problem:\n[qa.mydomain.com] error presenting token: googlecloud: googleapi: Error 409: The resource 'entity.change.additions[0].rrdata[1]' named '\"VuOLZS3tfLfrOswFtV3mayqOYaz5PvQK16nJdgMdMez\"' already exists, alreadyExists\n"
time="2018-11-01T11:46:41Z" level=error msg="Error obtaining certificate: acme: Error -> One or more domains had a problem:\n[development.mydomain.com] error presenting token: googlecloud: googleapi: Error 409: The resource 'entity.change.additions[0].rrdata[1]' named '\"Jfv6orVfbwi0IdQbmpvG3qnIltMX5x56vGUpHjoBzOa\"' already exists, alreadyExists\n"
time="2018-11-01T11:46:41Z" level=error msg="Unable to obtain ACME certificate for domains \"*.development.mydomain.com,development.mydomain.com\" : unable to generate a certificate for the domains [*.development.mydomain.com development.mydomain.com]: acme: Error -> One or more domains had a problem:\n[development.mydomain.com] error presenting token: googlecloud: googleapi: Error 409: The resource 'entity.change.additions[0].rrdata[1]' named '\"Jfv6orVfbwi0IdQbmpvG3qnIltMX5x56vGUpHjoBzOa\"' already exists, alreadyExists\n"
```


Patch:
diff --git a/Gopkg.lock b/Gopkg.lock
index 6ecb7f67..cc4ee7a9 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -682,6 +682,7 @@
     "golang.org/x/oauth2/clientcredentials",
     "golang.org/x/oauth2/google",
     "google.golang.org/api/dns/v1",
+    "google.golang.org/api/googleapi",
     "gopkg.in/ns1/ns1-go.v2/rest",
     "gopkg.in/ns1/ns1-go.v2/rest/model/dns",
     "gopkg.in/square/go-jose.v2",
diff --git a/challenge/dns01/dns_challenge.go b/challenge/dns01/dns_challenge.go
index 1dc457af..d0ff1ec3 100644
--- a/challenge/dns01/dns_challenge.go
+++ b/challenge/dns01/dns_challenge.go
@@ -123,7 +123,7 @@ func (c *Challenge) Solve(authz acme.Authorization) error {
 
 	log.Infof("[%s] acme: Checking DNS record propagation using %+v", domain, recursiveNameservers)
 
-	err = wait.For(timeout, interval, func() (bool, error) {
+	err = wait.For("propagation", timeout, interval, func() (bool, error) {
 		stop, errP := c.preCheck.call(fqdn, value)
 		if !stop || errP != nil {
 			log.Infof("[%s] acme: Waiting for DNS record propagation.", domain)
diff --git a/challenge/dns01/precheck.go b/challenge/dns01/precheck.go
index 2639dfea..63b72cef 100644
--- a/challenge/dns01/precheck.go
+++ b/challenge/dns01/precheck.go
@@ -91,10 +91,14 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro
 			return false, fmt.Errorf("NS %s returned %s for %s", ns, dns.RcodeToString[r.Rcode], fqdn)
 		}
 
+		var records []string
+
 		var found bool
 		for _, rr := range r.Answer {
 			if txt, ok := rr.(*dns.TXT); ok {
-				if strings.Join(txt.Txt, "") == value {
+				record := strings.Join(txt.Txt, "")
+				records = append(records, record)
+				if record == value {
 					found = true
 					break
 				}
@@ -102,7 +106,7 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro
 		}
 
 		if !found {
-			return false, fmt.Errorf("NS %s did not return the expected TXT record [fqdn: %s]", ns, fqdn)
+			return false, fmt.Errorf("NS %s did not return the expected TXT record [fqdn: %s, value: %s]: %s", ns, fqdn, value, strings.Join(records, " ,"))
 		}
 	}
 
diff --git a/platform/wait/wait.go b/platform/wait/wait.go
index 65090bf8..511e1f28 100644
--- a/platform/wait/wait.go
+++ b/platform/wait/wait.go
@@ -8,8 +8,8 @@ import (
 )
 
 // For polls the given function 'f', once every 'interval', up to 'timeout'.
-func For(timeout, interval time.Duration, f func() (bool, error)) error {
-	log.Infof("Wait [timeout: %s, interval: %s]", timeout, interval)
+func For(msg string, timeout, interval time.Duration, f func() (bool, error)) error {
+	log.Infof("Wait for %s [timeout: %s, interval: %s]", msg, timeout, interval)
 
 	var lastErr string
 	timeUp := time.After(timeout)
diff --git a/providers/dns/gcloud/googlecloud.go b/providers/dns/gcloud/googlecloud.go
index 13c7a3aa..52bc957a 100644
--- a/providers/dns/gcloud/googlecloud.go
+++ b/providers/dns/gcloud/googlecloud.go
@@ -8,17 +8,22 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"strconv"
 	"time"
 
 	"github.com/xenolf/lego/challenge/dns01"
+	"github.com/xenolf/lego/log"
 	"github.com/xenolf/lego/platform/config/env"
+	"github.com/xenolf/lego/platform/wait"
 	"golang.org/x/net/context"
 	"golang.org/x/oauth2/google"
 	"google.golang.org/api/dns/v1"
+	"google.golang.org/api/googleapi"
 )
 
 // Config is used to configure the creation of the DNSProvider
 type Config struct {
+	Debug              bool
 	Project            string
 	PropagationTimeout time.Duration
 	PollingInterval    time.Duration
@@ -29,6 +34,7 @@ type Config struct {
 // NewDefaultConfig returns a default configuration for the DNSProvider
 func NewDefaultConfig() *Config {
 	return &Config{
+		Debug:              env.GetOrDefaultBool("GCE_DEBUG", false),
 		TTL:                env.GetOrDefaultInt("GCE_TTL", dns01.DefaultTTL),
 		PropagationTimeout: env.GetOrDefaultSecond("GCE_PROPAGATION_TIMEOUT", 180*time.Second),
 		PollingInterval:    env.GetOrDefaultSecond("GCE_POLLING_INTERVAL", 5*time.Second),
@@ -131,11 +137,32 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 	}
 
 	// Look for existing records.
-	existing, err := d.findTxtRecords(zone, fqdn)
+	existingRrSet, err := d.findTxtRecords(zone, fqdn)
 	if err != nil {
 		return fmt.Errorf("googlecloud: %v", err)
 	}
 
+	for _, rrSet := range existingRrSet {
+		var rrd []string
+		for _, rr := range rrSet.Rrdatas {
+			data := mustUnquote(rr)
+			rrd = append(rrd, data)
+
+			if data == value {
+				log.Printf("skip: the record already exists: %s", value)
+				return nil
+			}
+		}
+		rrSet.Rrdatas = rrd
+	}
+
+	// Attempt to delete the existing records before adding the new one.
+	if len(existingRrSet) > 0 {
+		if err = d.applyChanges(zone, &dns.Change{Deletions: existingRrSet}); err != nil {
+			return fmt.Errorf("googlecloud: %v", err)
+		}
+	}
+
 	rec := &dns.ResourceRecordSet{
 		Name:    fqdn,
 		Rrdatas: []string{value},
@@ -143,36 +170,69 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 		Type:    "TXT",
 	}
 
-	change := &dns.Change{}
+	// Append existing TXT record data to the new TXT record data
+	for _, rrSet := range existingRrSet {
+		for _, rr := range rrSet.Rrdatas {
+			if rr != value {
+				rec.Rrdatas = append(rec.Rrdatas, rrSet.Rrdatas...)
+			}
+		}
+	}
 
-	if len(existing) > 0 {
-		// Attempt to delete the existing records when adding our new one.
-		change.Deletions = existing
+	change := &dns.Change{
+		Additions: []*dns.ResourceRecordSet{rec},
+	}
 
-		// Append existing TXT record data to the new TXT record data
-		for _, value := range existing {
-			rec.Rrdatas = append(rec.Rrdatas, value.Rrdatas...)
-		}
+	if err = d.applyChanges(zone, change); err != nil {
+		return fmt.Errorf("googlecloud: %v", err)
 	}
 
-	change.Additions = []*dns.ResourceRecordSet{rec}
+	return nil
+}
+
+func (d *DNSProvider) applyChanges(zone string, change *dns.Change) error {
+	if d.config.Debug {
+		data, _ := json.Marshal(change)
+		log.Printf("change (Create): %s", string(data))
+	}
 
 	chg, err := d.client.Changes.Create(d.config.Project, zone, change).Do()
 	if err != nil {
-		return fmt.Errorf("googlecloud: %v", err)
+		if v, ok := err.(*googleapi.Error); ok {
+			if v.Code == http.StatusNotFound {
+				return nil
+			}
+		}
+
+		data, _ := json.Marshal(change)
+		return fmt.Errorf("failed to perform changes [zone %s, change %s]: %v", zone, string(data), err)
 	}
 
+	if chg.Status == "done" {
+		return nil
+	}
+
+	chgID := chg.Id
+
 	// wait for change to be acknowledged
-	for chg.Status == "pending" {
-		time.Sleep(time.Second)
+	return wait.For("apply change", 30*time.Second, 3*time.Second, func() (bool, error) {
+		if d.config.Debug {
+			data, _ := json.Marshal(change)
+			log.Printf("change (Get): %s", string(data))
+		}
 
-		chg, err = d.client.Changes.Get(d.config.Project, zone, chg.Id).Do()
+		chg, err = d.client.Changes.Get(d.config.Project, zone, chgID).Do()
 		if err != nil {
-			return fmt.Errorf("googlecloud: %v", err)
+			data, _ := json.Marshal(change)
+			return false, fmt.Errorf("failed to get changes [zone %s, change %s]: %v", zone, string(data), err)
 		}
-	}
 
-	return nil
+		if chg.Status == "done" {
+			return true, nil
+		}
+
+		return false, fmt.Errorf("status: %s", chg.Status)
+	})
 }
 
 // CleanUp removes the TXT record matching the specified parameters.
@@ -236,3 +296,11 @@ func (d *DNSProvider) findTxtRecords(zone, fqdn string) ([]*dns.ResourceRecordSe
 
 	return recs.Rrsets, nil
 }
+
+func mustUnquote(raw string) string {
+	clean, err := strconv.Unquote(raw)
+	if err != nil {
+		return raw
+	}
+	return clean
+}
diff --git a/providers/dns/nifcloud/nifcloud.go b/providers/dns/nifcloud/nifcloud.go
index 022e34b1..51b6b70a 100644
--- a/providers/dns/nifcloud/nifcloud.go
+++ b/providers/dns/nifcloud/nifcloud.go
@@ -146,7 +146,7 @@ func (d *DNSProvider) changeRecord(action, fqdn, value, domain string, ttl int)
 
 	statusID := resp.ChangeInfo.ID
 
-	return wait.For(120*time.Second, 4*time.Second, func() (bool, error) {
+	return wait.For("nifcloud", 120*time.Second, 4*time.Second, func() (bool, error) {
 		resp, err := d.client.GetChange(statusID)
 		if err != nil {
 			return false, fmt.Errorf("failed to query NIFCLOUD DNS change status: %v", err)
diff --git a/providers/dns/route53/route53.go b/providers/dns/route53/route53.go
index 6a70ae09..ad6ab539 100644
--- a/providers/dns/route53/route53.go
+++ b/providers/dns/route53/route53.go
@@ -197,7 +197,7 @@ func (d *DNSProvider) changeRecord(action, hostedZoneID string, recordSet *route
 
 	changeID := resp.ChangeInfo.Id
 
-	return wait.For(d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) {
+	return wait.For("route53", d.config.PropagationTimeout, d.config.PollingInterval, func() (bool, error) {
 		reqParams := &route53.GetChangeInput{Id: changeID}
 
 		resp, err := d.client.GetChange(reqParams)


Test patch:
diff --git a/e2e/loader/loader.go b/e2e/loader/loader.go
index 958b3e55..ec95ffa0 100644
--- a/e2e/loader/loader.go
+++ b/e2e/loader/loader.go
@@ -141,7 +141,7 @@ func (l *EnvLoader) cmdPebble() (*exec.Cmd, *bytes.Buffer) {
 
 func pebbleHealthCheck(options *CmdOption) {
 	client := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}}
-	err := wait.For(10*time.Second, 500*time.Millisecond, func() (bool, error) {
+	err := wait.For("pebble", 10*time.Second, 500*time.Millisecond, func() (bool, error) {
 		resp, err := client.Get(options.HealthCheckURL)
 		if err != nil {
 			return false, err
diff --git a/platform/wait/wait_test.go b/platform/wait/wait_test.go
index 42f00894..fc6673c1 100644
--- a/platform/wait/wait_test.go
+++ b/platform/wait/wait_test.go
@@ -8,7 +8,7 @@ import (
 func TestForTimeout(t *testing.T) {
 	c := make(chan error)
 	go func() {
-		err := For(3*time.Second, 1*time.Second, func() (bool, error) {
+		err := For("", 3*time.Second, 1*time.Second, func() (bool, error) {
 			return false, nil
 		})
 		c <- err