From 08bfd3a2afa867d6ccc310a3dc373d23e582210b Mon Sep 17 00:00:00 2001
From: Sam Dowell <sdowell@google.com>
Date: Mon, 17 Jun 2024 14:44:58 -0700
Subject: [PATCH] fix: only use local config if file exists (#1275)

The rest config precedence was changed to prioritize the kubectl config
over the in cluster config. This was only checking if KubeConfigPath
returns a path rather than whether a file exists at that path.

It has been reported that certain k8s distributions set the user config
for the container, in which case KubeConfigPath will return a home
directory path for that user. This causes the process to attempt to use
a local file config instead of the in cluster config, which fails.

This change adds an additional check that the kube config file exists at
the specified path, which is intended to fix the above scenario.
---
 pkg/client/restconfig/config.go          |  4 +-
 pkg/client/restconfig/restconfig.go      | 33 ++++++++++++---
 pkg/client/restconfig/restconfig_test.go | 54 ++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 9 deletions(-)
 create mode 100644 pkg/client/restconfig/restconfig_test.go

diff --git a/pkg/client/restconfig/config.go b/pkg/client/restconfig/config.go
index 7cb46fbf10..f3f25a530b 100644
--- a/pkg/client/restconfig/config.go
+++ b/pkg/client/restconfig/config.go
@@ -48,11 +48,11 @@ func KubeConfigPath() (string, error) {
 	if envPath != "" {
 		return envPath, nil
 	}
-	curentUser, err := userCurrentTestHook()
+	currentUser, err := userCurrentTestHook()
 	if err != nil {
 		return "", fmt.Errorf("failed to get current user: %w", err)
 	}
-	path := filepath.Join(curentUser.HomeDir, kubectlConfigPath)
+	path := filepath.Join(currentUser.HomeDir, kubectlConfigPath)
 	return path, nil
 }
 
diff --git a/pkg/client/restconfig/restconfig.go b/pkg/client/restconfig/restconfig.go
index 45fba1ade0..ef83652f66 100644
--- a/pkg/client/restconfig/restconfig.go
+++ b/pkg/client/restconfig/restconfig.go
@@ -36,21 +36,40 @@ const DefaultTimeout = 15 * time.Second
 // is enabled or not. If server-side flow control is enabled, then client-side
 // throttling is disabled, vice versa.
 func NewRestConfig(timeout time.Duration) (*rest.Config, error) {
+	builder := restConfigBuilder{
+		newFromConfigFileFn:      NewFromConfigFile,
+		newFromInClusterConfigFn: NewFromInClusterConfig,
+	}
+	return builder.newRestConfig(timeout)
+}
+
+type restConfigBuilder struct {
+	newFromConfigFileFn      func(string) (*rest.Config, error)
+	newFromInClusterConfigFn func() (*rest.Config, error)
+}
+
+func (b *restConfigBuilder) newRestConfig(timeout time.Duration) (*rest.Config, error) {
 	var cfg *rest.Config
 	// Detect kubectl config file
 	path, err := KubeConfigPath()
-	if err != nil {
-		// Build from k8s downward API
-		cfg, err = NewFromInClusterConfig()
-		if err != nil {
-			return nil, fmt.Errorf("failed to build rest config: kubeconfig not found: reading in-cluster config: %w", err)
+	kubeConfigExists := false
+	if err == nil {
+		if _, err := os.Stat(path); err == nil {
+			kubeConfigExists = true
 		}
-	} else {
+	}
+	if kubeConfigExists {
 		// Build from local config file
-		cfg, err = NewFromConfigFile(path)
+		cfg, err = b.newFromConfigFileFn(path)
 		if err != nil {
 			return nil, fmt.Errorf("failed to build rest config: reading local kubeconfig: %w", err)
 		}
+	} else {
+		// Build from k8s downward API
+		cfg, err = b.newFromInClusterConfigFn()
+		if err != nil {
+			return nil, fmt.Errorf("failed to build rest config: kubeconfig not found: reading in-cluster config: %w", err)
+		}
 	}
 	// Set timeout, if specified.
 	if timeout != 0 {
diff --git a/pkg/client/restconfig/restconfig_test.go b/pkg/client/restconfig/restconfig_test.go
new file mode 100644
index 0000000000..0e76c786bb
--- /dev/null
+++ b/pkg/client/restconfig/restconfig_test.go
@@ -0,0 +1,54 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package restconfig
+
+import (
+	"fmt"
+	"os/user"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/client-go/rest"
+)
+
+func Test_NewRestConfig_HomeDirWithoutKubeConfig(t *testing.T) {
+	t.Cleanup(func() {
+		userCurrentTestHook = defaultGetCurrentUser
+	})
+
+	userCurrentTestHook = func() (*user.User, error) {
+		return &user.User{
+			HomeDir: "/some/path/that/hopefully/does/not/exist",
+		}, nil
+	}
+	calledNewFromConfigFile := false
+	calledNewFromInClusterConfig := false
+	fakeBuilder := restConfigBuilder{
+		newFromConfigFileFn: func(_ string) (*rest.Config, error) {
+			calledNewFromConfigFile = true
+			return nil, fmt.Errorf("unexpected call to local config")
+		},
+		newFromInClusterConfigFn: func() (*rest.Config, error) {
+			calledNewFromInClusterConfig = true
+			return &rest.Config{}, nil
+		},
+	}
+
+	_, err := fakeBuilder.newRestConfig(time.Minute)
+	assert.NoError(t, err)
+	assert.False(t, calledNewFromConfigFile, "unexpected call to NewFromConfigFile")
+	assert.True(t, calledNewFromInClusterConfig, "expected call to NewFromInClusterConfig")
+}