Repository: xenolf/lego
Pull request #744 (2018-12-31 10:31:40)

transip: concurrent access to the API.
The Transip API doesn't allow to add a record without update all the zone/domain records. 

In some case, this produce a race condition.

See https://github.com/containous/traefik/issues/4346

**Note:** a wildcard certificate is the best way to handle this case.


Patch:
diff --git a/providers/dns/transip/transip.go b/providers/dns/transip/transip.go
index 062fc91a..ee88af28 100644
--- a/providers/dns/transip/transip.go
+++ b/providers/dns/transip/transip.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/transip/gotransip"
@@ -33,8 +34,9 @@ func NewDefaultConfig() *Config {
 
 // DNSProvider describes a provider for TransIP
 type DNSProvider struct {
-	config *Config
-	client gotransip.SOAPClient
+	config       *Config
+	client       gotransip.Client
+	dnsEntriesMu sync.Mutex
 }
 
 // NewDNSProvider returns a DNSProvider instance configured for TransIP.
@@ -90,6 +92,10 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 	// get the subDomain
 	subDomain := strings.TrimSuffix(dns01.UnFqdn(fqdn), "."+domainName)
 
+	// use mutex to prevent race condition from GetInfo until SetDNSEntries
+	d.dnsEntriesMu.Lock()
+	defer d.dnsEntriesMu.Unlock()
+
 	// get all DNS entries
 	info, err := transipdomain.GetInfo(d.client, domainName)
 	if err != nil {
@@ -126,6 +132,10 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error {
 	// get the subDomain
 	subDomain := strings.TrimSuffix(dns01.UnFqdn(fqdn), "."+domainName)
 
+	// use mutex to prevent race condition from GetInfo until SetDNSEntries
+	d.dnsEntriesMu.Lock()
+	defer d.dnsEntriesMu.Unlock()
+
 	// get all DNS entries
 	info, err := transipdomain.GetInfo(d.client, domainName)
 	if err != nil {


Test patch:
diff --git a/providers/dns/transip/transip_test.go b/providers/dns/transip/transip_test.go
index 35c622ca..464b9424 100644
--- a/providers/dns/transip/transip_test.go
+++ b/providers/dns/transip/transip_test.go
@@ -1,13 +1,82 @@
 package transip
 
 import (
+	"encoding/xml"
+	"fmt"
+	"reflect"
+	"strings"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/transip/gotransip"
+	"github.com/transip/gotransip/domain"
+	"github.com/xenolf/lego/log"
 	"github.com/xenolf/lego/platform/tester"
 )
 
+type argDNSEntries struct {
+	Items domain.DNSEntries `xml:"item"`
+}
+
+type argDomainName struct {
+	DomainName string `xml:",chardata"`
+}
+
+type fakeClient struct {
+	dnsEntries           []domain.DNSEntry
+	setDNSEntriesLatency time.Duration
+	getInfoLatency       time.Duration
+}
+
+func (f *fakeClient) Call(r gotransip.SoapRequest, b interface{}) error {
+	switch r.Method {
+	case "getInfo":
+		d := b.(*domain.Domain)
+		cp := f.dnsEntries
+
+		if f.getInfoLatency != 0 {
+			time.Sleep(f.getInfoLatency)
+		}
+		d.DNSEntries = cp
+
+		log.Printf("getInfo: %+v\n", d.DNSEntries)
+		return nil
+	case "setDnsEntries":
+		var domainName argDomainName
+		var dnsEntries argDNSEntries
+
+		args := readArgs(r)
+		for _, arg := range args {
+			if strings.HasPrefix(arg, "<domainName") {
+				err := xml.Unmarshal([]byte(arg), &domainName)
+				if err != nil {
+					panic(err)
+				}
+			} else if strings.HasPrefix(arg, "<dnsEntries") {
+				err := xml.Unmarshal([]byte(arg), &dnsEntries)
+				if err != nil {
+					panic(err)
+				}
+			}
+		}
+
+		log.Printf("setDnsEntries domainName: %+v\n", domainName)
+		log.Printf("setDnsEntries dnsEntries: %+v\n", dnsEntries)
+
+		if f.setDNSEntriesLatency != 0 {
+			time.Sleep(f.setDNSEntriesLatency)
+		}
+
+		f.dnsEntries = dnsEntries.Items
+		return nil
+	default:
+		return nil
+	}
+}
+
 var envTest = tester.NewEnvTest(
 	"TRANSIP_ACCOUNT_NAME",
 	"TRANSIP_PRIVATE_KEY_PATH").
@@ -135,6 +204,110 @@ func TestNewDNSProviderConfig(t *testing.T) {
 	}
 }
 
+func TestDNSProvider_concurrentGetInfo(t *testing.T) {
+	client := &fakeClient{
+		getInfoLatency:       50 * time.Millisecond,
+		setDNSEntriesLatency: 500 * time.Millisecond,
+	}
+
+	p := &DNSProvider{
+		config: NewDefaultConfig(),
+		client: client,
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	solve := func(domain1 string, suffix string, timeoutPresent time.Duration, timeoutSolve time.Duration, timeoutCleanup time.Duration) error {
+		time.Sleep(timeoutPresent)
+		err := p.Present(domain1, "", "")
+		if err != nil {
+			return err
+		}
+
+		time.Sleep(timeoutSolve)
+		var found bool
+		for _, entry := range client.dnsEntries {
+			if strings.HasSuffix(entry.Name, suffix) {
+				found = true
+			}
+		}
+		if !found {
+			return fmt.Errorf("record %s not found: %v", suffix, client.dnsEntries)
+		}
+
+		time.Sleep(timeoutCleanup)
+		return p.CleanUp(domain1, "", "")
+	}
+
+	go func() {
+		defer wg.Done()
+		err := solve("bar.lego.wtf", ".bar", 500*time.Millisecond, 100*time.Millisecond, 100*time.Millisecond)
+		require.NoError(t, err)
+	}()
+
+	go func() {
+		defer wg.Done()
+		err := solve("foo.lego.wtf", ".foo", 500*time.Millisecond, 200*time.Millisecond, 100*time.Millisecond)
+		require.NoError(t, err)
+	}()
+
+	wg.Wait()
+
+	assert.Empty(t, client.dnsEntries)
+}
+
+func TestDNSProvider_concurrentSetDNSEntries(t *testing.T) {
+	client := &fakeClient{}
+
+	p := &DNSProvider{
+		config: NewDefaultConfig(),
+		client: client,
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	solve := func(domain1 string, timeoutPresent time.Duration, timeoutCleanup time.Duration) error {
+		time.Sleep(timeoutPresent)
+		err := p.Present(domain1, "", "")
+		if err != nil {
+			return err
+		}
+
+		time.Sleep(timeoutCleanup)
+		return p.CleanUp(domain1, "", "")
+	}
+
+	go func() {
+		defer wg.Done()
+		err := solve("bar.lego.wtf", 550*time.Millisecond, 500*time.Millisecond)
+		require.NoError(t, err)
+	}()
+
+	go func() {
+		defer wg.Done()
+		err := solve("foo.lego.wtf", 500*time.Millisecond, 100*time.Millisecond)
+		require.NoError(t, err)
+	}()
+
+	wg.Wait()
+
+	assert.Empty(t, client.dnsEntries)
+}
+
+func readArgs(req gotransip.SoapRequest) []string {
+	v := reflect.ValueOf(req)
+	f := v.FieldByName("args")
+
+	var args []string
+	for i := 0; i < f.Len(); i++ {
+		args = append(args, f.Slice(0, f.Len()).Index(i).String())
+	}
+
+	return args
+}
+
 func TestLivePresent(t *testing.T) {
 	if !envTest.IsLiveTest() {
 		t.Skip("skipping live test")