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

@@ -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
 }
 

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 {

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")
+}