From b6621c618f2a34cba9a1d8e83ab8b7f7cfba2d4a Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Sat, 27 Oct 2018 21:01:48 +0200 Subject: [PATCH] consul: move build route command logic to separate object ... and finally add some tests. --- registry/consul/parse.go | 49 ------ registry/consul/routecmd.go | 140 ++++++++++++++++++ .../{parse_test.go => routecmd_test.go} | 54 ++++++- registry/consul/service.go | 76 +--------- 4 files changed, 201 insertions(+), 118 deletions(-) delete mode 100644 registry/consul/parse.go create mode 100644 registry/consul/routecmd.go rename registry/consul/{parse_test.go => routecmd_test.go} (64%) diff --git a/registry/consul/parse.go b/registry/consul/parse.go deleted file mode 100644 index e34caa755..000000000 --- a/registry/consul/parse.go +++ /dev/null @@ -1,49 +0,0 @@ -package consul - -import ( - "log" - "os" - "strings" -) - -// 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 -} diff --git a/registry/consul/routecmd.go b/registry/consul/routecmd.go new file mode 100644 index 000000000..5281753bf --- /dev/null +++ b/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=,", 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 +} diff --git a/registry/consul/parse_test.go b/registry/consul/routecmd_test.go similarity index 64% rename from registry/consul/parse_test.go rename to registry/consul/routecmd_test.go index 6c3054a02..2c906e476 100644 --- a/registry/consul/parse_test.go +++ b/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-" diff --git a/registry/consul/service.go b/registry/consul/service.go index 5506ae7c2..f7d703307 100644 --- a/registry/consul/service.go +++ b/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=,", 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 }