Skip to content

IPv6 correctness: NetworkPolicy /128 CIDR (AC-2) and IPv6-safe HTTPEndpoint URL parsing (AC-3) #334

Description

@matthyx

Follow-up to #333 (IPv6-only bind address). That PR makes the apiserver serve on IPv6-only clusters; these two remaining items are data-correctness bugs that produce wrong output for IPv6 inputs but do not block startup. Tracking them here so they aren't lost.

Both fixes use per-address detection (no global flag) and must not change behavior for IPv4/hostname inputs.


AC-2 — getSingleIP hardcodes /32 (should be /128 for IPv6)

File: pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go:476

func getSingleIP(ipAddress string) *softwarecomposition.IPBlock {
	ipBlock := &softwarecomposition.IPBlock{CIDR: ipAddress + "/32"}
	return ipBlock
}

A single IPv6 peer gets an invalid /32 CIDR (e.g. 2001:db8::1/32) instead of /128, corrupting generated NetworkPolicies on clusters with IPv6 peers.

Fix: detect the family and pick the suffix.

func getSingleIP(ipAddress string) *softwarecomposition.IPBlock {
	suffix := "/32"
	if ip := net.ParseIP(ipAddress); ip != nil && ip.To4() == nil {
		suffix = "/128"
	}
	return &softwarecomposition.IPBlock{CIDR: ipAddress + suffix}
}
  • net is already imported. To4() == nil cleanly means "true IPv6"; IPv4-mapped (::ffff:1.2.3.4) correctly stays /32; unparseable input keeps the /32 fallback (no panic).
  • One function fix covers both callers: the known-server path (networkpolicy.go:365) and the ingress path (networkpolicy.go:443).

Test: convert the single-case TestGetSingleIP (networkpolicy_test.go:2029-2038) to a table test — IPv4→/32, IPv6 (2001:db8::1)→/128, IPv4-mapped→/32, malformed→/32.


AC-3 — AnalyzeURL mis-parses IPv6 hosts

File: pkg/registry/file/dynamicpathdetector/analyze_endpoints.go:82-103

The function blindly prepends http:// to unbracketed input, so an IPv6 host is mis-parsed. Verified against net/url: ::1/health currently yields :1/health (wrong port) rather than the intended empty-port :/health; it does not error, so an error-based guard would miss it.

Fix (parse-first, empirically verified): if no scheme, isolate the authority (substring before first /); if not already bracketed:

  • (a) if netip.ParseAddr(authority).Is6() → bracket the whole authority, no port split;
  • (b) else if it splits on the last colon into host+port where netip.ParseAddr(host).Is6() and port is all digits → emit [host]:port;
  • (c) else prepend http:// unchanged.

Output contract is unchanged (:<port><path>; host discarded downstream). Add net/netip import.

Expected outputs (test cases — assert the raw AnalyzeURL return, not post-merge state):

Input Expected Note
::1/health :/health the bug fixed (was :1/health)
2001:db8::1/health :/health bare IPv6, no port — must not regress
2001:db8::8080/x :/x trailing all-digit hextet is NOT a port
[2001:db8::1]:8080/x :8080/x already correct — regression guard
2001:db8::1:8080/x :/x unbracketed IPv6+port malformed per RFC 3986 → whole address
example.com:80/users/123 :80/users/123 IPv4/host unchanged — regression guard
:80/users/123 :80/users/123 existing canonical form — regression guard
http://example.com:80/x :80/x scheme pass-through — regression guard
192.168.1.1:8080/path :8080/path IPv4-with-port — regression guard

Out of scope: IPv6 zone IDs (fe80::1%eth0) trigger a net/url escape error; declare unsupported. Note AnalyzeEndpoints (line 30) discards the error (silent drop) while ProcessEndpoint (line 52) propagates it — pre-existing behavior, not a regression here.

Reachability: AnalyzeURL is reached in production via applicationprofile_processor.go:117 and containerprofile_processor.go:816. Since the host is discarded downstream, this is largely defensive hardening, but it prevents endpoints from being silently mis-keyed.


Acceptance

  • AC-2: getSingleIP emits /128 for IPv6, /32 for IPv4; table test added.
  • AC-3: parse-first IPv6 bracketing; all table rows asserted against raw AnalyzeURL.
  • make build + make test pass with existing IPv4 expectations unchanged.

Full analysis (Architect + Critic consensus): .omc/plans/ipv6-only-support-consensus.md.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Accepted

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions