fix(ssh): SSH port was not assigned to gvproxy#94
Conversation
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR makes two configuration-related changes: it modifies the gvproxy command to assign SSHPort from the machine configuration's SSH.Port field, enabling the gvproxy process to receive SSH port details in its command configuration. Additionally, it updates the port package's GetFree function to use "tcp4" instead of "tcp" for network binding, restricting listener binding to IPv4 addresses only. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/port/port.go (1)
16-16: Approve explicit IPv4 binding, but consider consistency withIsListening().The change to
"tcp4"makes the IPv4-only intent explicit, which is appropriate sinceLocalHostURLis"127.0.0.1". However,IsListening()on line 29 still uses"tcp"in itsDialTimeoutcall, creating an inconsistency in the codebase.🔄 Suggested refactor for consistency
Consider updating
IsListening()to also use"tcp4"for consistency:func IsListening(port int) bool { conn, err := net.DialTimeout("tcp4", fmt.Sprintf("%s:%d", define.LocalHostURL, port), defaultDialTimeout) if err != nil { return false } _ = conn.Close() return true }This ensures both functions use the same network type, making the code more maintainable and avoiding potential edge-case behavior differences across systems.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/machine/gvproxy/gvproxy.gopkg/port/port.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/port/port.go (1)
pkg/machine/define/define.go (1)
LocalHostURL(37-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
pkg/machine/gvproxy/gvproxy.go (1)
49-49: LGTM! SSH port assignment correctly implemented.The SSH port is now properly assigned to the gvproxy command configuration after being obtained via
GetSSHPort(). This ensures that the gvproxy process receives the SSH port information, which addresses the core issue described in the PR title.
No description provided.