Deep Review: 20260503-002937-pr-348

// loadClientConfig builds a rest.Config that honors KUBECONFIG plus any
// --kubeconfig and --context flags in args, the way kubectl itself
// resolves them. clientcmd does not parse command-line flags, so
// extractKubeFlags pulls the two we care about out of args by hand.
func loadClientConfig(args []string) (*rest.Config, error) {
	kubeconfig, context := extractKubeFlags(args)
	rules := clientcmd.NewDefaultClientConfigLoadingRules()
	rules.ExplicitPath = kubeconfig
	overrides := &clientcmd.ConfigOverrides{CurrentContext: context}
	return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(rules, overrides).ClientConfig()
}

// extractKubeFlags scans args for the kubectl flags Resolve cares about
// (--kubeconfig and --context) and returns their values. Both the spaced
// form (--kubeconfig path) and the equals form (--kubeconfig=path) match.
// A flag missing from args yields an empty string, which clientcmd treats
// as "use the default". Scanning stops at "--" because kubectl treats
// everything after it as positional (e.g. `kubectl exec pod -- cmd ...`).
func extractKubeFlags(args []string) (kubeconfig, context string) {
	for i, arg := range args {
		if arg == "--" {
			break
		}
		switch {
		case arg == "--kubeconfig" && i+1 < len(args):
			kubeconfig = args[i+1]
		case strings.HasPrefix(arg, "--kubeconfig="):
			kubeconfig = strings.TrimPrefix(arg, "--kubeconfig=")
		case arg == "--context" && i+1 < len(args):
			context = args[i+1]
		case strings.HasPrefix(arg, "--context="):
			context = strings.TrimPrefix(arg, "--context=")
		}
	}
	return kubeconfig, context
}

extractKubeFlags honors --kubeconfig and --context. kubectl honors several more cluster-selection flags: --server (-s), --cluster, --user, --token, --certificate-authority, --insecure-skip-tls-verify, --client-certificate, --client-key, --tls-server-name. kubectlAction passes the same raw args to Resolve and to the kubectl child, so a user running rdd kubectl --server=https://other.cluster:6443 get pods makes the resolver probe the kubeconfig's default cluster while kubectl talks to other.cluster. The two clusters can advertise different versions, so the resolver may decide "in skew, run embedded" against an out-of-skew target — the very case the resolver exists to handle. Round 1 flagged this as a design observation ("Manual kubectl flag parsing carries drift risk"); Round 2 makes it a concrete gap.

The most consequential override is --server: it routes kubectl at any URL with no kubeconfig involvement and the resolver has no knob for it.

Fix: thread the standard cluster overrides through clientcmd.ConfigOverrides. The minimum is --server → overrides.ClusterInfo.Server, --cluster → overrides.Context.Cluster, --user → overrides.Context.AuthInfo. A more durable fix delegates to genericclioptions.ConfigFlags so resolver and kubectl child cannot drift by construction.

