Skip to content

Conversation

@theRealWardo
Copy link
Member

setting this flag should force all packets to be routed to a specified port. I did not add support for overriding the destination address.

in practice, this allows users to not worry about setting a port in an HTTPBackendRef or any other API object that references a tunnel.


// Override port if specified
if overridePort != "" {
if port, err := netip.ParseAddrPort("127.0.0.1:" + overridePort); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just strconv.Atoi instead - this is confusing (why is there 127.0.0.1 in there?) Also it swallows the error

Comment on lines +60 to +64

// Extract IPv4 address from IPv6-mapped address
dstAddr := Unmap4in6(addrFromNetstackIP(reqDetails.LocalAddress))
dstPort := reqDetails.LocalPort

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Extract IPv4 address from IPv6-mapped address
dstAddr := Unmap4in6(addrFromNetstackIP(reqDetails.LocalAddress))
dstPort := reqDetails.LocalPort
dstAddrPort := netip.AddrPortFrom(
Unmap4in6(addrFromNetstackIP(reqDetails.LocalAddress)),
reqDetails.LocalPort,
)
if overridePort != 0 {
logger.Debug("Overriding port", "original_port", dstAddrPort.Port, "ovveride_port", overridePort)
}

These comments are not needed

tunnelRunCmd.Flags().BoolVar(&autoCreate, "auto", false, "Automatically create TunnelNode if it doesn't exist.")
tunnelRunCmd.Flags().StringVar(&healthAddr, "health-addr", ":8080", "Listen address for health endpoint (default: :8080).")
tunnelRunCmd.Flags().StringVar(&metricsAddr, "metrics-addr", ":8081", "Listen address for metrics endpoint (default: :8081).")
tunnelRunCmd.Flags().StringVar(&overridePort, "port", "", "Override destination port for forwarded packets (empty string preserves original port).")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Port should be a uint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants