Skip to content

Commit dc759eb

Browse files
committed
Add IPv6 support for EndpointSlice creation and port replacement
The autoscaler stat forwarder hardcoded AddressTypeIPv4 when creating EndpointSlices, which breaks on IPv6-only or dual-stack clusters. Additionally, useSecurePort used strings.Split on ":" to extract the host, which breaks on bracketed IPv6 addresses like [::1]:8012. Changes: - Add addressTypeForIP helper to dynamically determine AddressType - Fix useSecurePort to use net.SplitHostPort/net.JoinHostPort - Add tests for both functions with IPv4 and IPv6 cases Signed-off-by: Vincent Link <vlink@redhat.com>
1 parent fb213e0 commit dc759eb

4 files changed

Lines changed: 68 additions & 3 deletions

File tree

pkg/activator/handler/handler.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"net"
2324
"net/http"
2425
"net/http/httputil"
2526
"strconv"
@@ -184,8 +185,11 @@ func (a *activationHandler) proxyRequest(revID types.NamespacedName, w http.Resp
184185
// TODO: endpointsToDests() should support HTTPS instead of this overwrite but it needs metadata request to be encrypted.
185186
// This code should be removed when https://github.com/knative/serving/issues/12821 was solved.
186187
func useSecurePort(target string, port int) string {
187-
target = strings.Split(target, ":")[0]
188-
return target + ":" + strconv.Itoa(port)
188+
host, _, err := net.SplitHostPort(target)
189+
if err != nil {
190+
host = target
191+
}
192+
return net.JoinHostPort(host, strconv.Itoa(port))
189193
}
190194

191195
func WrapActivatorHandlerWithFullDuplex(h http.Handler, logger *zap.SugaredLogger) http.HandlerFunc {

pkg/activator/handler/handler_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,34 @@ func setupConfigStore(t testing.TB, logger *zap.SugaredLogger) *activatorconfig.
290290
return configStore
291291
}
292292

293+
func TestUseSecurePort(t *testing.T) {
294+
tests := map[string]struct {
295+
target string
296+
port int
297+
want string
298+
}{
299+
"ipv4": {
300+
target: "10.0.0.1:8012",
301+
port: 8112,
302+
want: "10.0.0.1:8112",
303+
},
304+
"ipv6": {
305+
target: "[2001:db8::1]:8012",
306+
port: 8112,
307+
want: "[2001:db8::1]:8112",
308+
},
309+
}
310+
311+
for name, tc := range tests {
312+
t.Run(name, func(t *testing.T) {
313+
got := useSecurePort(tc.target, tc.port)
314+
if got != tc.want {
315+
t.Errorf("useSecurePort(%q, %d) = %q, want %q", tc.target, tc.port, got, tc.want)
316+
}
317+
})
318+
}
319+
}
320+
293321
func BenchmarkHandler(b *testing.B) {
294322
ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(b)
295323
b.Cleanup(cancel)

pkg/autoscaler/statforwarder/forwarder_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,3 +509,28 @@ func TestIsBucketOwner(t *testing.T) {
509509
t.Errorf("IsBktOwner(not-in-record) = %v, want true", got)
510510
}
511511
}
512+
513+
func TestAddressTypeForIP(t *testing.T) {
514+
tests := map[string]struct {
515+
ip string
516+
want discoveryv1.AddressType
517+
}{
518+
"ipv4": {
519+
ip: "10.0.0.1",
520+
want: discoveryv1.AddressTypeIPv4,
521+
},
522+
"ipv6": {
523+
ip: "2001:db8::1",
524+
want: discoveryv1.AddressTypeIPv6,
525+
},
526+
}
527+
528+
for name, tc := range tests {
529+
t.Run(name, func(t *testing.T) {
530+
got := addressTypeForIP(tc.ip)
531+
if got != tc.want {
532+
t.Errorf("addressTypeForIP(%q) = %v, want %v", tc.ip, got, tc.want)
533+
}
534+
})
535+
}
536+
}

pkg/autoscaler/statforwarder/leases.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ func (f *leaseTracker) createOrUpdateEndpoints(ctx context.Context, ns, n string
245245
discoveryv1.LabelServiceName: n,
246246
},
247247
},
248-
AddressType: discoveryv1.AddressTypeIPv4,
248+
AddressType: addressTypeForIP(f.selfIP),
249249
Endpoints: []discoveryv1.Endpoint{{
250250
Addresses: []string{f.selfIP},
251251
Conditions: discoveryv1.EndpointConditions{
@@ -307,3 +307,11 @@ func (f *leaseTracker) createOrUpdateEndpoints(ctx context.Context, ns, n string
307307

308308
return nil
309309
}
310+
311+
func addressTypeForIP(ip string) discoveryv1.AddressType {
312+
parsed := net.ParseIP(ip)
313+
if parsed == nil || parsed.To4() != nil {
314+
return discoveryv1.AddressTypeIPv4
315+
}
316+
return discoveryv1.AddressTypeIPv6
317+
}

0 commit comments

Comments
 (0)