From 80a6baf58ed63bf1a279dfb5dd411070c844facb Mon Sep 17 00:00:00 2001 From: Guillaume Juge Date: Tue, 1 Sep 2026 16:29:07 +0200 Subject: [PATCH] fix(dns): restore the original OVH record when a type change fails Changing a record's type deletes the record then recreates it with the new type, because OVH's update payload carries no fieldType. If the creation failed the name was left with nothing and no rollback. The delete still has to come first, since OVH rejects a CNAME that would sit alongside other data on the same name. So on a failed creation the original record is put back from the copy already fetched before the delete, and the original error is rethrown. If the restore fails too, the error names the record that has to be recreated by hand. Reported by Greptile on #5258. Co-Authored-By: Claude Opus 5 (1M context) --- apps/dokploy/__test__/dns/ovh.test.ts | 66 +++++++++++++++++++++++++++ packages/server/src/utils/dns/ovh.ts | 57 +++++++++++++++++++---- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/apps/dokploy/__test__/dns/ovh.test.ts b/apps/dokploy/__test__/dns/ovh.test.ts index 4ddeff8fb..5fe829d27 100644 --- a/apps/dokploy/__test__/dns/ovh.test.ts +++ b/apps/dokploy/__test__/dns/ovh.test.ts @@ -365,6 +365,72 @@ describe("ovhClient.updateRecord", () => { expect(JSON.parse(calls[2]?.[1].body as string).fieldType).toBe("CNAME"); expect(calls[3]?.[0]).toContain("/refresh"); }); + + it("restores the original record when the replacement fails", async () => { + const cfg = freshConfig(); + const original = { + id: 4, + zone: "example.com", + fieldType: "A", + subDomain: "app", + target: "1.1.1.1", + ttl: 60, + }; + mockApi( + ovhSuccess(original), + ovhSuccess(null), + ovhError("Invalid target", 400), + ovhSuccess({ id: 12 }), + ovhSuccess(null), + ); + + await expect( + ovhClient.updateRecord(cfg, "example.com", "4", { + type: "CNAME", + name: "app.example.com", + content: "not a valid target", + }), + ).rejects.toThrow("Invalid target"); + + const calls = apiCalls(); + expect(calls[1]?.[1].method).toBe("DELETE"); + expect(calls[2]?.[1].method).toBe("POST"); + // The original record is put back with its own type, target and ttl. + expect(JSON.parse(calls[3]?.[1].body as string)).toEqual({ + fieldType: "A", + subDomain: "app", + target: "1.1.1.1", + ttl: 60, + }); + expect(calls[4]?.[0]).toContain("/refresh"); + }); + + it("reports the lost record when the restore also fails", async () => { + const cfg = freshConfig(); + mockApi( + ovhSuccess({ + id: 4, + zone: "example.com", + fieldType: "A", + subDomain: "app", + target: "1.1.1.1", + ttl: 60, + }), + ovhSuccess(null), + ovhError("Invalid target", 400), + ovhError("Service unavailable", 503), + ); + + await expect( + ovhClient.updateRecord(cfg, "example.com", "4", { + type: "CNAME", + name: "app.example.com", + content: "not a valid target", + }), + ).rejects.toThrow( + /Recreate it manually: A app\.example\.com -> 1\.1\.1\.1/, + ); + }); }); describe("ovhClient.deleteRecord", () => { diff --git a/packages/server/src/utils/dns/ovh.ts b/packages/server/src/utils/dns/ovh.ts index 8c4a98ea2..58c372356 100644 --- a/packages/server/src/utils/dns/ovh.ts +++ b/packages/server/src/utils/dns/ovh.ts @@ -171,6 +171,34 @@ const refreshZone = async (config: OvhConfig, zone: string) => { }); }; +// Used to undo the delete half of a type change when the replacement fails. +const restoreRecord = async ( + config: OvhConfig, + zone: string, + record: OvhRecord, + cause: unknown, +) => { + try { + await ovhFetch(config, `/domain/zone/${encodeURIComponent(zone)}/record`, { + method: "POST", + body: { + fieldType: record.fieldType, + subDomain: record.subDomain ?? "", + target: record.target, + ...(record.ttl === null ? {} : { ttl: record.ttl }), + }, + }); + await refreshZone(config, zone); + } catch { + const name = toFqdn(record.subDomain, zone); + throw new Error( + `OVH: could not replace the record and could not restore the original one, which has been deleted. Recreate it manually: ${record.fieldType} ${name} -> ${record.target}. Original failure: ${ + cause instanceof Error ? cause.message : String(cause) + }`, + ); + } +}; + const recordBody = ( record: { name: string; content: string; ttl?: number }, zone: string, @@ -244,19 +272,30 @@ export const ovhClient: DnsClient = { ); // The update payload carries no fieldType, so switching a record's type - // means replacing it. + // means replacing it. The delete has to come first: OVH rejects a CNAME + // that would sit alongside other data on the same name. If the creation + // then fails, put the original record back rather than leaving the name + // with nothing. if (existing.fieldType !== record.type) { await ovhFetch(config, `/domain/zone/${zone}/record/${recordId}`, { method: "DELETE", }); - const created = await ovhFetch( - config, - `/domain/zone/${zone}/record`, - { - method: "POST", - body: { fieldType: record.type, ...recordBody(record, zoneId) }, - }, - ); + + let created: OvhRecord; + try { + created = await ovhFetch( + config, + `/domain/zone/${zone}/record`, + { + method: "POST", + body: { fieldType: record.type, ...recordBody(record, zoneId) }, + }, + ); + } catch (error) { + await restoreRecord(config, zoneId, existing, error); + throw error; + } + await refreshZone(config, zoneId); return { id: String(created.id) }; }