kubectl proxy: append context host path to request path
Signed-off-by: fabiankramm <fab.kramm@googlemail.com>
This commit is contained in:
		| @@ -60,6 +60,8 @@ type UpgradeAwareHandler struct { | ||||
| 	// Location is the location of the upstream proxy. It is used as the location to Dial on the upstream server | ||||
| 	// for upgrade requests unless UseRequestLocationOnUpgrade is true. | ||||
| 	Location *url.URL | ||||
| 	// AppendLocationPath determines if the original path of the Location should be appended to the upstream proxy request path | ||||
| 	AppendLocationPath bool | ||||
| 	// Transport provides an optional round tripper to use to proxy. If nil, the default proxy transport is used | ||||
| 	Transport http.RoundTripper | ||||
| 	// UpgradeTransport, if specified, will be used as the backend transport when upgrade requests are provided. | ||||
| @@ -239,7 +241,13 @@ func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request | ||||
| 		newReq.Host = h.Location.Host | ||||
| 	} | ||||
|  | ||||
| 	proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: h.Location.Scheme, Host: h.Location.Host}) | ||||
| 	// create the target location to use for the reverse proxy | ||||
| 	reverseProxyLocation := &url.URL{Scheme: h.Location.Scheme, Host: h.Location.Host} | ||||
| 	if h.AppendLocationPath { | ||||
| 		reverseProxyLocation.Path = h.Location.Path | ||||
| 	} | ||||
|  | ||||
| 	proxy := httputil.NewSingleHostReverseProxy(reverseProxyLocation) | ||||
| 	proxy.Transport = h.Transport | ||||
| 	proxy.FlushInterval = h.FlushInterval | ||||
| 	proxy.ErrorLog = log.New(noSuppressPanicError{}, "", log.LstdFlags) | ||||
| @@ -282,6 +290,9 @@ func (h *UpgradeAwareHandler) tryUpgrade(w http.ResponseWriter, req *http.Reques | ||||
| 		location = *req.URL | ||||
| 		location.Scheme = h.Location.Scheme | ||||
| 		location.Host = h.Location.Host | ||||
| 		if h.AppendLocationPath { | ||||
| 			location.Path = singleJoiningSlash(h.Location.Path, location.Path) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	clone := utilnet.CloneRequest(req) | ||||
| @@ -414,6 +425,20 @@ func (h *UpgradeAwareHandler) tryUpgrade(w http.ResponseWriter, req *http.Reques | ||||
| 	return true | ||||
| } | ||||
|  | ||||
| // FIXME: Taken from net/http/httputil/reverseproxy.go as singleJoiningSlash is not exported to be re-used. | ||||
| // See-also: https://github.com/golang/go/issues/44290 | ||||
| func singleJoiningSlash(a, b string) string { | ||||
| 	aslash := strings.HasSuffix(a, "/") | ||||
| 	bslash := strings.HasPrefix(b, "/") | ||||
| 	switch { | ||||
| 	case aslash && bslash: | ||||
| 		return a + b[1:] | ||||
| 	case !aslash && !bslash: | ||||
| 		return a + "/" + b | ||||
| 	} | ||||
| 	return a + b | ||||
| } | ||||
|  | ||||
| func (h *UpgradeAwareHandler) DialForUpgrade(req *http.Request) (net.Conn, error) { | ||||
| 	if h.UpgradeTransport == nil { | ||||
| 		return dial(req, h.Transport) | ||||
|   | ||||
| @@ -163,6 +163,7 @@ func TestServeHTTP(t *testing.T) { | ||||
| 		expectedRespHeader    map[string]string | ||||
| 		notExpectedRespHeader []string | ||||
| 		upgradeRequired       bool | ||||
| 		appendLocationPath    bool | ||||
| 		expectError           func(err error) bool | ||||
| 		useLocationHost       bool | ||||
| 	}{ | ||||
| @@ -246,6 +247,27 @@ func TestServeHTTP(t *testing.T) { | ||||
| 			expectedPath:    "/some/path", | ||||
| 			useLocationHost: true, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:               "append server path to request path", | ||||
| 			method:             "GET", | ||||
| 			requestPath:        "/base", | ||||
| 			expectedPath:       "/base/base", | ||||
| 			appendLocationPath: true, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:               "append server path to request path with ending slash", | ||||
| 			method:             "GET", | ||||
| 			requestPath:        "/base/", | ||||
| 			expectedPath:       "/base/base/", | ||||
| 			appendLocationPath: true, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:               "don't append server path to request path", | ||||
| 			method:             "GET", | ||||
| 			requestPath:        "/base", | ||||
| 			expectedPath:       "/base", | ||||
| 			appendLocationPath: false, | ||||
| 		}, | ||||
| 	} | ||||
|  | ||||
| 	for i, test := range tests { | ||||
| @@ -269,6 +291,7 @@ func TestServeHTTP(t *testing.T) { | ||||
| 			backendURL.Path = test.requestPath | ||||
| 			proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, test.upgradeRequired, responder) | ||||
| 			proxyHandler.UseLocationHost = test.useLocationHost | ||||
| 			proxyHandler.AppendLocationPath = test.appendLocationPath | ||||
| 			proxyServer := httptest.NewServer(proxyHandler) | ||||
| 			defer proxyServer.Close() | ||||
| 			proxyURL, _ := url.Parse(proxyServer.URL) | ||||
|   | ||||
| @@ -20,6 +20,7 @@ import ( | ||||
| 	"errors" | ||||
| 	"fmt" | ||||
| 	"net" | ||||
| 	"net/url" | ||||
| 	"os" | ||||
| 	"strings" | ||||
| 	"time" | ||||
| @@ -50,6 +51,8 @@ type ProxyOptions struct { | ||||
| 	unixSocket    string | ||||
| 	keepalive     time.Duration | ||||
|  | ||||
| 	appendServerPath bool | ||||
|  | ||||
| 	clientConfig *rest.Config | ||||
| 	filter       *proxy.FilterServer | ||||
|  | ||||
| @@ -138,6 +141,7 @@ func NewCmdProxy(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobr | ||||
| 	cmd.Flags().BoolVar(&o.disableFilter, "disable-filter", o.disableFilter, "If true, disable request filtering in the proxy. This is dangerous, and can leave you vulnerable to XSRF attacks, when used with an accessible port.") | ||||
| 	cmd.Flags().StringVarP(&o.unixSocket, "unix-socket", "u", o.unixSocket, "Unix socket on which to run the proxy.") | ||||
| 	cmd.Flags().DurationVar(&o.keepalive, "keepalive", o.keepalive, "keepalive specifies the keep-alive period for an active network connection. Set to 0 to disable keepalive.") | ||||
| 	cmd.Flags().BoolVar(&o.appendServerPath, "append-server-path", o.appendServerPath, "If true, enables automatic path appending of the kube context server path to each request.") | ||||
| 	return cmd | ||||
| } | ||||
|  | ||||
| @@ -157,6 +161,15 @@ func (o *ProxyOptions) Complete(f cmdutil.Factory) error { | ||||
| 		o.apiPrefix += "/" | ||||
| 	} | ||||
|  | ||||
| 	if o.appendServerPath == false { | ||||
| 		target, err := url.Parse(clientConfig.Host) | ||||
| 		if err != nil { | ||||
| 			return err | ||||
| 		} | ||||
| 		if target.Path != "" && target.Path != "/" { | ||||
| 			klog.Warning("Your kube context contains a server path " + target.Path + ", use --append-server-path to automatically append the path to each request") | ||||
| 		} | ||||
| 	} | ||||
| 	if o.disableFilter { | ||||
| 		if o.unixSocket == "" { | ||||
| 			klog.Warning("Request filter disabled, your proxy is vulnerable to XSRF attacks, please be cautious") | ||||
| @@ -193,7 +206,7 @@ func (o ProxyOptions) Validate() error { | ||||
|  | ||||
| // RunProxy checks given arguments and executes command | ||||
| func (o ProxyOptions) RunProxy() error { | ||||
| 	server, err := proxy.NewServer(o.staticDir, o.apiPrefix, o.staticPrefix, o.filter, o.clientConfig, o.keepalive) | ||||
| 	server, err := proxy.NewServer(o.staticDir, o.apiPrefix, o.staticPrefix, o.filter, o.clientConfig, o.keepalive, o.appendServerPath) | ||||
|  | ||||
| 	if err != nil { | ||||
| 		return err | ||||
|   | ||||
| @@ -173,8 +173,8 @@ func makeUpgradeTransport(config *rest.Config, keepalive time.Duration) (proxy.U | ||||
|  | ||||
| // NewServer creates and installs a new Server. | ||||
| // 'filter', if non-nil, protects requests to the api only. | ||||
| func NewServer(filebase string, apiProxyPrefix string, staticPrefix string, filter *FilterServer, cfg *rest.Config, keepalive time.Duration) (*Server, error) { | ||||
| 	proxyHandler, err := NewProxyHandler(apiProxyPrefix, filter, cfg, keepalive) | ||||
| func NewServer(filebase string, apiProxyPrefix string, staticPrefix string, filter *FilterServer, cfg *rest.Config, keepalive time.Duration, appendLocationPath bool) (*Server, error) { | ||||
| 	proxyHandler, err := NewProxyHandler(apiProxyPrefix, filter, cfg, keepalive, appendLocationPath) | ||||
| 	if err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| @@ -189,7 +189,7 @@ func NewServer(filebase string, apiProxyPrefix string, staticPrefix string, filt | ||||
| } | ||||
|  | ||||
| // NewProxyHandler creates an api proxy handler for the cluster | ||||
| func NewProxyHandler(apiProxyPrefix string, filter *FilterServer, cfg *rest.Config, keepalive time.Duration) (http.Handler, error) { | ||||
| func NewProxyHandler(apiProxyPrefix string, filter *FilterServer, cfg *rest.Config, keepalive time.Duration, appendLocationPath bool) (http.Handler, error) { | ||||
| 	host := cfg.Host | ||||
| 	if !strings.HasSuffix(host, "/") { | ||||
| 		host = host + "/" | ||||
| @@ -212,6 +212,7 @@ func NewProxyHandler(apiProxyPrefix string, filter *FilterServer, cfg *rest.Conf | ||||
| 	proxy.UpgradeTransport = upgradeTransport | ||||
| 	proxy.UseRequestLocation = true | ||||
| 	proxy.UseLocationHost = true | ||||
| 	proxy.AppendLocationPath = appendLocationPath | ||||
|  | ||||
| 	proxyServer := http.Handler(proxy) | ||||
| 	if filter != nil { | ||||
|   | ||||
| @@ -433,17 +433,19 @@ func TestPathHandling(t *testing.T) { | ||||
| 		prefix     string | ||||
| 		reqPath    string | ||||
| 		expectPath string | ||||
| 		appendPath bool | ||||
| 	}{ | ||||
| 		{"test1", "/api/", "/metrics", "404 page not found\n"}, | ||||
| 		{"test2", "/api/", "/api/metrics", "/api/metrics"}, | ||||
| 		{"test3", "/api/", "/api/v1/pods/", "/api/v1/pods/"}, | ||||
| 		{"test4", "/", "/metrics", "/metrics"}, | ||||
| 		{"test5", "/", "/api/v1/pods/", "/api/v1/pods/"}, | ||||
| 		{"test6", "/custom/", "/metrics", "404 page not found\n"}, | ||||
| 		{"test7", "/custom/", "/api/metrics", "404 page not found\n"}, | ||||
| 		{"test8", "/custom/", "/api/v1/pods/", "404 page not found\n"}, | ||||
| 		{"test9", "/custom/", "/custom/api/metrics", "/api/metrics"}, | ||||
| 		{"test10", "/custom/", "/custom/api/v1/pods/", "/api/v1/pods/"}, | ||||
| 		{"test1", "/api/", "/metrics", "404 page not found\n", false}, | ||||
| 		{"test2", "/api/", "/api/metrics", "/api/metrics", false}, | ||||
| 		{"test3", "/api/", "/api/v1/pods/", "/api/v1/pods/", false}, | ||||
| 		{"test4", "/", "/metrics", "/metrics", false}, | ||||
| 		{"test5", "/", "/api/v1/pods/", "/api/v1/pods/", false}, | ||||
| 		{"test6", "/custom/", "/metrics", "404 page not found\n", false}, | ||||
| 		{"test7", "/custom/", "/api/metrics", "404 page not found\n", false}, | ||||
| 		{"test8", "/custom/", "/api/v1/pods/", "404 page not found\n", false}, | ||||
| 		{"test9", "/custom/", "/custom/api/metrics", "/api/metrics", false}, | ||||
| 		{"test10", "/custom/", "/custom/api/v1/pods/", "/api/v1/pods/", false}, | ||||
| 		{"test11", "/custom/", "/custom/api/v1/services/", "/api/v1/services/", true}, | ||||
| 	} | ||||
|  | ||||
| 	cc := &rest.Config{ | ||||
| @@ -452,7 +454,7 @@ func TestPathHandling(t *testing.T) { | ||||
|  | ||||
| 	for _, tt := range table { | ||||
| 		t.Run(tt.name, func(t *testing.T) { | ||||
| 			p, err := NewServer("", tt.prefix, "/not/used/for/this/test", nil, cc, 0) | ||||
| 			p, err := NewServer("", tt.prefix, "/not/used/for/this/test", nil, cc, 0, tt.appendPath) | ||||
| 			if err != nil { | ||||
| 				t.Fatalf("%#v: %v", tt, err) | ||||
| 			} | ||||
|   | ||||
| @@ -57,7 +57,7 @@ func TestWatchClientTimeout(t *testing.T) { | ||||
| 	}) | ||||
|  | ||||
| 	t.Run("kubectl proxy", func(t *testing.T) { | ||||
| 		kubectlProxyServer, err := kubectlproxy.NewServer("", "/", "/static/", nil, &restclient.Config{Host: s.URL, Timeout: 2 * time.Second}, 0) | ||||
| 		kubectlProxyServer, err := kubectlproxy.NewServer("", "/", "/static/", nil, &restclient.Config{Host: s.URL, Timeout: 2 * time.Second}, 0, false) | ||||
| 		if err != nil { | ||||
| 			t.Fatal(err) | ||||
| 		} | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 fabiankramm
					fabiankramm