Decouple defaulting from genericapiserver and master

cmd/kube-apiserver/app/server.go

@@ -330,7 +330,7 @@ func Run(s *options.APIServer) error {
 		cachesize.SetWatchCacheSizes(s.WatchCacheSizes)
 	}
 
-	m, err := config.New()
+	m, err := config.Complete().New()
 	if err != nil {
 		return err
 	}

examples/apiserver/apiserver.go

@@ -70,7 +70,7 @@ func Run(serverOptions *genericoptions.ServerRunOptions) error {
 	config := genericapiserver.NewConfig(serverOptions)
 	config.Authorizer = authorizer.NewAlwaysAllowAuthorizer()
 	config.Serializer = api.Codecs
-	s, err := config.New()
+	s, err := config.Complete().New()
 	if err != nil {
 		return fmt.Errorf("Error in bringing up the server: %v", err)
 	}

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

@@ -219,7 +219,7 @@ func Run(s *options.ServerRunOptions) error {
 		cachesize.SetWatchCacheSizes(s.WatchCacheSizes)
 	}
 
-	m, err := genericConfig.New()
+	m, err := genericConfig.Complete().New()
 	if err != nil {
 		return err
 	}

pkg/genericapiserver/config.go

@@ -210,8 +210,8 @@ func NewConfig(options *options.ServerRunOptions) *Config {
 	}
 }
 
-// setDefaults fills in any fields not set that are required to have valid data.
+// Complete fills in any fields not set that are required to have valid data. It's mutating the receiver.
-func (c *Config) setDefaults() {
+func (c *Config) Complete() *Config {
 	if c.ServiceClusterIPRange == nil {
 		defaultNet := "10.0.0.0/24"
 		glog.Warningf("Network range for service cluster IPs is unspecified. Defaulting to %v.", defaultNet)
@@ -264,6 +264,7 @@ func (c *Config) setDefaults() {
 		}
 		c.ExternalHost = hostAndPort
 	}
+	return c
 }
 
 // New returns a new instance of GenericAPIServer from the given config.
@@ -288,13 +289,11 @@ func (c *Config) setDefaults() {
 //   If the caller wants to add additional endpoints not using the GenericAPIServer's
 //   auth, then the caller should create a handler for those endpoints, which delegates the
 //   any unhandled paths to "Handler".
-func (c Config) New() (*GenericAPIServer, error) {
+func (c *Config) New() (*GenericAPIServer, error) {
 	if c.Serializer == nil {
 		return nil, fmt.Errorf("Genericapiserver.New() called with config.Serializer == nil")
 	}
 
-	c.setDefaults()
-
 	s := &GenericAPIServer{
 		ServiceClusterIPRange: c.ServiceClusterIPRange,
 		ServiceNodePortRange:  c.ServiceNodePortRange,
@@ -342,8 +341,8 @@ func (c Config) New() (*GenericAPIServer, error) {
 		})
 	}
 
-	s.installAPI(&c)
+	s.installAPI(c)
-	s.Handler, s.InsecureHandler = s.buildHandlerChains(&c, http.Handler(s.Mux.BaseMux().(*http.ServeMux)))
+	s.Handler, s.InsecureHandler = s.buildHandlerChains(c, http.Handler(s.Mux.BaseMux().(*http.ServeMux)))
 
 	return s, nil
 }

pkg/genericapiserver/genericapiserver_test.go

@@ -64,7 +64,7 @@ func setUp(t *testing.T) (*etcdtesting.EtcdTestServer, Config, *assert.Assertion
 func newMaster(t *testing.T) (*GenericAPIServer, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) {
 	etcdserver, config, assert := setUp(t)
 
-	s, err := config.New()
+	s, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the server: %v", err)
 	}
@@ -109,7 +109,7 @@ func TestInstallAPIGroups(t *testing.T) {
 	config.APIPrefix = "/apiPrefix"
 	config.APIGroupPrefix = "/apiGroupPrefix"
 
-	s, err := config.New()
+	s, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the server: %v", err)
 	}
@@ -212,7 +212,7 @@ func TestNotRestRoutesHaveAuth(t *testing.T) {
 	config.EnableSwaggerSupport = true
 	config.EnableVersion = true
 
-	s, err := config.New()
+	s, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the server: %v", err)
 	}

pkg/master/master.go

@@ -190,6 +190,16 @@ type RESTStorageProvider interface {
 	NewRESTStorage(apiResourceConfigSource genericapiserver.APIResourceConfigSource, restOptionsGetter RESTOptionsGetter) (groupInfo genericapiserver.APIGroupInfo, enabled bool)
 }
 
+// Complete fills in any fields not set that are required to have valid data. It's mutating the receiver.
+func (c *Config) Complete() *Config {
+	c.Config.Complete()
+
+	// enable swagger UI only if general UI support is on
+	c.Config.EnableSwaggerUI = c.Config.EnableSwaggerUI && c.EnableUISupport
+
+	return c
+}
+
 // New returns a new instance of Master from the given config.
 // Certain config fields will be set to a default value if unset.
 // Certain config fields must be specified, including:
@@ -199,9 +209,7 @@ func (c *Config) New() (*Master, error) {
 		return nil, fmt.Errorf("Master.New() called with config.KubeletClient == nil")
 	}
 
-	gc := *c.Config                                              // copy before mutations
+	s, err := c.Config.New()
-	gc.EnableSwaggerUI = gc.EnableSwaggerUI && c.EnableUISupport // disable swagger UI if general UI supports it
-	s, err := gc.New()
 	if err != nil {
 		return nil, err
 	}

pkg/master/master_test.go

@@ -125,7 +125,7 @@ func setUp(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.
 func newMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) {
 	_, etcdserver, config, assert := setUp(t)
 
-	master, err := config.New()
+	master, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)
 	}
@@ -151,7 +151,7 @@ func limitedAPIResourceConfigSource() *genericapiserver.ResourceConfig {
 func newLimitedMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) {
 	_, etcdserver, config, assert := setUp(t)
 	config.APIResourceConfigSource = limitedAPIResourceConfigSource()
-	master, err := config.New()
+	master, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)
 	}
@@ -1262,7 +1262,7 @@ func TestValidOpenAPISpec(t *testing.T) {
 			Version: "unversioned",
 		},
 	}
