ddns-scripts: add ionos.com dyndns provider - #30353
Conversation
b49751d to
608b62a
Compare
608b62a to
ff8d23b
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit. Nothing here blocks a merge.
Packaging side checks out:
PKG_RELEASE2 → 3 is the right move — a service definition is added without aPKG_VERSIONchange.ionos.combelongs infiles/usr/share/ddns/list: it is a plain URL service, not one of the special ones thatPackage/ddns-scripts-services/installdeletes and ships from a dedicated subpackage, so no Makefile subpackage is needed. Placement betweeninfomaniak.comandipnodns.rumatches the (loosely alphabetical) surroundings.[PASSWORD]is URL-encoded before substitution, so a token with reserved characters in theq=parameter is safe.- Omitting
"answer"is fine —send_update()skips the response check when it is empty, and the update is still verified through thelookup_hostDNS lookup. Omitting[IP](provider derives the address from the request source) also has in-tree precedent (dy.fi,now-dns.com,dhis.org, …). - Commit message accurately describes the diff.
Two inline remarks: one question about the missing ipv6 section, and one optional indentation nit.
Generated by Claude Code
| "name": "ionos.com", | ||
| "ipv4": { | ||
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]" | ||
| } |
There was a problem hiding this comment.
nit: indented with spaces; 98 of the 100 existing files in files/usr/share/ddns/default/ are tab-indented (e.g. infomaniak.com.json,`` the neighbour in the list). Purely cosmetic — JSON parses either way — but worth matching the family:
| "name": "ionos.com", | |
| "ipv4": { | |
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]" | |
| } | |
| "name": "ionos.com", | |
| "ipv4": { | |
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]" | |
| } |
Generated by Claude Code
| { | ||
| "name": "ionos.com", | ||
| "ipv4": { | ||
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]" |
There was a problem hiding this comment.
Is the missing "ipv6" block intentional? The host name here is explicitly ipv4.api.hosting.ionos.com, which suggests IONOS also hands out an ipv6.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD] counterpart for the same token — in which case an "ipv6" section with that URL would come for free.
Consequence of leaving it out: LuCI treats the provider as unusable for IPv6 — overview.js:131 sets `service_supported = false` when "Use IPv6" is on and the JSON has no `ipv6` key. On the CLI side [`get_service_data()`](https://github.com/openwrt/packages/blob/ff8d23b07a7144f7d7c37ad738c473b78c9423c7/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_functions.sh#L284-L292``) does json_select "ipv6", and with the key absent data ends up empty, so update_script is silently set to the bare directory /usr/lib/ddns/ and the section aborts with a confusing "Custom update_script not found!".
IPv4-only definitions do exist in-tree (30 of them), so this isn't wrong as such — just checking whether you verified the v6 endpoint doesn't work rather than not needing it.
Generated by Claude Code
There was a problem hiding this comment.
Thx for the info! Added a separate ipv6 block in a new commit, verified working separately for both A and AAAA record updates against a live domain, using the ipv4=/ipv6= query parameters as documented by IONOS.
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit (662f562, ipv6 support). One inline finding that does block a merge as written: [IP6] is not a placeholder ddns-scripts substitutes, so the AAAA update URL ships a literal [IP6].
The rest of the new commit is fine — the ipv4=[IP] addition uses a supported placeholder, PKG_RELEASE was already bumped to 3 by the first commit so no further bump is needed, and the commit message matches the diff.
The tab-vs-space indentation nit from the earlier review still stands; note its suggestion block predates the ipv6 block, so apply it by hand rather than with the one-click button.
Generated by Claude Code
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv4=[IP]" | ||
| }, | ||
| "ipv6": { | ||
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv6=[IP6]" |
There was a problem hiding this comment.
[IP6] is not a placeholder ddns-scripts knows about, so it is sent to IONOS verbatim. send_update() picks the address family first (use_ipv6 decides which address ends up in __IP) and then does a fixed set of replacements — dynamic_dns_functions.sh:841-842 substitutes [USERNAME], [PASSWORD], [PARAMENC], [PARAMOPT], [DOMAIN] and [IP] and nothing else. [IP6] appears nowhere in the tree; every one of the 40 in-tree ipv6 sections uses [IP] too, including ones with a v6-specific host such as afraid.org-v2-basic.json:7.`` The request as written therefore goes out as &ipv6=%5BIP6%5D (or literal brackets), never the detected AAAA address.
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv6=[IP6]" | |
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv6=[IP]" |
Worth re-testing after the fix: since the previous run sent a bogus ipv6= value, whatever made the AAAA update succeed was probably IONOS falling back to the request source address rather than the parameter — which would also mean the parameter is not what distinguishes the two families here, and the ipv6.api.hosting.ionos.com host may still be needed for v6.
Generated by Claude Code
db0b422 to
662f562
Compare
f82fc54 to
5873ad7
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit (5873ad7). The fix is correct: [IP] is the placeholder send_update() substitutes for both address families, and the commit message matches the diff exactly. Nothing here blocks a merge.
Two inline items, neither of them a blocker:
- A question about the host in the
ipv6section — it is stillipv4.api.hosting.ionos.com, which only matters if a user enables "Force IP Version" or binds to a network. - A
nit:on squashing the three commits, since the last two are review fallout on a 9-line file, plus a stale sentence in the first commit's message.
The space-vs-tab indentation nit from the first review is still open; its suggestion block predates the ipv6 section, so apply it by hand rather than with the one-click button. FormalityCheck is green on this head.
Generated by Claude Code
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv4=[IP]" | ||
| }, | ||
| "ipv6": { | ||
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv6=[IP]" |
There was a problem hiding this comment.
The placeholder is right now — send_update() substitutes [IP] for both families,`` so this sends the detected AAAA address. What is left on this line is the host: the ipv6 section points at `ipv4.api.hosting.ionos.com`.
That is fine for a default run, where the address travels in ipv6= and the transport family does not matter. It is not fine for the two settings that pin the transport to v6, both of which LuCI exposes:
force_ipversionadds-6to the fetcher — wget at dynamic_dns_functions.sh:644-646,[curl at 694-696](https://github.com/openwrt/packages/blob/5873ad72d7046f4e015379bc5ab7495b31bd1709/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_functions.sh#L694-L696).``bind_networkresolves the local address withnetwork_get_ipaddr6whenuse_ipv6is 1 and binds to it — 637-642.``
If ipv4.api.hosting.ionos.com has no AAAA record, both configurations fail to connect at all. I could not resolve AAAA from this environment, so I cannot tell either way — you can, against the live domain. Did the AAAA test run with "Force IP Version" enabled, or only in the default configuration?
For context: no other in-tree ipv6 section uses a v4-named host. The closest analogue, dynv6.com.json:7-9,`` does reuse one endpoint for both families with ipv4=/`ipv6=` exactly as you do here — but its host name is family-neutral. If IONOS reaches the same API over v6 under another name, using it here would make the section work in every configuration. (This supersedes my earlier speculation that a separate v6 endpoint was required — the shared endpoint plus `ipv6=` clearly matches how IONOS documents it.)
Generated by Claude Code
| { | ||
| "name": "ionos.com", | ||
| "ipv4": { | ||
| "url": "https://ipv4.api.hosting.ionos.com/dns/v1/dyndns?q=[PASSWORD]&ipv4=[IP]" |
There was a problem hiding this comment.
nit: series hygiene — the three commits add one 9-line file, and the last two are review fallout: 662f562 introduces &ipv6=[IP6], 5873ad7 only corrects that string. Squashing all three into a single "ddns-scripts: add ionos.com dyndns provider" commit would leave a cleaner history, and nothing in the series depends on the intermediate states.
While squashing, the message body is worth a pass too: it currently says the API "uses a single fixed update URL … no username or domain parameter is required in the request itself", which was written before this &ipv4=[IP] and its ipv6= counterpart existed. The final file passes the address explicitly per family, so the description and the file now disagree.
Generated by Claude Code
5873ad7 to
121b0bb
Compare
Adds ionos.com as predefined DDNS service. IONOS' dynamic DNS API only supports domains hosted at IONOS and uses a single fixed update URL with a generated token as password. Both ipv4 and ipv6 updates use the same endpoint, distinguished by explicitly passing the detected address via the ipv4= or ipv6= query parameter as documented by IONOS. Verified working separately for both A and AAAA record updates against a live domain. Signed-off-by: Lasse Hanschen <[email protected]>
121b0bb to
245a786
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit (245a786); no new issues found.
The three commits are now squashed into one, as suggested — git diff 5873ad72..245a7867 is empty, so the tree is byte-identical to the head I reviewed yesterday and only the history changed. The rewritten message body now describes the final file accurately (both families on one endpoint, distinguished by ipv4=/ipv6=), so the stale sentence flagged last time is gone. All three FormalityCheck jobs are green on this head.
Two earlier inline threads are still open and still apply to the current file — not repeated inline to avoid duplicates:
- The
ipv6section pointing atipv4.api.hosting.ionos.com(thread) — a question about whether the AAAA test ran with "Force IP Version" /bind_networkset, not a blocker for a default run. - Space-vs-tab indentation in the JSON (thread) — cosmetic; its suggestion block predates the
ipv6section, so apply it by hand rather than with the one-click button.
Neither blocks a merge.
Generated by Claude Code
📦 Package Details
Maintainer: @databloat
(You can find this by checking the history of the package
Makefile.)Description:
Adds ionos.com as predefined DDNS service. IONOS' dynamic DNS API only supports domains hosted at IONOS and uses a single fixed update URL with a generated token as password; no username or domain parameter is required in the request itself.
🧪 Run Testing Details
✅ Formalities