* [PATCH] ddns: add Cloudflare (v4) provider using API token
@ 2025-08-29 8:16 Chris Anton
2025-08-29 9:50 ` Michael Tremer
0 siblings, 1 reply; 6+ messages in thread
From: Chris Anton @ 2025-08-29 8:16 UTC (permalink / raw)
To: ddns
From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github.com>
Date: Wed, 27 Aug 2025 01:22:46 -0500
Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This adds a provider “cloudflare.com-v4” that updates an A record
via Cloudflare’s v4 API using a Bearer token. The token is accepted
from either ‘token’ or legacy ‘password’ for UI compatibility.
Tested on IPFire 2.29 / Core 196:
- no-op if A already matches WAN IP
- successful update when WAN IP changes
- logs include CFv4 breadcrumbs for troubleshooting
Signed-off-by: Chris Anton <chris.v.anton@gmail.com>
---
563f089d0820bd61ad4aecac248d5cc1f2adfc81
src/ddns/providers.py | 121 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 121 insertions(+)
diff --git a/src/ddns/providers.py b/src/ddns/providers.py
index 59f9665..df0f3a9 100644
--- a/src/ddns/providers.py
+++ b/src/ddns/providers.py
@@ -341,6 +341,127 @@ def have_address(self, proto):
return False
+class DDNSProviderCloudflareV4(DDNSProvider):
+ """
+ Cloudflare v4 API using a Bearer Token.
+ Put the API Token in the 'token' OR 'password' field of the DDNS entry.
+ Optional in ddns.conf:
+ proxied = false|true (default false; keep false for WireGuard)
+ ttl = 1|60|120... (default 1 = 'automatic')
+ """
+ handle = "cloudflare.com-v4"
+ name = "Cloudflare (v4)"
+ website = "https://www.cloudflare.com/"
+ protocols = ("ipv4",)
+ supports_token_auth = True
+ holdoff_failure_days = 0
+
+ def _bool(self, key, default=False):
+ v = str(self.get(key, default)).strip().lower()
+ return v in ("1", "true", "yes", "on")
+
+ def update(self):
+ import json, urllib.request, urllib.error
+
+ tok = self.get("token") or self.get("password")
+ if not tok:
+ raise DDNSConfigurationError("API Token (password/token)
is missing.")
+
+ proxied = self._bool("proxied", False)
+ try:
+ ttl = int(self.get("ttl", 1))
+ except Exception:
+ ttl = 1
+
+ headers = {
+ "Authorization": "Bearer {0}".format(tok),
+ "Content-Type": "application/json",
+ "User-Agent": "IPFireDDNSUpdater/CFv4",
+ }
+
+ # --- find zone ---
+ parts = self.hostname.split(".")
+ if len(parts) < 2:
+ raise DDNSRequestError("Hostname '{0}' is not a valid
domain.".format(self.hostname))
+
+ zone_id = None
+ zone_name = None
+ for i in range(len(parts) - 1):
+ candidate = ".".join(parts[i:])
+ url =
f"https://api.cloudflare.com/client/v4/zones?name={candidate}"
+ try:
+ req = urllib.request.Request(url, headers=headers,
method="GET")
+ with urllib.request.urlopen(req, timeout=20) as r:
+ data = json.loads(r.read().decode())
+ except Exception as e:
+ raise DDNSUpdateError(f"Failed to query Cloudflare
zones API: {e}")
+
+ if data.get("success") and data.get("result"):
+ zone_id = data["result"][0]["id"]
+ zone_name = candidate
+ break
+
+ if not zone_id:
+ raise DDNSRequestError(f"Could not find a Cloudflare Zone
for '{self.hostname}'.")
+
+ logger.info("CFv4: zone=%s id=%s", zone_name, zone_id)
+
+ # --- get record ---
+ rec_url =
f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records?type=A&name={self.hostname}"
+ try:
+ req = urllib.request.Request(rec_url, headers=headers,
method="GET")
+ with urllib.request.urlopen(req, timeout=20) as r:
+ rec_data = json.loads(r.read().decode())
+ except Exception as e:
+ raise DDNSUpdateError(f"Failed to query Cloudflare DNS
records API: {e}")
+
+ if not rec_data.get("success"):
+ errs = rec_data.get("errors") or []
+ if any("Authentication error" in (e.get("message", "") or
"") for e in errs):
+ raise DDNSAuthenticationError("Invalid API Token.")
+ raise DDNSUpdateError(f"Cloudflare API error finding
record: {errs}")
+
+ results = rec_data.get("result") or []
+ if not results:
+ raise DDNSRequestError(f"No A record found for
'{self.hostname}' in zone '{zone_name}'.")
+
+ record_id = results[0]["id"]
+ stored_ip = results[0]["content"]
+ logger.info("CFv4: record_id=%s stored_ip=%s", record_id, stored_ip)
+
+ # --- compare IPs ---
+ current_ip = self.get_address("ipv4")
+ logger.info("CFv4: current_ip=%s vs stored_ip=%s",
current_ip, stored_ip)
+ if current_ip == stored_ip:
+ logger.info("CFv4: no update needed")
+ return
+
+ # --- update ---
+ upd_url =
f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records/{record_id}"
+ payload = {
+ "type": "A",
+ "name": self.hostname,
+ "content": current_ip,
+ "ttl": ttl,
+ "proxied": proxied,
+ }
+ logger.info("CFv4: updating %s -> %s (proxied=%s ttl=%s)",
self.hostname, current_ip, proxied, ttl)
+
+ try:
+ req = urllib.request.Request(
+ upd_url, data=json.dumps(payload).encode(),
headers=headers, method="PUT"
+ )
+ with urllib.request.urlopen(req, timeout=20) as r:
+ upd = json.loads(r.read().decode())
+ except Exception as e:
+ raise DDNSUpdateError(f"Failed to send update request to
Cloudflare: {e}")
+
+ if not upd.get("success"):
+ raise DDNSUpdateError(f"Cloudflare API error on update:
{upd.get('errors')}")
+
+ logger.info("CFv4: update ok for %s -> %s", self.hostname, current_ip)
+ return
+
class DDNSProtocolDynDNS2(object):
"""
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ddns: add Cloudflare (v4) provider using API token
2025-08-29 8:16 [PATCH] ddns: add Cloudflare (v4) provider using API token Chris Anton
@ 2025-08-29 9:50 ` Michael Tremer
2025-08-29 10:20 ` Bernhard Beroun
0 siblings, 1 reply; 6+ messages in thread
From: Michael Tremer @ 2025-08-29 9:50 UTC (permalink / raw)
To: Chris Anton; +Cc: ddns
Hello Chris,
You don’t need to submit any patches more than once. We will get back to you as soon as there is time.
So let’s get into this...
> On 29 Aug 2025, at 09:16, Chris Anton <chris.v.anton@gmail.com> wrote:
>
> From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github.com>
I assume that you are faithinchaos21 and this code did not come from someone else?
> Date: Wed, 27 Aug 2025 01:22:46 -0500
> Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This adds a provider “cloudflare.com-v4” that updates an A record
> via Cloudflare’s v4 API using a Bearer token. The token is accepted
> from either ‘token’ or legacy ‘password’ for UI compatibility.
>
> Tested on IPFire 2.29 / Core 196:
> - no-op if A already matches WAN IP
> - successful update when WAN IP changes
> - logs include CFv4 breadcrumbs for troubleshooting
To make it short, this patch is sadly very far away from what is acceptable.
Before we get into the actual implementation, there are many design issues here that simply cannot be accepted into DDNS:
* This patch only adds support for IPv4. As far as I am aware, Cloudflare is capable of IPv6, too. Why would this be limited to IPv4 only?
* You are not using the scaffolding and tools that DDNS is providing. A lot of code is being written with plain Python and throwing away all the features that DDNS provides (catching Exceptions, proxy support and many, many more…)
* This all looks very AI-generated to me and is implemented on a green field. You are even importing all the modules that you need and ignore everything else from DDNS. Why not use this snippet as a standalone script then? There was no consideration for what code existed already.
> Signed-off-by: Chris Anton <chris.v.anton@gmail.com>
> ---
> 563f089d0820bd61ad4aecac248d5cc1f2adfc81
> src/ddns/providers.py | 121 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 121 insertions(+)
>
> diff --git a/src/ddns/providers.py b/src/ddns/providers.py
> index 59f9665..df0f3a9 100644
> --- a/src/ddns/providers.py
> +++ b/src/ddns/providers.py
> @@ -341,6 +341,127 @@ def have_address(self, proto):
>
> return False
>
> +class DDNSProviderCloudflareV4(DDNSProvider):
> + """
> + Cloudflare v4 API using a Bearer Token.
> + Put the API Token in the 'token' OR 'password' field of the DDNS entry.
> + Optional in ddns.conf:
> + proxied = false|true (default false; keep false for WireGuard)
> + ttl = 1|60|120... (default 1 = 'automatic')
> + """
> + handle = "cloudflare.com-v4"
> + name = "Cloudflare (v4)"
> + website = "https://www.cloudflare.com/"
> + protocols = ("ipv4",)
> + supports_token_auth = True
> + holdoff_failure_days = 0
> +
> + def _bool(self, key, default=False):
> + v = str(self.get(key, default)).strip().lower()
> + return v in ("1", "true", "yes", "on")
> +
This is a good example of a function which totally goes against the coding style of the rest of the code base. It uses str(), chains a lot of functions to gather to make the code look shorter, but very difficult to read.
> + def update(self):
> + import json, urllib.request, urllib.error
Just no. We don’t import anything further down the line. DDNS provides a toolkit of what to use and you should stay within it. If some functionally is missing, DDNS’ functionality should be extended so that other providers can re-use the same well-maintained and tested code base.
> +
> + tok = self.get("token") or self.get("password")
> + if not tok:
> + raise DDNSConfigurationError("API Token (password/token)
> is missing.")
> +
> + proxied = self._bool("proxied", False)
> + try:
> + ttl = int(self.get("ttl", 1))
> + except Exception:
> + ttl = 1
A TTL of one second is never a good idea.
> +
> + headers = {
> + "Authorization": "Bearer {0}".format(tok),
> + "Content-Type": "application/json",
> + "User-Agent": "IPFireDDNSUpdater/CFv4",
> + }
Since you are not using the internal request handler, you are creating your own headers and a completely nonsensical user agent. Why?
> +
> + # --- find zone ---
> + parts = self.hostname.split(".")
> + if len(parts) < 2:
> + raise DDNSRequestError("Hostname '{0}' is not a valid
> domain.".format(self.hostname))
> +
> + zone_id = None
> + zone_name = None
> + for i in range(len(parts) - 1):
> + candidate = ".".join(parts[i:])
> + url =
> f"https://api.cloudflare.com/client/v4/zones?name={candidate}"
> + try:
> + req = urllib.request.Request(url, headers=headers,
> method="GET")
> + with urllib.request.urlopen(req, timeout=20) as r:
> + data = json.loads(r.read().decode())
> + except Exception as e:
> + raise DDNSUpdateError(f"Failed to query Cloudflare
> zones API: {e}")
> +
> + if data.get("success") and data.get("result"):
> + zone_id = data["result"][0]["id"]
> + zone_name = candidate
> + break
It is not acceptable to build anything custom that sends a request. You are removing all other kinds of features that I have mentioned above.
To just “guess” the name of the zone is something that I would not consider good style.
> +
> + if not zone_id:
> + raise DDNSRequestError(f"Could not find a Cloudflare Zone
> for '{self.hostname}'.")
> +
> + logger.info("CFv4: zone=%s id=%s", zone_name, zone_id)
> +
> + # --- get record ---
> + rec_url =
> f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records?type=A&name={self.hostname}"
> + try:
> + req = urllib.request.Request(rec_url, headers=headers,
> method="GET")
> + with urllib.request.urlopen(req, timeout=20) as r:
> + rec_data = json.loads(r.read().decode())
> + except Exception as e:
> + raise DDNSUpdateError(f"Failed to query Cloudflare DNS
> records API: {e}")
> +
> + if not rec_data.get("success"):
> + errs = rec_data.get("errors") or []
> + if any("Authentication error" in (e.get("message", "") or
> "") for e in errs):
> + raise DDNSAuthenticationError("Invalid API Token.")
> + raise DDNSUpdateError(f"Cloudflare API error finding
> record: {errs}")
Same as above, hardcoded defaults like the timeout. Spaghetti code.
> +
> + results = rec_data.get("result") or []
> + if not results:
> + raise DDNSRequestError(f"No A record found for
> '{self.hostname}' in zone '{zone_name}'.")
> +
> + record_id = results[0]["id"]
> + stored_ip = results[0]["content"]
> + logger.info("CFv4: record_id=%s stored_ip=%s", record_id, stored_ip)
> +
> + # --- compare IPs ---
> + current_ip = self.get_address("ipv4")
> + logger.info("CFv4: current_ip=%s vs stored_ip=%s",
> current_ip, stored_ip)
> + if current_ip == stored_ip:
> + logger.info("CFv4: no update needed")
> + return
Why is this done using the API at all? We have functionality to use DNS for this which creates a lot less load than performing several API requests every couple of minutes.
> +
> + # --- update ---
> + upd_url =
> f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records/{record_id}"
> + payload = {
> + "type": "A",
> + "name": self.hostname,
> + "content": current_ip,
> + "ttl": ttl,
> + "proxied": proxied,
> + }
> + logger.info("CFv4: updating %s -> %s (proxied=%s ttl=%s)",
> self.hostname, current_ip, proxied, ttl)
> +
> + try:
> + req = urllib.request.Request(
> + upd_url, data=json.dumps(payload).encode(),
> headers=headers, method="PUT"
> + )
> + with urllib.request.urlopen(req, timeout=20) as r:
> + upd = json.loads(r.read().decode())
> + except Exception as e:
> + raise DDNSUpdateError(f"Failed to send update request to
> Cloudflare: {e}”)
>
Once again the same custom request logic.
> + if not upd.get("success"):
> + raise DDNSUpdateError(f"Cloudflare API error on update:
> {upd.get('errors')}")
> +
> + logger.info("CFv4: update ok for %s -> %s", self.hostname, current_ip)
> + return
> +
>
> class DDNSProtocolDynDNS2(object):
> """
>
I would really like to have support for Cloudflare in DDNS, but this is sadly not the way.
Please study my comments thoroughly and then lets come up with a plan together how to implement this properly.
Best,
-Michael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ddns: add Cloudflare (v4) provider using API token
2025-08-29 9:50 ` Michael Tremer
@ 2025-08-29 10:20 ` Bernhard Beroun
2025-08-29 10:25 ` Michael Tremer
0 siblings, 1 reply; 6+ messages in thread
From: Bernhard Beroun @ 2025-08-29 10:20 UTC (permalink / raw)
To: Michael Tremer; +Cc: Chris Anton, ddns
[-- Attachment #1: Type: text/plain, Size: 8903 bytes --]
Hi everybody,
I don't want to hijack this mail thread here, but just wanted to let you know that I've also submitted a patch a few months ago. See https://lists.ipfire.org/ddns/19993045-2374-4537-96a2-5e2b1900a750@liftingcoder.com/T/#u
Unfortunately I never received any feedback, so I assumed Cloudflare support wasn't desired. Again, sorry for hijacking this email chain here, just wanted to quickly drop it here, since it matches the topic.
Have a nice day,
Bernhard
Am Freitag, August 29, 2025 11:50 CEST, schrieb Michael Tremer <michael.tremer@ipfire.org>:
Hello Chris,
You don’t need to submit any patches more than once. We will get back to you as soon as there is time.
So let’s get into this...
> On 29 Aug 2025, at 09:16, Chris Anton <chris.v.anton@gmail.com> wrote:
>
> From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github.com>
I assume that you are faithinchaos21 and this code did not come from someone else?
> Date: Wed, 27 Aug 2025 01:22:46 -0500
> Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This adds a provider “cloudflare.com-v4” that updates an A record
> via Cloudflare’s v4 API using a Bearer token. The token is accepted
> from either ‘token’ or legacy ‘password’ for UI compatibility.
>
> Tested on IPFire 2.29 / Core 196:
> - no-op if A already matches WAN IP
> - successful update when WAN IP changes
> - logs include CFv4 breadcrumbs for troubleshooting
To make it short, this patch is sadly very far away from what is acceptable.
Before we get into the actual implementation, there are many design issues here that simply cannot be accepted into DDNS:
* This patch only adds support for IPv4. As far as I am aware, Cloudflare is capable of IPv6, too. Why would this be limited to IPv4 only?
* You are not using the scaffolding and tools that DDNS is providing. A lot of code is being written with plain Python and throwing away all the features that DDNS provides (catching Exceptions, proxy support and many, many more…)
* This all looks very AI-generated to me and is implemented on a green field. You are even importing all the modules that you need and ignore everything else from DDNS. Why not use this snippet as a standalone script then? There was no consideration for what code existed already.
> Signed-off-by: Chris Anton <chris.v.anton@gmail.com>
> ---
> 563f089d0820bd61ad4aecac248d5cc1f2adfc81
> src/ddns/providers.py | 121 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 121 insertions(+)
>
> diff --git a/src/ddns/providers.py b/src/ddns/providers.py
> index 59f9665..df0f3a9 100644
> --- a/src/ddns/providers.py
> +++ b/src/ddns/providers.py
> @@ -341,6 +341,127 @@ def have_address(self, proto):
>
> return False
>
> +class DDNSProviderCloudflareV4(DDNSProvider):
> + """
> + Cloudflare v4 API using a Bearer Token.
> + Put the API Token in the 'token' OR 'password' field of the DDNS entry.
> + Optional in ddns.conf:
> + proxied = false|true (default false; keep false for WireGuard)
> + ttl = 1|60|120... (default 1 = 'automatic')
> + """
> + handle = "cloudflare.com-v4"
> + name = "Cloudflare (v4)"
> + website = "https://www.cloudflare.com/"
> + protocols = ("ipv4",)
> + supports_token_auth = True
> + holdoff_failure_days = 0
> +
> + def _bool(self, key, default=False):
> + v = str(self.get(key, default)).strip().lower()
> + return v in ("1", "true", "yes", "on")
> +
This is a good example of a function which totally goes against the coding style of the rest of the code base. It uses str(), chains a lot of functions to gather to make the code look shorter, but very difficult to read.
> + def update(self):
> + import json, urllib.request, urllib.error
Just no. We don’t import anything further down the line. DDNS provides a toolkit of what to use and you should stay within it. If some functionally is missing, DDNS’ functionality should be extended so that other providers can re-use the same well-maintained and tested code base.
> +
> + tok = self.get("token") or self.get("password")
> + if not tok:
> + raise DDNSConfigurationError("API Token (password/token)
> is missing.")
> +
> + proxied = self._bool("proxied", False)
> + try:
> + ttl = int(self.get("ttl", 1))
> + except Exception:
> + ttl = 1
A TTL of one second is never a good idea.
> +
> + headers = {
> + "Authorization": "Bearer {0}".format(tok),
> + "Content-Type": "application/json",
> + "User-Agent": "IPFireDDNSUpdater/CFv4",
> + }
Since you are not using the internal request handler, you are creating your own headers and a completely nonsensical user agent. Why?
> +
> + # --- find zone ---
> + parts = self.hostname.split(".")
> + if len(parts) < 2:
> + raise DDNSRequestError("Hostname '{0}' is not a valid
> domain.".format(self.hostname))
> +
> + zone_id = None
> + zone_name = None
> + for i in range(len(parts) - 1):
> + candidate = ".".join(parts[i:])
> + url =
> f"https://api.cloudflare.com/client/v4/zones?name={candidate}"
> + try:
> + req = urllib.request.Request(url, headers=headers,
> method="GET")
> + with urllib.request.urlopen(req, timeout=20) as r:
> + data = json.loads(r.read().decode())
> + except Exception as e:
> + raise DDNSUpdateError(f"Failed to query Cloudflare
> zones API: {e}")
> +
> + if data.get("success") and data.get("result"):
> + zone_id = data["result"][0]["id"]
> + zone_name = candidate
> + break
It is not acceptable to build anything custom that sends a request. You are removing all other kinds of features that I have mentioned above.
To just “guess” the name of the zone is something that I would not consider good style.
> +
> + if not zone_id:
> + raise DDNSRequestError(f"Could not find a Cloudflare Zone
> for '{self.hostname}'.")
> +
> + logger.info("CFv4: zone=%s id=%s", zone_name, zone_id)
> +
> + # --- get record ---
> + rec_url =
> f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records?type=A&name={self.hostname}"
> + try:
> + req = urllib.request.Request(rec_url, headers=headers,
> method="GET")
> + with urllib.request.urlopen(req, timeout=20) as r:
> + rec_data = json.loads(r.read().decode())
> + except Exception as e:
> + raise DDNSUpdateError(f"Failed to query Cloudflare DNS
> records API: {e}")
> +
> + if not rec_data.get("success"):
> + errs = rec_data.get("errors") or []
> + if any("Authentication error" in (e.get("message", "") or
> "") for e in errs):
> + raise DDNSAuthenticationError("Invalid API Token.")
> + raise DDNSUpdateError(f"Cloudflare API error finding
> record: {errs}")
Same as above, hardcoded defaults like the timeout. Spaghetti code.
> +
> + results = rec_data.get("result") or []
> + if not results:
> + raise DDNSRequestError(f"No A record found for
> '{self.hostname}' in zone '{zone_name}'.")
> +
> + record_id = results[0]["id"]
> + stored_ip = results[0]["content"]
> + logger.info("CFv4: record_id=%s stored_ip=%s", record_id, stored_ip)
> +
> + # --- compare IPs ---
> + current_ip = self.get_address("ipv4")
> + logger.info("CFv4: current_ip=%s vs stored_ip=%s",
> current_ip, stored_ip)
> + if current_ip == stored_ip:
> + logger.info("CFv4: no update needed")
> + return
Why is this done using the API at all? We have functionality to use DNS for this which creates a lot less load than performing several API requests every couple of minutes.
> +
> + # --- update ---
> + upd_url =
> f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records/{record_id}"
> + payload = {
> + "type": "A",
> + "name": self.hostname,
> + "content": current_ip,
> + "ttl": ttl,
> + "proxied": proxied,
> + }
> + logger.info("CFv4: updating %s -> %s (proxied=%s ttl=%s)",
> self.hostname, current_ip, proxied, ttl)
> +
> + try:
> + req = urllib.request.Request(
> + upd_url, data=json.dumps(payload).encode(),
> headers=headers, method="PUT"
> + )
> + with urllib.request.urlopen(req, timeout=20) as r:
> + upd = json.loads(r.read().decode())
> + except Exception as e:
> + raise DDNSUpdateError(f"Failed to send update request to
> Cloudflare: {e}”)
>
Once again the same custom request logic.
> + if not upd.get("success"):
> + raise DDNSUpdateError(f"Cloudflare API error on update:
> {upd.get('errors')}")
> +
> + logger.info("CFv4: update ok for %s -> %s", self.hostname, current_ip)
> + return
> +
>
> class DDNSProtocolDynDNS2(object):
> """
>
I would really like to have support for Cloudflare in DDNS, but this is sadly not the way.
Please study my comments thoroughly and then lets come up with a plan together how to implement this properly.
Best,
-Michael
[-- Attachment #2: Type: text/html, Size: 10122 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ddns: add Cloudflare (v4) provider using API token
2025-08-29 10:20 ` Bernhard Beroun
@ 2025-08-29 10:25 ` Michael Tremer
2025-08-29 18:08 ` Bernhard Beroun
0 siblings, 1 reply; 6+ messages in thread
From: Michael Tremer @ 2025-08-29 10:25 UTC (permalink / raw)
To: Bernhard Beroun; +Cc: Chris Anton, ddns
Hello Bernhard,
No you are not hijacking.
I don’t know why your patch did not receive any feedback. It must have fallen off the radar. I am not even the maintainer for DDNS…
Your patch looks much more like it because it is using and extending the code of DDNS.
However, it also only implements IPv4. Does Cloudflare not support IPv6 for DNS at all?
-Michael
> On 29 Aug 2025, at 11:20, Bernhard Beroun <bernhard@liftingcoder.com> wrote:
>
> Hi everybody,
> I don't want to hijack this mail thread here, but just wanted to let you know that I've also submitted a patch a few months ago. See https://lists.ipfire.org/ddns/19993045-2374-4537-96a2-5e2b1900a750@liftingcoder.com/T/#u
> Unfortunately I never received any feedback, so I assumed Cloudflare support wasn't desired. Again, sorry for hijacking this email chain here, just wanted to quickly drop it here, since it matches the topic.
> Have a nice day,
> Bernhard
>
> Am Freitag, August 29, 2025 11:50 CEST, schrieb Michael Tremer <michael.tremer@ipfire.org>:
>
>
>>
>> Hello Chris,
>>
>> You don’t need to submit any patches more than once. We will get back to you as soon as there is time.
>>
>> So let’s get into this...
>>
>> > On 29 Aug 2025, at 09:16, Chris Anton <chris.v.anton@gmail.com> wrote:
>> >
>> > From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github.com>
>>
>> I assume that you are faithinchaos21 and this code did not come from someone else?
>>
>> > Date: Wed, 27 Aug 2025 01:22:46 -0500
>> > Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token
>> > MIME-Version: 1.0
>> > Content-Type: text/plain; charset=UTF-8
>> > Content-Transfer-Encoding: 8bit
>> >
>> > This adds a provider “cloudflare.com-v4” that updates an A record
>> > via Cloudflare’s v4 API using a Bearer token. The token is accepted
>> > from either ‘token’ or legacy ‘password’ for UI compatibility.
>> >
>> > Tested on IPFire 2.29 / Core 196:
>> > - no-op if A already matches WAN IP
>> > - successful update when WAN IP changes
>> > - logs include CFv4 breadcrumbs for troubleshooting
>>
>> To make it short, this patch is sadly very far away from what is acceptable.
>>
>> Before we get into the actual implementation, there are many design issues here that simply cannot be accepted into DDNS:
>>
>> * This patch only adds support for IPv4. As far as I am aware, Cloudflare is capable of IPv6, too. Why would this be limited to IPv4 only?
>>
>> * You are not using the scaffolding and tools that DDNS is providing. A lot of code is being written with plain Python and throwing away all the features that DDNS provides (catching Exceptions, proxy support and many, many more…)
>>
>> * This all looks very AI-generated to me and is implemented on a green field. You are even importing all the modules that you need and ignore everything else from DDNS. Why not use this snippet as a standalone script then? There was no consideration for what code existed already.
>>
>> > Signed-off-by: Chris Anton <chris.v.anton@gmail.com>
>> > ---
>> > 563f089d0820bd61ad4aecac248d5cc1f2adfc81
>> > src/ddns/providers.py | 121 ++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 121 insertions(+)
>> >
>> > diff --git a/src/ddns/providers.py b/src/ddns/providers.py
>> > index 59f9665..df0f3a9 100644
>> > --- a/src/ddns/providers.py
>> > +++ b/src/ddns/providers.py
>> > @@ -341,6 +341,127 @@ def have_address(self, proto):
>> >
>> > return False
>> >
>> > +class DDNSProviderCloudflareV4(DDNSProvider):
>> > + """
>> > + Cloudflare v4 API using a Bearer Token.
>> > + Put the API Token in the 'token' OR 'password' field of the DDNS entry.
>> > + Optional in ddns.conf:
>> > + proxied = false|true (default false; keep false for WireGuard)
>> > + ttl = 1|60|120... (default 1 = 'automatic')
>> > + """
>> > + handle = "cloudflare.com-v4"
>> > + name = "Cloudflare (v4)"
>> > + website = "https://www.cloudflare.com/"
>> > + protocols = ("ipv4",)
>> > + supports_token_auth = True
>> > + holdoff_failure_days = 0
>> > +
>> > + def _bool(self, key, default=False):
>> > + v = str(self.get(key, default)).strip().lower()
>> > + return v in ("1", "true", "yes", "on")
>> > +
>>
>> This is a good example of a function which totally goes against the coding style of the rest of the code base. It uses str(), chains a lot of functions to gather to make the code look shorter, but very difficult to read.
>>
>> > + def update(self):
>> > + import json, urllib.request, urllib.error
>>
>> Just no. We don’t import anything further down the line. DDNS provides a toolkit of what to use and you should stay within it. If some functionally is missing, DDNS’ functionality should be extended so that other providers can re-use the same well-maintained and tested code base.
>>
>> > +
>> > + tok = self.get("token") or self.get("password")
>> > + if not tok:
>> > + raise DDNSConfigurationError("API Token (password/token)
>> > is missing.")
>> > +
>> > + proxied = self._bool("proxied", False)
>> > + try:
>> > + ttl = int(self.get("ttl", 1))
>> > + except Exception:
>> > + ttl = 1
>>
>> A TTL of one second is never a good idea.
>>
>> > +
>> > + headers = {
>> > + "Authorization": "Bearer {0}".format(tok),
>> > + "Content-Type": "application/json",
>> > + "User-Agent": "IPFireDDNSUpdater/CFv4",
>> > + }
>>
>> Since you are not using the internal request handler, you are creating your own headers and a completely nonsensical user agent. Why?
>>
>> > +
>> > + # --- find zone ---
>> > + parts = self.hostname.split(".")
>> > + if len(parts) < 2:
>> > + raise DDNSRequestError("Hostname '{0}' is not a valid
>> > domain.".format(self.hostname))
>> > +
>> > + zone_id = None
>> > + zone_name = None
>> > + for i in range(len(parts) - 1):
>> > + candidate = ".".join(parts[i:])
>> > + url =
>> > f"https://api.cloudflare.com/client/v4/zones?name={candidate}"
>> > + try:
>> > + req = urllib.request.Request(url, headers=headers,
>> > method="GET")
>> > + with urllib.request.urlopen(req, timeout=20) as r:
>> > + data = json.loads(r.read().decode())
>> > + except Exception as e:
>> > + raise DDNSUpdateError(f"Failed to query Cloudflare
>> > zones API: {e}")
>> > +
>> > + if data.get("success") and data.get("result"):
>> > + zone_id = data["result"][0]["id"]
>> > + zone_name = candidate
>> > + break
>>
>> It is not acceptable to build anything custom that sends a request. You are removing all other kinds of features that I have mentioned above.
>>
>> To just “guess” the name of the zone is something that I would not consider good style.
>>
>> > +
>> > + if not zone_id:
>> > + raise DDNSRequestError(f"Could not find a Cloudflare Zone
>> > for '{self.hostname}'.")
>> > +
>> > + logger.info("CFv4: zone=%s id=%s", zone_name, zone_id)
>> > +
>> > + # --- get record ---
>> > + rec_url =
>> > f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records?type=A&name={self.hostname}"
>> > + try:
>> > + req = urllib.request.Request(rec_url, headers=headers,
>> > method="GET")
>> > + with urllib.request.urlopen(req, timeout=20) as r:
>> > + rec_data = json.loads(r.read().decode())
>> > + except Exception as e:
>> > + raise DDNSUpdateError(f"Failed to query Cloudflare DNS
>> > records API: {e}")
>> > +
>> > + if not rec_data.get("success"):
>> > + errs = rec_data.get("errors") or []
>> > + if any("Authentication error" in (e.get("message", "") or
>> > "") for e in errs):
>> > + raise DDNSAuthenticationError("Invalid API Token.")
>> > + raise DDNSUpdateError(f"Cloudflare API error finding
>> > record: {errs}")
>>
>> Same as above, hardcoded defaults like the timeout. Spaghetti code.
>>
>> > +
>> > + results = rec_data.get("result") or []
>> > + if not results:
>> > + raise DDNSRequestError(f"No A record found for
>> > '{self.hostname}' in zone '{zone_name}'.")
>> > +
>> > + record_id = results[0]["id"]
>> > + stored_ip = results[0]["content"]
>> > + logger.info("CFv4: record_id=%s stored_ip=%s", record_id, stored_ip)
>> > +
>> > + # --- compare IPs ---
>> > + current_ip = self.get_address("ipv4")
>> > + logger.info("CFv4: current_ip=%s vs stored_ip=%s",
>> > current_ip, stored_ip)
>> > + if current_ip == stored_ip:
>> > + logger.info("CFv4: no update needed")
>> > + return
>>
>> Why is this done using the API at all? We have functionality to use DNS for this which creates a lot less load than performing several API requests every couple of minutes.
>>
>> > +
>> > + # --- update ---
>> > + upd_url =
>> > f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records/{record_id}"
>> > + payload = {
>> > + "type": "A",
>> > + "name": self.hostname,
>> > + "content": current_ip,
>> > + "ttl": ttl,
>> > + "proxied": proxied,
>> > + }
>> > + logger.info("CFv4: updating %s -> %s (proxied=%s ttl=%s)",
>> > self.hostname, current_ip, proxied, ttl)
>> > +
>> > + try:
>> > + req = urllib.request.Request(
>> > + upd_url, data=json.dumps(payload).encode(),
>> > headers=headers, method="PUT"
>> > + )
>> > + with urllib.request.urlopen(req, timeout=20) as r:
>> > + upd = json.loads(r.read().decode())
>> > + except Exception as e:
>> > + raise DDNSUpdateError(f"Failed to send update request to
>> > Cloudflare: {e}”)
>> >
>>
>> Once again the same custom request logic.
>>
>> > + if not upd.get("success"):
>> > + raise DDNSUpdateError(f"Cloudflare API error on update:
>> > {upd.get('errors')}")
>> > +
>> > + logger.info("CFv4: update ok for %s -> %s", self.hostname, current_ip)
>> > + return
>> > +
>> >
>> > class DDNSProtocolDynDNS2(object):
>> > """
>> >
>>
>> I would really like to have support for Cloudflare in DDNS, but this is sadly not the way.
>>
>> Please study my comments thoroughly and then lets come up with a plan together how to implement this properly.
>>
>> Best,
>> -Michael
>>
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ddns: add Cloudflare (v4) provider using API token
2025-08-29 10:25 ` Michael Tremer
@ 2025-08-29 18:08 ` Bernhard Beroun
2025-08-31 14:38 ` Michael Tremer
0 siblings, 1 reply; 6+ messages in thread
From: Bernhard Beroun @ 2025-08-29 18:08 UTC (permalink / raw)
To: Michael Tremer; +Cc: Chris Anton, ddns
[-- Attachment #1: Type: text/plain, Size: 10747 bytes --]
Hello Michael,
as far as I know Cloudflare also supports IPv6 for DNS, but since I do not use any IPv6 in my home network, I haven't looked into that any further. I also didn't want to add anything I can't thoroughly test myself, therefore I decided to only add IPv4. I was hoping that it's possible to get the IPv4 part into the codebase and let others build upon that and add the missing IPv6 support. But I also totally understand if that's something you don't want to do.
Cheers,
Bernhard
Am Freitag, August 29, 2025 12:25 CEST, schrieb Michael Tremer <michael.tremer@ipfire.org>:
Hello Bernhard,
No you are not hijacking.
I don’t know why your patch did not receive any feedback. It must have fallen off the radar. I am not even the maintainer for DDNS…
Your patch looks much more like it because it is using and extending the code of DDNS.
However, it also only implements IPv4. Does Cloudflare not support IPv6 for DNS at all?
-Michael
> On 29 Aug 2025, at 11:20, Bernhard Beroun <bernhard@liftingcoder.com> wrote:
>
> Hi everybody,
> I don't want to hijack this mail thread here, but just wanted to let you know that I've also submitted a patch a few months ago. See https://lists.ipfire.org/ddns/19993045-2374-4537-96a2-5e2b1900a750@liftingcoder.com/T/#u
> Unfortunately I never received any feedback, so I assumed Cloudflare support wasn't desired. Again, sorry for hijacking this email chain here, just wanted to quickly drop it here, since it matches the topic.
> Have a nice day,
> Bernhard
>
> Am Freitag, August 29, 2025 11:50 CEST, schrieb Michael Tremer <michael.tremer@ipfire.org>:
>
>
>>
>> Hello Chris,
>>
>> You don’t need to submit any patches more than once. We will get back to you as soon as there is time.
>>
>> So let’s get into this...
>>
>> > On 29 Aug 2025, at 09:16, Chris Anton <chris.v.anton@gmail.com> wrote:
>> >
>> > From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github.com>
>>
>> I assume that you are faithinchaos21 and this code did not come from someone else?
>>
>> > Date: Wed, 27 Aug 2025 01:22:46 -0500
>> > Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token
>> > MIME-Version: 1.0
>> > Content-Type: text/plain; charset=UTF-8
>> > Content-Transfer-Encoding: 8bit
>> >
>> > This adds a provider “cloudflare.com-v4” that updates an A record
>> > via Cloudflare’s v4 API using a Bearer token. The token is accepted
>> > from either ‘token’ or legacy ‘password’ for UI compatibility.
>> >
>> > Tested on IPFire 2.29 / Core 196:
>> > - no-op if A already matches WAN IP
>> > - successful update when WAN IP changes
>> > - logs include CFv4 breadcrumbs for troubleshooting
>>
>> To make it short, this patch is sadly very far away from what is acceptable.
>>
>> Before we get into the actual implementation, there are many design issues here that simply cannot be accepted into DDNS:
>>
>> * This patch only adds support for IPv4. As far as I am aware, Cloudflare is capable of IPv6, too. Why would this be limited to IPv4 only?
>>
>> * You are not using the scaffolding and tools that DDNS is providing. A lot of code is being written with plain Python and throwing away all the features that DDNS provides (catching Exceptions, proxy support and many, many more…)
>>
>> * This all looks very AI-generated to me and is implemented on a green field. You are even importing all the modules that you need and ignore everything else from DDNS. Why not use this snippet as a standalone script then? There was no consideration for what code existed already.
>>
>> > Signed-off-by: Chris Anton <chris.v.anton@gmail.com>
>> > ---
>> > 563f089d0820bd61ad4aecac248d5cc1f2adfc81
>> > src/ddns/providers.py | 121 ++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 121 insertions(+)
>> >
>> > diff --git a/src/ddns/providers.py b/src/ddns/providers.py
>> > index 59f9665..df0f3a9 100644
>> > --- a/src/ddns/providers.py
>> > +++ b/src/ddns/providers.py
>> > @@ -341,6 +341,127 @@ def have_address(self, proto):
>> >
>> > return False
>> >
>> > +class DDNSProviderCloudflareV4(DDNSProvider):
>> > + """
>> > + Cloudflare v4 API using a Bearer Token.
>> > + Put the API Token in the 'token' OR 'password' field of the DDNS entry.
>> > + Optional in ddns.conf:
>> > + proxied = false|true (default false; keep false for WireGuard)
>> > + ttl = 1|60|120... (default 1 = 'automatic')
>> > + """
>> > + handle = "cloudflare.com-v4"
>> > + name = "Cloudflare (v4)"
>> > + website = "https://www.cloudflare.com/"
>> > + protocols = ("ipv4",)
>> > + supports_token_auth = True
>> > + holdoff_failure_days = 0
>> > +
>> > + def _bool(self, key, default=False):
>> > + v = str(self.get(key, default)).strip().lower()
>> > + return v in ("1", "true", "yes", "on")
>> > +
>>
>> This is a good example of a function which totally goes against the coding style of the rest of the code base. It uses str(), chains a lot of functions to gather to make the code look shorter, but very difficult to read.
>>
>> > + def update(self):
>> > + import json, urllib.request, urllib.error
>>
>> Just no. We don’t import anything further down the line. DDNS provides a toolkit of what to use and you should stay within it. If some functionally is missing, DDNS’ functionality should be extended so that other providers can re-use the same well-maintained and tested code base.
>>
>> > +
>> > + tok = self.get("token") or self.get("password")
>> > + if not tok:
>> > + raise DDNSConfigurationError("API Token (password/token)
>> > is missing.")
>> > +
>> > + proxied = self._bool("proxied", False)
>> > + try:
>> > + ttl = int(self.get("ttl", 1))
>> > + except Exception:
>> > + ttl = 1
>>
>> A TTL of one second is never a good idea.
>>
>> > +
>> > + headers = {
>> > + "Authorization": "Bearer {0}".format(tok),
>> > + "Content-Type": "application/json",
>> > + "User-Agent": "IPFireDDNSUpdater/CFv4",
>> > + }
>>
>> Since you are not using the internal request handler, you are creating your own headers and a completely nonsensical user agent. Why?
>>
>> > +
>> > + # --- find zone ---
>> > + parts = self.hostname.split(".")
>> > + if len(parts) < 2:
>> > + raise DDNSRequestError("Hostname '{0}' is not a valid
>> > domain.".format(self.hostname))
>> > +
>> > + zone_id = None
>> > + zone_name = None
>> > + for i in range(len(parts) - 1):
>> > + candidate = ".".join(parts[i:])
>> > + url =
>> > f"https://api.cloudflare.com/client/v4/zones?name={candidate}"
>> > + try:
>> > + req = urllib.request.Request(url, headers=headers,
>> > method="GET")
>> > + with urllib.request.urlopen(req, timeout=20) as r:
>> > + data = json.loads(r.read().decode())
>> > + except Exception as e:
>> > + raise DDNSUpdateError(f"Failed to query Cloudflare
>> > zones API: {e}")
>> > +
>> > + if data.get("success") and data.get("result"):
>> > + zone_id = data["result"][0]["id"]
>> > + zone_name = candidate
>> > + break
>>
>> It is not acceptable to build anything custom that sends a request. You are removing all other kinds of features that I have mentioned above.
>>
>> To just “guess” the name of the zone is something that I would not consider good style.
>>
>> > +
>> > + if not zone_id:
>> > + raise DDNSRequestError(f"Could not find a Cloudflare Zone
>> > for '{self.hostname}'.")
>> > +
>> > + logger.info("CFv4: zone=%s id=%s", zone_name, zone_id)
>> > +
>> > + # --- get record ---
>> > + rec_url =
>> > f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records?type=A&name={self.hostname}"
>> > + try:
>> > + req = urllib.request.Request(rec_url, headers=headers,
>> > method="GET")
>> > + with urllib.request.urlopen(req, timeout=20) as r:
>> > + rec_data = json.loads(r.read().decode())
>> > + except Exception as e:
>> > + raise DDNSUpdateError(f"Failed to query Cloudflare DNS
>> > records API: {e}")
>> > +
>> > + if not rec_data.get("success"):
>> > + errs = rec_data.get("errors") or []
>> > + if any("Authentication error" in (e.get("message", "") or
>> > "") for e in errs):
>> > + raise DDNSAuthenticationError("Invalid API Token.")
>> > + raise DDNSUpdateError(f"Cloudflare API error finding
>> > record: {errs}")
>>
>> Same as above, hardcoded defaults like the timeout. Spaghetti code.
>>
>> > +
>> > + results = rec_data.get("result") or []
>> > + if not results:
>> > + raise DDNSRequestError(f"No A record found for
>> > '{self.hostname}' in zone '{zone_name}'.")
>> > +
>> > + record_id = results[0]["id"]
>> > + stored_ip = results[0]["content"]
>> > + logger.info("CFv4: record_id=%s stored_ip=%s", record_id, stored_ip)
>> > +
>> > + # --- compare IPs ---
>> > + current_ip = self.get_address("ipv4")
>> > + logger.info("CFv4: current_ip=%s vs stored_ip=%s",
>> > current_ip, stored_ip)
>> > + if current_ip == stored_ip:
>> > + logger.info("CFv4: no update needed")
>> > + return
>>
>> Why is this done using the API at all? We have functionality to use DNS for this which creates a lot less load than performing several API requests every couple of minutes.
>>
>> > +
>> > + # --- update ---
>> > + upd_url =
>> > f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records/{record_id}"
>> > + payload = {
>> > + "type": "A",
>> > + "name": self.hostname,
>> > + "content": current_ip,
>> > + "ttl": ttl,
>> > + "proxied": proxied,
>> > + }
>> > + logger.info("CFv4: updating %s -> %s (proxied=%s ttl=%s)",
>> > self.hostname, current_ip, proxied, ttl)
>> > +
>> > + try:
>> > + req = urllib.request.Request(
>> > + upd_url, data=json.dumps(payload).encode(),
>> > headers=headers, method="PUT"
>> > + )
>> > + with urllib.request.urlopen(req, timeout=20) as r:
>> > + upd = json.loads(r.read().decode())
>> > + except Exception as e:
>> > + raise DDNSUpdateError(f"Failed to send update request to
>> > Cloudflare: {e}”)
>> >
>>
>> Once again the same custom request logic.
>>
>> > + if not upd.get("success"):
>> > + raise DDNSUpdateError(f"Cloudflare API error on update:
>> > {upd.get('errors')}")
>> > +
>> > + logger.info("CFv4: update ok for %s -> %s", self.hostname, current_ip)
>> > + return
>> > +
>> >
>> > class DDNSProtocolDynDNS2(object):
>> > """
>> >
>>
>> I would really like to have support for Cloudflare in DDNS, but this is sadly not the way.
>>
>> Please study my comments thoroughly and then lets come up with a plan together how to implement this properly.
>>
>> Best,
>> -Michael
>>
>>
>>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 13635 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ddns: add Cloudflare (v4) provider using API token
2025-08-29 18:08 ` Bernhard Beroun
@ 2025-08-31 14:38 ` Michael Tremer
0 siblings, 0 replies; 6+ messages in thread
From: Michael Tremer @ 2025-08-31 14:38 UTC (permalink / raw)
To: Bernhard Beroun; +Cc: Chris Anton, ddns
Hello,
Well, some of the providers are actually not (too) well tested. That is because I simply don’t have an account with all of them. So I don’t feel bad to add IPv6 here without having it tested, because if it does not work, we won’t loose anything.
I have studied your patch a little and I have taken some changes from it that should go into the core:
https://git.ipfire.org/?p=ddns.git;a=shortlog;h=refs/heads/json
This is basically that the request function can process JSON better without having to care about all the encoding/decoding stuff in the provider handler. I think that will make the code much more readable.
Then, it concerns me that we will have to send so many requests. That does not sound very efficient to me. And maybe we don’t have to do this, because there is another option:
https://developers.cloudflare.com/api/resources/dns/subresources/records/methods/update/
That way, we can just send the update as a single PUT operation and replace whatever has been there before. That should work. We are even able to send an A and AAAA record in one go. The only annoying part that remains is fetching the ID of the zone. I am not sure whether ddns should have the ability to cache data like this.
-Michael
> On 29 Aug 2025, at 19:08, Bernhard Beroun <bernhard@liftingcoder.com> wrote:
>
> Hello Michael,
>
> as far as I know Cloudflare also supports IPv6 for DNS, but since I do not use any IPv6 in my home network, I haven't looked into that any further. I also didn't want to add anything I can't thoroughly test myself, therefore I decided to only add IPv4. I was hoping that it's possible to get the IPv4 part into the codebase and let others build upon that and add the missing IPv6 support. But I also totally understand if that's something you don't want to do.
>
> Cheers,
> Bernhard
>
>
> Am Freitag, August 29, 2025 12:25 CEST, schrieb Michael Tremer <michael.tremer@ipfire.org>:
>
>
>>
>> Hello Bernhard,
>>
>> No you are not hijacking.
>>
>> I don’t know why your patch did not receive any feedback. It must have fallen off the radar. I am not even the maintainer for DDNS…
>>
>> Your patch looks much more like it because it is using and extending the code of DDNS.
>>
>> However, it also only implements IPv4. Does Cloudflare not support IPv6 for DNS at all?
>>
>> -Michael
>>
>> > On 29 Aug 2025, at 11:20, Bernhard Beroun <bernhard@liftingcoder.com> wrote:
>> >
>> > Hi everybody,
>> > I don't want to hijack this mail thread here, but just wanted to let you know that I've also submitted a patch a few months ago. See https://lists.ipfire.org/ddns/19993045-2374-4537-96a2-5e2b1900a750@liftingcoder.com/T/#u
>> > Unfortunately I never received any feedback, so I assumed Cloudflare support wasn't desired. Again, sorry for hijacking this email chain here, just wanted to quickly drop it here, since it matches the topic.
>> > Have a nice day,
>> > Bernhard
>> >
>> > Am Freitag, August 29, 2025 11:50 CEST, schrieb Michael Tremer <michael.tremer@ipfire.org>:
>> >
>> >
>> >>
>> >> Hello Chris,
>> >>
>> >> You don’t need to submit any patches more than once. We will get back to you as soon as there is time.
>> >>
>> >> So let’s get into this...
>> >>
>> >> > On 29 Aug 2025, at 09:16, Chris Anton <chris.v.anton@gmail.com> wrote:
>> >> >
>> >> > From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github.com>
>> >>
>> >> I assume that you are faithinchaos21 and this code did not come from someone else?
>> >>
>> >> > Date: Wed, 27 Aug 2025 01:22:46 -0500
>> >> > Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token
>> >> > MIME-Version: 1.0
>> >> > Content-Type: text/plain; charset=UTF-8
>> >> > Content-Transfer-Encoding: 8bit
>> >> >
>> >> > This adds a provider “cloudflare.com-v4” that updates an A record
>> >> > via Cloudflare’s v4 API using a Bearer token. The token is accepted
>> >> > from either ‘token’ or legacy ‘password’ for UI compatibility.
>> >> >
>> >> > Tested on IPFire 2.29 / Core 196:
>> >> > - no-op if A already matches WAN IP
>> >> > - successful update when WAN IP changes
>> >> > - logs include CFv4 breadcrumbs for troubleshooting
>> >>
>> >> To make it short, this patch is sadly very far away from what is acceptable.
>> >>
>> >> Before we get into the actual implementation, there are many design issues here that simply cannot be accepted into DDNS:
>> >>
>> >> * This patch only adds support for IPv4. As far as I am aware, Cloudflare is capable of IPv6, too. Why would this be limited to IPv4 only?
>> >>
>> >> * You are not using the scaffolding and tools that DDNS is providing. A lot of code is being written with plain Python and throwing away all the features that DDNS provides (catching Exceptions, proxy support and many, many more…)
>> >>
>> >> * This all looks very AI-generated to me and is implemented on a green field. You are even importing all the modules that you need and ignore everything else from DDNS. Why not use this snippet as a standalone script then? There was no consideration for what code existed already.
>> >>
>> >> > Signed-off-by: Chris Anton <chris.v.anton@gmail.com>
>> >> > ---
>> >> > 563f089d0820bd61ad4aecac248d5cc1f2adfc81
>> >> > src/ddns/providers.py | 121 ++++++++++++++++++++++++++++++++++++++++++
>> >> > 1 file changed, 121 insertions(+)
>> >> >
>> >> > diff --git a/src/ddns/providers.py b/src/ddns/providers.py
>> >> > index 59f9665..df0f3a9 100644
>> >> > --- a/src/ddns/providers.py
>> >> > +++ b/src/ddns/providers.py
>> >> > @@ -341,6 +341,127 @@ def have_address(self, proto):
>> >> >
>> >> > return False
>> >> >
>> >> > +class DDNSProviderCloudflareV4(DDNSProvider):
>> >> > + """
>> >> > + Cloudflare v4 API using a Bearer Token.
>> >> > + Put the API Token in the 'token' OR 'password' field of the DDNS entry.
>> >> > + Optional in ddns.conf:
>> >> > + proxied = false|true (default false; keep false for WireGuard)
>> >> > + ttl = 1|60|120... (default 1 = 'automatic')
>> >> > + """
>> >> > + handle = "cloudflare.com-v4"
>> >> > + name = "Cloudflare (v4)"
>> >> > + website = "https://www.cloudflare.com/"
>> >> > + protocols = ("ipv4",)
>> >> > + supports_token_auth = True
>> >> > + holdoff_failure_days = 0
>> >> > +
>> >> > + def _bool(self, key, default=False):
>> >> > + v = str(self.get(key, default)).strip().lower()
>> >> > + return v in ("1", "true", "yes", "on")
>> >> > +
>> >>
>> >> This is a good example of a function which totally goes against the coding style of the rest of the code base. It uses str(), chains a lot of functions to gather to make the code look shorter, but very difficult to read.
>> >>
>> >> > + def update(self):
>> >> > + import json, urllib.request, urllib.error
>> >>
>> >> Just no. We don’t import anything further down the line. DDNS provides a toolkit of what to use and you should stay within it. If some functionally is missing, DDNS’ functionality should be extended so that other providers can re-use the same well-maintained and tested code base.
>> >>
>> >> > +
>> >> > + tok = self.get("token") or self.get("password")
>> >> > + if not tok:
>> >> > + raise DDNSConfigurationError("API Token (password/token)
>> >> > is missing.")
>> >> > +
>> >> > + proxied = self._bool("proxied", False)
>> >> > + try:
>> >> > + ttl = int(self.get("ttl", 1))
>> >> > + except Exception:
>> >> > + ttl = 1
>> >>
>> >> A TTL of one second is never a good idea.
>> >>
>> >> > +
>> >> > + headers = {
>> >> > + "Authorization": "Bearer {0}".format(tok),
>> >> > + "Content-Type": "application/json",
>> >> > + "User-Agent": "IPFireDDNSUpdater/CFv4",
>> >> > + }
>> >>
>> >> Since you are not using the internal request handler, you are creating your own headers and a completely nonsensical user agent. Why?
>> >>
>> >> > +
>> >> > + # --- find zone ---
>> >> > + parts = self.hostname.split(".")
>> >> > + if len(parts) < 2:
>> >> > + raise DDNSRequestError("Hostname '{0}' is not a valid
>> >> > domain.".format(self.hostname))
>> >> > +
>> >> > + zone_id = None
>> >> > + zone_name = None
>> >> > + for i in range(len(parts) - 1):
>> >> > + candidate = ".".join(parts[i:])
>> >> > + url =
>> >> > f"https://api.cloudflare.com/client/v4/zones?name={candidate}"
>> >> > + try:
>> >> > + req = urllib.request.Request(url, headers=headers,
>> >> > method="GET")
>> >> > + with urllib.request.urlopen(req, timeout=20) as r:
>> >> > + data = json.loads(r.read().decode())
>> >> > + except Exception as e:
>> >> > + raise DDNSUpdateError(f"Failed to query Cloudflare
>> >> > zones API: {e}")
>> >> > +
>> >> > + if data.get("success") and data.get("result"):
>> >> > + zone_id = data["result"][0]["id"]
>> >> > + zone_name = candidate
>> >> > + break
>> >>
>> >> It is not acceptable to build anything custom that sends a request. You are removing all other kinds of features that I have mentioned above.
>> >>
>> >> To just “guess” the name of the zone is something that I would not consider good style.
>> >>
>> >> > +
>> >> > + if not zone_id:
>> >> > + raise DDNSRequestError(f"Could not find a Cloudflare Zone
>> >> > for '{self.hostname}'.")
>> >> > +
>> >> > + logger.info("CFv4: zone=%s id=%s", zone_name, zone_id)
>> >> > +
>> >> > + # --- get record ---
>> >> > + rec_url =
>> >> > f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records?type=A&name={self.hostname}"
>> >> > + try:
>> >> > + req = urllib.request.Request(rec_url, headers=headers,
>> >> > method="GET")
>> >> > + with urllib.request.urlopen(req, timeout=20) as r:
>> >> > + rec_data = json.loads(r.read().decode())
>> >> > + except Exception as e:
>> >> > + raise DDNSUpdateError(f"Failed to query Cloudflare DNS
>> >> > records API: {e}")
>> >> > +
>> >> > + if not rec_data.get("success"):
>> >> > + errs = rec_data.get("errors") or []
>> >> > + if any("Authentication error" in (e.get("message", "") or
>> >> > "") for e in errs):
>> >> > + raise DDNSAuthenticationError("Invalid API Token.")
>> >> > + raise DDNSUpdateError(f"Cloudflare API error finding
>> >> > record: {errs}")
>> >>
>> >> Same as above, hardcoded defaults like the timeout. Spaghetti code.
>> >>
>> >> > +
>> >> > + results = rec_data.get("result") or []
>> >> > + if not results:
>> >> > + raise DDNSRequestError(f"No A record found for
>> >> > '{self.hostname}' in zone '{zone_name}'.")
>> >> > +
>> >> > + record_id = results[0]["id"]
>> >> > + stored_ip = results[0]["content"]
>> >> > + logger.info("CFv4: record_id=%s stored_ip=%s", record_id, stored_ip)
>> >> > +
>> >> > + # --- compare IPs ---
>> >> > + current_ip = self.get_address("ipv4")
>> >> > + logger.info("CFv4: current_ip=%s vs stored_ip=%s",
>> >> > current_ip, stored_ip)
>> >> > + if current_ip == stored_ip:
>> >> > + logger.info("CFv4: no update needed")
>> >> > + return
>> >>
>> >> Why is this done using the API at all? We have functionality to use DNS for this which creates a lot less load than performing several API requests every couple of minutes.
>> >>
>> >> > +
>> >> > + # --- update ---
>> >> > + upd_url =
>> >> > f"https://api.cloudflare.com/client/v4/zones/{zone_id}/dns_records/{record_id}"
>> >> > + payload = {
>> >> > + "type": "A",
>> >> > + "name": self.hostname,
>> >> > + "content": current_ip,
>> >> > + "ttl": ttl,
>> >> > + "proxied": proxied,
>> >> > + }
>> >> > + logger.info("CFv4: updating %s -> %s (proxied=%s ttl=%s)",
>> >> > self.hostname, current_ip, proxied, ttl)
>> >> > +
>> >> > + try:
>> >> > + req = urllib.request.Request(
>> >> > + upd_url, data=json.dumps(payload).encode(),
>> >> > headers=headers, method="PUT"
>> >> > + )
>> >> > + with urllib.request.urlopen(req, timeout=20) as r:
>> >> > + upd = json.loads(r.read().decode())
>> >> > + except Exception as e:
>> >> > + raise DDNSUpdateError(f"Failed to send update request to
>> >> > Cloudflare: {e}”)
>> >> >
>> >>
>> >> Once again the same custom request logic.
>> >>
>> >> > + if not upd.get("success"):
>> >> > + raise DDNSUpdateError(f"Cloudflare API error on update:
>> >> > {upd.get('errors')}")
>> >> > +
>> >> > + logger.info("CFv4: update ok for %s -> %s", self.hostname, current_ip)
>> >> > + return
>> >> > +
>> >> >
>> >> > class DDNSProtocolDynDNS2(object):
>> >> > """
>> >> >
>> >>
>> >> I would really like to have support for Cloudflare in DDNS, but this is sadly not the way.
>> >>
>> >> Please study my comments thoroughly and then lets come up with a plan together how to implement this properly.
>> >>
>> >> Best,
>> >> -Michael
>> >>
>> >>
>> >>
>> >
>> >
>> >
>> >
>>
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-31 14:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-29 8:16 [PATCH] ddns: add Cloudflare (v4) provider using API token Chris Anton
2025-08-29 9:50 ` Michael Tremer
2025-08-29 10:20 ` Bernhard Beroun
2025-08-29 10:25 ` Michael Tremer
2025-08-29 18:08 ` Bernhard Beroun
2025-08-31 14:38 ` Michael Tremer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox