Feat/reliable ip fetching#14
Open
siennathesane wants to merge 4 commits into
Open
Conversation
…e library
This commit introduces several enhancements to make the dynamic DNS updater
more resilient and robust:
1. **Updated Cloudflare Go Library:**
* Upgraded `github.com/cloudflare/cloudflare-go` to version `v4.4.0`.
* Adapted existing code to breaking changes in the new library version,
particularly for listing and updating DNS records in `ip/ip_manager.go`.
This involved changing function signatures and parameter structures.
2. **Resilient IP Address Fetching (`ip/ipify.go`):**
* Replaced `log.Fatalf` calls with non-fatal logging (`logger.Printf`)
to prevent application crashes if IP fetching fails.
* Modified `GetCurrentAddress` to always send an `IP` struct to the
processing channel, even if IPv4 or IPv6 fields are `nil` (due to
fetching errors).
* Implemented iteration through a list of multiple external IP address
providers (e.g., ipify, icanhazip) for both IPv4 and IPv6. If one
provider fails, the system automatically tries the next one.
* Improved handling of HTTP errors and response body closing.
3. **Safe IP Handling in Manager (`ip/ip_manager.go`, `ip/ip.go`):**
* Corrected the `IsIPv6Available()` method in `ip/ip.go` to accurately
detect valid IPv6 addresses.
* Updated `ip_manager.go` to only attempt Cloudflare DNS record updates
if a valid (non-nil) IP address of the corresponding type (A or AAAA)
has been successfully fetched. This prevents attempts to update records
with invalid IP data.
4. **Unit Tests (`ip/ipify_test.go`):**
* Added a comprehensive suite of unit tests for the IP address fetching
logic in `ipify.go`. These tests use a mock HTTP server (`httptest`)
to simulate various scenarios, including:
- Successful IP fetching (primary and fallback providers).
- Complete failure of all providers for a given IP type.
- Partial success (e.g., IPv4 fetched, IPv6 fails).
- Handling of malformed or non-IP responses from providers.
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances the dynamic DNS updater’s resilience by implementing fallback IP address providers, adapting to breaking changes in the Cloudflare Go library, and improving error handling.
- Introduces iteration over multiple IPv4 and IPv6 providers with graceful degradation on failures.
- Adapts DNS record listing and updating to the new cloudflare-go v4.4.0 API.
- Updates unit tests, dependency versions, and build configurations to support the changes.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ip/ipify.go | Implements robust fetching logic for IPv4 and IPv6 addresses using multiple providers and closures. |
| ip/ip_manager.go | Adapts DNS record update calls to the updated Cloudflare Go API and includes additional logging. |
| ip/ip.go | Corrects the IPv6 validity check to ensure proper differentiation between IPv4 and IPv6 addresses. |
| go.mod | Upgrades Go version and updates dependencies, including the Cloudflare library. |
| .github/workflows/build.yml | Updates the Go version matrix to reflect the new toolchain compatibility. |
Comment on lines
+69
to
+96
| func() { | ||
| defer func() { | ||
| if closeErr := resp.Body.Close(); closeErr != nil { | ||
| ipy.logger.Printf("Error closing response body from %s: %v", providerUrl, closeErr) | ||
| } | ||
| }() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| ipy.logger.Printf("Failed to get IPv4 from %s: status code %d", providerUrl, resp.StatusCode) | ||
| return // continue to next provider via outer loop's continue | ||
| } | ||
|
|
||
| body, err := ioutil.ReadAll(resp.Body) | ||
| if err != nil { | ||
| ipy.logger.Printf("Failed to read body from %s (IPv4): %v", providerUrl, err) | ||
| return // continue | ||
| } | ||
|
|
||
| parsedIP := net.ParseIP(string(body)) | ||
| // Ensure it's a valid IPv4 address | ||
| if parsedIP == nil || parsedIP.To4() == nil { | ||
| ipy.logger.Printf("Failed to parse IPv4 address from %s, response: %s", providerUrl, string(body)) | ||
| return // continue | ||
| } | ||
|
|
||
| ipRef.IPv4 = parsedIP | ||
| ipy.logger.Printf("Successfully fetched IPv4 %s from %s", ipRef.IPv4, providerUrl) | ||
| }() // End of scope for individual provider response handling |
There was a problem hiding this comment.
[nitpick] The closure used for handling the provider's response is nearly identical for both IPv4 and IPv6. Consider extracting this repeated logic into a helper function to reduce duplication and improve readability.
| } | ||
| } | ||
|
|
||
| // THIS IS WHERE THE BROKEN UpdateDNSRecord CALLS ARE |
There was a problem hiding this comment.
[nitpick] The placeholder comment 'THIS IS WHERE THE BROKEN UpdateDNSRecord CALLS ARE' may cause confusion for future maintainers. Consider removing or updating it to reflect the current state of the code.
This file contains hidden or 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
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.
This commit introduces several enhancements to make the dynamic DNS updater more resilient and robust:
Updated Cloudflare Go Library:
github.com/cloudflare/cloudflare-goto versionv4.4.0.particularly for listing and updating DNS records in
ip/ip_manager.go.This involved changing function signatures and parameter structures.
Resilient IP Address Fetching (
ip/ipify.go):log.Fatalfcalls with non-fatal logging (logger.Printf)to prevent application crashes if IP fetching fails.
GetCurrentAddressto always send anIPstruct to theprocessing channel, even if IPv4 or IPv6 fields are
nil(due tofetching errors).
providers (e.g., ipify, icanhazip) for both IPv4 and IPv6. If one
provider fails, the system automatically tries the next one.
Safe IP Handling in Manager (
ip/ip_manager.go,ip/ip.go):IsIPv6Available()method inip/ip.goto accuratelydetect valid IPv6 addresses.
ip_manager.goto only attempt Cloudflare DNS record updatesif a valid (non-nil) IP address of the corresponding type (A or AAAA)
has been successfully fetched. This prevents attempts to update records
with invalid IP data.
Unit Tests (
ip/ipify_test.go):logic in
ipify.go. These tests use a mock HTTP server (httptest)to simulate various scenarios, including:
Note: This is a test of Google's new Jules service.