Skip to content

fix(dns): match record content in upsertRecord to prevent overwriting existing records - #5382

Merged
narcisonunez merged 7 commits into
Dokploy:canaryfrom
imrja8:fix/dns-upsert-preserve-existing-records
Sep 8, 2026
Merged

narcisonunez merged 7 commits into
Dokploy:canaryfrom
imrja8:fix/dns-upsert-preserve-existing-records

Conversation

@imrja8

@imrja8 imrja8 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

What is this PR about?

Previously, upsertRecord matched only by type and name, silently overwriting any existing record of the same type/name with different content. This caused data loss when users had multiple records of the same type (e.g. multiple TXT records for SPF + site verification).

This PR updates upsertRecord to also check the record content/target before deciding to update. If the content differs, a new record is created alongside the existing one instead of replacing it. This fix is applied globally to Cloudflare, Porkbun, Infomaniak, and OVH providers. The respective unit tests have been updated to test this new behavior.

Checklist

Before submitting this PR, please make sure that:

Issues related (if applicable)

Follow up to the issue found in PR #5381

Screenshots (if applicable)

NA

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable new defects or outstanding blocking findings remain.

Summary

  • Normalizes provider-specific DNS values before matching.
  • Handles Cloudflare structured SRV data and MX priority values.
  • Preserves Porkbun MX/SRV priorities during listing, matching, and updates.
  • Fetches OVH record details to identify the matching target.
  • Adds provider tests for matching and differing content.

… existing records

Previously, upsertRecord matched only by type and name, silently
overwriting any existing record of the same type/name with different
content. This caused data loss when users had multiple records of the
same type (e.g. multiple TXT records for SPF + site verification).

Now upsertRecord also checks the record content/target before deciding
to update. If the content differs, a new record is created alongside
the existing one instead of replacing it.

Affected providers: Cloudflare, Porkbun, Infomaniak, OVH.
Tests updated accordingly.
@imrja8
imrja8 requested a review from Siumauricio as a code owner September 8, 2026 11:37
Comment thread packages/server/src/utils/dns/cloudflare.ts Outdated
Comment thread packages/server/src/utils/dns/infomaniak.ts Outdated
@imrja8
imrja8 marked this pull request as draft September 8, 2026 11:41
@imrja8
imrja8 marked this pull request as ready for review September 8, 2026 11:46
Comment thread packages/server/src/utils/dns/cloudflare.ts Outdated
@imrja8

imrja8 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread packages/server/src/utils/dns/porkbun.ts Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@imrja8

imrja8 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptileai re review

@imrja8

imrja8 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@narcisonunez Please, Review this PR.

@narcisonunez

Copy link
Copy Markdown
Collaborator

@imrja8 Please, fix the conflicts and should be ready to approve

@imrja8

imrja8 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor Author

@imrja8 Please, fix the conflicts and should be ready to approve

@narcisonunez Ok, gimme 05-10 mins.

…eserve-existing-records

# Conflicts:
#	packages/server/src/utils/dns/infomaniak.ts
@imrja8

imrja8 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptileai re review

@greptileai re re review

@imrja8

imrja8 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@imrja8 Please, fix the conflicts and should be ready to approve

@narcisonunez Ok, gimme 05-10 mins.

@narcisonunez Conflict resolved, you can have alook..

@narcisonunez
narcisonunez merged commit aaff71b into Dokploy:canary Sep 8, 2026
5 checks passed
@imrja8
imrja8 deleted the fix/dns-upsert-preserve-existing-records branch September 8, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants