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 4cD5xL1yPRz30RZ for ; Fri, 29 Aug 2025 18:13:42 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519) (Client CN "mail01.haj.ipfire.org", Issuer "R13" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4cD5xK3h2wz2xLm for ; Fri, 29 Aug 2025 18:13:41 +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 4cD5xJ2Qz6z310 for ; Fri, 29 Aug 2025 18:13:40 +0000 (UTC) Authentication-Results: mail01.ipfire.org; dkim=pass header.d=liftingcoder.com header.s=default header.b="i12Xp/Xp"; 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=1756491220; 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=UqM5L4iY0h/UhJnQUY4g0vr3hjgjPN2AYz5UZBHMojY=; b=oS2SQYmCTmUlubhBpv+oIpN8kqSUqKOZenuFEPl232LKZm4ut+nrKlywMrNQRDtxY1mmcw tTy1T7cxUfuTbtkHt785tlroD9yRWBO/RnnT0ZcybtC9qMUJ+RyzRTGYU+NSmivqIdfmSb 21p9j2CJQSLy2XOsY4ZRK3Zy/BD6q1mmylwV/kUXIGYgPb+iyQtoqp52N3WnjFOLtixckx v8bn2gVkcNEKV/rpnFMidlL8BCovanOfotyPs5cy16qHN858nbMXwgPrCTYe7yuBB/Ddn1 Xbj8hNudjfHqukPWb61eAu6ztNTnCT/Y1Cyd5bEcq3Iaj68z3PTCit+fTwjEDA== ARC-Authentication-Results: i=1; mail01.ipfire.org; dkim=pass header.d=liftingcoder.com header.s=default header.b="i12Xp/Xp"; 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=1756491220; a=rsa-sha256; cv=none; b=EhBKgRhGgXy4sWynmIKi3uNMTqdHOKFG3cUvfGpTgT5Kad337Qcz4FKfRWGWkU4HshUTKT FZNYMcV7LDZfXhQ7NVtc/DixsLXopdcW38Hkoc9VG0AjIhC2btkiTSBl/t+kIl1v6FN4se D5Tg+GbKK18VPyC0XTP7Dursi8lpepVjLlLoipg69t6Js2NpOvWAC4hHvlj00VHTNK9ObK VpTvkDdRWsNQJ/PlVLMW2A2dzqxRIFA9sW86mB+r89uSXawYYdzF4VImlXVnpe4bSv/90U D8uocDVHj/FWR0h4AL0GjdnL2mWyqV2u5PdTFED2A8jezLnTR6jw0OtthvMOtQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=liftingcoder.com; s=default; t=1756490919; h=from:subject:date:message-id:to:cc:mime-version:content-type: in-reply-to:references; bh=UqM5L4iY0h/UhJnQUY4g0vr3hjgjPN2AYz5UZBHMojY=; b=i12Xp/XpiZwWYOSX6UULBUEUlzmEuESEyb/MsOO6z4BnXImTlnBKI1QntUlnQC21wwGrar Z70VruA35fyzyJDbOwe91wwUELQ8lOThPVXZO5SKmQrHFK2VXI54TK0QNSrP8OaWWhHoyh LtykobMgBscYguPYRWe6mLv5Dxw47FU= 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 970565C71C3; Fri, 29 Aug 2025 20:08:39 +0200 (CEST) From: "Bernhard Beroun" In-Reply-To: <32702E44-5264-4D75-8645-F297E4AF920B@ipfire.org> Content-Type: multipart/alternative; boundary="----=_=-_OpenGroupware_org_NGMime-3133866-1756490919.465189-1------" References: <2139f7-68b17f00-3-48fa3400@237050521> <32702E44-5264-4D75-8645-F297E4AF920B@ipfire.org> Date: Fri, 29 Aug 2025 20:08:39 +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: <2fd1aa-68b1ec80-2b-633f1b80@145901099> 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: 4cD5xJ2Qz6z310 X-Rspamd-Action: no action X-Spamd-Result: default: False [-4.48 / 11.00]; BAYES_HAM(-2.79)[99.10%]; R_DKIM_ALLOW(-1.45)[liftingcoder.com:s=default]; SUBJ_EXCESS_QP(1.20)[]; NEURAL_HAM(-1.00)[-1.000]; DKIM_REPUTATION(-0.73)[-0.73316138851623]; 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)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; ARC_NA(0.00)[]; FREEMAIL_CC(0.00)[gmail.com,lists.ipfire.org]; FUZZY_RATELIMITED(0.00)[rspamd.com]; DMARC_NA(0.00)[liftingcoder.com]; DKIM_TRACE(0.00)[liftingcoder.com:+]; R_SPF_NA(0.00)[no SPF record]; MISSING_XM_UA(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; 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-3133866-1756490919.465189-1------ Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Length: 11171 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 fur= ther. 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 : =C2=A0 Hello Bernhard, No you are not hijacking. I don=E2=80=99t know why your patch did not receive any feedback. It mu= st 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 th= e 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 = wrote: >=C2=A0 > 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@liftingcode= r.com/T/#u > Unfortunately I never received any feedback, so I assumed Cloudflare = support wasn't desired. Again, sorry for hijacking this email chain her= e, 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 will= 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 wr= ote: >> >=C2=A0 >> > From: faithinchaos21 <45313722+faithinchaos21@users.noreply.github= .com> >>=C2=A0 >> I assume that you are faithinchaos21 and this code did not come from= someone else? >>=C2=A0 >> > Date: Wed, 27 Aug 2025 01:22:46 -0500 >> > Subject: [PATCH] ddns: add Cloudflare (v4) provider using API toke= n >> > 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 upda= tes 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 acc= eptable. >>=C2=A0 >> Before we get into the actual implementation, there are many design = issues here that simply cannot be accepted into DDNS: >>=C2=A0 >> * This patch only adds support for IPv4. As far as I am aware, Cloud= flare is capable of IPv6, too. Why would this be limited to IPv4 only? >>=C2=A0 >> * 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 al= l the features that DDNS provides (catching Exceptions, proxy support a= nd many, many more=E2=80=A6) >>=C2=A0 >> * This all looks very AI-generated to me and is implemented on a gre= en field. You are even importing all the modules that you need and igno= re everything else from DDNS. Why not use this snippet as a standalone = script then? There was no consideration for what code existed already. >>=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 DDNS= entry. >> > + Optional in ddns.conf: >> > + proxied =3D false|true (default 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=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 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 difficul= t 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. DDN= S provides a toolkit of what to use and you should stay within it. If s= ome functionally is missing, DDNS=E2=80=99 functionality should be exte= nded so that other providers can re-use the same well-maintained and te= sted 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 creati= ng 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 request. = You are removing all other kinds of features that I have mentioned abov= e. >>=C2=A0 >> To just =E2=80=9Cguess=E2=80=9D the name of the zone is something th= at 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=5Frec= ords?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 code. >>=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=5F= id, 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 use = DNS for this which creates a lot less load than performing several API = requests every couple of minutes. >>=C2=A0 >> > + >> > + # --- update --- >> > + upd=5Furl =3D >> > f"https://api.cloudflare.com/client/v4/zones/{zone=5Fid}/dns=5Frec= ords/{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, curre= nt=5Fip) >> > + return >> > + >> >=C2=A0 >> > class DDNSProtocolDynDNS2(object): >> > """ >> >=C2=A0 >>=C2=A0 >> I would really like to have support for Cloudflare in DDNS, but this= is sadly not the way. >>=C2=A0 >> Please study my comments thoroughly and then lets come up with a pla= n 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 ------=_=-_OpenGroupware_org_NGMime-3133866-1756490919.465189-1------ Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Length: 14539

Hello Michael,

as far as I know Cloudflare also support= s IPv6 for DNS, but since I do not use any IPv6 in my home network, I h= aven'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>:

&nbs= p;

Hello Bernhard,

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 DD= NS=E2=80=A6

Your patch looks much more like it because it is usi= ng and extending the code of DDNS.

However, it also only impleme= nts IPv4. Does Cloudflare not support IPv6 for DNS at all?

-Mich= ael

> 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 y= ou 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 ass= umed Cloudflare support wasn't desired. Again, sorry for hijacking this= email chain here, just wanted to quickly drop it here, since it matche= s the topic.
> Have a nice day,
> Bernhard
> > Am Freitag, August 29, 2025 11:50 CEST, schrieb Michael Tremer &l= t;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.
>> 
>> So let=E2=80= =99s get into this...
>> 
>> > On 29 Aug 2025= , at 09:16, Chris Anton <chris.v.anton@gmail.com> wrote:
>&= gt; > 
>> > From: faithinchaos21 <45313722+faith= inchaos21@users.noreply.github.com>
>> 
>> I = assume that you are faithinchaos21 and this code did not come from some= one 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=3DUTF-8
>> > Conte= nt-Transfer-Encoding: 8bit
>> > 
>> > This= adds a provider =E2=80=9Ccloudflare.com-v4=E2=80=9D that updates an A = record
>> > via Cloudflare=E2=80=99s v4 API using a Bearer = token. The token is accepted
>> > from either =E2=80=98toke= n=E2=80=99 or legacy =E2=80=98password=E2=80=99 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 includ= e CFv4 breadcrumbs for troubleshooting
>> 
>> To= make it short, this patch is sadly very far away from what is acceptab= le.
>> 
>> Before we get into the actual impleme= ntation, there are many design issues here that simply cannot be accept= ed into DDNS:
>> 
>> * This patch only adds supp= ort 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 t= he features that DDNS provides (catching Exceptions, proxy support and = many, many more=E2=80=A6)
>> 
>> * This all look= s 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.
>> <= br>>> > Signed-off-by: Chris Anton <chris.v.anton@gmail.com= >
>> > ---
>> > 563f089d0820bd61ad4aecac248d= 5cc1f2adfc81
>> > src/ddns/providers.py | 121 +++++++++++++= +++++++++++++++++++++++++++++
>> > 1 file changed, 121 inse= rtions(+)
>> > 
>> > diff --git a/src/ddns= /providers.py b/src/ddns/providers.py
>> > index 59f9665..d= f0f3a9 100644
>> > --- a/src/ddns/providers.py
>> = > +++ b/src/ddns/providers.py
>> > @@ -341,6 +341,127 @@= def have=5Faddress(self, proto):
>> > 
>> &g= t; return False
>> > 
>> > +class DDNSProv= iderCloudflareV4(DDNSProvider):
>> > + """
>> >= + Cloudflare v4 API using a Bearer Token.
>> > + Put the A= PI Token in the 'token' OR 'password' field of the DDNS entry.
>&= gt; > + Optional in ddns.conf:
>> > + proxied =3D false|= true (default 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.cloudflar= e.com/"
>> > + protocols =3D ("ipv4",)
>> > + s= upports=5Ftoken=5Fauth =3D True
>> > + holdoff=5Ffailure=5F= days =3D 0
>> > +
>> > + def =5Fbool(self, key,= default=3DFalse):
>> > + v =3D str(self.get(key, default))= .strip().lower()
>> > + return v in ("1", "true", "yes", "o= n")
>> > +
>> 
>> This is a good exa= mple of a function which totally goes against the coding style of the r= est of the code base. It uses str(), chains a lot of functions to gathe= r to make the code look shorter, but very difficult to read.
>>= ; 
>> > + def update(self):
>> > + import = json, urllib.request, urllib.error
>> 
>> Just n= o. We don=E2=80=99t import anything further down the line. DDNS provide= s a toolkit of what to use and you should stay within it. If some funct= ionally is missing, DDNS=E2=80=99 functionality should be extended so t= hat other providers can re-use the same well-maintained and tested code= base.
>> 
>> > +
>> > + tok =3D = self.get("token") or self.get("password")
>> > + if not tok= :
>> > + raise DDNSConfigurationError("API Token (password/= token)
>> > is missing.")
>> > +
>> &g= t; + proxied =3D self.=5Fbool("proxied", False)
>> > + try:=
>> > + ttl =3D int(self.get("ttl", 1))
>> > + = except Exception:
>> > + ttl =3D 1
>> 
>= ;> A TTL of one second is never a good idea.
>> 
&g= t;> > +
>> > + headers =3D {
>> > + "Auth= orization": "Bearer {0}".format(tok),
>> > + "Content-Type"= : "application/json",
>> > + "User-Agent": "IPFireDDNSUpdat= er/CFv4",
>> > + }
>> 
>> Since you = are not using the internal request handler, you are creating your own h= eaders and a completely nonsensical user agent. Why?
>> <= br>>> > +
>> > + # --- find zone ---
>> &= gt; + parts =3D self.hostname.split(".")
>> > + if len(part= s) < 2:
>> > + raise DDNSRequestError("Hostname '{0}' is= not a valid
>> > domain.".format(self.hostname))
>&g= t; > +
>> > + zone=5Fid =3D None
>> > + zone= =5Fname =3D None
>> > + for i in range(len(parts) - 1):
= >> > + candidate =3D ".".join(parts[i:])
>> > + ur= l =3D
>> > f"https://api.cloudflare.com/client/v4/zones?nam= e=3D{candidate}"
>> > + try:
>> > + req =3D url= lib.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 DDNSUpdate= Error(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 custo= m that sends a request. You are removing all other kinds of features th= at I have mentioned above.
>> 
>> To just =E2=80= =9Cguess=E2=80=9D the name of the zone is something that I would not co= nsider good style.
>> 
>> > +
>> >= ; + 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"ht= tps://api.cloudflare.com/client/v4/zones/{zone=5Fid}/dns=5Frecords?type= =3DA&name=3D{self.hostname}"
>> > + try:
>> &g= t; + req =3D urllib.request.Request(rec=5Furl, headers=3Dheaders,
&g= t;> > method=3D"GET")
>> > + with urllib.request.urlo= pen(req, timeout=3D20) as r:
>> > + rec=5Fdata =3D json.loa= ds(r.read().decode())
>> > + except Exception as e:
>= > > + raise DDNSUpdateError(f"Failed to query Cloudflare DNS
&= gt;> > records API: {e}")
>> > +
>> > + i= f not rec=5Fdata.get("success"):
>> > + errs =3D rec=5Fdata= .get("errors") or []
>> > + if any("Authentication error" i= n (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 =3D rec=5Fdata.get("res= ult") or []
>> > + if not results:
>> > + raise= DDNSRequestError(f"No A record found for
>> > '{self.hostn= ame}' in zone '{zone=5Fname}'.")
>> > +
>> > + = record=5Fid =3D results[0]["id"]
>> > + stored=5Fip =3D res= ults[0]["content"]
>> > + logger.info("CFv4: record=5Fid=3D= %s stored=5Fip=3D%s", record=5Fid, stored=5Fip)
>> > +
&= gt;> > + # --- compare IPs ---
>> > + current=5Fip =3D= self.get=5Faddress("ipv4")
>> > + logger.info("CFv4: curre= nt=5Fip=3D%s vs stored=5Fip=3D%s",
>> > current=5Fip, store= d=5Fip)
>> > + if current=5Fip =3D=3D stored=5Fip:
>&= gt; > + logger.info("CFv4: no update needed")
>> > + ret= urn
>> 
>> Why is this done using the API at all= ? We have functionality to use DNS for this which creates a lot less lo= ad than performing several API requests every couple of minutes.
>= ;> 
>> > +
>> > + # --- update ---
&= gt;> > + upd=5Furl =3D
>> > f"https://api.cloudflare.= com/client/v4/zones/{zone=5Fid}/dns=5Frecords/{record=5Fid}"
>>= ; > + payload =3D {
>> > + "type": "A",
>> >= + "name": self.hostname,
>> > + "content": current=5Fip,>> > + "ttl": ttl,
>> > + "proxied": proxied,>> > + }
>> > + logger.info("CFv4: updating %s -&= gt; %s (proxied=3D%s ttl=3D%s)",
>> > self.hostname, curren= t=5Fip, proxied, ttl)
>> > +
>> > + try:
>= ;> > + req =3D urllib.request.Request(
>> > + upd=5Fu= rl, data=3Djson.dumps(payload).encode(),
>> > headers=3Dhea= ders, method=3D"PUT"
>> > + )
>> > + with urlli= b.request.urlopen(req, timeout=3D20) as r:
>> > + upd =3D j= son.loads(r.read().decode())
>> > + except Exception as e:<= br>>> > + raise DDNSUpdateError(f"Failed to send update reques= t to
>> > Cloudflare: {e}=E2=80=9D)
>> > <= br>>> 
>> 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, curre= nt=5Fip)
>> > + return
>> > +
>> >&= nbsp;
>> > class DDNSProtocolDynDNS2(object):
>> &= gt; """
>> > 
>> 
>> I would re= ally like to have support for Cloudflare in DDNS, but this is sadly not= the way.
>> 
>> Please study my comments thorou= ghly and then lets come up with a plan together how to implement this p= roperly.
>> 
>> Best,
>> -Michael
&g= t;> 
>> 
>> 
> 
>&= nbsp;
> 
> 


 

=


 

------=_=-_OpenGroupware_org_NGMime-3133866-1756490919.465189-1--------