feat: support IPv6-only clusters via configurable bind address#333
Conversation
The storage apiserver never set a bind address, leaving it at the apiserver default 0.0.0.0, which cannot accept connections on IPv6-only clusters. The binary takes no apiserver flags (main.go runs flag.Parse with only -kubeconfig registered), so --bind-address cannot be passed at runtime; configuration is the only lever. Add a serverBindAddress config field wired into SecureServing.BindAddress (mirroring the existing serverBindPort), defaulting to "::" so IPv6-only clusters serve out of the box. With bindv6only=0 (Linux default) "::" also accepts IPv4/dual-stack traffic; operators can force IPv4-only with serverBindAddress: "0.0.0.0". Also add ::1 to the self-signed cert loopback SANs so IPv6 loopback TLS works on the self-signed path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
📝 WalkthroughWalkthroughThis PR adds IPv6 dual-stack support to the storage server by introducing a configurable ChangesIPv6 Dual-Stack Server Bind Address
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/cmd/server/start.go`:
- Around line 127-129: The code currently ignores an unparsable
o.StorageConfig.ServerBindAddress in the ParseIPSloppy block; change the
behavior to fail fast by adding validation in the Validate() method: call
netutils.ParseIPSloppy(o.StorageConfig.ServerBindAddress) there and if the
provided ServerBindAddress is non-empty and ParseIPSloppy returns nil, return a
descriptive error; keep the current assignment to
o.RecommendedOptions.SecureServing.BindAddress only when ParseIPSloppy succeeds.
Ensure you reference ParseIPSloppy, o.StorageConfig.ServerBindAddress,
o.RecommendedOptions.SecureServing.BindAddress and the Validate() method when
implementing this check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22e76f6e-8e0f-4a8e-bf2c-f1cde062e46f
📒 Files selected for processing (3)
pkg/cmd/server/start.gopkg/config/config.gopkg/config/config_test.go
| if bindIP := netutils.ParseIPSloppy(o.StorageConfig.ServerBindAddress); bindIP != nil { | ||
| o.RecommendedOptions.SecureServing.BindAddress = bindIP | ||
| } |
There was a problem hiding this comment.
Fail fast on invalid serverBindAddress instead of silently falling back.
At Line 127, an unparsable value is ignored and the server continues with the default bind address. A typo in config can unintentionally expose/listen on an unintended interface and break IPv6-only intent. Validate this in Validate() and return an error.
Suggested fix
func (o WardleServerOptions) Validate(_ []string) error {
var errors []error
errors = append(errors, o.RecommendedOptions.Validate()...)
errors = append(errors, o.ComponentGlobalsRegistry.Validate()...)
+ if ip := netutils.ParseIPSloppy(o.StorageConfig.ServerBindAddress); ip == nil {
+ errors = append(errors, fmt.Errorf("invalid serverBindAddress %q: expected an IP literal", o.StorageConfig.ServerBindAddress))
+ }
return utilerrors.NewAggregate(errors)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/cmd/server/start.go` around lines 127 - 129, The code currently ignores
an unparsable o.StorageConfig.ServerBindAddress in the ParseIPSloppy block;
change the behavior to fail fast by adding validation in the Validate() method:
call netutils.ParseIPSloppy(o.StorageConfig.ServerBindAddress) there and if the
provided ServerBindAddress is non-empty and ParseIPSloppy returns nil, return a
descriptive error; keep the current assignment to
o.RecommendedOptions.SecureServing.BindAddress only when ParseIPSloppy succeeds.
Ensure you reference ParseIPSloppy, o.StorageConfig.ServerBindAddress,
o.RecommendedOptions.SecureServing.BindAddress and the Validate() method when
implementing this check.
|
Summary:
|
Problem
The storage apiserver does not come up on IPv6-only clusters. Confirmed from a live pod: it listens on
0.0.0.0:8443(the apiserver default), which cannot accept IPv6 connections.Root cause:
SecureServing.BindAddressis never set. The binary is config-driven, not flag-driven —main.goruns stdlibflag.Parse()with only-kubeconfigregistered, so passing--bind-addressto the process is rejected (flag provided but not defined: -bind-address). Configuration is the only available lever.Change
serverBindAddressconfig field (pkg/config/config.go) and wire it intoSecureServing.BindAddressinpkg/cmd/server/start.go, mirroring howserverBindPortis already handled."::"so IPv6-only clusters serve with no Helm changes. Withbindv6only=0(the Linux default)"::"also accepts IPv4/dual-stack traffic, so existing clusters keep working.serverBindAddress: "0.0.0.0"to force IPv4-only binding.::1to the self-signed cert loopback SANs (start.go) for IPv6 loopback TLS on the self-signed path.Testing
go build ./...passes.go test ./pkg/config/...passes (updatedTestLoadConfigexpectation for the new default).Notes
serverBindAddressvalue flows through the existing config mount (/etc/config/config.json); the Helm chart can expose it but no chart change is required for the default to take effect./128) and IPv6-safe HTTPEndpoint URL parsing — data-correctness fixes tracked separately.🤖 Generated with Claude Code
Summary by CodeRabbit