From a750872139046334e090140aab2acd72dca73f1b Mon Sep 17 00:00:00 2001 From: ZeikoFr <54892453+ZeikoFr@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:33:35 +0200 Subject: [PATCH 1/4] fix(shell): use portable KUBECONFIG merge loop for zsh compatibility --- cmd/kswitch/init.go | 12 ++++++++---- hack/switch/switch.sh | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cmd/kswitch/init.go b/cmd/kswitch/init.go index f751ba0c4..7d7194b4a 100644 --- a/cmd/kswitch/init.go +++ b/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 diff --git a/hack/switch/switch.sh b/hack/switch/switch.sh index c426b4923..3c4e1a679 100755 --- a/hack/switch/switch.sh +++ b/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 From df3a0d71c684f1161f5bdbf9c3f903d970f91e41 Mon Sep 17 00:00:00 2001 From: ZeikoFr <54892453+ZeikoFr@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:34:20 +0200 Subject: [PATCH 2/4] fix(switcher): resolve selected item's store and tags by identity, not by path --- pkg/main.go | 50 +++++++++----------------------------------------- pkg/tui/run.go | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 49 deletions(-) diff --git a/pkg/main.go b/pkg/main.go index ac064596c..23a85d979 100644 --- a/pkg/main.go +++ b/pkg/main.go @@ -33,12 +33,6 @@ import ( ) var ( - pathToTagsMapping = make(map[string]map[string]string) - pathToTagsMappingLock = sync.RWMutex{} - - pathToStoreID = make(map[string]string) - pathToStoreLock = sync.RWMutex{} - aliasToContext = make(map[string]string) aliasToContextLock = sync.RWMutex{} @@ -64,8 +58,8 @@ func Switcher(stores []storetypes.KubeconfigStore, config *types.Config, stateDi kindToStore[s.GetID()] = s } - // Collect alias and mapping data from the discovery channel so that after - // the TUI returns we can look up path / tags / storeID by context name. + // Collect alias data from the discovery channel so that after the TUI + // returns we can look up the original context name behind an alias. // We also feed a ContextItem channel to tui.Run so the TUI can stream items // as they are discovered. tuiCh := make(chan tui.ContextItem) @@ -99,9 +93,6 @@ func Switcher(stores []storetypes.KubeconfigStore, config *types.Config, stateDi writeToAliasToContext(dc.Alias, dc.Name) } - writeToPathToTagsMapping(dc.Path, dc.Tags) - writeToPathToStoreID(dc.Path, kubeconfigStore.GetID()) - tuiCh <- tui.ContextItem{ ContextName: contextName, Alias: dc.Alias, @@ -116,7 +107,7 @@ func Switcher(stores []storetypes.KubeconfigStore, config *types.Config, stateDi defer logSearchErrors() - kubeconfigPath, selectedContext, dynamicStore, err := tui.Run(tuiCh, kindToStore, showPreview) + kubeconfigPath, selectedContext, storeID, tags, dynamicStore, err := tui.Run(tuiCh, kindToStore, showPreview) if err != nil { return nil, nil, err } @@ -125,18 +116,19 @@ func Switcher(stores []storetypes.KubeconfigStore, config *types.Config, stateDi return nil, nil, nil } - // use dynamic store (k0smotron sub-cluster) if present, else look up by path + // use dynamic store (k0smotron sub-cluster) if present, else the store the + // selected item was discovered from. Resolving via the selected item's own + // storeID (rather than a global path->store lookup) matters because "path" + // is only unique within a single store: two different stores can each have + // a cluster with the same path (e.g. both named "prod"), which would + // otherwise resolve to whichever store happened to be indexed last. var store storetypes.KubeconfigStore if dynamicStore != nil { store = dynamicStore } else { - storeID := readFromPathToStoreID(kubeconfigPath) store = kindToStore[storeID] } - // get the tags associated with the selected kubeconfig path - tags := readFromPathToTagsMapping(kubeconfigPath) - // use the store to get the kubeconfig for the selected kubeconfig path kubeconfigData, err := store.GetKubeconfigForPath(kubeconfigPath, tags) if err != nil { @@ -200,30 +192,6 @@ func writeIndex(store storetypes.KubeconfigStore, searchIndex *index.SearchIndex } } -func readFromPathToTagsMapping(key string) map[string]string { - pathToTagsMappingLock.RLock() - defer pathToTagsMappingLock.RUnlock() - return pathToTagsMapping[key] -} - -func writeToPathToTagsMapping(key string, value map[string]string) { - pathToTagsMappingLock.Lock() - defer pathToTagsMappingLock.Unlock() - pathToTagsMapping[key] = value -} - -func readFromPathToStoreID(key string) string { - pathToStoreLock.RLock() - defer pathToStoreLock.RUnlock() - return pathToStoreID[key] -} - -func writeToPathToStoreID(key string, value string) { - pathToStoreLock.Lock() - defer pathToStoreLock.Unlock() - pathToStoreID[key] = value -} - func writeToAliasToContext(key, value string) { aliasToContextLock.Lock() defer aliasToContextLock.Unlock() diff --git a/pkg/tui/run.go b/pkg/tui/run.go index 41578f357..9b78c5e8f 100644 --- a/pkg/tui/run.go +++ b/pkg/tui/run.go @@ -45,14 +45,17 @@ type ContextItem struct { // Run launches the interactive bubbletea TUI and blocks until the user selects // a context or aborts. itemCh must be closed by the caller when discovery ends. -// Returns the path and display name of the selected item, or ErrAbort if the -// user cancelled. dynamicStore is non-nil when the selection is a k0smotron -// sub-cluster whose kubeconfig lives in an in-memory store. +// Returns the path, display name, backing storeID and tags of the selected +// item, or ErrAbort if the user cancelled. dynamicStore is non-nil when the +// selection is a k0smotron sub-cluster whose kubeconfig lives in an in-memory +// store. storeID/tags are taken from the selected item itself (rather than a +// path-keyed lookup) since "path" is only unique within a single store, not +// across all stores. func Run( itemCh <-chan ContextItem, storeIDToStore map[string]storetypes.KubeconfigStore, showPreview bool, -) (kubeconfigPath string, selectedContext string, dynamicStore storetypes.KubeconfigStore, err error) { +) (kubeconfigPath string, selectedContext string, storeID string, tags map[string]string, dynamicStore storetypes.KubeconfigStore, err error) { model := NewModel(storeIDToStore, showPreview) p := tea.NewProgram(model, tea.WithOutput(os.Stderr)) @@ -82,16 +85,16 @@ func Run( finalModel, runErr := p.Run() if runErr != nil { - return "", "", nil, fmt.Errorf("tui error: %w", runErr) + return "", "", "", nil, nil, fmt.Errorf("tui error: %w", runErr) } m, ok := finalModel.(Model) if !ok { - return "", "", nil, fmt.Errorf("unexpected model type") + return "", "", "", nil, nil, fmt.Errorf("unexpected model type") } if m.Aborted || m.Selected == nil { - return "", "", nil, ErrAbort + return "", "", "", nil, nil, ErrAbort } var dynStore storetypes.KubeconfigStore @@ -99,5 +102,5 @@ func Run( dynStore = ds } - return m.Selected.path, m.Selected.contextName, dynStore, nil + return m.Selected.path, m.Selected.contextName, m.Selected.storeID, m.Selected.tags, dynStore, nil } From 47acb3cbdee759d984a73e8c6c28c688a3295d56 Mon Sep 17 00:00:00 2001 From: ZeikoFr <54892453+ZeikoFr@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:24:01 +0200 Subject: [PATCH 3/4] test(shell): cover the portable KUBECONFIG merge in bash and zsh Sources the wrapper function in a real shell and asserts the resulting KUBECONFIG, for both copies of the script (the `kswitch init bash|zsh` template and the homebrew `hack/switch/switch.sh`) in both bash and zsh: entries are kept in order, stale switch temp kubeconfigs and empty segments are dropped and entries containing spaces survive. Reverting the portable merge loop fails the zsh half of the matrix, because zsh has no `read -a` and silently dropped every pre-existing KUBECONFIG entry. Two static checks come along: neither copy may reintroduce `read -a`, and their merge blocks have to stay in sync. --- cmd/kswitch/init_test.go | 303 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 cmd/kswitch/init_test.go diff --git a/cmd/kswitch/init_test.go b/cmd/kswitch/init_test.go new file mode 100644 index 000000000..9340c781b --- /dev/null +++ b/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") +} From 874fff3da0037e55e8974be5a6489853e689391d Mon Sep 17 00:00:00 2001 From: ZeikoFr <54892453+ZeikoFr@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:24:27 +0200 Subject: [PATCH 4/4] test(switcher): cover resolving the selected item's store by identity Extracts the selection plumbing into helpers that can be tested without a TTY: tui.itemFor builds a list item from a discovered context, and tui.selectionFor returns what Run returns for a finished model. The store lookup in Switcher moves into pkg.resolveStore, which now reports an unknown storeID instead of dereferencing a nil store. The tests pin down that the storeID and tags travel with the selected item: two stores exposing a cluster under the same path ("prod") each resolve to their own store and tags, whichever one the cursor sits on. --- pkg/main.go | 33 ++++-- pkg/resolve_store_test.go | 102 ++++++++++++++++++ pkg/tui/run.go | 31 ++++-- pkg/tui/run_test.go | 222 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 368 insertions(+), 20 deletions(-) create mode 100644 pkg/resolve_store_test.go create mode 100644 pkg/tui/run_test.go diff --git a/pkg/main.go b/pkg/main.go index 23a85d979..9d315fc9d 100644 --- a/pkg/main.go +++ b/pkg/main.go @@ -116,17 +116,9 @@ func Switcher(stores []storetypes.KubeconfigStore, config *types.Config, stateDi return nil, nil, nil } - // use dynamic store (k0smotron sub-cluster) if present, else the store the - // selected item was discovered from. Resolving via the selected item's own - // storeID (rather than a global path->store lookup) matters because "path" - // is only unique within a single store: two different stores can each have - // a cluster with the same path (e.g. both named "prod"), which would - // otherwise resolve to whichever store happened to be indexed last. - var store storetypes.KubeconfigStore - if dynamicStore != nil { - store = dynamicStore - } else { - store = kindToStore[storeID] + store, err := resolveStore(kindToStore, storeID, dynamicStore) + if err != nil { + return nil, nil, err } // use the store to get the kubeconfig for the selected kubeconfig path @@ -169,6 +161,25 @@ func Switcher(stores []storetypes.KubeconfigStore, config *types.Config, stateDi return &tempKubeconfigPath, &selectedContext, nil } +// resolveStore returns the store that holds the kubeconfig for the selected item. +// It uses the dynamic store (k0smotron sub-cluster) if present, else the store the +// selected item was discovered from. Resolving via the selected item's own +// storeID (rather than a global path->store lookup) matters because "path" +// is only unique within a single store: two different stores can each have +// a cluster with the same path (e.g. both named "prod"), which would +// otherwise resolve to whichever store happened to be indexed last. +func resolveStore(kindToStore map[string]storetypes.KubeconfigStore, storeID string, dynamicStore storetypes.KubeconfigStore) (storetypes.KubeconfigStore, error) { + if dynamicStore != nil { + return dynamicStore, nil + } + + store, ok := kindToStore[storeID] + if !ok || store == nil { + return nil, fmt.Errorf("failed to find the kubeconfig store %q the selected context was discovered from", storeID) + } + return store, nil +} + // writeIndex tries to write the Index file for the kubeconfig store func writeIndex(store storetypes.KubeconfigStore, searchIndex *index.SearchIndex, ctxToPathMapping map[string]string, ctxToTagsMapping map[string]map[string]string) { index := types.Index{ diff --git a/pkg/resolve_store_test.go b/pkg/resolve_store_test.go new file mode 100644 index 000000000..76e580bf9 --- /dev/null +++ b/pkg/resolve_store_test.go @@ -0,0 +1,102 @@ +// 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 pkg + +import ( + "testing" + + storetypes "github.com/MichaelSp/kswitch/pkg/store/types" + "github.com/MichaelSp/kswitch/types" +) + +// storesByID indexes stores the way Switcher does. +func storesByID(stores ...storetypes.KubeconfigStore) map[string]storetypes.KubeconfigStore { + byID := map[string]storetypes.KubeconfigStore{} + for _, s := range stores { + byID[s.GetID()] = s + } + return byID +} + +// TestResolveStore_PathCollisionAcrossStores is the regression test for +// resolving the selected context's store by identity: both stores expose a +// cluster under the path "prod", so a path-keyed lookup would return whichever +// store was indexed last. +func TestResolveStore_PathCollisionAcrossStores(t *testing.T) { + gke := &stubStore{ + kind: types.StoreKindGKE, + kubeconfigs: map[string][]byte{"prod": []byte("gke-prod")}, + } + eks := &stubStore{ + kind: types.StoreKindEKS, + kubeconfigs: map[string][]byte{"prod": []byte("eks-prod")}, + } + kindToStore := storesByID(gke, eks) + + tests := []struct { + storeID string + want string + }{ + {storeID: gke.GetID(), want: "gke-prod"}, + {storeID: eks.GetID(), want: "eks-prod"}, + } + + for _, tt := range tests { + store, err := resolveStore(kindToStore, tt.storeID, nil) + if err != nil { + t.Fatalf("resolveStore(%q) error = %v", tt.storeID, err) + } + + data, err := store.GetKubeconfigForPath("prod", nil) + if err != nil { + t.Fatalf("GetKubeconfigForPath() error = %v", err) + } + if string(data) != tt.want { + t.Errorf("storeID %q returned kubeconfig %q, want %q", tt.storeID, data, tt.want) + } + } +} + +func TestResolveStore_DynamicStoreTakesPrecedence(t *testing.T) { + gke := &stubStore{kind: types.StoreKindGKE, kubeconfigs: map[string][]byte{"prod": []byte("gke-prod")}} + dynamic := &stubStore{ + kind: types.StoreKindK0smotron, + kubeconfigs: map[string][]byte{"prod": []byte("k0smotron-child")}, + } + + // Even with a storeID pointing at a regular store, the dynamic (in-memory) + // k0smotron store wins for sub-cluster selections. + store, err := resolveStore(storesByID(gke), gke.GetID(), dynamic) + if err != nil { + t.Fatalf("resolveStore() error = %v", err) + } + if store != dynamic { + t.Fatalf("resolveStore() = %v, want the dynamic store", store) + } +} + +func TestResolveStore_UnknownStoreID(t *testing.T) { + gke := &stubStore{kind: types.StoreKindGKE} + + if _, err := resolveStore(storesByID(gke), "vault.default", nil); err == nil { + t.Error("resolveStore() with an unknown storeID: expected an error, got nil") + } +} + +func TestResolveStore_EmptyStoreID(t *testing.T) { + if _, err := resolveStore(map[string]storetypes.KubeconfigStore{}, "", nil); err == nil { + t.Error("resolveStore() with no stores: expected an error, got nil") + } +} diff --git a/pkg/tui/run.go b/pkg/tui/run.go index 9b78c5e8f..1673e1bb1 100644 --- a/pkg/tui/run.go +++ b/pkg/tui/run.go @@ -63,15 +63,7 @@ func Run( go func() { var batch []item for ci := range itemCh { - primary, suffix := FormatDisplayName(types.StoreKind(ci.StoreKind), ci.Path, ci.ContextName, ci.Alias, ci.Tags, ci.LabelDisplayKeys) - batch = append(batch, item{ - displayName: primary, - dimSuffix: suffix, - contextName: ci.ContextName, - path: ci.Path, - tags: ci.Tags, - storeID: ci.StoreID, - }) + batch = append(batch, itemFor(ci)) if len(batch) >= 50 { p.Send(itemsMsg(batch)) batch = nil @@ -93,6 +85,27 @@ func Run( return "", "", "", nil, nil, fmt.Errorf("unexpected model type") } + return selectionFor(m) +} + +// itemFor converts a discovered context into a list item. The storeID and tags +// travel with the item so the selection can later be resolved by identity +// instead of by path, which is only unique within a single store. +func itemFor(ci ContextItem) item { + primary, suffix := FormatDisplayName(types.StoreKind(ci.StoreKind), ci.Path, ci.ContextName, ci.Alias, ci.Tags, ci.LabelDisplayKeys) + return item{ + displayName: primary, + dimSuffix: suffix, + contextName: ci.ContextName, + path: ci.Path, + tags: ci.Tags, + storeID: ci.StoreID, + } +} + +// selectionFor extracts the selection from a finished TUI model, returning the +// same values as Run. +func selectionFor(m Model) (kubeconfigPath string, selectedContext string, storeID string, tags map[string]string, dynamicStore storetypes.KubeconfigStore, err error) { if m.Aborted || m.Selected == nil { return "", "", "", nil, nil, ErrAbort } diff --git a/pkg/tui/run_test.go b/pkg/tui/run_test.go new file mode 100644 index 000000000..fa55ae085 --- /dev/null +++ b/pkg/tui/run_test.go @@ -0,0 +1,222 @@ +// Copyright 2021 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 tui + +import ( + "errors" + "reflect" + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/MichaelSp/kswitch/types" +) + +// ---- itemFor ---- + +func TestItemFor_CarriesStoreIDAndTags(t *testing.T) { + tags := map[string]string{"project": "garden-foo"} + it := itemFor(ContextItem{ + ContextName: "my-context", + StoreKind: string(types.StoreKindGKE), + Path: "gke_proj--europe-west1--prod", + Tags: tags, + StoreID: "gke.default", + }) + + if it.contextName != "my-context" { + t.Errorf("contextName = %q, want my-context", it.contextName) + } + if it.path != "gke_proj--europe-west1--prod" { + t.Errorf("path = %q", it.path) + } + if it.storeID != "gke.default" { + t.Errorf("storeID = %q, want gke.default", it.storeID) + } + if !reflect.DeepEqual(it.tags, tags) { + t.Errorf("tags = %v, want %v", it.tags, tags) + } +} + +func TestItemFor_SamePathFromDifferentStores(t *testing.T) { + // Two stores can each expose a cluster under the same path (e.g. "prod"). + // Every item must keep the storeID and tags of the store it came from. + a := itemFor(ContextItem{ + ContextName: "prod", + StoreKind: string(types.StoreKindGKE), + Path: "prod", + Tags: map[string]string{"account": "a"}, + StoreID: "gke.default", + }) + b := itemFor(ContextItem{ + ContextName: "prod", + StoreKind: string(types.StoreKindEKS), + Path: "prod", + Tags: map[string]string{"account": "b"}, + StoreID: "eks.default", + }) + + if a.storeID == b.storeID { + t.Fatalf("both items resolved to store %q", a.storeID) + } + if a.storeID != "gke.default" || b.storeID != "eks.default" { + t.Errorf("storeIDs = %q / %q, want gke.default / eks.default", a.storeID, b.storeID) + } + if a.tags["account"] != "a" || b.tags["account"] != "b" { + t.Errorf("tags = %v / %v, want account a / account b", a.tags, b.tags) + } +} + +// ---- selectionFor ---- + +func TestSelectionFor_ReturnsIdentityOfSelectedItem(t *testing.T) { + tags := map[string]string{"project": "garden-foo"} + m := NewModel(nil, false) + m.Selected = &item{ + contextName: "my-context", + path: "some/path", + storeID: "gardener.default", + tags: tags, + } + + path, ctx, storeID, gotTags, dynStore, err := selectionFor(m) + if err != nil { + t.Fatalf("selectionFor() error = %v", err) + } + if path != "some/path" { + t.Errorf("path = %q, want some/path", path) + } + if ctx != "my-context" { + t.Errorf("selectedContext = %q, want my-context", ctx) + } + if storeID != "gardener.default" { + t.Errorf("storeID = %q, want gardener.default", storeID) + } + if !reflect.DeepEqual(gotTags, tags) { + t.Errorf("tags = %v, want %v", gotTags, tags) + } + if dynStore != nil { + t.Errorf("dynamicStore = %v, want nil", dynStore) + } +} + +func TestSelectionFor_Aborted(t *testing.T) { + m := NewModel(nil, false) + m.Aborted = true + m.Selected = &item{path: "p"} + + if _, _, _, _, _, err := selectionFor(m); !errors.Is(err, ErrAbort) { + t.Errorf("error = %v, want ErrAbort", err) + } +} + +func TestSelectionFor_NothingSelected(t *testing.T) { + if _, _, _, _, _, err := selectionFor(NewModel(nil, false)); !errors.Is(err, ErrAbort) { + t.Errorf("error = %v, want ErrAbort", err) + } +} + +func TestSelectionFor_DynamicStore(t *testing.T) { + dyn := &stubStore{id: "k0smotron.parent"} + m := NewModel(nil, false) + m.dynamicStores["k0smotron.parent"] = dyn + m.Selected = &item{path: "parent/ns/child", storeID: "k0smotron.parent"} + + _, _, storeID, _, dynStore, err := selectionFor(m) + if err != nil { + t.Fatalf("selectionFor() error = %v", err) + } + if storeID != "k0smotron.parent" { + t.Errorf("storeID = %q, want k0smotron.parent", storeID) + } + if dynStore != dyn { + t.Errorf("dynamicStore = %v, want the registered store", dynStore) + } +} + +func TestSelectionFor_NoDynamicStoreForRegularItem(t *testing.T) { + m := NewModel(nil, false) + m.dynamicStores["k0smotron.parent"] = &stubStore{id: "k0smotron.parent"} + m.Selected = &item{path: "prod", storeID: "gke.default"} + + if _, _, _, _, dynStore, _ := selectionFor(m); dynStore != nil { + t.Errorf("dynamicStore = %v, want nil for a non-k0smotron selection", dynStore) + } +} + +// ---- selection through the model, as Run drives it ---- + +// updateModel feeds msg to the model the way the bubbletea event loop does. +func updateModel(t *testing.T, m Model, msg tea.Msg) Model { + t.Helper() + + updated, _ := m.Update(msg) + next, ok := updated.(Model) + if !ok { + t.Fatalf("Update() returned %T, want tui.Model", updated) + } + return next +} + +// TestEnterSelectsStoreOfCursorItem is the regression test for resolving the +// selection by identity: two stores expose a cluster under the same path, and +// whichever one the cursor sits on must be the one returned — not the one that +// happened to be discovered last. +func TestEnterSelectsStoreOfCursorItem(t *testing.T) { + items := []ContextItem{ + {ContextName: "gke-prod", StoreKind: string(types.StoreKindGKE), Path: "prod", StoreID: "gke.default", Tags: map[string]string{"account": "a"}}, + {ContextName: "eks-prod", StoreKind: string(types.StoreKindEKS), Path: "prod", StoreID: "eks.default", Tags: map[string]string{"account": "b"}}, + } + + for cursor, want := range items { + m := NewModel(nil, false) + m.width, m.height = 80, 24 + + batch := make([]item, 0, len(items)) + for _, ci := range items { + batch = append(batch, itemFor(ci)) + } + m = updateModel(t, m, itemsMsg(batch)) + m.cursor = cursor + m = updateModel(t, m, tea.KeyPressMsg{Code: tea.KeyEnter}) + + path, ctx, storeID, tags, _, err := selectionFor(m) + if err != nil { + t.Fatalf("cursor %d: selectionFor() error = %v", cursor, err) + } + if path != want.Path { + t.Errorf("cursor %d: path = %q, want %q", cursor, path, want.Path) + } + if ctx != want.ContextName { + t.Errorf("cursor %d: selectedContext = %q, want %q", cursor, ctx, want.ContextName) + } + if storeID != want.StoreID { + t.Errorf("cursor %d: storeID = %q, want %q", cursor, storeID, want.StoreID) + } + if !reflect.DeepEqual(tags, want.Tags) { + t.Errorf("cursor %d: tags = %v, want %v", cursor, tags, want.Tags) + } + } +} + +func TestEnterOnEmptyListAborts(t *testing.T) { + m := NewModel(nil, false) + m.width, m.height = 80, 24 + + m = updateModel(t, m, tea.KeyPressMsg{Code: tea.KeyEnter}) + + if _, _, _, _, _, err := selectionFor(m); !errors.Is(err, ErrAbort) { + t.Errorf("error = %v, want ErrAbort", err) + } +}