Message ID | CAJ2Zu3ChjmWhHTFS907DaHT+OHYOb+_wqz8g4UVJza3V3KxNaA@mail.gmail.com |
---|---|
State | New |
Headers |
Return-Path: <ddns+bounces-1-patchwork=ipfire.org@lists.ipfire.org> 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 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 web04.haj.ipfire.org (Postfix) with ESMTPS id 4cCrhb2t15z3wbG for <patchwork@web04.haj.ipfire.org>; Fri, 29 Aug 2025 08:16:47 +0000 (UTC) Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [IPv6:2001:678:b28::201]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519) (Client CN "mail02.haj.ipfire.org", Issuer "E8" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 4cCrhZ2cvNz5NH for <patchwork@ipfire.org>; Fri, 29 Aug 2025 08:16:46 +0000 (UTC) Authentication-Results: mail01.ipfire.org; dkim=pass header.d=gmail.com header.s=20230601 header.b=gQtsSzTj; dmarc=pass (policy=none) header.from=gmail.com; arc=pass ("lists.ipfire.org:s=202003rsa:i=1"); spf=softfail (mail01.ipfire.org: 2001:678:b28::201 is neither permitted nor denied by domain of "ddns+bounces-1-patchwork=ipfire.org@lists.ipfire.org") smtp.mailfrom="ddns+bounces-1-patchwork=ipfire.org@lists.ipfire.org" ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1756455406; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=vl6UlywgAxN/D8Z3NGi0rNwkP/xfobHwL4IJKzULYQc=; b=kWQBSRkRwUldv8YvhGBE07XLflTAbbsq23L3UlHDBbZzq9uXy7GkxygxhRUlQ5StyMdT91 DK3ure2RS8Qs5Jj2TgEpzFRXH9QWg5YICAkzYAEZB8rK2+uKB9ay4QpPl5RO2n0lhvZcvz 0JHsA8SiprhxQbtk+1MMy9RBbaauX750eSalXRrx7FD/y0J7+fIOLyYoMhP8l7yY1FAXGA 6buPutYMyH/xb1F+ixm1nV14u0yJheDPPgjkfGqdp379YQba0f4hz/Vg2qzZtn+aNofgZh ZntbHy1dRBvSGY2qfXTA+misCOfqBavnINJQAF00ruifPr01R34v6szWwmzzDg== ARC-Authentication-Results: i=2; mail01.ipfire.org; dkim=pass header.d=gmail.com header.s=20230601 header.b=gQtsSzTj; dmarc=pass (policy=none) header.from=gmail.com; arc=pass ("lists.ipfire.org:s=202003rsa:i=1"); spf=softfail (mail01.ipfire.org: 2001:678:b28::201 is neither permitted nor denied by domain of "ddns+bounces-1-patchwork=ipfire.org@lists.ipfire.org") smtp.mailfrom="ddns+bounces-1-patchwork=ipfire.org@lists.ipfire.org" ARC-Seal: i=2; s=202003rsa; d=ipfire.org; t=1756455406; a=rsa-sha256; cv=pass; b=Wib96FHCZMby5KFIteG4tho4uCYZrufat+PcJVgX2VOn0YvyN2FF/FWRR0/RCoy4ynQFtF J0PS7UHm+bwug8temYxlWYyaa0Yg3Xj1h4ewx/KBMsSL8H4/20Y9LilyZxwSRaF04UFZ8k TYl0iORi6m7XdXHmfGU7VVB55EXO4p1fKww+5q+leCzAaWmCJNKs2W9FWPtEnJ+dZeNVtZ v3OW3Mjr48W4BiWrvFbj6sHFkWLrJf/wVwmsOInONlfCL5hG36hmWs1TJ4oR4naa0VAwEF 7dcF/Te8hAJvBeptnZW7+ksR6msxKTjJfi9h4FqzdeQqKXyzo8QEKcXjFWXxuA== Received: from mail02.haj.ipfire.org (localhost [IPv6:::1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4cCrhY68vHz30L0 for <patchwork@ipfire.org>; Fri, 29 Aug 2025 08:16:45 +0000 (UTC) X-Original-To: ddns@lists.ipfire.org 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 4cCrhY2Tf9z2xDp for <ddns@lists.ipfire.org>; Fri, 29 Aug 2025 08:16:45 +0000 (UTC) Received: from mail-yw1-x1132.google.com (mail-yw1-x1132.google.com [IPv6:2607:f8b0:4864:20::1132]) (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-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 4cCrhX4cqwzLs for <ddns@lists.ipfire.org>; Fri, 29 Aug 2025 08:16:44 +0000 (UTC) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=lists.ipfire.org; s=202003rsa; t=1756455404; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding:dkim-signature; bh=vl6UlywgAxN/D8Z3NGi0rNwkP/xfobHwL4IJKzULYQc=; b=EzBJ03N92BfZZoQlWrSizfzoQhtP+QDKEcsZvdmA44ICwV81tIg+AVkkyqLfxXQjQMkUEJ a004nHKwK2W2n/TTuTkmn7URdU939UksFyX8aphAcyDumXk0T1+QQt9xP9+T819Wi5sL44 mvS4V2ozTdybbS0nELihXXbQjyp1FrX0PwdXvaPO7wYrM6aBRB3GuLtvQUjjOsL5s+nML6 k8ZWerI6QbhiAqsz8lVNNUmEBEfuT3INlPuBTtauLI5fvggijzs2jL8f1JI8UkbhKJaDEw 2kKk7DNLzxuSd8pgM9Hbfq5M4dER3AreDwTgykFR+1BMXltXRPhNg69FJKHfAw== ARC-Authentication-Results: i=1; mail01.ipfire.org; dkim=pass header.d=gmail.com header.s=20230601 header.b=gQtsSzTj; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mail01.ipfire.org: domain of chris.v.anton@gmail.com designates 2607:f8b0:4864:20::1132 as permitted sender) smtp.mailfrom=chris.v.anton@gmail.com ARC-Seal: i=1; s=202003rsa; d=lists.ipfire.org; t=1756455404; a=rsa-sha256; cv=none; b=gegRgD89+H+27LVOpMHp4ApT9BdcNH9sURgazEmUPBFV7gwbSUsSQASIuV5Yjq/Dg942lP YaeZSQq04cB5/9BaJf5F6TG1VXxM/2ZVWTAAFaTNll10n6cvdyixm6uJNXtB4pvblBiMWw GdLHfKu48heiWnqTk+jBMZbZAIk79/0rNYGxlNwWAshjcz+SGwQxz0TU7A8t3Q4kh50B2E nwFCkaR12WdVdEyavlcLtMz2xIBp302kODelxa1v+6vv936Et1vzIWHWNIymRECXHbbxmC XgWuBuNH83I658gXYgeQavN0AIO4aW7hy7fHsmN9LI1DZQt73ZyD0qJbMMyHug== Received: by mail-yw1-x1132.google.com with SMTP id 00721157ae682-71fab75fc97so13531767b3.3 for <ddns@lists.ipfire.org>; Fri, 29 Aug 2025 01:16:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1756455403; x=1757060203; darn=lists.ipfire.org; h=content-transfer-encoding:to:subject:message-id:date:from :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=vl6UlywgAxN/D8Z3NGi0rNwkP/xfobHwL4IJKzULYQc=; b=gQtsSzTjcjtPLjUW/KjQi+BAKVAQG9oWhkx2ImwRScKqWVNqP7j3keQfACJv3sN+z+ rh3HEJKDhwWbPSFs+xvYVz+sUPAMrXvfTaIYiPqAspLd76cGhr631jC6LMr+eN++ySDW A53Q/a+rgPfKhEPT6upvSVB/M92fMs3PabKbpm3TTzqicO+UHbi/snavEV9ll1JPdvdv j7YZhNSNT1GO128717MXkF96VuVqPS1nsqrVWviPgiG8VVBDiQjyp2BN4jgksbnCyRAi xQnEonTbMUYrSjrEir7s4p6IOLBtlrCLlkBstLHx6yhdvH7MfIBf9oqMw9s9lOIydJn9 ZLXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1756455403; x=1757060203; h=content-transfer-encoding:to:subject:message-id:date:from :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vl6UlywgAxN/D8Z3NGi0rNwkP/xfobHwL4IJKzULYQc=; b=nC3hntvHhl/GzoUhapXOp9bI+rDkQm5H6D8RqJcK+KQQgPe9wMnLN/liZ4GB9zkovf 1AeYsrEs8WSSk1i5vHNUwh9e9szQFzBLZ1hOYPxX3kWkR2CIshJNdQNL5F0jkyKATB4p WZRl0P+qRWNsMqHDV753u/dRebTKa2RMfpI5qtnPbINivR2VjqTyc73BiGKnCtecB33c WVzWuDlrVehT5ogT7Z1kDmTw6U5GLeQYfefNWcNCyC0u9xky2lHajiJQVUe1R8B0DGyO OjdhxmD/DiW7aP2N3NOULlu3HDBPtzUrFXnCSDp1I7H2vNOkFPk3eHinpwAWHQtuUfYJ 6/RQ== X-Gm-Message-State: AOJu0Yyo/YVFgngbbTqLuFmgcTr6OpXK0/iKtx7u+MjGqS/skEwXyTGL 9bxOugY98mwPj4k9h27niSrYG+ZYkcyq92Dprdhx+Q8tn9wYhdTbKl0+qmtFSCOuRpnU6x4/GGg v+2+FpG3v2OjA45BZBqWpIUz7z3R6Az3s4Pr5F/BURQ== X-Gm-Gg: ASbGncuCQWQpRkqDzakrlsW0uCR7IwedXmeODyUyo0QBhtX+iI0SecUwHltVe9xU9lZ XXeu1Do0nckQQHEF4qHxkmV9kSGMqvVKtdXwCWDz2HhLC9cnWbmC5PuC9FfYXTHumn9EO8wtnSY W5TWh8/U1vtNEpKnRIMvrDLFNJ6kj2UclRuxpzUZc5V0g1Mc1q7xQ8Hjwsgo1uLEup7VvUoyLbk ibJqDiyfW87ddFXX08= X-Google-Smtp-Source: AGHT+IHx6uUI1EWvkmXtpVkDyp77FIbn1JTYKSvWqQm2XNec00jK9Z/gCEH5klilx4CkXeiT7dVQ+IVenkZg66tgVLg= X-Received: by 2002:a05:690c:892:b0:71f:b944:1035 with SMTP id 00721157ae682-71fdc5313b3mr288118647b3.50.1756455402862; Fri, 29 Aug 2025 01:16:42 -0700 (PDT) Precedence: list List-Id: <ddns.lists.ipfire.org> List-Subscribe: <https://lists.ipfire.org/>, <mailto:ddns+subscribe@lists.ipfire.org?subject=subscribe> List-Unsubscribe: <https://lists.ipfire.org/>, <mailto:ddns+unsubscribe@lists.ipfire.org?subject=unsubscribe> List-Post: <mailto:ddns@lists.ipfire.org> List-Help: <mailto:ddns+help@lists.ipfire.org?subject=help> Sender: <ddns@lists.ipfire.org> Mail-Followup-To: <ddns@lists.ipfire.org> MIME-Version: 1.0 From: Chris Anton <chris.v.anton@gmail.com> Date: Fri, 29 Aug 2025 03:16:32 -0500 X-Gm-Features: Ac12FXx3uTwYvMArPCdBr6aQah3fXYBy7c3-bdxo2qLTLlPdthTUGit9XiD-L0c Message-ID: <CAJ2Zu3ChjmWhHTFS907DaHT+OHYOb+_wqz8g4UVJza3V3KxNaA@mail.gmail.com> Subject: [PATCH] ddns: add Cloudflare (v4) provider using API token To: ddns@lists.ipfire.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: mail01.haj.ipfire.org X-Rspamd-Queue-Id: 4cCrhZ2cvNz5NH X-Rspamd-Action: no action X-Spamd-Result: default: False [-10.73 / 11.00]; BAYES_HAM(-2.89)[99.55%]; FROM_INTERNAL_BULK_SENDERS(-2.00)[2001:678:b28::201]; R_DKIM_ALLOW(-1.67)[gmail.com:s=20230601]; NEURAL_HAM(-1.00)[-1.000]; ARC_ALLOW(-1.00)[lists.ipfire.org:s=202003rsa:i=1]; DKIM_REPUTATION(-0.93)[-0.93496063078125]; DMARC_POLICY_ALLOW_WITH_FAILURES(-0.50)[]; IP_REPUTATION_HAM(-0.44)[asn: 204867(-0.13), country: DE(-0.00), ip: 2001:678:b28::(-0.32)]; MAILLIST(-0.18)[generic]; MIME_GOOD(-0.10)[text/plain]; MX_GOOD(-0.01)[]; HAS_LIST_UNSUB(-0.01)[]; RCVD_TLS_LAST(0.00)[]; RCVD_COUNT_THREE(0.00)[4]; FREEMAIL_FROM(0.00)[gmail.com]; TAGGED_FROM(0.00)[bounces-1-patchwork=ipfire.org]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; MIME_TRACE(0.00)[0:+]; RCPT_COUNT_ONE(0.00)[1]; FUZZY_RATELIMITED(0.00)[rspamd.com]; FORGED_RECIPIENTS_MAILLIST(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; R_SPF_SOFTFAIL(0.00)[~all:c]; TO_DN_NONE(0.00)[]; FROM_NEQ_ENVFROM(0.00)[chrisvanton@gmail.com,ddns@lists.ipfire.org]; FROM_HAS_DN(0.00)[]; DMARC_POLICY_ALLOW(0.00)[gmail.com,none]; MID_RHS_MATCH_FROMTLD(0.00)[]; ASN(0.00)[asn:204867, ipnet:2001:678:b28::/48, country:DE]; ARC_SIGNED(0.00)[ipfire.org:s=202003rsa:i=2]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::1132:received]; MISSING_XM_UA(0.00)[]; FORGED_SENDER_MAILLIST(0.00)[] |
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
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
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
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 >> >> >> > > > >
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 >> >> >> > > > >
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(),