Fix error handling and add basic test

Signed-off-by: Bartosz Slawianowski <bartosz.slawianowski@natzka.com>
cert-manager · Nov 13, 2023 · d93a816 · d93a816
1 parent 94e63c1
commit d93a816
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 3 deletions.
diff --git a/pkg/issuer/acme/dns/azuredns/azuredns.go b/pkg/issuer/acme/dns/azuredns/azuredns.go
@@ -12,6 +12,7 @@ package azuredns
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -303,7 +304,8 @@ func (c *DNSProvider) trimFqdn(fqdn string, zone string) string {
 func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.RecordSet)) error {
 	set, err := c.recordClient.Get(context.TODO(), c.resourceGroupName, zone, name, dns.TXT)
 	if err != nil {
-		if de, ok := err.(*autorest.DetailedError); ok && de.StatusCode.(int) == 404 {
+		var de autorest.DetailedError
+		if errors.As(err, &de); de.StatusCode.(int) == 404 {
 			set = dns.RecordSet{
 				RecordSetProperties: &dns.RecordSetProperties{
 					TTL:        to.Int64Ptr(60),
@@ -320,6 +322,7 @@ func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.Recor
 
 	if len(*set.TxtRecords) == 0 {
 		if *set.Etag != "" {
+			// Etag will cause the deletion to fail if any updates happen concurrently
 			_, err = c.recordClient.Delete(context.TODO(), c.resourceGroupName, zone, name, dns.TXT, *set.Etag)
 			if err != nil {
 				return fmt.Errorf("cannot delete DNS record set: %w", err)
@@ -329,6 +332,8 @@ func (c *DNSProvider) updateTXTRecord(zone, name string, updater func(*dns.Recor
 		return nil
 	}
 
+	// This is used to indicate that we want the API call to fail if a conflicting record was created concurrently
+	// Only relevant when this is a new record, for updates conflicts are solved with Etag
 	var ifNoneMatch string
 	if *set.Etag == "" {
 		ifNoneMatch = "*"

diff --git a/pkg/issuer/acme/dns/azuredns/azuredns_test.go b/pkg/issuer/acme/dns/azuredns/azuredns_test.go
@@ -57,8 +57,18 @@ func TestLiveAzureDnsPresent(t *testing.T) {
 
 	err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "123d==")
 	assert.NoError(t, err)
+}
+
+func TestLiveAzureDnsPresentMultiple(t *testing.T) {
+	if !azureLiveTest {
+		t.Skip("skipping live test")
+	}
+	provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, &v1.AzureManagedIdentity{})
+	assert.NoError(t, err)
 
-	err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "456d==")
+	err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "123d==")
+	assert.NoError(t, err)
+	err = provider.Present(azureDomain, "_acme-challenge."+azureDomain+".", "1123d==")
 	assert.NoError(t, err)
 }
 
@@ -74,8 +84,21 @@ func TestLiveAzureDnsCleanUp(t *testing.T) {
 
 	err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "123d==")
 	assert.NoError(t, err)
+}
+
+func TestLiveAzureDnsCleanUpMultiple(t *testing.T) {
+	if !azureLiveTest {
+		t.Skip("skipping live test")
+	}
+
+	time.Sleep(time.Second * 10)
+
+	provider, err := NewDNSProviderCredentials("", azureClientID, azureClientSecret, azuresubscriptionID, azureTenantID, azureResourceGroupName, azureHostedZoneName, util.RecursiveNameservers, false, &v1.AzureManagedIdentity{})
+	assert.NoError(t, err)
 
-	err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "456d==")
+	err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "123d==")
+	assert.NoError(t, err)
+	err = provider.CleanUp(azureDomain, "_acme-challenge."+azureDomain+".", "1123d==")
 	assert.NoError(t, err)
 }