-	master, err := config.New()
+	master, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)
 	}

test/integration/framework/master_utils.go

@@ -149,7 +149,7 @@ func startMasterOrDie(masterConfig *master.Config) (*master.Master, *httptest.Se
 			},
 		}
 	}
-	m, err := masterConfig.New()
+	m, err := masterConfig.Complete().New()
 	if err != nil {
 		glog.Fatalf("error in bringing up the master: %v", err)
 	}

test/integration/quota/quota_test.go

@@ -73,7 +73,7 @@ func TestQuota(t *testing.T) {
 
 	masterConfig := framework.NewIntegrationTestMasterConfig()
 	masterConfig.AdmissionControl = admission
-	m, err = masterConfig.New()
+	m, err = masterConfig.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)
 	}

test/integration/scheduler_perf/util.go

@@ -48,7 +48,7 @@ func mustSetupScheduler() (schedulerConfigFactory *factory.ConfigFactory, destro
 
 	var m *master.Master
 	masterConfig := framework.NewIntegrationTestMasterConfig()
-	m, err := masterConfig.New()
+	m, err := masterConfig.Complete().New()
 	if err != nil {
 		panic("error in brining up the master: " + err.Error())
 	}

test/integration/serviceaccount/service_account_test.go

@@ -412,7 +412,7 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie
 	masterConfig.AdmissionControl = serviceAccountAdmission
 
 	// Create a master and install handlers into mux.
-	m, err := masterConfig.New()
+	m, err := masterConfig.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)
 	}