Merge pull request #33846 from deads2k/api-20-server-needs-serving-info

Automatic merge from submit-queue

stop plumbing options to start

The API server should have sufficient information to start itself without relying on an `options` object from a different packages focused on CLI interaction.  This provides that separation

@liggitt distinct from other changes

cmd/kube-apiserver/app/server.go

@@ -343,6 +343,6 @@ func Run(s *options.APIServer) error {
 	}
 
 	sharedInformers.Start(wait.NeverStop)
-	m.Run(s.ServerRunOptions)
+	m.Run()
 	return nil
 }

examples/apiserver/apiserver.go

@@ -101,6 +101,6 @@ func Run(serverOptions *genericoptions.ServerRunOptions) error {
 	if err := s.InstallAPIGroup(&apiGroupInfo); err != nil {
 		return fmt.Errorf("Error in installing API: %v", err)
 	}
-	s.Run(serverOptions)
+	s.Run()
 	return nil
 }

federation/cmd/federation-apiserver/app/server.go

@@ -249,7 +249,7 @@ func Run(s *options.ServerRunOptions) error {
 	installExtensionsAPIs(m, restOptionsFactory)
 
 	sharedInformers.Start(wait.NeverStop)
-	m.Run(s.ServerRunOptions)
+	m.Run()
 	return nil
 }
 

pkg/genericapiserver/config.go

@@ -23,6 +23,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"path"
 	"regexp"
 	"strconv"
 	"strings"
@@ -104,6 +105,9 @@ type Config struct {
 	// same value for this field. (Numbers > 1 currently untested.)
 	MasterCount int
 
+	SecureServingInfo   *ServingInfo
+	InsecureServingInfo *ServingInfo
+
 	// The port on PublicAddress where a read-write server will be installed.
 	// Defaults to 6443 if not set.
 	ReadWritePort int
@@ -171,6 +175,24 @@ type Config struct {
 	LongRunningFunc genericfilters.LongRunningRequestCheck
 }
 
+type ServingInfo struct {
+	// BindAddress is the ip:port to serve on
+	BindAddress string
+	// ServerCert is the TLS cert info for serving secure traffic
+	ServerCert CertInfo
+	// ClientCA is the certificate bundle for all the signers that you'll recognize for incoming client certificates
+	ClientCA string
+}
+
+type CertInfo struct {
+	// CertFile is a file containing a PEM-encoded certificate
+	CertFile string
+	// KeyFile is a file containing a PEM-encoded private key for the certificate specified by CertFile
+	KeyFile string
+	// Generate indicates that the cert/key pair should be generated if its not present.
+	Generate bool
+}
+
 func NewConfig(options *options.ServerRunOptions) *Config {
 	longRunningRE := regexp.MustCompile(options.LongRunningRequestRE)
 
@@ -184,6 +206,30 @@ func NewConfig(options *options.ServerRunOptions) *Config {
 		}
 	}
 
+	var secureServingInfo *ServingInfo
+	if options.SecurePort > 0 {
+		secureServingInfo = &ServingInfo{
+			BindAddress: net.JoinHostPort(options.BindAddress.String(), strconv.Itoa(options.SecurePort)),
+			ServerCert: CertInfo{
+				CertFile: options.TLSCertFile,
+				KeyFile:  options.TLSPrivateKeyFile,
+			},
+			ClientCA: options.ClientCAFile,
+		}
+		if options.TLSCertFile == "" && options.TLSPrivateKeyFile == "" {
+			secureServingInfo.ServerCert.Generate = true
+			secureServingInfo.ServerCert.CertFile = path.Join(options.CertDirectory, "apiserver.crt")
+			secureServingInfo.ServerCert.KeyFile = path.Join(options.CertDirectory, "apiserver.key")
+		}
+	}
+
+	var insecureServingInfo *ServingInfo
+	if options.InsecurePort > 0 {
+		insecureServingInfo = &ServingInfo{
+			BindAddress: net.JoinHostPort(options.InsecureBindAddress.String(), strconv.Itoa(options.InsecurePort)),
+		}
+	}
+
 	return &Config{
 		APIGroupPrefix:            options.APIGroupPrefix,
 		APIPrefix:                 options.APIPrefix,
@@ -199,6 +245,8 @@ func NewConfig(options *options.ServerRunOptions) *Config {
 		KubernetesServiceNodePort: options.KubernetesServiceNodePort,
 		MasterCount:               options.MasterCount,
 		MinRequestTimeout:         options.MinRequestTimeout,
+		SecureServingInfo:         secureServingInfo,
+		InsecureServingInfo:       insecureServingInfo,
 		PublicAddress:             options.AdvertiseAddress,
 		ReadWritePort:             options.SecurePort,
 		ServiceClusterIPRange:     &options.ServiceClusterIPRange,
@@ -324,6 +372,8 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 		enableSwaggerSupport: c.EnableSwaggerSupport,
 
 		MasterCount:          c.MasterCount,
+		SecureServingInfo:    c.SecureServingInfo,
+		InsecureServingInfo:  c.InsecureServingInfo,
 		ExternalAddress:      c.ExternalHost,
 		ClusterIP:            c.PublicAddress,
 		PublicReadWritePort:  c.ReadWritePort,

pkg/genericapiserver/genericapiserver.go

@@ -22,7 +22,6 @@ import (
 	"mime"
 	"net"
 	"net/http"
-	"path"
 	"sort"
 	"strconv"
 	"strings"
@@ -45,7 +44,6 @@ import (
 	"k8s.io/kubernetes/pkg/client/restclient"
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi"
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
-	"k8s.io/kubernetes/pkg/genericapiserver/options"
 	"k8s.io/kubernetes/pkg/runtime"
 	certutil "k8s.io/kubernetes/pkg/util/cert"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
@@ -124,6 +122,9 @@ type GenericAPIServer struct {
 	HandlerContainer *restful.Container
 	MasterCount      int
 
+	SecureServingInfo   *ServingInfo
+	InsecureServingInfo *ServingInfo
+
 	// ExternalAddress is the address (hostname or IP and port) that should be used in
 	// external (public internet) URLs for this GenericAPIServer.
 	ExternalAddress string
@@ -217,7 +218,7 @@ func NewHandlerContainer(mux *http.ServeMux, s runtime.NegotiatedSerializer) *re
 	return container
 }
 
-func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
+func (s *GenericAPIServer) Run() {
 	// install APIs which depend on other APIs to be installed
 	if s.enableSwaggerSupport {
 		s.InstallSwaggerAPI()
@@ -227,10 +228,9 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
 	}
 
 	secureStartedCh := make(chan struct{})
-	if options.SecurePort != 0 {
+	if s.SecureServingInfo != nil {
-		secureLocation := net.JoinHostPort(options.BindAddress.String(), strconv.Itoa(options.SecurePort))
 		secureServer := &http.Server{
-			Addr:           secureLocation,
+			Addr:           s.SecureServingInfo.BindAddress,
 			Handler:        s.Handler,
 			MaxHeaderBytes: 1 << 20,
 			TLSConfig: &tls.Config{
@@ -241,8 +241,8 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
 			},
 		}
 
-		if len(options.ClientCAFile) > 0 {
+		if len(s.SecureServingInfo.ClientCA) > 0 {
-			clientCAs, err := certutil.NewPool(options.ClientCAFile)
+			clientCAs, err := certutil.NewPool(s.SecureServingInfo.ClientCA)
 			if err != nil {
 				glog.Fatalf("Unable to load client CA file: %v", err)
 			}
@@ -256,30 +256,27 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
 
 		}
 
-		glog.Infof("Serving securely on %s", secureLocation)
+		// It would be nice to set a fqdn subject alt name, but only the kubelets know, the apiserver is clueless
-		if options.TLSCertFile == "" && options.TLSPrivateKeyFile == "" {
+		// alternateDNS = append(alternateDNS, "kubernetes.default.svc.CLUSTER.DNS.NAME")
-			options.TLSCertFile = path.Join(options.CertDirectory, "apiserver.crt")
+		if s.SecureServingInfo.ServerCert.Generate && !certutil.CanReadCertOrKey(s.SecureServingInfo.ServerCert.CertFile, s.SecureServingInfo.ServerCert.KeyFile) {
-			options.TLSPrivateKeyFile = path.Join(options.CertDirectory, "apiserver.key")
 			// TODO (cjcullen): Is ClusterIP the right address to sign a cert with?
 			alternateIPs := []net.IP{s.ServiceReadWriteIP}
 			alternateDNS := []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes", "localhost"}
-			// It would be nice to set a fqdn subject alt name, but only the kubelets know, the apiserver is clueless
+
-			// alternateDNS = append(alternateDNS, "kubernetes.default.svc.CLUSTER.DNS.NAME")
+			if err := certutil.GenerateSelfSignedCert(s.ClusterIP.String(), s.SecureServingInfo.ServerCert.CertFile, s.SecureServingInfo.ServerCert.KeyFile, alternateIPs, alternateDNS); err != nil {
-			if !certutil.CanReadCertOrKey(options.TLSCertFile, options.TLSPrivateKeyFile) {
-				if err := certutil.GenerateSelfSignedCert(s.ClusterIP.String(), options.TLSCertFile, options.TLSPrivateKeyFile, alternateIPs, alternateDNS); err != nil {
 				glog.Errorf("Unable to generate self signed cert: %v", err)
 			} else {
-					glog.Infof("Using self-signed cert (%s, %s)", options.TLSCertFile, options.TLSPrivateKeyFile)
+				glog.Infof("Using self-signed cert (%s, %s)", s.SecureServingInfo.ServerCert.CertFile, s.SecureServingInfo.ServerCert.KeyFile)
-				}
 			}
 		}
 
+		glog.Infof("Serving securely on %s", s.SecureServingInfo.BindAddress)
 		go func() {
 			defer utilruntime.HandleCrash()
 
 			notifyStarted := sync.Once{}
 			for {
-				if err := secureServer.ListenAndServeTLS(options.TLSCertFile, options.TLSPrivateKeyFile); err != nil {
+				if err := secureServer.ListenAndServeTLS(s.SecureServingInfo.ServerCert.CertFile, s.SecureServingInfo.ServerCert.KeyFile); err != nil {
 					glog.Errorf("Unable to listen for secure (%v); will try again.", err)
 				} else {
 					notifyStarted.Do(func() {
@@ -293,14 +290,14 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
 		close(secureStartedCh)
 	}
 
-	insecureLocation := net.JoinHostPort(options.InsecureBindAddress.String(), strconv.Itoa(options.InsecurePort))
+	insecureStartedCh := make(chan struct{})
+	if s.InsecureServingInfo != nil {
 		insecureServer := &http.Server{
-		Addr:           insecureLocation,
+			Addr:           s.InsecureServingInfo.BindAddress,
 			Handler:        s.InsecureHandler,
 			MaxHeaderBytes: 1 << 20,
 		}
-	insecureStartedCh := make(chan struct{})
+		glog.Infof("Serving insecurely on %s", s.InsecureServingInfo.BindAddress)
-	glog.Infof("Serving insecurely on %s", insecureLocation)
 		go func() {
 			defer utilruntime.HandleCrash()
 
@@ -316,6 +313,9 @@ func (s *GenericAPIServer) Run(options *options.ServerRunOptions) {
 				time.Sleep(15 * time.Second)
 			}
 		}()
+	} else {
+		close(insecureStartedCh)
+	}
 
 	<-secureStartedCh
 	<-insecureStartedCh