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.

Note: Any other domain name we simply attempt to create a record if no record exists, which will return a 400 error with no helpful error reason if there is a conflicting record of another type. We may want to consider checking for any conflicting records to produce a more helpful error for the user, but that change is well outside of the scope of this PR, and should likely be done in a more general way and apply to all DNS providers, rather than porkbun exclusively.

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)

docs/porkbun.md

@@ -41,5 +41,22 @@
 ## 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.
+
+## Conflicting default records
+
+Porkbun sets the following default DNS entries for new domains:
+
+- ALIAS `domain.tld` -> `pixie.porkbun.com`
+- CNAME `*.domain.tld` -> `pixie.porkbun.com`
+
+`pixie.porkbun.com` is porkbun's Parked domain website.
+
+[Parked domain screenshot](https://github.com/user-attachments/assets/d73c4fbd-f6a9-48c9-9dcb-01818541ceb1)
+
+In-order to create an A or AAAA DNS record, when setting the `domain.tld` or `*.domain.tld`, if a respective ALIAS or CNAME record is already set to `pixie.porkbun.com`, the respective default record will be automatically deleted by DDNS-Updater and a new A or AAAA record will be created.
 More details is in [this comment by @everydaycombat](https://github.com/qdm12/ddns-updater/issues/546#issuecomment-1773960193).
+
+>NOTE: DDNS-Updater will only attempt to delete the default records for `domain.tld` and `*.domain.tld`.
+>
+>If there are any other conflicting records set for other domains, DDNS-Updater will not check for them, and it will simply attempt to create an `A` or `AAAA` record.
+>Porkbun's API error messages are extremely unhelpful (`400: something went wrong`), so it is recommended to check the domain record from the Porkbun Web-UI in the event of unexpected 400 API errors to ensure there are no conflicting records present.

internal/provider/constants/ip.go

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

internal/provider/providers/porkbun/api.go

@@ -12,9 +12,20 @@ import (
 	"github.com/qdm12/ddns-updater/internal/provider/errors"
 )
 
+// Leaving unused values commented out to improve resilience to API changes.
+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 string) (
+	records []dnsRecord, err error) {
 	u := url.URL{
 		Scheme: "https",
 		Host:   "porkbun.com",
@@ -60,21 +71,15 @@ func (p *Provider) getRecordIDs(ctx context.Context, client *http.Client, record
 	}
 
 	var responseData struct {
-		Records []struct {
-			ID string `json:"id"`
-		} `json:"records"`
+		Records []dnsRecord `json:"records"`
 	}
 	decoder := json.NewDecoder(response.Body)
 	err = decoder.Decode(&responseData)
 	if err != nil {
 		return nil, fmt.Errorf("json decoding response body: %w", err)
 	}
 
-	for _, record := range responseData.Records {
-		recordIDs = append(recordIDs, record.ID)
-	}
-
-	return recordIDs, nil
+	return responseData.Records, nil
 }
 
 // See https://porkbun.com/api/json/v3/documentation#DNS%20Create%20Record
@@ -176,12 +181,12 @@ func (p *Provider) updateRecord(ctx context.Context, client *http.Client,
 }
 
 // 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) {
+func (p *Provider) deleteRecord(ctx context.Context, client *http.Client, recordType string) (err error) {
 	u := url.URL{
 		Scheme: "https",
 		Host:   "porkbun.com",
 		// owner is added below after string-ing the URL
-		Path: "/api/json/v3/dns/deleteByNameType/" + p.domain + "/ALIAS/",
+		Path: "/api/json/v3/dns/deleteByNameType/" + p.domain + "/" + recordType + "/",
 	}
 	postRecordsParams := struct {
 		SecretAPIKey string `json:"secretapikey"`

internal/provider/providers/porkbun/provider.go

@@ -119,16 +119,31 @@ 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)
 	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 err != nil {
-			return netip.Addr{}, fmt.Errorf("deleting ALIAS record if needed: %w", err)
+	if len(records) == 0 {
+		// For new domains, Porkbun Creates 2 Default DNS Records which point to their "parked" domain page:
+		// ALIAS domain.tld -> pixie.porkbun.com
+		// CNAME *.domain.tld -> pixie.porkbun.com
+		// ALIAS and CNAME records conflict with A and AAAA records, and attempting to create an A or AAAA record
+		// will return a 400 error if they aren't first deleted.
+		porkbunParkedDomain := "pixie.porkbun.com"
+		switch {
+		case p.owner == "@":
+			// Delete ALIAS domain.tld -> pixie.porkbun.com record
+			err = p.deleteMatchingRecord(ctx, client, constants.ALIAS, porkbunParkedDomain)
+			if err != nil {
+				return netip.Addr{}, err
+			}
+		case p.owner == "*":
+			// Delete CNAME *.domain.tld -> pixie.porkbun.com record
+			err = p.deleteMatchingRecord(ctx, client, constants.CNAME, porkbunParkedDomain)
+			if err != nil {
+				return netip.Addr{}, err
+			}
 		}
 
 		err = p.createRecord(ctx, client, recordType, ipStr)
@@ -138,8 +153,8 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip netip.Add
 		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, ipStr, record.ID)
 		if err != nil {
 			return netip.Addr{}, fmt.Errorf("updating record: %w", err)
 		}
@@ -148,17 +163,31 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip netip.Add
 	return ip, nil
 }
 
-func (p *Provider) deleteALIASRecordIfNeeded(ctx context.Context, client *http.Client) (err error) {
-	aliasRecordIDs, err := p.getRecordIDs(ctx, client, "ALIAS")
+// Deletes the matching record for a specific recordType iff the content matches the expected content value.
+// Returns an error if there is a matching record but an unexpected content value.
+// If there is no matching record, there is nothing to do.
+func (p *Provider) deleteMatchingRecord(
+	ctx context.Context,
+	client *http.Client,
+	recordType string,
+	expectedContent string,
+) error {
+	records, err := p.getRecords(ctx, client, recordType)
 	if err != nil {
-		return fmt.Errorf("getting ALIAS record IDs: %w", err)
-	} else if len(aliasRecordIDs) == 0 {
-		return nil
+		return fmt.Errorf("getting %s records: %w", recordType, err)
 	}
 
-	err = p.deleteAliasRecord(ctx, client)
-	if err != nil {
-		return fmt.Errorf("deleting ALIAS record: %w", err)
+	if len(records) == 1 && records[0].Content == expectedContent {
+		err = p.deleteRecord(ctx, client, recordType)
+		if err != nil {
+			return fmt.Errorf("deleting %s record: %w", recordType, err)
+		}
+		return nil
+	} else if len(records) >= 1 {
+		// We have 1 or more matching records, but the expectedContent didn't match. Throw a conflicting record error.
+		return fmt.Errorf("deleting %s record: %w", recordType, errors.ErrConflictingRecord)
 	}
+
+	// No record found for recordType, nothing to do.
 	return nil
 }