devantler-tech / devantler-tech/ksail

fix(clusterapi): colliding raw and detected context names resolve one unmanaged row onto another's cluster

Open
#6,909 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.