fix(k8s-client): fix error classification for failed kubeconfig ingestion (NVIDIA#1743)

Signed-off-by: Tjark Gunnar Rasche <trasche@nvidia.com>

Author: tjrasche

pkg/cli/validate.go

@@ -158,7 +158,7 @@ func deployAgentForValidation(ctx context.Context, cfg *validateAgentConfig) (*s
 	// Ensure namespace exists before deploying the agent Job.
 	clientset, _, err := k8sclient.GetKubeClient()
 	if err != nil {
-		return nil, "", errors.Wrap(errors.ErrCodeInternal, "failed to create kubernetes client", err)
+		return nil, "", errors.PropagateOrWrap(err, errors.ErrCodeInternal, "failed to create kubernetes client")
 	}
 	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: cfg.namespace}}
 	if _, nsErr := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}); nsErr != nil {

pkg/k8s/client/client.go

@@ -151,7 +151,11 @@ func GetKubeClient() (Interface, *rest.Config, error) {
 // Returns:
 //   - *kubernetes.Clientset: The Kubernetes client
 //   - *rest.Config: The rest configuration used to create the client
-//   - error: Any error encountered during client creation
+//   - error: ErrCodeInvalidRequest when a file-derived kubeconfig (the explicit
+//     path, KUBECONFIG, or auto-discovered ~/.kube/config) cannot be loaded or
+//     used to construct a client. These caller-input failures are deterministic
+//     and non-retryable. ErrCodeInternal is returned when in-cluster config
+//     discovery or in-cluster client construction fails.
 //
 // Example with custom kubeconfig:
 //
@@ -175,14 +179,19 @@ func BuildKubeClient(kubeconfig string) (*kubernetes.Clientset, *rest.Config, er
 	} else {
 		config, err = clientcmd.BuildConfigFromFlags("", kubeconfig)
 		if err != nil {
-			return nil, nil, errors.WrapWithContext(errors.ErrCodeInternal, "failed to build kube config", err, map[string]interface{}{
+			return nil, nil, errors.WrapWithContext(errors.ErrCodeInvalidRequest, "failed to build kube config", err, map[string]interface{}{
 				"kubeconfig": kubeconfig,
 			})
 		}
 	}
 
 	client, err := kubernetes.NewForConfig(config)
 	if err != nil {
+		if kubeconfig != "" {
+			return nil, nil, errors.WrapWithContext(errors.ErrCodeInvalidRequest,
+				"failed to create kubernetes client from kubeconfig", err,
+				map[string]interface{}{"kubeconfig": kubeconfig})
+		}
 		return nil, nil, errors.Wrap(errors.ErrCodeInternal, "failed to create kubernetes client", err)
 	}
 

pkg/k8s/client/client_test.go

@@ -15,13 +15,32 @@
 package client
 
 import (
+	stderrors "errors"
 	"os"
 	"path/filepath"
 	"strings"
 	"sync"
 	"testing"
+
+	"github.com/NVIDIA/aicr/pkg/errors"
 )
 
+func assertKubeconfigErrorContext(t *testing.T, err error, wantKubeconfig string) {
+	t.Helper()
+
+	var structuredErr *errors.StructuredError
+	if !stderrors.As(err, &structuredErr) {
+		t.Fatalf("error = %v, want *errors.StructuredError", err)
+	}
+	gotKubeconfig, ok := structuredErr.Context["kubeconfig"].(string)
+	if !ok {
+		t.Fatalf("error context kubeconfig = %v, want string", structuredErr.Context["kubeconfig"])
+	}
+	if gotKubeconfig != wantKubeconfig {
+		t.Errorf("error context kubeconfig = %q, want %q", gotKubeconfig, wantKubeconfig)
+	}
+}
+
 // TestBuildKubeClient_PathResolution tests the kubeconfig path resolution logic
 // without attempting to connect to a cluster.
 func TestBuildKubeClient_PathResolution(t *testing.T) {
@@ -30,24 +49,30 @@ func TestBuildKubeClient_PathResolution(t *testing.T) {
 	t.Setenv("KUBECONFIG", os.Getenv("KUBECONFIG"))
 
 	tests := []struct {
-		name          string
-		kubeconfigArg string
-		kubeconfigEnv string
-		wantErr       bool
-		errorContains string
+		name           string
+		kubeconfigArg  string
+		kubeconfigEnv  string
+		wantErr        bool
+		errorContains  string
+		wantCode       errors.ErrorCode
+		wantKubeconfig string
 	}{
 		{
-			name:          "explicit invalid path",
-			kubeconfigArg: "/nonexistent/path/to/kubeconfig",
-			wantErr:       true,
-			errorContains: "failed to build kube config",
+			name:           "explicit invalid path",
+			kubeconfigArg:  "  /nonexistent/path/to/kubeconfig  ",
+			wantErr:        true,
+			errorContains:  "failed to build kube config",
+			wantCode:       errors.ErrCodeInvalidRequest,
+			wantKubeconfig: "/nonexistent/path/to/kubeconfig",
 		},
 		{
-			name:          "env var with invalid path",
-			kubeconfigArg: "",
-			kubeconfigEnv: "/nonexistent/env/kubeconfig",
-			wantErr:       true,
-			errorContains: "failed to build kube config",
+			name:           "env var with invalid path",
+			kubeconfigArg:  "",
+			kubeconfigEnv:  "/nonexistent/env/kubeconfig",
+			wantErr:        true,
+			errorContains:  "failed to build kube config",
+			wantCode:       errors.ErrCodeInvalidRequest,
+			wantKubeconfig: "/nonexistent/env/kubeconfig",
 		},
 	}
 
@@ -72,6 +97,12 @@ func TestBuildKubeClient_PathResolution(t *testing.T) {
 					t.Errorf("BuildKubeClient() error = %v, want error containing %q", err, tt.errorContains)
 				}
 			}
+			if err != nil && tt.wantCode != "" && !stderrors.Is(err, errors.New(tt.wantCode, "")) {
+				t.Errorf("BuildKubeClient() error = %v, want code %s", err, tt.wantCode)
+			}
+			if err != nil && tt.wantKubeconfig != "" {
+				assertKubeconfigErrorContext(t, err, tt.wantKubeconfig)
+			}
 		})
 	}
 }
@@ -117,6 +148,50 @@ func TestBuildKubeClient_ExplicitPath(t *testing.T) {
 	if !strings.Contains(err.Error(), "failed to build kube config") {
 		t.Errorf("BuildKubeClient() error = %v, want error containing 'failed to build kube config'", err)
 	}
+	if !stderrors.Is(err, errors.New(errors.ErrCodeInvalidRequest, "")) {
+		t.Errorf("BuildKubeClient() error = %v, want ErrCodeInvalidRequest", err)
+	}
+	assertKubeconfigErrorContext(t, err, invalidConfig)
+}
+
+// TestBuildKubeClient_InvalidClientConfigReturnsInvalidRequest verifies that a
+// kubeconfig which parses successfully but cannot initialize a Kubernetes
+// client is still classified as caller input rather than an internal failure.
+func TestBuildKubeClient_InvalidClientConfigReturnsInvalidRequest(t *testing.T) {
+	kubeconfig := filepath.Join(t.TempDir(), "invalid-client-config")
+	content := `apiVersion: v1
+kind: Config
+clusters:
+  - name: test
+    cluster:
+      server: https://127.0.0.1
+      certificate-authority-data: bm90IGEgcGVtIGNlcnRpZmljYXRl
+contexts:
+  - name: test
+    context:
+      cluster: test
+      user: test
+current-context: test
+users:
+  - name: test
+    user:
+      token: test
+`
+	if err := os.WriteFile(kubeconfig, []byte(content), 0o600); err != nil {
+		t.Fatalf("failed to write test kubeconfig: %v", err)
+	}
+
+	_, _, err := BuildKubeClient(kubeconfig)
+	if err == nil {
+		t.Fatal("BuildKubeClient() error = nil, want invalid request")
+	}
+	if !stderrors.Is(err, errors.New(errors.ErrCodeInvalidRequest, "")) {
+		t.Errorf("BuildKubeClient() error = %v, want ErrCodeInvalidRequest", err)
+	}
+	if !strings.Contains(err.Error(), "failed to create kubernetes client from kubeconfig") {
+		t.Errorf("BuildKubeClient() error = %v, want client construction failure", err)
+	}
+	assertKubeconfigErrorContext(t, err, kubeconfig)
 }
 
 // TestGetKubeClient_Singleton tests that GetKubeClient returns the same instance.

pkg/serializer/configmap.go

@@ -83,7 +83,7 @@ func (w *ConfigMapWriter) Serialize(ctx context.Context, snapshot any) error {
 	// as reader.go's ConfigMap read path.
 	k8sClient, config, err := client.GetKubeClientWithConfig(w.kubeconfig)
 	if err != nil {
-		return errors.Wrap(errors.ErrCodeInternal, "failed to get kubernetes client", err)
+		return errors.PropagateOrWrap(err, errors.ErrCodeInternal, "failed to get kubernetes client")
 	}
 
 	// Log authentication context for audit

pkg/serializer/configmap_test.go

@@ -15,8 +15,12 @@
 package serializer
 
 import (
+	stderrors "errors"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/NVIDIA/aicr/pkg/errors"
 	"github.com/NVIDIA/aicr/pkg/k8s/pod"
 )
 
@@ -191,3 +195,36 @@ func TestNewConfigMapWriter_PreservesFormatCoercion(t *testing.T) {
 		t.Errorf("kubeconfig = %q, want empty (default discovery)", writer.kubeconfig)
 	}
 }
+
+func TestConfigMapWriterSerializePreservesKubeconfigErrorCode(t *testing.T) {
+	kubeconfig := writeInvalidKubeconfig(t)
+	writer := NewConfigMapWriterWithKubeconfig("default", "test", kubeconfig, FormatYAML)
+
+	err := writer.Serialize(t.Context(), map[string]string{"key": "value"})
+	assertOutermostErrorCode(t, err, errors.ErrCodeInvalidRequest)
+}
+
+func writeInvalidKubeconfig(t *testing.T) string {
+	t.Helper()
+
+	kubeconfig := filepath.Join(t.TempDir(), "invalid-kubeconfig")
+	if err := os.WriteFile(kubeconfig, []byte("invalid yaml content"), 0o600); err != nil {
+		t.Fatalf("failed to write invalid kubeconfig: %v", err)
+	}
+	return kubeconfig
+}
+
+func assertOutermostErrorCode(t *testing.T, err error, want errors.ErrorCode) {
+	t.Helper()
+	if err == nil {
+		t.Fatalf("error = nil, want code %s", want)
+	}
+
+	var structuredErr *errors.StructuredError
+	if !stderrors.As(err, &structuredErr) {
+		t.Fatalf("error = %v, want *errors.StructuredError", err)
+	}
+	if structuredErr.Code != want {
+		t.Errorf("error code = %s, want %s", structuredErr.Code, want)
+	}
+}

pkg/serializer/reader.go

@@ -506,7 +506,7 @@ func fromConfigMapWithKubeconfigContext[T any](ctx context.Context, namespace, n
 		k8sClient, _, err = client.GetKubeClient()
 	}
 	if err != nil {
-		return nil, errors.Wrap(errors.ErrCodeInternal, "failed to get kubernetes client", err)
+		return nil, errors.PropagateOrWrap(err, errors.ErrCodeInternal, "failed to get kubernetes client")
 	}
 
 	readCtx, cancel := context.WithTimeout(ctx, defaults.ConfigMapWriteTimeout)

pkg/serializer/reader_test.go

@@ -185,6 +185,17 @@ func TestNewReader(t *testing.T) {
 	})
 }
 
+func TestFromConfigMapPreservesKubeconfigErrorCode(t *testing.T) {
+	kubeconfig := writeInvalidKubeconfig(t)
+
+	_, err := FromFileWithKubeconfigContext[testConfig](
+		t.Context(),
+		"cm://default/test",
+		kubeconfig,
+	)
+	assertOutermostErrorCode(t, err, errors.ErrCodeInvalidRequest)
+}
+
 func TestReader_DeserializeJSON(t *testing.T) {
 	t.Run("valid json object", func(t *testing.T) {
 		jsonData := `{"name":"test","value":123}`

pkg/snapshotter/agent.go

@@ -296,7 +296,7 @@ func getKubeClient(kubeconfig string) (k8sclient.Interface, error) {
 		clientset, _, err = k8sclient.GetKubeClient()
 	}
 	if err != nil {
-		return nil, errors.Wrap(errors.ErrCodeInternal, "failed to create Kubernetes client", err)
+		return nil, errors.PropagateOrWrap(err, errors.ErrCodeInternal, "failed to create Kubernetes client")
 	}
 	return clientset, nil
 }

pkg/snapshotter/agent_test.go

@@ -15,9 +15,12 @@
 package snapshotter
 
 import (
+	stderrors "errors"
 	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/NVIDIA/aicr/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -65,6 +68,26 @@ func TestAgentConfig_Defaults(t *testing.T) {
 	}
 }
 
+func TestGetKubeClientPreservesKubeconfigErrorCode(t *testing.T) {
+	kubeconfig := filepath.Join(t.TempDir(), "invalid-kubeconfig")
+	if err := os.WriteFile(kubeconfig, []byte("invalid yaml content"), 0o600); err != nil {
+		t.Fatalf("failed to write invalid kubeconfig: %v", err)
+	}
+
+	_, err := getKubeClient(kubeconfig)
+	if err == nil {
+		t.Fatal("getKubeClient() error = nil, want ErrCodeInvalidRequest")
+	}
+
+	var structuredErr *errors.StructuredError
+	if !stderrors.As(err, &structuredErr) {
+		t.Fatalf("getKubeClient() error = %v, want *errors.StructuredError", err)
+	}
+	if structuredErr.Code != errors.ErrCodeInvalidRequest {
+		t.Errorf("getKubeClient() error code = %s, want %s", structuredErr.Code, errors.ErrCodeInvalidRequest)
+	}
+}
+
 func TestParseNodeSelectors(t *testing.T) {
 	tests := []struct {
 		name      string