consul: move build route command logic to separate object

... and finally add some tests.

registry/consul/routecmd.go

@@ -0,0 +1,140 @@
+package consul
+
+import (
+	"fmt"
+	"log"
+	"net"
+	"os"
+	"runtime"
+	"strconv"
+	"strings"
+
+	"github.com/hashicorp/consul/api"
+)
+
+// routecmd builds a route command.
+type routecmd struct {
+	// svc is the consul service instance.
+	svc *api.CatalogService
+
+	// prefix is the prefix of urlprefix tags. e.g. 'urlprefix-'.
+	prefix string
+
+	env map[string]string
+}
+
+func (r routecmd) build() []string {
+	var svctags, routetags []string
+	for _, t := range r.svc.ServiceTags {
+		if strings.HasPrefix(t, r.prefix) {
+			routetags = append(routetags, t)
+		} else {
+			svctags = append(svctags, t)
+		}
+	}
+
+	// generate route commands
+	var config []string
+	for _, tag := range routetags {
+		if route, opts, ok := parseURLPrefixTag(tag, r.prefix, r.env); ok {
+			name, addr, port := r.svc.ServiceName, r.svc.ServiceAddress, r.svc.ServicePort
+
+			// use consul node address if service address is not set
+			if addr == "" {
+				addr = r.svc.Address
+			}
+
+			// add .local suffix on OSX for simple host names w/o domain
+			if runtime.GOOS == "darwin" && !strings.Contains(addr, ".") && !strings.HasSuffix(addr, ".local") {
+				addr += ".local"
+			}
+
+			addr = net.JoinHostPort(addr, strconv.Itoa(port))
+			//tags := strings.Join(r.tags, ",")
+			dst := "http://" + addr + "/"
+
+			var weight string
+			var ropts []string
+			for _, o := range strings.Fields(opts) {
+				switch {
+				case o == "proto=tcp":
+					dst = "tcp://" + addr
+
+				case o == "proto=https":
+					dst = "https://" + addr
+
+				case strings.HasPrefix(o, "weight="):
+					weight = o[len("weight="):]
+
+				case strings.HasPrefix(o, "redirect="):
+					redir := strings.Split(o[len("redirect="):], ",")
+					if len(redir) == 2 {
+						dst = redir[1]
+						ropts = append(ropts, fmt.Sprintf("redirect=%s", redir[0]))
+					} else {
+						log.Printf("[ERROR] Invalid syntax for redirect: %s. should be redirect=<code>,<url>", o)
+						continue
+					}
+				default:
+					ropts = append(ropts, o)
+				}
+			}
+
+			cfg := "route add " + name + " " + route + " " + dst
+			if weight != "" {
+				cfg += " weight " + weight
+			}
+			if len(svctags) > 0 {
+				cfg += " tags " + strconv.Quote(strings.Join(svctags, ","))
+			}
+			if len(ropts) > 0 {
+				cfg += " opts " + strconv.Quote(strings.Join(ropts, " "))
+			}
+
+			config = append(config, cfg)
+		}
+	}
+	return config
+}
+
+// parseURLPrefixTag expects an input in the form of 'tag-host/path[ opts]'
+// and returns the lower cased host and the unaltered path if the
+// prefix matches the tag.
+func parseURLPrefixTag(s, prefix string, env map[string]string) (route, opts string, ok bool) {
+	// expand $x or ${x} to env[x] or ""
+	expand := func(s string) string {
+		return os.Expand(s, func(x string) string {
+			if env == nil {
+				return ""
+			}
+			return env[x]
+		})
+	}
+
+	s = strings.TrimSpace(s)
+	if !strings.HasPrefix(s, prefix) {
+		return "", "", false
+	}
+	s = strings.TrimSpace(s[len(prefix):])
+
+	p := strings.SplitN(s, " ", 2)
+	if len(p) == 2 {
+		opts = p[1]
+	}
+	s = p[0]
+
+	// prefix is ":port"
+	if strings.HasPrefix(s, ":") {
+		return s, opts, true
+	}
+
+	// prefix is "host/path"
+	p = strings.SplitN(s, "/", 2)
+	if len(p) == 1 {
+		log.Printf("[WARN] consul: Invalid %s tag %q - You need to have a trailing slash!", prefix, s)
+		return "", "", false
+	}
+	host, path := p[0], p[1]
+
+	return strings.ToLower(expand(host)) + "/" + expand(path), opts, true
+}

registry/consul/parse_test.go → registry/consul/routecmd_test.go

@@ -1,6 +1,58 @@
 package consul
 
