devantler-tech / devantler-tech/ksail
fix(clusterapi): colliding raw and detected context names resolve one unmanaged row onto another's cluster
- Dominant language
- Go
- Stars
- 165
- Forks
- 12
- Avg merge
- 5h 41m
- Merged PRs (30d)
- 337
Description
> 🤖 Generated by the Agentic Engineer
## Evidence
`contextForCluster` (`pkg/cli/clusterapi/resources.go`) resolves a cluster name to a kubeconfig context in two passes: first any context whose **KSail-detected** name matches, then an exact **raw context name** match. The detected-name pass deliberately wins, so a KSail-managed cluster `prod` keeps resolving to its distribution context `kind-prod` even if some other context is literally named `prod`.
That precedence is right for a managed cluster. It is ambiguous when **both** names belong to unmanaged rows.
With a kubeconfig holding contexts `prod` and `kind-prod`, and neither corresponding to a KSail-managed cluster:
- `List` surfaces **both** as unmanaged rows, keyed by raw context name (`newUnmanagedCluster`), because neither is in the managed set.
- Selecting the row named `prod` resolves through the detected-name pass to **`kind-prod`** — `kind-prod` detects to cluster name `prod` — so the operation targets the other row's cluster.
- Selecting `kind-prod` resolves to `kind-prod` via the raw-name fallback, which is correct.
So one visible row silently operates on another row's cluster, across every resource surface: reads, apply, scale, delete, exec and proxy.
## Scope and history
This is **pre-existing, not introduced by the raw-name fallback** (#6893 / #6116). Before that change the `prod` row resolved to `kind-prod` in exactly the same way — the detected-name pass is untouched. What the fallback changed is that `kind-prod` became operable at all instead of returning 404. The ambiguity for `prod` is unchanged, and no behaviour got worse; it is simply now the only remaining wrong-cluster path in that resolver, which is why it is worth closing.
`TestContextForClusterDetectedNameKeepsPrecedence` currently pins the precedence as intended behaviour. Whatever fix is chosen will need to revisit that test's premise rather than work around it.
## Affected audience and impact
Anyone whose kubeconfig contains an unmanaged context whose name is a distribution-prefixed form of another unmanaged context's name — `prod` alongside `kind-prod`, `k3d-prod`, or `admin@prod`. Narrow, but the failure mode is the worst kind: no error, a plausible result, and the wrong cluster. It is the same class of defect as #6117, one layer down.
## Expected behaviour
Selecting a listed row operates on that row's own context, or fails loudly. No row silently resolves to a different row's cluster.
## Proposed direction
Two shapes, and the second is the more thorough:
1. **Make the resolver ownership-aware.** Apply the detected-name pass only for names that correspond to a KSail-managed cluster, and resolve an unmanaged row by its raw context name alone. This preserves the precedence that protects managed clusters while removing it exactly where it causes the collision.
2. **Give unmanaged rows unambiguous keys.** The deeper issue is that a row's identity and a cluster's name share one namespace. Keying unmanaged rows so they cannot collide with a detected name would remove the class rather than this instance — a larger change touching the surfaces that render and address those rows.
Either way, a name that genuinely cannot be resolved unambiguously should return an ambiguity error rather than pick one, mirroring how `cluster info` and `cluster switch` already handle a name matching several contexts.
## Acceptance criteria
- [ ] With unmanaged contexts `prod` and `kind-prod` present and no managed cluster of either name, selecting each row resolves to its own context.
- [ ] A managed cluster still resolves to its distribution context when an unrelated context shares its name.
- [ ] A genuinely ambiguous name reports ambiguity instead of silently choosing.
- [ ] `TestContextForClusterDetectedNameKeepsPrecedence` is updated to express the chosen rule rather than the current unconditional precedence.
## Rough size
S–M, depending on which shape is chosen.
Found while reviewing #6893. Reported by Codex on that PR and confirmed against the code here.
Contributor guide
Research direction
Start in pkg/cli/clusterapi/resources.go at contextForCluster, then read TestContextForClusterDetectedNameKeepsPrecedence and the surrounding clusterapi tests. Run the focused tests with unmanaged contexts such as prod and kind-prod, and trace the listed-row resolution paths. Done means each unmanaged row targets its own context, managed clusters retain their precedence, and genuinely ambiguous names return an error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 56/100