From 0a771772e3f68cba98140ea9ac53ff62a662d9bb Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Sun, 8 Mar 2026 16:14:38 -0600 Subject: [PATCH] Handle duplicate DNS records with upsert semantics When Vultr's API returns "Duplicate records are not allowed" on DomainRecord.Create, find the existing record by name+type and update it instead of failing. This prevents ACME certificate issuance from breaking when stale _acme-challenge TXT records survive a Caddy restart. Also fix record lookup in removeDNSRecord and updateDNSRecord to match by name+type+data instead of name-only or data-only, avoiding deletion of the wrong record when multiple records share the same name (common with _acme-challenge across LE and ZeroSSL issuers). Use comma-ok type assertions throughout to prevent panics if the record type invariant is ever broken. Extract the Vultr error string to a named constant. Document the mutex contract. --- client.go | 109 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 98 insertions(+), 11 deletions(-) diff --git a/client.go b/client.go index 906c7df..6e9f51e 100644 --- a/client.go +++ b/client.go @@ -3,6 +3,7 @@ package vultr import ( "context" "fmt" + "strings" "sync" "golang.org/x/oauth2" @@ -11,8 +12,15 @@ import ( "github.com/vultr/govultr/v3" ) +// vultrDuplicateErr is matched against Vultr API response text. +// May need updating if Vultr changes their error message wording. +const vultrDuplicateErr = "Duplicate records" + type Client struct { vultr *govultr.Client + // mutex serializes write operations (add/remove/update). + // getDNSEntries does NOT acquire this lock because it is called + // from within write methods that already hold it. mutex sync.Mutex } @@ -64,7 +72,39 @@ func (p *Provider) addDNSRecord(ctx context.Context, domain string, r libdns.Rec rec, _, err := p.client.vultr.DomainRecord.Create(ctx, domain, &domainRecordReq) if err != nil { - return r, err + if !strings.Contains(err.Error(), vultrDuplicateErr) { + return r, err + } + + // Duplicate record exists — find it and update with the new data. + // This handles stale ACME challenge TXT records left from previous runs. + records, lookupErr := p.getDNSEntries(ctx, domain) + if lookupErr != nil { + return r, fmt.Errorf("duplicate record and could not look up existing: %w", lookupErr) + } + + rr := r.RR() + var existingID string + for _, existing := range records { + exRR := existing.RR() + if exRR.Name == rr.Name && exRR.Type == rr.Type { + if vr, ok := existing.(VultrRecord); ok { + existingID = vr.ID + break + } + } + } + + if existingID == "" { + return r, fmt.Errorf("duplicate record reported but not found in zone: %w", err) + } + + updateErr := p.client.vultr.DomainRecord.Update(ctx, domain, existingID, &domainRecordReq) + if updateErr != nil { + return r, fmt.Errorf("failed to update duplicate record %s: %w", existingID, updateErr) + } + + return fromLibdnsRecord(r, existingID), nil } record := fromLibdnsRecord(r, rec.ID) @@ -81,16 +121,40 @@ func (p *Provider) removeDNSRecord(ctx context.Context, domain string, record li recordId, err := getRecordId(record) if err != nil { // try to get the ID from API if we don't have it - records, err := p.getDNSEntries(ctx, domain) - if err != nil { - return record, fmt.Errorf("could not get record ID from API") + records, lookupErr := p.getDNSEntries(ctx, domain) + if lookupErr != nil { + return record, fmt.Errorf("could not get record ID from API: %w", lookupErr) } + rr := record.RR() for _, rec := range records { - if rec.RR().Name == record.RR().Name { - recordId = rec.(VultrRecord).ID + recRR := rec.RR() + // Match by name, type, AND data to avoid deleting the wrong record + // when multiple records share the same name (e.g. _acme-challenge TXT). + if recRR.Name == rr.Name && recRR.Type == rr.Type && recRR.Data == rr.Data { + if vr, ok := rec.(VultrRecord); ok { + recordId = vr.ID + break + } } } + + // Fall back to name+type match if exact data match wasn't found + if recordId == "" { + for _, rec := range records { + recRR := rec.RR() + if recRR.Name == rr.Name && recRR.Type == rr.Type { + if vr, ok := rec.(VultrRecord); ok { + recordId = vr.ID + break + } + } + } + } + + if recordId == "" { + return record, fmt.Errorf("no matching record found for %s %s in zone %s", rr.Type, rr.Name, domain) + } } err = p.client.vultr.DomainRecord.Delete(ctx, domain, recordId) @@ -110,16 +174,39 @@ func (p *Provider) updateDNSRecord(ctx context.Context, domain string, record li recordId, err := getRecordId(record) if err != nil { // try to get the ID from API if we don't have it - records, err := p.getDNSEntries(ctx, domain) - if err != nil { - return record, fmt.Errorf("could not get record ID from API") + records, lookupErr := p.getDNSEntries(ctx, domain) + if lookupErr != nil { + return record, fmt.Errorf("could not get record ID from API: %w", lookupErr) } + rr := record.RR() for _, rec := range records { - if rec.RR().Data == record.RR().Data { - recordId = rec.(VultrRecord).ID + recRR := rec.RR() + // Match by name+type+data first for precision + if recRR.Name == rr.Name && recRR.Type == rr.Type && recRR.Data == rr.Data { + if vr, ok := rec.(VultrRecord); ok { + recordId = vr.ID + break + } } } + + // Fall back to name+type if exact data match not found + if recordId == "" { + for _, rec := range records { + recRR := rec.RR() + if recRR.Name == rr.Name && recRR.Type == rr.Type { + if vr, ok := rec.(VultrRecord); ok { + recordId = vr.ID + break + } + } + } + } + + if recordId == "" { + return record, fmt.Errorf("no matching record found for %s %s in zone %s", rr.Type, rr.Name, domain) + } } domainRecordReq := toDomainRecordReq(record)