From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.haj.ipfire.org (localhost [IPv6:::1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4cG0Zz4wRZz2ynh for ; Mon, 01 Sep 2025 20:19:23 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [IPv6:2001:678:b28::25]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mail01.haj.ipfire.org", Issuer "R13" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4cG0Zy5mbLz2xP7 for ; Mon, 01 Sep 2025 20:19:22 +0000 (UTC) Received: from hosted.mailcow.de (hosted.mailcow.de [5.1.76.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mail01.ipfire.org (Postfix) with ESMTPS id 4cG0Zv5nTHzD5 for ; Mon, 01 Sep 2025 20:19:19 +0000 (UTC) Authentication-Results: mail01.ipfire.org; dkim=pass header.d=liftingcoder.com header.s=default header.b=FoRQKeXl; dmarc=none; spf=none (mail01.ipfire.org: domain of bernhard@liftingcoder.com has no SPF policy when checking 5.1.76.202) smtp.mailfrom=bernhard@liftingcoder.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=lists.ipfire.org; s=202003rsa; t=1756757960; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WTg4CmkUvs3A8xzny4giBz+YpMk3RAONXhJOkA3mc5I=; b=RLEP7KR/tdXNFGMmjtAotd9QSmpGq9aZRXEY2+QIgLfUoXour2sT69bVeXz3u5aHjlhsVy mYlzglxslesky5ibCTu81Iofe4UYShs15XxviX6evDRPEb5XRgcTTTI0TU4nmaL1Lj5t5o ORRyJREG36DaWrd12FCtS+NlEYi0xv/6Myve9IWS1D46ZC1PidreL8bGg/GrdRaTPGZrJa lTingQncEi3UN5+eAkrj3lGjwzXtR8TF8tO+vbNi53SI9ZeXKNR6veaOe7JnIAkI3y7VGW OGNmdlNPQEuKqlNlotpmnemcMIhvI8/I52btm5D4LkUkzbXB9EXwlk/c2x9dHg== ARC-Authentication-Results: i=1; mail01.ipfire.org; dkim=pass header.d=liftingcoder.com header.s=default header.b=FoRQKeXl; dmarc=none; spf=none (mail01.ipfire.org: domain of bernhard@liftingcoder.com has no SPF policy when checking 5.1.76.202) smtp.mailfrom=bernhard@liftingcoder.com ARC-Seal: i=1; s=202003rsa; d=lists.ipfire.org; t=1756757960; a=rsa-sha256; cv=none; b=e9ppBlmZ4xt1fdCyGeCmz4Ypj4fG7oI0wyUAweZh+X9+O9nckUYTLXMo66CJ7h7ZbiiLtU eefaOcpbHfH8iucUbS76f8yWYkp9XoJ15ncwFskcOQHk4tUrsRKiTH6syuxId07Y0fLT+/ ih4M2f6GRhUp3TLV1/gCFdxIs9LP+pVjNyarECbf32rmDgqOq464asUn2/dd6oMQAJpClX AGFNmxy8HyYRL+8YEbVddK3gH2cMMemin8dYjlGztZYRhS4vk3+DAP46wvFZVnpLkVPNkq BKTOvvyi1jtya5A6H0x4ZUEF9nOJ12fcRQrrq0l8h0WXxv0CtlPD7FhiwcaLXQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=liftingcoder.com; s=default; t=1756757958; h=from:subject:date:message-id:to:cc:mime-version:content-type: in-reply-to:references; bh=WTg4CmkUvs3A8xzny4giBz+YpMk3RAONXhJOkA3mc5I=; b=FoRQKeXlJvK5XJNx+NkRDIDdH8Ssn+YVw6+g3VRoqe5Jj+K9FLzIaiWZImz3TDcENT8H/M vw9kmWzHgk0FcgAlqRABGYAw9NL27/5uZWEN61jHxVDipyn5VhfQIsE/Q4pWd9VsejARl0 g5igGmuKLk+4Y+64dE0eyniQMOht7Fc= Received: from hosted.mailcow.de (hosted.mailcow.de [5.1.76.202]) (Authenticated sender: bernhard@beroun.io) by hosted.mailcow.de (Postcow) with ESMTPA id B4FC65C00E3; Mon, 1 Sep 2025 22:19:18 +0200 (CEST) From: "Bernhard Beroun" In-Reply-To: Content-Type: multipart/alternative; boundary="----=_=-_OpenGroupware_org_NGMime-1307550-1756757958.588736-0------" References: <2139f7-68b17f00-3-48fa3400@237050521> <32702E44-5264-4D75-8645-F297E4AF920B@ipfire.org> <2fd1aa-68b1ec80-2b-633f1b80@145901099> Date: Mon, 01 Sep 2025 22:19:18 +0200 Cc: "Chris Anton" , ddns@lists.ipfire.org To: "Michael Tremer" Precedence: list List-Id: List-Subscribe: , List-Unsubscribe: , List-Post: List-Help: Sender: Mail-Followup-To: MIME-Version: 1.0 Message-ID: <13f39e-68b60000-1-59b1d300@94062361> Subject: =?utf-8?q?Re=3A?= [PATCH] =?utf-8?q?ddns=3A?= add Cloudflare (v4) provider using API token X-Rspamd-Server: mail01.haj.ipfire.org X-Rspamd-Queue-Id: 4cG0Zv5nTHzD5 X-Rspamd-Action: no action X-Spamd-Result: default: False [-2.38 / 11.00]; R_DKIM_ALLOW(-1.45)[liftingcoder.com:s=default]; SUBJ_EXCESS_QP(1.20)[]; NEURAL_HAM(-0.98)[-0.978]; DKIM_REPUTATION(-0.73)[-0.73317505490232]; BAYES_HAM(-0.71)[83.46%]; MID_RHS_NOT_FQDN(0.50)[]; RCVD_IN_DNSWL_LOW(-0.20)[5.1.76.202:received,5.1.76.202:from]; RCVD_NO_TLS_LAST(0.10)[]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; MX_GOOD(-0.01)[]; IP_REPUTATION_HAM(-0.00)[asn: 34549(0.00), country: DE(-0.00), ip: 5.1.76.202(0.00)]; R_SPF_NA(0.00)[no SPF record]; DMARC_NA(0.00)[liftingcoder.com]; ARC_NA(0.00)[]; FUZZY_RATELIMITED(0.00)[rspamd.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; FREEMAIL_CC(0.00)[gmail.com,lists.ipfire.org]; MISSING_XM_UA(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; TO_MATCH_ENVRCPT_SOME(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; DKIM_TRACE(0.00)[liftingcoder.com:+]; TAGGED_RCPT(0.00)[]; ASN(0.00)[asn:34549, ipnet:5.1.76.0/24, country:DE]; RCVD_VIA_SMTP_AUTH(0.00)[]; TO_DN_SOME(0.00)[]; RCVD_COUNT_ONE(0.00)[1]; ARC_SIGNED(0.00)[lists.ipfire.org:s=202003rsa:i=1] ------=_=-_OpenGroupware_org_NGMime-1307550-1756757958.588736-0------ Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Length: 14383 Hello Michael, thanks for adding the changes to the core - I'll adapt the changeset to= make use of it. What I do not fully understand though is, what the actual benefit of us= ing the PUT (https://developers.cloudflare.com/api/resources/dns/subres= ources/records/methods/update/) endpoint compared to the PATCH (https:/= /developers.cloudflare.com/api/resources/dns/subresources/records/metho= ds/edit/) endpoint, which currently using, is.=C2=A0 As far as I've seen, both the PUT and the PATCH endpoint require a zone= =5Fid and a dns=5Frecord=5Fid, which both need to be fetched upfront. T= o me both endpoints seems to be pretty much equal in terms of necessary= roundtrips? Or am I missing something (obvious) here? Thanks a lot! Cheers Bernhard Am Sonntag, August 31, 2025 16:38 CEST, schrieb Michael Tremer : =C2=A0 Hello, Well, some of the providers are actually not (too) well tested. That is= because I simply don=E2=80=99t have an account with all of them. So I = don=E2=80=99t feel bad to add IPv6 here without having it tested, becau= se if it does not work, we won=E2=80=99t loose anything. I have studied your patch a little and I have taken some changes from i= t that should go into the core: https://git.ipfire.org/?p=3Dddns.git;a=3Dshortlog;h=3Drefs/heads/json This is basically that the request function can process JSON better wit= hout having to care about all the encoding/decoding stuff in the provid= er 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 d= oes not sound very efficient to me. And maybe we don=E2=80=99t have to = do this, because there is another option: https://developers.cloudflare.com/api/resources/dns/subresources/record= s/methods/update/ That way, we can just send the update as a single PUT operation and rep= lace 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 re= mains 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 = wrote: >=C2=A0 > Hello Michael, >=C2=A0 > as far as I know Cloudflare also supports IPv6 for DNS, but since I d= o not use any IPv6 in my home network, I haven't looked into that any f= urther. I also didn't want to add anything I can't thoroughly test myse= lf, therefore I decided to only add IPv4. I was hoping that it's possib= le to get the IPv4 part into the codebase and let others build upon tha= t and add the missing IPv6 support. But I also totally understand if th= at's something you don't want to do. >=C2=A0 > Cheers, > Bernhard >=C2=A0 >=C2=A0 > Am Freitag, August 29, 2025 12:25 CEST, schrieb Michael Tremer : >=C2=A0 >=C2=A0 >>=C2=A0 >> Hello Bernhard, >>=C2=A0 >> No you are not hijacking. >>=C2=A0 >> I don=E2=80=99t know why your patch did not receive any feedback. It= must have fallen off the radar. I am not even the maintainer for DDNS=E2= =80=A6 >>=C2=A0 >> Your patch looks much more like it because it is using and extending= the code of DDNS. >>=C2=A0 >> However, it also only implements IPv4. Does Cloudflare not support I= Pv6 for DNS at all? >>=C2=A0 >> -Michael >>=C2=A0 >> > On 29 Aug 2025, at 11:20, Bernhard Beroun wrote: >> >=C2=A0 >> > Hi everybody, >> > I don't want to hijack this mail thread here, but just wanted to l= et you know that I've also submitted a patch a few months ago. See http= s://lists.ipfire.org/ddns/19993045-2374-4537-96a2-5e2b1900a750@liftingc= oder.com/T/#u >> > Unfortunately I never received any feedback, so I assumed Cloudfla= re 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 >> >=C2=A0 >> > Am Freitag, August 29, 2025 11:50 CEST, schrieb Michael Tremer : >> >=C2=A0 >> >=C2=A0 >> >>=C2=A0 >> >> Hello Chris, >> >>=C2=A0 >> >> You don=E2=80=99t need to submit any patches more than once. We w= ill get back to you as soon as there is time. >> >>=C2=A0 >> >> So let=E2=80=99s get into this... >> >>=C2=A0 >> >> > On 29 Aug 2025, at 09:16, Chris Anton = wrote: >> >> >=C2=A0 >> >> > From: faithinchaos21 <45313722+faithinchaos21@users.noreply.git= hub.com> >> >>=C2=A0 >> >> I assume that you are faithinchaos21 and this code did not come f= rom someone else? >> >>=C2=A0 >> >> > Date: Wed, 27 Aug 2025 01:22:46 -0500 >> >> > Subject: [PATCH] ddns: add Cloudflare (v4) provider using API t= oken >> >> > MIME-Version: 1.0 >> >> > Content-Type: text/plain; charset=3DUTF-8 >> >> > Content-Transfer-Encoding: 8bit >> >> >=C2=A0 >> >> > This adds a provider =E2=80=9Ccloudflare.com-v4=E2=80=9D that u= pdates an A record >> >> > via Cloudflare=E2=80=99s v4 API using a Bearer token. The token= is accepted >> >> > from either =E2=80=98token=E2=80=99 or legacy =E2=80=98password= =E2=80=99 for UI compatibility. >> >> >=C2=A0 >> >> > 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 >> >>=C2=A0 >> >> To make it short, this patch is sadly very far away from what is = acceptable. >> >>=C2=A0 >> >> Before we get into the actual implementation, there are many desi= gn issues here that simply cannot be accepted into DDNS: >> >>=C2=A0 >> >> * This patch only adds support for IPv4. As far as I am aware, Cl= oudflare is capable of IPv6, too. Why would this be limited to IPv4 onl= y? >> >>=C2=A0 >> >> * You are not using the scaffolding and tools that DDNS is provid= ing. A lot of code is being written with plain Python and throwing away= all the features that DDNS provides (catching Exceptions, proxy suppor= t and many, many more=E2=80=A6) >> >>=C2=A0 >> >> * 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 i= gnore everything else from DDNS. Why not use this snippet as a standalo= ne script then? There was no consideration for what code existed alread= y. >> >>=C2=A0 >> >> > Signed-off-by: Chris Anton >> >> > --- >> >> > 563f089d0820bd61ad4aecac248d5cc1f2adfc81 >> >> > src/ddns/providers.py | 121 +++++++++++++++++++++++++++++++++++= +++++++ >> >> > 1 file changed, 121 insertions(+) >> >> >=C2=A0 >> >> > 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=5Faddress(self, proto): >> >> >=C2=A0 >> >> > return False >> >> >=C2=A0 >> >> > +class DDNSProviderCloudflareV4(DDNSProvider): >> >> > + """ >> >> > + Cloudflare v4 API using a Bearer Token. >> >> > + Put the API Token in the 'token' OR 'password' field of the D= DNS entry. >> >> > + Optional in ddns.conf: >> >> > + proxied =3D false|true (default false; keep false for WireGua= rd) >> >> > + ttl =3D 1|60|120... (default 1 =3D 'automatic') >> >> > + """ >> >> > + handle =3D "cloudflare.com-v4" >> >> > + name =3D "Cloudflare (v4)" >> >> > + website =3D "https://www.cloudflare.com/" >> >> > + protocols =3D ("ipv4",) >> >> > + supports=5Ftoken=5Fauth =3D True >> >> > + holdoff=5Ffailure=5Fdays =3D 0 >> >> > + >> >> > + def =5Fbool(self, key, default=3DFalse): >> >> > + v =3D str(self.get(key, default)).strip().lower() >> >> > + return v in ("1", "true", "yes", "on") >> >> > + >> >>=C2=A0 >> >> This is a good example of a function which totally goes against t= he coding style of the rest of the code base. It uses str(), chains a l= ot of functions to gather to make the code look shorter, but very diffi= cult to read. >> >>=C2=A0 >> >> > + def update(self): >> >> > + import json, urllib.request, urllib.error >> >>=C2=A0 >> >> Just no. We don=E2=80=99t import anything further down the line. = DDNS provides a toolkit of what to use and you should stay within it. I= f some functionally is missing, DDNS=E2=80=99 functionality should be e= xtended so that other providers can re-use the same well-maintained and= tested code base. >> >>=C2=A0 >> >> > + >> >> > + tok =3D self.get("token") or self.get("password") >> >> > + if not tok: >> >> > + raise DDNSConfigurationError("API Token (password/token) >> >> > is missing.") >> >> > + >> >> > + proxied =3D self.=5Fbool("proxied", False) >> >> > + try: >> >> > + ttl =3D int(self.get("ttl", 1)) >> >> > + except Exception: >> >> > + ttl =3D 1 >> >>=C2=A0 >> >> A TTL of one second is never a good idea. >> >>=C2=A0 >> >> > + >> >> > + headers =3D { >> >> > + "Authorization": "Bearer {0}".format(tok), >> >> > + "Content-Type": "application/json", >> >> > + "User-Agent": "IPFireDDNSUpdater/CFv4", >> >> > + } >> >>=C2=A0 >> >> Since you are not using the internal request handler, you are cre= ating your own headers and a completely nonsensical user agent. Why? >> >>=C2=A0 >> >> > + >> >> > + # --- find zone --- >> >> > + parts =3D self.hostname.split(".") >> >> > + if len(parts) < 2: >> >> > + raise DDNSRequestError("Hostname '{0}' is not a valid >> >> > domain.".format(self.hostname)) >> >> > + >> >> > + zone=5Fid =3D None >> >> > + zone=5Fname =3D None >> >> > + for i in range(len(parts) - 1): >> >> > + candidate =3D ".".join(parts[i:]) >> >> > + url =3D >> >> > f"https://api.cloudflare.com/client/v4/zones?name=3D{candidate}= " >> >> > + try: >> >> > + req =3D urllib.request.Request(url, headers=3Dheaders, >> >> > method=3D"GET") >> >> > + with urllib.request.urlopen(req, timeout=3D20) as r: >> >> > + data =3D 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=5Fid =3D data["result"][0]["id"] >> >> > + zone=5Fname =3D candidate >> >> > + break >> >>=C2=A0 >> >> It is not acceptable to build anything custom that sends a reques= t. You are removing all other kinds of features that I have mentioned a= bove. >> >>=C2=A0 >> >> To just =E2=80=9Cguess=E2=80=9D the name of the zone is something= that I would not consider good style. >> >>=C2=A0 >> >> > + >> >> > + if not zone=5Fid: >> >> > + raise DDNSRequestError(f"Could not find a Cloudflare Zone >> >> > for '{self.hostname}'.") >> >> > + >> >> > + logger.info("CFv4: zone=3D%s id=3D%s", zone=5Fname, zone=5Fid= ) >> >> > + >> >> > + # --- get record --- >> >> > + rec=5Furl =3D >> >> > f"https://api.cloudflare.com/client/v4/zones/{zone=5Fid}/dns=5F= records?type=3DA&name=3D{self.hostname}" >> >> > + try: >> >> > + req =3D urllib.request.Request(rec=5Furl, headers=3Dheaders, >> >> > method=3D"GET") >> >> > + with urllib.request.urlopen(req, timeout=3D20) as r: >> >> > + rec=5Fdata =3D json.loads(r.read().decode()) >> >> > + except Exception as e: >> >> > + raise DDNSUpdateError(f"Failed to query Cloudflare DNS >> >> > records API: {e}") >> >> > + >> >> > + if not rec=5Fdata.get("success"): >> >> > + errs =3D rec=5Fdata.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}") >> >>=C2=A0 >> >> Same as above, hardcoded defaults like the timeout. Spaghetti cod= e. >> >>=C2=A0 >> >> > + >> >> > + results =3D rec=5Fdata.get("result") or [] >> >> > + if not results: >> >> > + raise DDNSRequestError(f"No A record found for >> >> > '{self.hostname}' in zone '{zone=5Fname}'.") >> >> > + >> >> > + record=5Fid =3D results[0]["id"] >> >> > + stored=5Fip =3D results[0]["content"] >> >> > + logger.info("CFv4: record=5Fid=3D%s stored=5Fip=3D%s", record= =5Fid, stored=5Fip) >> >> > + >> >> > + # --- compare IPs --- >> >> > + current=5Fip =3D self.get=5Faddress("ipv4") >> >> > + logger.info("CFv4: current=5Fip=3D%s vs stored=5Fip=3D%s", >> >> > current=5Fip, stored=5Fip) >> >> > + if current=5Fip =3D=3D stored=5Fip: >> >> > + logger.info("CFv4: no update needed") >> >> > + return >> >>=C2=A0 >> >> Why is this done using the API at all? We have functionality to u= se DNS for this which creates a lot less load than performing several A= PI requests every couple of minutes. >> >>=C2=A0 >> >> > + >> >> > + # --- update --- >> >> > + upd=5Furl =3D >> >> > f"https://api.cloudflare.com/client/v4/zones/{zone=5Fid}/dns=5F= records/{record=5Fid}" >> >> > + payload =3D { >> >> > + "type": "A", >> >> > + "name": self.hostname, >> >> > + "content": current=5Fip, >> >> > + "ttl": ttl, >> >> > + "proxied": proxied, >> >> > + } >> >> > + logger.info("CFv4: updating %s -> %s (proxied=3D%s ttl=3D%s)"= , >> >> > self.hostname, current=5Fip, proxied, ttl) >> >> > + >> >> > + try: >> >> > + req =3D urllib.request.Request( >> >> > + upd=5Furl, data=3Djson.dumps(payload).encode(), >> >> > headers=3Dheaders, method=3D"PUT" >> >> > + ) >> >> > + with urllib.request.urlopen(req, timeout=3D20) as r: >> >> > + upd =3D json.loads(r.read().decode()) >> >> > + except Exception as e: >> >> > + raise DDNSUpdateError(f"Failed to send update request to >> >> > Cloudflare: {e}=E2=80=9D) >> >> >=C2=A0 >> >>=C2=A0 >> >> Once again the same custom request logic. >> >>=C2=A0 >> >> > + 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, cu= rrent=5Fip) >> >> > + return >> >> > + >> >> >=C2=A0 >> >> > class DDNSProtocolDynDNS2(object): >> >> > """ >> >> >=C2=A0 >> >>=C2=A0 >> >> I would really like to have support for Cloudflare in DDNS, but t= his is sadly not the way. >> >>=C2=A0 >> >> Please study my comments thoroughly and then lets come up with a = plan together how to implement this properly. >> >>=C2=A0 >> >> Best, >> >> -Michael >> >>=C2=A0 >> >>=C2=A0 >> >>=C2=A0 >> >=C2=A0 >> >=C2=A0 >> >=C2=A0 >> >=C2=A0 >>=C2=A0 >>=C2=A0 >>=C2=A0 >=C2=A0 >=C2=A0 >=C2=A0 >=C2=A0 =C2=A0 =C2=A0 ------=_=-_OpenGroupware_org_NGMime-1307550-1756757958.588736-0------ Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Length: 19632

Hello Michael,

thanks for adding the changes to the cor= e - I'll adapt the changeset to make use of it.

What I do not fu= lly understand though is, what the actual benefit of using the PUT (htt= ps://developers.cloudflare.com/api/resources/dns/subresources/records/m= ethods/update/) endpoint compared to the PATCH (https://developers.clou= dflare.com/api/resources/dns/subresources/records/methods/edit/) endpoi= nt, which currently using, is. 

As far as I've seen, both t= he PUT and the PATCH endpoint require a zone=5Fid and a dns=5Frecord=5F= id, which both need to be fetched upfront. To me both endpoints seems t= o be pretty much equal in terms of necessary roundtrips? Or am I missin= g something (obvious) here?

Thanks a lot!

Cheers
Bernh= ard

Am Sonntag, August 31, 2025 16:38 CEST, schrieb Michael Trem= er <michael.tremer@ipfire.org>:

 

H= ello,

Well, some of the providers are actually not (too) well te= sted. That is because I simply don=E2=80=99t have an account with all o= f them. So I don=E2=80=99t feel bad to add IPv6 here without having it = tested, because if it does not work, we won=E2=80=99t 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=3D= ddns.git;a=3Dshortlog;h=3Drefs/heads/json

This is basically that= the request function can process JSON better without having to care ab= out all the encoding/decoding stuff in the provider handler. I think th= at will make the code much more readable.

Then, it concerns me t= hat we will have to send so many requests. That does not sound very eff= icient to me. And maybe we don=E2=80=99t have to do this, because there= is another option:

https://developers.cloudflare.com/api/resour= ces/dns/subresources/records/methods/update/

That way, we can ju= st send the update as a single PUT operation and replace whatever has b= een there before. That should work. We are even able to send an A and A= AAA record in one go. The only annoying part that remains is fetching t= he ID of the zone. I am not sure whether ddns should have the ability t= o 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 Cl= oudflare 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 decid= ed to only add IPv4. I was hoping that it's possible to get the IPv4 pa= rt 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 do= n't want to do.

> Cheers,
> Bernhard
>&= nbsp;

> Am Freitag, August 29, 2025 12:25 CEST, sch= rieb Michael Tremer <michael.tremer@ipfire.org>:

>> 
>> Hello Bernhard,
>>&n= bsp;
>> No you are not hijacking.
>> 
>>= ; I don=E2=80=99t know why your patch did not receive any feedback. It = must have fallen off the radar. I am not even the maintainer for DDNS=E2= =80=A6
>> 
>> Your patch looks much more like it= because it is using and extending the code of DDNS.
>> <= br>>> However, it also only implements IPv4. Does Cloudflare not = support IPv6 for DNS at all?
>> 
>> -Michael
= >> 
>> > On 29 Aug 2025, at 11:20, Bernhard Bero= un <bernhard@liftingcoder.com> wrote:
>> > 
&= gt;> > Hi everybody,
>> > I don't want to hijack this= mail thread here, but just wanted to let you know that I've also submi= tted a patch a few months ago. See https://lists.ipfire.org/ddns/199930= 45-2374-4537-96a2-5e2b1900a750@liftingcoder.com/T/#u
>> > U= nfortunately I never received any feedback, so I assumed Cloudflare sup= port 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
>> >&= nbsp;
>> > Am Freitag, August 29, 2025 11:50 CEST, schrieb = Michael Tremer <michael.tremer@ipfire.org>:
>> > = ;
>> > 
>> >> 
>> >>= ; Hello Chris,
>> >> 
>> >> You don=E2= =80=99t need to submit any patches more than once. We will get back to = you as soon as there is time.
>> >> 
>> &g= t;> So let=E2=80=99s 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 so= meone else?
>> >> 
>> >> > Date: = Wed, 27 Aug 2025 01:22:46 -0500
>> >> > Subject: [PAT= CH] ddns: add Cloudflare (v4) provider using API token
>> >= > > MIME-Version: 1.0
>> >> > Content-Type: tex= t/plain; charset=3DUTF-8
>> >> > Content-Transfer-Enc= oding: 8bit
>> >> > 
>> >> > T= his adds a provider =E2=80=9Ccloudflare.com-v4=E2=80=9D that updates an= A record
>> >> > via Cloudflare=E2=80=99s v4 API usi= ng a Bearer token. The token is accepted
>> >> > from= either =E2=80=98token=E2=80=99 or legacy =E2=80=98password=E2=80=99 fo= r 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 CFv= 4 breadcrumbs for troubleshooting
>> >> 
>>= ; >> To make it short, this patch is sadly very far away from wha= t is acceptable.
>> >> 
>> >> Before= we get into the actual implementation, there are many design issues he= re that simply cannot be accepted into DDNS:
>> >> =
>> >> * This patch only adds support for IPv4. As far a= s I am aware, Cloudflare is capable of IPv6, too. Why would this be lim= ited to IPv4 only?
>> >> 
>> >> * Yo= u 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 f= eatures that DDNS provides (catching Exceptions, proxy support and many= , many more=E2=80=A6)
>> >> 
>> >> *= This all looks very AI-generated to me and is implemented on a green f= ield. You are even importing all the modules that you need and ignore e= verything else from DDNS. Why not use this snippet as a standalone scri= pt then? There was no consideration for what code existed already.
&= gt;> >> 
>> >> > Signed-off-by: Chris A= nton <chris.v.anton@gmail.com>
>> >> > ---
&= gt;> >> > 563f089d0820bd61ad4aecac248d5cc1f2adfc81
>&= gt; >> > src/ddns/providers.py | 121 +++++++++++++++++++++++++= +++++++++++++++++
>> >> > 1 file changed, 121 inserti= ons(+)
>> >> > 
>> >> > diff -= -git a/src/ddns/providers.py b/src/ddns/providers.py
>> >&g= t; > index 59f9665..df0f3a9 100644
>> >> > --- a/s= rc/ddns/providers.py
>> >> > +++ b/src/ddns/providers= .py
>> >> > @@ -341,6 +341,127 @@ def have=5Faddress(= self, proto):
>> >> > 
>> >> >= return False
>> >> > 
>> >> >= +class DDNSProviderCloudflareV4(DDNSProvider):
>> >> &g= t; + """
>> >> > + Cloudflare v4 API using a Bearer T= oken.
>> >> > + Put the API Token in the 'token' OR '= password' field of the DDNS entry.
>> >> > + Optional= in ddns.conf:
>> >> > + proxied =3D false|true (defa= ult false; keep false for WireGuard)
>> >> > + ttl =3D= 1|60|120... (default 1 =3D 'automatic')
>> >> > + ""= "
>> >> > + handle =3D "cloudflare.com-v4"
>>= ; >> > + name =3D "Cloudflare (v4)"
>> >> > = + website =3D "https://www.cloudflare.com/"
>> >> > += protocols =3D ("ipv4",)
>> >> > + supports=5Ftoken=5F= auth =3D True
>> >> > + holdoff=5Ffailure=5Fdays =3D = 0
>> >> > +
>> >> > + def =5Fbool(s= elf, key, default=3DFalse):
>> >> > + v =3D str(self.= get(key, default)).strip().lower()
>> >> > + return v= in ("1", "true", "yes", "on")
>> >> > +
>> = >> 
>> >> This is a good example of a functio= n which totally goes against the coding style of the rest of the code b= ase. It uses str(), chains a lot of functions to gather to make the cod= e look shorter, but very difficult to read.
>> >> <= br>>> >> > + def update(self):
>> >> >= + import json, urllib.request, urllib.error
>> >> =
>> >> Just no. We don=E2=80=99t import anything further= down the line. DDNS provides a toolkit of what to use and you should s= tay within it. If some functionally is missing, DDNS=E2=80=99 functiona= lity should be extended so that other providers can re-use the same wel= l-maintained and tested code base.
>> >> 
>&g= t; >> > +
>> >> > + tok =3D self.get("token"= ) or self.get("password")
>> >> > + if not tok:
&g= t;> >> > + raise DDNSConfigurationError("API Token (passwor= d/token)
>> >> > is missing.")
>> >> &= gt; +
>> >> > + proxied =3D self.=5Fbool("proxied", F= alse)
>> >> > + try:
>> >> > + ttl = =3D int(self.get("ttl", 1))
>> >> > + except Exceptio= n:
>> >> > + ttl =3D 1
>> >> 
= >> >> A TTL of one second is never a good idea.
>>= >> 
>> >> > +
>> >> > += headers =3D {
>> >> > + "Authorization": "Bearer {0}= ".format(tok),
>> >> > + "Content-Type": "application= /json",
>> >> > + "User-Agent": "IPFireDDNSUpdater/CF= v4",
>> >> > + }
>> >> 
>&g= t; >> Since you are not using the internal request handler, you a= re creating your own headers and a completely nonsensical user agent. W= hy?
>> >> 
>> >> > +
>> = >> > + # --- find zone ---
>> >> > + parts =3D= self.hostname.split(".")
>> >> > + if len(parts) <= ; 2:
>> >> > + raise DDNSRequestError("Hostname '{0}'= is not a valid
>> >> > domain.".format(self.hostname= ))
>> >> > +
>> >> > + zone=5Fid =3D= None
>> >> > + zone=5Fname =3D None
>> >= > > + for i in range(len(parts) - 1):
>> >> > += candidate =3D ".".join(parts[i:])
>> >> > + url =3D<= br>>> >> > f"https://api.cloudflare.com/client/v4/zones?= name=3D{candidate}"
>> >> > + try:
>> >&g= t; > + req =3D urllib.request.Request(url, headers=3Dheaders,
>= ;> >> > method=3D"GET")
>> >> > + with ur= llib.request.urlopen(req, timeout=3D20) as r:
>> >> >= + data =3D json.loads(r.read().decode())
>> >> > + e= xcept Exception as e:
>> >> > + raise DDNSUpdateError= (f"Failed to query Cloudflare
>> >> > zones API: {e}"= )
>> >> > +
>> >> > + if data.get("= success") and data.get("result"):
>> >> > + zone=5Fid= =3D data["result"][0]["id"]
>> >> > + zone=5Fname =3D= candidate
>> >> > + break
>> >> =
>> >> It is not acceptable to build anything custom tha= t sends a request. You are removing all other kinds of features that I = have mentioned above.
>> >> 
>> >> T= o just =E2=80=9Cguess=E2=80=9D the name of the zone is something that I= would not consider good style.
>> >> 
>> = >> > +
>> >> > + if not zone=5Fid:
>&g= t; >> > + raise DDNSRequestError(f"Could not find a Cloudflare= Zone
>> >> > for '{self.hostname}'.")
>> &g= t;> > +
>> >> > + logger.info("CFv4: zone=3D%s = id=3D%s", zone=5Fname, zone=5Fid)
>> >> > +
>&g= t; >> > + # --- get record ---
>> >> > + rec= =5Furl =3D
>> >> > f"https://api.cloudflare.com/clien= t/v4/zones/{zone=5Fid}/dns=5Frecords?type=3DA&name=3D{self.hostname= }"
>> >> > + try:
>> >> > + req =3D= urllib.request.Request(rec=5Furl, headers=3Dheaders,
>> >&= gt; > method=3D"GET")
>> >> > + with urllib.reques= t.urlopen(req, timeout=3D20) as r:
>> >> > + rec=5Fda= ta =3D json.loads(r.read().decode())
>> >> > + except= Exception as e:
>> >> > + raise DDNSUpdateError(f"Fa= iled to query Cloudflare DNS
>> >> > records API: {e}= ")
>> >> > +
>> >> > + if not rec=5F= data.get("success"):
>> >> > + errs =3D rec=5Fdata.ge= t("errors") or []
>> >> > + if any("Authentication er= ror" in (e.get("message", "") or
>> >> > "") for e in= errs):
>> >> > + raise DDNSAuthenticationError("Inva= lid API Token.")
>> >> > + raise DDNSUpdateError(f"Cl= oudflare API error finding
>> >> > record: {errs}")>> >> 
>> >> Same as above, hardcoded= defaults like the timeout. Spaghetti code.
>> >> <= br>>> >> > +
>> >> > + results =3D rec= =5Fdata.get("result") or []
>> >> > + if not results:=
>> >> > + raise DDNSRequestError(f"No A record found= for
>> >> > '{self.hostname}' in zone '{zone=5Fname}= '.")
>> >> > +
>> >> > + record=5Fi= d =3D results[0]["id"]
>> >> > + stored=5Fip =3D resu= lts[0]["content"]
>> >> > + logger.info("CFv4: record= =5Fid=3D%s stored=5Fip=3D%s", record=5Fid, stored=5Fip)
>> >= ;> > +
>> >> > + # --- compare IPs ---
>&= gt; >> > + current=5Fip =3D self.get=5Faddress("ipv4")
>= > >> > + logger.info("CFv4: current=5Fip=3D%s vs stored=5Fi= p=3D%s",
>> >> > current=5Fip, stored=5Fip)
>&g= t; >> > + if current=5Fip =3D=3D stored=5Fip:
>> >= > > + logger.info("CFv4: no update needed")
>> >> = > + return
>> >> 
>> >> Why is th= is 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 eve= ry couple of minutes.
>> >> 
>> >> &= gt; +
>> >> > + # --- update ---
>> >>= > + upd=5Furl =3D
>> >> > f"https://api.cloudflar= e.com/client/v4/zones/{zone=5Fid}/dns=5Frecords/{record=5Fid}"
>&= gt; >> > + payload =3D {
>> >> > + "type": "= A",
>> >> > + "name": self.hostname,
>> >= > > + "content": current=5Fip,
>> >> > + "ttl":= ttl,
>> >> > + "proxied": proxied,
>> >&= gt; > + }
>> >> > + logger.info("CFv4: updating %s= -> %s (proxied=3D%s ttl=3D%s)",
>> >> > self.host= name, current=5Fip, proxied, ttl)
>> >> > +
>&g= t; >> > + try:
>> >> > + req =3D urllib.requ= est.Request(
>> >> > + upd=5Furl, data=3Djson.dumps(p= ayload).encode(),
>> >> > headers=3Dheaders, method=3D= "PUT"
>> >> > + )
>> >> > + with ur= llib.request.urlopen(req, timeout=3D20) as r:
>> >> >= + upd =3D json.loads(r.read().decode())
>> >> > + ex= cept Exception as e:
>> >> > + raise DDNSUpdateError(= f"Failed to send update request to
>> >> > Cloudflare= : {e}=E2=80=9D)
>> >> > 
>> >>&nb= sp;
>> >> Once again the same custom request logic.
&= gt;> >> 
>> >> > + if not upd.get("succ= ess"):
>> >> > + raise DDNSUpdateError(f"Cloudflare A= PI error on update:
>> >> > {upd.get('errors')}")
= >> >> > +
>> >> > + logger.info("CFv4:= update ok for %s -> %s", self.hostname, current=5Fip)
>> &= gt;> > + return
>> >> > +
>> >> = > 
>> >> > class DDNSProtocolDynDNS2(object):=
>> >> > """
>> >> > 
>&= gt; >> 
>> >> I would really like to have sup= port for Cloudflare in DDNS, but this is sadly not the way.
>>= >> 
>> >> Please study my comments thoroughl= y and then lets come up with a plan together how to implement this prop= erly.
>> >> 
>> >> Best,
>>= >> -Michael
>> >> 
>> >> = ;
>> >> 
>> > 
>> >&nb= sp;
>> > 
>> > 
>> 
= >> 
>> 


>&nb= sp;


 




  ------=_=-_OpenGroupware_org_NGMime-1307550-1756757958.588736-0--------