[management] Fix DNS peer IP corruption to ::1 during migration

[John-Dixon-IV]

Summary

• Root cause fix: The blob-to-JSON migration for peer IPs silently fell back to net.IPv6loopback (::1) when parsing failed, permanently corrupting the peer's IP in the database. Replaced with an error return so invalid data is caught at migration time.
• Defense-in-depth: Added IP validation in GetPeersCustomZone — peers with nil, IPv6, loopback, or unspecified IPs are now skipped with a logged error instead of generating invalid A records that miekg/dns silently rejects.
• No client-side changes needed: The local resolver's AAAA handling already correctly returns NODATA when only A records exist.

Impact

Peers with corrupted IPs (e.g. from failed blob migration on hosts with IPv6 disabled) would lose their DNS records entirely. On Windows clients, this broke mapped drives because Windows would fall through to upstream DNS and prefer IPv6 loopback over the correct WireGuard IP.

Test plan

• TestMigrateNetIPFieldFromBlobToJSON_WithInvalidBlobData — verifies migration fails on corrupt IP blobs instead of silently using ::1
• TestGetPeersCustomZone_InvalidIPs — verifies peers with nil/IPv6/loopback/unspecified IPs are skipped
• All existing migration tests pass (17/17)
• All existing account types tests pass (78/78)

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling during IP field migration: invalid IP data now returns detailed error messages identifying the affected row instead of silently applying default values.
  • Added IP validation in DNS zone generation: peer IPs are now validated to ensure they are valid IPv4 addresses and properly formatted before DNS record creation.

• Tests

  • Added test coverage for invalid IP data handling during migrations and DNS operations.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca5953 and ad4560d.

📒 Files selected for processing (4)

• management/server/migration/migration.go
• management/server/migration/migration_test.go
• management/server/types/account.go
• management/server/types/account_test.go

---

📝 Walkthrough

Walkthrough

Changes add stricter validation for IP addresses in two contexts: during database migration to detect and report invalid IP blob data, and in DNS zone generation to filter out invalid peer IPs (non-IPv4, loopback, nil, or unspecified). Migration failures now return explicit errors instead of falling back to defaults.

Changes

Cohort / File(s)	Summary
Migration Error Handling management/server/migration/migration.go, management/server/migration/migration_test.go	Modified IP parse failure behavior to return formatted errors with column and row ID instead of logging and using IPv6 loopback fallback. Added test validating error message for invalid blob data.
DNS Zone Generation Validation management/server/types/account.go, management/server/types/account_test.go	Added IP validation in GetPeersCustomZone to filter peers: requires non-nil, IPv4, non-loopback, and non-unspecified IPs before creating A records. Added test covering multiple invalid IP scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• crn4

Poem

🐰 hop hop Through migrations we now dare,
Invalid IPs caught with extra care!
No silent falls to loopback's embrace,
DNS records get proper vetting grace.
wiggles nose approvingly ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the root cause, the fix applied, impact, and test plan, but does not follow the required template structure with the specified sections and checklist items.	Follow the repository's description template by filling in all required sections including 'Describe your changes', 'Issue ticket number', checklist items, and documentation requirements.
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main fix: resolving DNS peer IP corruption to IPv6 loopback during the migration process.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud