feat(dyndns): IP-detect + DNS-update daemon (provider-agnostic)#736
Closed
intel352 wants to merge 1 commit into
Closed
feat(dyndns): IP-detect + DNS-update daemon (provider-agnostic)#736intel352 wants to merge 1 commit into
intel352 wants to merge 1 commit into
Conversation
Per docs/plans/2026-05-20-dns-providers.md T14..T16. dyndns/dyndns.go (260 lines): - IPDetector interface: Detect + Name. - HTTPDetector default impl; DefaultDetectors() returns the three canonical sources (icanhazip / ifconfig.me / ipify.org). - Updater callback — provider-agnostic; caller wires DO/Namecheap/ Hover via the IaC ResourceDriver UpdateRecord RPC. - Config: PollInterval (≥30s), QuorumSize (default ceil(N/2)), MaxBackoff (default 1h), Now + Sleep injectable for tests. - Daemon.Tick(): detect-quorum → diff vs Current → Update on change. Concurrent-safe via mu. - Daemon.Run(): blocking loop until ctx cancel, with exponential backoff + ±10% jitter on consecutive failures. - detectQuorum: fan-out goroutines, tolerate per-detector errors, return first IP that QuorumSize detectors agree on. 12 unit tests: - Update fires on IP change, noop on unchanged. - Quorum: majority requirement, 2-of-3 success, single-detector-err tolerated. - Failure backoff escalates between consecutive ticks. - New() rejects nil Updater + <30s PollInterval. - Updater errors surface + leave Current() unset. - Run() exits on context cancel. - Concurrent Tick() calls are safe (10 goroutines). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new dyndns package that implements a provider-agnostic dynamic DNS daemon: it periodically detects the host’s public IP via a detector quorum, diffs against the last confirmed value, and calls a supplied update callback only when the IP changes.
Changes:
- Added
dyndns.DaemonwithNew,Run, andTick, including quorum-based IP detection and exponential backoff with jitter. - Added
HTTPDetectorandDefaultDetectors()for public IP discovery via simple HTTP endpoints. - Added unit tests covering core daemon behaviors (update-on-change, quorum, backoff, cancellation, and basic concurrency safety).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| dyndns/dyndns.go | New dyndns daemon implementation (quorum detect → diff → update) plus HTTP-based detectors and defaults. |
| dyndns/dyndns_test.go | Unit tests for daemon tick/run behaviors, quorum logic, and backoff/cancellation. |
Comment on lines
+64
to
+66
|
|
||
| // Sleep is injectable for tests. Defaults to time.Sleep. | ||
| Sleep func(time.Duration) |
Comment on lines
+134
to
+154
| // Tick executes one detect/diff/update cycle. Tests call this | ||
| // directly to bypass the timer; Run() invokes it in a loop. | ||
| func (d *Daemon) Tick(ctx context.Context) error { | ||
| ip, err := d.detectQuorum(ctx) | ||
| if err != nil { | ||
| d.recordFailure() | ||
| return err | ||
| } | ||
|
|
||
| d.mu.Lock() | ||
| currentSame := d.current != nil && d.current.Equal(ip) | ||
| d.mu.Unlock() | ||
| if currentSame { | ||
| d.recordSuccess() | ||
| return nil | ||
| } | ||
|
|
||
| if err := d.cfg.Update(ctx, ip); err != nil { | ||
| d.recordFailure() | ||
| return fmt.Errorf("dyndns: update IP %s: %w", ip, err) | ||
| } |
Comment on lines
+192
to
+193
| ip, err := det.Detect(ctx) | ||
| results <- result{ip: ip, name: det.Name(), err: err} |
Comment on lines
+215
to
+218
| if winner == "" { | ||
| return nil, fmt.Errorf("dyndns: no IP reached quorum (%d/%d); errors: %s", d.cfg.QuorumSize, len(d.cfg.Detectors), strings.Join(errs, "; ")) | ||
| } | ||
| return net.ParseIP(winner), nil |
Comment on lines
+287
to
+293
| return nil, fmt.Errorf("HTTP %d", resp.StatusCode) | ||
| } | ||
| body, _ := io.ReadAll(io.LimitReader(resp.Body, 256)) | ||
| s := strings.TrimSpace(string(body)) | ||
| ip := net.ParseIP(s) | ||
| if ip == nil { | ||
| return nil, fmt.Errorf("not an IP: %q", s) |
Comment on lines
+143
to
+157
| d.mu.Lock() | ||
| currentSame := d.current != nil && d.current.Equal(ip) | ||
| d.mu.Unlock() | ||
| if currentSame { | ||
| d.recordSuccess() | ||
| return nil | ||
| } | ||
|
|
||
| if err := d.cfg.Update(ctx, ip); err != nil { | ||
| d.recordFailure() | ||
| return fmt.Errorf("dyndns: update IP %s: %w", ip, err) | ||
| } | ||
|
|
||
| d.mu.Lock() | ||
| d.current = ip |
Comment on lines
+264
to
+296
| // HTTPDetector queries a simple "what's my IP" HTTP endpoint. | ||
| type HTTPDetector struct { | ||
| URL string | ||
| Label string | ||
| HTTP *http.Client | ||
| } | ||
|
|
||
| // Detect implements IPDetector. | ||
| func (h HTTPDetector) Detect(ctx context.Context) (net.IP, error) { | ||
| client := h.HTTP | ||
| if client == nil { | ||
| client = &http.Client{Timeout: 10 * time.Second} | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, h.URL, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
| if resp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("HTTP %d", resp.StatusCode) | ||
| } | ||
| body, _ := io.ReadAll(io.LimitReader(resp.Body, 256)) | ||
| s := strings.TrimSpace(string(body)) | ||
| ip := net.ParseIP(s) | ||
| if ip == nil { | ||
| return nil, fmt.Errorf("not an IP: %q", s) | ||
| } | ||
| return ip, nil | ||
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Contributor
Author
|
Superseded by the consolidated PR with lint fixes. |
intel352
added a commit
that referenced
this pull request
May 20, 2026
Supersedes #736 + #737 with lint fixes applied: dyndns/ (T14..T16): - IP-detect multi-source quorum + diff + Update callback + exp-backoff with jitter. 12 tests. cmd/wfctl/secrets_setup_plugin.go (T3+T4): - secrets setup --plugin reads plugin.json required_secrets[], prompts each (masked iff sensitive), writes to chosen GH scope (repo|env|org). 7 tests. Lint fixes vs the original PRs: - dyndns/dyndns.go Run() — replaced empty-branch (SA9003) with explicit `_ = d.Tick(...)` ignoring the error. - dyndns/dyndns.go jitter — math/rand/v2 G404 false-positive silenced with nolint:gosec annotation (decorrelation, not crypto). - dyndns/dyndns.go timeAfter — replaced unlambda wrapper with direct `var timeAfter = time.After`. - cmd/wfctl/secrets.go — hoisted `args[1:]` into a local var to silence gosec G602 (already bounded by outer switch). All tests pass; golangci-lint clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Per workflow#735 SPEC T14..T16. Pure-Go IP-quorum + diff + Update callback. 12 tests; ready to wire to any DNS driver.