Fix record update attempt with invalid IP #203
+28
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reduces code repetition and ensures failures get raised
Allows acquiring IPv4/6 address from key-value (i.e. https://1.1.1.1/cdn-cgi/trace) and single-value (i.e. https://ipv4.icanhazip.com)
These vars could potentially be added as new config options to set the primary and fallback endpoints
Sensible fallback endpoint in case Cloudflare's are unavailable
makutaku
approved these changes
Nov 26, 2024
KhooHaoYit
approved these changes
Nov 27, 2024
|
Had the same issue where underlying IP rotated and it failed to update the IPs. This fixed my issue as well. |
|
Here for the same outage as well. Thanks for the PR!! |
|
Shouldn't this be handled like a transaction, meaning, it should not clear existing A record unless it can successfully update it ? |
makutaku
approved these changes
Dec 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issues addressed
Changes
This PR is a two-parter:
Part 1
This is a safe change and non-disruptive in the sense that it does not change any existing behaviour other than the fact that it ensures when the endpoint that is being used to determine the public IPv4/6 address is unavailable or not returning the intended address correctly, it fails instead of returning the error response as the supposed IP address (which causes unwanted, repetitive attempts to update DNS records with this error response).
It fixes the linked issue only partially - it fixes the unintended attempts of updating the DNS records with error response values it thought was the IPv4/6 address, but it still is affected by downtimes or currently broken (primary/fallback) Cloudflare endpoints (
https://1.1.1.1/cdn-cgi/traceandhttps://1.0.0.1/cdn-cgi/trace).Commits: f0d9510...cbb5ead
Test image: https://github.com/irfanhakim-as/cloudflare-ddns/pkgs/container/cloudflare-ddns/312084285?tag=1.0.2-fix-wrong-ip-r1
Part 2
Includes Part 1, is safe, but adds a few more modifications that changes an existing behaviour slightly.
It fixes the linked issue completely - by adding support for acquiring IPv4/6 address from single-value endpoints (i.e.
https://ipv6.icanhazip.com) in addition to key-value endpoints as before (i.e.https://[2606:4700:4700::1111]/cdn-cgi/trace).This includes adding 2 new global variables (
ipv4_endpointsandipv6_endpoints), which are used to specify the primary and fallback endpoints for both IP types. These global variables could potentially be used as new config options (by supplying 2-item list of primary and fallback endpoints for each IP type) in the future.The previous default fallback endpoints were updated from
https://1.0.0.1/cdn-cgi/traceandhttps://[2606:4700:4700::1001]/cdn-cgi/tracetohttps://ipv4.icanhazip.comandhttps://ipv6.icanhazip.comrespectively.Commits: f0d9510...cbb5ead and cbb5ead...ba37a97
Test image: https://github.com/irfanhakim-as/cloudflare-ddns/pkgs/container/cloudflare-ddns/312143281?tag=1.0.2-fix-wrong-ip-r2
Notes
I'm not sure about the "usual" or "proper" way of contributing to this repo, i.e. if I need to update the version number in the main script, but feel free to let me know if there's anything I can do to smoothen this PR in case you intend to merge it!