Compare commits

...

1 Commits

Author SHA1 Message Date
Ahmet Alp Balkan
a444364ed3 fix: improve error handling and resource management in kubeconfig
This change addresses eight key improvements to the kubectx/kubens codebase:

Resource Management Fixes:
- Fix use-after-close bugs where Kubeconfig was accessed after Close()
- Fix resource leaks on error paths by ensuring defer kc.Close() is called
- Fix YAML encoder not being closed after Encode(), causing buffered data loss

API Design Improvements:
- Change ContextNames() to return ([]string, error) instead of silently returning
  nil on error, making parse failures distinguishable from empty results
- Change GetCurrentContext() to return (string, error) instead of returning ""
  for both "not set" and parse error cases
- Update all 16 call sites across cmd/kubectx and cmd/kubens packages to handle
  the new error returns while preserving backward-compatible behavior

Error Handling:
- Add explicit error handling for printer.Success() calls in 5+ locations
  by prefixing unchecked calls with _ =

Performance:
- Add slice pre-allocation in namespace list pagination using slices.Grow()
  before append loops, reducing allocations when fetching 500+ item batches

All changes maintain backward compatibility for missing kubeconfig keys while
improving error transparency and resource safety.
2026-03-08 17:42:34 -07:00
18 changed files with 133 additions and 50 deletions

View File

@@ -35,12 +35,14 @@ func (_op CurrentOp) Run(stdout, _ io.Writer) error {
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
v := kc.GetCurrentContext() v, err := kc.GetCurrentContext()
if err != nil {
return fmt.Errorf("failed to get current context: %w", err)
}
if v == "" { if v == "" {
return errors.New("current-context is not set") return errors.New("current-context is not set")
} }
_, err := fmt.Fprintln(stdout, v) if _, err := fmt.Fprintln(stdout, v); err != nil {
if err != nil {
return fmt.Errorf("write error: %w", err) return fmt.Errorf("write error: %w", err)
} }
return nil return nil

View File

@@ -44,7 +44,7 @@ func (op DeleteOp) Run(_, stderr io.Writer) error {
selfName()) selfName())
} }
printer.Success(stderr, `Deleted context %s.`, printer.SuccessColor.Sprint(deletedName)) _ = printer.Success(stderr, `Deleted context %s.`, printer.SuccessColor.Sprint(deletedName))
} }
return nil return nil
} }
@@ -58,7 +58,10 @@ func deleteContext(name string) (deleteName string, wasActiveContext bool, err e
return deleteName, false, fmt.Errorf("kubeconfig error: %w", err) return deleteName, false, fmt.Errorf("kubeconfig error: %w", err)
} }
cur := kc.GetCurrentContext() cur, err := kc.GetCurrentContext()
if err != nil {
return deleteName, false, fmt.Errorf("failed to get current context: %w", err)
}
// resolve "." to a real name // resolve "." to a real name
if name == "." { if name == "." {
if cur == "" { if cur == "" {
@@ -68,7 +71,11 @@ func deleteContext(name string) (deleteName string, wasActiveContext bool, err e
name = cur name = cur
} }
if !kc.ContextExists(name) { exists, err := kc.ContextExists(name)
if err != nil {
return name, false, fmt.Errorf("failed to check context: %w", err)
}
if !exists {
return name, false, errors.New("context does not exist") return name, false, errors.New("context does not exist")
} }

View File

@@ -43,6 +43,7 @@ func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error {
} }
// parse kubeconfig just to see if it can be loaded // parse kubeconfig just to see if it can be loaded
kc := new(kubeconfig.Kubeconfig).WithLoader(kubeconfig.DefaultLoader) kc := new(kubeconfig.Kubeconfig).WithLoader(kubeconfig.DefaultLoader)
defer kc.Close()
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
if cmdutil.IsNotFoundErr(err) { if cmdutil.IsNotFoundErr(err) {
printer.Warning(stderr, "kubeconfig file not found") printer.Warning(stderr, "kubeconfig file not found")
@@ -50,7 +51,6 @@ func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error {
} }
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
kc.Close()
cmd := exec.Command("fzf", "--ansi", "--no-preview") cmd := exec.Command("fzf", "--ansi", "--no-preview")
var out bytes.Buffer var out bytes.Buffer
@@ -75,7 +75,7 @@ func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error {
if err != nil { if err != nil {
return fmt.Errorf("failed to switch context: %w", err) return fmt.Errorf("failed to switch context: %w", err)
} }
printer.Success(stderr, "Switched to context \"%s\".", printer.SuccessColor.Sprint(name)) _ = printer.Success(stderr, "Switched to context \"%s\".", printer.SuccessColor.Sprint(name))
return nil return nil
} }
@@ -85,6 +85,7 @@ func (op InteractiveDeleteOp) Run(_, stderr io.Writer) error {
} }
// parse kubeconfig just to see if it can be loaded // parse kubeconfig just to see if it can be loaded
kc := new(kubeconfig.Kubeconfig).WithLoader(kubeconfig.DefaultLoader) kc := new(kubeconfig.Kubeconfig).WithLoader(kubeconfig.DefaultLoader)
defer kc.Close()
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
if cmdutil.IsNotFoundErr(err) { if cmdutil.IsNotFoundErr(err) {
printer.Warning(stderr, "kubeconfig file not found") printer.Warning(stderr, "kubeconfig file not found")
@@ -92,9 +93,12 @@ func (op InteractiveDeleteOp) Run(_, stderr io.Writer) error {
} }
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
kc.Close()
if len(kc.ContextNames()) == 0 { ctxNames, err := kc.ContextNames()
if err != nil {
return fmt.Errorf("failed to get context names: %w", err)
}
if len(ctxNames) == 0 {
return errors.New("no contexts found in config") return errors.New("no contexts found in config")
} }
@@ -129,7 +133,7 @@ func (op InteractiveDeleteOp) Run(_, stderr io.Writer) error {
selfName()) selfName())
} }
printer.Success(stderr, `Deleted context %s.`, printer.SuccessColor.Sprint(name)) _ = printer.Success(stderr, `Deleted context %s.`, printer.SuccessColor.Sprint(name))
return nil return nil
} }

