I had a look at both Cloudflare approaches together with the current providers.py and system.py.
My solution was a proof of concept and fits the current code with minimal changes. It adds a DDNSProviderCloudflare class directly in providers.py and uses urllib.request.Request(..., method="PATCH") inside the provider itself. That means it does not require any changes to the shared system.py request handling.
The downside is that it bypasses the common send_request() logic, so it does not automatically benefit from the existing proxy handling, user agent, logging and common network error handling. It also uses the token field for three values:
zone_id:record_id:api_token
That works, but it is more of a workaround than a clean provider design.
Fang xn’s solution looks better as a long-term design. It keeps the Cloudflare API token as the token, looks up the zone and DNS record through the Cloudflare API, and adds support for both A and AAAA records. The idea of extending system.py so send_request() can accept custom headers and methods such as PATCH and DELETE also makes sense.
The main concern I see is error handling. In the current system.py, HTTP errors are caught centrally and converted into DDNS exceptions before the provider can read the response body. For Cloudflare this means useful JSON error messages from the API may be lost. One possible improvement would be to add an optional flag to send_request(), for example something like return_http_error=True or raise_http_errors=False, so the Cloudflare provider can read and parse the JSON error response itself.
Another concern is automatic record creation. Fang xn’s provider creates a new DNS record if none is found. This maybe sounds convenient, but I think it is risky as a default. If the user mistypes the hostname, for example vpnn.example.com instead of vpn.example.com, the provider could create a new unwanted DNS record. I think a safer default would be to only update an existing record and fail with a clear message if the record does not exist. Automatic creation could perhaps be an explicit option later.
I would also be careful with can_remove_records. Since the base provider defaults to allowing record removal, a Cloudflare provider with IPv6 support might remove an existing AAAA record if the system currently has no IPv6 address. For a first version I think it may be safer to set can_remove_records = False.
So my conclusion would be:
- Fang xn’s version is a better basis for a proper Cloudflare provider.
- Before using Fang xn’s version more widely, I would suggest improving HTTP error handling, disabling automatic record creation by default, and being careful with automatic record removal.
EDIT:
Fang xn, one thing I would strongly suggest before presenting the next version of the patch is to test more than just whether the provider can retrieve the domain name. For a DDNS provider, the important part is not only discovery, but also whether updates, failures and edge cases behave safely and predictably.
Doing these tests before presenting the final patch would also give you a chance to find problems yourself, fix them, and include those fixes in the next version. That would make the patch easier to review and reduce the risk that reviewers only discover basic runtime or edge-case issues after submission.
Some practical tests that would be useful to run on an IPFire test system:
- Verify that the provider is actually registered
After installing the patch, please check that the Cloudflare provider appears in the DDNS client:
ddns -d list-providers | grep -i cloudflare
If it does not show up here, the WebUI may still display something, but the actual updater may not be able to use the provider correctly.
- Check the detected public IP addresses
Before testing Cloudflare itself, it would be useful to verify what IPFire thinks the current public address is:
ddns -d guess-ip-addresses
Also compare this with the RED interface/local address, especially if the IPFire box is behind another router:
cat /var/ipfire/red/local-ipaddress
If IPFire is behind NAT, the “guess external IP” setting becomes important, because the RED address may be a private LAN address rather than the public WAN address.
- Run a forced update from the shell
After adding a Cloudflare DDNS entry in the WebUI, please test the update manually from the command line:
ddns -d update your.hostname.example.com --force
or, to test all configured DDNS entries:
ddns -d update-all --force
Then check that the output says the update was successful and that there are no unhandled Python exceptions.
- Watch the DDNS logs while testing
In another SSH session, follow the system log while running the update:
tail -f /var/log/messages | grep ddns
or check recent DDNS messages with:
grep ddns /var/log/messages | tail -50
The WebUI logs are also useful: WebUI → Logs → System Log → Section: Dynamic DNS.
- Verify the DNS result externally
After the update, please check that Cloudflare actually returns the expected value. For IPv4:
dig +short A your.hostname.example.com @1.1.1.1
dig +short A your.hostname.example.com @8.8.8.8
For IPv6, if supported:
dig +short AAAA your.hostname.example.com @1.1.1.1
dig +short AAAA your.hostname.example.com @8.8.8.8
Compare this with:
ddns -d guess-ip-addresses
The update should not only return “success”; the DNS record should actually contain the expected IP address.
- Confirm the record through the Cloudflare API
It would also be useful to verify the record directly through Cloudflare’s API:
curl -s \
-H "Authorization: Bearer YOUR_API_TOKEN" \
-H "Content-Type: application/json" \
"https://api.cloudflare.com/client/v4/zones/YOUR_ZONE_ID/dns_records?name=your.hostname.example.com" \
| python -m json.tool
Check that:
-
successistrue -
the record
typeis correct, for exampleAorAAAA -
the
contentvalue is the expected IP address -
the provider did not create an unexpected duplicate record
- Test “already up to date”
Run the forced update once, then run a normal update:
ddns -d update your.hostname.example.com
If the DNS record already points to the current IP, the client should detect that it is up to date and should not perform unnecessary updates.
- Test wrong credentials
Temporarily use an invalid Cloudflare API token and run:
ddns -d update your.hostname.example.com --force
The expected result should be a clear authentication error, not a generic update failure and not an unhandled traceback. This is especially important because Cloudflare returns useful JSON error messages.
- Test wrong hostname / missing record
Try a hostname that does not exist in the Cloudflare zone, for example:
ddns -d update typo-your.hostname.example.com --force
The safe behaviour should be to fail with a clear “record not found” style message. I would not expect the provider to silently create a new DNS record by default, because a typo in the hostname could otherwise create unwanted records.
- Test IPv4-only, IPv6-only and dual-stack situations
If the provider supports both A and AAAA records, it should be tested in at least these cases:
-
hostname has only an A record
-
hostname has only an AAAA record
-
hostname has both A and AAAA records
-
IPFire has no usable IPv6 address
The last case is especially important. The provider should not remove an existing AAAA record just because the current system does not have an IPv6 address, unless that behaviour is explicitly intended and documented.
- Test proxy handling if
system.pyis changed
If the patch extends send_request() in system.py, it should be tested with and without an upstream proxy configured in IPFire. One reason to use the common send_request() path is that all providers should get the same proxy handling, user agent, logging and network error behaviour.
- Check the DDNS state database after failures
If a test fails, IPFire may withhold further updates for some time. It is useful to inspect the DDNS state database:
sqlite3 /var/lib/ddns.db "SELECT * FROM updates;"
If necessary on a test system only, failed update state can be cleared with:
sqlite3 /var/lib/ddns.db "DELETE FROM updates WHERE status = 'failure';"
Then test again with:
ddns -d update your.hostname.example.com --force
- Test uninstall/reinstall cleanly
Since the patch includes install.sh and uninstall.sh, I would also test:
sh install.sh
ddns -d list-providers | grep -i cloudflare
sh uninstall.sh
ddns -d list-providers | grep -i cloudflare
After uninstalling, the Cloudflare provider should no longer be registered, and existing files should be restored exactly as before.
In short, I think it would be very helpful if the next patch included a short test report showing which of these cases have been tested:
-
provider registration
-
WebUI configuration
-
command-line update
-
Cloudflare API response
-
DNS result
-
repeated “already up to date” runs
-
bad token
-
missing record
-
IPv4/IPv6 edge cases
-
logs and error handling
-
clean uninstall
That would make it much easier for others to review the patch. It would also give you a chance to catch and fix problems before presenting the final version, instead of leaving those issues for reviewers or users to discover later.
