Fix zsh shell function & cluster-name collision across different stores

cmd/kswitch/init.go

@@ -99,14 +99,18 @@ function kswitch(){
 
   local _new_kc="$KUBECONFIG_PATH"
   if [[ -n "$KUBECONFIG" ]]; then
-	local _part
-	IFS=':' read -ra _kc_parts <<< "$KUBECONFIG"
-	for _part in "${_kc_parts[@]}"; do
+	local _rest="$KUBECONFIG" _part
+	while [[ -n "$_rest" ]]; do
+	  _part="${_rest%%:*}"
+	  case "$_rest" in
+		*:*) _rest="${_rest#*:}" ;;
+		*) _rest="" ;;
+	  esac
 	  if [[ -n "$_part" && "$_part" != *"$switchTmpDirectory"* ]]; then
 		_new_kc="$_new_kc:$_part"
 	  fi
 	done
-	unset _kc_parts _part
+	unset _rest _part
   fi
   export KUBECONFIG="$_new_kc"
   unset _new_kc

cmd/kswitch/init_test.go

@@ -0,0 +1,303 @@
+// Copyright 2024 The Kswitch authors
+//
+// 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 kswitch
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+)
+
+// switchShellScriptPath is the copy of the shell function shipped for the
+// homebrew installation. It has to stay in sync with shellScriptTemplate.
+const switchShellScriptPath = "../../hack/switch/switch.sh"
+
+// shellsUnderTest are the POSIX-ish shells the wrapper function must work in.
+// zsh is the interesting one: it has no `read -a`, so bash-only array idioms
+// break there.
+var shellsUnderTest = []string{"bash", "zsh"}
+
+// writeFakeKswitch writes a stub kswitch binary that answers with the response
+// the real binary sends to the shell wrapper after a context was selected.
+func writeFakeKswitch(t *testing.T, dir, kubeconfigPath, selectedContext string) string {
+	t.Helper()
+
+	path := filepath.Join(dir, "fake-kswitch")
+	script := fmt.Sprintf("#!/bin/sh\nprintf '__ %s,%s\\n'\n", kubeconfigPath, selectedContext)
+	if err := os.WriteFile(path, []byte(script), 0o755); err != nil {
+		t.Fatalf("write fake kswitch: %v", err)
+	}
+	return path
+}
+
+// scriptsUnderTest returns the shell wrapper implementations to exercise:
+// the one generated by `kswitch init bash|zsh` and the homebrew copy.
+func scriptsUnderTest(t *testing.T, dir string) map[string]string {
+	t.Helper()
+
+	generated := filepath.Join(dir, "generated.sh")
+	if err := os.WriteFile(generated, []byte(shellScriptTemplate), 0o600); err != nil {
+		t.Fatalf("write generated script: %v", err)
+	}
+
+	homebrew, err := filepath.Abs(switchShellScriptPath)
+	if err != nil {
+		t.Fatalf("resolve %s: %v", switchShellScriptPath, err)
+	}
+
+	return map[string]string{
+		"init":      generated,
+		"switch.sh": homebrew,
+	}
+}
+
+// runKswitchFunction sources scriptPath in shell, calls the kswitch function
+// with the stub binary and returns the resulting KUBECONFIG.
+func runKswitchFunction(t *testing.T, shell, scriptPath, fakeBinary, home, kubeconfigIn string) string {
+	t.Helper()
+
+	// Print KUBECONFIG on a line of its own so the "switched to context …"
+	// output of the function does not interfere.
+	snippet := fmt.Sprintf(
+		". %q\nkswitch --executable-path %q >/dev/null\nprintf 'KUBECONFIG=%%s\\n' \"$KUBECONFIG\"\n",
+		scriptPath, fakeBinary)
+
+	cmd := exec.Command(shell, "-c", snippet)
+	cmd.Env = append(os.Environ(),
+		"HOME="+home,
+		"KUBECONFIG="+kubeconfigIn,
+	)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("%s: %v\noutput: %s", shell, err, out)
+	}
+
+	for line := range strings.SplitSeq(string(out), "\n") {
+		if after, ok := strings.CutPrefix(line, "KUBECONFIG="); ok {
+			return after
+		}
+	}
+	t.Fatalf("%s: no KUBECONFIG in output: %s", shell, out)
+	return ""
+}
+
+// TestShellWrapperMergesKubeconfig checks that the wrapper prepends the new
+// kubeconfig to KUBECONFIG while keeping the user's own entries, in order, in
+// every supported shell. zsh used to lose all pre-existing entries here because
+// the merge relied on `IFS=':' read -ra`, which zsh does not support.
+func TestShellWrapperMergesKubeconfig(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX shell wrapper is not used on windows")
+	}
+
+	tests := []struct {
+		name         string
+		kubeconfigIn string // $KUBECONFIG before switching; $TMP is the switch tmp dir
+		want         string // expected $KUBECONFIG; $NEW is the newly selected kubeconfig
+	}{
+		{
+			name:         "empty KUBECONFIG",
+			kubeconfigIn: "",
+			want:         "$NEW",
+		},
+		{
+			name:         "single entry is kept",
+			kubeconfigIn: "/a/config",
+			want:         "$NEW:/a/config",
+		},
+		{
+			name:         "multiple entries keep their order",
+			kubeconfigIn: "/a/config:/b/config:/c/config",
+			want:         "$NEW:/a/config:/b/config:/c/config",
+		},
+		{
+			name:         "previous switch tmp kubeconfig is dropped",
+			kubeconfigIn: "$TMP/old:/a/config",
+			want:         "$NEW:/a/config",
+		},
+		{
+			name:         "only a switch tmp kubeconfig",
+			kubeconfigIn: "$TMP/old",
+			want:         "$NEW",
+		},
+		{
+			name:         "empty entries are dropped",
+			kubeconfigIn: ":/a/config::/b/config:",
+			want:         "$NEW:/a/config:/b/config",
+		},
+		{
+			name:         "entries containing spaces stay intact",
+			kubeconfigIn: "/a/my config:/b/config",
+			want:         "$NEW:/a/my config:/b/config",
+		},
+	}
+
+	for _, shell := range shellsUnderTest {
+		if _, err := exec.LookPath(shell); err != nil {
+			t.Logf("skipping %s: not installed", shell)
+			continue
+		}
+
+		for scriptName, scriptPath := range scriptsUnderTest(t, t.TempDir()) {
+			for _, tt := range tests {
+				t.Run(fmt.Sprintf("%s/%s/%s", shell, scriptName, tt.name), func(t *testing.T) {
+					home := t.TempDir()
+					tmpDir := filepath.Join(home, ".kube", ".switch_tmp", "config")
+					if err := os.MkdirAll(tmpDir, 0o755); err != nil {
+						t.Fatalf("create switch tmp dir: %v", err)
+					}
+					newKubeconfig := filepath.Join(tmpDir, "new")
+					if err := os.WriteFile(newKubeconfig, []byte("new\n"), 0o600); err != nil {
+						t.Fatalf("write new kubeconfig: %v", err)
+					}
+
+					expand := func(s string) string {
+						s = strings.ReplaceAll(s, "$TMP", tmpDir)
+						return strings.ReplaceAll(s, "$NEW", newKubeconfig)
+					}
+					kubeconfigIn := expand(tt.kubeconfigIn)
+					want := expand(tt.want)
+
+					// the wrapper deletes a stale switch kubeconfig it finds in KUBECONFIG
+					for part := range strings.SplitSeq(kubeconfigIn, ":") {
+						if strings.HasPrefix(part, tmpDir) {
+							if err := os.WriteFile(part, []byte("stale\n"), 0o600); err != nil {
+								t.Fatalf("write stale kubeconfig: %v", err)
+							}
+						}
+					}
+
+					fake := writeFakeKswitch(t, home, newKubeconfig, "my-context")
+					got := runKswitchFunction(t, shell, scriptPath, fake, home, kubeconfigIn)
+
+					if got != want {
+						t.Errorf("KUBECONFIG = %q, want %q", got, want)
+					}
+				})
+			}
+		}
+	}
+}
+
+// TestShellWrapperRemovesStaleKubeconfig covers the cleanup of the temporary
+// kubeconfig written by the previous switch.
+func TestShellWrapperRemovesStaleKubeconfig(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX shell wrapper is not used on windows")
+	}
+
+	for _, shell := range shellsUnderTest {
+		if _, err := exec.LookPath(shell); err != nil {
+			t.Logf("skipping %s: not installed", shell)
+			continue
+		}
+
+		for scriptName, scriptPath := range scriptsUnderTest(t, t.TempDir()) {
+			t.Run(shell+"/"+scriptName, func(t *testing.T) {
+				home := t.TempDir()
+				tmpDir := filepath.Join(home, ".kube", ".switch_tmp", "config")
+				if err := os.MkdirAll(tmpDir, 0o755); err != nil {
+					t.Fatalf("create switch tmp dir: %v", err)
+				}
+				stale := filepath.Join(tmpDir, "old")
+				if err := os.WriteFile(stale, []byte("stale\n"), 0o600); err != nil {
+					t.Fatalf("write stale kubeconfig: %v", err)
+				}
+				newKubeconfig := filepath.Join(tmpDir, "new")
+				if err := os.WriteFile(newKubeconfig, []byte("new\n"), 0o600); err != nil {
+					t.Fatalf("write new kubeconfig: %v", err)
+				}
+
+				fake := writeFakeKswitch(t, home, newKubeconfig, "my-context")
+				runKswitchFunction(t, shell, scriptPath, fake, home, stale)
+
+				if _, err := os.Stat(stale); !os.IsNotExist(err) {
+					t.Errorf("stale kubeconfig %s still exists (stat err = %v)", stale, err)
+				}
+			})
+		}
+	}
+}
+
+// TestShellWrapperAvoidsBashOnlyArrayRead guards against reintroducing
+// `read -a`, which zsh does not implement: the merge loop would then silently
+// drop every pre-existing KUBECONFIG entry for zsh users.
+func TestShellWrapperAvoidsBashOnlyArrayRead(t *testing.T) {
+	homebrew, err := os.ReadFile(switchShellScriptPath)
+	if err != nil {
+		t.Fatalf("read %s: %v", switchShellScriptPath, err)
+	}
+
+	scripts := map[string]string{
+		"shellScriptTemplate": shellScriptTemplate,
+		switchShellScriptPath: string(homebrew),
+	}
+	for name, script := range scripts {
+		for _, forbidden := range []string{"read -ra", "read -r -a", "read -a"} {
+			if strings.Contains(script, forbidden) {
+				t.Errorf("%s contains %q, which is not supported by zsh", name, forbidden)
+			}
+		}
+	}
+}
+
+// TestShellWrapperCopiesAreInSync checks the KUBECONFIG merge block of the
+// generated script and of the homebrew copy are identical, as required by the
+// "PLEASE KEEP IN SYNC" note at the top of switch.sh.
+func TestShellWrapperCopiesAreInSync(t *testing.T) {
+	homebrew, err := os.ReadFile(switchShellScriptPath)
+	if err != nil {
+		t.Fatalf("read %s: %v", switchShellScriptPath, err)
+	}
+
+	generatedBlock := mergeBlock(t, "shellScriptTemplate", shellScriptTemplate)
+	homebrewBlock := mergeBlock(t, switchShellScriptPath, string(homebrew))
+
+	if generatedBlock != homebrewBlock {
+		t.Errorf("KUBECONFIG merge block out of sync:\n%s\n--- vs ---\n%s", generatedBlock, homebrewBlock)
+	}
+}
+
+// mergeBlock extracts the KUBECONFIG merge block from a shell script with
+// indentation normalized, so the two copies can be compared.
+func mergeBlock(t *testing.T, name, script string) string {
+	t.Helper()
+
+	const (
+		start = `local _new_kc="$KUBECONFIG_PATH"`
+		end   = `unset _new_kc`
+	)
+
+	_, after, found := strings.Cut(script, start)
+	if !found {
+		t.Fatalf("%s: start of the KUBECONFIG merge block not found", name)
+	}
+	block, _, found := strings.Cut(after, end)
+	if !found {
+		t.Fatalf("%s: end of the KUBECONFIG merge block not found", name)
+	}
+
+	var normalized []string
+	for line := range strings.SplitSeq(block, "\n") {
+		if line = strings.TrimSpace(line); line != "" {
+			normalized = append(normalized, line)
+		}
+	}
+	return strings.Join(normalized, "\n")
+}

hack/switch/switch.sh

@@ -71,14 +71,18 @@ function kswitch(){
 
   local _new_kc="$KUBECONFIG_PATH"
   if [[ -n "$KUBECONFIG" ]]; then
-    local _part
-    IFS=':' read -ra _kc_parts <<< "$KUBECONFIG"
-    for _part in "${_kc_parts[@]}"; do
+    local _rest="$KUBECONFIG" _part
+    while [[ -n "$_rest" ]]; do
+      _part="${_rest%%:*}"
+      case "$_rest" in
+        *:*) _rest="${_rest#*:}" ;;
+        *) _rest="" ;;
+      esac
       if [[ -n "$_part" && "$_part" != *"$switchTmpDirectory"* ]]; then
         _new_kc="$_new_kc:$_part"
       fi
     done
-    unset _kc_parts _part
+    unset _rest _part
   fi
   export KUBECONFIG="$_new_kc"
   unset _new_kc