View File

@@ -19,6 +19,6 @@ func checkIsolatedMode() error {
return fmt.Errorf("you are in a locked single-context shell, use 'exit' to leave") return fmt.Errorf("you are in a locked single-context shell, use 'exit' to leave")
} }
cur := kc.GetCurrentContext() cur, _ := kc.GetCurrentContext()
return fmt.Errorf("you are in a locked single-context shell (\"%s\"), use 'exit' to leave", cur) return fmt.Errorf("you are in a locked single-context shell (\"%s\"), use 'exit' to leave", cur)
} }

View File

@@ -42,10 +42,16 @@ func (_ ListOp) Run(stdout, stderr io.Writer) error {
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
ctxs := kc.ContextNames() ctxs, err := kc.ContextNames()
if err != nil {
return fmt.Errorf("failed to get context names: %w", err)
}
natsort.Sort(ctxs) natsort.Sort(ctxs)
cur := kc.GetCurrentContext() cur, err := kc.GetCurrentContext()
if err != nil {
return fmt.Errorf("failed to get current context: %w", err)
}
for _, c := range ctxs { for _, c := range ctxs {
s := c s := c
if c == cur { if c == cur {

View File

@@ -52,16 +52,27 @@ func (op RenameOp) Run(_, stderr io.Writer) error {
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
cur := kc.GetCurrentContext() cur, err := kc.GetCurrentContext()
if err != nil {
return fmt.Errorf("failed to get current context: %w", err)
}
if op.Old == "." { if op.Old == "." {
op.Old = cur op.Old = cur
} }
if !kc.ContextExists(op.Old) { oldExists, err := kc.ContextExists(op.Old)
if err != nil {
return fmt.Errorf("failed to check context: %w", err)
}
if !oldExists {
return fmt.Errorf("context \"%s\" not found, can't rename it", op.Old) return fmt.Errorf("context \"%s\" not found, can't rename it", op.Old)
} }
if kc.ContextExists(op.New) { newExists, err := kc.ContextExists(op.New)
if err != nil {
return fmt.Errorf("failed to check context: %w", err)
}
if newExists {
printer.Warning(stderr, "context \"%s\" exists, overwriting it.", op.New) printer.Warning(stderr, "context \"%s\" exists, overwriting it.", op.New)
if err := kc.DeleteContextEntry(op.New); err != nil { if err := kc.DeleteContextEntry(op.New); err != nil {
return fmt.Errorf("failed to delete new context to overwrite it: %w", err) return fmt.Errorf("failed to delete new context to overwrite it: %w", err)
@@ -79,7 +90,7 @@ func (op RenameOp) Run(_, stderr io.Writer) error {
if err := kc.Save(); err != nil { if err := kc.Save(); err != nil {
return fmt.Errorf("failed to save modified kubeconfig: %w", err) return fmt.Errorf("failed to save modified kubeconfig: %w", err)
} }
printer.Success(stderr, "Context %s renamed to %s.", _ = printer.Success(stderr, "Context %s renamed to %s.",
printer.SuccessColor.Sprint(op.Old), printer.SuccessColor.Sprint(op.Old),
printer.SuccessColor.Sprint(op.New)) printer.SuccessColor.Sprint(op.New))
return nil return nil

View File

@@ -35,10 +35,17 @@ func (op ShellOp) Run(_, stderr io.Writer) error {
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
if !kc.ContextExists(op.Target) { exists, err := kc.ContextExists(op.Target)
if err != nil {
return fmt.Errorf("failed to check context: %w", err)
}
if !exists {
return fmt.Errorf("no context exists with the name: \"%s\"", op.Target) return fmt.Errorf("no context exists with the name: \"%s\"", op.Target)
} }
previousCtx := kc.GetCurrentContext() previousCtx, err := kc.GetCurrentContext()
if err != nil {
return fmt.Errorf("failed to get current context: %w", err)
}
// Extract minimal kubeconfig using kubectl // Extract minimal kubeconfig using kubectl
data, err := extractMinimalKubeconfig(kubectlPath, op.Target) data, err := extractMinimalKubeconfig(kubectlPath, op.Target)

View File

@@ -61,8 +61,15 @@ func switchContext(name string) (string, error) {
return "", fmt.Errorf("kubeconfig error: %w", err) return "", fmt.Errorf("kubeconfig error: %w", err)
} }
prev := kc.GetCurrentContext() prev, err := kc.GetCurrentContext()
if !kc.ContextExists(name) { if err != nil {
return "", fmt.Errorf("failed to get current context: %w", err)
}
exists, err := kc.ContextExists(name)
if err != nil {
return "", fmt.Errorf("failed to check context: %w", err)
}
if !exists {
return "", fmt.Errorf("no context exists with the name: \"%s\"", name) return "", fmt.Errorf("no context exists with the name: \"%s\"", name)
} }
if err := kc.ModifyCurrentContext(name); err != nil { if err := kc.ModifyCurrentContext(name); err != nil {

View File

@@ -31,7 +31,10 @@ func (c CurrentOp) Run(stdout, _ io.Writer) error {
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
ctx := kc.GetCurrentContext() ctx, err := kc.GetCurrentContext()
if err != nil {
return fmt.Errorf("failed to get current context: %w", err)
}
if ctx == "" { if ctx == "" {
return errors.New("current-context is not set") return errors.New("current-context is not set")
} }

View File

@@ -37,6 +37,7 @@ type InteractiveSwitchOp struct {
func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error { func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error {
// parse kubeconfig just to see if it can be loaded // parse kubeconfig just to see if it can be loaded
kc := new(kubeconfig.Kubeconfig).WithLoader(kubeconfig.DefaultLoader) kc := new(kubeconfig.Kubeconfig).WithLoader(kubeconfig.DefaultLoader)
defer kc.Close()
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
if cmdutil.IsNotFoundErr(err) { if cmdutil.IsNotFoundErr(err) {
printer.Warning(stderr, "kubeconfig file not found") printer.Warning(stderr, "kubeconfig file not found")
@@ -44,7 +45,6 @@ func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error {
} }
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
defer kc.Close()
cmd := exec.Command("fzf", "--ansi", "--no-preview") cmd := exec.Command("fzf", "--ansi", "--no-preview")
var out bytes.Buffer var out bytes.Buffer
@@ -69,6 +69,6 @@ func (op InteractiveSwitchOp) Run(_, stderr io.Writer) error {
if err != nil { if err != nil {
return fmt.Errorf("failed to switch namespace: %w", err) return fmt.Errorf("failed to switch namespace: %w", err)
} }
printer.Success(stderr, "Active namespace is \"%s\".", printer.SuccessColor.Sprint(name)) _ = printer.Success(stderr, "Active namespace is \"%s\".", printer.SuccessColor.Sprint(name))
return nil return nil
} }

View File

@@ -20,6 +20,7 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"slices"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes"
@@ -39,7 +40,10 @@ func (op ListOp) Run(stdout, stderr io.Writer) error {
return fmt.Errorf("kubeconfig error: %w", err) return fmt.Errorf("kubeconfig error: %w", err)
} }
ctx := kc.GetCurrentContext() ctx, err := kc.GetCurrentContext()
if err != nil {
return fmt.Errorf("failed to get current context: %w", err)
}
if ctx == "" { if ctx == "" {
return errors.New("current-context is not set") return errors.New("current-context is not set")
} }
@@ -86,6 +90,7 @@ func queryNamespaces(kc *kubeconfig.Kubeconfig) ([]string, error) {
return nil, fmt.Errorf("failed to list namespaces from k8s API: %w", err) return nil, fmt.Errorf("failed to list namespaces from k8s API: %w", err)
} }
next = list.Continue next = list.Continue
out = slices.Grow(out, len(list.Items))
for _, it := range list.Items { for _, it := range list.Items {
out = append(out, it.Name) out = append(out, it.Name)
} }

View File

@@ -49,7 +49,10 @@ func (s SwitchOp) Run(_, stderr io.Writer) error {
} }
func switchNamespace(kc *kubeconfig.Kubeconfig, ns string, force bool) (string, error) { func switchNamespace(kc *kubeconfig.Kubeconfig, ns string, force bool) (string, error) {
ctx := kc.GetCurrentContext() ctx, err := kc.GetCurrentContext()
if err != nil {
return "", fmt.Errorf("failed to get current context: %w", err)
}
if ctx == "" { if ctx == "" {
return "", errors.New("current-context is not set") return "", errors.New("current-context is not set")
} }

View File

@@ -42,7 +42,10 @@ func (_ UnsetOp) Run(_, stderr io.Writer) error {
} }
func clearNamespace(kc *kubeconfig.Kubeconfig) (string, error) { func clearNamespace(kc *kubeconfig.Kubeconfig) (string, error) {
ctx := kc.GetCurrentContext() ctx, err := kc.GetCurrentContext()
if err != nil {
return "", fmt.Errorf("failed to get current context: %w", err)
}
ns := "default" ns := "default"
if ctx == "" { if ctx == "" {
return "", errors.New("current-context is not set") return "", errors.New("current-context is not set")

View File

@@ -50,18 +50,25 @@ func (k *Kubeconfig) contextNode(name string) (*yaml.RNode, error) {
return context, nil return context, nil
} }
func (k *Kubeconfig) ContextNames() []string { func (k *Kubeconfig) ContextNames() ([]string, error) {
contexts, err := k.config.Pipe(yaml.Get("contexts")) contexts, err := k.config.Pipe(yaml.Get("contexts"))
if err != nil { if err != nil {
return nil return nil, fmt.Errorf("failed to get contexts: %w", err)
}
if contexts == nil {
return nil, nil
} }
names, err := contexts.ElementValues("name") names, err := contexts.ElementValues("name")
if err != nil { if err != nil {
return nil return nil, fmt.Errorf("failed to get context names: %w", err)
} }
return names return names, nil
} }
func (k *Kubeconfig) ContextExists(name string) bool { func (k *Kubeconfig) ContextExists(name string) (bool, error) {
return slices.Contains(k.ContextNames(), name) names, err := k.ContextNames()
if err != nil {
return false, err
}
return slices.Contains(names, name), nil
} }

View File

@@ -33,7 +33,10 @@ func TestKubeconfig_ContextNames(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
ctx := kc.ContextNames() ctx, err := kc.ContextNames()
if err != nil {
t.Fatal(err)
}
expected := []string{"abc", "def", "ghi"} expected := []string{"abc", "def", "ghi"}
if diff := cmp.Diff(expected, ctx); diff != "" { if diff := cmp.Diff(expected, ctx); diff != "" {
t.Fatalf("%s", diff) t.Fatalf("%s", diff)
@@ -46,7 +49,10 @@ func TestKubeconfig_ContextNames_noContextsEntry(t *testing.T) {
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
t.Fatal(err) t.Fatal(err)
} }
ctx := kc.ContextNames() ctx, err := kc.ContextNames()
if err != nil {
t.Fatal(err)
}
var expected []string = nil var expected []string = nil
if diff := cmp.Diff(expected, ctx); diff != "" { if diff := cmp.Diff(expected, ctx); diff != "" {
t.Fatalf("%s", diff) t.Fatalf("%s", diff)
@@ -59,10 +65,9 @@ func TestKubeconfig_ContextNames_nonArrayContextsEntry(t *testing.T) {
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
t.Fatal(err) t.Fatal(err)
} }
ctx := kc.ContextNames() _, err := kc.ContextNames()
var expected []string = nil if err == nil {
if diff := cmp.Diff(expected, ctx); diff != "" { t.Fatal("expected error for non-array contexts entry")
t.Fatalf("%s", diff)
} }
} }
@@ -77,13 +82,15 @@ func TestKubeconfig_CheckContextExists(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
if !kc.ContextExists("c1") { if exists, err := kc.ContextExists("c1"); err != nil || !exists {
t.Fatal("c1 actually exists; reported false") t.Fatal("c1 actually exists; reported false")
} }
if !kc.ContextExists("c2") { if exists, err := kc.ContextExists("c2"); err != nil || !exists {
t.Fatal("c2 actually exists; reported false") t.Fatal("c2 actually exists; reported false")
} }
if kc.ContextExists("c3") { if exists, err := kc.ContextExists("c3"); err != nil {
t.Fatal(err)
} else if exists {
t.Fatal("c3 does not exist; but reported true") t.Fatal("c3 does not exist; but reported true")
} }
} }

View File

@@ -15,17 +15,19 @@
package kubeconfig package kubeconfig
import ( import (
"fmt"
"sigs.k8s.io/kustomize/kyaml/yaml" "sigs.k8s.io/kustomize/kyaml/yaml"
) )
// GetCurrentContext returns "current-context" value in given // GetCurrentContext returns "current-context" value in given
// kubeconfig object Node, or returns "" if not found. // kubeconfig object Node, or returns ("", nil) if not found.
func (k *Kubeconfig) GetCurrentContext() string { func (k *Kubeconfig) GetCurrentContext() (string, error) {
v, err := k.config.Pipe(yaml.Get("current-context")) v, err := k.config.Pipe(yaml.Get("current-context"))
if err != nil { if err != nil {
return "" return "", fmt.Errorf("failed to read current-context: %w", err)
} }
return yaml.GetValue(v) return yaml.GetValue(v), nil
} }
func (k *Kubeconfig) UnsetCurrentContext() error { func (k *Kubeconfig) UnsetCurrentContext() error {

View File

@@ -26,7 +26,10 @@ func TestKubeconfig_GetCurrentContext(t *testing.T) {
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
t.Fatal(err) t.Fatal(err)
} }
v := kc.GetCurrentContext() v, err := kc.GetCurrentContext()
if err != nil {
t.Fatal(err)
}
expected := "foo" expected := "foo"
if v != expected { if v != expected {
@@ -40,7 +43,10 @@ func TestKubeconfig_GetCurrentContext_missingField(t *testing.T) {
if err := kc.Parse(); err != nil { if err := kc.Parse(); err != nil {
t.Fatal(err) t.Fatal(err)
} }
v := kc.GetCurrentContext() v, err := kc.GetCurrentContext()
if err != nil {
t.Fatal(err)
}
expected := "" expected := ""
if v != expected { if v != expected {

View File

@@ -87,5 +87,8 @@ func (k *Kubeconfig) Save() error {
} }
enc := yaml.NewEncoder(k.f) enc := yaml.NewEncoder(k.f)
enc.SetIndent(0) enc.SetIndent(0)
return enc.Encode(k.config.YNode()) if err := enc.Encode(k.config.YNode()); err != nil {
return err
}
return enc.Close()
} }