From 75f487f7bfbe0fa2d6ba23933b1a2e80e1939509 Mon Sep 17 00:00:00 2001 From: James DeFelice Date: Wed, 6 Jan 2016 23:59:54 +0000 Subject: [PATCH] AbsPath should be compatible with proxy-prefixes: - replace Config.Prefix with .Host and .APIPath - Request .path promoted to .pathPrefix, .baseURL holds required prefix --- .../generators/generator-for-group.go | 6 +- .../testgroup/unversioned/testgroup_client.go | 2 +- .../unversioned/extensions_client.go | 2 +- .../legacy/unversioned/legacy_client.go | 2 +- .../unversioned/clientcmd/client_config.go | 2 - .../clientcmd/client_config_test.go | 24 +++--- pkg/client/unversioned/discovery_client.go | 2 +- pkg/client/unversioned/extensions.go | 2 +- pkg/client/unversioned/fake/fake.go | 14 ++-- pkg/client/unversioned/helper.go | 82 +++++++++++-------- pkg/client/unversioned/helper_test.go | 14 ++-- .../remotecommand/remotecommand_test.go | 4 +- pkg/client/unversioned/request.go | 32 ++++---- pkg/client/unversioned/request_test.go | 64 ++++++++++----- pkg/client/unversioned/restclient.go | 18 ++-- pkg/kubectl/cmd/config/config_test.go | 6 +- test/e2e/kubectl.go | 1 - 17 files changed, 158 insertions(+), 119 deletions(-) diff --git a/cmd/libs/go2idl/client-gen/generators/generator-for-group.go b/cmd/libs/go2idl/client-gen/generators/generator-for-group.go index a425c6631a2..37c8cb65ab1 100644 --- a/cmd/libs/go2idl/client-gen/generators/generator-for-group.go +++ b/cmd/libs/go2idl/client-gen/generators/generator-for-group.go @@ -53,7 +53,7 @@ func (g *genGroup) GenerateType(c *generator.Context, t *types.Type, w io.Writer sw := generator.NewSnippetWriter(w, c, "$", "$") const pkgUnversioned = "k8s.io/kubernetes/pkg/client/unversioned" const pkgLatest = "k8s.io/kubernetes/pkg/api/latest" - prefix := func(group string) string { + apiPath := func(group string) string { if group == "legacy" { return `"/api"` } @@ -78,7 +78,7 @@ func (g *genGroup) GenerateType(c *generator.Context, t *types.Type, w io.Writer "RESTClientFor": c.Universe.Function(types.Name{Package: pkgUnversioned, Name: "RESTClientFor"}), "latestGroup": c.Universe.Variable(types.Name{Package: pkgLatest, Name: "Group"}), "GroupOrDie": c.Universe.Variable(types.Name{Package: pkgLatest, Name: "GroupOrDie"}), - "prefix": prefix(g.group), + "apiPath": apiPath(g.group), } sw.Do(groupInterfaceTemplate, m) sw.Do(groupClientTemplate, m) @@ -157,7 +157,7 @@ func setConfigDefaults(config *$.Config|raw$) error { if err != nil { return err } - config.Prefix = $.prefix$ + config.APIPath = $.apiPath$ if config.UserAgent == "" { config.UserAgent = $.DefaultKubernetesUserAgent|raw$() } diff --git a/cmd/libs/go2idl/client-gen/testoutput/testgroup/unversioned/testgroup_client.go b/cmd/libs/go2idl/client-gen/testoutput/testgroup/unversioned/testgroup_client.go index 9a2635f4cad..c98bc141c25 100644 --- a/cmd/libs/go2idl/client-gen/testoutput/testgroup/unversioned/testgroup_client.go +++ b/cmd/libs/go2idl/client-gen/testoutput/testgroup/unversioned/testgroup_client.go @@ -69,7 +69,7 @@ func setConfigDefaults(config *unversioned.Config) error { if err != nil { return err } - config.Prefix = "/apis" + config.APIPath = "/apis" if config.UserAgent == "" { config.UserAgent = unversioned.DefaultKubernetesUserAgent() } diff --git a/pkg/client/typed/generated/extensions/unversioned/extensions_client.go b/pkg/client/typed/generated/extensions/unversioned/extensions_client.go index edee23c6353..386c8ee909e 100644 --- a/pkg/client/typed/generated/extensions/unversioned/extensions_client.go +++ b/pkg/client/typed/generated/extensions/unversioned/extensions_client.go @@ -94,7 +94,7 @@ func setConfigDefaults(config *unversioned.Config) error { if err != nil { return err } - config.Prefix = "/apis" + config.APIPath = "/apis" if config.UserAgent == "" { config.UserAgent = unversioned.DefaultKubernetesUserAgent() } diff --git a/pkg/client/typed/generated/legacy/unversioned/legacy_client.go b/pkg/client/typed/generated/legacy/unversioned/legacy_client.go index 427fc436dd4..1d3180a6db5 100644 --- a/pkg/client/typed/generated/legacy/unversioned/legacy_client.go +++ b/pkg/client/typed/generated/legacy/unversioned/legacy_client.go @@ -139,7 +139,7 @@ func setConfigDefaults(config *unversioned.Config) error { if err != nil { return err } - config.Prefix = "/api" + config.APIPath = "/api" if config.UserAgent == "" { config.UserAgent = unversioned.DefaultKubernetesUserAgent() } diff --git a/pkg/client/unversioned/clientcmd/client_config.go b/pkg/client/unversioned/clientcmd/client_config.go index 9a58b9f98bd..0b3a598bce1 100644 --- a/pkg/client/unversioned/clientcmd/client_config.go +++ b/pkg/client/unversioned/clientcmd/client_config.go @@ -94,8 +94,6 @@ func (config DirectClientConfig) ClientConfig() (*client.Config, error) { clientConfig := &client.Config{} clientConfig.Host = configClusterInfo.Server if u, err := url.ParseRequestURI(clientConfig.Host); err == nil && u.Opaque == "" && len(u.Path) > 1 { - clientConfig.Prefix = u.Path - u.Path = "" u.RawQuery = "" u.Fragment = "" clientConfig.Host = u.String() diff --git a/pkg/client/unversioned/clientcmd/client_config_test.go b/pkg/client/unversioned/clientcmd/client_config_test.go index 0822d384004..83486967787 100644 --- a/pkg/client/unversioned/clientcmd/client_config_test.go +++ b/pkg/client/unversioned/clientcmd/client_config_test.go @@ -156,7 +156,7 @@ func TestCreateClean(t *testing.T) { } matchStringArg(config.Clusters["clean"].Server, clientConfig.Host, t) - matchStringArg("", clientConfig.Prefix, t) + matchStringArg("", clientConfig.APIPath, t) matchStringArg(config.Clusters["clean"].APIVersion, clientConfig.GroupVersion.String(), t) matchBoolArg(config.Clusters["clean"].InsecureSkipTLSVerify, clientConfig.Insecure, t) matchStringArg(config.AuthInfos["clean"].Token, clientConfig.BearerToken, t) @@ -166,22 +166,21 @@ func TestCreateCleanWithPrefix(t *testing.T) { tt := []struct { server string host string - prefix string }{ - {"https://anything.com:8080/foo/bar", "https://anything.com:8080", "/foo/bar"}, - {"http://anything.com:8080/foo/bar", "http://anything.com:8080", "/foo/bar"}, - {"http://anything.com:8080/foo/bar/", "http://anything.com:8080", "/foo/bar/"}, - {"http://anything.com:8080/", "http://anything.com:8080/", ""}, - {"http://anything.com:8080//", "http://anything.com:8080", "//"}, - {"anything.com:8080/foo/bar", "anything.com:8080/foo/bar", ""}, - {"anything.com:8080", "anything.com:8080", ""}, - {"anything.com", "anything.com", ""}, - {"anything", "anything", ""}, + {"https://anything.com:8080/foo/bar", "https://anything.com:8080/foo/bar"}, + {"http://anything.com:8080/foo/bar", "http://anything.com:8080/foo/bar"}, + {"http://anything.com:8080/foo/bar/", "http://anything.com:8080/foo/bar/"}, + {"http://anything.com:8080/", "http://anything.com:8080/"}, + {"http://anything.com:8080//", "http://anything.com:8080//"}, + {"anything.com:8080/foo/bar", "anything.com:8080/foo/bar"}, + {"anything.com:8080", "anything.com:8080"}, + {"anything.com", "anything.com"}, + {"anything", "anything"}, } // WARNING: EnvVarCluster.Server is set during package loading time and can not be overriden by os.Setenv inside this test EnvVarCluster.Server = "" - tt = append(tt, struct{ server, host, prefix string }{"", "http://localhost:8080", ""}) + tt = append(tt, struct{ server, host string }{"", "http://localhost:8080"}) for _, tc := range tt { config := createValidTestConfig() @@ -198,7 +197,6 @@ func TestCreateCleanWithPrefix(t *testing.T) { } matchStringArg(tc.host, clientConfig.Host, t) - matchStringArg(tc.prefix, clientConfig.Prefix, t) } } diff --git a/pkg/client/unversioned/discovery_client.go b/pkg/client/unversioned/discovery_client.go index 3287fa24f0c..9b81b742d39 100644 --- a/pkg/client/unversioned/discovery_client.go +++ b/pkg/client/unversioned/discovery_client.go @@ -139,7 +139,7 @@ func (d *DiscoveryClient) ServerResources() (map[string]*unversioned.APIResource } func setDiscoveryDefaults(config *Config) error { - config.Prefix = "" + config.APIPath = "" config.GroupVersion = nil // Discovery client deals with unversioned objects, so we use api.Codec. config.Codec = api.Codec diff --git a/pkg/client/unversioned/extensions.go b/pkg/client/unversioned/extensions.go index c0ff2fbe9c0..ddca763918a 100644 --- a/pkg/client/unversioned/extensions.go +++ b/pkg/client/unversioned/extensions.go @@ -128,7 +128,7 @@ func setExtensionsDefaults(config *Config) error { if err != nil { return err } - config.Prefix = "apis/" + config.APIPath = defaultAPIPath if config.UserAgent == "" { config.UserAgent = DefaultKubernetesUserAgent() } diff --git a/pkg/client/unversioned/fake/fake.go b/pkg/client/unversioned/fake/fake.go index f7aba822da0..d8b1bd4ccf0 100644 --- a/pkg/client/unversioned/fake/fake.go +++ b/pkg/client/unversioned/fake/fake.go @@ -50,23 +50,27 @@ type RESTClient struct { } func (c *RESTClient) Get() *unversioned.Request { - return unversioned.NewRequest(c, "GET", &url.URL{Host: "localhost"}, *testapi.Default.GroupVersion(), c.Codec, nil) + return c.request("GET") } func (c *RESTClient) Put() *unversioned.Request { - return unversioned.NewRequest(c, "PUT", &url.URL{Host: "localhost"}, *testapi.Default.GroupVersion(), c.Codec, nil) + return c.request("PUT") } func (c *RESTClient) Patch(_ api.PatchType) *unversioned.Request { - return unversioned.NewRequest(c, "PATCH", &url.URL{Host: "localhost"}, *testapi.Default.GroupVersion(), c.Codec, nil) + return c.request("PATCH") } func (c *RESTClient) Post() *unversioned.Request { - return unversioned.NewRequest(c, "POST", &url.URL{Host: "localhost"}, *testapi.Default.GroupVersion(), c.Codec, nil) + return c.request("POST") } func (c *RESTClient) Delete() *unversioned.Request { - return unversioned.NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, *testapi.Default.GroupVersion(), c.Codec, nil) + return c.request("DELETE") +} + +func (c *RESTClient) request(verb string) *unversioned.Request { + return unversioned.NewRequest(c, verb, &url.URL{Host: "localhost"}, "", *testapi.Default.GroupVersion(), c.Codec, nil) } func (c *RESTClient) Do(req *http.Request) (*http.Response, error) { diff --git a/pkg/client/unversioned/helper.go b/pkg/client/unversioned/helper.go index 5b5ef5dac3c..968b999e064 100644 --- a/pkg/client/unversioned/helper.go +++ b/pkg/client/unversioned/helper.go @@ -40,14 +40,21 @@ import ( "k8s.io/kubernetes/pkg/version" ) +const ( + legacyAPIPath = "/api" + defaultAPIPath = "/apis" +) + // Config holds the common attributes that can be passed to a Kubernetes client on // initialization. type Config struct { - // Host must be a host string, a host:port pair, or a URL to the base of the API. + // Host must be a host string, a host:port pair, or a URL to the base of the apiserver. + // If a URL is given then the (optional) Path of that URL represents a prefix that must + // be appended to all request URIs used to access the apiserver. This allows a frontend + // proxy to easily relocate all of the apiserver endpoints. Host string - // Prefix is the sub path of the server. If not specified, the client will set - // a default value. Use "/" to indicate the server root should be used - Prefix string + // APIPath is a sub-path that points to an API root. + APIPath string // GroupVersion is the API version to talk to. Must be provided when initializing // a RESTClient directly. When initializing a Client, will be set with the default // code version. @@ -180,7 +187,7 @@ func ExtractGroupVersions(l *unversioned.APIGroupList) []string { // ServerAPIVersions returns the GroupVersions supported by the API server. // It creates a RESTClient based on the passed in config, but it doesn't rely -// on the Version, Codec, and Prefix of the config, because it uses AbsPath and +// on the Version and Codec of the config, because it uses AbsPath and // takes the raw response. func ServerAPIVersions(c *Config) (groupVersions []string, err error) { transport, err := TransportFor(c) @@ -191,13 +198,14 @@ func ServerAPIVersions(c *Config) (groupVersions []string, err error) { configCopy := *c configCopy.GroupVersion = nil - configCopy.Prefix = "" - baseURL, err := defaultServerUrlFor(c) + configCopy.APIPath = "" + baseURL, _, err := defaultServerUrlFor(&configCopy) if err != nil { return nil, err } // Get the groupVersions exposed at /api - baseURL.Path = "/api" + originalPath := baseURL.Path + baseURL.Path = path.Join(originalPath, legacyAPIPath) resp, err := client.Get(baseURL.String()) if err != nil { return nil, err @@ -211,7 +219,7 @@ func ServerAPIVersions(c *Config) (groupVersions []string, err error) { groupVersions = append(groupVersions, v.Versions...) // Get the groupVersions exposed at /apis - baseURL.Path = "/apis" + baseURL.Path = path.Join(originalPath, defaultAPIPath) resp2, err := client.Get(baseURL.String()) if err != nil { return nil, err @@ -357,8 +365,8 @@ func NewInCluster() (*Client, error) { // Kubernetes API or returns an error if any of the defaults are impossible or invalid. // TODO: this method needs to be split into one that sets defaults per group, expected to be fix in PR "Refactoring clientcache.go and helper.go #14592" func SetKubernetesDefaults(config *Config) error { - if config.Prefix == "" { - config.Prefix = "/api" + if config.APIPath == "" { + config.APIPath = legacyAPIPath } if len(config.UserAgent) == 0 { config.UserAgent = DefaultKubernetesUserAgent() @@ -398,12 +406,12 @@ func RESTClientFor(config *Config) (*RESTClient, error) { return nil, fmt.Errorf("Codec is required when initializing a RESTClient") } - baseURL, err := defaultServerUrlFor(config) + baseURL, versionedAPIPath, err := defaultServerUrlFor(config) if err != nil { return nil, err } - client := NewRESTClient(baseURL, *config.GroupVersion, config.Codec, config.QPS, config.Burst) + client := NewRESTClient(baseURL, versionedAPIPath, *config.GroupVersion, config.Codec, config.QPS, config.Burst) transport, err := TransportFor(config) if err != nil { @@ -423,12 +431,12 @@ func UnversionedRESTClientFor(config *Config) (*RESTClient, error) { return nil, fmt.Errorf("Codec is required when initializing a RESTClient") } - baseURL, err := defaultServerUrlFor(config) + baseURL, versionedAPIPath, err := defaultServerUrlFor(config) if err != nil { return nil, err } - client := NewRESTClient(baseURL, unversioned.SchemeGroupVersion, config.Codec, config.QPS, config.Burst) + client := NewRESTClient(baseURL, versionedAPIPath, unversioned.SchemeGroupVersion, config.Codec, config.QPS, config.Burst) transport, err := TransportFor(config) if err != nil { @@ -444,14 +452,14 @@ func UnversionedRESTClientFor(config *Config) (*RESTClient, error) { // DefaultServerURL converts a host, host:port, or URL string to the default base server API path // to use with a Client at a given API version following the standard conventions for a // Kubernetes API. -func DefaultServerURL(host, prefix string, groupVersion unversioned.GroupVersion, defaultTLS bool) (*url.URL, error) { +func DefaultServerURL(host, apiPath string, groupVersion unversioned.GroupVersion, defaultTLS bool) (*url.URL, string, error) { if host == "" { - return nil, fmt.Errorf("host must be a URL or a host:port pair") + return nil, "", fmt.Errorf("host must be a URL or a host:port pair") } base := host hostURL, err := url.Parse(base) if err != nil { - return nil, err + return nil, "", err } if hostURL.Scheme == "" { scheme := "http://" @@ -460,32 +468,34 @@ func DefaultServerURL(host, prefix string, groupVersion unversioned.GroupVersion } hostURL, err = url.Parse(scheme + base) if err != nil { - return nil, err + return nil, "", err } if hostURL.Path != "" && hostURL.Path != "/" { - return nil, fmt.Errorf("host must be a URL or a host:port pair: %q", base) + return nil, "", fmt.Errorf("host must be a URL or a host:port pair: %q", base) } } - // If the user specified a URL without a path component (http://server.com), automatically - // append the default prefix - if hostURL.Path == "" { - if prefix == "" { - prefix = "/" - } - hostURL.Path = prefix - } + // hostURL.Path is optional; a non-empty Path is treated as a prefix that is to be applied to + // all URIs used to access the host. this is useful when there's a proxy in front of the + // apiserver that has relocated the apiserver endpoints, forwarding all requests from, for + // example, /a/b/c to the apiserver. in this case the Path should be /a/b/c. + // + // if running without a frontend proxy (that changes the location of the apiserver), then + // hostURL.Path should be blank. + // + // versionedAPIPath, a path relative to baseURL.Path, points to a versioned API base + versionedAPIPath := path.Join("/", apiPath) // Add the version to the end of the path if len(groupVersion.Group) > 0 { - hostURL.Path = path.Join(hostURL.Path, groupVersion.Group, groupVersion.Version) + versionedAPIPath = path.Join(versionedAPIPath, groupVersion.Group, groupVersion.Version) } else { - hostURL.Path = path.Join(hostURL.Path, groupVersion.Version) + versionedAPIPath = path.Join(versionedAPIPath, groupVersion.Version) } - return hostURL, nil + return hostURL, versionedAPIPath, nil } // IsConfigTransportTLS returns true if and only if the provided config will result in a protected @@ -499,7 +509,7 @@ func IsConfigTransportTLS(config Config) bool { // modify the copy of the config we got to satisfy preconditions for defaultServerUrlFor config.GroupVersion = defaultVersionFor(&config) - baseURL, err := defaultServerUrlFor(&config) + baseURL, _, err := defaultServerUrlFor(&config) if err != nil { return false } @@ -508,7 +518,7 @@ func IsConfigTransportTLS(config Config) bool { // defaultServerUrlFor is shared between IsConfigTransportTLS and RESTClientFor. It // requires Host and Version to be set prior to being called. -func defaultServerUrlFor(config *Config) (*url.URL, error) { +func defaultServerUrlFor(config *Config) (*url.URL, string, error) { // TODO: move the default to secure when the apiserver supports TLS by default // config.Insecure is taken to mean "I want HTTPS but don't bother checking the certs against a CA." hasCA := len(config.CAFile) != 0 || len(config.CAData) != 0 @@ -520,12 +530,12 @@ func defaultServerUrlFor(config *Config) (*url.URL, error) { } if config.GroupVersion != nil { - return DefaultServerURL(host, config.Prefix, *config.GroupVersion, defaultTLS) + return DefaultServerURL(host, config.APIPath, *config.GroupVersion, defaultTLS) } - return DefaultServerURL(host, config.Prefix, unversioned.GroupVersion{}, defaultTLS) + return DefaultServerURL(host, config.APIPath, unversioned.GroupVersion{}, defaultTLS) } -// defaultVersionFor is shared between defaultServerUrlFor and RESTClientFor +// defaultVersionFor is shared between IsConfigTransportTLS and RESTClientFor func defaultVersionFor(config *Config) *unversioned.GroupVersion { if config.GroupVersion == nil { // Clients default to the preferred code API version diff --git a/pkg/client/unversioned/helper_test.go b/pkg/client/unversioned/helper_test.go index f7b5f8be254..95cab924e8f 100644 --- a/pkg/client/unversioned/helper_test.go +++ b/pkg/client/unversioned/helper_test.go @@ -20,6 +20,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "path" "reflect" "strings" "testing" @@ -91,7 +92,7 @@ func TestSetKubernetesDefaults(t *testing.T) { { Config{}, Config{ - Prefix: "/api", + APIPath: "/api", GroupVersion: testapi.Default.GroupVersion(), Codec: testapi.Default.Codec(), QPS: 5, @@ -200,7 +201,7 @@ func TestSetsCodec(t *testing.T) { Prefix string Codec runtime.Codec }{ - testapi.Default.GroupVersion().Version: {false, "/api/" + testapi.Default.GroupVersion().Version + "/", testapi.Default.Codec()}, + testapi.Default.GroupVersion().Version: {false, "/api/" + testapi.Default.GroupVersion().Version, testapi.Default.Codec()}, // Add this test back when we fixed config and SetKubernetesDefaults // "invalidVersion": {true, "", nil}, } @@ -216,7 +217,7 @@ func TestSetsCodec(t *testing.T) { case err != nil: continue } - if e, a := expected.Prefix, client.RESTClient.baseURL.Path; e != a { + if e, a := expected.Prefix, client.RESTClient.versionedAPIPath; e != a { t.Errorf("expected %#v, got %#v", e, a) } if e, a := expected.Codec, client.RESTClient.Codec; e != a { @@ -239,8 +240,8 @@ func TestRESTClientRequires(t *testing.T) { func TestValidatesHostParameter(t *testing.T) { testCases := []struct { - Host string - Prefix string + Host string + APIPath string URL string Err bool @@ -255,7 +256,7 @@ func TestValidatesHostParameter(t *testing.T) { {"host/server", "", "", true}, } for i, testCase := range testCases { - u, err := DefaultServerURL(testCase.Host, testCase.Prefix, *testapi.Default.GroupVersion(), false) + u, versionedAPIPath, err := DefaultServerURL(testCase.Host, testCase.APIPath, *testapi.Default.GroupVersion(), false) switch { case err == nil && testCase.Err: t.Errorf("expected error but was nil") @@ -266,6 +267,7 @@ func TestValidatesHostParameter(t *testing.T) { case err != nil: continue } + u.Path = path.Join(u.Path, versionedAPIPath) if e, a := testCase.URL, u.String(); e != a { t.Errorf("%d: expected host %s, got %s", i, e, a) continue diff --git a/pkg/client/unversioned/remotecommand/remotecommand_test.go b/pkg/client/unversioned/remotecommand/remotecommand_test.go index 1941f22b936..80c60be48c5 100644 --- a/pkg/client/unversioned/remotecommand/remotecommand_test.go +++ b/pkg/client/unversioned/remotecommand/remotecommand_test.go @@ -190,7 +190,7 @@ func TestRequestExecuteRemoteCommand(t *testing.T) { server := httptest.NewServer(fakeExecServer(t, i, testCase.Stdin, testCase.Stdout, testCase.Stderr, testCase.Error, testCase.Tty, testCase.MessageCount)) url, _ := url.ParseRequestURI(server.URL) - c := client.NewRESTClient(url, unversioned.GroupVersion{Group: "x"}, nil, -1, -1) + c := client.NewRESTClient(url, "", unversioned.GroupVersion{Group: "x"}, nil, -1, -1) req := c.Post().Resource("testing") req.SetHeader(httpstream.HeaderProtocolVersion, StreamProtocolV2Name) req.Param("command", "ls") @@ -272,7 +272,7 @@ func TestRequestAttachRemoteCommand(t *testing.T) { server := httptest.NewServer(fakeExecServer(t, i, testCase.Stdin, testCase.Stdout, testCase.Stderr, testCase.Error, testCase.Tty, 1)) url, _ := url.ParseRequestURI(server.URL) - c := client.NewRESTClient(url, unversioned.GroupVersion{Group: "x"}, nil, -1, -1) + c := client.NewRESTClient(url, "", unversioned.GroupVersion{Group: "x"}, nil, -1, -1) req := c.Post().Resource("testing") conf := &client.Config{ diff --git a/pkg/client/unversioned/request.go b/pkg/client/unversioned/request.go index 29cc6f7b49a..e043e4e3ae9 100644 --- a/pkg/client/unversioned/request.go +++ b/pkg/client/unversioned/request.go @@ -87,10 +87,10 @@ type Request struct { codec runtime.Codec // generic components accessible via method setters - path string - subpath string - params url.Values - headers http.Header + pathPrefix string + subpath string + params url.Values + headers http.Header // structural elements of the request that are part of the Kubernetes API conventions namespace string @@ -115,17 +115,22 @@ type Request struct { } // NewRequest creates a new request helper object for accessing runtime.Objects on a server. -func NewRequest(client HTTPClient, verb string, baseURL *url.URL, groupVersion unversioned.GroupVersion, codec runtime.Codec, backoff BackoffManager) *Request { +func NewRequest(client HTTPClient, verb string, baseURL *url.URL, versionedAPIPath string, groupVersion unversioned.GroupVersion, codec runtime.Codec, backoff BackoffManager) *Request { if backoff == nil { glog.V(2).Infof("Not implementing request backoff strategy.") backoff = &NoBackoff{} } metrics.Register() + + pathPrefix := "/" + if baseURL != nil { + pathPrefix = path.Join(pathPrefix, baseURL.Path) + } return &Request{ client: client, verb: verb, baseURL: baseURL, - path: baseURL.Path, + pathPrefix: path.Join(pathPrefix, versionedAPIPath), groupVersion: groupVersion, codec: codec, backoffMgr: backoff, @@ -139,7 +144,7 @@ func (r *Request) Prefix(segments ...string) *Request { if r.err != nil { return r } - r.path = path.Join(r.path, path.Join(segments...)) + r.pathPrefix = path.Join(r.pathPrefix, path.Join(segments...)) return r } @@ -244,11 +249,10 @@ func (r *Request) AbsPath(segments ...string) *Request { if r.err != nil { return r } - if len(segments) == 1 { + r.pathPrefix = path.Join(r.baseURL.Path, path.Join(segments...)) + if len(segments) == 1 && (len(r.baseURL.Path) > 1 || len(segments[0]) > 1) && strings.HasSuffix(segments[0], "/") { // preserve any trailing slashes for legacy behavior - r.path = segments[0] - } else { - r.path = path.Join(segments...) + r.pathPrefix += "/" } return r } @@ -264,7 +268,7 @@ func (r *Request) RequestURI(uri string) *Request { r.err = err return r } - r.path = locator.Path + r.pathPrefix = locator.Path if len(locator.Query()) > 0 { if r.params == nil { r.params = make(url.Values) @@ -554,14 +558,14 @@ func (r *Request) Body(obj interface{}) *Request { // URL returns the current working URL. func (r *Request) URL() *url.URL { - p := r.path + p := r.pathPrefix if r.namespaceSet && len(r.namespace) > 0 { p = path.Join(p, "namespaces", r.namespace) } if len(r.resource) != 0 { p = path.Join(p, strings.ToLower(r.resource)) } - // Join trims trailing slashes, so preserve r.path's trailing slash for backwards compat if nothing was changed + // Join trims trailing slashes, so preserve r.pathPrefix's trailing slash for backwards compat if nothing was changed if len(r.resourceName) != 0 || len(r.subpath) != 0 || len(r.subresource) != 0 { p = path.Join(p, r.resourceName, r.subresource, r.subpath) } diff --git a/pkg/client/unversioned/request_test.go b/pkg/client/unversioned/request_test.go index 7345954998c..2633d08a20f 100644 --- a/pkg/client/unversioned/request_test.go +++ b/pkg/client/unversioned/request_test.go @@ -70,7 +70,7 @@ func TestRequestWithErrorWontChange(t *testing.T) { } func TestRequestPreservesBaseTrailingSlash(t *testing.T) { - r := &Request{baseURL: &url.URL{}, path: "/path/"} + r := &Request{baseURL: &url.URL{}, pathPrefix: "/path/"} if s := r.URL().String(); s != "/path/" { t.Errorf("trailing slash should be preserved: %s", s) } @@ -112,8 +112,8 @@ func TestRequestSetsNamespace(t *testing.T) { func TestRequestOrdersNamespaceInPath(t *testing.T) { r := (&Request{ - baseURL: &url.URL{}, - path: "/test/", + baseURL: &url.URL{}, + pathPrefix: "/test/", }).Name("bar").Resource("baz").Namespace("foo") if s := r.URL().String(); s != "/test/namespaces/foo/baz/bar" { t.Errorf("namespace should be in order in path: %s", s) @@ -122,8 +122,8 @@ func TestRequestOrdersNamespaceInPath(t *testing.T) { func TestRequestOrdersSubResource(t *testing.T) { r := (&Request{ - baseURL: &url.URL{}, - path: "/test/", + baseURL: &url.URL{}, + pathPrefix: "/test/", }).Name("bar").Resource("baz").Namespace("foo").Suffix("test").SubResource("a", "b") if s := r.URL().String(); s != "/test/namespaces/foo/baz/bar/a/b/test" { t.Errorf("namespace should be in order in path: %s", s) @@ -216,7 +216,7 @@ func TestRequestURI(t *testing.T) { r := (&Request{}).Param("foo", "a") r.Prefix("other") r.RequestURI("/test?foo=b&a=b&c=1&c=2") - if r.path != "/test" { + if r.pathPrefix != "/test" { t.Errorf("path is wrong: %#v", r) } if !reflect.DeepEqual(r.params, url.Values{"a": []string{"b"}, "foo": []string{"b"}, "c": []string{"1", "2"}}) { @@ -263,7 +263,7 @@ func TestResultIntoWithErrReturnsErr(t *testing.T) { func TestURLTemplate(t *testing.T) { uri, _ := url.Parse("http://localhost") - r := NewRequest(nil, "POST", uri, unversioned.GroupVersion{Group: "test"}, nil, nil) + r := NewRequest(nil, "POST", uri, "", unversioned.GroupVersion{Group: "test"}, nil, nil) r.Prefix("pre1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0") full := r.URL() if full.String() != "http://localhost/pre1/namespaces/ns/r1/nm?p0=v0" { @@ -324,7 +324,7 @@ func TestTransformResponse(t *testing.T) { {Response: &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader(invalid))}, Data: invalid}, } for i, test := range testCases { - r := NewRequest(nil, "", uri, *testapi.Default.GroupVersion(), testapi.Default.Codec(), nil) + r := NewRequest(nil, "", uri, "", *testapi.Default.GroupVersion(), testapi.Default.Codec(), nil) if test.Response.Body == nil { test.Response.Body = ioutil.NopCloser(bytes.NewReader([]byte{})) } @@ -448,7 +448,7 @@ func TestRequestWatch(t *testing.T) { Err: true, }, { - Request: &Request{baseURL: &url.URL{}, path: "%"}, + Request: &Request{baseURL: &url.URL{}, pathPrefix: "%"}, Err: true, }, { @@ -577,7 +577,7 @@ func TestRequestStream(t *testing.T) { Err: true, }, { - Request: &Request{baseURL: &url.URL{}, path: "%"}, + Request: &Request{baseURL: &url.URL{}, pathPrefix: "%"}, Err: true, }, { @@ -663,7 +663,7 @@ func TestRequestDo(t *testing.T) { Err: true, }, { - Request: &Request{baseURL: &url.URL{}, path: "%"}, + Request: &Request{baseURL: &url.URL{}, pathPrefix: "%"}, Err: true, }, { @@ -1050,11 +1050,37 @@ func TestVerbs(t *testing.T) { } func TestAbsPath(t *testing.T) { - expectedPath := "/bar/foo" - c := testRESTClient(t, nil) - r := c.Post().Prefix("/foo").AbsPath(expectedPath) - if r.path != expectedPath { - t.Errorf("unexpected path: %s, expected %s", r.path, expectedPath) + for i, tc := range []struct { + configPrefix string + resourcePrefix string + absPath string + wantsAbsPath string + }{ + {"", "", "", "/"}, + {"", "", "/", "/"}, + {"", "", "/api", "/api"}, + {"", "", "/api/", "/api/"}, + {"", "", "/apis", "/apis"}, + {"", "/foo", "/bar/foo", "/bar/foo"}, + {"", "/api/foo/123", "/bar/foo", "/bar/foo"}, + {"/p1", "", "", "/p1"}, + {"/p1", "", "/", "/p1/"}, + {"/p1", "", "/api", "/p1/api"}, + {"/p1", "", "/apis", "/p1/apis"}, + {"/p1", "/r1", "/apis", "/p1/apis"}, + {"/p1", "/api/r1", "/apis", "/p1/apis"}, + {"/p1/api/p2", "", "", "/p1/api/p2"}, + {"/p1/api/p2", "", "/", "/p1/api/p2/"}, + {"/p1/api/p2", "", "/api", "/p1/api/p2/api"}, + {"/p1/api/p2", "", "/api/", "/p1/api/p2/api/"}, + {"/p1/api/p2", "/r1", "/api/", "/p1/api/p2/api/"}, + {"/p1/api/p2", "/api/r1", "/api/", "/p1/api/p2/api/"}, + } { + c := NewOrDie(&Config{Host: "http://localhost:123" + tc.configPrefix}) + r := c.Post().Prefix(tc.resourcePrefix).AbsPath(tc.absPath) + if r.pathPrefix != tc.wantsAbsPath { + t.Errorf("test case %d failed, unexpected path: %q, expected %q", i, r.pathPrefix, tc.wantsAbsPath) + } } } @@ -1071,7 +1097,7 @@ func TestUintParam(t *testing.T) { for _, item := range table { u, _ := url.Parse("http://localhost") - r := NewRequest(nil, "GET", u, unversioned.GroupVersion{Group: "test"}, nil, nil).AbsPath("").UintParam(item.name, item.testVal) + r := NewRequest(nil, "GET", u, "", unversioned.GroupVersion{Group: "test"}, nil, nil).AbsPath("").UintParam(item.name, item.testVal) if e, a := item.expectStr, r.URL().String(); e != a { t.Errorf("expected %v, got %v", e, a) } @@ -1241,6 +1267,6 @@ func testRESTClient(t testing.TB, srv *httptest.Server) *RESTClient { t.Fatalf("failed to parse test URL: %v", err) } } - baseURL.Path = testapi.Default.ResourcePath("", "", "") - return NewRESTClient(baseURL, *testapi.Default.GroupVersion(), testapi.Default.Codec(), 0, 0) + versionedAPIPath := testapi.Default.ResourcePath("", "", "") + return NewRESTClient(baseURL, versionedAPIPath, *testapi.Default.GroupVersion(), testapi.Default.Codec(), 0, 0) } diff --git a/pkg/client/unversioned/restclient.go b/pkg/client/unversioned/restclient.go index e377ed2c40d..30cecc870e2 100644 --- a/pkg/client/unversioned/restclient.go +++ b/pkg/client/unversioned/restclient.go @@ -45,7 +45,8 @@ const ( // // Most consumers should use client.New() to get a Kubernetes API client. type RESTClient struct { - baseURL *url.URL + baseURL *url.URL + versionedAPIPath string // A string identifying the version of the API this client is expected to use. groupVersion unversioned.GroupVersion @@ -64,7 +65,7 @@ type RESTClient struct { // NewRESTClient creates a new RESTClient. This client performs generic REST functions // such as Get, Put, Post, and Delete on specified paths. Codec controls encoding and // decoding of responses from the server. -func NewRESTClient(baseURL *url.URL, groupVersion unversioned.GroupVersion, c runtime.Codec, maxQPS float32, maxBurst int) *RESTClient { +func NewRESTClient(baseURL *url.URL, versionedAPIPath string, groupVersion unversioned.GroupVersion, c runtime.Codec, maxQPS float32, maxBurst int) *RESTClient { base := *baseURL if !strings.HasSuffix(base.Path, "/") { base.Path += "/" @@ -77,10 +78,11 @@ func NewRESTClient(baseURL *url.URL, groupVersion unversioned.GroupVersion, c ru throttle = util.NewTokenBucketRateLimiter(maxQPS, maxBurst) } return &RESTClient{ - baseURL: &base, - groupVersion: groupVersion, - Codec: c, - Throttle: throttle, + baseURL: &base, + versionedAPIPath: versionedAPIPath, + groupVersion: groupVersion, + Codec: c, + Throttle: throttle, } } @@ -123,9 +125,9 @@ func (c *RESTClient) Verb(verb string) *Request { backoff := readExpBackoffConfig() if c.Client == nil { - return NewRequest(nil, verb, c.baseURL, c.groupVersion, c.Codec, backoff) + return NewRequest(nil, verb, c.baseURL, c.versionedAPIPath, c.groupVersion, c.Codec, backoff) } - return NewRequest(c.Client, verb, c.baseURL, c.groupVersion, c.Codec, backoff) + return NewRequest(c.Client, verb, c.baseURL, c.versionedAPIPath, c.groupVersion, c.Codec, backoff) } // Post begins a POST request. Short for c.Verb("POST"). diff --git a/pkg/kubectl/cmd/config/config_test.go b/pkg/kubectl/cmd/config/config_test.go index 1f98633fa47..f099cb734ac 100644 --- a/pkg/kubectl/cmd/config/config_test.go +++ b/pkg/kubectl/cmd/config/config_test.go @@ -134,14 +134,10 @@ func TestSetWithPathPrefixIntoExistingStruct(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - expectedHost := "http://cow.org:8080" + expectedHost := "http://cow.org:8080/foo/baz" if expectedHost != dcc.Host { t.Fatalf("expected client.Config.Host = %q instead of %q", expectedHost, dcc.Host) } - expectedPrefix := "/foo/baz" - if expectedPrefix != dcc.Prefix { - t.Fatalf("expected client.Config.Prefix = %q instead of %q", expectedPrefix, dcc.Prefix) - } } func TestUnsetStruct(t *testing.T) { diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index 55a85cc2196..bc6f24f5efe 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -203,7 +203,6 @@ var _ = Describe("Kubectl client", func() { Failf("Unable to parse URL %s. Error=%s", apiServer, err) } apiServerUrl.Scheme = "https" - apiServerUrl.Path = "/api" if !strings.Contains(apiServer, ":443") { apiServerUrl.Host = apiServerUrl.Host + ":443" }