Add ServiceUnitKey and ServiceAliasConfigKey types

Define and use the ServiceUnitKey and ServiceAliasConfigKey types for key
values of maps of ServiceUnit values and of ServiceAliasConfig values,
respectively, in order to make code dealing with such maps clearer and
reduce risk of mixing key values.

* pkg/router/template/configmanager/haproxy/backend.go (Backend): Use
ServiceAliasConfigKey for the name.
(buildHAProxyBackends, newBackend, Name, ApplyChanges): Adapt to use the
new ServiceAliasConfigKey type.
* pkg/router/template/configmanager/haproxy/blueprint_plugin_test.go
(fakeConfigManager): Use ServiceAliasConfigKey for the blueprints map.
(newFakeConfigManager, FindBlueprint, Register, AddRoute, RemoveRoute)
(ReplaceRouteEndpoints, RemoveRouteEndpoints, routeKey):
* pkg/router/template/configmanager/haproxy/client.go (FindBackend):
* pkg/router/template/configmanager/haproxy/client_test.go
(TestClientCommit, TestClientBackends): Adapt to use the
ServiceAliasConfigKey type.
* pkg/router/template/configmanager/haproxy/manager.go (configEntryMap):
Use the ServiceAliasConfigKey type for the map's values.
(routeBackendEntry): Use the ServiceAliasConfigKey type for backendName and
the poolRouteBackendName, backendEntries, and poolUsage maps' keys.
(NewHAProxyConfigManager, Register, AddRoute, RemoveRoute)
(ReplaceRouteEndpoints, RemoveRouteEndpoints, findFreeBackendPoolSlot)
(reset, BackendName, BuildMapAssociations, routeBackendName)
(applyMapAssociations):
* pkg/router/template/configmanager/haproxy/map.go (Add, addEntry):
* pkg/router/template/configmanager/haproxy/map_test.go
(TestHAProxyMapAdd): Adapt to use the ServiceAliasConfigKey type.
* pkg/router/template/fake.go (NewFakeTemplateRouter): Adapt to use
ServiceUnitKey and ServiceAliasConfigKey.
* pkg/router/template/plugin.go (RouterInterface, endpointsKey)
(endpointsKeyFromParts, getPartsFromEndpointsKey): Adapt to use
ServiceUnitKey.
* pkg/router/template/plugin_test.go (TestRouter, newTestRouter)
(CreateServiceUnit, FindServiceUnit, AddEndpoints, DeleteEndpoints)
(calculateServiceWeights, FilterNamespaces, getKey, TestHandleEndpoints)
(TestHandleTCPEndpoints, TestHandleRouteExtendedValidation)
(TestHandleRoute, TestNamespaceScopingFromEmpty):
* pkg/router/template/router.go (templateRouter, templateData)
(newTemplateRouter, readState, FilterNamespaces, CreateServiceUnit)
(createServiceUnitInternal, findMatchingServiceUnit, FindServiceUnit)
(DeleteServiceUnit, addServiceAliasAssociation)
(removeServiceAliasAssociation, dynamicallyAddRoute)
(dynamicallyRemoveRoute, dynamicallyReplaceEndpoints, DeleteEndpoints)
(routeKey, routeKeyFromParts, getPartsFromRouteKey)
(createServiceAliasConfig, numberOfEndpoints, AddEndpoints)
(getServiceUnits, getActiveEndpoints, calculateServiceWeights):
* pkg/router/template/router_test.go (TestCreateServiceUnit)
(TestDeleteServiceUnit, TestAddEndpoints, TestAddEndpointDuplicates)
(TestDeleteEndpoints, TestCreateServiceAliasConfig, TestAddRoute)
(TestFilterNamespaces):
* pkg/router/template/template_helper.go (generateHAProxyCertConfigMap)
(getHTTPAliasesGroupedByHost, generateHAProxyMap):
* pkg/router/template/template_helper_test.go (buildTestTemplateState)
(TestGenerateHAProxyCertConfigMap, TestGenerateHAProxyMap)
(TestGetHTTPAliasesGroupedByHost): Adapt to use ServiceAliasConfigKey and
ServiceUnitKey.
* pkg/router/template/types.go (ServiceUnit): Use ServiceAliasConfigKey for
the ServiceAliasAssociations map.
(ServiceUnitKey): New type.
(ServiceAliasConfig): Use ServiceUnitKey for the ServiceUnits and
ServiceUnitNames maps.
(ServiceAliasConfigKey): New type.
(ConfigManager): Adapt to use ServiceAliasConfigKey.

