Repository: xenolf/lego
Pull request #742 (2018-12-24 00:44:25)

gcloud: fix duplicate RR
- fix duplicate RR
- add tests


Patch:
diff --git a/.golangci.toml b/.golangci.toml
index 56a54da7..8d052187 100644
--- a/.golangci.toml
+++ b/.golangci.toml
@@ -40,6 +40,7 @@
     "exported (type|method|function) (.+) should have comment or be unexported",
     "possible misuse of unsafe.Pointer",
     "cyclomatic complexity (.+) of func `NewDNSChallengeProviderByName` is high (.+)", # providers/dns/dns_providers.go
+    "string `(lego\\.wtf|manhattan)` has (\\d+) occurrences, make it a constant", #providers/dns/gcloud/googlecloud_test.go
 
     "`(tlsFeatureExtensionOID|ocspMustStapleFeature)` is a global variable", # certcrypto/crypto.go
     "`(defaultNameservers|recursiveNameservers|dnsTimeout|fqdnToZone|muFqdnToZone)` is a global variable", # challenge/dns01/nameserver.go
diff --git a/providers/dns/gcloud/googlecloud.go b/providers/dns/gcloud/googlecloud.go
index 52bc957a..9d065174 100644
--- a/providers/dns/gcloud/googlecloud.go
+++ b/providers/dns/gcloud/googlecloud.go
@@ -21,6 +21,10 @@ import (
 	"google.golang.org/api/googleapi"
 )
 
