Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support exec/attach/portforward in kubectl proxy #49534

Merged
merged 7 commits into from
Aug 9, 2017
Merged

Support exec/attach/portforward in kubectl proxy #49534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7013047
Move proxy code to its own package
smarterclayton Jul 13, 2017
fa009f3
Ensure proxy server code is logically distinct
smarterclayton Jul 13, 2017
1df5817
Allow the UpgradeAwareProxy to have an upgrade specific transport
smarterclayton Jul 25, 2017
d2b8cdb
React to changes in UpgradeAwareProxy
smarterclayton Jul 25, 2017
0daee3a
Use the UpgradeAwareProxy in `kubectl proxy`
smarterclayton Jul 25, 2017
97a2b76
Add e2e test for kubectl exec via kubectl proxy
smarterclayton Jul 25, 2017
c42531c
Add pkg/kubectl/proxy to list of pkg/kubectl/util consumers
smarterclayton Jul 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/visible_to/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ package_group(
packages = [
"//pkg/kubectl",
"//pkg/kubectl/cmd",
"//pkg/kubectl/proxy",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty empty commit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not generated code.

],
)

Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ go_test(
"generate_test.go",
"kubectl_test.go",
"namespace_test.go",
"proxy_server_test.go",
"quota_test.go",
"resource_filter_test.go",
"rolebinding_test.go",
Expand Down Expand Up @@ -97,7 +96,6 @@ go_library(
"kubectl.go",
"namespace.go",
"pdb.go",
"proxy_server.go",
"quota.go",
"resource_filter.go",
"rolebinding.go",
Expand Down Expand Up @@ -197,6 +195,7 @@ filegroup(
"//pkg/kubectl/cmd:all-srcs",
"//pkg/kubectl/metricsutil:all-srcs",
"//pkg/kubectl/plugins:all-srcs",
"//pkg/kubectl/proxy:all-srcs",
"//pkg/kubectl/resource:all-srcs",
"//pkg/kubectl/testing:all-srcs",
"//pkg/kubectl/util:all-srcs",
Expand Down
1 change: 1 addition & 0 deletions pkg/kubectl/cmd/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ go_library(
"//pkg/kubectl/cmd/util/openapi:go_default_library",
"//pkg/kubectl/metricsutil:go_default_library",
"//pkg/kubectl/plugins:go_default_library",
"//pkg/kubectl/proxy:go_default_library",
"//pkg/kubectl/resource:go_default_library",
"//pkg/kubectl/util:go_default_library",
"//pkg/kubectl/util/term:go_default_library",
Expand Down
24 changes: 12 additions & 12 deletions pkg/kubectl/cmd/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import (

"github.com/golang/glog"
"github.com/spf13/cobra"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/proxy"
"k8s.io/kubernetes/pkg/util/i18n"
)

Expand Down Expand Up @@ -83,10 +83,10 @@ func NewCmdProxy(f cmdutil.Factory, out io.Writer) *cobra.Command {
cmd.Flags().StringP("www", "w", "", "Also serve static files from the given directory under the specified prefix.")
cmd.Flags().StringP("www-prefix", "P", "/static/", "Prefix to serve static files under, if static file directory is specified.")
cmd.Flags().StringP("api-prefix", "", "/", "Prefix to serve the proxied API under.")
cmd.Flags().String("accept-paths", kubectl.DefaultPathAcceptRE, "Regular expression for paths that the proxy should accept.")
cmd.Flags().String("reject-paths", kubectl.DefaultPathRejectRE, "Regular expression for paths that the proxy should reject. Paths specified here will be rejected even accepted by --accept-paths.")
cmd.Flags().String("accept-hosts", kubectl.DefaultHostAcceptRE, "Regular expression for hosts that the proxy should accept.")
cmd.Flags().String("reject-methods", kubectl.DefaultMethodRejectRE, "Regular expression for HTTP methods that the proxy should reject (example --reject-methods='POST,PUT,PATCH'). ")
cmd.Flags().String("accept-paths", proxy.DefaultPathAcceptRE, "Regular expression for paths that the proxy should accept.")
cmd.Flags().String("reject-paths", proxy.DefaultPathRejectRE, "Regular expression for paths that the proxy should reject. Paths specified here will be rejected even accepted by --accept-paths.")
cmd.Flags().String("accept-hosts", proxy.DefaultHostAcceptRE, "Regular expression for hosts that the proxy should accept.")
cmd.Flags().String("reject-methods", proxy.DefaultMethodRejectRE, "Regular expression for HTTP methods that the proxy should reject (example --reject-methods='POST,PUT,PATCH'). ")
cmd.Flags().IntP("port", "p", defaultPort, "The port on which to run the proxy. Set to 0 to pick a random port.")
cmd.Flags().StringP("address", "", "127.0.0.1", "The IP address on which to serve on.")
cmd.Flags().Bool("disable-filter", false, "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.")
Expand Down Expand Up @@ -126,11 +126,11 @@ func RunProxy(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error {
if !strings.HasSuffix(apiProxyPrefix, "/") {
apiProxyPrefix += "/"
}
filter := &kubectl.FilterServer{
AcceptPaths: kubectl.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "accept-paths")),
RejectPaths: kubectl.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "reject-paths")),
AcceptHosts: kubectl.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "accept-hosts")),
RejectMethods: kubectl.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "reject-methods")),
filter := &proxy.FilterServer{
AcceptPaths: proxy.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "accept-paths")),
RejectPaths: proxy.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "reject-paths")),
AcceptHosts: proxy.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "accept-hosts")),
RejectMethods: proxy.MakeRegexpArrayOrDie(cmdutil.GetFlagString(cmd, "reject-methods")),
}
if cmdutil.GetFlagBool(cmd, "disable-filter") {
if path == "" {
Expand All @@ -139,7 +139,7 @@ func RunProxy(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error {
filter = nil
}

server, err := kubectl.NewProxyServer(staticDir, apiProxyPrefix, staticPrefix, filter, clientConfig)
server, err := proxy.NewServer(staticDir, apiProxyPrefix, staticPrefix, filter, clientConfig)

// Separate listening from serving so we can report the bound port
// when it is chosen by os (eg: port == 0)
Expand All @@ -152,7 +152,7 @@ func RunProxy(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error {
if err != nil {
glog.Fatal(err)
}
fmt.Fprintf(out, "Starting to serve on %s", l.Addr().String())
fmt.Fprintf(out, "Starting to serve on %s\n", l.Addr().String())
glog.Fatal(server.ServeOnListener(l))
return nil
}
47 changes: 47 additions & 0 deletions pkg/kubectl/proxy/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package(default_visibility = ["//visibility:public"])

licenses(["notice"])

load(
"@io_bazel_rules_go//go:def.bzl",
"go_library",
"go_test",
)

go_test(
name = "go_default_test",
srcs = ["proxy_server_test.go"],
library = ":go_default_library",
tags = ["automanaged"],
deps = [
"//vendor/k8s.io/apimachinery/pkg/util/proxy:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
],
)

go_library(
name = "go_default_library",
srcs = ["proxy_server.go"],
tags = ["automanaged"],
deps = [
"//pkg/kubectl/util:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/proxy:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
"//vendor/k8s.io/client-go/transport:go_default_library",
],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
)
89 changes: 60 additions & 29 deletions pkg/kubectl/proxy_server.go → pkg/kubectl/proxy/proxy_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,39 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package kubectl
package proxy

import (
"fmt"
"net"
"net/http"
"net/http/httputil"
"net/url"
"os"
"regexp"
"strings"
"time"

"github.com/golang/glog"
restclient "k8s.io/client-go/rest"
utilnet "k8s.io/apimachinery/pkg/util/net"
"k8s.io/apimachinery/pkg/util/proxy"
"k8s.io/client-go/rest"
"k8s.io/client-go/transport"
"k8s.io/kubernetes/pkg/kubectl/util"
)

const (
DefaultHostAcceptRE = "^localhost$,^127\\.0\\.0\\.1$,^\\[::1\\]$"
DefaultPathAcceptRE = "^.*"
DefaultPathRejectRE = "^/api/.*/pods/.*/exec,^/api/.*/pods/.*/attach"
// DefaultHostAcceptRE is the default value for which hosts to accept.
DefaultHostAcceptRE = "^localhost$,^127\\.0\\.0\\.1$,^\\[::1\\]$"
// DefaultPathAcceptRE is the default path to accept.
DefaultPathAcceptRE = "^.*"
// DefaultPathRejectRE is the default set of paths to reject.
DefaultPathRejectRE = "^/api/.*/pods/.*/exec,^/api/.*/pods/.*/attach"
// DefaultMethodRejectRE is the set of HTTP methods to reject by default.
DefaultMethodRejectRE = "^$"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

funny to encode a string set in a regex.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this was existing code.

)

var (
// The reverse proxy will periodically flush the io writer at this frequency.
// ReverseProxyFlushInterval is the frequency to flush the reverse proxy.
// Only matters for long poll connections like the one used to watch. With an
// interval of 0 the reverse proxy will buffer content sent on any connection
// with transfer-encoding=chunked.
Expand All @@ -63,7 +69,7 @@ type FilterServer struct {
delegate http.Handler
}

// Splits a comma separated list of regexps into an array of Regexp objects.
// MakeRegexpArray splits a comma separated list of regexps into an array of Regexp objects.
func MakeRegexpArray(str string) ([]*regexp.Regexp, error) {
parts := strings.Split(str, ",")
result := make([]*regexp.Regexp, len(parts))
Expand All @@ -77,6 +83,7 @@ func MakeRegexpArray(str string) ([]*regexp.Regexp, error) {
return result, nil
}

// MakeRegexpArrayOrDie creates an array of regular expression objects from a string or exits.
func MakeRegexpArrayOrDie(str string) []*regexp.Regexp {
result, err := MakeRegexpArray(str)
if err != nil {
Expand Down Expand Up @@ -137,15 +144,38 @@ func (f *FilterServer) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
rw.Write([]byte("<h3>Unauthorized</h3>"))
}

// ProxyServer is a http.Handler which proxies Kubernetes APIs to remote API server.
type ProxyServer struct {
// Server is a http.Handler which proxies Kubernetes APIs to remote API server.
type Server struct {
handler http.Handler
}

// NewProxyServer creates and installs a new ProxyServer.
// It automatically registers the created ProxyServer to http.DefaultServeMux.
type responder struct{}

func (r *responder) Error(w http.ResponseWriter, req *http.Request, err error) {
glog.Errorf("Error while proxying request: %v", err)
http.Error(w, err.Error(), http.StatusInternalServerError)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least here the Error signature makes sense. Wondering whether the other legacy responder impl can be switch to using w as well instead of storing another writer in the body.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's almost two different call patterns, but I think the other one can switch. I just initially stopped at refactoring that because I'm not sure how good the tests are and didn't want to complicate this. Easy to be a follow up.

}

// makeUpgradeTransport creates a transport that explicitly bypasses HTTP2 support
// for proxy connections that must upgrade.
func makeUpgradeTransport(config *rest.Config) (http.RoundTripper, error) {
transportConfig, err := config.TransportConfig()
if err != nil {
return nil, err
}
tlsConfig, err := transport.TLSConfigFor(transportConfig)
if err != nil {
return nil, err
}
rt := utilnet.SetOldTransportDefaults(&http.Transport{
TLSClientConfig: tlsConfig,
})
return transport.HTTPWrappersForConfig(transportConfig, rt)
}

// NewServer creates and installs a new Server.
// 'filter', if non-nil, protects requests to the api only.
func NewProxyServer(filebase string, apiProxyPrefix string, staticPrefix string, filter *FilterServer, cfg *restclient.Config) (*ProxyServer, error) {
func NewServer(filebase string, apiProxyPrefix string, staticPrefix string, filter *FilterServer, cfg *rest.Config) (*Server, error) {
host := cfg.Host
if !strings.HasSuffix(host, "/") {
host = host + "/"
Expand All @@ -154,10 +184,20 @@ func NewProxyServer(filebase string, apiProxyPrefix string, staticPrefix string,
if err != nil {
return nil, err
}
proxy := newProxy(target)
if proxy.Transport, err = restclient.TransportFor(cfg); err != nil {

responder := &responder{}
transport, err := rest.TransportFor(cfg)
if err != nil {
return nil, err
}
upgradeTransport, err := makeUpgradeTransport(cfg)
if err != nil {
return nil, err
}
proxy := proxy.NewUpgradeAwareHandler(target, transport, false, false, responder)
proxy.UpgradeTransport = upgradeTransport
proxy.UseRequestLocation = true

proxyServer := http.Handler(proxy)
if filter != nil {
proxyServer = filter.HandlerFor(proxyServer)
Expand All @@ -174,16 +214,16 @@ func NewProxyServer(filebase string, apiProxyPrefix string, staticPrefix string,
// serving their working directory by default.
mux.Handle(staticPrefix, newFileHandler(staticPrefix, filebase))
}
return &ProxyServer{handler: mux}, nil
return &Server{handler: mux}, nil
}

// Listen is a simple wrapper around net.Listen.
func (s *ProxyServer) Listen(address string, port int) (net.Listener, error) {
func (s *Server) Listen(address string, port int) (net.Listener, error) {
return net.Listen("tcp", fmt.Sprintf("%s:%d", address, port))
}

// ListenUnix does net.Listen for a unix socket
func (s *ProxyServer) ListenUnix(path string) (net.Listener, error) {
func (s *Server) ListenUnix(path string) (net.Listener, error) {
// Remove any socket, stale or not, but fall through for other files
fi, err := os.Stat(path)
if err == nil && (fi.Mode()&os.ModeSocket) != 0 {
Expand All @@ -196,23 +236,14 @@ func (s *ProxyServer) ListenUnix(path string) (net.Listener, error) {
return l, err
}

// Serve starts the server using given listener, loops forever.
func (s *ProxyServer) ServeOnListener(l net.Listener) error {
// ServeOnListener starts the server using given listener, loops forever.
func (s *Server) ServeOnListener(l net.Listener) error {
server := http.Server{
Handler: s.handler,
}
return server.Serve(l)
}

func newProxy(target *url.URL) *httputil.ReverseProxy {
director := func(req *http.Request) {
req.URL.Scheme = target.Scheme
req.URL.Host = target.Host
req.URL.Path = singleJoiningSlash(target.Path, req.URL.Path)
}
return &httputil.ReverseProxy{Director: director, FlushInterval: ReverseProxyFlushInterval}
}

func newFileHandler(prefix, base string) http.Handler {
return http.StripPrefix(prefix, http.FileServer(http.Dir(base)))
}
Expand Down
16 changes: 12 additions & 4 deletions pkg/kubectl/proxy_server_test.go → pkg/kubectl/proxy/proxy_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package kubectl
package proxy

import (
"fmt"
Expand All @@ -27,7 +27,8 @@ import (
"strings"
"testing"

restclient "k8s.io/client-go/rest"
"k8s.io/apimachinery/pkg/util/proxy"
"k8s.io/client-go/rest"
)

func TestAccept(t *testing.T) {
Expand Down Expand Up @@ -340,6 +341,12 @@ func TestFileServing(t *testing.T) {
}
}

func newProxy(target *url.URL) http.Handler {
p := proxy.NewUpgradeAwareHandler(target, http.DefaultTransport, false, false, &responder{})
p.UseRequestLocation = true
return p
}

func TestAPIRequests(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
b, err := ioutil.ReadAll(r.Body)
Expand All @@ -353,6 +360,7 @@ func TestAPIRequests(t *testing.T) {

// httptest.NewServer should always generate a valid URL.
target, _ := url.Parse(ts.URL)
target.Path = "/"
proxy := newProxy(target)

tests := []struct{ method, body string }{
Expand Down Expand Up @@ -404,13 +412,13 @@ func TestPathHandling(t *testing.T) {
{"/custom/", "/custom/api/v1/pods/", "/api/v1/pods/"},
}

cc := &restclient.Config{
cc := &rest.Config{
Host: ts.URL,
}

for _, item := range table {
func() {
p, err := NewProxyServer("", item.prefix, "/not/used/for/this/test", nil, cc)
p, err := NewServer("", item.prefix, "/not/used/for/this/test", nil, cc)
if err != nil {
t.Fatalf("%#v: %v", item, err)
}
Expand Down
Loading