From c9e225d2df96a87050f78b2d5e32961fdc471780 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 7 Aug 2019 10:27:02 -0700 Subject: [PATCH 1/4] sd/dnssrv: fix Instancer method receivers --- sd/dnssrv/instancer.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/sd/dnssrv/instancer.go b/sd/dnssrv/instancer.go index bed2cb5be..652fbc977 100644 --- a/sd/dnssrv/instancer.go +++ b/sd/dnssrv/instancer.go @@ -57,31 +57,31 @@ func NewInstancerDetailed( } // Stop terminates the Instancer. -func (p *Instancer) Stop() { - close(p.quit) +func (in *Instancer) Stop() { + close(in.quit) } -func (p *Instancer) loop(t *time.Ticker, lookup Lookup) { +func (in *Instancer) loop(t *time.Ticker, lookup Lookup) { defer t.Stop() for { select { case <-t.C: - instances, err := p.resolve(lookup) + instances, err := in.resolve(lookup) if err != nil { - p.logger.Log("name", p.name, "err", err) - p.cache.Update(sd.Event{Err: err}) + in.logger.Log("name", in.name, "err", err) + in.cache.Update(sd.Event{Err: err}) continue // don't replace potentially-good with bad } - p.cache.Update(sd.Event{Instances: instances}) + in.cache.Update(sd.Event{Instances: instances}) - case <-p.quit: + case <-in.quit: return } } } -func (p *Instancer) resolve(lookup Lookup) ([]string, error) { - _, addrs, err := lookup("", "", p.name) +func (in *Instancer) resolve(lookup Lookup) ([]string, error) { + _, addrs, err := lookup("", "", in.name) if err != nil { return nil, err } @@ -93,11 +93,11 @@ func (p *Instancer) resolve(lookup Lookup) ([]string, error) { } // Register implements Instancer. -func (s *Instancer) Register(ch chan<- sd.Event) { - s.cache.Register(ch) +func (in *Instancer) Register(ch chan<- sd.Event) { + in.cache.Register(ch) } // Deregister implements Instancer. -func (s *Instancer) Deregister(ch chan<- sd.Event) { - s.cache.Deregister(ch) +func (in *Instancer) Deregister(ch chan<- sd.Event) { + in.cache.Deregister(ch) } From b8ef9f67cf86cefe6942f2b1dd934c6a12cbad6d Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 7 Aug 2019 10:27:22 -0700 Subject: [PATCH 2/4] sd/dnssrv: SRV record with port 0 is an error --- sd/dnssrv/instancer.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sd/dnssrv/instancer.go b/sd/dnssrv/instancer.go index 652fbc977..bc1386d7d 100644 --- a/sd/dnssrv/instancer.go +++ b/sd/dnssrv/instancer.go @@ -87,6 +87,9 @@ func (in *Instancer) resolve(lookup Lookup) ([]string, error) { } instances := make([]string, len(addrs)) for i, addr := range addrs { + if addr.Port == 0 { + return nil, fmt.Errorf("resolver returned SRV record with port 0") + } instances[i] = net.JoinHostPort(addr.Target, fmt.Sprint(addr.Port)) } return instances, nil From c40cbeba114d30f456d4b4c9e0f4e51b5c5befb3 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 7 Aug 2019 10:33:48 -0700 Subject: [PATCH 3/4] sd/dnssrv: test for SRV port zero issue --- sd/dnssrv/instancer.go | 8 +++++++- sd/dnssrv/instancer_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/sd/dnssrv/instancer.go b/sd/dnssrv/instancer.go index bc1386d7d..448ff933c 100644 --- a/sd/dnssrv/instancer.go +++ b/sd/dnssrv/instancer.go @@ -1,6 +1,7 @@ package dnssrv import ( + "errors" "fmt" "net" "time" @@ -10,6 +11,11 @@ import ( "github.com/go-kit/kit/sd/internal/instance" ) +// ErrPortZero is returned by the resolve machinery +// when a DNS resolver returns an SRV record with its +// port set to zero. +var ErrPortZero = errors.New("resolver returned SRV record with port 0") + // Instancer yields instances from the named DNS SRV record. The name is // resolved on a fixed schedule. Priorities and weights are ignored. type Instancer struct { @@ -88,7 +94,7 @@ func (in *Instancer) resolve(lookup Lookup) ([]string, error) { instances := make([]string, len(addrs)) for i, addr := range addrs { if addr.Port == 0 { - return nil, fmt.Errorf("resolver returned SRV record with port 0") + return nil, ErrPortZero } instances[i] = net.JoinHostPort(addr.Target, fmt.Sprint(addr.Port)) } diff --git a/sd/dnssrv/instancer_test.go b/sd/dnssrv/instancer_test.go index 6c8eca506..fdf1e3359 100644 --- a/sd/dnssrv/instancer_test.go +++ b/sd/dnssrv/instancer_test.go @@ -68,6 +68,33 @@ func TestRefresh(t *testing.T) { } } +func TestIssue892(t *testing.T) { + ticker := time.NewTicker(time.Second) + ticker.Stop() + tickc := make(chan time.Time) + ticker.C = tickc + + records := []*net.SRV{ + {Target: "1.0.0.1", Port: 80}, + {Target: "1.0.0.2", Port: 0}, + {Target: "1.0.0.3", Port: 80}, + } + + lookup := func(service, proto, name string) (string, []*net.SRV, error) { + return "cname", records, nil + } + + instancer := NewInstancerDetailed("name", ticker, lookup, log.NewNopLogger()) + defer instancer.Stop() + + tickc <- time.Now() + time.Sleep(100 * time.Millisecond) + + if want, have := ErrPortZero, instancer.cache.State().Err; want != have { + t.Fatalf("want %v, have %v", want, have) + } +} + type nopCloser struct{} func (nopCloser) Close() error { return nil } From 9c9c0914dcdccf2223bdc3bb6552bfcfeb5329f1 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 7 Aug 2019 10:35:42 -0700 Subject: [PATCH 4/4] .build.yml: update and fix --- .build.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.build.yml b/.build.yml index 558aa064f..659a4a545 100644 --- a/.build.yml +++ b/.build.yml @@ -1,15 +1,16 @@ image: debian/stable +packages: + - wget sources: - https://github.com/go-kit/kit tasks: - go_toolchain: | - wget -q https://dl.google.com/go/go1.12.6.linux-amd64.tar.gz - sudo tar -C /usr/local -xzf go1.12.6.linux-amd64.tar.gz + go_version="$(wget -q -O- https://golang.org/VERSION?m=text)" + wget -q https://dl.google.com/go/$go_version.linux-amd64.tar.gz + sudo tar -C /usr/local -xzf $go_version.linux-amd64.tar.gz sudo ln -s /usr/local/go/bin/go /usr/bin/go go env - dependencies: | - mkdir -p $HOME/go/src/github.com/go-kit - mv kit $HOME/go/src/github.com/go-kit go get -t github.com/go-kit/kit/... - test: | go test -race -v github.com/go-kit/kit/...