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.

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

Revert "Allow specifying scheme when proxying" #15220

Merged
merged 1 commit into from
Oct 7, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 13 additions & 18 deletions pkg/registry/generic/rest/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,19 @@ import (
type UpgradeAwareProxyHandler struct {
UpgradeRequired bool
Location *url.URL
// Transport provides an optional round tripper to use to proxy. If nil, the default proxy transport is used
Transport http.RoundTripper
// WrapTransport indicates whether the provided Transport should be wrapped with default proxy transport behavior (URL rewriting, X-Forwarded-* header setting)
WrapTransport bool
FlushInterval time.Duration
MaxBytesPerSec int64
err error
Transport http.RoundTripper
FlushInterval time.Duration
MaxBytesPerSec int64
err error
}

const defaultFlushInterval = 200 * time.Millisecond

// NewUpgradeAwareProxyHandler creates a new proxy handler with a default flush interval
func NewUpgradeAwareProxyHandler(location *url.URL, transport http.RoundTripper, wrapTransport, upgradeRequired bool) *UpgradeAwareProxyHandler {
func NewUpgradeAwareProxyHandler(location *url.URL, transport http.RoundTripper, upgradeRequired bool) *UpgradeAwareProxyHandler {
return &UpgradeAwareProxyHandler{
Location: location,
Transport: transport,
WrapTransport: wrapTransport,
UpgradeRequired: upgradeRequired,
FlushInterval: defaultFlushInterval,
}
Expand Down Expand Up @@ -105,8 +101,8 @@ func (h *UpgradeAwareProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Re
return
}

if h.Transport == nil || h.WrapTransport {
h.Transport = h.defaultProxyTransport(req.URL, h.Transport)
if h.Transport == nil {
h.Transport = h.defaultProxyTransport(req.URL)
}

newReq, err := http.NewRequest(req.Method, loc.String(), req.Body)
Expand Down Expand Up @@ -262,22 +258,21 @@ func (h *UpgradeAwareProxyHandler) dialURL() (net.Conn, error) {
}
}

func (h *UpgradeAwareProxyHandler) defaultProxyTransport(url *url.URL, internalTransport http.RoundTripper) http.RoundTripper {
func (h *UpgradeAwareProxyHandler) defaultProxyTransport(url *url.URL) http.RoundTripper {
scheme := url.Scheme
host := url.Host
suffix := h.Location.Path
if strings.HasSuffix(url.Path, "/") && !strings.HasSuffix(suffix, "/") {
suffix += "/"
}
pathPrepend := strings.TrimSuffix(url.Path, suffix)
rewritingTransport := &proxy.Transport{
Scheme: scheme,
Host: host,
PathPrepend: pathPrepend,
RoundTripper: internalTransport,
internalTransport := &proxy.Transport{
Scheme: scheme,
Host: host,
PathPrepend: pathPrepend,
}
return &corsRemovingTransport{
RoundTripper: rewritingTransport,
RoundTripper: internalTransport,
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/generic/rest/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func TestDefaultProxyTransport(t *testing.T) {
h := UpgradeAwareProxyHandler{
Location: locURL,
}
result := h.defaultProxyTransport(URL, nil)
result := h.defaultProxyTransport(URL)
transport := result.(*corsRemovingTransport).RoundTripper.(*proxy.Transport)
if transport.Scheme != test.expectedScheme {
t.Errorf("%s: unexpected scheme. Actual: %s, Expected: %s", test.name, transport.Scheme, test.expectedScheme)
Expand Down
5 changes: 2 additions & 3 deletions pkg/registry/node/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func MatchNode(label labels.Selector, field fields.Selector) generic.Matcher {

// ResourceLocation returns an URL and transport which one can use to send traffic for the specified node.
func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGetter, ctx api.Context, id string) (*url.URL, http.RoundTripper, error) {
schemeReq, name, portReq, valid := util.SplitSchemeNamePort(id)
name, portReq, valid := util.SplitPort(id)
if !valid {
return nil, nil, errors.NewBadRequest(fmt.Sprintf("invalid node request %q", id))
}
Expand All @@ -154,7 +154,6 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet
host := hostIP.String()

if portReq == "" || strconv.Itoa(ports.KubeletPort) == portReq {
// Ignore requested scheme, use scheme provided by GetConnectionInfo
scheme, port, transport, err := connection.GetConnectionInfo(host)
if err != nil {
return nil, nil, err
Expand All @@ -169,5 +168,5 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet
transport,
nil
}
return &url.URL{Scheme: schemeReq, Host: net.JoinHostPort(host, portReq)}, nil, nil
return &url.URL{Host: net.JoinHostPort(host, portReq)}, nil, nil
}
15 changes: 7 additions & 8 deletions pkg/registry/pod/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,12 @@ func (r *ProxyREST) Connect(ctx api.Context, id string, opts runtime.Object) (re
if !ok {
return nil, fmt.Errorf("Invalid options object: %#v", opts)
}
location, transport, err := pod.ResourceLocation(r.store, ctx, id)
location, _, err := pod.ResourceLocation(r.store, ctx, id)
if err != nil {
return nil, err
}
location.Path = path.Join(location.Path, proxyOpts.Path)
// Return a proxy handler that uses the desired transport, wrapped with additional proxy handling (to get URL rewriting, X-Forwarded-* headers, etc)
return newThrottledUpgradeAwareProxyHandler(location, transport, true, false), nil
return newUpgradeAwareProxyHandler(location, nil, false), nil
}

// Support both GET and POST methods. Over time, we want to move all clients to start using POST and then stop supporting GET.
Expand Down Expand Up @@ -322,7 +321,7 @@ func (r *AttachREST) Connect(ctx api.Context, name string, opts runtime.Object)
if err != nil {
return nil, err
}
return newThrottledUpgradeAwareProxyHandler(location, transport, false, true), nil
return genericrest.NewUpgradeAwareProxyHandler(location, transport, true), nil
}

// NewConnectOptions returns the versioned object that represents exec parameters
Expand Down Expand Up @@ -360,7 +359,7 @@ func (r *ExecREST) Connect(ctx api.Context, name string, opts runtime.Object) (r
if err != nil {
return nil, err
}
return newThrottledUpgradeAwareProxyHandler(location, transport, false, true), nil
return newUpgradeAwareProxyHandler(location, transport, true), nil
}

// NewConnectOptions returns the versioned object that represents exec parameters
Expand Down Expand Up @@ -404,11 +403,11 @@ func (r *PortForwardREST) Connect(ctx api.Context, name string, opts runtime.Obj
if err != nil {
return nil, err
}
return newThrottledUpgradeAwareProxyHandler(location, transport, false, true), nil
return newUpgradeAwareProxyHandler(location, transport, true), nil
}

func newThrottledUpgradeAwareProxyHandler(location *url.URL, transport http.RoundTripper, wrapTransport, upgradeRequired bool) *genericrest.UpgradeAwareProxyHandler {
handler := genericrest.NewUpgradeAwareProxyHandler(location, transport, wrapTransport, upgradeRequired)
func newUpgradeAwareProxyHandler(location *url.URL, transport http.RoundTripper, upgradeRequired bool) *genericrest.UpgradeAwareProxyHandler {
handler := genericrest.NewUpgradeAwareProxyHandler(location, transport, upgradeRequired)
handler.MaxBytesPerSec = capabilities.Get().PerConnectionBandwidthLimitBytesPerSec
return handler
}
22 changes: 7 additions & 15 deletions pkg/registry/pod/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package pod

import (
"crypto/tls"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -47,13 +46,6 @@ type podStrategy struct {
// objects via the REST API.
var Strategy = podStrategy{api.Scheme, api.SimpleNameGenerator}

// PodProxyTransport is used by the API proxy to connect to pods
// Exported to allow overriding TLS options (like adding a client certificate)
var PodProxyTransport = util.SetTransportDefaults(&http.Transport{
// Turn off hostname verification, because connections are to assigned IPs, not deterministic
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
})

// NamespaceScoped is true for pods.
func (podStrategy) NamespaceScoped() bool {
return true
Expand Down Expand Up @@ -196,9 +188,9 @@ func getPod(getter ResourceGetter, ctx api.Context, name string) (*api.Pod, erro

// ResourceLocation returns a URL to which one can send traffic for the specified pod.
func ResourceLocation(getter ResourceGetter, ctx api.Context, id string) (*url.URL, http.RoundTripper, error) {
// Allow ID as "podname" or "podname:port" or "scheme:podname:port".
// If port is not specified, try to use the first defined port on the pod.
scheme, name, port, valid := util.SplitSchemeNamePort(id)
// Allow ID as "podname" or "podname:port". If port is not specified,
// try to use the first defined port on the pod.
name, port, valid := util.SplitPort(id)
if !valid {
return nil, nil, errors.NewBadRequest(fmt.Sprintf("invalid pod request %q", id))
}
Expand All @@ -219,15 +211,15 @@ func ResourceLocation(getter ResourceGetter, ctx api.Context, id string) (*url.U
}
}

loc := &url.URL{
Scheme: scheme,
}
// We leave off the scheme ('http://') because we have no idea what sort of server
// is listening at this endpoint.
loc := &url.URL{}
if port == "" {
loc.Host = pod.Status.PodIP
} else {
loc.Host = net.JoinHostPort(pod.Status.PodIP, port)
}
return loc, PodProxyTransport, nil
return loc, nil, nil
}

// LogLocation returns the log URL for a pod container. If opts.Container is blank
Expand Down
19 changes: 6 additions & 13 deletions pkg/registry/service/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package service

import (
"crypto/tls"
"fmt"
"math/rand"
"net"
Expand Down Expand Up @@ -50,13 +49,6 @@ type REST struct {
serviceNodePorts portallocator.Interface
}

// ServiceProxyTransport is used by the API proxy to connect to services
// Exported to allow overriding TLS options (like adding a client certificate)
var ServiceProxyTransport = util.SetTransportDefaults(&http.Transport{
// Turn off hostname verification, because connections are to assigned IPs, not deterministic
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
})

// NewStorage returns a new REST.
func NewStorage(registry Registry, endpoints endpoint.Registry, serviceIPs ipallocator.Interface,
serviceNodePorts portallocator.Interface) *REST {
Expand Down Expand Up @@ -285,8 +277,8 @@ var _ = rest.Redirector(&REST{})

// ResourceLocation returns a URL to which one can send traffic for the specified service.
func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.RoundTripper, error) {
// Allow ID as "svcname", "svcname:port", or "scheme:svcname:port".
svcScheme, svcName, portStr, valid := util.SplitSchemeNamePort(id)
// Allow ID as "svcname" or "svcname:port".
svcName, portStr, valid := util.SplitPort(id)
if !valid {
return nil, nil, errors.NewBadRequest(fmt.Sprintf("invalid service request %q", id))
}
Expand All @@ -311,10 +303,11 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Rou
// Pick a random address.
ip := ss.Addresses[rand.Intn(len(ss.Addresses))].IP
port := ss.Ports[i].Port
// We leave off the scheme ('http://') because we have no idea what sort of server
// is listening at this endpoint.
return &url.URL{
Scheme: svcScheme,
Host: net.JoinHostPort(ip, strconv.Itoa(port)),
}, ServiceProxyTransport, nil
Host: net.JoinHostPort(ip, strconv.Itoa(port)),
}, nil, nil
}
}
}
Expand Down
12 changes: 0 additions & 12 deletions pkg/registry/service/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,18 +491,6 @@ func TestServiceRegistryResourceLocation(t *testing.T) {
t.Errorf("Expected %v, but got %v", e, a)
}

// Test a scheme + name + port.
location, _, err = redirector.ResourceLocation(ctx, "https:foo:p")
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if location == nil {
t.Errorf("Unexpected nil: %v", location)
}
if e, a := "https://1.2.3.4:93", location.String(); e != a {
t.Errorf("Expected %v, but got %v", e, a)
}

// Test a non-existent name + port.
location, _, err = redirector.ResourceLocation(ctx, "foo:q")
if err == nil {
Expand Down
59 changes: 11 additions & 48 deletions pkg/util/port_split.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,60 +18,23 @@ package util

import (
"strings"

"k8s.io/kubernetes/pkg/util/sets"
)

var validSchemes = sets.NewString("http", "https", "")

// SplitSchemeNamePort takes a string of the following forms:
// * "<name>", returns "", "<name>","", true
// * "<name>:<port>", returns "", "<name>","<port>",true
// * "<scheme>:<name>:<port>", returns "<scheme>","<name>","<port>",true
// Takes a string of the form "name:port" or "name".
// * If id is of the form "name" or "name:", then return (name, "", true)
// * If id is of the form "name:port", then return (name, port, true)
// * Otherwise, return ("", "", false)
// Additionally, name must be non-empty or valid will be returned false.
//
// Name must be non-empty or valid will be returned false.
// Scheme must be "http" or "https" if specified
// Port is returned as a string, and it is not required to be numeric (could be
// used for a named port, for example).
func SplitSchemeNamePort(id string) (scheme, name, port string, valid bool) {
func SplitPort(id string) (name, port string, valid bool) {
parts := strings.Split(id, ":")
switch len(parts) {
case 1:
name = parts[0]
case 2:
name = parts[0]
port = parts[1]
case 3:
scheme = parts[0]
name = parts[1]
port = parts[2]
default:
return "", "", "", false
}

if len(name) > 0 && validSchemes.Has(scheme) {
return scheme, name, port, true
} else {
return "", "", "", false
}
}

// JoinSchemeNamePort returns a string that specifies the scheme, name, and port:
// * "<name>"
// * "<name>:<port>"
// * "<scheme>:<name>:<port>"
// None of the parameters may contain a ':' character
// Name is required
// Scheme must be "", "http", or "https"
func JoinSchemeNamePort(scheme, name, port string) string {
if len(scheme) > 0 {
// Must include three segments to specify scheme
return scheme + ":" + name + ":" + port
if len(parts) > 2 {
return "", "", false
}
if len(port) > 0 {
// Must include two segments to specify port
return name + ":" + port
if len(parts) == 2 {
return parts[0], parts[1], len(parts[0]) > 0
}
// Return name alone
return name
return id, "", len(id) > 0
}