-import "testing"
+import (
+	"reflect"
+	"testing"
+
+	"github.com/hashicorp/consul/api"
+)
+
+func TestRouteCmd(t *testing.T) {
+	cases := []struct {
+		name string
+		r    routecmd
+		cfg  []string
+	}{
+		{
+			name: "http",
+			r: routecmd{
+				prefix: "p-",
+				svc: &api.CatalogService{
+					ServiceName:    "svc-1",
+					ServiceAddress: "1.1.1.1",
+					ServicePort:    2222,
+					ServiceTags:    []string{`p-foo/bar`},
+				},
+			},
+			cfg: []string{
+				`route add svc-1 foo/bar http://1.1.1.1:2222/`,
+			},
+		},
+		{
+			name: "tcp",
+			r: routecmd{
+				prefix: "p-",
+				svc: &api.CatalogService{
+					ServiceName:    "svc-1",
+					ServiceAddress: "1.1.1.1",
+					ServicePort:    2222,
+					ServiceTags:    []string{`p-:1234 proto=tcp`},
+				},
+			},
+			cfg: []string{
+				`route add svc-1 :1234 tcp://1.1.1.1:2222`,
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got, want := c.r.build(), c.cfg; !reflect.DeepEqual(got, want) {
+				t.Fatalf("\ngot  %#v\nwant %#v", got, want)
+			}
+		})
+	}
+}
 
 func TestParseTag(t *testing.T) {
 	prefix := "p-"

registry/consul/service.go

@@ -3,10 +3,7 @@ package consul
 import (
 	"fmt"
 	"log"
-	"net"
-	"runtime"
 	"sort"
-	"strconv"
 	"strings"
 	"time"
 
@@ -104,76 +101,19 @@ func (w *ServiceMonitor) serviceConfig(name string, passing map[string]bool) (co
 	}
 
 	for _, svc := range svcs {
-		// check if the instance is in the list of instances
-		// which passed the health check
-		if _, ok := passing[fmt.Sprintf("%s.%s", svc.Node, svc.ServiceID)]; !ok {
+		// check if this instance passed the health check
+		if _, ok := passing[svc.Node+"."+svc.ServiceID]; !ok {
 			continue
 		}
 
-		// get all tags which do not have the tag prefix
-		var svctags []string
-		for _, tag := range svc.ServiceTags {
-			if !strings.HasPrefix(tag, w.config.TagPrefix) {
-				svctags = append(svctags, tag)
-			}
+		r := routecmd{
+			svc:    svc,
+			env:    env,
+			prefix: w.config.TagPrefix,
 		}
+		cmds := r.build()
 
-		// generate route commands
-		for _, tag := range svc.ServiceTags {
-			if route, opts, ok := parseURLPrefixTag(tag, w.config.TagPrefix, env); ok {
-				name, addr, port := svc.ServiceName, svc.ServiceAddress, svc.ServicePort
-
-				// use consul node address if service address is not set
-				if addr == "" {
-					addr = svc.Address
-				}
-
-				// add .local suffix on OSX for simple host names w/o domain
-				if runtime.GOOS == "darwin" && !strings.Contains(addr, ".") && !strings.HasSuffix(addr, ".local") {
-					addr += ".local"
-				}
-
-				// build route command
-				weight := ""
-				ropts := []string{}
-				tags := strings.Join(svctags, ",")
-				addr = net.JoinHostPort(addr, strconv.Itoa(port))
-				dst := "http://" + addr + "/"
-				for _, o := range strings.Fields(opts) {
-					switch {
-					case o == "proto=tcp":
-						dst = "tcp://" + addr
-					case o == "proto=https":
-						dst = "https://" + addr
-					case strings.HasPrefix(o, "weight="):
-						weight = o[len("weight="):]
-					case strings.HasPrefix(o, "redirect="):
-						redir := strings.Split(o[len("redirect="):], ",")
-						if len(redir) == 2 {
-							dst = redir[1]
-							ropts = append(ropts, fmt.Sprintf("redirect=%s", redir[0]))
-						} else {
-							log.Printf("[ERROR] Invalid syntax for redirect: %s. should be redirect=<code>,<url>", o)
-							continue
-						}
-					default:
-						ropts = append(ropts, o)
-					}
-				}
-
-				cfg := "route add " + name + " " + route + " " + dst
-				if weight != "" {
-					cfg += " weight " + weight
-				}
-				if tags != "" {
-					cfg += " tags " + strconv.Quote(tags)
-				}
-				if len(ropts) > 0 {
-					cfg += " opts " + strconv.Quote(strings.Join(ropts, " "))
-				}
-				config = append(config, cfg)
-			}
-		}
+		config = append(config, cmds...)
 	}
 	return config
 }