Fix handling of kubeconfig server urls that include paths (#2305)

Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>

pkg/k8s/api.go

@@ -121,12 +121,12 @@ func (kubeAPI *KubernetesAPI) getPods(ctx context.Context, client *http.Client,
 }
 
 // URLFor generates a URL based on the Kubernetes config.
-func (kubeAPI *KubernetesAPI) URLFor(namespace string, extraPathStartingWithSlash string) (*url.URL, error) {
+func (kubeAPI *KubernetesAPI) URLFor(namespace, path string) (*url.URL, error) {
-	return generateKubernetesAPIBaseURLFor(kubeAPI.Host, namespace, extraPathStartingWithSlash)
+	return generateKubernetesAPIURLFor(kubeAPI.Host, namespace, path)
 }
 
 func (kubeAPI *KubernetesAPI) getRequest(ctx context.Context, client *http.Client, path string) (*http.Response, error) {
-	endpoint, err := BuildURL(kubeAPI.Host, path)
+	endpoint, err := generateKubernetesURL(kubeAPI.Host, path)
 	if err != nil {
 		return nil, err
 	}

pkg/k8s/api_test.go

@@ -1,7 +1,6 @@
 package k8s
 
 import (
-	"fmt"
 	"testing"
 )
 
@@ -11,15 +10,24 @@ func TestKubernetesApiUrlFor(t *testing.T) {
 
 	t.Run("Returns base config containing k8s endpoint listed in config.test", func(t *testing.T) {
 		tests := []struct {
-			server      string
 			kubeContext string
+			expected    string
 		}{
-			{"https://55.197.171.239", ""},
+			{
-			{"https://162.128.50.11", "clusterTrailingSlash"},
+				kubeContext: "",
+				expected:    "https://55.197.171.239/api/v1/namespaces/some-namespace/some/extra/path",
+			},
+			{
+				kubeContext: "clusterTrailingSlash",
+				expected:    "https://162.128.50.11/api/v1/namespaces/some-namespace/some/extra/path",
+			},
+			{
+				kubeContext: "clusterWithPath",
+				expected:    "https://162.128.50.12/k8s/clusters/c-fhjws/api/v1/namespaces/some-namespace/some/extra/path",
+			},
 		}
 
 		for _, test := range tests {
-			expected := fmt.Sprintf("%s/api/v1/namespaces/%s%s", test.server, namespace, extraPath)
 			api, err := NewAPI("testdata/config.test", test.kubeContext)
 			if err != nil {
 				t.Fatalf("Unexpected error creating Kubernetes API: %+v", err)
@@ -28,8 +36,8 @@ func TestKubernetesApiUrlFor(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Unexpected error generating URL: %+v", err)
 			}
-			if actualURL.String() != expected {
+			if actualURL.String() != test.expected {
-				t.Fatalf("Expected generated URL to be [%s], but got [%s]", expected, actualURL.String())
+				t.Fatalf("Expected generated URL to be [%s], but got [%s]", test.expected, actualURL.String())
 			}
 		}
 	})

pkg/k8s/k8s.go

@@ -3,6 +3,7 @@ package k8s
 import (
 	"fmt"
 	"net/url"
+	"strings"
 
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
@@ -54,27 +55,21 @@ var StatAllResourceTypes = []string{
 	Authority,
 }
 
-func generateKubernetesAPIBaseURLFor(schemeHostAndPort string, namespace string, extraPathStartingWithSlash string) (*url.URL, error) {
+func generateKubernetesAPIURLFor(serverURL, namespace, path string) (*url.URL, error) {
-	if string(extraPathStartingWithSlash[0]) != "/" {
+	if !strings.HasPrefix(path, "/") {
-		return nil, fmt.Errorf("Path must start with a [/], was [%s]", extraPathStartingWithSlash)
+		return nil, fmt.Errorf("path must start with a /, got [%s]", path)
 	}
 
-	baseURL, err := generateBaseKubernetesAPIURL(schemeHostAndPort)
+	fullPath := "/api/v1/namespaces/" + namespace + path
-	if err != nil {
+	return generateKubernetesURL(serverURL, fullPath)
-		return nil, err
-	}
-
-	urlString := fmt.Sprintf("%snamespaces/%s%s", baseURL.String(), namespace, extraPathStartingWithSlash)
-	url, err := url.Parse(urlString)
-	if err != nil {
-		return nil, fmt.Errorf("error generating namespace URL for Kubernetes API from [%s]", urlString)
-	}
-
-	return url, nil
 }
 
-func generateBaseKubernetesAPIURL(schemeHostAndPort string) (*url.URL, error) {
+func generateKubernetesURL(serverURL, path string) (*url.URL, error) {
-	return BuildURL(schemeHostAndPort, "/api/v1/")
+	if !strings.HasPrefix(path, "/") {
+		return nil, fmt.Errorf("path must start with a /, got [%s]", path)
+	}
+
+	return url.Parse(strings.TrimSuffix(serverURL, "/") + path)
 }
 
 // GetConfig returns kubernetes config based on the current environment.
@@ -167,19 +162,3 @@ func KindToL5DLabel(k8sKind string) string {
 	}
 	return k8sKind
 }
-
-// BuildURL returns an abosolute URL from a reference URL. It parses a base
-// rawurl and reference rawurl. Then, it tries to resolve the reference from
-// the absolute base.
-func BuildURL(base string, ref string) (*url.URL, error) {
-	u, err := url.Parse(ref)
-	if err != nil {
-		return nil, fmt.Errorf("error generating reference URL for endpoint from [%s]", base)
-	}
-	b, err := url.Parse(base)
-	if err != nil {
-		return nil, fmt.Errorf("error generating base URL for endpoint from [%s]", base)
-	}
-	url := b.ResolveReference(u)
-	return url, nil
-}

pkg/k8s/k8s_test.go

@@ -6,10 +6,10 @@ import (
 
 func TestGenerateKubernetesApiBaseUrlFor(t *testing.T) {
 	t.Run("Generates correct URL when all elements are present", func(t *testing.T) {
-		schemeHostAndPort := "ftp://some-server.example.com:666"
+		serverURL := "ftp://some-server.example.com:666"
 		namespace := "some-namespace"
 		extraPath := "/starts/with/slash"
-		url, err := generateKubernetesAPIBaseURLFor(schemeHostAndPort, namespace, extraPath)
+		url, err := generateKubernetesAPIURLFor(serverURL, namespace, extraPath)
 
 		if err != nil {
 			t.Fatalf("Unexpected error starting proxy: %v", err)
@@ -22,10 +22,10 @@ func TestGenerateKubernetesApiBaseUrlFor(t *testing.T) {
 	})
 
 	t.Run("Return error if extra path doesn't start with slash", func(t *testing.T) {
-		schemeHostAndPort := "ftp://some-server.example.com:666"
+		serverURL := "ftp://some-server.example.com:666"
 		namespace := "some-namespace"
 		extraPath := "does-not-start/with/slash"
-		_, err := generateKubernetesAPIBaseURLFor(schemeHostAndPort, namespace, extraPath)
+		_, err := generateKubernetesAPIURLFor(serverURL, namespace, extraPath)
 
 		if err == nil {
 			t.Fatalf("Expected error when tryiong to generate URL with extra path without leading slash, got nothing")
@@ -33,11 +33,11 @@ func TestGenerateKubernetesApiBaseUrlFor(t *testing.T) {
 	})
 }
 
-func TestGenerateBaseKubernetesApiUrl(t *testing.T) {
+func TestGenerateBaseKubernetesUrl(t *testing.T) {
 	t.Run("Generates correct URL when all elements are present", func(t *testing.T) {
-		schemeHostAndPort := "gopher://some-server.example.com:661"
+		serverURL := "gopher://some-server.example.com:661"
 
-		url, err := generateBaseKubernetesAPIURL(schemeHostAndPort)
+		url, err := generateKubernetesURL(serverURL, "/api/v1/")
 		if err != nil {
 			t.Fatalf("Unexpected error starting proxy: %v", err)
 		}
@@ -49,11 +49,11 @@ func TestGenerateBaseKubernetesApiUrl(t *testing.T) {
 	})
 
 	t.Run("Return error if invalid host and port", func(t *testing.T) {
-		schemeHostAndPort := "ftp://some-server.exampl     e.com:666"
+		serverURL := "ftp://some-server.exampl     e.com:666"
-		_, err := generateBaseKubernetesAPIURL(schemeHostAndPort)
+		_, err := generateKubernetesURL(serverURL, "/api/v1/")
 
 		if err == nil {
-			t.Fatalf("Expected error when tryiong to generate URL with extra path without leading slash, got nothing")
+			t.Fatalf("Expected error when trying to generate URL with extra path without leading slash, got nothing")
 		}
 	})
 }

pkg/k8s/testdata/config.test

@@ -19,7 +19,11 @@ clusters:
 - cluster:
     certificate-authority-data: cXVlIHBhcmFkYSBhdHJhc2FkYQ==
     server: https://162.128.50.11/
-    name: clusterTrailingSlash
+  name: clusterTrailingSlash
+- cluster:
+    certificate-authority-data: cXVlIHBhcmFkYSBhdHJhc2FkYQ==
+    server: https://162.128.50.12/k8s/clusters/c-fhjws
+  name: clusterWithPath
 contexts:
 - context:
     cluster: cluster3
@@ -43,9 +47,13 @@ contexts:
     user: cluster4
   name: cluster4
 - context:
-  cluster: clusterTrailingSlash
+    cluster: clusterTrailingSlash
-  user: clusterTrailingSlash
+    user: clusterTrailingSlash
   name: clusterTrailingSlash
+- context:
+    cluster: clusterWithPath
+    user: clusterWithPath
+  name: clusterWithPath
 current-context: cluster1
 kind: Config
 preferences: {}
@@ -105,3 +113,14 @@ users:
         expiry-key: '{.credential.token_expiry}'
         token-key: '{.credential.access_token}'
       name: gcp
+- name: clusterWithPath
+  user:
+    auth-provider:
+      config:
+        access-token: 4cc3sspassatempoq
+        cmd-args: config config-helper --format=json
+        cmd-path: /Users/bobojones/bin/google-cloud-sdk/bin/gcloud
+        expiry: 2017-11-22 22:13:05
+        expiry-key: '{.credential.token_expiry}'
+        token-key: '{.credential.access_token}'
+      name: gcp