pkg/router/template/configmanager/haproxy/backend.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+
+	templaterouter "github.com/openshift/router/pkg/router/template"
 )
 
 // BackendServerState indicates the state for a haproxy backend server.
@@ -86,7 +88,7 @@ type BackendServerInfo struct {
 
 // Backend represents a specific haproxy backend.
 type Backend struct {
-	name    string
+	name    templaterouter.ServiceAliasConfigKey
 	servers map[string]*backendServer
 
 	client *Client
@@ -113,14 +115,14 @@ func buildHAProxyBackends(c *Client) ([]*Backend, error) {
 
 	backends := make([]*Backend, len(entries))
 	for k, v := range entries {
-		backends[k] = newBackend(v.Name, c)
+		backends[k] = newBackend(templaterouter.ServiceAliasConfigKey(v.Name), c)
 	}
 
 	return backends, nil
 }
 
 // newBackend returns a new Backend representing a haproxy backend.
-func newBackend(name string, c *Client) *Backend {
+func newBackend(name templaterouter.ServiceAliasConfigKey, c *Client) *Backend {
 	return &Backend{
 		name:    name,
 		servers: make(map[string]*backendServer),
@@ -129,7 +131,7 @@ func newBackend(name string, c *Client) *Backend {
 }
 
 // Name returns the name of this haproxy backend.
-func (b *Backend) Name() string {
+func (b *Backend) Name() templaterouter.ServiceAliasConfigKey {
 	return b.name
 }
 
@@ -317,11 +319,11 @@ func newBackendServer(info BackendServerInfo) *backendServer {
 }
 
 // ApplyChanges applies all the local backend server changes.
-func (s *backendServer) ApplyChanges(backendName string, client *Client) error {
+func (s *backendServer) ApplyChanges(backendName templaterouter.ServiceAliasConfigKey, client *Client) error {
 	// Build the haproxy dynamic config API commands.
 	commands := []string{}
 
-	cmdPrefix := fmt.Sprintf("%s %s/%s", SetServerCommand, backendName, s.Name)
+	cmdPrefix := fmt.Sprintf("%s %s/%s", SetServerCommand, string(backendName), s.Name)
 
 	if s.updatedIPAddress != s.IPAddress || s.updatedPort != s.Port {
 		cmd := fmt.Sprintf("%s addr %s", cmdPrefix, s.updatedIPAddress)

pkg/router/template/configmanager/haproxy/blueprint_plugin_test.go

@@ -15,12 +15,12 @@ import (
 )
 
 type fakeConfigManager struct {
-	blueprints map[string]*routev1.Route
+	blueprints map[templaterouter.ServiceAliasConfigKey]*routev1.Route
 }
 
 func newFakeConfigManager() *fakeConfigManager {
 	return &fakeConfigManager{
-		blueprints: make(map[string]*routev1.Route),
+		blueprints: make(map[templaterouter.ServiceAliasConfigKey]*routev1.Route),
 	}
 }
 
@@ -36,27 +36,27 @@ func (cm *fakeConfigManager) RemoveBlueprint(route *routev1.Route) {
 	delete(cm.blueprints, routeKey(route))
 }
 
-func (cm *fakeConfigManager) FindBlueprint(id string) (*routev1.Route, bool) {
+func (cm *fakeConfigManager) FindBlueprint(id templaterouter.ServiceAliasConfigKey) (*routev1.Route, bool) {
 	route, ok := cm.blueprints[id]
 	return route, ok
 }
 
-func (cm *fakeConfigManager) Register(id string, route *routev1.Route) {
+func (cm *fakeConfigManager) Register(id templaterouter.ServiceAliasConfigKey, route *routev1.Route) {
 }
 
-func (cm *fakeConfigManager) AddRoute(id, routingKey string, route *routev1.Route) error {
+func (cm *fakeConfigManager) AddRoute(id templaterouter.ServiceAliasConfigKey, routingKey string, route *routev1.Route) error {
 	return nil
 }
 
-func (cm *fakeConfigManager) RemoveRoute(id string, route *routev1.Route) error {
+func (cm *fakeConfigManager) RemoveRoute(id templaterouter.ServiceAliasConfigKey, route *routev1.Route) error {
 	return nil
 }
 
-func (cm *fakeConfigManager) ReplaceRouteEndpoints(id string, oldEndpoints, newEndpoints []templaterouter.Endpoint, weight int32) error {
+func (cm *fakeConfigManager) ReplaceRouteEndpoints(id templaterouter.ServiceAliasConfigKey, oldEndpoints, newEndpoints []templaterouter.Endpoint, weight int32) error {
 	return nil
 }
 
-func (cm *fakeConfigManager) RemoveRouteEndpoints(id string, endpoints []templaterouter.Endpoint) error {
+func (cm *fakeConfigManager) RemoveRouteEndpoints(id templaterouter.ServiceAliasConfigKey, endpoints []templaterouter.Endpoint) error {
 	return nil
 }
 
@@ -75,8 +75,8 @@ func (cm *fakeConfigManager) GenerateDynamicServerNames(id string) []string {
 	return []string{}
 }
 
-func routeKey(route *routev1.Route) string {
-	return fmt.Sprintf("%s:%s", route.Name, route.Namespace)
+func routeKey(route *routev1.Route) templaterouter.ServiceAliasConfigKey {
+	return templaterouter.ServiceAliasConfigKey(fmt.Sprintf("%s:%s", route.Name, route.Namespace))
 }
 
 // TestHandleRoute test route watch events

pkg/router/template/configmanager/haproxy/client.go

@@ -6,6 +6,8 @@ import (
 	"strings"
 	"time"
 
+	templaterouter "github.com/openshift/router/pkg/router/template"
+
 	haproxy "github.com/bcicen/go-haproxy"
 
 	utilwait "k8s.io/apimachinery/pkg/util/wait"
@@ -107,7 +109,7 @@ func (c *Client) Backends() ([]*Backend, error) {
 }
 
 // FindBackend returns a specific haproxy backend if it is configured.
-func (c *Client) FindBackend(id string) (*Backend, error) {
+func (c *Client) FindBackend(id templaterouter.ServiceAliasConfigKey) (*Backend, error) {
 	if _, err := c.Backends(); err != nil {
 		return nil, err
 	}

pkg/router/template/configmanager/haproxy/client_test.go

@@ -3,6 +3,7 @@ package haproxy
 import (
 	"testing"
 
+	templaterouter "github.com/openshift/router/pkg/router/template"
 	haproxytesting "github.com/openshift/router/pkg/router/template/configmanager/haproxy/testing"
 )
 
@@ -406,7 +407,7 @@ func TestClientCommit(t *testing.T) {
 
 	server.Reset()
 	for _, be := range backends {
-		if _, ok := skipNames[be.Name()]; ok {
+		if _, ok := skipNames[string(be.Name())]; ok {
 			continue
 		}
 
@@ -423,7 +424,7 @@ func TestClientCommit(t *testing.T) {
 
 	server.Reset()
 	for _, be := range backends {
-		if _, ok := skipNames[be.Name()]; ok {
+		if _, ok := skipNames[string(be.Name())]; ok {
 			continue
 		}
 
@@ -458,7 +459,7 @@ func TestClientBackends(t *testing.T) {
 func TestClientFindBackend(t *testing.T) {
 	testCases := []struct {
 		name            string
-		backendName     string
+		backendName     templaterouter.ServiceAliasConfigKey
 		failureExpected bool
 	}{
 		{

pkg/router/template/configmanager/haproxy/manager.go

@@ -67,7 +67,7 @@ type endpointToDynamicServerMap map[string]string
 
 // configEntryMap is a map containing name-value pairs representing the
 // config entries to add to an haproxy map.
-type configEntryMap map[string]string
+type configEntryMap map[string]templaterouter.ServiceAliasConfigKey
 
 // haproxyMapAssociation is a map of haproxy maps and their config entries for a backend.
 type haproxyMapAssociation map[string]configEntryMap
@@ -84,15 +84,15 @@ type routeBackendEntry struct {
 	wildcard bool
 
 	// BackendName is the name of the associated haproxy backend.
-	backendName string
+	backendName templaterouter.ServiceAliasConfigKey
 
 	// mapAssociations is the associated set of haproxy maps and their
 	// config entries.
 	mapAssociations haproxyMapAssociation
 
 	// poolRouteBackendName is backend name for any associated route
 	// from the pre-configured blueprint route pool.
-	poolRouteBackendName string
+	poolRouteBackendName templaterouter.ServiceAliasConfigKey
 
 	// DynamicServerMap is a map of all the allocated dynamic servers.
 	dynamicServerMap endpointToDynamicServerMap
@@ -140,11 +140,11 @@ type haproxyConfigManager struct {
 	reloadInProgress bool
 
 	// backendEntries is a map of route id to the route backend entry.
-	backendEntries map[string]*routeBackendEntry
+	backendEntries map[templaterouter.ServiceAliasConfigKey]*routeBackendEntry
 
 	// poolUsage is a mapping of blueprint route pool entries to their
 	// corresponding routes.
-	poolUsage map[string]string
+	poolUsage map[templaterouter.ServiceAliasConfigKey]templaterouter.ServiceAliasConfigKey
 
 	// lock is a mutex used to prevent concurrent config changes.
 	lock sync.Mutex
@@ -171,8 +171,8 @@ func NewHAProxyConfigManager(options templaterouter.ConfigManagerOptions) *hapro
 
 		client:           client,
 		reloadInProgress: false,
-		backendEntries:   make(map[string]*routeBackendEntry),
-		poolUsage:        make(map[string]string),
+		backendEntries:   make(map[templaterouter.ServiceAliasConfigKey]*routeBackendEntry),
+		poolUsage:        make(map[templaterouter.ServiceAliasConfigKey]templaterouter.ServiceAliasConfigKey),
 	}
 }
 
@@ -284,10 +284,10 @@ func (cm *haproxyConfigManager) RemoveBlueprint(route *routev1.Route) {
 }
 
 // Register registers an id with an expected haproxy backend for a route.
-func (cm *haproxyConfigManager) Register(id string, route *routev1.Route) {
+func (cm *haproxyConfigManager) Register(id templaterouter.ServiceAliasConfigKey, route *routev1.Route) {
 	wildcard := cm.wildcardRoutesAllowed && (route.Spec.WildcardPolicy == routev1.WildcardPolicySubdomain)
 	entry := &routeBackendEntry{
-		id:               id,
+		id:               string(id),
 		termination:      routeTerminationType(route),
 		wildcard:         wildcard,
 		backendName:      routeBackendName(id, route),
@@ -302,7 +302,7 @@ func (cm *haproxyConfigManager) Register(id string, route *routev1.Route) {
 }
 
 // AddRoute adds a new route or updates an existing route.
-func (cm *haproxyConfigManager) AddRoute(id, routingKey string, route *routev1.Route) error {
+func (cm *haproxyConfigManager) AddRoute(id templaterouter.ServiceAliasConfigKey, routingKey string, route *routev1.Route) error {
 	if cm.isReloading() {
 		return fmt.Errorf("Router reload in progress, cannot dynamically add route %s", id)
 	}
@@ -367,7 +367,7 @@ func (cm *haproxyConfigManager) AddRoute(id, routingKey string, route *routev1.R
 }
 
 // RemoveRoute removes a route.
-func (cm *haproxyConfigManager) RemoveRoute(id string, route *routev1.Route) error {
+func (cm *haproxyConfigManager) RemoveRoute(id templaterouter.ServiceAliasConfigKey, route *routev1.Route) error {
 	log.V(4).Info("removing route", "id", id)
 	if cm.isReloading() {
 		return fmt.Errorf("Router reload in progress, cannot dynamically remove route id %s", id)
@@ -422,7 +422,7 @@ func (cm *haproxyConfigManager) RemoveRoute(id string, route *routev1.Route) err
 
 // ReplaceRouteEndpoints dynamically replaces a subset of the endpoints for
 // a route - modifies a subset of the servers on an haproxy backend.
-func (cm *haproxyConfigManager) ReplaceRouteEndpoints(id string, oldEndpoints, newEndpoints []templaterouter.Endpoint, weight int32) error {
+func (cm *haproxyConfigManager) ReplaceRouteEndpoints(id templaterouter.ServiceAliasConfigKey, oldEndpoints, newEndpoints []templaterouter.Endpoint, weight int32) error {
 	log.V(4).Info("replacing route endpoints", "id", id, "weight", weight)
 	if cm.isReloading() {
 		return fmt.Errorf("Router reload in progress, cannot dynamically add endpoints for %s", id)
@@ -562,7 +562,7 @@ func (cm *haproxyConfigManager) ReplaceRouteEndpoints(id string, oldEndpoints, n
 }
 
 // RemoveRouteEndpoints removes servers matching the endpoints from a haproxy backend.
-func (cm *haproxyConfigManager) RemoveRouteEndpoints(id string, endpoints []templaterouter.Endpoint) error {
+func (cm *haproxyConfigManager) RemoveRouteEndpoints(id templaterouter.ServiceAliasConfigKey, endpoints []templaterouter.Endpoint) error {
 	log.V(4).Info("removing endpoints", "id", id)
 	if cm.isReloading() {
 		return fmt.Errorf("Router reload in progress, cannot dynamically delete endpoints for %s", id)
@@ -759,11 +759,11 @@ func (cm *haproxyConfigManager) processMapAssociations(associations haproxyMapAs
 }
 
 // findFreeBackendPoolSlot returns a free pool slot backend name.
-func (cm *haproxyConfigManager) findFreeBackendPoolSlot(blueprint *routev1.Route) (string, error) {
+func (cm *haproxyConfigManager) findFreeBackendPoolSlot(blueprint *routev1.Route) (templaterouter.ServiceAliasConfigKey, error) {
 	poolSize := getPoolSize(blueprint, cm.blueprintRoutePoolSize)
 	idPrefix := fmt.Sprintf("%s:%s", blueprint.Namespace, blueprint.Name)
 	for i := 0; i < poolSize; i++ {
-		id := fmt.Sprintf("%s-%v", idPrefix, i+1)
+		id := templaterouter.ServiceAliasConfigKey(fmt.Sprintf("%s-%v", idPrefix, i+1))
 		name := routeBackendName(id, blueprint)
 		if _, ok := cm.poolUsage[name]; !ok {
 			return name, nil
@@ -797,7 +797,7 @@ func (cm *haproxyConfigManager) reset() {
 
 	// Reset the blueprint route pool use and dynamic server maps as
 	// the router was reloaded.
-	cm.poolUsage = make(map[string]string)
+	cm.poolUsage = make(map[templaterouter.ServiceAliasConfigKey]templaterouter.ServiceAliasConfigKey)
 	for _, entry := range cm.backendEntries {
 		entry.poolRouteBackendName = ""
 		if len(entry.dynamicServerMap) > 0 {
@@ -858,7 +858,7 @@ func (cm *haproxyConfigManager) findMatchingBlueprint(route *routev1.Route) *rou
 }
 
 // BackendName returns the associated backend name for a route.
-func (entry *routeBackendEntry) BackendName() string {
+func (entry *routeBackendEntry) BackendName() templaterouter.ServiceAliasConfigKey {
 	if len(entry.poolRouteBackendName) > 0 {
 		return entry.poolRouteBackendName
 	}
@@ -875,7 +875,7 @@ func (entry *routeBackendEntry) BuildMapAssociations(route *routev1.Route) {
 	}
 
 	entry.mapAssociations = make(haproxyMapAssociation)
-	associate := func(name, k, v string) {
+	associate := func(name, k string, v templaterouter.ServiceAliasConfigKey) {
 		m, ok := entry.mapAssociations[name]
 		if !ok {
 			m = make(configEntryMap)
@@ -1002,10 +1002,10 @@ func createBlueprintRoute(routeType routev1.TLSTerminationType) *routev1.Route {
 }
 
 // routeBackendName returns the haproxy backend name for a route.
-func routeBackendName(id string, route *routev1.Route) string {
+func routeBackendName(id templaterouter.ServiceAliasConfigKey, route *routev1.Route) templaterouter.ServiceAliasConfigKey {
 	termination := routeTerminationType(route)
 	prefix := templateutil.GenerateBackendNamePrefix(termination)
-	return fmt.Sprintf("%s:%s", prefix, id)
+	return templaterouter.ServiceAliasConfigKey(fmt.Sprintf("%s:%s", prefix, string(id)))
 }
 
 // getPoolSize returns the size to allocate for the pool for the specified
@@ -1045,7 +1045,7 @@ func isDynamicBackendServer(server BackendServerInfo) bool {
 }
 
 // applyMapAssociations applies the backend associations to a haproxy map.
-func applyMapAssociations(m *HAProxyMap, associations map[string]string, add bool) error {
+func applyMapAssociations(m *HAProxyMap, associations configEntryMap, add bool) error {
 	for k, v := range associations {
 		log.V(4).Info("applying to map", "name", m.Name(), "key", k, "value", v, "add", add)
 		if add {

pkg/router/template/configmanager/haproxy/map.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+
+	templaterouter "github.com/openshift/router/pkg/router/template"
 )
 
 const (
@@ -133,7 +135,7 @@ func (m *HAProxyMap) Find(k string) ([]HAProxyMapEntry, error) {
 
 // Add adds a new key and value to the haproxy map and allows all previous
 // entries in the map to be deleted (replaced).
-func (m *HAProxyMap) Add(k, v string, replace bool) error {
+func (m *HAProxyMap) Add(k string, v templaterouter.ServiceAliasConfigKey, replace bool) error {
 	if replace {
 		if err := m.Delete(k); err != nil {
 			return err
@@ -165,8 +167,8 @@ func (m *HAProxyMap) DeleteEntry(id string) error {
 }
 
 // addEntry adds a new haproxy map entry.
-func (m *HAProxyMap) addEntry(k, v string) error {
-	keyExpr := escapeKeyExpr(k)
+func (m *HAProxyMap) addEntry(k string, v templaterouter.ServiceAliasConfigKey) error {
+	keyExpr := escapeKeyExpr(string(k))
 	cmd := fmt.Sprintf("add map %s %s %s", m.name, keyExpr, v)
 	responseBytes, err := m.client.Execute(cmd)
 	if err != nil {

pkg/router/template/configmanager/haproxy/map_test.go

@@ -3,6 +3,7 @@ package haproxy
 import (
 	"testing"
 
+	templaterouter "github.com/openshift/router/pkg/router/template"
 	haproxytesting "github.com/openshift/router/pkg/router/template/configmanager/haproxy/testing"
 )
 
@@ -451,7 +452,7 @@ func TestHAProxyMapAdd(t *testing.T) {
 		sockFile        string
 		mapName         string
 		keyName         string
-		value           string
+		value           templaterouter.ServiceAliasConfigKey
 		replace         bool
 		failureExpected bool
 	}{

pkg/router/template/fake.go

@@ -5,8 +5,8 @@ package templaterouter
 func NewFakeTemplateRouter() *templateRouter {
 	fakeCertManager, _ := newSimpleCertificateManager(newFakeCertificateManagerConfig(), &fakeCertWriter{})
 	return &templateRouter{
-		state:                     map[string]ServiceAliasConfig{},
-		serviceUnits:              make(map[string]ServiceUnit),
+		state:                     map[ServiceAliasConfigKey]ServiceAliasConfig{},
+		serviceUnits:              make(map[ServiceUnitKey]ServiceUnit),
 		certManager:               fakeCertManager,
 		rateLimitedCommitFunction: nil,
 	}