fix: fixed tcp proxying to container ip

[dj0024javia]

Fixes #395

[dj0024javia]

I don't know what the action failure is. can you help me out here? seems like a warning.

[dj0024javia]

i noticed the tailscale version is 1.95.x, which is causing cert issues.
the v2 image has 1.80.3.

[almeidapaulopt]

PR #397 Review — fix: fixed tcp proxying to container ip

Summary

The PR attempts to fix TCP proxying to Docker containers (issue #395) by using container IP instead of host.docker.internal for TCP traffic. It also bundles an unrelated Tailscale version downgrade, API migration, and Dockerfile changes.

---

🔴 Critical Issues

1. Unrelated changes bundled together (scope creep)

This PR mixes 3 distinct concerns that should be separate PRs:

• TCP proxying fix (the actual bug fix for #395) — container.go, port.go
• Tailscale client API migration (local → v2) — proxy.go
• Dependency downgrades + Dockerfile changes — go.mod, Dockerfile

If the Tailscale downgrade breaks something, it blocks the TCP fix and vice versa. Each should be independently reviewable and revertable.

2. Tailscale downgrade from v1.96.5 → v1.80.3 is dangerous

Downgrading tailscale.com by 16 minor versions is a significant regression:

• v1.80.3 is from ~mid-2024. The current codebase may depend on features/fixes in newer versions.
• If there are cert issues with v1.96.5, the right fix is to investigate the root cause, not downgrade.
• This could introduce security vulnerabilities fixed in the 16 versions between.

Recommendation: Remove the downgrade. Investigate the cert issue separately if it exists.

3. Deleting go.sum is a bad practice

Deleting the entire go.sum file (438 lines) should never be committed. Instead:

• Run go mod tidy to regenerate it properly
• CI may fail if go mod verify runs
• Loss of all integrity checksums until regenerated

4. Tailscale API migration needs verification

-	"tailscale.com/client/local"
+	tsc "tailscale.com/client/tailscale/v2"

local.Client (from tsnet.Server.LocalClient()) and tsc.Client (from tailscale.com/client/tailscale/v2) have different APIs. The methods used (WhoIs, WatchIPNBus, Status, CertPair) must be verified to exist on the v2 client with the same signatures. Without evidence, this is a potential breaking change.

5. parseTargetSegment changes the default protocol for ALL users

-	targetProtocol := "http"
+	targetProtocol := "tcp"

This changes the default target protocol from http to tcp. Any existing port label without an explicit protocol (e.g., "443:8080") would now get tcp:// as scheme, then gets "fixed back" to http by this convoluted logic:

if scheme != "tcp" && (scheme == "" || scheme == "http" || scheme == "https") {
    scheme = "http"
}

This is fragile and confusing. Keep targetProtocol = "http" as default and only set IsTCPPassthrough when tcp is explicitly specified:

targetProtocol := "http"
if len(targetParts) == 2 {
    targetProtocol = targetParts[1]
}
config.IsTCPPassthrough = targetProtocol == "tcp"

---

🟡 Moderate Issues (TCP Fix)

6. Container IP fallback should warn

if isTCPPassthrough && len(c.ipAddress) > 0 {
    targetHostname = c.ipAddress[0]

If c.ipAddress is empty, it silently falls back to defaultTargetHostname (typically host.docker.internal). For TCP passthrough, routing through Docker's port mapping is exactly the bug described in #395. The fallback should log a warning.

7. getTargetURL logic is confusing with mixed usePort/publishedPort gates

The outer condition still checks publishedPort == "" but the inner check uses usePort. When isTCPPassthrough is true and internalPort is set but publishedPort is empty, the code works but the logic flow is hard to follow. Consider simplifying.

8. Dockerfile Alpine → Debian change is bundled without justification

The switch from golang:1.24 Alpine (apk) to golang:1.26 Debian (apt-get) increases builder stage size from ~300MB to ~800MB. While the FROM scratch final stage isn't affected, this increases CI build time and should be a separate change with reasoning.

---

🟢 What's Good

• The core idea is correct: TCP needs direct container access via IP, not Docker's port mapping layer (host.docker.internal).
• The tcpPort handler in proxymanager/port.go already correctly dials the target using raw TCP — the fix is in the right place (target URL construction).
• The IsTCPPassthrough field on PortConfig is a clean way to propagate this information.

---

Recommendations

1. Split into 3 separate PRs — TCP fix only for this one
2. Remove the Tailscale downgrade — investigate cert issues separately
3. Don't delete go.sum — run go mod tidy instead
4. Keep targetProtocol default as "http" — set IsTCPPassthrough only on explicit tcp
5. Add a test for parseTargetSegment TCP case to prevent regression
6. Log a warning when container IP isn't available for TCP passthrough

Removing review - posted by mistake

[dj0024javia]

yep, i am trying to fix some other issues, related to cert, but it seemed like that was my domain, hitting rate limits, I'll clean the PR in few days and send it for re-review

thanks for taking a look @almeidapaulopt 🚀

Cheers!