+const (
+	changeStatusDone = "done"
+)
+
 // Config is used to configure the creation of the DNSProvider
 type Config struct {
 	Debug              bool
@@ -174,7 +178,7 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 	for _, rrSet := range existingRrSet {
 		for _, rr := range rrSet.Rrdatas {
 			if rr != value {
-				rec.Rrdatas = append(rec.Rrdatas, rrSet.Rrdatas...)
+				rec.Rrdatas = append(rec.Rrdatas, rr)
 			}
 		}
 	}
@@ -208,7 +212,7 @@ func (d *DNSProvider) applyChanges(zone string, change *dns.Change) error {
 		return fmt.Errorf("failed to perform changes [zone %s, change %s]: %v", zone, string(data), err)
 	}
 
-	if chg.Status == "done" {
+	if chg.Status == changeStatusDone {
 		return nil
 	}
 
@@ -227,7 +231,7 @@ func (d *DNSProvider) applyChanges(zone string, change *dns.Change) error {
 			return false, fmt.Errorf("failed to get changes [zone %s, change %s]: %v", zone, string(data), err)
 		}
 
-		if chg.Status == "done" {
+		if chg.Status == changeStatusDone {
 			return true, nil
 		}
 


Test patch:
diff --git a/providers/dns/gcloud/googlecloud_test.go b/providers/dns/gcloud/googlecloud_test.go
index f58c697c..0cde500c 100644
--- a/providers/dns/gcloud/googlecloud_test.go
+++ b/providers/dns/gcloud/googlecloud_test.go
@@ -1,6 +1,11 @@
 package gcloud
 
 import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"sort"
 	"testing"
 	"time"
 
@@ -114,6 +119,249 @@ func TestNewDNSProviderConfig(t *testing.T) {
 	}
 }
 
+func TestPresentNoExistingRR(t *testing.T) {
+	mux := http.NewServeMux()
+
+	// getHostedZone: /manhattan/managedZones?alt=json&dnsName=lego.wtf.
+	mux.HandleFunc("/manhattan/managedZones", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		mzlrs := &dns.ManagedZonesListResponse{
+			ManagedZones: []*dns.ManagedZone{
+				{Name: "test"},
+			},
+		}
+
+		err := json.NewEncoder(w).Encode(mzlrs)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	// findTxtRecords: /manhattan/managedZones/test/rrsets?alt=json&name=_acme-challenge.lego.wtf.&type=TXT
+	mux.HandleFunc("/manhattan/managedZones/test/rrsets", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		rrslr := &dns.ResourceRecordSetsListResponse{
+			Rrsets: []*dns.ResourceRecordSet{},
+		}
+
+		err := json.NewEncoder(w).Encode(rrslr)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	// applyChanges [Create]: /manhattan/managedZones/test/changes?alt=json
+	mux.HandleFunc("/manhattan/managedZones/test/changes", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		var chgReq dns.Change
+		if err := json.NewDecoder(r.Body).Decode(&chgReq); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		chgResp := chgReq
+		chgResp.Status = changeStatusDone
+
+		if err := json.NewEncoder(w).Encode(chgResp); err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	server := httptest.NewServer(mux)
+
+	config := NewDefaultConfig()
+	config.HTTPClient = &http.Client{}
+	config.Project = "manhattan"
+
+	p, err := NewDNSProviderConfig(config)
+	require.NoError(t, err)
+
+	p.client.BasePath = server.URL
+
+	domain := "lego.wtf"
+
+	err = p.Present(domain, "", "")
+	require.NoError(t, err)
+}
+
+func TestPresentWithExistingRR(t *testing.T) {
+	mux := http.NewServeMux()
+
+	// getHostedZone: /manhattan/managedZones?alt=json&dnsName=lego.wtf.
+	mux.HandleFunc("/manhattan/managedZones", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		mzlrs := &dns.ManagedZonesListResponse{
+			ManagedZones: []*dns.ManagedZone{
+				{Name: "test"},
+			},
+		}
+
+		err := json.NewEncoder(w).Encode(mzlrs)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	// findTxtRecords: /manhattan/managedZones/test/rrsets?alt=json&name=_acme-challenge.lego.wtf.&type=TXT
+	mux.HandleFunc("/manhattan/managedZones/test/rrsets", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		rrslr := &dns.ResourceRecordSetsListResponse{
+			Rrsets: []*dns.ResourceRecordSet{{
+				Name:    "_acme-challenge.lego.wtf.",
+				Rrdatas: []string{`"X7DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU"`, `"huji"`},
+				Ttl:     120,
+				Type:    "TXT",
+			}},
+		}
+
+		err := json.NewEncoder(w).Encode(rrslr)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	// applyChanges [Create]: /manhattan/managedZones/test/changes?alt=json
+	mux.HandleFunc("/manhattan/managedZones/test/changes", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		var chgReq dns.Change
+		if err := json.NewDecoder(r.Body).Decode(&chgReq); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if len(chgReq.Additions) > 0 {
+			sort.Strings(chgReq.Additions[0].Rrdatas)
+		}
+
+		var prevVal string
+		for _, addition := range chgReq.Additions {
+			for _, value := range addition.Rrdatas {
+				if prevVal == value {
+					http.Error(w, fmt.Sprintf("The resource %s already exists", value), http.StatusConflict)
+					return
+				}
+				prevVal = value
+			}
+		}
+
+		chgResp := chgReq
+		chgResp.Status = changeStatusDone
+
+		if err := json.NewEncoder(w).Encode(chgResp); err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	server := httptest.NewServer(mux)
+
+	config := NewDefaultConfig()
+	config.HTTPClient = &http.Client{}
+	config.Project = "manhattan"
+
+	p, err := NewDNSProviderConfig(config)
+	require.NoError(t, err)
+
+	p.client.BasePath = server.URL
+
+	domain := "lego.wtf"
+
+	err = p.Present(domain, "", "")
+	require.NoError(t, err)
+}
+
+func TestPresentSkipExistingRR(t *testing.T) {
+	mux := http.NewServeMux()
+
+	// getHostedZone: /manhattan/managedZones?alt=json&dnsName=lego.wtf.
+	mux.HandleFunc("/manhattan/managedZones", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		mzlrs := &dns.ManagedZonesListResponse{
+			ManagedZones: []*dns.ManagedZone{
+				{Name: "test"},
+			},
+		}
+
+		err := json.NewEncoder(w).Encode(mzlrs)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	// findTxtRecords: /manhattan/managedZones/test/rrsets?alt=json&name=_acme-challenge.lego.wtf.&type=TXT
+	mux.HandleFunc("/manhattan/managedZones/test/rrsets", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+			return
+		}
+
+		rrslr := &dns.ResourceRecordSetsListResponse{
+			Rrsets: []*dns.ResourceRecordSet{{
+				Name:    "_acme-challenge.lego.wtf.",
+				Rrdatas: []string{`"47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU"`, `"X7DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU"`, `"huji"`},
+				Ttl:     120,
+				Type:    "TXT",
+			}},
+		}
+
+		err := json.NewEncoder(w).Encode(rrslr)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+
+	server := httptest.NewServer(mux)
+
+	config := NewDefaultConfig()
+	config.HTTPClient = &http.Client{}
+	config.Project = "manhattan"
+
+	p, err := NewDNSProviderConfig(config)
+	require.NoError(t, err)
+
+	p.client.BasePath = server.URL
+
+	domain := "lego.wtf"
+
+	err = p.Present(domain, "", "")
+	require.NoError(t, err)
+}
+
 func TestLivePresent(t *testing.T) {
 	if !envTest.IsLiveTest() {
 		t.Skip("skipping live test")