Merge pull request #5382 from imrja8/fix/dns-upsert-preserve-existing-records

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

apps/dokploy/__test__/dns/cloudflare.test.ts

@@ -323,9 +323,11 @@ describe("cloudflareClient.upsertRecord", () => {
 		expect(createInit.method).toBe("POST");
 	});
 
-	it("updates the existing record instead of creating a duplicate", async () => {
+	it("updates the existing record when content matches", async () => {
 		mockFetch
-			.mockResolvedValueOnce(cfSuccess([{ id: "existing-1" }]))
+			.mockResolvedValueOnce(
+				cfSuccess([{ id: "existing-1", type: "A", content: "5.6.7.8" }]),
+			)
 			.mockResolvedValueOnce(cfSuccess({ id: "existing-1" }));
 
 		const result = await cloudflareClient.upsertRecord(config, {
@@ -344,6 +346,25 @@ describe("cloudflareClient.upsertRecord", () => {
 		expect(updateInit.method).toBe("PUT");
 	});
 
+	it("creates a new record when content differs from existing", async () => {
+		mockFetch
+			.mockResolvedValueOnce(
+				cfSuccess([{ id: "existing-1", type: "A", content: "1.1.1.1" }]),
+			)
+			.mockResolvedValueOnce(cfSuccess({ id: "new-2" }));
+
+		const result = await cloudflareClient.upsertRecord(config, {
+			zoneId: "zone-1",
+			type: "A",
+			name: "app.example.com",
+			content: "5.6.7.8",
+		});
+
+		expect(result).toEqual({ id: "new-2" });
+		const [, createInit] = mockFetch.mock.calls[1] as [string, RequestInit];
+		expect(createInit.method).toBe("POST");
+	});
+
 	it("defaults ttl to 1 (automatic) when not provided", async () => {
 		mockFetch
 			.mockResolvedValueOnce(cfSuccess([]))

apps/dokploy/__test__/dns/infomaniak.test.ts

@@ -203,11 +203,11 @@ describe("infomaniakClient.upsertRecord", () => {
 		});
 	});
 
-	it("updates the existing record instead of creating a duplicate", async () => {
+	it("updates the existing record when content matches", async () => {
 		mockFetch
 			.mockResolvedValueOnce(
 				ikSuccess([
-					{ id: 7, type: "A", source: "app", target: "1.1.1.1", ttl: 300 },
+					{ id: 7, type: "A", source: "app", target: "1.2.3.4", ttl: 300 },
 				]),
 			)
 			.mockResolvedValueOnce(ikSuccess({ id: 7 }));
@@ -228,6 +228,28 @@ describe("infomaniakClient.upsertRecord", () => {
 		expect(lastBody().ttl).toBe(300);
 	});
 
+	it("creates a new record when content differs from existing", async () => {
+		mockFetch
+			.mockResolvedValueOnce(
+				ikSuccess([
+					{ id: 7, type: "A", source: "app", target: "1.1.1.1", ttl: 300 },
+				]),
+			)
+			.mockResolvedValueOnce(ikSuccess({ id: 50 }));
+
+		const result = await infomaniakClient.upsertRecord(config, {
+			zoneId: "example.com",
+			type: "A",
+			name: "app.example.com",
+			content: "1.2.3.4",
+		});
+
+		expect(result).toEqual({ id: "50" });
+		const [url, init] = lastCall();
+		expect(url).toBe("https://api.infomaniak.com/2/zones/example.com/records");
+		expect(init.method).toBe("POST");
+	});
+
 	it("writes a root dot as the source for an apex record and strips the trailing dot", async () => {
 		mockFetch
 			.mockResolvedValueOnce(ikSuccess([]))
@@ -249,7 +271,7 @@ describe("infomaniakClient.upsertRecord", () => {
 			mockFetch
 				.mockResolvedValueOnce(
 					ikSuccess([
-						{ id: 8, type: "A", source, target: "1.1.1.1", ttl: 3600 },
+						{ id: 8, type: "A", source, target: "1.2.3.4", ttl: 3600 },
 					]),
 				)
 				.mockResolvedValueOnce(ikSuccess({ id: 8 }));

apps/dokploy/__test__/dns/ovh.test.ts

@@ -261,9 +261,21 @@ describe("ovhClient.upsertRecord", () => {
 		expect(calls[2]?.[1].method).toBe("POST");
 	});
 
-	it("updates the existing record instead of creating a duplicate", async () => {
+	it("updates the existing record when content matches", async () => {
 		const cfg = freshConfig();
-		mockApi(ovhSuccess([4]), ovhSuccess(null), ovhSuccess(null));
+		mockApi(
+			ovhSuccess([4]),
+			ovhSuccess({
+				id: 4,
+				zone: "example.com",
+				fieldType: "A",
+				subDomain: "app",
+				target: "1.2.3.4",
+				ttl: 60,
+			}),
+			ovhSuccess(null),
+			ovhSuccess(null),
+		);
 
 		const result = await ovhClient.upsertRecord(cfg, {
 			zoneId: "example.com",
@@ -274,9 +286,38 @@ describe("ovhClient.upsertRecord", () => {
 
 		expect(result).toEqual({ id: "4" });
 		const calls = apiCalls();
-		expect(calls[1]?.[0]).toContain("/domain/zone/example.com/record/4");
-		expect(calls[1]?.[1].method).toBe("PUT");
-		expect(calls[2]?.[0]).toContain("/refresh");
+		expect(calls[2]?.[0]).toContain("/domain/zone/example.com/record/4");
+		expect(calls[2]?.[1].method).toBe("PUT");
+		expect(calls[3]?.[0]).toContain("/refresh");
+	});
+
+	it("creates a new record when content differs from existing", async () => {
+		const cfg = freshConfig();
+		mockApi(
+			ovhSuccess([4]),
+			ovhSuccess({
+				id: 4,
+				zone: "example.com",
+				fieldType: "A",
+				subDomain: "app",
+				target: "1.1.1.1",
+				ttl: 60,
+			}),
+			ovhSuccess({ id: 10 }),
+			ovhSuccess(null),
+		);
+
+		const result = await ovhClient.upsertRecord(cfg, {
+			zoneId: "example.com",
+			type: "A",
+			name: "app.example.com",
+			content: "5.6.7.8",
+		});
+
+		expect(result).toEqual({ id: "10" });
+		const calls = apiCalls();
+		expect(calls[2]?.[1].method).toBe("POST");
+		expect(calls[3]?.[0]).toContain("/refresh");
 	});
 
 	it("omits the ttl so OVH applies the zone default", async () => {

apps/dokploy/__test__/dns/porkbun.test.ts

@@ -121,9 +121,11 @@ describe("porkbunClient.upsertRecord", () => {
 		expect(lookupUrl).toContain("/dns/retrieveByNameType/example.com/A/");
 	});
 
-	it("edits the existing record instead of creating a duplicate", async () => {
+	it("edits the existing record when content matches", async () => {
 		mockFetch
-			.mockResolvedValueOnce(pbSuccess({ records: [{ id: "existing-1" }] }))
+			.mockResolvedValueOnce(
+				pbSuccess({ records: [{ id: "existing-1", content: "5.6.7.8" }] }),
+			)
 			.mockResolvedValueOnce(pbSuccess({}));
 
 		const result = await porkbunClient.upsertRecord(config, {
@@ -138,6 +140,30 @@ describe("porkbunClient.upsertRecord", () => {
 		expect(editUrl).toContain("/dns/edit/example.com/existing-1");
 	});
 
+	it("creates a new record when content differs from existing", async () => {
+		mockFetch
+			.mockResolvedValueOnce(
+				pbSuccess({ records: [{ id: "existing-1", content: "1.1.1.1" }] }),
+			)
+			.mockResolvedValueOnce(pbSuccess({ id: "new-2" }));
+
+		const result = await porkbunClient.upsertRecord(config, {
+			zoneId: "example.com",
+			type: "A",
+			name: "app.example.com",
+			content: "5.6.7.8",
+		});
+
+		expect(result).toEqual({ id: "new-2" });
+		const [createUrl, createInit] = mockFetch.mock.calls[1] as [
+			string,
+			RequestInit,
+		];
+		expect(createUrl).toContain("/dns/create/example.com");
+		const body = JSON.parse(createInit.body as string);
+		expect(body).toMatchObject({ name: "app", type: "A", content: "5.6.7.8" });
+	});
+
 	it("defaults ttl to 600 when not provided", async () => {
 		mockFetch
 			.mockResolvedValueOnce(pbSuccess({ records: [] }))

packages/server/src/utils/dns/cloudflare.ts

@@ -168,12 +168,34 @@ export const cloudflareClient: DnsClient<CloudflareConfig> = {
 			ttl: record.ttl ?? 1,
 		};
 
-		const existing = await cfFetch<{ id: string }[]>(
+		const existing = await cfFetch<
+			{
+				id: string;
+				type: string;
+				content: string;
+				priority?: number;
+				data?: Record<string, unknown>;
+			}[]
+		>(
 			config,
 			`/zones/${record.zoneId}/dns_records?type=${record.type}&name=${encodeURIComponent(record.name)}`,
 		);
 
-		const existingRecord = existing[0];
+		const built = buildValue(record);
+
+		const existingRecord = existing.find((r) => {
+			if (built.data) {
+				return Object.entries(built.data).every(
+					([k, v]) => r.data && r.data[k] === v,
+				);
+			}
+			const normalizedRecord = {
+				type: record.type,
+				content: built.content ?? record.content.trim(),
+				priority: built.priority,
+			};
+			return inlinePriority(r) === inlinePriority(normalizedRecord);
+		});
 		if (existingRecord) {
 			const updated = await cfFetch<{ id: string }>(
 				config,

packages/server/src/utils/dns/infomaniak.ts

@@ -158,6 +158,7 @@ const findRecord = async (
 	zoneId: string,
 	type: string,
 	source: string,
+	expectedContent: string,
 ) => {
 	const query = new URLSearchParams({
 		"filter[source]": source,
@@ -169,7 +170,9 @@ const findRecord = async (
 	);
 	return candidates.find(
 		(candidate) =>
-			candidate.type === type && normalizeSource(candidate.source) === source,
+			candidate.type === type &&
+			normalizeSource(candidate.source) === source &&
+			unquoteTarget(candidate.target) === expectedContent,
 	);
 };
 
@@ -196,7 +199,16 @@ export const infomaniakClient: DnsClient<InfomaniakConfig> = {
 
 	async upsertRecord(config, record) {
 		const source = toSource(record.name, record.zoneId);
-		const match = await findRecord(config, record.zoneId, record.type, source);
+		const expectedContent = unquoteTarget(
+			quoteTarget(record.type, record.content),
+		);
+		const match = await findRecord(
+			config,
+			record.zoneId,
+			record.type,
+			source,
+			expectedContent,
+		);
 
 		const body = JSON.stringify(recordPayload(record, record.zoneId));
 		const zone = encodeURIComponent(record.zoneId);

packages/server/src/utils/dns/ovh.ts

@@ -285,7 +285,18 @@ export const ovhClient: DnsClient<OvhConfig> = {
 			)}&subDomain=${encodeURIComponent(subDomain)}`,
 		);
 
-		const existingId = existing[0];
+		let existingId: number | undefined;
+		for (const id of existing) {
+			const candidate = await ovhFetch<OvhRecord>(
+				config,
+				`/domain/zone/${zone}/record/${id}`,
+			);
+			if (candidate.target === record.content) {
+				existingId = id;
+				break;
+			}
+		}
+
 		if (existingId !== undefined) {
 			await ovhFetch(config, `/domain/zone/${zone}/record/${existingId}`, {
 				method: "PUT",

packages/server/src/utils/dns/porkbun.ts

@@ -56,6 +56,26 @@ interface PorkbunRecord {
 	notes: string;
 }
 
+const inlinePriority = (record: {
+	type: string;
+	content: string;
+	prio?: string | null;
+}) =>
+	(record.type === "MX" || record.type === "SRV") && record.prio != null
+		? `${record.prio} ${record.content}`
+		: record.content;
+
+const buildValue = (record: { type: string; content: string }) => {
+	const value = record.content.trim();
+	if (record.type === "MX" || record.type === "SRV") {
+		const match = /^(\d+)\s+(\S.*)$/.exec(value);
+		if (match) {
+			return { content: match[2] as string, prio: match[1] as string };
+		}
+	}
+	return { content: value };
+};
+
 export const porkbunClient: DnsClient<PorkbunConfig> = {
 	async listZones(config) {
 		const result = await pbFetch<{ domains: { domain: string }[] }>(
@@ -77,7 +97,7 @@ export const porkbunClient: DnsClient<PorkbunConfig> = {
 			id: record.id,
 			type: record.type,
 			name: record.name,
-			content: record.content,
+			content: inlinePriority(record),
 			ttl: Number(record.ttl),
 		}));
 	},
@@ -89,14 +109,24 @@ export const porkbunClient: DnsClient<PorkbunConfig> = {
 			`/dns/retrieveByNameType/${record.zoneId}/${record.type}/${subdomain}`,
 		);
 
+		const built = buildValue(record);
 		const payload = {
 			name: subdomain,
 			type: record.type,
-			content: record.content,
+			content: built.content,
+			...(built.prio ? { prio: built.prio } : {}),
 			ttl: record.ttl ?? 600,
 		};
 
-		const existingRecord = existing.records[0];
+		const expectedContent = inlinePriority({
+			type: record.type,
+			content: built.content,
+			prio: built.prio,
+		});
+
+		const existingRecord = existing.records.find(
+			(r) => inlinePriority(r) === expectedContent,
+		);
 		if (existingRecord) {
 			await pbFetch(
 				config,
@@ -115,10 +145,12 @@ export const porkbunClient: DnsClient<PorkbunConfig> = {
 	},
 
 	async updateRecord(config, zoneId, recordId, record) {
+		const built = buildValue(record);
 		await pbFetch(config, `/dns/edit/${zoneId}/${recordId}`, {
 			name: toSubdomain(record.name, zoneId),
 			type: record.type,
-			content: record.content,
+			content: built.content,
+			...(built.prio ? { prio: built.prio } : {}),
 			ttl: record.ttl ?? 600,
 		});
 		return { id: recordId };