Skip to content

Use net.JoinHostPort() for IPv6-safe host:port formatting#11959

Merged
hjiawei merged 1 commit into
projectcalico:masterfrom
hjiawei:join-host-port-fv
Mar 2, 2026
Merged

Use net.JoinHostPort() for IPv6-safe host:port formatting#11959
hjiawei merged 1 commit into
projectcalico:masterfrom
hjiawei:join-host-port-fv

Conversation

@hjiawei
Copy link
Copy Markdown
Contributor

@hjiawei hjiawei commented Feb 28, 2026

Go 1.26 tightened net/url parsing to reject bare IPv6 addresses in URLs (issue #75223). URLs like http://2001:db8::1:9099/path were silently accepted in Go 1.25 but now correctly return a parse error. This broke all IPv6 BPF FV tests which construct health check URLs with unbracketed IPv6 addresses.

Replace string concatenation with net.JoinHostPort() which correctly brackets IPv6 addresses ([::1]:9099) and leaves IPv4 unchanged.

Release note:

TBD

Go 1.26 tightened net/url parsing to reject bare IPv6 addresses in
URLs (issue #75223). URLs like http://2001:db8::1:9099/path were
silently accepted in Go 1.25 but now correctly return a parse error.
This broke all IPv6 BPF FV tests which construct health check URLs
with unbracketed IPv6 addresses.

Replace string concatenation with net.JoinHostPort() which correctly
brackets IPv6 addresses ([::1]:9099) and leaves IPv4 unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hjiawei hjiawei requested review from coutinhop and fasaxc February 28, 2026 16:52
@hjiawei hjiawei requested a review from a team as a code owner February 28, 2026 16:52
Copilot AI review requested due to automatic review settings February 28, 2026 16:52
@hjiawei hjiawei requested a review from a team as a code owner February 28, 2026 16:52
@marvin-tigera marvin-tigera added this to the Calico v3.32.0 milestone Feb 28, 2026
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Feb 28, 2026
@hjiawei hjiawei added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact and removed release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Feb 28, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates several Calico components and FV helpers to use net.JoinHostPort() instead of manual host + ":" + port formatting, ensuring IPv6 addresses are correctly bracketed when constructing host:port strings and URLs under stricter Go net/url parsing.

Changes:

  • Replace manual host:port string concatenation/formatting with net.JoinHostPort() in multiple packages.
  • Fix Felix/node health and FV metrics/readiness URL construction to be IPv6-safe.
  • Update server listen address formatting to be IPv6-safe.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
whisker-backend/pkg/config/api.go Use net.JoinHostPort() for HostAddr() to support IPv6-safe bind/address formatting.
node/pkg/health/health.go Build Felix liveness/readiness endpoints with net.JoinHostPort() for IPv6-safe health URLs.
guardian/pkg/server/server.go Use net.JoinHostPort() when binding the cluster listener to support IPv6 hosts.
felix/fv/metrics/metrics.go Use net.JoinHostPort() when constructing the metrics URL to support IPv6 Felix IPs.
felix/fv/infrastructure/felix.go Use net.JoinHostPort() for Felix readiness URL construction in FV infrastructure helper.
app-policy/cmd/dikastes/dikastes.go Use net.JoinHostPort() for the HTTP termination server address to support IPv6 binds.

Comment on lines 130 to 137
logrus.Infof("Listening on %s:%s for connections to proxy to voltron", srv.listenHost, srv.listenPort)
if err := srv.tunnel.Connect(srv.shutdownCtx); err != nil {
return fmt.Errorf("failed to connect to tunnel: %w", err)
}

listener, err := net.Listen("tcp", fmt.Sprintf("%s:%s", srv.listenHost, srv.listenPort))
listener, err := net.Listen("tcp", net.JoinHostPort(srv.listenHost, srv.listenPort))
if err != nil {
return fmt.Errorf("failed to listen on %s:%s: %w", srv.listenHost, srv.listenPort, err)
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The log/error messages still format the bind address as "%s:%s", which will be misleading for IPv6 hosts (e.g., "::" becomes ":::8080") even though the listener now uses net.JoinHostPort(). Consider using net.JoinHostPort(srv.listenHost, srv.listenPort) (or listener.Addr().String()) in the Infof and error wrapping so IPv6 addresses are rendered unambiguously.

Copilot uses AI. Check for mistakes.
Comment on lines 423 to 425

url := "http://" + healthAddr + ":9099/readiness"
url := "http://" + net.JoinHostPort(healthAddr, "9099") + "/readiness"
ctx, cancel := context.WithTimeout(context.Background(), time.Second*2)
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

PR description says IPv6 BPF FV tests broke due to unbracketed IPv6 in URLs and that this PR replaces string concatenation with JoinHostPort(), but there are still unbracketed host:port URL constructions elsewhere (e.g., felix/fv/health_test.go:429 uses "http://"+felix.IP+":9099/..." and felix/k8sfv/metric.go:28 uses "http://"+felixIP+":9091/...", plus felix/fv/utils/utils.go:203 for etcd endpoints). Either expand this PR to cover these remaining cases or narrow the description/scope accordingly; otherwise IPv6 test coverage may still fail under the stricter net/url parsing.

Copilot uses AI. Check for mistakes.
Comment on lines 58 to 60
func (cfg *Config) HostAddr() string {
return fmt.Sprintf("%s:%s", cfg.Host, cfg.Port)
return net.JoinHostPort(cfg.Host, cfg.Port)
}
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The PR body still contains a release-note block with "TBD". The repo's release-note validation workflow fails when release notes are required and the block is left as "TBD", so this may need to be updated to a one-line user-visible note (or set to "None" if appropriate).

Copilot uses AI. Check for mistakes.
@hjiawei hjiawei merged commit 9920ad0 into projectcalico:master Mar 2, 2026
10 of 12 checks passed
@hjiawei hjiawei deleted the join-host-port-fv branch March 2, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants