From 8a11f1955feb77236383c90b482e5a3569eff8b7 Mon Sep 17 00:00:00 2001 From: Benjamin Temple Date: Tue, 23 Jul 2024 23:47:01 -0700 Subject: [PATCH] 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 | 19 +++++- internal/provider/constants/ip.go | 6 +- internal/provider/providers/porkbun/api.go | 29 +++++---- .../provider/providers/porkbun/provider.go | 61 ++++++++++++++----- 4 files changed, 84 insertions(+), 31 deletions(-) diff --git a/docs/porkbun.md b/docs/porkbun.md index bfc7118fa..e80bcae96 100644 --- a/docs/porkbun.md +++ b/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. diff --git a/internal/provider/constants/ip.go b/internal/provider/constants/ip.go index fafa39576..1f2d6d84d 100644 --- a/internal/provider/constants/ip.go +++ b/internal/provider/constants/ip.go @@ -1,6 +1,8 @@ package constants const ( - A = "A" - AAAA = "AAAA" + 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 26815ee15..c16b325d1 100644 --- a/internal/provider/providers/porkbun/api.go +++ b/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,9 +71,7 @@ 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) @@ -70,11 +79,7 @@ func (p *Provider) getRecordIDs(ctx context.Context, client *http.Client, record 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"` diff --git a/internal/provider/providers/porkbun/provider.go b/internal/provider/providers/porkbun/provider.go index 5424c20b5..b2bd50d32 100644 --- a/internal/provider/providers/porkbun/provider.go +++ b/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 }