diff --git a/cmd/commands/kubectl.go b/cmd/commands/kubectl.go index f9e9cfc9..bece732c 100644 --- a/cmd/commands/kubectl.go +++ b/cmd/commands/kubectl.go @@ -21,21 +21,31 @@ import ( "errors" "fmt" "io" + "net/http" "os" "os/signal" "regexp" + "strings" "syscall" "time" "github.com/spf13/cobra" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/genericiooptions" + "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/cached/memory" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" + "k8s.io/klog/v2" kubecmd "k8s.io/kubectl/pkg/cmd" "k8s.io/kubectl/pkg/cmd/plugin" + cmdutil "k8s.io/kubectl/pkg/cmd/util" ) const ( @@ -44,10 +54,304 @@ const ( cmImageKey = "image" ) +// Header names the kube-apiserver ACL-filtering patch reads to resolve +// --scope. Kept in sync by hand with +// k8s.io/apiserver/pkg/registry/generic/scopefilter.HeaderScope/HeaderProject +// in the deckhouse/deckhouse kubernetes patch -- there is no shared Go +// package between the two repos to import this from. +const ( + scopeHeaderName = "X-Deckhouse-Scope" + scopeProjectHeaderName = "X-Deckhouse-Project" +) + +// scopeFlagName intentionally matches the unrelated --scope flag already +// registered on `d8 iam access grant/revoke` (different enum: cluster, +// all-namespaces, labels=K=V). They live on disjoint cobra command trees, so +// there is no flag-registration conflict, but a user pasting a value meant +// for one into the other must get a clear "invalid value" error rather than +// having it silently misinterpreted -- see parseScopeFlag. +const scopeFlagName = "scope" + +// scopeProjectValuePrefix is the flag-value spelling of the single-project +// scope: `--scope=project:`. +const scopeProjectValuePrefix = "project:" + +// parseScopeFlag validates raw against the accessible|projects|system|project: +// enum and splits it into the two wire headers scopeHeaderName expects: the +// scope kind and the project name (non-empty only for the project: +// form). Returns ("", "", nil) for an empty raw value -- the flag was not +// passed at all, and callers must send no headers whatsoever in that case, so +// a client without --scope stays byte-for-byte identical to plain kubectl. +func parseScopeFlag(raw string) (string, string, error) { + if raw == "" { + return "", "", nil + } + + if projectName, ok := strings.CutPrefix(raw, scopeProjectValuePrefix); ok { + if projectName == "" { + return "", "", fmt.Errorf("--%s=project: requires a project name, e.g. --%s=project:myproject", scopeFlagName, scopeFlagName) + } + + return "project", projectName, nil + } + + switch raw { + case "accessible", "projects", "system": + return raw, "", nil + default: + return "", "", fmt.Errorf("invalid --%s value %q: must be one of accessible|projects|system|project:", scopeFlagName, raw) + } +} + +// scopeUsageError enforces that --scope is only used where it can actually +// work. --scope filters a cross-namespace listing, which needs the +// cluster-scoped request URL that -A/--all-namespaces produces; without -A the +// request is locked to a single namespace and --scope could only ever narrow +// that one namespace (usually to empty), silently turning a clean 403 into a +// misleading empty result. An explicit -n/--namespace has the same problem, so +// both degenerate forms are refused up front with an actionable message. +// Returns nil when scopeValue is empty (flag not passed) or the usage is valid. +func scopeUsageError(cmd *cobra.Command, scopeValue string) error { + if scopeValue == "" { + return nil + } + // Check the -n conflict first: when the user explicitly passed -n, that is + // the more specific mistake, and reporting "requires -A" would just send them + // into the -n error on the retry. + if f := cmd.Flag("namespace"); f != nil && f.Changed { + return fmt.Errorf("--%s cannot be combined with -n/--namespace; use -A/--all-namespaces to list across namespaces", scopeFlagName) + } + + // Check the VALUE, not flag.Changed: an explicit --all-namespaces=false + // marks the flag as changed while leaving the request namespaced, which is + // exactly the degenerate case this guard exists to refuse. + if f := cmd.Flag("all-namespaces"); f == nil || f.Value.String() != "true" { + return fmt.Errorf("--%s requires -A/--all-namespaces", scopeFlagName) + } + + return nil +} + +// scopeForbiddenHint returns the hint to print right after msg, or "" when +// msg is not the cluster-scope Forbidden error or the invocation could not +// have used --scope anyway (eligible is false). +// +// eligible and resource are captured at PersistentPreRunE time by +// NewKubectlCommand and describe the current invocation: eligible means "a +// `get` with a true -A and no --scope" -- one --scope could have helped; +// resource is whatever resource argument the user actually typed (e.g. +// "pods", "deployments", "pods,svc"), or "" when it could not be determined, +// in which case the examples fall back to a placeholder rather +// than guessing. +// +// The wording is conditional ("If this is a namespaced resource") on +// purpose: a 403 for a cluster-scoped resource (e.g. `get nodes -A` -- +// kubectl accepts and ignores -A there) matches the same substrings, but +// --scope cannot help it -- the server filters namespaces and cluster-scoped +// resources live in none. The resource's scope cannot be determined from the +// message text, and resolving it via discovery inside a fatal handler would +// trade a misleading sentence for a possible hang, so the text hedges +// instead. +func scopeForbiddenHint(msg string, eligible bool, resource string) string { + if !eligible { + return "" + } + + if !strings.Contains(msg, "is forbidden:") || !strings.Contains(msg, "at the cluster scope") { + return "" + } + + if resource == "" { + resource = "" + } + + return fmt.Sprintf(` +You don't have permission to list this resource at the cluster scope. If +this is a namespaced resource, the Deckhouse platform can return just the +subset you CAN access -- opt in with the --scope flag: + + d8 k get %[1]s -A --scope=accessible # every namespace your RBAC grants + d8 k get %[1]s -A --scope=projects # all namespaces of your projects + d8 k get %[1]s -A --scope=project: # a single project's namespaces + d8 k get %[1]s -A --scope=system # non-project (system) namespaces + +(For cluster-scoped resources, e.g. nodes, --scope does not apply.) +`, resource) +} + +// scopeHeaderRoundTripper injects the two scope wire headers into every +// outgoing request. Wraps (rather than replaces) whatever RoundTripper it's +// given so it composes with exec-credential auth plugins and any other +// transport wrapping already in the chain -- see wrapConfigWithScope. +type scopeHeaderRoundTripper struct { + rt http.RoundTripper + kind string + project string +} + +func (t *scopeHeaderRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.Header.Set(scopeHeaderName, t.kind) + + if t.project != "" { + req.Header.Set(scopeProjectHeaderName, t.project) + } + + return t.rt.RoundTrip(req) +} + +// wrapConfigWithScope returns a copy of c whose transport injects the scope +// headers for (kind, project) into every request. Composes with any +// pre-existing WrapTransport (e.g. from exec-credential auth) by nesting our +// RoundTripper OUTSIDE it: headers are set first, then the request is handed +// down to whatever auth/transport layers were already configured -- they add +// their own headers (e.g. Authorization) and pass the request through +// unaffected by our unrelated X-Deckhouse-* ones. +func wrapConfigWithScope(c *rest.Config, kind, project string) *rest.Config { + cfg := rest.CopyConfig(c) + previous := cfg.WrapTransport + cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper { + if previous != nil { + rt = previous(rt) + } + + return &scopeHeaderRoundTripper{rt: rt, kind: kind, project: project} + } + + return cfg +} + +// scopeStaticValues are the non-project --scope enum values. +var scopeStaticValues = []string{"accessible", "projects", "system"} + +// scopeCompletionTimeout bounds every network round-trip shell completion may +// make (discovery and the Project LIST alike). Completion runs on a tab +// press: past a few seconds the user has long stopped waiting. +const scopeCompletionTimeout = 3 * time.Second + +// scopeCompletion provides shell completion for --scope. It offers the static +// enum values, and -- once the user has typed the `project:` prefix -- a +// `project:` candidate for each real project. Completion is +// best-effort: any failure to enumerate projects yields no dynamic candidates +// rather than an error (the user can always type the name). +// +// The cluster is contacted ONLY when toComplete already starts with +// "project:". For anything shorter (a bare , "pro", ...) a literal +// `project:` candidate is offered instead: the static values need no cluster +// at all, and a slow or unreachable cluster must not freeze the shell for a +// user who pressed tab just to see what the flag accepts. Completing the +// literal then naturally leads to a second tab press with the full prefix, +// and only that one fetches real names. +// +// configFlags is the SAME instance the kubectl command tree parses its +// connection flags into, so completion talks to whatever cluster the actual +// command would: --server, --token, --as, --cluster, --request-timeout and +// friends are all honored, not just --kubeconfig/--context. +func scopeCompletion(configFlags *genericclioptions.ConfigFlags, toComplete string) ([]string, cobra.ShellCompDirective) { + var out []string + + for _, v := range scopeStaticValues { + if strings.HasPrefix(v, toComplete) { + out = append(out, v) + } + } + + if strings.HasPrefix(toComplete, scopeProjectValuePrefix) { + for _, name := range fetchProjectNames(configFlags) { + if candidate := scopeProjectValuePrefix + name; strings.HasPrefix(candidate, toComplete) { + out = append(out, candidate) + } + } + } else if strings.HasPrefix(scopeProjectValuePrefix, toComplete) { + // toComplete is a (possibly empty) prefix of "project:" -- the user + // may be typing toward the project form. Offer the literal without + // touching the cluster. + out = append(out, scopeProjectValuePrefix) + } + + return out, cobra.ShellCompDirectiveNoFileComp +} + +// fetchProjectNames lists Deckhouse Project names for completion. The Project +// resource's served version is bumped over time, so the version is resolved +// dynamically via a discovery RESTMapper (the group "deckhouse.io" and Kind +// "Project" are the stable parts) instead of being hardcoded -- a hardcoded +// version would silently stop completing after an upgrade. +// +// Every request -- discovery included -- runs on a client whose Timeout is +// capped at scopeCompletionTimeout, so completion cannot hang the shell on an +// unreachable cluster. An explicit shorter --request-timeout from the user is +// respected (it arrives via configFlags.ToRESTConfig). +func fetchProjectNames(configFlags *genericclioptions.ConfigFlags) []string { + restConfig, err := configFlags.ToRESTConfig() + if err != nil { + return nil + } + + cfg := rest.CopyConfig(restConfig) + if cfg.Timeout == 0 || cfg.Timeout > scopeCompletionTimeout { + cfg.Timeout = scopeCompletionTimeout + } + + // Build the RESTMapper from the SAME time-bounded config: ToRESTMapper's + // discovery would otherwise run outside any deadline and could block far + // longer than the LIST below. + disc, err := discovery.NewDiscoveryClientForConfig(cfg) + if err != nil { + return nil + } + + mapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(disc)) + + mapping, err := mapper.RESTMapping(schema.GroupKind{Group: "deckhouse.io", Kind: "Project"}) + if err != nil { + return nil // Project CRD absent or discovery failed -- no dynamic candidates + } + + dyn, err := dynamic.NewForConfig(cfg) + if err != nil { + return nil + } + + ctx, cancel := context.WithTimeout(context.Background(), scopeCompletionTimeout) + defer cancel() + + list, err := dyn.Resource(mapping.Resource).List(ctx, v1.ListOptions{}) + if err != nil { + return nil // no rights / unreachable -- best-effort, stay silent + } + + names := make([]string, 0, len(list.Items)) + for i := range list.Items { + names = append(names, list.Items[i].GetName()) + } + + return names +} + var d8CommandRegex = regexp.MustCompile("([\"'`])d8 (\\w+)") +// rewriteD8Commands rewrites kubectl's quoted "d8 " references to +// "d8 k ". kubectl builds such messages two ways: from os.Args[0] +// ("d8 get ..."), which needs the insertion, and from cmd.CommandPath() +// ("d8 k get ..."), which is already correct -- inserting there would produce +// "d8 k k get". RE2 has no lookahead, so the skip is done in a replace +// callback: a match whose subcommand is already `k` is left untouched. Every +// rewrite path (the IOStreams writer, wrapRunE, the fatal handler) must go +// through this helper so they can't drift apart. +func rewriteD8Commands(s string) string { + return d8CommandRegex.ReplaceAllStringFunc(s, func(m string) string { + sub := d8CommandRegex.FindStringSubmatch(m) + if sub[2] == "k" { + return m + } + + return sub[1] + "d8 k " + sub[2] + }) +} + // d8KubectlWriter wraps an io.Writer and rewrites kubectl's "d8 " -// references (matched by d8CommandRegex) to "d8 k " on each Write call. +// references to "d8 k " on each Write call (see rewriteD8Commands). // // kubectl uses os.Args[0] as the displayed command name in user-facing // suggestions (e.g. "You can run `d8 replace -f ...` to try this update again."). @@ -64,7 +368,7 @@ func newD8KubectlWriter(w io.Writer) *d8KubectlWriter { } func (d *d8KubectlWriter) Write(p []byte) (int, error) { - rewritten := d8CommandRegex.ReplaceAllString(string(p), "${1}d8 k ${2}") + rewritten := rewriteD8Commands(string(p)) if _, err := d.w.Write([]byte(rewritten)); err != nil { return 0, err } @@ -108,7 +412,7 @@ func wrapRunE(cmd *cobra.Command) { } // Modify the output to fix the command suggestion - modifiedOutput := d8CommandRegex.ReplaceAllString(string(output), "${1}d8 k ${2}") + modifiedOutput := rewriteD8Commands(string(output)) // Write the modified output to real stderr fmt.Fprint(oldStderr, modifiedOutput) @@ -176,33 +480,120 @@ func NewKubectlCommand() *cobra.Command { ErrOut: newD8KubectlWriter(os.Stderr), } + // scopeKind/scopeProject are populated in PersistentPreRunE below, once + // cobra has parsed --scope from argv -- WithWrapConfigFn's closure reads + // them lazily at ToRESTConfig() time, which every subcommand only reaches + // after PersistentPreRunE has already run. + var scopeKind, scopeProject string + + // scopeHintEligible/scopeHintResource capture, at PersistentPreRunE time, + // whether the current invocation is one --scope could have helped (a `get` + // with a true -A and no --scope) and which resource it asked for. Locals + // deliberately: both users -- PersistentPreRunE and the BehaviorOnFatal + // closure -- are defined in this function, so command trees built by + // separate NewKubectlCommand calls cannot leak hint state into each other. + var ( + scopeHintEligible bool + scopeHintResource string + ) + + // configFlags is shared between the kubectl command tree (cobra parses all + // connection flags straight into it) and the --scope shell completion, + // which therefore talks to the same cluster the actual command would. + configFlags := genericclioptions.NewConfigFlags(true). + WithDeprecatedPasswordFlag(). + WithDiscoveryBurst(300). + WithDiscoveryQPS(50.0). + WithWarningPrinter(ioStreams). + WithWrapConfigFn(func(c *rest.Config) *rest.Config { + if scopeKind == "" { + return c + } + + return wrapConfigWithScope(c, scopeKind, scopeProject) + }) + kubectlCmd := kubecmd.NewDefaultKubectlCommandWithArgs(kubecmd.KubectlOptions{ PluginHandler: kubecmd.NewDefaultPluginHandler(plugin.ValidPluginFilenamePrefixes), Arguments: os.Args, - ConfigFlags: genericclioptions.NewConfigFlags(true). - WithDeprecatedPasswordFlag(). - WithDiscoveryBurst(300). - WithDiscoveryQPS(50.0). - WithWarningPrinter(ioStreams), - IOStreams: ioStreams, + ConfigFlags: configFlags, + IOStreams: ioStreams, }) kubectlCmd.Use = "k" kubectlCmd.Aliases = []string{"kubectl"} kubectlCmd = ReplaceCommandName("kubectl", "d8 k", kubectlCmd) + // scopeFlagValue is bound to `--scope` on the `get` command only (registered + // in the command loop below), NOT as a persistent flag on the `k` root. The + // server only ACL-filters list/get/watch of top-level resources, which in + // kubectl terms is `get` (and `get -w`). On other subcommands the header is + // either ignored (top hits metrics.k8s.io; delete/patch are not bypassed; + // logs/exec are subresources) or actively misleading (describe's internal + // LIST would be silently filtered), so `--scope` must not be offered there. + var scopeFlagValue string + // Fallback rewrite for kubectl code paths that write to the global // os.Stderr directly instead of using IOStreams.ErrOut. wrapRunE(kubectlCmd) - var debugCmd *cobra.Command + // kubectl surfaces API errors through cmdutil.CheckErr -> fatal, which + // prints to os.Stderr and calls os.Exit directly -- bypassing both the + // IOStreams writer and wrapRunE above. Override it to (a) run the same + // "d8 " -> "d8 k " rewrite on fatal messages, and (b) + // append the --scope hint when a `get -A` without --scope dies with the + // cluster-scope Forbidden error (see scopeForbiddenHint). Otherwise this + // replicates the default handler byte-for-byte, including the klog.V(99) + // debugging path (`d8 k -v=99` panics through klog with stack + // traces, the stock way to find where a fatal error originates). + cmdutil.BehaviorOnFatal(func(msg string, code int) { + if klog.V(99).Enabled() { + klog.FatalDepth(2, msg) + } + + if len(msg) > 0 { + if !strings.HasSuffix(msg, "\n") { + msg += "\n" + } + + msg = rewriteD8Commands(msg) + fmt.Fprint(os.Stderr, msg) + + if hint := scopeForbiddenHint(msg, scopeHintEligible, scopeHintResource); hint != "" { + fmt.Fprint(os.Stderr, hint) + } + } + + os.Exit(code) + }) + + var ( + debugCmd *cobra.Command + getCmd *cobra.Command + ) for _, cmd := range kubectlCmd.Commands() { - if cmd.Name() == "debug" { + switch cmd.Name() { + case "debug": debugCmd = cmd - break + case "get": + getCmd = cmd } } + if getCmd != nil { + getCmd.Flags().StringVar(&scopeFlagValue, scopeFlagName, "", + "Cross-namespace listing scope for users without cluster-wide list/watch RBAC: "+ + "accessible (everything RBAC grants you), projects, system, or project:. "+ + "Requires -A/--all-namespaces and Deckhouse platform support on the server side.") + // Shell completion for --scope values (enum + project:). The + // closure hands scopeCompletion the shared configFlags so project + // names come from the same cluster the command itself would query. + _ = getCmd.RegisterFlagCompletionFunc(scopeFlagName, + func(_ *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return scopeCompletion(configFlags, toComplete) + }) + } + if debugCmd != nil { if imageFlag := debugCmd.Flags().Lookup("image"); imageFlag != nil { imageFlag.Usage = "Container image to use for debug container. If not specified, the platform's recommended image will be used." @@ -227,6 +618,40 @@ func NewKubectlCommand() *cobra.Command { // the process immediately. signal.Reset(syscall.SIGINT, syscall.SIGTERM) + // Reset the hint state on every invocation so a command tree reused + // in-process (tests, embedding) cannot carry a previous run's arming + // into an unrelated fatal message. + scopeHintEligible, scopeHintResource = false, "" + + if scopeFlagValue != "" { + // Validate the VALUE first: `--scope=bogus` without -A must be + // reported as an invalid value, not as "requires -A" -- the + // latter would just send the user into a second error after + // they add -A. + kind, project, err := parseScopeFlag(scopeFlagValue) + if err != nil { + return err + } + + if err := scopeUsageError(cmd, scopeFlagValue); err != nil { + return err + } + + scopeKind, scopeProject = kind, project + } else if cmd.Name() == "get" { + // Arm the cluster-scope Forbidden hint: this invocation could have + // used --scope but didn't (see scopeForbiddenHint). Check the flag + // VALUE, not flag.Changed: --all-namespaces=false keeps the request + // namespaced, so --scope would not have helped. + if f := cmd.Flag("all-namespaces"); f != nil && f.Value.String() == "true" { + scopeHintEligible = true + + if len(args) > 0 { + scopeHintResource = args[0] + } + } + } + if cmd.Name() == "debug" || (cmd.Parent() != nil && cmd.Parent().Name() == "debug") { imageFlag := cmd.Flags().Lookup("image") if imageFlag != nil && imageFlag.Value.String() == "" { diff --git a/cmd/commands/kubectl_scope_test.go b/cmd/commands/kubectl_scope_test.go new file mode 100644 index 00000000..fee01917 --- /dev/null +++ b/cmd/commands/kubectl_scope_test.go @@ -0,0 +1,385 @@ +/* +Copyright 2026 Flant JSC + +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 commands + +import ( + "net/http" + "strings" + "testing" + + "github.com/spf13/cobra" + "k8s.io/client-go/rest" +) + +func TestParseScopeFlag(t *testing.T) { + tests := []struct { + name string + raw string + wantKind string + wantProject string + wantErr bool + }{ + {name: "not passed at all", raw: "", wantKind: "", wantProject: ""}, + {name: "accessible", raw: "accessible", wantKind: "accessible"}, + {name: "projects", raw: "projects", wantKind: "projects"}, + {name: "system", raw: "system", wantKind: "system"}, + {name: "project with name", raw: "project:myproject", wantKind: "project", wantProject: "myproject"}, + {name: "project without name is an error", raw: "project:", wantErr: true}, + {name: "unknown value is an error", raw: "bogus", wantErr: true}, + // "user" was the pre-review name of the "projects" scope; it must be a + // clean error now, not a silently-working alias. + {name: "renamed-away user value is an error", raw: "user", wantErr: true}, + // The following two are valid values for the OTHER, unrelated --scope + // flag on `d8 iam access grant/revoke` (cluster|all-namespaces|labels=K=V). + // Pasting one here must be a clean "invalid value" error, never a silent + // (mis)parse -- see scopeFlagName's doc. + {name: "iam's cluster value must be rejected here", raw: "cluster", wantErr: true}, + {name: "iam's all-namespaces value must be rejected here", raw: "all-namespaces", wantErr: true}, + {name: "iam's labels=K=V value must be rejected here", raw: "labels=env=prod", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kind, project, err := parseScopeFlag(tt.raw) + if tt.wantErr { + if err == nil { + t.Fatalf("parseScopeFlag(%q): expected error, got nil (kind=%q, project=%q)", tt.raw, kind, project) + } + return + } + if err != nil { + t.Fatalf("parseScopeFlag(%q): unexpected error: %v", tt.raw, err) + } + if kind != tt.wantKind || project != tt.wantProject { + t.Fatalf("parseScopeFlag(%q) = (%q, %q), want (%q, %q)", tt.raw, kind, project, tt.wantKind, tt.wantProject) + } + }) + } +} + +// TestScopeUsageError covers the guard that keeps --scope from being used in a +// context where it cannot work: it needs the cluster-scoped request URL that +// -A/--all-namespaces produces, and an explicit -n/--namespace defeats it. The +// -n conflict is reported in preference to the missing -A, since that is the +// more specific mistake. +func TestScopeUsageError(t *testing.T) { + // mkCmd builds a command carrying the same two flags scopeUsageError reads. + // allNsVal is the explicit command-line value of --all-namespaces: "" means + // the flag was not passed at all, "true"/"false" mean it was passed with + // that value (both mark the flag as "changed" -- which is exactly the trap + // the guard must not fall into). + mkCmd := func(hasAllNs bool, allNsVal string, nsSet bool) *cobra.Command { + cmd := &cobra.Command{Use: "get"} + if hasAllNs { + cmd.Flags().BoolP("all-namespaces", "A", false, "") + } + cmd.Flags().StringP("namespace", "n", "", "") + if allNsVal != "" { + _ = cmd.Flags().Set("all-namespaces", allNsVal) + } + if nsSet { + _ = cmd.Flags().Set("namespace", "demo") + } + return cmd + } + + tests := []struct { + name string + scope string + hasAllNs bool + allNsVal string + nsSet bool + wantErr string // substring; "" means expect no error + }{ + {name: "empty scope is always fine", scope: "", hasAllNs: true}, + {name: "empty scope fine even with -n", scope: "", hasAllNs: true, nsSet: true}, + {name: "scope with -A and no -n is valid", scope: "system", hasAllNs: true, allNsVal: "true"}, + {name: "scope without -A errors", scope: "system", hasAllNs: true, wantErr: "requires -A"}, + {name: "scope with explicit --all-namespaces=false is treated as missing -A", scope: "system", hasAllNs: true, allNsVal: "false", wantErr: "requires -A"}, + {name: "scope with -n errors even without -A", scope: "system", hasAllNs: true, nsSet: true, wantErr: "cannot be combined with -n"}, + {name: "scope with both -A and -n errors on -n (more specific)", scope: "system", hasAllNs: true, allNsVal: "true", nsSet: true, wantErr: "cannot be combined with -n"}, + {name: "command lacking an -A flag errors as requires -A", scope: "system", hasAllNs: false, wantErr: "requires -A"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := scopeUsageError(mkCmd(tt.hasAllNs, tt.allNsVal, tt.nsSet), tt.scope) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("scopeUsageError: unexpected error: %v", err) + } + return + } + if err == nil { + t.Fatalf("scopeUsageError: expected error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("scopeUsageError error = %q, want it to contain %q", err.Error(), tt.wantErr) + } + }) + } +} + +// TestScopeForbiddenHint covers the hint appended to kubectl's cluster-scope +// Forbidden error when the invocation was a `get -A` without --scope. +func TestScopeForbiddenHint(t *testing.T) { + const forbiddenMsg = `Error from server (Forbidden): deployments.apps is forbidden: User "u" cannot list resource "deployments" in API group "apps" at the cluster scope` + + t.Run("not eligible yields no hint", func(t *testing.T) { + if got := scopeForbiddenHint(forbiddenMsg, false, "deployments"); got != "" { + t.Fatalf("expected no hint when not eligible, got %q", got) + } + }) + + t.Run("eligible but unrelated error yields no hint", func(t *testing.T) { + for _, msg := range []string{ + "Error from server (NotFound): the server could not find the requested resource", + `Error from server (Forbidden): pods is forbidden: User "u" cannot list resource "pods" in the namespace "default"`, // namespaced 403, not cluster scope + "unable to connect to the server: dial tcp: lookup nope: no such host", + } { + if got := scopeForbiddenHint(msg, true, "deployments"); got != "" { + t.Fatalf("expected no hint for %q, got %q", msg, got) + } + } + }) + + t.Run("eligible cluster-scope forbidden yields hint with the actual resource", func(t *testing.T) { + got := scopeForbiddenHint(forbiddenMsg, true, "deployments") + if got == "" { + t.Fatal("expected a hint, got none") + } + for _, want := range []string{ + "--scope=accessible", + "--scope=projects", + "--scope=project:", + "--scope=system", + "d8 k get deployments -A", + // A cluster-scoped resource (e.g. nodes) produces the same 403 + // shape, and --scope cannot help it; the wording must stay + // conditional so the hint is not an outright lie there. + "If\nthis is a namespaced resource", + } { + if !strings.Contains(got, want) { + t.Fatalf("hint missing %q:\n%s", want, got) + } + } + }) + + t.Run("unknown resource falls back to a placeholder, never a guess", func(t *testing.T) { + got := scopeForbiddenHint(forbiddenMsg, true, "") + if !strings.Contains(got, "d8 k get -A") { + t.Fatalf("expected placeholder, got:\n%s", got) + } + if strings.Contains(got, "get pods") { + t.Fatalf("hint must not guess a resource name:\n%s", got) + } + }) +} + +// TestRewriteD8Commands pins the two message shapes the rewrite must +// distinguish: kubectl builds suggestions either from os.Args[0] ("d8 get"), +// which needs `k` inserted, or from cmd.CommandPath() ("d8 k get"), which is +// already correct -- a naive regex replace turned the latter into +// "d8 k k get" (caught in review). +func TestRewriteD8Commands(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + { + name: "os.Args[0]-built suggestion gets k inserted", + in: "You can run `d8 replace -f ...` to try this update again.", + want: "You can run `d8 k replace -f ...` to try this update again.", + }, + { + name: "CommandPath-built suggestion stays untouched", + in: "See 'd8 k get -h' for help and examples", + want: "See 'd8 k get -h' for help and examples", + }, + { + name: "double-quoted CommandPath form stays untouched", + in: `Use "d8 k explain " for a detailed description`, + want: `Use "d8 k explain " for a detailed description`, + }, + { + name: "mixed message rewrites only the bare form", + in: "See 'd8 get -h'. Also see 'd8 k get -h'.", + want: "See 'd8 k get -h'. Also see 'd8 k get -h'.", + }, + { + name: "unquoted d8 references are not touched at all", + in: "d8 get pods is not quoted here", + want: "d8 get pods is not quoted here", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := rewriteD8Commands(tt.in); got != tt.want { + t.Fatalf("rewriteD8Commands(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +// TestScopeCompletionOfflinePaths pins the no-cluster contract of --scope +// completion: anything short of a full "project:" prefix must be served +// without any network round-trip -- a tab press on a slow or unreachable +// cluster must never freeze the shell just to show static enum values. +// Passing nil configFlags proves it: any code path that reached for the +// cluster would panic on the nil receiver. +func TestScopeCompletionOfflinePaths(t *testing.T) { + tests := []struct { + name string + toComplete string + want []string + }{ + {name: "bare tab lists statics plus the project: literal", toComplete: "", want: []string{"accessible", "projects", "system", "project:"}}, + {name: "prefix of a static value", toComplete: "sys", want: []string{"system"}}, + {name: "prefix on the project: path offers the literal, no fetch", toComplete: "pro", want: []string{"projects", "project:"}}, + {name: "full static value", toComplete: "accessible", want: []string{"accessible"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, directive := scopeCompletion(nil, tt.toComplete) + if directive != cobra.ShellCompDirectiveNoFileComp { + t.Fatalf("directive = %v, want ShellCompDirectiveNoFileComp", directive) + } + if len(got) != len(tt.want) { + t.Fatalf("candidates = %v, want %v", got, tt.want) + } + for i := range tt.want { + if got[i] != tt.want[i] { + t.Fatalf("candidates = %v, want %v", got, tt.want) + } + } + }) + } +} + +// recordingRoundTripper captures the last request's headers without making +// any real network call. +type recordingRoundTripper struct { + lastHeaders http.Header + calls int +} + +func (r *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + r.calls++ + r.lastHeaders = req.Header.Clone() + return &http.Response{StatusCode: http.StatusOK, Body: http.NoBody, Header: make(http.Header)}, nil +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) } + +func TestWrapConfigWithScope_InjectsBothHeadersForProject(t *testing.T) { + rec := &recordingRoundTripper{} + wrapped := wrapConfigWithScope(&rest.Config{}, "project", "myproject") + + req, err := http.NewRequest(http.MethodGet, "http://example.invalid/api/v1/pods", nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + if _, err := wrapped.WrapTransport(rec).RoundTrip(req); err != nil { + t.Fatalf("RoundTrip: %v", err) + } + + if got := rec.lastHeaders.Get(scopeHeaderName); got != "project" { + t.Fatalf("%s header = %q, want %q", scopeHeaderName, got, "project") + } + if got := rec.lastHeaders.Get(scopeProjectHeaderName); got != "myproject" { + t.Fatalf("%s header = %q, want %q", scopeProjectHeaderName, got, "myproject") + } + // The original *http.Request must never be mutated -- RoundTrippers that + // share a request across retries/redirects would otherwise leak our + // headers into unrelated calls. + if got := req.Header.Get(scopeHeaderName); got != "" { + t.Fatalf("original request was mutated in place; got %s=%q, want no header set", scopeHeaderName, got) + } +} + +func TestWrapConfigWithScope_NoProjectHeaderForNonProjectKind(t *testing.T) { + rec := &recordingRoundTripper{} + wrapped := wrapConfigWithScope(&rest.Config{}, "accessible", "") + + req, _ := http.NewRequest(http.MethodGet, "http://example.invalid/api/v1/pods", nil) + if _, err := wrapped.WrapTransport(rec).RoundTrip(req); err != nil { + t.Fatalf("RoundTrip: %v", err) + } + + if got := rec.lastHeaders.Get(scopeHeaderName); got != "accessible" { + t.Fatalf("%s header = %q, want %q", scopeHeaderName, got, "accessible") + } + if got := rec.lastHeaders.Get(scopeProjectHeaderName); got != "" { + t.Fatalf("%s header should be absent, got %q", scopeProjectHeaderName, got) + } +} + +// TestWrapConfigWithScope_ComposesWithExistingWrapTransport is the direct +// regression test for the plan's "verify interaction with an exec-credential +// auth plugin" requirement: exec-credential auth is implemented via the same +// rest.Config.WrapTransport mechanism we use, so a naive implementation could +// silently clobber it instead of composing. +func TestWrapConfigWithScope_ComposesWithExistingWrapTransport(t *testing.T) { + rec := &recordingRoundTripper{} + authInjected := false + + cfg := &rest.Config{ + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { + return roundTripFunc(func(req *http.Request) (*http.Response, error) { + authInjected = true + req = req.Clone(req.Context()) + req.Header.Set("Authorization", "Bearer exec-credential-token") + return rt.RoundTrip(req) + }) + }, + } + + wrapped := wrapConfigWithScope(cfg, "system", "") + req, _ := http.NewRequest(http.MethodGet, "http://example.invalid/api/v1/pods", nil) + if _, err := wrapped.WrapTransport(rec).RoundTrip(req); err != nil { + t.Fatalf("RoundTrip: %v", err) + } + + if !authInjected { + t.Fatal("pre-existing WrapTransport (exec-credential simulation) was never invoked -- our wrapping clobbered it instead of composing") + } + if got := rec.lastHeaders.Get(scopeHeaderName); got != "system" { + t.Fatalf("%s header = %q, want %q", scopeHeaderName, got, "system") + } + if got := rec.lastHeaders.Get("Authorization"); got != "Bearer exec-credential-token" { + t.Fatalf("Authorization header = %q, want the exec-credential one to survive composition", got) + } +} + +// TestWrapConfigWithScope_OriginalConfigUntouched guards against a +// copy-paste bug where the wrapper mutates the caller's *rest.Config in +// place instead of operating on rest.CopyConfig's result -- ConfigFlags' +// WithWrapConfigFn is called once per client built from the same base +// config, so mutating the original would leak scope headers into unrelated +// clients built later without --scope. +func TestWrapConfigWithScope_OriginalConfigUntouched(t *testing.T) { + cfg := &rest.Config{} + _ = wrapConfigWithScope(cfg, "accessible", "") + if cfg.WrapTransport != nil { + t.Fatal("wrapConfigWithScope mutated the caller's *rest.Config in place; it must operate on a copy") + } +}