Merge pull request #41932 from ericchiang/self-client-config-scheme

Automatic merge from submit-queue (batch tested with PRs 42126, 42130, 42232, 42245, 41932)

apiserver/pkg/server: include scheme in insecure self client config

Noticed this during bootkube development: https://github.com/kubernetes-incubator/bootkube/issues/325

In Go 1.8's `url.Parse` became more strict, and `url.Parse("127.0.0.1:8080")` now fails.

https://beta.golang.org/doc/go1.8#net_url
https://play.golang.org/p/dw_cPeotG4

Accidentally compiled bootkube with 1.8 and tracked a panic down to the loopback client config. Though we're still using the old "genericapiserver", this seems to be translated from the old one.

The actual panic we observed was

```
E0222 19:40:11.364949       5 server.go:254] Failed to create clientset: parse 127.0.0.1:8080: first path segment in URL cannot contain colon
panic: parse 127.0.0.1:8080: first path segment in URL cannot contain colon
goroutine 35 [running]:
github.com/kubernetes-incubator/bootkube/vendor/k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion.NewForConfigOrDie(0xc420728ea0, 0xc420738e30)
        /home/eric/src/github.com/kubernetes-incubator/bootkube/vendor/k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion/core_client.go:132 +0x62
github.com/kubernetes-incubator/bootkube/vendor/k8s.io/kubernetes/pkg/master.(*Config).Complete(0xc420739db8, 0x0)
        /home/eric/src/github.com/kubernetes-incubator/bootkube/vendor/k8s.io/kubernetes/pkg/master/master.go:180 +0x40a
github.com/kubernetes-incubator/bootkube/vendor/k8s.io/kubernetes/cmd/kube-apiserver/app.Run(0xc42007a3c0, 0x0, 0x0)
        /home/eric/src/github.com/kubernetes-incubator/bootkube/vendor/k8s.io/kubernetes/cmd/kube-apiserver/app/server.go:347 +0x1e8d
github.com/kubernetes-incubator/bootkube/pkg/bootkube.(*bootkube).Run.func1(0xc4206b01e0, 0xc420164300)
        /home/eric/src/github.com/kubernetes-incubator/bootkube/pkg/bootkube/bootkube.go:124 +0x2f
created by github.com/kubernetes-incubator/bootkube/pkg/bootkube.(*bootkube).Run
        /home/eric/src/github.com/kubernetes-incubator/bootkube/pkg/bootkube/bootkube.go:124 +0xb0
```

I don't actually know if this is the correct fix or if there should be changes to `NewForConfigOrDie`. Am looking for comments more than anything.

edit: @abourget pointed out over on bootkube that the actual panic was fixed in the internalclient by https://github.com/kubernetes/kubernetes/pull/38519.

cc @deads2k @sttts @kubernetes/sig-api-machinery-pr-reviews
pull/6/head
Kubernetes Submit Queue 2017-03-02 02:07:31 -08:00 committed by GitHub
commit fc31dae165
1 changed files with 26 additions and 8 deletions

View File

@ -33,14 +33,9 @@ func (s *SecureServingInfo) NewLoopbackClientConfig(token string, loopbackCert [
return nil, nil
}
host, port, err := net.SplitHostPort(s.ServingInfo.BindAddress)
host, port, err := s.ServingInfo.loopbackHostPort()
if err != nil {
// should never happen
return nil, fmt.Errorf("invalid secure bind address: %q", s.ServingInfo.BindAddress)
}
if host == "0.0.0.0" {
// compare MaybeDefaultWithSelfSignedCerts which adds "localhost" to the cert as alternateDNS
host = "localhost"
return nil, err
}
return &restclient.Config{
@ -95,12 +90,35 @@ func findCA(chain []*x509.Certificate) (*x509.Certificate, error) {
return nil, fmt.Errorf("no certificate with CA:TRUE found in chain")
}
// loopbackHostPort returns the host and port loopback REST clients should use
// to contact the server.
func (s *ServingInfo) loopbackHostPort() (string, string, error) {
host, port, err := net.SplitHostPort(s.BindAddress)
if err != nil {
// should never happen
return "", "", fmt.Errorf("invalid server bind address: %q", s.BindAddress)
}
// Value is expected to be an IP or DNS name, not "0.0.0.0".
if host == "0.0.0.0" {
// compare MaybeDefaultWithSelfSignedCerts which adds "localhost" to the cert as alternateDNS
host = "localhost"
}
return host, port, nil
}
func (s *ServingInfo) NewLoopbackClientConfig(token string) (*restclient.Config, error) {
if s == nil {
return nil, nil
}
host, port, err := s.loopbackHostPort()
if err != nil {
return nil, err
}
return &restclient.Config{
Host: s.BindAddress,
Host: "http://" + net.JoinHostPort(host, port),
// Increase QPS limits. The client is currently passed to all admission plugins,
// and those can be throttled in case of higher load on apiserver - see #22340 and #22422
// for more details. Once #22422 is fixed, we may want to remove it.