ddns: add Cloudflare (v4) provider using API token

Message ID CAJ2Zu3ChjmWhHTFS907DaHT+OHYOb+_wqz8g4UVJza3V3KxNaA@mail.gmail.com
State New
Headers
Series ddns: add Cloudflare (v4) provider using API token |

Commit Message

Chris Anton 29 Aug 2025, 8:16 a.m. UTC
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(+)

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):
  """
  

Comments

Michael Tremer 29 Aug 2025, 9:50 a.m. UTC | #1
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
  
Bernhard Beroun 29 Aug 2025, 10:20 a.m. UTC | #2
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
  
Michael Tremer 29 Aug 2025, 10:25 a.m. UTC | #3
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
>> 
>> 
>>  
> 
> 
> 
>
  
Bernhard Beroun 29 Aug 2025, 6:08 p.m. UTC | #4
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
>> 
>> 
>> 



>
  

Patch

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(),