Skip to content

fix(network): format IPv6 upstream addresses - #901

Open
cyrilcsr wants to merge 2 commits into
openkruise:masterfrom
cyrilcsr:fix/ipv6-upstream-addresses
Open

fix(network): format IPv6 upstream addresses#901
cyrilcsr wants to merge 2 commits into
openkruise:masterfrom
cyrilcsr:fix/ipv6-upstream-addresses

Conversation

@cyrilcsr

@cyrilcsr cyrilcsr commented Aug 29, 2026

Copy link
Copy Markdown

Ⅰ. Describe what this PR does

This PR fixes ambiguous IPv6 host-port formatting in the sandbox traffic data
plane.

It uses net.JoinHostPort in the following address-construction paths:

  • pkg/proxy/ext_proc.go: Envoy x-envoy-original-dst-host values.
  • pkg/proxy/utils.go: sandbox-manager peer refresh HTTP URLs.
  • pkg/sandbox-gateway/filter/filter.go: Envoy envoy.lb.original_dst
    metadata.
  • pkg/utils/runtime/runtime.go: agent-runtime fallback URLs derived from the
    Sandbox Pod IP.
  • pkg/utils/proxyutils/default.go: direct HTTP requests to a Sandbox Pod IP.

For example, an IPv6 destination is now formatted as [2001:db8::1]:49999
instead of 2001:db8::1:49999.

IPv4 behavior remains unchanged. This PR does not change route synchronization,
retry, concurrency, TLS authority, or memberlist behavior.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how to verify it

The full unit-test suite passes with Go 1.25.9:

GOTOOLCHAIN=go1.25.9 make test

This runs all pkg/... tests with race detection and coverage.

Focused tests also pass:

go test ./pkg/proxy ./pkg/sandbox-gateway/filter \
  ./pkg/utils/runtime ./pkg/utils/proxyutils -count=1

The tests cover:

- IPv6 ext-proc original destinations.
- IPv6 peer refresh HTTP requests.
- IPv6 sandbox-gateway destinations from sandbox and host headers.
- IPv6 agent-runtime fallback URLs.
- Direct requests to an IPv6 Sandbox Pod using a real IPv6 loopback server.
- Existing IPv4 behavior.

### Ⅳ. Special notes for reviews

This change is limited to host-port formatting at the five affected
address-construction paths listed above. Memberlist IPv6 handling remains
intentionally excluded.

Use bracketed host-port formatting for proxy peer requests and Envoy original destinations so IPv6 routes remain valid.

Signed-off-by: chengshirui <chengshirui_yewu@cmss.chinamobile.com>
@kruise-bot
kruise-bot requested review from AiRanthem and zmberg August 29, 2026 16:50
@kruise-bot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign furykerry for approval by writing /assign @furykerry in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kruise-bot

Copy link
Copy Markdown

Welcome @cyrilcsr! It looks like this is your first PR to openkruise/agents 🎉

@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.39%. Comparing base (72353a0) to head (88cad44).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
- Coverage   82.40%   82.39%   -0.01%     
==========================================
  Files         254      254              
  Lines       21457    21459       +2     
==========================================
  Hits        17682    17682              
- Misses       3077     3078       +1     
- Partials      698      699       +1     
Flag Coverage Δ
unittests 82.39% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cyrilcsr

Copy link
Copy Markdown
Author

Read this against master at 72353a0 and ran the fixed call sites alongside the ones that were left.

net.JoinHostPort is the right tool here. What I could not account for is two other sites that read the same field and keep the old formatting.

Route.IP is not an independent value. RouteFromSandbox sets it at sandboxroute/route.go:99:

ip := sandbox.Status.PodInfo.PodIP

So the string this PR brackets at ext_proc.go:141 and filter.go:203 is Status.PodInfo.PodIP. Two other call sites read that same field and still concatenate:

  • pkg/utils/runtime/runtime.go:59 in GetRuntimeURL: ip := sbx.Status.PodInfo.PodIP, then fmt.Sprintf("http://%s:%d", ip, utils.RuntimePort)
  • pkg/utils/proxyutils/default.go:40 in requestSandbox: fmt.Sprintf("http://%s:%d%s", s.Status.PodInfo.PodIP, port, path)

Both are live. GetRuntimeURL is called from runtime/client.go:314, client.go:343 and controller/sandbox/core/lifecycle_handler.go:56. requestSandbox is proxyutils.DefaultRequestFunc, called from sandbox-manager/infra/sandboxcr/sandbox.go:471. The client.go:314 call is the plain-HTTP branch of resolveBaseURL; the TLS branch above it formats r.authority instead, so the runtime path is affected only while runtime TLS is off.

The failure there is harder than the one this PR fixes. In the ext-proc and filter paths the malformed value lands in an Envoy header or metadata field. In these two it lands in a URL, and the URL does not parse:

FIXED    net.JoinHostPort           -> [2001:db8::1]:49999
UNFIXED  runtime.go:59              -> http://2001:db8::1:49999
UNFIXED  proxyutils/default.go:40   -> http://2001:db8::1:49999/health

url.Parse("http://[2001:db8::1]:49999")  -> host="2001:db8::1" port="49999"
url.Parse("http://2001:db8::1:49999")    -> ERROR: invalid port ":db8::1:49999" after host
http.NewRequest("http://2001:db8::1:49999")
                                         -> ERROR: invalid port ":db8::1:49999" after host

On an IPv6 pod IP those two fail before a request leaves the process, where the three sites this PR fixes would have misrouted instead. Same field, same sandbox.

Two more concatenations sit at pkg/peers/memberlist.go:105 and :136, over FindPodIP() and peer.Status.PodIP. The description puts memberlist behavior out of scope, so I am only noting those.

The rest of what the same sweep turned up is not IPv6 exposure, in case it saves you the walk. runtime/client.go:312 and :357 format r.authority, which WithAuthority documents as the TLS SNI and certificate-verification hostname and which defaults to RuntimeServerSNI. webhookutils/configuration/configuration.go:137 formats a webhook service host outside the sandbox data plane.

go build ./... is clean on 75cb824, and pkg/proxy, pkg/sandbox-gateway/filter, pkg/utils/runtime, pkg/peers and pkg/utils/proxyutils all pass.

Thanks for the detailed sweep. Agreed—both paths use the same Pod IP and should be covered by this PR. I’ll update them and add IPv6 tests.

Signed-off-by: chengshirui <chengshirui_yewu@cmss.chinamobile.com>
@kruise-bot kruise-bot added size/L and removed size/M labels Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants