Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cli/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment thread
tjrasche marked this conversation as resolved.
}
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: cfg.namespace}}
if _, nsErr := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}); nsErr != nil {
Expand Down
13 changes: 11 additions & 2 deletions pkg/k8s/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
//
Expand All @@ -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{}{
Comment thread
tjrasche marked this conversation as resolved.
"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})
}
Comment thread
tjrasche marked this conversation as resolved.
return nil, nil, errors.Wrap(errors.ErrCodeInternal, "failed to create kubernetes client", err)
}

Expand Down
103 changes: 89 additions & 14 deletions pkg/k8s/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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",
},
}

Expand All @@ -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)
}
})
}
}
Expand Down Expand Up @@ -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
Comment thread
tjrasche marked this conversation as resolved.
// 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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/serializer/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions pkg/serializer/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion pkg/serializer/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions pkg/serializer/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
Expand Down
2 changes: 1 addition & 1 deletion pkg/snapshotter/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions pkg/snapshotter/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
Loading