From 4c6af55e7c2fef291d121aa363c828d1acdf81b7 Mon Sep 17 00:00:00 2001 From: Ethan Mosbaugh Date: Fri, 1 May 2026 13:03:42 -0700 Subject: [PATCH] fix: correct RBAC verb for pods/exec from get to create (#2037) fix: correct RBAC verb and WebSocket fallback for pods/exec This commit fixes three related issues that prevented exec collectors from working with minimal RBAC permissions: 1. RBAC preflight check used wrong verb for pods/exec Changed from "get" to "create" in v1beta1 and v1beta2 AccessReviewSpecs. The pods/exec subresource requires "create" to execute commands. 2. WebSocket fallback used wrong httpstream package import The fallback executor checked IsUpgradeFailure using the apimachinery httpstream package, but the roundtripper creates UpgradeFailureError using the streaming httpstream package. These are different Go types, so errors.As always returned false and fallback to SPDY never triggered. Changed import to k8s.io/streaming/pkg/httpstream. 3. Stdin mismatch caused SPDY fallback to hang PodExecOptions always set Stdin:true but StreamOptions always passed Stdin:nil. When WebSocket failed and fell back to SPDY, the server waited for stdin data that never arrived. Changed Stdin to false in PodExecOptions for exec, copy, and copy_from_host collectors. --- go.mod | 2 +- pkg/apis/troubleshoot/v1beta1/collector_shared.go | 4 ++-- pkg/apis/troubleshoot/v1beta2/collector_shared.go | 4 ++-- pkg/collect/copy.go | 5 ++++- pkg/collect/copy_from_host.go | 5 ++++- pkg/collect/exec.go | 5 ++++- pkg/k8sutil/exec.go | 7 +++---- 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index fe00688d..83b0573c 100644 --- a/go.mod +++ b/go.mod @@ -57,6 +57,7 @@ require ( k8s.io/cli-runtime v0.36.0 k8s.io/client-go v0.36.0 k8s.io/klog/v2 v2.140.0 + k8s.io/streaming v0.36.0 oras.land/oras-go/v2 v2.6.0 sigs.k8s.io/controller-runtime v0.23.3 sigs.k8s.io/e2e-framework v0.7.0 @@ -164,7 +165,6 @@ require ( gotest.tools/v3 v3.5.2 // indirect k8s.io/component-base v0.36.0 // indirect k8s.io/kubectl v0.36.0 // indirect - k8s.io/streaming v0.36.0 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect ) diff --git a/pkg/apis/troubleshoot/v1beta1/collector_shared.go b/pkg/apis/troubleshoot/v1beta1/collector_shared.go index 358fdb55..fb5d0341 100644 --- a/pkg/apis/troubleshoot/v1beta1/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta1/collector_shared.go @@ -261,7 +261,7 @@ func (c *Collect) AccessReviewSpecs(overrideNS string) []authorizationv1.SelfSub result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: pickNamespaceOrDefault(c.Exec.Namespace, overrideNS), - Verb: "get", + Verb: "create", Group: "", Version: "", Resource: "pods", @@ -286,7 +286,7 @@ func (c *Collect) AccessReviewSpecs(overrideNS string) []authorizationv1.SelfSub result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: pickNamespaceOrDefault(c.Copy.Namespace, overrideNS), - Verb: "get", + Verb: "create", Group: "", Version: "", Resource: "pods", diff --git a/pkg/apis/troubleshoot/v1beta2/collector_shared.go b/pkg/apis/troubleshoot/v1beta2/collector_shared.go index ae715408..cc567f8a 100644 --- a/pkg/apis/troubleshoot/v1beta2/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/collector_shared.go @@ -529,7 +529,7 @@ func (c *Collect) AccessReviewSpecs(overrideNS string) []authorizationv1.SelfSub result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: pickNamespaceOrDefault(c.Exec.Namespace, overrideNS), - Verb: "get", + Verb: "create", Group: "", Version: "", Resource: "pods", @@ -554,7 +554,7 @@ func (c *Collect) AccessReviewSpecs(overrideNS string) []authorizationv1.SelfSub result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: pickNamespaceOrDefault(c.Copy.Namespace, overrideNS), - Verb: "get", + Verb: "create", Group: "", Version: "", Resource: "pods", diff --git a/pkg/collect/copy.go b/pkg/collect/copy.go index df69fc1c..d7eb1859 100644 --- a/pkg/collect/copy.go +++ b/pkg/collect/copy.go @@ -98,11 +98,14 @@ func copyFilesFromPod(ctx context.Context, dstPath string, clientConfig *restcli return nil, nil, errors.Wrap(err, "failed to add runtime scheme") } + // Stdin must be false because StreamOptions.Stdin is nil below. + // A mismatch causes the SPDY fallback (after WebSocket fails on RBAC) + // to hang: the API server opens a stdin stream but never receives EOF. parameterCodec := runtime.NewParameterCodec(scheme) req.VersionedParams(&corev1.PodExecOptions{ Command: command, Container: containerName, - Stdin: true, + Stdin: false, Stdout: true, Stderr: true, TTY: false, diff --git a/pkg/collect/copy_from_host.go b/pkg/collect/copy_from_host.go index 1d1e87ec..5970c528 100644 --- a/pkg/collect/copy_from_host.go +++ b/pkg/collect/copy_from_host.go @@ -299,11 +299,14 @@ func copyFilesFromHost(ctx context.Context, dstPath string, clientConfig *restcl return nil, nil, errors.Wrap(err, "failed to add runtime scheme") } + // Stdin must be false because StreamOptions.Stdin is nil below. + // A mismatch causes the SPDY fallback (after WebSocket fails on RBAC) + // to hang: the API server opens a stdin stream but never receives EOF. parameterCodec := runtime.NewParameterCodec(scheme) req.VersionedParams(&corev1.PodExecOptions{ Command: command, Container: containerName, - Stdin: true, + Stdin: false, Stdout: true, Stderr: true, TTY: false, diff --git a/pkg/collect/exec.go b/pkg/collect/exec.go index d5a2cdc4..d4df56e4 100644 --- a/pkg/collect/exec.go +++ b/pkg/collect/exec.go @@ -129,10 +129,13 @@ func getExecOutputs( } parameterCodec := runtime.NewParameterCodec(scheme) + // Stdin must be false because StreamOptions.Stdin is nil below. + // A mismatch causes the SPDY fallback (after WebSocket fails on RBAC) + // to hang: the API server opens a stdin stream but never receives EOF. req.VersionedParams(&corev1.PodExecOptions{ Command: append(execCollector.Command, execCollector.Args...), Container: container, - Stdin: true, + Stdin: false, Stdout: true, Stderr: true, TTY: false, diff --git a/pkg/k8sutil/exec.go b/pkg/k8sutil/exec.go index 79344a87..c26aeee7 100644 --- a/pkg/k8sutil/exec.go +++ b/pkg/k8sutil/exec.go @@ -3,9 +3,9 @@ package k8sutil import ( "net/url" - "k8s.io/apimachinery/pkg/util/httpstream" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/remotecommand" + "k8s.io/streaming/pkg/httpstream" ) // NewFallbackExecutor creates an executor that tries WebSocket first and falls @@ -21,8 +21,7 @@ func NewFallbackExecutor(config *restclient.Config, u *url.URL) (remotecommand.E if err != nil { return nil, err } - shouldFallback := func(err error) bool { + return remotecommand.NewFallbackExecutor(wsExec, spdyExec, func(err error) bool { return httpstream.IsUpgradeFailure(err) || httpstream.IsHTTPSProxyError(err) - } - return remotecommand.NewFallbackExecutor(wsExec, spdyExec, shouldFallback) + }) }