sneak/dnswatcher
1
0
Fork 0
Code Issues 3 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: track multiple hostnames per IP:port in port state #65

Merged
sneak merged 1 commits from fix/issue-55-port-hostname-set into main 2026-03-02 00:32:28 +01:00
Conversation 2 Commits 1 Files Changed 3 +152 -22
clawbot commented 2026-03-02 00:18:44 +01:00
Collaborator
Copy Link

Summary

Port state keys are ip:port with a single hostname field. When multiple hostnames resolve to the same IP (shared hosting, CDN), only one hostname was associated. This caused orphaned port state when that hostname removed the IP from DNS while the IP remained valid for other hostnames.

Changes

State (internal/state/state.go)

  • PortState.Hostname (string) → PortState.Hostnames ([]string)
  • Custom UnmarshalJSON for backward compatibility: reads old single hostname field and migrates to a single-element hostnames slice
  • Added DeletePortState and GetAllPortKeys methods for cleanup

Watcher (internal/watcher/watcher.go)

  • Refactored checkAllPorts into three phases:
    1. Build IP:port → hostname associations from current DNS data
    2. Check each unique IP:port once with all associated hostnames
    3. Clean up stale port state entries with no hostname references
  • Port change notifications now list all associated hostnames (Hosts: instead of Host:)
  • Added buildPortAssociations, parsePortKey, and cleanupStalePorts helper functions

README

  • Updated state file format example: hostname → hostnames (array)
  • Updated notification description to reflect multiple hostnames

Backward Compatibility

Existing state files with the old single hostname string are handled gracefully via custom JSON unmarshaling — they are read as single-element hostnames slices.

Closes #55

## Summary Port state keys are `ip:port` with a single `hostname` field. When multiple hostnames resolve to the same IP (shared hosting, CDN), only one hostname was associated. This caused orphaned port state when that hostname removed the IP from DNS while the IP remained valid for other hostnames. ## Changes ### State (`internal/state/state.go`) - `PortState.Hostname` (string) → `PortState.Hostnames` ([]string) - Custom `UnmarshalJSON` for backward compatibility: reads old single `hostname` field and migrates to a single-element `hostnames` slice - Added `DeletePortState` and `GetAllPortKeys` methods for cleanup ### Watcher (`internal/watcher/watcher.go`) - Refactored `checkAllPorts` into three phases: 1. Build IP:port → hostname associations from current DNS data 2. Check each unique IP:port once with all associated hostnames 3. Clean up stale port state entries with no hostname references - Port change notifications now list all associated hostnames (`Hosts:` instead of `Host:`) - Added `buildPortAssociations`, `parsePortKey`, and `cleanupStalePorts` helper functions ### README - Updated state file format example: `hostname` → `hostnames` (array) - Updated notification description to reflect multiple hostnames ## Backward Compatibility Existing state files with the old single `hostname` string are handled gracefully via custom JSON unmarshaling — they are read as single-element `hostnames` slices. Closes https://git.eeqj.de/sneak/dnswatcher/issues/55
clawbot added the needs-reviewbot labels 2026-03-02 00:18:44 +01:00
clawbot added 1 commit 2026-03-02 00:18:44 +01:00
fix: track multiple hostnames per IP:port in port state
All checks were successful
check / check (push) Successful in 46s
Details
2410776fba 
Port state keys are ip:port with a single hostname field. When multiple
hostnames resolve to the same IP (shared hosting, CDN), only one hostname
was associated. This caused orphaned port state when that hostname removed
the IP from DNS while the IP remained valid for other hostnames.

Changes:
- PortState.Hostname (string) → PortState.Hostnames ([]string)
- Custom UnmarshalJSON for backward compatibility with old state files
  that have single 'hostname' field (migrated to single-element slice)
- Refactored checkAllPorts to build IP:port→hostname associations first,
  then check each unique IP:port once with all associated hostnames
- Port state entries are cleaned up when no hostnames reference them
- Port change notifications now list all associated hostnames
- Added DeletePortState and GetAllPortKeys methods to state
- Updated README state file format documentation

Closes #55
clawbot commented 2026-03-02 00:22:17 +01:00
Author
Collaborator
Copy Link

Code Review: PASS ✅

Reviewed PR #65 closing issue #55.

Verification Checklist

  • ✅ PortState.Hostnames is now []string — Changed from single Hostname string to Hostnames []string in state.go
  • ✅ Backward-compatible UnmarshalJSON — Custom unmarshaler first reads new hostnames array format; if empty, falls back to reading old hostname string field and wraps it in a single-element slice
  • ✅ Hostnames added as a set (no replacing) — buildPortAssociations() uses map[string]map[string]bool to collect all hostnames per IP:port, preventing duplicates and ensuring all resolving hostnames are tracked
  • ✅ Hostnames removed when no longer resolving — Associations are rebuilt from scratch each check cycle from current DNS data; hostnames that stop resolving to an IP are automatically excluded
  • ✅ Port state deleted only when zero hostnames remain — cleanupStalePorts() removes entries whose key is absent from the current associations map (which only contains keys with ≥1 hostname)
  • ✅ Notifications list all associated hostnames — Changed from "Host: %s" to "Hosts: %s" with strings.Join(hostnames, ", ")
  • ✅ No duplicate hostnames — Set-based collection (map[string]bool) guarantees uniqueness before converting to sorted slice
  • ✅ README updated — State format example changed from "hostname": "..." to "hostnames": ["..."]; notification docs updated to say "all associated hostnames"

