Skip to content

Commit

Permalink
Provider Porkbun: Don't attempt to update a record if it already matc…
Browse files Browse the repository at this point in the history
…hes the expected ipStr

Description:
Porkbun will return a 400 error if you attempt to update the IP for a DNS record that is already set.
This updates the API to return the full DNS record from Porkbun's API, and then to not attempt to update the record if it's already correct. Ultimately this may have been a fix for what ultimately was the root cause of the * domain being incorrect, but especially when people are setting up their DDNS Updater for the first time, this can help reduce unnecessary and unwanted errors in the interface.
Alternatively, we could try to update and simply return a more helpful error message that likely the Porkbun DNS value is already correct.

Test-Plan:
Set `domain.tld` to 127.0.0.1
Started DDNS-Updater
DDNS-Updater updated the domain successfully, at this time the domain
has not propogated the new valid IP update
Stop DDNS-Updater
Deleted the updates.json file
Re-launched DDNS-Updater
DDNS-Updater gets into a bad state and 400's indefinitely until
restarted after the valid domain has propogated

Re-performed the above tests with the fixed code
DDNS-Updater updates successfully

Remove unnecessary newline

Revert some changes to match PR comments and reduce delta
  • Loading branch information
bentemple committed Jul 29, 2024
1 parent 1cd57d6 commit f678492
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 15 deletions.
24 changes: 14 additions & 10 deletions internal/provider/providers/porkbun/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,19 @@ 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 string) (
records []dnsRecord, err error) {
u := url.URL{
Scheme: "https",
Host: "porkbun.com",
Expand Down Expand Up @@ -56,21 +66,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
Expand Down
16 changes: 11 additions & 5 deletions internal/provider/providers/porkbun/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ 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 {
if len(records) == 0 {
// ALIAS record needs to be deleted to allow creating an A record.
err = p.deleteALIASRecordIfNeeded(ctx, client)
if err != nil {
Expand All @@ -138,8 +138,14 @@ 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 {
if record.Content == ipStr {
// Skip update. Porkbun already has the IP correctly set.
// Porkbun will respond with a 400 error if we attempt to update a record to a value that is already set.
continue
}

err = p.updateRecord(ctx, client, recordType, ipStr, record.ID)
if err != nil {
return netip.Addr{}, fmt.Errorf("updating record: %w", err)
}
Expand All @@ -149,7 +155,7 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip netip.Add
}

func (p *Provider) deleteALIASRecordIfNeeded(ctx context.Context, client *http.Client) (err error) {
aliasRecordIDs, err := p.getRecordIDs(ctx, client, "ALIAS")
aliasRecordIDs, err := p.getRecords(ctx, client, "ALIAS")
if err != nil {
return fmt.Errorf("getting ALIAS record IDs: %w", err)
} else if len(aliasRecordIDs) == 0 {
Expand Down

0 comments on commit f678492

Please sign in to comment.