Merge pull request #97350 from FabianKramm/master

kubectl proxy: append context host path to request path
This commit is contained in:
Kubernetes Prow Robot 2021-08-20 11:19:31 -07:00 committed by GitHub
commit 341d312066
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 81 additions and 17 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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

View File

@ -176,8 +176,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
}
@ -192,7 +192,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 + "/"
@ -215,6 +215,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 {

View File

@ -455,17 +455,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{
@ -474,7 +476,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)
}

View File

@ -164,7 +164,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)
}