From 0b42a02bb98742b34f8ca9ad0cefaebd8f91d2d2 Mon Sep 17 00:00:00 2001
From: Alejandro Pedraza <alejandro.pedraza@gmail.com>
Date: Wed, 6 Feb 2019 13:22:17 -0500
Subject: [PATCH] Build request urls in the most generic possible way (#2206)

Build request urls in the most generic possible way

Fixes #2132

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>
---
 pkg/k8s/api.go               |  2 +-
 pkg/k8s/api_test.go          | 30 ++++++++++++++++++++----------
 pkg/k8s/k8s.go               | 23 +++++++++++++++++------
 pkg/k8s/testdata/config.test | 19 +++++++++++++++++++
 4 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/pkg/k8s/api.go b/pkg/k8s/api.go
index 35855a8f7..625231162 100644
--- a/pkg/k8s/api.go
+++ b/pkg/k8s/api.go
@@ -126,7 +126,7 @@ func (kubeAPI *KubernetesAPI) URLFor(namespace string, extraPathStartingWithSlas
 }
 
 func (kubeAPI *KubernetesAPI) getRequest(ctx context.Context, client *http.Client, path string) (*http.Response, error) {
-	endpoint, err := url.Parse(kubeAPI.Host + path)
+	endpoint, err := BuildURL(kubeAPI.Host, path)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/k8s/api_test.go b/pkg/k8s/api_test.go
index 9945bcfef..d48ca024e 100644
--- a/pkg/k8s/api_test.go
+++ b/pkg/k8s/api_test.go
@@ -10,17 +10,27 @@ func TestKubernetesApiUrlFor(t *testing.T) {
 	const extraPath = "/some/extra/path"
 
 	t.Run("Returns base config containing k8s endpoint listed in config.test", func(t *testing.T) {
-		expected := fmt.Sprintf("https://55.197.171.239/api/v1/namespaces/%s%s", namespace, extraPath)
-		api, err := NewAPI("testdata/config.test", "")
-		if err != nil {
-			t.Fatalf("Unexpected error creating Kubernetes API: %+v", err)
+		tests := []struct {
+			server      string
+			kubeContext string
+		}{
+			{"https://55.197.171.239", ""},
+			{"https://162.128.50.11", "clusterTrailingSlash"},
 		}
-		actualURL, err := api.URLFor(namespace, extraPath)
-		if err != nil {
-			t.Fatalf("Unexpected error generating URL: %+v", err)
-		}
-		if actualURL.String() != expected {
-			t.Fatalf("Expected generated URL to be [%s], but got [%s]", expected, actualURL.String())
+
+		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)
+			}
+			actualURL, err := api.URLFor(namespace, extraPath)
+			if err != nil {
+				t.Fatalf("Unexpected error generating URL: %+v", err)
+			}
+			if actualURL.String() != expected {
+				t.Fatalf("Expected generated URL to be [%s], but got [%s]", expected, actualURL.String())
+			}
 		}
 	})
 }
diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go
index 541bfc489..8b919257d 100644
--- a/pkg/k8s/k8s.go
+++ b/pkg/k8s/k8s.go
@@ -73,12 +73,7 @@ func generateKubernetesAPIBaseURLFor(schemeHostAndPort string, namespace string,
 }
 
 func generateBaseKubernetesAPIURL(schemeHostAndPort string) (*url.URL, error) {
-	urlString := fmt.Sprintf("%s/api/v1/", schemeHostAndPort)
-	url, err := url.Parse(urlString)
-	if err != nil {
-		return nil, fmt.Errorf("error generating base URL for Kubernetes API from [%s]", urlString)
-	}
-	return url, nil
+	return BuildURL(schemeHostAndPort, "/api/v1/")
 }
 
 // GetConfig returns kubernetes config based on the current environment.
@@ -171,3 +166,19 @@ 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
+}
diff --git a/pkg/k8s/testdata/config.test b/pkg/k8s/testdata/config.test
index e914ecc6a..488117658 100644
--- a/pkg/k8s/testdata/config.test
+++ b/pkg/k8s/testdata/config.test
@@ -16,6 +16,10 @@ clusters:
     certificate-authority-data: cXVlIHBhcmFkYSBhdHJhc2FkYQ==
     server: https://162.128.50.10
   name: cluster4
+- cluster:
+    certificate-authority-data: cXVlIHBhcmFkYSBhdHJhc2FkYQ==
+    server: https://162.128.50.11/
+    name: clusterTrailingSlash
 contexts:
 - context:
     cluster: cluster3
@@ -38,6 +42,10 @@ contexts:
     cluster: cluster4
     user: cluster4
   name: cluster4
+- context:
+  cluster: clusterTrailingSlash
+  user: clusterTrailingSlash
+  name: clusterTrailingSlash
 current-context: cluster1
 kind: Config
 preferences: {}
@@ -86,3 +94,14 @@ users:
         expiry-key: '{.credential.token_expiry}'
         token-key: '{.credential.access_token}'
       name: gcp
+- name: clusterTrailingSlash
+  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