abstract out discovery IP determination

pkg/genericapiserver/config.go

@@ -118,8 +118,8 @@ type Config struct {
 	// Defaults to 6443 if not set.
 	ReadWritePort int
 
-	// ExternalHost is the host name to use for external (public internet) facing URLs (e.g. Swagger)
-	ExternalHost string
+	// ExternalAddress is the host name to use for external (public internet) facing URLs (e.g. Swagger)
+	ExternalAddress string
 
 	// PublicAddress is the IP address where members of the cluster (kubelet,
 	// kube-proxy, services, etc.) can reach the GenericAPIServer.
@@ -317,7 +317,7 @@ func (c *Config) ApplyOptions(options *options.ServerRunOptions) *Config {
 	c.EnableGarbageCollection = options.EnableGarbageCollection
 	c.EnableProfiling = options.EnableProfiling
 	c.EnableSwaggerUI = options.EnableSwaggerUI
-	c.ExternalHost = options.ExternalHost
+	c.ExternalAddress = options.ExternalHost
 	c.KubernetesServiceNodePort = options.KubernetesServiceNodePort
 	c.MasterCount = options.MasterCount
 	c.MaxRequestsInFlight = options.MaxRequestsInFlight
@@ -366,12 +366,12 @@ func (c *Config) Complete() completedConfig {
 		c.ServiceNodePortRange = options.DefaultServiceNodePortRange
 		glog.Infof("Node port range unspecified. Defaulting to %v.", c.ServiceNodePortRange)
 	}
-	if len(c.ExternalHost) == 0 && c.PublicAddress != nil {
+	if len(c.ExternalAddress) == 0 && c.PublicAddress != nil {
 		hostAndPort := c.PublicAddress.String()
 		if c.ReadWritePort != 0 {
 			hostAndPort = net.JoinHostPort(hostAndPort, strconv.Itoa(c.ReadWritePort))
 		}
-		c.ExternalHost = hostAndPort
+		c.ExternalAddress = hostAndPort
 	}
 	// All APIs will have the same authentication for now.
 	if c.OpenAPIConfig != nil && c.OpenAPIConfig.SecurityDefinitions != nil {
@@ -430,8 +430,14 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 		return nil, fmt.Errorf("Genericapiserver.New() called with config.Serializer == nil")
 	}
 
+	discoveryAddresses := DefaultDiscoveryAddresses{DefaultAddress: c.ExternalAddress}
+	if c.ServiceClusterIPRange != nil {
+		discoveryAddresses.DiscoveryCIDRRules = append(discoveryAddresses.DiscoveryCIDRRules,
+			DiscoveryCIDRRule{IPRange: *c.ServiceClusterIPRange, Address: net.JoinHostPort(c.ServiceReadWriteIP.String(), strconv.Itoa(c.ServiceReadWritePort))})
+	}
+
 	s := &GenericAPIServer{
-		ServiceClusterIPRange:  c.ServiceClusterIPRange,
+		discoveryAddresses:     discoveryAddresses,
 		LoopbackClientConfig:   c.LoopbackClientConfig,
 		legacyAPIGroupPrefixes: c.LegacyAPIGroupPrefixes,
 		admissionControl:       c.AdmissionControl,
@@ -441,15 +447,12 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 		minRequestTimeout:    time.Duration(c.MinRequestTimeout) * time.Second,
 		enableSwaggerSupport: c.EnableSwaggerSupport,
 
-		MasterCount:          c.MasterCount,
-		SecureServingInfo:    c.SecureServingInfo,
-		InsecureServingInfo:  c.InsecureServingInfo,
-		ExternalAddress:      c.ExternalHost,
-		ServiceReadWriteIP:   c.ServiceReadWriteIP,
-		ServiceReadWritePort: c.ServiceReadWritePort,
+		MasterCount:         c.MasterCount,
+		SecureServingInfo:   c.SecureServingInfo,
+		InsecureServingInfo: c.InsecureServingInfo,
+		ExternalAddress:     c.ExternalAddress,
 
-		KubernetesServiceNodePort: c.KubernetesServiceNodePort,
-		apiGroupsForDiscovery:     map[string]unversioned.APIGroup{},
+		apiGroupsForDiscovery: map[string]unversioned.APIGroup{},
 
 		enableOpenAPISupport: c.EnableOpenAPISupport,
 		openAPIConfig:        c.OpenAPIConfig,
@@ -555,7 +558,7 @@ func DefaultAndValidateRunOptions(options *options.ServerRunOptions) {
 	}
 	glog.Infof("Will report %v as public IP address.", options.AdvertiseAddress)
 
-	// Set default value for ExternalHost if not specified.
+	// Set default value for ExternalAddress if not specified.
 	if len(options.ExternalHost) == 0 {
 		// TODO: extend for other providers
 		if options.CloudProvider == "gce" {

pkg/genericapiserver/discovery.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package genericapiserver
+
+import (
+	"net"
+
+	"k8s.io/kubernetes/pkg/api/unversioned"
+)
+
+type DiscoveryAddresses interface {
+	ServerAddressByClientCIDRs(net.IP) []unversioned.ServerAddressByClientCIDR
+}
+
+// DefaultDiscoveryAddresses is a default implementation of DiscoveryAddresses that will work in most cases
+type DefaultDiscoveryAddresses struct {
+	// DiscoveryCIDRRules is a list of CIDRs and Addresses to use if a client is in the range
+	DiscoveryCIDRRules []DiscoveryCIDRRule
+
+	// DefaultAddress is the address (hostname or IP and port) that should be used in
+	// if no CIDR matches more specifically.
+	DefaultAddress string
+}
+
+// DiscoveryCIDRRule is a rule for adding an alternate path to the master based on matching CIDR
+type DiscoveryCIDRRule struct {
+	IPRange net.IPNet
+
+	// Address is the address (hostname or IP and port) that should be used in
+	// if this CIDR matches
+	Address string
+}
+
+func (d DefaultDiscoveryAddresses) ServerAddressByClientCIDRs(clientIP net.IP) []unversioned.ServerAddressByClientCIDR {
+	addressCIDRMap := []unversioned.ServerAddressByClientCIDR{
+		{
+			ClientCIDR:    "0.0.0.0/0",
+			ServerAddress: d.DefaultAddress,
+		},
+	}
+
+	for _, rule := range d.DiscoveryCIDRRules {
+		addressCIDRMap = append(addressCIDRMap, rule.ServerAddressByClientCIDRs(clientIP)...)
+	}
+	return addressCIDRMap
+}
+
+func (d DiscoveryCIDRRule) ServerAddressByClientCIDRs(clientIP net.IP) []unversioned.ServerAddressByClientCIDR {
+	addressCIDRMap := []unversioned.ServerAddressByClientCIDR{}
+
+	if d.IPRange.Contains(clientIP) {
+		addressCIDRMap = append(addressCIDRMap, unversioned.ServerAddressByClientCIDR{
+			ClientCIDR:    d.IPRange.String(),
+			ServerAddress: d.Address,
+		})
+	}
+	return addressCIDRMap
+}

pkg/genericapiserver/genericapiserver.go

@@ -22,7 +22,6 @@ import (
 	"net"
 	"net/http"
 	"sort"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -78,12 +77,8 @@ type APIGroupInfo struct {
 
 // GenericAPIServer contains state for a Kubernetes cluster api server.
 type GenericAPIServer struct {
-	// ServiceClusterIPRange is used to build cluster IPs for discovery.  It is exposed so that `master.go` can
-	// construct service storage.
-	// TODO refactor this so that `master.go` drives the value used for discovery and the value here isn't exposed.
-	// that structure will force usage in the correct direction where the "owner" of the value is the source of
-	// truth for its value.
-	ServiceClusterIPRange *net.IPNet
+	// discoveryAddresses is used to build cluster IPs for discovery.
+	discoveryAddresses DiscoveryAddresses
 
 	// LoopbackClientConfig is a config for a privileged loopback connection to the API server
 	LoopbackClientConfig *restclient.Config
@@ -155,10 +150,7 @@ type GenericAPIServer struct {
 
 	// See Config.$name for documentation of these flags:
 
-	MasterCount               int
-	KubernetesServiceNodePort int // TODO(sttts): move into master
-	ServiceReadWriteIP        net.IP
-	ServiceReadWritePort      int
+	MasterCount int
 }
 
 func init() {
@@ -260,8 +252,10 @@ func (s *GenericAPIServer) InstallLegacyAPIGroup(apiPrefix string, apiGroupInfo
 	// Install the version handler.
 	// Add a handler at /<apiPrefix> to enumerate the supported api versions.
 	apiserver.AddApiWebService(s.Serializer, s.HandlerContainer.Container, apiPrefix, func(req *restful.Request) *unversioned.APIVersions {
+		clientIP := utilnet.GetClientIP(req.Request)
+
 		apiVersionsForDiscovery := unversioned.APIVersions{
-			ServerAddressByClientCIDRs: s.getServerAddressByClientCIDRs(req.Request),
+			ServerAddressByClientCIDRs: s.discoveryAddresses.ServerAddressByClientCIDRs(clientIP),
 			Versions:                   apiVersions,
 		}
 		return &apiVersionsForDiscovery
@@ -328,26 +322,6 @@ func (s *GenericAPIServer) RemoveAPIGroupForDiscovery(groupName string) {
 	delete(s.apiGroupsForDiscovery, groupName)
 }
 
-func (s *GenericAPIServer) getServerAddressByClientCIDRs(req *http.Request) []unversioned.ServerAddressByClientCIDR {
-	addressCIDRMap := []unversioned.ServerAddressByClientCIDR{
-		{
-			ClientCIDR:    "0.0.0.0/0",
-			ServerAddress: s.ExternalAddress,
-		},
-	}
-
-	// Add internal CIDR if the request came from internal IP.
-	clientIP := utilnet.GetClientIP(req)
-	clusterCIDR := s.ServiceClusterIPRange
-	if clusterCIDR.Contains(clientIP) {
-		addressCIDRMap = append(addressCIDRMap, unversioned.ServerAddressByClientCIDR{
-			ClientCIDR:    clusterCIDR.String(),
-			ServerAddress: net.JoinHostPort(s.ServiceReadWriteIP.String(), strconv.Itoa(s.ServiceReadWritePort)),
-		})
-	}
-	return addressCIDRMap
-}
-
 func (s *GenericAPIServer) getAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupVersion unversioned.GroupVersion, apiPrefix string) (*apiserver.APIGroupVersion, error) {
 	storage := make(map[string]rest.Storage)
 	for k, v := range apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version] {
@@ -397,7 +371,8 @@ func (s *GenericAPIServer) DynamicApisDiscovery() *restful.WebService {
 			sortedGroups = append(sortedGroups, s.apiGroupsForDiscovery[groupName])
 		}
 
-		serverCIDR := s.getServerAddressByClientCIDRs(req.Request)
+		clientIP := utilnet.GetClientIP(req.Request)
+		serverCIDR := s.discoveryAddresses.ServerAddressByClientCIDRs(clientIP)
 		groups := make([]unversioned.APIGroup, len(sortedGroups))
 		for i := range sortedGroups {
 			groups[i] = sortedGroups[i]

pkg/genericapiserver/genericapiserver_test.go

@@ -37,8 +37,8 @@ import (
 	"k8s.io/kubernetes/pkg/auth/authorizer"
 	"k8s.io/kubernetes/pkg/auth/user"
 	openapigen "k8s.io/kubernetes/pkg/generated/openapi"
-	ipallocator "k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
+	utilnet "k8s.io/kubernetes/pkg/util/net"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/version"
 
@@ -91,9 +91,6 @@ func TestNew(t *testing.T) {
 	assert.Equal(s.RequestContextMapper(), config.RequestContextMapper)
 
 	// these values get defaulted
-	_, serviceClusterIPRange, _ := net.ParseCIDR("10.0.0.0/24")
-	serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
-	assert.Equal(s.ServiceReadWriteIP, serviceReadWriteIP)
 	assert.Equal(s.ExternalAddress, net.JoinHostPort(config.PublicAddress.String(), "6443"))
 }
 
@@ -366,7 +363,7 @@ func TestDiscoveryAtAPIS(t *testing.T) {
 	assert.Equal(extensions.GroupName, groupListGroup.Name)
 	assert.Equal(extensionsVersions, groupListGroup.Versions)
 	assert.Equal(extensionsPreferredVersion, groupListGroup.PreferredVersion)
-	assert.Equal(master.getServerAddressByClientCIDRs(&http.Request{}), groupListGroup.ServerAddressByClientCIDRs)
+	assert.Equal(master.discoveryAddresses.ServerAddressByClientCIDRs(utilnet.GetClientIP(&http.Request{})), groupListGroup.ServerAddressByClientCIDRs)
 
 	// Remove the group.
 	master.RemoveAPIGroupForDiscovery(extensions.GroupName)
@@ -379,20 +376,20 @@ func TestDiscoveryAtAPIS(t *testing.T) {
 }
 
 func TestGetServerAddressByClientCIDRs(t *testing.T) {
-	s, etcdserver, _, _ := newMaster(t)
+	s, etcdserver, config, _ := newMaster(t)
 	defer etcdserver.Terminate(t)
 
 	publicAddressCIDRMap := []unversioned.ServerAddressByClientCIDR{
 		{
 			ClientCIDR:    "0.0.0.0/0",
-			ServerAddress: s.ExternalAddress,
+			ServerAddress: config.ExternalAddress,
 		},
 	}
 	internalAddressCIDRMap := []unversioned.ServerAddressByClientCIDR{
 		publicAddressCIDRMap[0],
 		{
-			ClientCIDR:    s.ServiceClusterIPRange.String(),
-			ServerAddress: net.JoinHostPort(s.ServiceReadWriteIP.String(), strconv.Itoa(s.ServiceReadWritePort)),
+			ClientCIDR:    config.ServiceClusterIPRange.String(),
+			ServerAddress: net.JoinHostPort(config.ServiceReadWriteIP.String(), strconv.Itoa(config.ServiceReadWritePort)),
 		},
 	}
 	internalIP := "10.0.0.1"
@@ -459,7 +456,7 @@ func TestGetServerAddressByClientCIDRs(t *testing.T) {
 	}
 
 	for i, test := range testCases {
-		if a, e := s.getServerAddressByClientCIDRs(&test.Request), test.ExpectedMap; reflect.DeepEqual(e, a) != true {
+		if a, e := s.discoveryAddresses.ServerAddressByClientCIDRs(utilnet.GetClientIP(&test.Request)), test.ExpectedMap; reflect.DeepEqual(e, a) != true {
 			t.Fatalf("test case %d failed. expected: %v, actual: %v", i+1, e, a)
 		}
 	}

pkg/master/master_test.go

@@ -47,7 +47,6 @@ import (
 	openapigen "k8s.io/kubernetes/pkg/generated/openapi"
 	"k8s.io/kubernetes/pkg/genericapiserver"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
-	ipallocator "k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
 	"k8s.io/kubernetes/pkg/runtime"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
@@ -150,10 +149,7 @@ func TestNew(t *testing.T) {
 	defer etcdserver.Terminate(t)
 
 	// these values get defaulted
-	_, serviceClusterIPRange, _ := net.ParseCIDR("10.0.0.0/24")
-	serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
 	assert.Equal(master.GenericAPIServer.MasterCount, 1)
-	assert.Equal(master.GenericAPIServer.ServiceReadWriteIP, serviceReadWriteIP)
 }
 
 // TestVersion tests /version