Provider Porkbun: Delete Default Parked DNS Entry for *.domain.tld (#774

)

* Provider Porkbun: Delete Default Parked DNS Entry for *.domain.tld

Description:

By default, Porkbun creates default ALIAS and CNAME domain records pointing to `pixie.porkbun.com` (Porkbun's parked domain website)

The current logic flow prior to this PR would look for an A or AAAA domain record, and if none exists, attempt to delete the ALIAS record for any subdomain.
This updates the logic flow to only look for a conflicting ALIAS record for the top level `domain.tld`, and a conflicting CNAME record for the `*.domain.tld`. Additionally, we verify that the content of this record matches `pixie.porkbun.com` and we only delete for the expected default values.
If the value does not match the expected `pixie.porkbun.com` we produce more helpful error messages.

Test-Plan:

Created a new domain.tld on Porkbun
Verified the default records were created:
`ALIAS domain.tld -> pixie.porkbun.com`
`CNAME *.domain.tld -> pixie.porkbun.com`
Started DDNS-Updater
Verified that both domain records were successfully deleted and updated

Reset the ALIAS domain record to point to `not-pixie.porkbun.com`
Reset the CNAME domain record to point to `not-pixie.porkbun.com`
Started DDNS-Updater
Verified that both domain records failed with the expected conflicting record error message.

![screenshot_2024-08-17-0210 20](https://github.com/user-attachments/assets/eb567401-ad4b-454d-a7aa-70ab1db1e3e9)

- add `deleteDefaultConflictingRecordsIfNeeded` method
- handle non conflicting errors from `deleteSingleMatchingRecord`
- simplify comments by linking to documentation
- improve error wrappings

---------

Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>

docs/porkbun.md

@@ -41,5 +41,12 @@
 ## Record creation
 
 In case you don't have an A or AAAA record for your host and domain combination, it will be created by DDNS-Updater.
-However, to do so, the corresponding ALIAS record, that is automatically created by Porkbun, is automatically deleted to allow this.
+
+Porkbun creates default DNS entries for new domains, which can conflict with creating a root or wildcard A/AAAA record. Therefore, ddns-updater automatically removes any conflicting default record before creating records, as described in the table below:
+
+| Record type | Owner | Record value | Situation requiring a removal |
+| --- | --- | --- | --- |
+| `ALIAS` | `@` | pixie.porkbun.com | Creating A or AAAA record for the root domain **or** wildcard domain |
+| `CNAME` | `*` | pixie.porkbun.com | Creating A or AAAA record for the wildcard domain |
+
 More details is in [this comment by @everydaycombat](https://github.com/qdm12/ddns-updater/issues/546#issuecomment-1773960193).

internal/provider/constants/recordtypes.go

@@ -0,0 +1,8 @@
+package constants
+
+const (
+	A     = "A"
+	AAAA  = "AAAA"
+	CNAME = "CNAME"
+	ALIAS = "ALIAS"
+)

internal/provider/providers/porkbun/api.go

@@ -11,13 +11,23 @@ import (
 	"github.com/qdm12/ddns-updater/internal/provider/errors"
 )
 
+type dnsRecord struct {
+	ID       string `json:"id"`
+	Name     string `json:"name"`
+	Type     string `json:"type"`
+	Content  string `json:"content"`
+	TTL      string `json:"ttl"`
+	Priority string `json:"prio"`
+	Notes    string `json:"notes"`
+}
+
 // See https://porkbun.com/api/json/v3/documentation#DNS%20Retrieve%20Records%20by%20Domain,%20Subdomain%20and%20Type
-func (p *Provider) getRecordIDs(ctx context.Context, client *http.Client, recordType string) (
-	recordIDs []string, err error) {
+func (p *Provider) getRecords(ctx context.Context, client *http.Client, recordType, owner string) (
+	records []dnsRecord, err error) {
 	url := "https://porkbun.com/api/json/v3/dns/retrieveByNameType/" + p.domain + "/" + recordType + "/"
-	if p.owner != "@" {
+	if owner != "@" {
 		// Note Porkbun requires we send the unescaped '*' character.
-		url += p.owner
+		url += owner
 	}
 
 	postRecordsParams := struct {
@@ -29,28 +39,21 @@ func (p *Provider) getRecordIDs(ctx context.Context, client *http.Client, record
 	}
 
 	type jsonResponseData struct {
-		Records []struct {
-			ID string `json:"id"`
-		} `json:"records"`
+		Records []dnsRecord `json:"records"`
 	}
 	const decodeBody = true
 	responseData, err := httpPost[jsonResponseData](ctx, client, url, postRecordsParams, decodeBody)
 	if err != nil {
-		return nil, fmt.Errorf("for record type %s: %w",
-			recordType, err)
-	}
-
-	recordIDs = make([]string, len(responseData.Records))
-	for i := range responseData.Records {
-		recordIDs[i] = responseData.Records[i].ID
+		return nil, fmt.Errorf("for record type %s and record owner %s: %w",
+			recordType, owner, err)
 	}
 
-	return recordIDs, nil
+	return responseData.Records, nil
 }
 
 // See https://porkbun.com/api/json/v3/documentation#DNS%20Create%20Record
 func (p *Provider) createRecord(ctx context.Context, client *http.Client,
-	recordType, ipStr string) (err error) {
+	recordType, owner, ipStr string) (err error) {
 	url := "https://porkbun.com/api/json/v3/dns/create/" + p.domain
 	postRecordsParams := struct {
 		SecretAPIKey string `json:"secretapikey"`
@@ -64,22 +67,22 @@ func (p *Provider) createRecord(ctx context.Context, client *http.Client,
 		APIKey:       p.apiKey,
 		Content:      ipStr,
 		Type:         recordType,
-		Name:         p.owner,
+		Name:         owner,
 		TTL:          strconv.FormatUint(uint64(p.ttl), 10),
 	}
 	const decodeBody = false
 	_, err = httpPost[struct{}](ctx, client, url, postRecordsParams, decodeBody)
 	if err != nil {
-		return fmt.Errorf("for record type %s: %w",
-			recordType, err)
+		return fmt.Errorf("for record type %s and record owner %s: %w",
+			recordType, owner, err)
 	}
 
 	return nil
 }
 
 // See https://porkbun.com/api/json/v3/documentation#DNS%20Edit%20Record%20by%20Domain%20and%20ID
 func (p *Provider) updateRecord(ctx context.Context, client *http.Client,
-	recordType, ipStr, recordID string) (err error) {
+	recordType, owner, ipStr, recordID string) (err error) {
 	url := "https://porkbun.com/api/json/v3/dns/edit/" + p.domain + "/" + recordID
 	postRecordsParams := struct {
 		SecretAPIKey string `json:"secretapikey"`
@@ -94,24 +97,24 @@ func (p *Provider) updateRecord(ctx context.Context, client *http.Client,
 		Content:      ipStr,
 		Type:         recordType,
 		TTL:          strconv.FormatUint(uint64(p.ttl), 10),
-		Name:         p.owner,
+		Name:         owner,
 	}
 	const decodeBody = false
 	_, err = httpPost[struct{}](ctx, client, url, postRecordsParams, decodeBody)
 	if err != nil {
-		return fmt.Errorf("for record type %s and record id %s: %w",
-			recordType, recordID, err)
+		return fmt.Errorf("for record type %s, record owner %s and record id %s: %w",
+			recordType, owner, recordID, err)
 	}
 
 	return nil
 }
 
 // See https://porkbun.com/api/json/v3/documentation#DNS%20Delete%20Records%20by%20Domain,%20Subdomain%20and%20Type
-func (p *Provider) deleteAliasRecord(ctx context.Context, client *http.Client) (err error) {
-	url := "https://porkbun.com/api/json/v3/dns/deleteByNameType/" + p.domain + "/ALIAS/"
-	if p.owner != "@" {
+func (p *Provider) deleteRecord(ctx context.Context, client *http.Client, recordType, owner string) (err error) {
+	url := "https://porkbun.com/api/json/v3/dns/deleteByNameType/" + p.domain + "/" + recordType + "/"
+	if owner != "@" {
 		// Note Porkbun requires we send the unescaped '*' character.
-		url += p.owner
+		url += owner
 	}
 	postRecordsParams := struct {
 		SecretAPIKey string `json:"secretapikey"`
@@ -124,7 +127,8 @@ func (p *Provider) deleteAliasRecord(ctx context.Context, client *http.Client) (
 	const decodeBody = false
 	_, err = httpPost[struct{}](ctx, client, url, postRecordsParams, decodeBody)
 	if err != nil {
-		return err
+		return fmt.Errorf("for record type %s and record owner %s: %w",
+			recordType, owner, err)
 	}
 
 	return nil

internal/provider/providers/porkbun/provider.go

@@ -3,6 +3,7 @@ package porkbun
 import (
 	"context"
 	"encoding/json"
+	stderrors "errors"
 	"fmt"
 	"net/http"
 	"net/netip"
@@ -119,46 +120,85 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip netip.Add
 		recordType = constants.AAAA
 	}
 	ipStr := ip.String()
-	recordIDs, err := p.getRecordIDs(ctx, client, recordType)
+	records, err := p.getRecords(ctx, client, recordType, p.owner)
 	if err != nil {
 		return netip.Addr{}, fmt.Errorf("getting record IDs: %w", err)
 	}
 
-	if len(recordIDs) == 0 {
-		// ALIAS record needs to be deleted to allow creating an A record.
-		err = p.deleteALIASRecordIfNeeded(ctx, client)
+	if len(records) == 0 {
+		err = p.deleteDefaultConflictingRecordsIfNeeded(ctx, client)
 		if err != nil {
-			return netip.Addr{}, fmt.Errorf("deleting ALIAS record if needed: %w", err)
+			return netip.Addr{}, fmt.Errorf("deleting default conflicting records: %w", err)
 		}
 
-		err = p.createRecord(ctx, client, recordType, ipStr)
+		err = p.createRecord(ctx, client, recordType, p.owner, ipStr)
 		if err != nil {
 			return netip.Addr{}, fmt.Errorf("creating record: %w", err)
 		}
 		return ip, nil
 	}
 
-	for _, recordID := range recordIDs {
-		err = p.updateRecord(ctx, client, recordType, ipStr, recordID)
+	for _, record := range records {
+		err = p.updateRecord(ctx, client, recordType, p.owner, ipStr, record.ID)
 		if err != nil {
 			return netip.Addr{}, fmt.Errorf("updating record: %w", err)
 		}
 	}
-
 	return ip, nil
 }
 
-func (p *Provider) deleteALIASRecordIfNeeded(ctx context.Context, client *http.Client) (err error) {
-	aliasRecordIDs, err := p.getRecordIDs(ctx, client, "ALIAS")
+// deleteDefaultConflictingRecordsIfNeeded deletes any default records that would conflict with a new record,
+// see https://github.com/qdm12/ddns-updater/blob/master/docs/porkbun.md#record-creation
+func (p *Provider) deleteDefaultConflictingRecordsIfNeeded(ctx context.Context, client *http.Client) (err error) {
+	const porkbunParkedDomain = "pixie.porkbun.com"
+	switch p.owner {
+	case "@":
+		err = p.deleteSingleMatchingRecord(ctx, client, constants.ALIAS, "@", porkbunParkedDomain)
+		if err != nil {
+			return fmt.Errorf("deleting default ALIAS @ parked domain record: %w", err)
+		}
+		return nil
+	case "*":
+		err = p.deleteSingleMatchingRecord(ctx, client, constants.CNAME, "*", porkbunParkedDomain)
+		if err != nil {
+			return fmt.Errorf("deleting default CNAME * parked domain record: %w", err)
+		}
+
+		err = p.deleteSingleMatchingRecord(ctx, client, constants.ALIAS, "@", porkbunParkedDomain)
+		if err == nil || stderrors.Is(err, errors.ErrConflictingRecord) {
+			// allow conflict ALIAS records to be set to something besides the parked domain
+			return nil
+		}
+		return fmt.Errorf("deleting default ALIAS @ parked domain record: %w", err)
+	default:
+		return nil
+	}
+}
+
+// deleteSingleMatchingRecord deletes an eventually present record matching a specific record type if the content
+// matches the expected content value.
+// It returns an error if multiple records are found or if one record is found with an unexpected value.
+func (p *Provider) deleteSingleMatchingRecord(ctx context.Context, client *http.Client,
+	recordType, owner, expectedContent string) (err error) {
+	records, err := p.getRecords(ctx, client, recordType, owner)
 	if err != nil {
-		return fmt.Errorf("getting ALIAS record IDs: %w", err)
-	} else if len(aliasRecordIDs) == 0 {
+		return fmt.Errorf("getting records: %w", err)
+	}
+
+	switch {
+	case len(records) == 0:
 		return nil
+	case len(records) > 1:
+		return fmt.Errorf("%w: %d %s records are already set", errors.ErrConflictingRecord, len(records), recordType)
+	case records[0].Content != expectedContent:
+		return fmt.Errorf("%w: %s record has content %q mismatching expected content %q",
+			errors.ErrConflictingRecord, recordType, records[0].Content, expectedContent)
 	}
 
-	err = p.deleteAliasRecord(ctx, client)
+	// Single record with content matching expected content.
+	err = p.deleteRecord(ctx, client, recordType, owner)
 	if err != nil {
-		return fmt.Errorf("deleting ALIAS record: %w", err)
+		return fmt.Errorf("deleting record: %w", err)
 	}
 	return nil
 }