Skip to content

Commit b5ed326

Browse files
committed
fix: distinguish ErrNotInCluster in probeAPIServer
probeAPIServer returned nil for any error from ctrl.GetConfig(), silently masking malformed kubeconfig files. Now it distinguishes the "not in-cluster" case from actual kubeconfig load failures, surfacing the latter so STATUS reports the misconfiguration. - Extract kubeconfig loading into getKubeconfig variable for testability - Check errors.Is against rest.ErrNotInCluster before skipping API check - Surface non-cluster kubeconfig errors wrapped with "load kubeconfig" fixes #169
1 parent 46bb2cf commit b5ed326

2 files changed

Lines changed: 51 additions & 3 deletions

File tree

internal/cni/cni_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
apierrors "k8s.io/apimachinery/pkg/api/errors"
2020
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2121
"k8s.io/apimachinery/pkg/runtime/schema"
22+
"k8s.io/client-go/rest"
2223
"sigs.k8s.io/controller-runtime/pkg/client"
2324
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2425
)
@@ -1175,3 +1176,43 @@ func TestRetryK8sOpsExhaustsDeadline(t *testing.T) {
11751176
t.Errorf("expected 'unavailable' in error, got %v", err)
11761177
}
11771178
}
1179+
1180+
// ---- probeAPIServer ------------------------------------------------------
1181+
1182+
func TestProbeAPIServerErrNotInCluster(t *testing.T) {
1183+
// When getKubeconfig returns ErrNotInCluster, probeAPIServer should
1184+
// return nil (not running in-cluster; skip API check).
1185+
original := getKubeconfig
1186+
defer func() { getKubeconfig = original }()
1187+
1188+
getKubeconfig = func() (*rest.Config, error) {
1189+
return nil, rest.ErrNotInCluster
1190+
}
1191+
1192+
if err := probeAPIServer(); err != nil {
1193+
t.Fatalf("expected nil for ErrNotInCluster, got %v", err)
1194+
}
1195+
}
1196+
1197+
func TestProbeAPIServerMalformedKubeconfig(t *testing.T) {
1198+
// When getKubeconfig returns a non-ErrNotInCluster error (e.g. a
1199+
// malformed kubeconfig file), probeAPIServer should surface it wrapped.
1200+
original := getKubeconfig
1201+
defer func() { getKubeconfig = original }()
1202+
1203+
malformedErr := errors.New("invalid kubeconfig: permission denied")
1204+
getKubeconfig = func() (*rest.Config, error) {
1205+
return nil, malformedErr
1206+
}
1207+
1208+
err := probeAPIServer()
1209+
if err == nil {
1210+
t.Fatal("expected error for malformed kubeconfig, got nil")
1211+
}
1212+
if !strings.Contains(err.Error(), "load kubeconfig") {
1213+
t.Fatalf("error %q does not contain 'load kubeconfig'", err.Error())
1214+
}
1215+
if !strings.Contains(err.Error(), "permission denied") {
1216+
t.Fatalf("error %q does not contain original error", err.Error())
1217+
}
1218+
}

internal/cni/ops_check.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,18 @@ func cmdStatus(args *skel.CmdArgs) error {
102102
// probeAPIServer performs a lightweight GET against the in-cluster API server
103103
// to verify reachability. Returns nil when the server responds (any HTTP
104104
// status code) or when running outside a cluster with no kubeconfig.
105+
//
106+
// getKubeconfig is a package-level variable so tests can inject error paths.
107+
var getKubeconfig = func() (*rest.Config, error) { return ctrl.GetConfig() }
108+
105109
func probeAPIServer() error {
106-
kubeconfig, err := ctrl.GetConfig()
110+
kubeconfig, err := getKubeconfig()
107111
if err != nil {
108-
// No kubeconfig (running outside a cluster); skip API check.
109-
return nil
112+
if errors.Is(err, rest.ErrNotInCluster) {
113+
// Not running in-cluster; skip API check.
114+
return nil
115+
}
116+
return fmt.Errorf("load kubeconfig: %w", err)
110117
}
111118
kubeconfig.Timeout = 2 * time.Second
112119
httpClient, err := rest.HTTPClientFor(kubeconfig)

0 commit comments

Comments
 (0)