From 4254600bcf346d47b1b1479c6e5fed581399f5dc Mon Sep 17 00:00:00 2001 From: Benjamin Temple Date: Mon, 7 Oct 2024 21:46:12 -0700 Subject: [PATCH] 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 --- docs/porkbun.md | 9 ++- internal/provider/constants/ip.go | 6 -- internal/provider/constants/recordtypes.go | 8 +++ internal/provider/providers/porkbun/api.go | 60 ++++++++-------- .../provider/providers/porkbun/provider.go | 70 +++++++++++++++---- 5 files changed, 103 insertions(+), 50 deletions(-) delete mode 100644 internal/provider/constants/ip.go create mode 100644 internal/provider/constants/recordtypes.go diff --git a/docs/porkbun.md b/docs/porkbun.md index a9393b1c0..86dfc1053 100644 --- a/docs/porkbun.md +++ b/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). diff --git a/internal/provider/constants/ip.go b/internal/provider/constants/ip.go deleted file mode 100644 index fafa39576..000000000 --- a/internal/provider/constants/ip.go +++ /dev/null @@ -1,6 +0,0 @@ -package constants - -const ( - A = "A" - AAAA = "AAAA" -) diff --git a/internal/provider/constants/recordtypes.go b/internal/provider/constants/recordtypes.go new file mode 100644 index 000000000..1f2d6d84d --- /dev/null +++ b/internal/provider/constants/recordtypes.go @@ -0,0 +1,8 @@ +package constants + +const ( + A = "A" + AAAA = "AAAA" + CNAME = "CNAME" + ALIAS = "ALIAS" +) diff --git a/internal/provider/providers/porkbun/api.go b/internal/provider/providers/porkbun/api.go index e83eda265..60c53dc6b 100644 --- a/internal/provider/providers/porkbun/api.go +++ b/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,14 +67,14 @@ 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 @@ -79,7 +82,7 @@ func (p *Provider) createRecord(ctx context.Context, client *http.Client, // 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 diff --git a/internal/provider/providers/porkbun/provider.go b/internal/provider/providers/porkbun/provider.go index 5424c20b5..ac8823695 100644 --- a/internal/provider/providers/porkbun/provider.go +++ b/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 }