From b1a6b6ca669f10cc637b351ff478e360f97a2e3b Mon Sep 17 00:00:00 2001 From: Kevin Lingerfelt Date: Mon, 18 Feb 2019 17:57:27 -0800 Subject: [PATCH] Fix handling of kubeconfig server urls that include paths (#2305) Signed-off-by: Kevin Lingerfelt --- pkg/k8s/api.go | 6 ++--- pkg/k8s/api_test.go | 22 ++++++++++++------ pkg/k8s/k8s.go | 45 ++++++++++-------------------------- pkg/k8s/k8s_test.go | 20 ++++++++-------- pkg/k8s/testdata/config.test | 25 +++++++++++++++++--- 5 files changed, 62 insertions(+), 56 deletions(-) diff --git a/pkg/k8s/api.go b/pkg/k8s/api.go index 625231162..9f02baf3e 100644 --- a/pkg/k8s/api.go +++ b/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) { - return generateKubernetesAPIBaseURLFor(kubeAPI.Host, namespace, extraPathStartingWithSlash) +func (kubeAPI *KubernetesAPI) URLFor(namespace, path string) (*url.URL, error) { + 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 } diff --git a/pkg/k8s/api_test.go b/pkg/k8s/api_test.go index d48ca024e..16a33cfb4 100644 --- a/pkg/k8s/api_test.go +++ b/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 { - t.Fatalf("Expected generated URL to be [%s], but got [%s]", expected, actualURL.String()) + if actualURL.String() != test.expected { + t.Fatalf("Expected generated URL to be [%s], but got [%s]", test.expected, actualURL.String()) } } }) diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index 238f8e724..4de44f37a 100644 --- a/pkg/k8s/k8s.go +++ b/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) { - if string(extraPathStartingWithSlash[0]) != "/" { - return nil, fmt.Errorf("Path must start with a [/], was [%s]", extraPathStartingWithSlash) +func generateKubernetesAPIURLFor(serverURL, namespace, path string) (*url.URL, error) { + if !strings.HasPrefix(path, "/") { + return nil, fmt.Errorf("path must start with a /, got [%s]", path) } - baseURL, err := generateBaseKubernetesAPIURL(schemeHostAndPort) - if err != nil { - 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 + fullPath := "/api/v1/namespaces/" + namespace + path + return generateKubernetesURL(serverURL, fullPath) } -func generateBaseKubernetesAPIURL(schemeHostAndPort string) (*url.URL, error) { - return BuildURL(schemeHostAndPort, "/api/v1/") +func generateKubernetesURL(serverURL, path string) (*url.URL, error) { + 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 -} diff --git a/pkg/k8s/k8s_test.go b/pkg/k8s/k8s_test.go index 3ab5088a5..3a51035f4 100644 --- a/pkg/k8s/k8s_test.go +++ b/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" - _, err := generateBaseKubernetesAPIURL(schemeHostAndPort) + serverURL := "ftp://some-server.exampl e.com:666" + _, 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") } }) } diff --git a/pkg/k8s/testdata/config.test b/pkg/k8s/testdata/config.test index 488117658..fcfb86b87 100644 --- a/pkg/k8s/testdata/config.test +++ b/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 - user: clusterTrailingSlash + cluster: 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