fix: Inconsistent results with probe-all-ips if the port number is set in the URL

[majiayu000]

Fixes #2346

Changes

• Use URL.Hostname() instead of URL.Host when extracting host for DNS lookup in probe-all-ips mode
• Added test case for probe-all-ips with port in URL

Summary by CodeRabbit

• Bug Fixes

  • Enhanced DNS lookup logic to generate separate target entries for each resolved IP address while preserving the original hostname. Gracefully falls back to the original target if DNS resolution fails.

• Tests

  • Added test coverage for DNS-based target resolution, validating correct behavior with port-included inputs and resolved IP assignment.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes fix the probe-all-IPs functionality when URLs include port numbers by switching DNS lookups from URL.Host (which includes the port) to URL.Hostname() (hostname only). This ensures proper DNS resolution and generation of multiple Target entries with CustomIP when multiple IPs are resolved, rather than returning only a single IP.

Changes

Cohort / File(s)	Summary
DNS Lookup Logic runner/runner.go	Changed DNS data lookup to use URL.Hostname() instead of URL.Host to exclude port from hostname resolution. When DNS resolution succeeds, emits Target entries with Host set to the original target string and CustomIP set to each resolved IP; otherwise falls back to original target.
Test Coverage runner/runner_test.go	Added TestRunner_probeall_targets_with_port to validate that probe-all-IPs preserves the full input host when a port is included and produces CustomIP entries for all resolved IPs. Minor whitespace alignment in TestCreateNetworkpolicyInstance_AllowDenyFlags.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A port in the name caused DNS despair,
But Hostname() now resolves with care!
All IPs discovered, no more just one,
The probe-all-IPs feature—finally done! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the bug fix: inconsistent results with probe-all-ips when a port is set in the URL, which matches the main change.
Linked Issues check	✅ Passed	The changes implement the required fix: using URL.Hostname() instead of URL.Host for DNS lookup, and include a test case for probe-all-ips with ports.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the probe-all-ips port handling issue. Minor whitespace adjustments in tests are incidental formatting improvements.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f9403e and 7cfcff3.

📒 Files selected for processing (2)

• runner/runner.go
• runner/runner_test.go

🧰 Additional context used

🧬 Code graph analysis (1)

runner/runner_test.go (2)

runner/options.go (1)

• Options (173-358)

common/httpx/types.go (1)

• Target (4-8)

🔇 Additional comments (2)

runner/runner.go (1)

1565-1565: LGTM! Correct fix for DNS lookup with ports.

The change from URL.Host to URL.Hostname() is the proper fix. DNS lookups require the hostname only, not the hostname:port combination. This aligns with how getDNSData is called elsewhere in the codebase (line 2027 extracts the hostname first via net.SplitHostPort).

runner/runner_test.go (1)

70-98: LGTM! Comprehensive test coverage for the port handling fix.

The test validates the key behaviors:

• Same number of IPs resolved with or without port
• Host field preserves the full input including the port
• CustomIP is populated for each resolved IP

This provides strong coverage for the fix in runner.go line 1565.

---

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.}

[dogancanbakir]

Thanks for the PR!