-func extractKubeFlags(args []string) (kubeconfig, context string) {
+func extractKubeFlags(args []string) (kubeconfig, context, server, cluster, user string) {
     for i, arg := range args {
         if arg == "--" {
             break
         }
         switch {
         case arg == "--kubeconfig" && i+1 < len(args):
             kubeconfig = args[i+1]
         case strings.HasPrefix(arg, "--kubeconfig="):
             kubeconfig = strings.TrimPrefix(arg, "--kubeconfig=")
         case arg == "--context" && i+1 < len(args):
             context = args[i+1]
         case strings.HasPrefix(arg, "--context="):
             context = strings.TrimPrefix(arg, "--context=")
+        case arg == "--server" && i+1 < len(args), arg == "-s" && i+1 < len(args):
+            server = args[i+1]
+        case strings.HasPrefix(arg, "--server="), strings.HasPrefix(arg, "-s="):
+            server = strings.TrimPrefix(strings.TrimPrefix(arg, "--server="), "-s=")
+        // …same for --cluster, --user
         }
     }
-    return kubeconfig, context
+    return
 }

If the full flag set is too much code for this PR, document the gap in the Resolve doc and add a unit-test row for --server so the limitation is explicit.

// to skip the resolver and fall through to the embedded kubectl. Each
// path logs at warn so an operator running with the default level
// learns why kubectl skipped a download.
func serverVersion(args []string) (_ semver.Version, ok bool, _ error) {
	cfg, err := loadClientConfig(args)
	if err != nil {
		logrus.WithError(err).Warn("kubectl resolver: cannot load kubeconfig; using embedded kubectl")
		return semver.Version{}, false, nil //nolint:nilerr // legitimate fall-through to embedded kubectl
	}
	cfg.Timeout = serverProbeTimeout
	client, err := discovery.NewDiscoveryClientForConfig(cfg)
	if err != nil {
		logrus.WithError(err).Warn("kubectl resolver: cannot build discovery client; using embedded kubectl")
		return semver.Version{}, false, nil //nolint:nilerr // legitimate fall-through to embedded kubectl
	}
	info, err := client.ServerVersion()
	if err != nil {
		logrus.WithError(err).Warn("kubectl resolver: cluster probe failed; using embedded kubectl")
		return semver.Version{}, false, nil //nolint:nilerr // legitimate fall-through to embedded kubectl
	}
	v, err := semver.ParseTolerant(info.GitVersion)
	if err != nil {
		logrus.WithError(err).Warnf("kubectl resolver: cannot parse server version %q; using embedded kubectl", info.GitVersion)
		return semver.Version{}, false, nil //nolint:nilerr // legitimate fall-through to embedded kubectl
	}
	return v, true, nil
}

// loadClientConfig builds a rest.Config that honors KUBECONFIG plus any

cmd/rdd/log.go:86-90 defaults the log level to warn outside developer mode, and docs/design/environment.md:14 documents the same default. Every rdd kubectl version --client, rdd kubectl --help, and rdd kubectl completion on a host with no kubeconfig (or with a kubeconfig pointing at a currently-unreachable cluster) now prints level=warning msg="kubectl resolver: cannot load kubeconfig; using embedded kubectl" before kubectl runs. Round 1's Resolution table records that the warn-level logging was deliberate — bundled with the S2 parse-failure fix to "let an operator running with the default level learn why kubectl skipped a download" — but the chosen level fires once per command rather than once per surprising state.

| Variable | Description | Default |
| --- | --- | --- |
| `RDD_INSTANCE` | Instance identifier. Determines which control plane and directories to use. Also settable via `rdd --instance`. | `2` |
| `RDD_DEVELOPER_MODE` | Enables developer mode: exposes hidden CLI flags, detects source tree for local builds. | unset |
| `RDD_KEEP_LOGS` | Preserves logs for post-mortem debugging. See [Log Preservation](#log-preservation) for details. | unset |
| `RDD_LOG_LEVEL` | Sets the log level (`fatal`, `error`, `warn`, `info`, `debug`, `trace`). Overridden by `--log-level` flag. When unset, defaults to `debug` in developer mode, `warn` otherwise. | unset |
| `RDD_LOG_TITLE` | When set, writes this string as the first line of each new log file. Useful for identifying log files from specific test runs or sessions. | unset |
| `RDD_CACHE_DIR` | Overrides the rdd-wide cache root. The kubectl resolver appends `kubectl/<os>-<arch>/` inside it. | `os.UserCacheDir()`/`rancher-desktop` (e.g. `~/Library/Caches/rancher-desktop` on macOS) |
| `RDD_KUBECTL_MIRROR` | Overrides the Kubernetes release mirror used to download version-matched kubectl binaries. | `https://dl.k8s.io` |

### BATS Test Variables

The "cannot parse server version" branch (line 113) is genuinely surprising — keep that one at warn. The other three branches (no kubeconfig, no discovery client, cluster probe failed) are routine for a host that uses rdd kubectl for client-only work and should be Debug.

Fix:

-    logrus.WithError(err).Warn("kubectl resolver: cannot load kubeconfig; using embedded kubectl")
+    logrus.WithError(err).Debug("kubectl resolver: cannot load kubeconfig; using embedded kubectl")

Same for the cannot build discovery client and cluster probe failed branches. Leave the cannot parse server version branch at warn.

const envSkipResolver = "RDD_KUBECTL_RESOLVED"

// serverProbeTimeout caps the discovery call. A reachable cluster answers
// in under 100 ms; an unreachable one would otherwise stall every kubectl
// invocation.
const serverProbeTimeout = 2 * time.Second

// Resolve returns the path to a kubectl binary compatible with the cluster
// the user's kubeconfig points at. An empty path means "use the embedded
// kubectl"; a non-empty path means "exec this binary instead". args holds
// the kubectl-style arguments the caller will pass through; Resolve scans

Resolve runs unconditionally on every rdd kubectl and rdd ctl invocation. When the configured cluster is unreachable — laptop on a flight, VPN down, firewalled CI runner — client.ServerVersion() blocks until cfg.Timeout fires. Two seconds of latency on rdd kubectl version --client, rdd kubectl --help, and rdd kubectl completion bash is a regression versus the embedded-only behavior these client-only commands had before this PR.

Fix: short-circuit the obvious client-only invocations before probing. The kubectl subcommands that never need a server are version --client, completion, config, and the help paths (--help, -h):

func Resolve(ctx context.Context, args []string) (string, error) {
    if os.Getenv(envSkipResolver) != "" {
        return "", nil
    }
    if isClientOnly(args) {
        return "", nil
    }
    …
}

If a precise client-only matcher feels brittle, halve serverProbeTimeout to 1 s — modern apiservers answer /version in well under 100 ms, so the headroom does not need to be that large.