diff --git a/kubelet-to-gcm/monitor/controller/client.go b/kubelet-to-gcm/monitor/controller/client.go index 6abaef409..9d488cd4e 100644 --- a/kubelet-to-gcm/monitor/controller/client.go +++ b/kubelet-to-gcm/monitor/controller/client.go @@ -19,6 +19,7 @@ package controller import ( "fmt" "io" + "net" "net/http" "net/url" "strings" @@ -87,7 +88,7 @@ type Client struct { // NewClient generates a client to hit the given controller. func NewClient(host string, port uint, client *http.Client) (*Client, error) { // Parse our URL upfront, so we can fail fast. - urlStr := fmt.Sprintf("http://%s:%d/metrics", host, port) + urlStr := fmt.Sprintf("http://%s/metrics", net.JoinHostPort(util.NormalizeHost(host), fmt.Sprint(port))) metricsURL, err := url.Parse(urlStr) if err != nil { return nil, err diff --git a/kubelet-to-gcm/monitor/controller/client_test.go b/kubelet-to-gcm/monitor/controller/client_test.go index 2bff8fca0..a2e0f6532 100644 --- a/kubelet-to-gcm/monitor/controller/client_test.go +++ b/kubelet-to-gcm/monitor/controller/client_test.go @@ -26,6 +26,52 @@ import ( "github.com/GoogleCloudPlatform/k8s-stackdriver/kubelet-to-gcm/monitor/util" ) +func TestNewClient(t *testing.T) { + testCases := []struct { + name string + host string + port uint + expectedURL string + }{ + { + name: "IPv4", + host: "127.0.0.1", + port: 10250, + expectedURL: "http://127.0.0.1:10250/metrics", + }, + { + name: "IPv6", + host: "2001:db8::1", + port: 10250, + expectedURL: "http://[2001:db8::1]:10250/metrics", + }, + { + name: "Bracketed IPv6", + host: "[2001:db8::1]", + port: 10250, + expectedURL: "http://[2001:db8::1]:10250/metrics", + }, + { + name: "Hostname", + host: "localhost", + port: 8080, + expectedURL: "http://localhost:8080/metrics", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + c, err := NewClient(tc.host, tc.port, http.DefaultClient) + if err != nil { + t.Fatalf("NewClient failed: %v", err) + } + if c.metricsURL.String() != tc.expectedURL { + t.Errorf("Expected URL %q, got %q", tc.expectedURL, c.metricsURL.String()) + } + }) + } +} + func TestDoRequestAndParse_SizeLimit(t *testing.T) { // Create a mock server that returns a response larger than maxResponseBodySize. largeDataSize := maxResponseBodySize + 1024 diff --git a/kubelet-to-gcm/monitor/kubelet/client.go b/kubelet-to-gcm/monitor/kubelet/client.go index 222545154..6b1a99654 100644 --- a/kubelet-to-gcm/monitor/kubelet/client.go +++ b/kubelet-to-gcm/monitor/kubelet/client.go @@ -19,6 +19,7 @@ package kubelet import ( "encoding/json" "fmt" + "net" "net/http" "net/url" @@ -46,7 +47,7 @@ func NewClient(host string, port uint, client *http.Client, useAuthPort bool) (* if useAuthPort { protocol = "https" } - urlStr := fmt.Sprintf("%s://%s:%d/stats/summary", protocol, host, port) + urlStr := fmt.Sprintf("%s://%s/stats/summary", protocol, net.JoinHostPort(util.NormalizeHost(host), fmt.Sprint(port))) summaryURL, err := url.Parse(urlStr) if err != nil { return nil, err diff --git a/kubelet-to-gcm/monitor/kubelet/client_test.go b/kubelet-to-gcm/monitor/kubelet/client_test.go index cdbf0d994..0c6edb8c8 100644 --- a/kubelet-to-gcm/monitor/kubelet/client_test.go +++ b/kubelet-to-gcm/monitor/kubelet/client_test.go @@ -28,6 +28,71 @@ import ( stats "k8s.io/kubelet/pkg/apis/stats/v1alpha1" ) +func TestNewClient(t *testing.T) { + testCases := []struct { + name string + host string + port uint + useAuthPort bool + expectedURL string + }{ + { + name: "IPv4 HTTP", + host: "127.0.0.1", + port: 10255, + useAuthPort: false, + expectedURL: "http://127.0.0.1:10255/stats/summary", + }, + { + name: "IPv4 HTTPS", + host: "127.0.0.1", + port: 10250, + useAuthPort: true, + expectedURL: "https://127.0.0.1:10250/stats/summary", + }, + { + name: "IPv6 HTTP", + host: "2001:db8::1", + port: 10255, + useAuthPort: false, + expectedURL: "http://[2001:db8::1]:10255/stats/summary", + }, + { + name: "IPv6 HTTPS", + host: "2001:db8::1", + port: 10250, + useAuthPort: true, + expectedURL: "https://[2001:db8::1]:10250/stats/summary", + }, + { + name: "Bracketed IPv6 HTTP", + host: "[2001:db8::1]", + port: 10255, + useAuthPort: false, + expectedURL: "http://[2001:db8::1]:10255/stats/summary", + }, + { + name: "Bracketed IPv6 HTTPS", + host: "[2001:db8::1]", + port: 10250, + useAuthPort: true, + expectedURL: "https://[2001:db8::1]:10250/stats/summary", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + c, err := NewClient(tc.host, tc.port, http.DefaultClient, tc.useAuthPort) + if err != nil { + t.Fatalf("NewClient failed: %v", err) + } + if c.summaryURL.String() != tc.expectedURL { + t.Errorf("Expected URL %q, got %q", tc.expectedURL, c.summaryURL.String()) + } + }) + } +} + func TestDoRequestAndUnmarshal_SizeLimit(t *testing.T) { largeDataSize := maxResponseBodySize + 1024 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/kubelet-to-gcm/monitor/util/util.go b/kubelet-to-gcm/monitor/util/util.go index 93fbee007..d84a965fc 100644 --- a/kubelet-to-gcm/monitor/util/util.go +++ b/kubelet-to-gcm/monitor/util/util.go @@ -19,6 +19,7 @@ package util import ( "errors" "io" + "strings" ) var ( @@ -39,3 +40,14 @@ func ReadWithLimit(r io.Reader, limit int64) ([]byte, error) { } return data, nil } + +// NormalizeHost handles host strings that may already be bracketed (like IPv6). +// net.JoinHostPort expects a raw IP or hostname; if it receives a bracketed IPv6, +// it will double-bracket it. +func NormalizeHost(host string) string { + host = strings.TrimSpace(host) + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { + return host[1 : len(host)-1] + } + return host +} diff --git a/kubelet-to-gcm/monitor/util/util_test.go b/kubelet-to-gcm/monitor/util/util_test.go index 09a2e7b4d..0434c83bb 100644 --- a/kubelet-to-gcm/monitor/util/util_test.go +++ b/kubelet-to-gcm/monitor/util/util_test.go @@ -65,3 +65,45 @@ func TestReadWithLimit(t *testing.T) { }) } } + +func TestNormalizeHost(t *testing.T) { + testCases := []struct { + desc string + input string + expected string + }{ + { + desc: "IPv4", + input: "127.0.0.1", + expected: "127.0.0.1", + }, + { + desc: "hostname", + input: "localhost", + expected: "localhost", + }, + { + desc: "raw IPv6", + input: "2001:db8::1", + expected: "2001:db8::1", + }, + { + desc: "bracketed IPv6", + input: "[2001:db8::1]", + expected: "2001:db8::1", + }, + { + desc: "bracketed IPv6 with spaces", + input: " [2001:db8::1] ", + expected: "2001:db8::1", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + if got := NormalizeHost(tc.input); got != tc.expected { + t.Errorf("NormalizeHost(%q) = %q, want %q", tc.input, got, tc.expected) + } + }) + } +}