From a439bc55724f6560c14bf4e025946c0e6312629b Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Mon, 9 Nov 2020 11:50:11 -0800 Subject: [PATCH] Remove --redirect-container-streaming functionality (#95935) * Remove --redirect-container-streaming functionality * Update bazel --- cmd/kubelet/app/options/container_runtime.go | 11 ++- pkg/kubelet/config/flags.go | 13 +--- pkg/kubelet/dockershim/docker_service.go | 28 +++---- pkg/kubelet/kubelet.go | 24 +----- pkg/kubelet/kubelet_dockershim.go | 5 +- pkg/kubelet/server/BUILD | 1 - pkg/kubelet/server/auth_test.go | 6 +- pkg/kubelet/server/server.go | 61 +++++---------- pkg/kubelet/server/server_test.go | 82 ++++---------------- pkg/kubelet/server/server_websocket_test.go | 4 +- 10 files changed, 60 insertions(+), 175 deletions(-) diff --git a/cmd/kubelet/app/options/container_runtime.go b/cmd/kubelet/app/options/container_runtime.go index fae4620ecb6..b58953b88ef 100644 --- a/cmd/kubelet/app/options/container_runtime.go +++ b/cmd/kubelet/app/options/container_runtime.go @@ -45,12 +45,11 @@ func NewContainerRuntimeOptions() *config.ContainerRuntimeOptions { } return &config.ContainerRuntimeOptions{ - ContainerRuntime: kubetypes.DockerContainerRuntime, - RedirectContainerStreaming: false, - DockerEndpoint: dockerEndpoint, - DockershimRootDirectory: "/var/lib/dockershim", - PodSandboxImage: defaultPodSandboxImage, - ImagePullProgressDeadline: metav1.Duration{Duration: 1 * time.Minute}, + ContainerRuntime: kubetypes.DockerContainerRuntime, + DockerEndpoint: dockerEndpoint, + DockershimRootDirectory: "/var/lib/dockershim", + PodSandboxImage: defaultPodSandboxImage, + ImagePullProgressDeadline: metav1.Duration{Duration: 1 * time.Minute}, CNIBinDir: "/opt/cni/bin", CNIConfDir: "/etc/cni/net.d", diff --git a/pkg/kubelet/config/flags.go b/pkg/kubelet/config/flags.go index 5882b0d6275..79191255ae9 100644 --- a/pkg/kubelet/config/flags.go +++ b/pkg/kubelet/config/flags.go @@ -31,15 +31,6 @@ type ContainerRuntimeOptions struct { ContainerRuntime string // RuntimeCgroups that container runtime is expected to be isolated in. RuntimeCgroups string - // RedirectContainerStreaming enables container streaming redirect. - // When RedirectContainerStreaming is false, kubelet will proxy container streaming data - // between apiserver and container runtime. This approach is more secure, but the proxy - // introduces some overhead. - // When RedirectContainerStreaming is true, kubelet will return an http redirect to apiserver, - // and apiserver will access container runtime directly. This approach is more performant, - // but less secure because the connection between apiserver and container runtime is not - // authenticated. - RedirectContainerStreaming bool // Docker-specific options. @@ -83,8 +74,8 @@ func (s *ContainerRuntimeOptions) AddFlags(fs *pflag.FlagSet) { // General settings. fs.StringVar(&s.ContainerRuntime, "container-runtime", s.ContainerRuntime, "The container runtime to use. Possible values: 'docker', 'remote'.") fs.StringVar(&s.RuntimeCgroups, "runtime-cgroups", s.RuntimeCgroups, "Optional absolute name of cgroups to create and run the runtime in.") - fs.BoolVar(&s.RedirectContainerStreaming, "redirect-container-streaming", s.RedirectContainerStreaming, "Enables container streaming redirect. If false, kubelet will proxy container streaming data between apiserver and container runtime; if true, kubelet will return an http redirect to apiserver, and apiserver will access container runtime directly. The proxy approach is more secure, but introduces some overhead. The redirect approach is more performant, but less secure because the connection between apiserver and container runtime may not be authenticated.") - fs.MarkDeprecated("redirect-container-streaming", "Container streaming redirection will be removed from the kubelet in v1.20, and this flag will be removed in v1.22. For more details, see http://git.k8s.io/enhancements/keps/sig-node/20191205-container-streaming-requests.md") + _ = fs.Bool("redirect-container-streaming", false, "[REMOVED]") // TODO: Delete in v1.22 + fs.MarkDeprecated("redirect-container-streaming", "Container streaming redirection has been removed from the kubelet as of v1.20, and this flag will be removed in v1.22. For more details, see http://git.k8s.io/enhancements/keps/sig-node/20191205-container-streaming-requests.md") // Docker-specific settings. fs.StringVar(&s.DockershimRootDirectory, "experimental-dockershim-root-directory", s.DockershimRootDirectory, "Path to the dockershim root directory.") diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index 74a7b5d0812..48581c95614 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -192,7 +192,7 @@ func NewDockerClientFromConfig(config *ClientConfig) libdocker.Interface { // NewDockerService creates a new `DockerService` struct. // NOTE: Anything passed to DockerService should be eventually handled in another way when we switch to running the shim as a different process. func NewDockerService(config *ClientConfig, podSandboxImage string, streamingConfig *streaming.Config, pluginSettings *NetworkPluginSettings, - cgroupsName string, kubeCgroupDriver string, dockershimRootDir string, startLocalStreamingServer bool) (DockerService, error) { + cgroupsName string, kubeCgroupDriver string, dockershimRootDir string) (DockerService, error) { client := NewDockerClientFromConfig(config) @@ -211,11 +211,10 @@ func NewDockerService(config *ClientConfig, podSandboxImage string, streamingCon client: client, execHandler: &NativeExecHandler{}, }, - containerManager: cm.NewContainerManager(cgroupsName, client), - checkpointManager: checkpointManager, - startLocalStreamingServer: startLocalStreamingServer, - networkReady: make(map[string]bool), - containerCleanupInfos: make(map[string]*containerCleanupInfo), + containerManager: cm.NewContainerManager(cgroupsName, client), + checkpointManager: checkpointManager, + networkReady: make(map[string]bool), + containerCleanupInfos: make(map[string]*containerCleanupInfo), } // check docker version compatibility. @@ -307,9 +306,6 @@ type dockerService struct { // version checking for some operations. Use this cache to avoid querying // the docker daemon every time we need to do such checks. versionCache *cache.ObjectCache - // startLocalStreamingServer indicates whether dockershim should start a - // streaming server on localhost. - startLocalStreamingServer bool // containerCleanupInfos maps container IDs to the `containerCleanupInfo` structs // needed to clean up after containers have been removed. @@ -409,14 +405,12 @@ func (ds *dockerService) GetPodPortMappings(podSandboxID string) ([]*hostport.Po func (ds *dockerService) Start() error { ds.initCleanup() - // Initialize the legacy cleanup flag. - if ds.startLocalStreamingServer { - go func() { - if err := ds.streamingServer.Start(true); err != nil { - klog.Fatalf("Streaming server stopped unexpectedly: %v", err) - } - }() - } + go func() { + if err := ds.streamingServer.Start(true); err != nil { + klog.Fatalf("Streaming server stopped unexpectedly: %v", err) + } + }() + return ds.containerManager.Start() } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 5b427a68839..d6d620790c0 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -22,7 +22,6 @@ import ( "math" "net" "net/http" - "net/url" "os" "path" sysruntime "runtime" @@ -228,7 +227,6 @@ type Dependencies struct { KubeletConfigController *kubeletconfig.Controller RemoteRuntimeService internalapi.RuntimeService RemoteImageService internalapi.ImageManagerService - criHandler http.Handler dockerLegacyService legacy.DockerLegacyService // remove it after cadvisor.UsingLegacyCadvisorStats dropped. useLegacyCadvisorStats bool @@ -503,7 +501,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, daemonEndpoints: daemonEndpoints, containerManager: kubeDeps.ContainerManager, containerRuntimeName: containerRuntime, - redirectContainerStreaming: crOptions.RedirectContainerStreaming, nodeIPs: nodeIPs, nodeIPValidator: validateNodeIP, clock: clock.RealClock{}, @@ -567,7 +564,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, klet.resourceAnalyzer = serverstats.NewResourceAnalyzer(klet, kubeCfg.VolumeStatsAggPeriod.Duration) klet.dockerLegacyService = kubeDeps.dockerLegacyService - klet.criHandler = kubeDeps.criHandler klet.runtimeService = kubeDeps.RemoteRuntimeService if utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) && kubeDeps.KubeClient != nil { @@ -942,9 +938,6 @@ type Kubelet struct { // The name of the container runtime containerRuntimeName string - // redirectContainerStreaming enables container streaming redirect. - redirectContainerStreaming bool - // Container runtime. containerRuntime kubecontainer.Runtime @@ -1112,9 +1105,6 @@ type Kubelet struct { // The AppArmor validator for checking whether AppArmor is supported. appArmorValidator apparmor.Validator - // The handler serving CRI streaming calls (exec/attach/port-forward). - criHandler http.Handler - // experimentalHostUserNamespaceDefaulting sets userns=true when users request host namespaces (pid, ipc, net), // are using non-namespaced capabilities (mknod, sys_time, sys_module), the pod contains a privileged container, // or using host path volumes. @@ -2188,7 +2178,7 @@ func (kl *Kubelet) ResyncInterval() time.Duration { // ListenAndServe runs the kubelet HTTP server. func (kl *Kubelet) ListenAndServe(address net.IP, port uint, tlsOptions *server.TLSOptions, auth server.AuthInterface, enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, enableSystemLogHandler bool) { - server.ListenAndServeKubeletServer(kl, kl.resourceAnalyzer, address, port, tlsOptions, auth, enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, kl.redirectContainerStreaming, enableSystemLogHandler, kl.criHandler) + server.ListenAndServeKubeletServer(kl, kl.resourceAnalyzer, address, port, tlsOptions, auth, enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, enableSystemLogHandler) } // ListenAndServeReadOnly runs the kubelet HTTP server in read-only mode. @@ -2270,16 +2260,6 @@ func getStreamingConfig(kubeCfg *kubeletconfiginternal.KubeletConfiguration, kub SupportedRemoteCommandProtocols: streaming.DefaultConfig.SupportedRemoteCommandProtocols, SupportedPortForwardProtocols: streaming.DefaultConfig.SupportedPortForwardProtocols, } - if !crOptions.RedirectContainerStreaming { - config.Addr = net.JoinHostPort("localhost", "0") - } else { - // Use a relative redirect (no scheme or host). - config.BaseURL = &url.URL{ - Path: "/cri/", - } - if kubeDeps.TLSOptions != nil { - config.TLSConfig = kubeDeps.TLSOptions.Config - } - } + config.Addr = net.JoinHostPort("localhost", "0") return config } diff --git a/pkg/kubelet/kubelet_dockershim.go b/pkg/kubelet/kubelet_dockershim.go index 05ebebaec2e..b5de50cba3b 100644 --- a/pkg/kubelet/kubelet_dockershim.go +++ b/pkg/kubelet/kubelet_dockershim.go @@ -52,13 +52,10 @@ func runDockershim(kubeCfg *kubeletconfiginternal.KubeletConfiguration, ImagePullProgressDeadline: kubeDeps.DockerOptions.ImagePullProgressDeadline, } ds, err := dockershim.NewDockerService(dockerClientConfig, crOptions.PodSandboxImage, streamingConfig, - &pluginSettings, runtimeCgroups, kubeCfg.CgroupDriver, crOptions.DockershimRootDirectory, !crOptions.RedirectContainerStreaming) + &pluginSettings, runtimeCgroups, kubeCfg.CgroupDriver, crOptions.DockershimRootDirectory) if err != nil { return err } - if crOptions.RedirectContainerStreaming { - kubeDeps.criHandler = ds - } // The unix socket for kubelet <-> dockershim communication, dockershim start before runtime service init. klog.V(5).Infof("RemoteRuntimeEndpoint: %q, RemoteImageEndpoint: %q", diff --git a/pkg/kubelet/server/BUILD b/pkg/kubelet/server/BUILD index 4f7b5857c86..0b91da87faa 100644 --- a/pkg/kubelet/server/BUILD +++ b/pkg/kubelet/server/BUILD @@ -89,7 +89,6 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", "//staging/src/k8s.io/client-go/tools/remotecommand:go_default_library", - "//staging/src/k8s.io/client-go/util/testing:go_default_library", "//staging/src/k8s.io/cri-api/pkg/apis/runtime/v1alpha2:go_default_library", "//staging/src/k8s.io/kubelet/pkg/apis/stats/v1alpha1:go_default_library", "//vendor/github.com/google/cadvisor/info/v1:go_default_library", diff --git a/pkg/kubelet/server/auth_test.go b/pkg/kubelet/server/auth_test.go index 76a86165008..ad2c2c61540 100644 --- a/pkg/kubelet/server/auth_test.go +++ b/pkg/kubelet/server/auth_test.go @@ -112,10 +112,8 @@ func AuthzTestCases() []AuthzTestCase { "/attach/{podNamespace}/{podID}/{uid}/{containerName}": "proxy", "/configz": "proxy", "/containerLogs/{podNamespace}/{podID}/{containerName}": "proxy", - "/cri/": "proxy", - "/cri/foo": "proxy", - "/debug/flags/v": "proxy", - "/debug/pprof/{subpath:*}": "proxy", + "/debug/flags/v": "proxy", + "/debug/pprof/{subpath:*}": "proxy", "/exec/{podNamespace}/{podID}/{containerName}": "proxy", "/exec/{podNamespace}/{podID}/{uid}/{containerName}": "proxy", "/healthz": "proxy", diff --git a/pkg/kubelet/server/server.go b/pkg/kubelet/server/server.go index 689a76f2ee7..6a682f35d1d 100644 --- a/pkg/kubelet/server/server.go +++ b/pkg/kubelet/server/server.go @@ -41,7 +41,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/metrics/collectors" "k8s.io/utils/clock" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -89,13 +89,12 @@ const ( // Server is a http.Handler which exposes kubelet functionality over HTTP. type Server struct { - auth AuthInterface - host HostInterface - restfulCont containerInterface - metricsBuckets sets.String - metricsMethodBuckets sets.String - resourceAnalyzer stats.ResourceAnalyzer - redirectContainerStreaming bool + auth AuthInterface + host HostInterface + restfulCont containerInterface + metricsBuckets sets.String + metricsMethodBuckets sets.String + resourceAnalyzer stats.ResourceAnalyzer } // TLSOptions holds the TLS options. @@ -144,11 +143,9 @@ func ListenAndServeKubeletServer( enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, - redirectContainerStreaming, - enableSystemLogHandler bool, - criHandler http.Handler) { + enableSystemLogHandler bool) { klog.Infof("Starting to listen on %s:%d", address, port) - handler := NewServer(host, resourceAnalyzer, auth, enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, redirectContainerStreaming, enableSystemLogHandler, criHandler) + handler := NewServer(host, resourceAnalyzer, auth, enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, enableSystemLogHandler) s := &http.Server{ Addr: net.JoinHostPort(address.String(), strconv.FormatUint(uint64(port), 10)), Handler: &handler, @@ -170,7 +167,7 @@ func ListenAndServeKubeletServer( // ListenAndServeKubeletReadOnlyServer initializes a server to respond to HTTP network requests on the Kubelet. func ListenAndServeKubeletReadOnlyServer(host HostInterface, resourceAnalyzer stats.ResourceAnalyzer, address net.IP, port uint, enableCAdvisorJSONEndpoints bool) { klog.V(1).Infof("Starting to listen read-only on %s:%d", address, port) - s := NewServer(host, resourceAnalyzer, nil, enableCAdvisorJSONEndpoints, false, false, false, false, nil) + s := NewServer(host, resourceAnalyzer, nil, enableCAdvisorJSONEndpoints, false, false, false) server := &http.Server{ Addr: net.JoinHostPort(address.String(), strconv.FormatUint(uint64(port), 10)), @@ -224,24 +221,21 @@ func NewServer( enableCAdvisorJSONEndpoints, enableDebuggingHandlers, enableContentionProfiling, - redirectContainerStreaming, - enableSystemLogHandler bool, - criHandler http.Handler) Server { + enableSystemLogHandler bool) Server { server := Server{ - host: host, - resourceAnalyzer: resourceAnalyzer, - auth: auth, - restfulCont: &filteringContainer{Container: restful.NewContainer()}, - metricsBuckets: sets.NewString(), - metricsMethodBuckets: sets.NewString("OPTIONS", "GET", "HEAD", "POST", "PUT", "DELETE", "TRACE", "CONNECT"), - redirectContainerStreaming: redirectContainerStreaming, + host: host, + resourceAnalyzer: resourceAnalyzer, + auth: auth, + restfulCont: &filteringContainer{Container: restful.NewContainer()}, + metricsBuckets: sets.NewString(), + metricsMethodBuckets: sets.NewString("OPTIONS", "GET", "HEAD", "POST", "PUT", "DELETE", "TRACE", "CONNECT"), } if auth != nil { server.InstallAuthFilter() } server.InstallDefaultHandlers(enableCAdvisorJSONEndpoints) if enableDebuggingHandlers { - server.InstallDebuggingHandlers(criHandler) + server.InstallDebuggingHandlers() // To maintain backward compatibility serve logs only when enableDebuggingHandlers is also enabled // see https://github.com/kubernetes/kubernetes/pull/87273 server.InstallSystemLogHandler(enableSystemLogHandler) @@ -409,7 +403,7 @@ func (s *Server) InstallDefaultHandlers(enableCAdvisorJSONEndpoints bool) { const pprofBasePath = "/debug/pprof/" // InstallDebuggingHandlers registers the HTTP request patterns that serve logs or run commands/containers -func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) { +func (s *Server) InstallDebuggingHandlers() { klog.Infof("Adding debug handlers to kubelet server.") s.addMetricsBucketMatcher("run") @@ -527,11 +521,6 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) { To(s.getRunningPods). Operation("getRunningPods")) s.restfulCont.Add(ws) - - s.addMetricsBucketMatcher("cri") - if criHandler != nil { - s.restfulCont.Handle("/cri/", criHandler) - } } // InstallDebuggingDisabledHandlers registers the HTTP request patterns that provide better error message @@ -785,10 +774,6 @@ func (s *Server) getAttach(request *restful.Request, response *restful.Response) return } - if s.redirectContainerStreaming { - http.Redirect(response.ResponseWriter, request.Request, url.String(), http.StatusFound) - return - } proxyStream(response.ResponseWriter, request.Request, url) } @@ -813,10 +798,6 @@ func (s *Server) getExec(request *restful.Request, response *restful.Response) { streaming.WriteError(err, response.ResponseWriter) return } - if s.redirectContainerStreaming { - http.Redirect(response.ResponseWriter, request.Request, url.String(), http.StatusFound) - return - } proxyStream(response.ResponseWriter, request.Request, url) } @@ -879,10 +860,6 @@ func (s *Server) getPortForward(request *restful.Request, response *restful.Resp streaming.WriteError(err, response.ResponseWriter) return } - if s.redirectContainerStreaming { - http.Redirect(response.ResponseWriter, request.Request, url.String(), http.StatusFound) - return - } proxyStream(response.ResponseWriter, request.Request, url) } diff --git a/pkg/kubelet/server/server_test.go b/pkg/kubelet/server/server_test.go index 30958599c75..263e9c9ac8a 100644 --- a/pkg/kubelet/server/server_test.go +++ b/pkg/kubelet/server/server_test.go @@ -39,7 +39,7 @@ import ( cadvisorapiv2 "github.com/google/cadvisor/info/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/httpstream" @@ -48,7 +48,6 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/client-go/tools/remotecommand" - utiltesting "k8s.io/client-go/util/testing" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1" api "k8s.io/kubernetes/pkg/apis/core" @@ -298,19 +297,17 @@ type serverTestFramework struct { fakeKubelet *fakeKubelet fakeAuth *fakeAuth testHTTPServer *httptest.Server - criHandler *utiltesting.FakeHandler } func newServerTest() *serverTestFramework { - return newServerTestWithDebug(true, false, nil) + return newServerTestWithDebug(true, nil) } -func newServerTestWithDebug(enableDebugging, redirectContainerStreaming bool, streamingServer streaming.Server) *serverTestFramework { - return newServerTestWithDebuggingHandlers(enableDebugging, enableDebugging, redirectContainerStreaming, streamingServer) +func newServerTestWithDebug(enableDebugging bool, streamingServer streaming.Server) *serverTestFramework { + return newServerTestWithDebuggingHandlers(enableDebugging, enableDebugging, streamingServer) } -func newServerTestWithDebuggingHandlers(enableDebugging, enableSystemLogHandler, redirectContainerStreaming bool, - streamingServer streaming.Server) *serverTestFramework { +func newServerTestWithDebuggingHandlers(enableDebugging, enableSystemLogHandler bool, streamingServer streaming.Server) *serverTestFramework { fw := &serverTestFramework{} fw.fakeKubelet = &fakeKubelet{ hostnameFunc: func() string { @@ -339,9 +336,6 @@ func newServerTestWithDebuggingHandlers(enableDebugging, enableSystemLogHandler, return authorizer.DecisionAllow, "", nil }, } - fw.criHandler = &utiltesting.FakeHandler{ - StatusCode: http.StatusOK, - } server := NewServer( fw.fakeKubelet, stats.NewResourceAnalyzer(fw.fakeKubelet, time.Minute), @@ -349,9 +343,7 @@ func newServerTestWithDebuggingHandlers(enableDebugging, enableSystemLogHandler, true, enableDebugging, false, - redirectContainerStreaming, - enableSystemLogHandler, - fw.criHandler) + enableSystemLogHandler) fw.serverUnderTest = &server fw.testHTTPServer = httptest.NewServer(fw.serverUnderTest) return fw @@ -1036,7 +1028,7 @@ func TestServeExecInContainerIdleTimeout(t *testing.T) { ss, err := newTestStreamingServer(100 * time.Millisecond) require.NoError(t, err) defer ss.testHTTPServer.Close() - fw := newServerTestWithDebug(true, false, ss) + fw := newServerTestWithDebug(true, ss) defer fw.testHTTPServer.Close() podNamespace := "other" @@ -1077,7 +1069,6 @@ func testExecAttach(t *testing.T, verb string) { tty bool responseStatusCode int uid bool - redirect bool }{ "no input or output": {responseStatusCode: http.StatusBadRequest}, "stdin": {stdin: true, responseStatusCode: http.StatusSwitchingProtocols}, @@ -1086,7 +1077,6 @@ func testExecAttach(t *testing.T, verb string) { "stdout and stderr": {stdout: true, stderr: true, responseStatusCode: http.StatusSwitchingProtocols}, "stdin stdout and stderr": {stdin: true, stdout: true, stderr: true, responseStatusCode: http.StatusSwitchingProtocols}, "stdin stdout stderr with uid": {stdin: true, stdout: true, stderr: true, responseStatusCode: http.StatusSwitchingProtocols, uid: true}, - "stdout with redirect": {stdout: true, responseStatusCode: http.StatusFound, redirect: true}, } for desc := range tests { @@ -1095,7 +1085,7 @@ func testExecAttach(t *testing.T, verb string) { ss, err := newTestStreamingServer(0) require.NoError(t, err) defer ss.testHTTPServer.Close() - fw := newServerTestWithDebug(true, test.redirect, ss) + fw := newServerTestWithDebug(true, ss) defer fw.testHTTPServer.Close() fmt.Println(desc) @@ -1203,16 +1193,8 @@ func testExecAttach(t *testing.T, verb string) { upgradeRoundTripper httpstream.UpgradeRoundTripper c *http.Client ) - if test.redirect { - c = &http.Client{} - // Don't follow redirects, since we want to inspect the redirect response. - c.CheckRedirect = func(*http.Request, []*http.Request) error { - return http.ErrUseLastResponse - } - } else { - upgradeRoundTripper = spdy.NewRoundTripper(nil, true, true) - c = &http.Client{Transport: upgradeRoundTripper} - } + upgradeRoundTripper = spdy.NewRoundTripper(nil, true, true) + c = &http.Client{Transport: upgradeRoundTripper} resp, err = c.Do(makeReq(t, "POST", url, "v4.channel.k8s.io")) require.NoError(t, err, "POSTing") @@ -1299,7 +1281,7 @@ func TestServePortForwardIdleTimeout(t *testing.T) { ss, err := newTestStreamingServer(100 * time.Millisecond) require.NoError(t, err) defer ss.testHTTPServer.Close() - fw := newServerTestWithDebug(true, false, ss) + fw := newServerTestWithDebug(true, ss) defer fw.testHTTPServer.Close() podNamespace := "other" @@ -1335,7 +1317,6 @@ func TestServePortForward(t *testing.T) { uid bool clientData string containerData string - redirect bool shouldError bool }{ "no port": {port: "", shouldError: true}, @@ -1348,7 +1329,6 @@ func TestServePortForward(t *testing.T) { "normal port with data forward": {port: "8000", clientData: "client data", containerData: "container data", shouldError: false}, "max port": {port: "65535", shouldError: false}, "normal port with uid": {port: "8000", uid: true, shouldError: false}, - "normal port with redirect": {port: "8000", redirect: true, shouldError: false}, } podNamespace := "other" @@ -1360,7 +1340,7 @@ func TestServePortForward(t *testing.T) { ss, err := newTestStreamingServer(0) require.NoError(t, err) defer ss.testHTTPServer.Close() - fw := newServerTestWithDebug(true, test.redirect, ss) + fw := newServerTestWithDebug(true, ss) defer fw.testHTTPServer.Close() portForwardFuncDone := make(chan struct{}) @@ -1408,26 +1388,14 @@ func TestServePortForward(t *testing.T) { c *http.Client ) - if test.redirect { - c = &http.Client{} - // Don't follow redirects, since we want to inspect the redirect response. - c.CheckRedirect = func(*http.Request, []*http.Request) error { - return http.ErrUseLastResponse - } - } else { - upgradeRoundTripper = spdy.NewRoundTripper(nil, true, true) - c = &http.Client{Transport: upgradeRoundTripper} - } + upgradeRoundTripper = spdy.NewRoundTripper(nil, true, true) + c = &http.Client{Transport: upgradeRoundTripper} req := makeReq(t, "POST", url, "portforward.k8s.io") resp, err := c.Do(req) require.NoError(t, err, "POSTing") defer resp.Body.Close() - if test.redirect { - assert.Equal(t, http.StatusFound, resp.StatusCode, "status code") - return - } assert.Equal(t, http.StatusSwitchingProtocols, resp.StatusCode, "status code") conn, err := upgradeRoundTripper.NewConnection(resp) @@ -1466,22 +1434,6 @@ func TestServePortForward(t *testing.T) { } } -func TestCRIHandler(t *testing.T) { - fw := newServerTest() - defer fw.testHTTPServer.Close() - - const ( - path = "/cri/exec/123456abcdef" - query = "cmd=echo+foo" - ) - resp, err := http.Get(fw.testHTTPServer.URL + path + "?" + query) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) - assert.Equal(t, "GET", fw.criHandler.RequestReceived.Method) - assert.Equal(t, path, fw.criHandler.RequestReceived.URL.Path) - assert.Equal(t, query, fw.criHandler.RequestReceived.URL.RawQuery) -} - func TestMetricBuckets(t *testing.T) { tests := map[string]struct { url string @@ -1492,8 +1444,6 @@ func TestMetricBuckets(t *testing.T) { "attach with uid": {url: "/attach/podNamespace/podID/uid/containerName", bucket: "attach"}, "configz": {url: "/configz", bucket: "configz"}, "containerLogs": {url: "/containerLogs/podNamespace/podID/containerName", bucket: "containerLogs"}, - "cri": {url: "/cri/", bucket: "cri"}, - "cri with sub": {url: "/cri/foo", bucket: "cri"}, "debug v flags": {url: "/debug/flags/v", bucket: "debug"}, "pprof with sub": {url: "/debug/pprof/subpath", bucket: "debug"}, "exec": {url: "/exec/podNamespace/podID/containerName", bucket: "exec"}, @@ -1556,7 +1506,7 @@ func TestMetricMethodBuckets(t *testing.T) { func TestDebuggingDisabledHandlers(t *testing.T) { // for backward compatibility even if enablesystemLogHandler is set but not enableDebuggingHandler then /logs //shouldn't be served. - fw := newServerTestWithDebuggingHandlers(false, true, false, nil) + fw := newServerTestWithDebuggingHandlers(false, true, nil) defer fw.testHTTPServer.Close() paths := []string{ @@ -1600,7 +1550,7 @@ func TestDebuggingDisabledHandlers(t *testing.T) { } func TestDisablingSystemLogHandler(t *testing.T) { - fw := newServerTestWithDebuggingHandlers(true, false, false, nil) + fw := newServerTestWithDebuggingHandlers(true, false, nil) defer fw.testHTTPServer.Close() // verify logs endpoint is disabled diff --git a/pkg/kubelet/server/server_websocket_test.go b/pkg/kubelet/server/server_websocket_test.go index 92a876423d4..e5fa010d876 100644 --- a/pkg/kubelet/server/server_websocket_test.go +++ b/pkg/kubelet/server/server_websocket_test.go @@ -66,7 +66,7 @@ func TestServeWSPortForward(t *testing.T) { ss, err := newTestStreamingServer(0) require.NoError(t, err) defer ss.testHTTPServer.Close() - fw := newServerTestWithDebug(true, false, ss) + fw := newServerTestWithDebug(true, ss) defer fw.testHTTPServer.Close() portForwardFuncDone := make(chan struct{}) @@ -158,7 +158,7 @@ func TestServeWSMultiplePortForward(t *testing.T) { ss, err := newTestStreamingServer(0) require.NoError(t, err) defer ss.testHTTPServer.Close() - fw := newServerTestWithDebug(true, false, ss) + fw := newServerTestWithDebug(true, ss) defer fw.testHTTPServer.Close() portForwardWG := sync.WaitGroup{}