Fix zsh shell function & cluster-name collision across different stores - #236
Open
ZeikoFr wants to merge 4 commits into
Open
Fix zsh shell function & cluster-name collision across different stores#236ZeikoFr wants to merge 4 commits into
ZeikoFr wants to merge 4 commits into
Conversation
Owner
|
hey @ZeikoFr thanks for catching these glitches. |
Author
|
Hi @MichaelSp It should be good now, I've generated the test using cc-golang-skill, I've also add an handling of timeout for GKE, I had an organisation that have hundreds of projects and only 1 or 2 GKE cluster in one of them and the timeout was global, causing some issues |
MichaelSp
reviewed
Sep 9, 2026
| channel <- storetypes.SearchResult{ | ||
| Error: fmt.Errorf("failed to list GKE clusters for project with ID %q: %w", projectId, err), | ||
| wg.Add(1) | ||
| go func(projectName, projectId string) { |
Owner
There was a problem hiding this comment.
I think this is a non-trivial decision. What you do here is an unbound number of requests with following side-effects
- Google API 429 / quota failures
- valid clusters disappearing from results and thus non-deterministic behavior based on account size
- that would be a deal-breaker because I can already see issues coming my way 🤣
- valid clusters disappearing from results and thus non-deterministic behavior based on account size
- connection and goroutine spikes - I could live with just that ✅
What do you think?
Author
There was a problem hiding this comment.
I've removed this from the PR, it should not have landed here my bad for opening a PR on my main branch, this change is superseeded in #296 with defaultMaxConcurrentProjectRequests
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two bugs in kswitch's interactive switcher, found while running it day-to-day against a mix of OVH, Scaleway, GKE, EKS, and Gardener stores:
kswitch init zsh/kswitch init bash(and the Homebrew companionhack/switch/switch.sh) usedread -ra ... <<<, which is bash-only. Under zsh this printsbad option: -aon every switch and silently drops any other paths already present in$KUBECONFIGduring the merge.pkg.Switcher()resolved the interactively-selected item's backing store and tags via global maps keyed only by the bare cluster path (e.g."prod"). Path is only unique within a single store. When two different stores each have a cluster with the same path (e.g. an OVH store and a Scaleway store both have a cluster namedprod), whichever store was indexed last silently won the map entry so selecting one store's context could fetch and write out a different store's kubeconfig, then fail withcontext was not found for specified contextbecause the two providers' context-naming conventions differ.Changes
cmd/kswitch/init.go,hack/switch/switch.sh: replace the bash-onlyread -ra ... <<<loop with a portablewhile/case/parameter-expansion loop that behaves identically under bash and zsh.pkg/tui/run.go:Run()now also returns the selected item's ownstoreIDandtags(already carried on the TUI's internalitemstruct), instead of only the path and context name.pkg/main.go:Switcher()uses the storeID/tags returned by the TUI directly to resolve the backing store and fetch the kubeconfig, removing the path-keyedpathToStoreID/pathToTagsMappingglobal maps and their helpers entirely.pkg/subcommands/set-context/set_context.go(the non-interactiveset-contextcommand) already resolved the store per-item correctly and needed no change.Testing
go test ./...passes.kwarto, Scalewayscw-loginline) each with a cluster namedprod; before the fix, selecting the OVH entry fetched the Scaleway cluster's kubeconfig (confirmed via the fetched cluster's ID/server not matching the selected store), andcurrent-contextwas left un-stripped with no matching context entry.kwarto/kubernetes-admin@prodnow correctly fetches the OVH cluster's kubeconfig, withcurrent-contextmatching its own context entry, andkubectlresolves cluster/context/server correctly against the result.bad option: -a, and extra$KUBECONFIGentries are preserved through the merge.