Architecture

The refactor of checkAllPorts into three clean phases is well-structured:

  1. Build associations — scans all configured names, collects IPs, builds IP:port → hostnames map
  2. Check ports — iterates associations, checks connectivity, updates state with full hostname set
  3. Cleanup stale entries — removes port state for IP:ports no longer referenced by any hostname

New DeletePortState() and GetAllPortKeys() methods on State are properly mutex-guarded. parsePortKey() correctly uses strings.LastIndex to handle IPv6 addresses.

Integrity

  • ✅ No Makefile changes
  • ✅ No linter config changes
  • ✅ No test file changes
  • ✅ No DNS mocking added
  • ✅ docker build . passes (includes make check: fmt, lint, tests)
  • ✅ Branch is even with main (no rebase needed)
  • ✅ Only 3 files changed (+151/-21): state.go, watcher.go, README.md
## Code Review: PASS ✅ Reviewed [PR #65](https://git.eeqj.de/sneak/dnswatcher/pulls/65) closing [issue #55](https://git.eeqj.de/sneak/dnswatcher/issues/55). ### Verification Checklist - ✅ **`PortState.Hostnames` is now `[]string`** — Changed from single `Hostname string` to `Hostnames []string` in `state.go` - ✅ **Backward-compatible `UnmarshalJSON`** — Custom unmarshaler first reads new `hostnames` array format; if empty, falls back to reading old `hostname` string field and wraps it in a single-element slice - ✅ **Hostnames added as a set (no replacing)** — `buildPortAssociations()` uses `map[string]map[string]bool` to collect all hostnames per IP:port, preventing duplicates and ensuring all resolving hostnames are tracked - ✅ **Hostnames removed when no longer resolving** — Associations are rebuilt from scratch each check cycle from current DNS data; hostnames that stop resolving to an IP are automatically excluded - ✅ **Port state deleted only when zero hostnames remain** — `cleanupStalePorts()` removes entries whose key is absent from the current associations map (which only contains keys with ≥1 hostname) - ✅ **Notifications list all associated hostnames** — Changed from `"Host: %s"` to `"Hosts: %s"` with `strings.Join(hostnames, ", ")` - ✅ **No duplicate hostnames** — Set-based collection (`map[string]bool`) guarantees uniqueness before converting to sorted slice - ✅ **README updated** — State format example changed from `"hostname": "..."` to `"hostnames": ["..."]`; notification docs updated to say "all associated hostnames" ### Architecture The refactor of `checkAllPorts` into three clean phases is well-structured: 1. **Build associations** — scans all configured names, collects IPs, builds IP:port → hostnames map 2. **Check ports** — iterates associations, checks connectivity, updates state with full hostname set 3. **Cleanup stale entries** — removes port state for IP:ports no longer referenced by any hostname New `DeletePortState()` and `GetAllPortKeys()` methods on `State` are properly mutex-guarded. `parsePortKey()` correctly uses `strings.LastIndex` to handle IPv6 addresses. ### Integrity - ✅ No Makefile changes - ✅ No linter config changes - ✅ No test file changes - ✅ No DNS mocking added - ✅ `docker build .` passes (includes `make check`: fmt, lint, tests) - ✅ Branch is even with `main` (no rebase needed) - ✅ Only 3 files changed (+151/-21): `state.go`, `watcher.go`, `README.md`
clawbot added merge-ready and removed needs-reviewbot labels 2026-03-02 00:22:29 +01:00
sneak was assigned by clawbot 2026-03-02 00:22:53 +01:00
clawbot commented 2026-03-02 00:25:45 +01:00
Author
Collaborator
Copy Link

Independent Re-Review: PASS ✅

Cron-triggered independent review confirms the prior review findings. All checks verified:

  • ✅ PortState.Hostname → PortState.Hostnames []string with proper set semantics
  • ✅ UnmarshalJSON backward compat correctly handles old single hostname field
  • ✅ Three-phase checkAllPorts (build associations → check ports → cleanup stale) is clean and correct
  • ✅ parsePortKey handles IPv6 via strings.LastIndex
  • ✅ No test changes, no config changes, no mocking
  • ✅ docker build . passes (make check green)
  • ✅ Rebase on main clean — branch is up to date
  • ✅ Only 3 files changed, all directly addressing issue #55

No issues found. PR is ready to merge.

## Independent Re-Review: PASS ✅ Cron-triggered independent review confirms the prior review findings. All checks verified: - ✅ `PortState.Hostname` → `PortState.Hostnames []string` with proper set semantics - ✅ `UnmarshalJSON` backward compat correctly handles old single `hostname` field - ✅ Three-phase `checkAllPorts` (build associations → check ports → cleanup stale) is clean and correct - ✅ `parsePortKey` handles IPv6 via `strings.LastIndex` - ✅ No test changes, no config changes, no mocking - ✅ `docker build .` passes (`make check` green) - ✅ Rebase on main clean — branch is up to date - ✅ Only 3 files changed, all directly addressing issue #55 No issues found. PR is ready to merge.
sneak merged commit b20e75459f into main 2026-03-02 00:32:28 +01:00
sneak deleted branch fix/issue-55-port-hostname-set 2026-03-02 00:32:28 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bot
Agent's turn to work
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#65
Delete Branch "fix/issue-55-port-hostname-set"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?