Add possibility to use multiple names for one service (#31)

Co-authored-by: Geoff Bourne <itzgeoff@gmail.com>

.github/workflows/test.yml

@@ -21,6 +21,9 @@ jobs:
         with:
           go-version: 1.17
 
+      - name: Test
+        run: go test ./...
+
       - name: Run GoReleaser Snapshot
         uses: goreleaser/goreleaser-action@v2
         with:

README.md

@@ -106,7 +106,7 @@ To test out this example, I added these two entries to my "hosts" file:
 When running `mc-router` as a kubernetes pod and you pass the `--in-kube-cluster` command-line argument, then
 it will automatically watch for any services annotated with 
 - `mc-router.itzg.me/externalServerName` : The value of the annotation will be registered as the external hostname Minecraft clients would used to connect to the
-   routed service. The service's clusterIP and target port are used as the routed backend.
+   routed service. The service's clusterIP and target port are used as the routed backend. You can use more hostnames by splitting them with comma.
 - `mc-router.itzg.me/defaultServer` : The service's clusterIP and target port are used as the default if
   no other `externalServiceName` annotations applies.
 
@@ -129,6 +129,17 @@ metadata:
     "mc-router.itzg.me/externalServerName": "external.host.name"
 ```
 
+you can use multiple host names:
+
+```yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: mc-forge
+  annotations:
+    "mc-router.itzg.me/externalServerName": "external.host.name,other.host.name"
+```
+
 ## Example kubernetes deployment
 
 [This example deployment](docs/k8s-example-auto.yaml) 
@@ -136,7 +147,7 @@ metadata:
 * Declares a service account with access to watch and list services
 * Declares `--in-kube-cluster` in the `mc-router` container arguments
 * Two "backend" Minecraft servers are declared each with an 
-  `"mc-router.itzg.me/externalServerName"` annotation that declares their external server name
+  `"mc-router.itzg.me/externalServerName"` annotation that declares their external server name(s)
 
 ```bash
 kubectl apply -f https://raw.githubusercontent.com/itzg/mc-router/master/docs/k8s-example-auto.yaml

server/k8s.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 	"net"
 	"strconv"
+	"strings"
 )
 
 const (
@@ -66,47 +67,9 @@ func (w *k8sWatcherImpl) startWithLoadedConfig(config *rest.Config) error {
 		&v1.Service{},
 		0,
 		cache.ResourceEventHandlerFuncs{
-			AddFunc: func(obj interface{}) {
+			AddFunc:    w.handleAdd,
-				routableService := extractRoutableService(obj)
+			DeleteFunc: w.handleDelete,
-				if routableService != nil {
+			UpdateFunc: w.handleUpdate,
-					logrus.WithField("routableService", routableService).Debug("ADD")
-
-					if routableService.externalServiceName != "" {
-						Routes.CreateMapping(routableService.externalServiceName, routableService.containerEndpoint)
-					} else {
-						Routes.SetDefaultRoute(routableService.containerEndpoint)
-					}
-				}
-			},
-			DeleteFunc: func(obj interface{}) {
-				routableService := extractRoutableService(obj)
-				if routableService != nil {
-					logrus.WithField("routableService", routableService).Debug("DELETE")
-
-					if routableService.externalServiceName != "" {
-						Routes.DeleteMapping(routableService.externalServiceName)
-					} else {
-						Routes.SetDefaultRoute("")
-					}
-				}
-			},
-			UpdateFunc: func(oldObj, newObj interface{}) {
-				oldRoutableService := extractRoutableService(oldObj)
-				newRoutableService := extractRoutableService(newObj)
-				if oldRoutableService != nil && newRoutableService != nil {
-					logrus.WithFields(logrus.Fields{
-						"old": oldRoutableService,
-						"new": newRoutableService,
-					}).Debug("UPDATE")
-
-					if oldRoutableService.externalServiceName != "" && newRoutableService.externalServiceName != "" {
-						Routes.DeleteMapping(oldRoutableService.externalServiceName)
-						Routes.CreateMapping(newRoutableService.externalServiceName, newRoutableService.containerEndpoint)
-					} else {
-						Routes.SetDefaultRoute(newRoutableService.containerEndpoint)
-					}
-				}
-			},
 		},
 	)
 
@@ -117,6 +80,61 @@ func (w *k8sWatcherImpl) startWithLoadedConfig(config *rest.Config) error {
 	return nil
 }
 
+// oldObj and newObj are expected to be *v1.Service
+func (w *k8sWatcherImpl) handleUpdate(oldObj interface{}, newObj interface{}) {
+	for _, oldRoutableService := range extractRoutableServices(oldObj) {
+		logrus.WithFields(logrus.Fields{
+			"old": oldRoutableService,
+		}).Debug("UPDATE")
+		if oldRoutableService.externalServiceName != "" {
+			Routes.DeleteMapping(oldRoutableService.externalServiceName)
+		}
+	}
+
+	for _, newRoutableService := range extractRoutableServices(newObj) {
+		logrus.WithFields(logrus.Fields{
+			"new": newRoutableService,
+		}).Debug("UPDATE")
+		if newRoutableService.externalServiceName != "" {
+			Routes.CreateMapping(newRoutableService.externalServiceName, newRoutableService.containerEndpoint)
+		} else {
+			Routes.SetDefaultRoute(newRoutableService.containerEndpoint)
+		}
+	}
+}
+
+// obj is expected to be a *v1.Service
+func (w *k8sWatcherImpl) handleDelete(obj interface{}) {
+	routableServices := extractRoutableServices(obj)
+	for _, routableService := range routableServices {
+		if routableService != nil {
+			logrus.WithField("routableService", routableService).Debug("DELETE")
+
+			if routableService.externalServiceName != "" {
+				Routes.DeleteMapping(routableService.externalServiceName)
+			} else {
+				Routes.SetDefaultRoute("")
+			}
+		}
+	}
+}
+
+// obj is expected to be a *v1.Service
+func (w *k8sWatcherImpl) handleAdd(obj interface{}) {
+	routableServices := extractRoutableServices(obj)
+	for _, routableService := range routableServices {
+		if routableService != nil {
+			logrus.WithField("routableService", routableService).Debug("ADD")
+
+			if routableService.externalServiceName != "" {
+				Routes.CreateMapping(routableService.externalServiceName, routableService.containerEndpoint)
+			} else {
+				Routes.SetDefaultRoute(routableService.containerEndpoint)
+			}
+		}
+	}
+}
+
 func (w *k8sWatcherImpl) Stop() {
 	if w.stop != nil {
 		w.stop <- struct{}{}
@@ -128,16 +146,22 @@ type routableService struct {
 	containerEndpoint   string
 }
 
-func extractRoutableService(obj interface{}) *routableService {
+// obj is expected to be a *v1.Service
+func extractRoutableServices(obj interface{}) []*routableService {
 	service, ok := obj.(*v1.Service)
 	if !ok {
 		return nil
 	}
 
+	routableServices := make([]*routableService, 0)
 	if externalServiceName, exists := service.Annotations[AnnotationExternalServerName]; exists {
-		return buildDetails(service, externalServiceName)
+		serviceNames := strings.Split(externalServiceName, ",")
+		for _, serviceName := range serviceNames {
+			routableServices = append(routableServices, buildDetails(service, serviceName))
+		}
+		return routableServices
 	} else if _, exists := service.Annotations[AnnotationDefaultServer]; exists {
-		return buildDetails(service, "")
+		return []*routableService{buildDetails(service, "")}
 	}
 
 	return nil

server/k8s_test.go

@@ -0,0 +1,172 @@
+package server
+
+import (
+	"encoding/json"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	"testing"
+)
+
+func TestK8sWatcherImpl_handleAddThenUpdate(t *testing.T) {
+	type scenario struct {
+		given  string
+		expect string
+	}
+	type svcAndScenarios struct {
+		svc       string
+		scenarios []scenario
+	}
+	tests := []struct {
+		name    string
+		initial svcAndScenarios
+		update  svcAndScenarios
+	}{
+		{
+			name: "a to b",
+			initial: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "a.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: "1.1.1.1:25565"},
+					{given: "b.com", expect: ""},
+				},
+			},
+			update: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "b.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: ""},
+					{given: "b.com", expect: "1.1.1.1:25565"},
+				},
+			},
+		},
+		{
+			name: "a to a,b",
+			initial: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "a.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: "1.1.1.1:25565"},
+					{given: "b.com", expect: ""},
+				},
+			},
+			update: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "a.com,b.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: "1.1.1.1:25565"},
+					{given: "b.com", expect: "1.1.1.1:25565"},
+				},
+			},
+		},
+		{
+			name: "a,b to b",
+			initial: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "a.com,b.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: "1.1.1.1:25565"},
+					{given: "b.com", expect: "1.1.1.1:25565"},
+				},
+			},
+			update: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "b.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: ""},
+					{given: "b.com", expect: "1.1.1.1:25565"},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// reset the routes
+			Routes.RegisterAll(map[string]string{})
+
+			watcher := &k8sWatcherImpl{}
+			initialSvc := v1.Service{}
+			err := json.Unmarshal([]byte(test.initial.svc), &initialSvc)
+			require.NoError(t, err)
+
+			watcher.handleAdd(&initialSvc)
+			for _, s := range test.initial.scenarios {
+				backend, _ := Routes.FindBackendForServerAddress(s.given)
+				assert.Equal(t, s.expect, backend, "initial: given=%s", s.given)
+			}
+
+			updatedSvc := v1.Service{}
+			err = json.Unmarshal([]byte(test.update.svc), &updatedSvc)
+			require.NoError(t, err)
+
+			watcher.handleUpdate(&initialSvc, &updatedSvc)
+			for _, s := range test.update.scenarios {
+				backend, _ := Routes.FindBackendForServerAddress(s.given)
+				assert.Equal(t, s.expect, backend, "update: given=%s", s.given)
+			}
+		})
+	}
+}
+
+func TestK8sWatcherImpl_handleAddThenDelete(t *testing.T) {
+	type scenario struct {
+		given  string
+		expect string
+	}
+	type svcAndScenarios struct {
+		svc       string
+		scenarios []scenario
+	}
+	tests := []struct {
+		name    string
+		initial svcAndScenarios
+		delete  []scenario
+	}{
+		{
+			name: "single",
+			initial: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "a.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: "1.1.1.1:25565"},
+					{given: "b.com", expect: ""},
+				},
+			},
+			delete: []scenario{
+				{given: "a.com", expect: ""},
+				{given: "b.com", expect: ""},
+			},
+		},
+		{
+			name: "multi",
+			initial: svcAndScenarios{
+				svc: ` {"metadata": {"annotations": {"mc-router.itzg.me/externalServerName": "a.com,b.com"}}, "spec":{"clusterIP": "1.1.1.1"}}`,
+				scenarios: []scenario{
+					{given: "a.com", expect: "1.1.1.1:25565"},
+					{given: "b.com", expect: "1.1.1.1:25565"},
+				},
+			},
+			delete: []scenario{
+				{given: "a.com", expect: ""},
+				{given: "b.com", expect: ""},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// reset the routes
+			Routes.RegisterAll(map[string]string{})
+
+			watcher := &k8sWatcherImpl{}
+			initialSvc := v1.Service{}
+			err := json.Unmarshal([]byte(test.initial.svc), &initialSvc)
+			require.NoError(t, err)
+
+			watcher.handleAdd(&initialSvc)
+			for _, s := range test.initial.scenarios {
+				backend, _ := Routes.FindBackendForServerAddress(s.given)
+				assert.Equal(t, s.expect, backend, "initial: given=%s", s.given)
+			}
+
+			watcher.handleDelete(&initialSvc)
+			for _, s := range test.delete {
+				backend, _ := Routes.FindBackendForServerAddress(s.given)
+				assert.Equal(t, s.expect, backend, "update: given=%s", s.given)
+			}
+		})
+	}
+}

server/routes.go

@@ -87,6 +87,7 @@ type IRoutes interface {
 	RegisterAll(mappings map[string]string)
 	// FindBackendForServerAddress returns the host:port for the external server address, if registered.
 	// Otherwise, an empty string is returned
+	// Also returns the normalized version of the given serverAddress
 	FindBackendForServerAddress(serverAddress string) (string, string)
 	GetMappings() map[string]string
 	DeleteMapping(serverAddress string) bool