Preserve DNS provider record data for cleanup (#382)

Co-authored-by: bjornmp <bjornmp@users.noreply.github.com>

Author: bjornmp

solvers.go

@@ -410,7 +410,7 @@ func (m *DNSManager) createRecord(ctx context.Context, dnsName, recordType, reco
 		return zoneRecord{}, fmt.Errorf("expected one record, got %d: %v", len(results), results)
 	}
 
-	return zoneRecord{zone, results[0].RR()}, nil
+	return zoneRecord{zone, results[0]}, nil
 }
 
 // wait blocks until the TXT record created in Present() appears in
@@ -445,12 +445,13 @@ func (m *DNSManager) wait(ctx context.Context, zrec zoneRecord) error {
 	checkAuthoritativeServers := len(m.Resolvers) == 0
 	resolvers := RecursiveNameservers(m.Resolvers)
 
+	rr := zrec.record.RR()
 	recType := dns.TypeTXT
-	if zrec.record.RR().Type == "CNAME" {
+	if rr.Type == "CNAME" {
 		recType = dns.TypeCNAME
 	}
 
-	absName := libdns.AbsoluteName(zrec.record.Name, zrec.zone)
+	absName := libdns.AbsoluteName(rr.Name, zrec.zone)
 
 	var err error
 	start := time.Now()
@@ -463,14 +464,14 @@ func (m *DNSManager) wait(ctx context.Context, zrec zoneRecord) error {
 
 		logger.Debug("checking DNS propagation",
 			zap.String("fqdn", absName),
-			zap.String("record_type", zrec.record.Type),
-			zap.String("expected_data", zrec.record.Data),
+			zap.String("record_type", rr.Type),
+			zap.String("expected_data", rr.Data),
 			zap.Strings("resolvers", resolvers))
 
 		var ready bool
-		ready, err = checkDNSPropagation(ctx, logger, absName, recType, zrec.record.Data, checkAuthoritativeServers, resolvers)
+		ready, err = checkDNSPropagation(ctx, logger, absName, recType, rr.Data, checkAuthoritativeServers, resolvers)
 		if err != nil {
-			return fmt.Errorf("checking DNS propagation of %q (relative=%s zone=%s resolvers=%v): %w", absName, zrec.record.Name, zrec.zone, resolvers, err)
+			return fmt.Errorf("checking DNS propagation of %q (relative=%s zone=%s resolvers=%v): %w", absName, rr.Name, zrec.zone, resolvers, err)
 		}
 		if ready {
 			return nil
@@ -482,7 +483,7 @@ func (m *DNSManager) wait(ctx context.Context, zrec zoneRecord) error {
 
 type zoneRecord struct {
 	zone   string
-	record libdns.RR
+	record libdns.Record
 }
 
 // CleanUp deletes the DNS TXT record created in Present().
@@ -506,11 +507,12 @@ func (m *DNSManager) cleanUpRecord(_ context.Context, zrec zoneRecord) error {
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
 
+	rr := zrec.record.RR()
 	logger.Debug("deleting DNS record",
 		zap.String("zone", zrec.zone),
-		zap.String("record_name", zrec.record.Name),
-		zap.String("record_type", zrec.record.Type),
-		zap.String("record_data", zrec.record.Data))
+		zap.String("record_name", rr.Name),
+		zap.String("record_type", rr.Type),
+		zap.String("record_data", rr.Data))
 
 	_, err := m.DNSProvider.DeleteRecords(ctx, zrec.zone, []libdns.Record{zrec.record})
 	if err != nil {
@@ -552,7 +554,8 @@ func (s *DNSManager) getDNSPresentMemory(dnsName, recType, value string) (dnsPre
 	var memory dnsPresentMemory
 	var found bool
 	for _, mem := range s.records[dnsName] {
-		if mem.zoneRec.record.Type == recType && mem.zoneRec.record.Data == value {
+		rr := mem.zoneRec.record.RR()
+		if rr.Type == recType && rr.Data == value {
 			memory = mem
 			found = true
 			break
@@ -570,7 +573,7 @@ func (s *DNSManager) deleteDNSPresentMemory(dnsName, keyAuth string) {
 	defer s.recordsMu.Unlock()
 
 	for i, mem := range s.records[dnsName] {
-		if mem.zoneRec.record.Data == keyAuth {
+		if mem.zoneRec.record.RR().Data == keyAuth {
 			s.records[dnsName] = append(s.records[dnsName][:i], s.records[dnsName][i+1:]...)
 			return
 		}

solvers_test.go

@@ -15,11 +15,14 @@
 package certmagic
 
 import (
+	"context"
 	"net"
 	"strconv"
 	"testing"
+	"time"
 
 	"github.com/caddyserver/certmagic/internal/filedescriptor"
+	"github.com/libdns/libdns"
 	"github.com/mholt/acmez/v3/acme"
 )
 
@@ -159,6 +162,63 @@ func Test_challengeKey(t *testing.T) {
 	}
 }
 
+func TestDNSManagerCleanUpRecordPreservesProviderData(t *testing.T) {
+	provider := &providerDataDeleteProvider{t: t}
+	manager := DNSManager{
+		DNSProvider:        provider,
+		PropagationTimeout: time.Second,
+	}
+
+	err := manager.cleanUpRecord(context.Background(), zoneRecord{
+		zone: "example.com.",
+		record: libdns.TXT{
+			Name: "_acme-challenge",
+			Text: "token",
+			TTL:  time.Minute,
+			ProviderData: map[string]string{
+				"id": "123",
+			},
+		},
+	})
+	if err != nil {
+		t.Fatalf("cleanup failed: %v", err)
+	}
+	if !provider.deleted {
+		t.Fatal("expected DeleteRecords to be called")
+	}
+}
+
+type providerDataDeleteProvider struct {
+	t       *testing.T
+	deleted bool
+}
+
+func (p *providerDataDeleteProvider) AppendRecords(ctx context.Context, zone string, records []libdns.Record) ([]libdns.Record, error) {
+	return records, nil
+}
+
+func (p *providerDataDeleteProvider) DeleteRecords(ctx context.Context, zone string, records []libdns.Record) ([]libdns.Record, error) {
+	p.deleted = true
+	if zone != "example.com." {
+		p.t.Fatalf("expected zone example.com., got %q", zone)
+	}
+	if len(records) != 1 {
+		p.t.Fatalf("expected 1 record, got %d", len(records))
+	}
+	txt, ok := records[0].(libdns.TXT)
+	if !ok {
+		p.t.Fatalf("expected libdns.TXT with provider data, got %T", records[0])
+	}
+	pd, ok := txt.ProviderData.(map[string]string)
+	if !ok {
+		p.t.Fatalf("expected ProviderData map, got %T", txt.ProviderData)
+	}
+	if pd["id"] != "123" {
+		p.t.Fatalf("expected provider ID 123, got %q", pd["id"])
+	}
+	return records, nil
+}
+
 func TestGetACMEChallenge_IPv6Brackets(t *testing.T) {
 	// Store a challenge under a bare IPv6 identifier (as CertMagic does internally).
 	bare := "::1"