From 1e62de50bda165c531df2a0080763b3f7c0d7c79 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sat, 2 Jun 2018 18:05:09 +0200 Subject: [PATCH] factor dnsmasq code into testing helper, verify dhcp6 client id --- integrationdhcpv4_test.go | 149 +++++++---------------- integrationdhcpv6_test.go | 96 ++++++--------- internal/testing/dnsmasq/dnsmasq.go | 147 ++++++++++++++++++++++ internal/testing/dnsmasq/example_test.go | 36 ++++++ 4 files changed, 262 insertions(+), 166 deletions(-) create mode 100644 internal/testing/dnsmasq/dnsmasq.go create mode 100644 internal/testing/dnsmasq/example_test.go diff --git a/integrationdhcpv4_test.go b/integrationdhcpv4_test.go index 7bbb74b..3eabe5b 100644 --- a/integrationdhcpv4_test.go +++ b/integrationdhcpv4_test.go @@ -1,25 +1,18 @@ package integration_test import ( - "bytes" "io/ioutil" - "log" "net" "os" "os/exec" - "regexp" - "strconv" "strings" "testing" "time" "router7/internal/dhcp4" + "router7/internal/testing/dnsmasq" "github.com/google/go-cmp/cmp" - "github.com/google/gopacket" - "github.com/google/gopacket/layers" - "github.com/google/gopacket/pcap" - "github.com/google/gopacket/pcapgo" ) func TestDHCPv4(t *testing.T) { @@ -51,77 +44,41 @@ func TestDHCPv4(t *testing.T) { ready.Close() // dnsmasq will re-create the file anyway defer os.Remove(ready.Name()) // dnsmasq does not clean up its pid file - var stderr bytes.Buffer - dnsmasq := exec.Command("ip", "netns", "exec", ns, "dnsmasq", - "--keep-in-foreground", // cannot use --no-daemon because we need --pid-file - "--log-facility=-", // log to stderr - "--pid-file="+ready.Name(), - "--bind-interfaces", - "--interface=veth0b", - "--dhcp-range=192.168.23.2,192.168.23.10", - "--dhcp-range=::1,::10,constructor:veth0b", - "--dhcp-authoritative", // eliminate timeouts - "--no-ping", // disable ICMP confirmation of unused addresses to eliminate tedious timeout - "--leasefile-ro", // do not create a lease database - ) - dnsmasq.Stdout = os.Stdout - dnsmasq.Stderr = &stderr - if err := dnsmasq.Start(); err != nil { - t.Fatal(err) - } - done := false // TODO: fix data race - go func() { - err := dnsmasq.Wait() - if !done { - t.Fatalf("dnsmasq exited prematurely: %v", err) - } - }() - defer func() { - done = true - dnsmasq.Process.Kill() - }() + dnsmasq := dnsmasq.Run(t) + defer dnsmasq.Kill() - // TODO(later): use inotify instead of polling - // Wait for dnsmasq to write its process id, at which point it is already - // listening for requests. - for { - b, err := ioutil.ReadFile(ready.Name()) - if err != nil { - t.Fatal(err) - } - if strings.TrimSpace(string(b)) == strconv.Itoa(dnsmasq.Process.Pid) { - break - } - time.Sleep(10 * time.Millisecond) - } - - f, err := os.Create("/tmp/pcap") - if err != nil { - t.Fatal(err) - } - defer f.Close() - pcapw := pcapgo.NewWriter(f) - if err := pcapw.WriteFileHeader(1600, layers.LinkTypeEthernet); err != nil { - t.Fatal(err) - } - handle, err := pcap.OpenLive("veth0a", 1600, true, pcap.BlockForever) - if err != nil { - t.Fatal(err) - } - pkgsrc := gopacket.NewPacketSource(handle, handle.LinkType()) - closed := make(chan struct{}) - go func() { - for packet := range pkgsrc.Packets() { - if packet.Layer(layers.LayerTypeDHCPv4) != nil { - log.Printf("packet: %+v", packet) - if err := pcapw.WritePacket(packet.Metadata().CaptureInfo, packet.Data()); err != nil { - t.Fatalf("pcap.WritePacket(): %v", err) - } - } - } - close(closed) - }() - // TODO: test the capture daemon + // f, err := os.Create("/tmp/pcap") + // if err != nil { + // t.Fatal(err) + // } + // defer f.Close() + // pcapw := pcapgo.NewWriter(f) + // if err := pcapw.WriteFileHeader(1600, layers.LinkTypeEthernet); err != nil { + // t.Fatal(err) + // } + // handle, err := pcap.OpenLive("veth0a", 1600, true, pcap.BlockForever) + // if err != nil { + // t.Fatal(err) + // } + // pkgsrc := gopacket.NewPacketSource(handle, handle.LinkType()) + // closed := make(chan struct{}) + // go func() { + // for packet := range pkgsrc.Packets() { + // if packet.Layer(layers.LayerTypeDHCPv4) != nil { + // log.Printf("packet: %+v", packet) + // if err := pcapw.WritePacket(packet.Metadata().CaptureInfo, packet.Data()); err != nil { + // t.Fatalf("pcap.WritePacket(): %v", err) + // } + // } + // } + // close(closed) + // }() + // // TODO: test the capture daemon + // defer func() { + // time.Sleep(1 * time.Second) + // handle.Close() + // <-closed + // }() iface, err := net.InterfaceByName("veth0a") if err != nil { @@ -147,27 +104,8 @@ func TestDHCPv4(t *testing.T) { // TODO: alternatively, replace bytes.Buffer with a pipe and read from that time.Sleep(100 * time.Millisecond) // give dnsmasq some time to process the DHCPRELEASE - // Kill dnsmasq to flush its log - done = true - dnsmasq.Process.Kill() - - mac := iface.HardwareAddr.String() - lines := strings.Split(strings.TrimSpace(stderr.String()), "\n") - var dhcpActionRe = regexp.MustCompile(` (DHCP[^ ]*)`) - var actions []string - for _, line := range lines { - if !strings.HasPrefix(line, "dnsmasq-dhcp") { - continue - } - if !strings.Contains(line, mac) { - continue - } - matches := dhcpActionRe.FindStringSubmatch(line) - if matches == nil { - continue - } - actions = append(actions, matches[1]) - } + dnsmasq.Kill() // to flush logs + got := dnsmasq.Actions() want := []string{ "DHCPDISCOVER(veth0b)", "DHCPOFFER(veth0b)", @@ -175,11 +113,14 @@ func TestDHCPv4(t *testing.T) { "DHCPACK(veth0b)", "DHCPRELEASE(veth0b)", } - if diff := cmp.Diff(actions, want); diff != "" { + actionOnly := func(line string) string { + result := line + if idx := strings.Index(result, " "); idx > -1 { + return result[:idx] + } + return result + } + if diff := cmp.Diff(got, want, cmp.Transformer("ActionOnly", actionOnly)); diff != "" { t.Errorf("dnsmasq log does not contain expected DHCP sequence: diff (-got +want):\n%s", diff) } - - time.Sleep(1 * time.Second) - handle.Close() - <-closed } diff --git a/integrationdhcpv6_test.go b/integrationdhcpv6_test.go index 4230a3e..e58f99e 100644 --- a/integrationdhcpv6_test.go +++ b/integrationdhcpv6_test.go @@ -1,19 +1,19 @@ package integration_test import ( - "io/ioutil" - "os" "os/exec" - "strconv" + "regexp" "strings" "testing" - "time" "router7/internal/dhcp6" + "router7/internal/testing/dnsmasq" "github.com/google/go-cmp/cmp" ) +var v6AddrRe = regexp.MustCompile(`2001:db8::[^ ]+`) + func TestDHCPv6(t *testing.T) { const ns = "ns0" // name of the network namespace to use for this test @@ -43,57 +43,10 @@ func TestDHCPv6(t *testing.T) { } } - ready, err := ioutil.TempFile("", "router7") - if err != nil { - t.Fatal(err) - } - ready.Close() // dnsmasq will re-create the file anyway - defer os.Remove(ready.Name()) // dnsmasq does not clean up its pid file + dnsmasq := dnsmasq.Run(t) + defer dnsmasq.Kill() - dnsmasq := exec.Command("ip", "netns", "exec", ns, "dnsmasq", - "--keep-in-foreground", // cannot use --no-daemon because we need --pid-file - "--log-facility=-", // log to stderr - "--pid-file="+ready.Name(), - "--bind-interfaces", - "--interface=veth0b", - "--dhcp-range=192.168.23.2,192.168.23.10", - "--dhcp-range=::1,::10,constructor:veth0b", - "--dhcp-authoritative", // eliminate timeouts - "--no-ping", // disable ICMP confirmation of unused addresses to eliminate tedious timeout - "--leasefile-ro", // do not create a lease database - ) - dnsmasq.Stdout = os.Stdout - dnsmasq.Stderr = os.Stderr - if err := dnsmasq.Start(); err != nil { - t.Fatal(err) - } - done := false // TODO: fix data race - go func() { - err := dnsmasq.Wait() - if !done { - t.Fatalf("dnsmasq exited prematurely: %v", err) - } - }() - defer func() { - done = true - dnsmasq.Process.Kill() - }() - - // TODO(later): use inotify instead of polling - // Wait for dnsmasq to write its process id, at which point it is already - // listening for requests. - for { - b, err := ioutil.ReadFile(ready.Name()) - if err != nil { - t.Fatal(err) - } - if strings.TrimSpace(string(b)) == strconv.Itoa(dnsmasq.Process.Pid) { - break - } - time.Sleep(10 * time.Millisecond) - } - - // f, err := os.Create("/tmp/pcap") + // f, err := os.Create("/tmp/pcap6") // if err != nil { // t.Fatal(err) // } @@ -110,19 +63,26 @@ func TestDHCPv6(t *testing.T) { // closed := make(chan struct{}) // go func() { // for packet := range pkgsrc.Packets() { - // if packet.Layer(layers.LayerTypeDHCPv4) != nil { - // log.Printf("packet: %+v", packet) - // if err := pcapw.WritePacket(packet.Metadata().CaptureInfo, packet.Data()); err != nil { - // t.Fatalf("pcap.WritePacket(): %v", err) - // } + // //if packet.Layer(layers.LayerTypeDHCPv6) != nil { + // fmt.Printf("packet: %+v\n", packet) + // if err := pcapw.WritePacket(packet.Metadata().CaptureInfo, packet.Data()); err != nil { + // t.Fatalf("pcap.WritePacket(): %v", err) // } + // //} // } // close(closed) // }() // // TODO: test the capture daemon + // defer func() { + // time.Sleep(1 * time.Second) + // handle.Close() + // <-closed + // }() + duid := []byte{0x00, 0x0a, 0x00, 0x03, 0x00, 0x01, 0x4c, 0x5e, 0xc, 0x41, 0xbf, 0x39} c, err := dhcp6.NewClient(dhcp6.ClientConfig{ InterfaceName: "veth0a", + DUID: duid, }) if err != nil { t.Fatal(err) @@ -139,7 +99,19 @@ func TestDHCPv6(t *testing.T) { t.Fatalf("unexpected config: diff (-got +want):\n%s", diff) } - // time.Sleep(1 * time.Second) - // handle.Close() - // <-closed + { + got := dnsmasq.Actions() + want := []string{ + "DHCPSOLICIT(veth0b) 00:0a:00:03:00:01:4c:5e:0c:41:bf:39", + "DHCPADVERTISE(veth0b) 2001:db8::c 00:0a:00:03:00:01:4c:5e:0c:41:bf:39", + "DHCPREQUEST(veth0b) 00:0a:00:03:00:01:4c:5e:0c:41:bf:39", + "DHCPREPLY(veth0b) 2001:db8::c 00:0a:00:03:00:01:4c:5e:0c:41:bf:39", + } + withoutMac := func(line string) string { + return v6AddrRe.ReplaceAllString(strings.TrimSpace(line), "") + } + if diff := cmp.Diff(got, want, cmp.Transformer("WithoutMAC", withoutMac)); diff != "" { + t.Errorf("dnsmasq log does not contain expected DHCP sequence: diff (-got +want):\n%s", diff) + } + } } diff --git a/internal/testing/dnsmasq/dnsmasq.go b/internal/testing/dnsmasq/dnsmasq.go new file mode 100644 index 0000000..ae561e4 --- /dev/null +++ b/internal/testing/dnsmasq/dnsmasq.go @@ -0,0 +1,147 @@ +// Package dnsmasq manages the process lifecycle of the dnsmasq(8) DHCP server. +package dnsmasq + +import ( + "bufio" + "fmt" + "io" + "io/ioutil" + "os" + "os/exec" + "regexp" + "strconv" + "strings" + "sync" + "testing" + "time" +) + +// Process is a handle for a dnsmasq(8) process. +type Process struct { + killed bool // whether Kill was called + done chan struct{} // closed when Done() is called + wait chan struct{} // closed when Wait() returns + dnsmasq *exec.Cmd + + mu sync.Mutex + actions []string +} + +var dhcpActionRe = regexp.MustCompile(` (DHCP[^(]+\(.*)$`) + +// Run starts a dnsmasq(8) process and returns a handle to it. +func Run(t *testing.T, args ...string) *Process { + const ns = "ns0" // TODO: configurable? + + ready, err := ioutil.TempFile("", "router7") + if err != nil { + t.Fatal(err) + } + ready.Close() // dnsmasq will re-create the file anyway + defer os.Remove(ready.Name()) // dnsmasq does not clean up its pid file + + // iface, err := net.InterfaceByName("veth0a") + // if err != nil { + // t.Fatal(err) + // } + + p := &Process{ + wait: make(chan struct{}), + } + + p.dnsmasq = exec.Command("ip", "netns", "exec", ns, "dnsmasq", + "--keep-in-foreground", // cannot use --no-daemon because we need --pid-file + "--log-facility=-", // log to stderr + "--pid-file="+ready.Name(), + "--bind-interfaces", + "--interface=veth0b", + "--dhcp-range=192.168.23.2,192.168.23.10", + "--dhcp-range=::1,::10,constructor:veth0b", + "--dhcp-authoritative", // eliminate timeouts + "--no-ping", // disable ICMP confirmation of unused addresses to eliminate tedious timeout + "--leasefile-ro", // do not create a lease database + ) + + p.dnsmasq.Stdout = os.Stdout + stderr := make(chan string) + r, w := io.Pipe() + scanner := bufio.NewScanner(r) + go func() { + for scanner.Scan() { + stderr <- scanner.Text() + } + close(stderr) + }() + p.dnsmasq.Stderr = w + //mac := iface.HardwareAddr.String() + go func() { + for line := range stderr { + fmt.Printf("dnsmasq log line: %s\n", line) + if !strings.HasPrefix(line, "dnsmasq-dhcp") { + continue + } + // if !strings.Contains(line, mac) { + // continue + // } + matches := dhcpActionRe.FindStringSubmatch(line) + if matches == nil { + continue + } + p.mu.Lock() + p.actions = append(p.actions, matches[1]) + p.mu.Unlock() + } + }() + if err := p.dnsmasq.Start(); err != nil { + t.Fatal(err) + } + + p.done = make(chan struct{}) + go func() { + err := p.dnsmasq.Wait() + close(p.wait) + select { + case <-p.done: + return // test done, any errors are from our Kill() + default: + t.Fatalf("dnsmasq exited prematurely: %v", err) + } + }() + + // TODO(later): use inotify instead of polling + // Wait for dnsmasq to write its process id, at which point it is already + // listening for requests. + for { + b, err := ioutil.ReadFile(ready.Name()) + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(b)) == strconv.Itoa(p.dnsmasq.Process.Pid) { + break + } + time.Sleep(10 * time.Millisecond) + } + + return p +} + +// Kill shuts down the dnsmasq(8) process and returns once waitpid returns. +func (p *Process) Kill() { + if p.killed { + return + } + p.killed = true + close(p.done) + p.dnsmasq.Process.Kill() + <-p.wait +} + +// Actions returns a string slice of dnsmasq(8) actions (as per its stderr log) +// received up until now. Use Kill before calling Actions to force a log flush. +func (p *Process) Actions() []string { + p.mu.Lock() + defer p.mu.Unlock() + result := make([]string, len(p.actions)) + copy(result, p.actions) + return result +} diff --git a/internal/testing/dnsmasq/example_test.go b/internal/testing/dnsmasq/example_test.go new file mode 100644 index 0000000..4760ae5 --- /dev/null +++ b/internal/testing/dnsmasq/example_test.go @@ -0,0 +1,36 @@ +package dnsmasq_test + +import ( + "router7/internal/testing/dnsmasq" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func Example(t *testing.T) { + dnsmasq := dnsmasq.Run(t) + defer dnsmasq.Kill() + // test code here + + // optionally introspect the dnsmasq log: + dnsmasq.Kill() + got := dnsmasq.Actions() + want := []string{ + "DHCPDISCOVER(veth0b)", + "DHCPOFFER(veth0b)", + "DHCPREQUEST(veth0b)", + "DHCPACK(veth0b)", + "DHCPRELEASE(veth0b)", + } + actionOnly := func(line string) string { + result := line + if idx := strings.Index(result, " "); idx > -1 { + return result[:idx] + } + return result + } + if diff := cmp.Diff(got, want, cmp.Transformer("ActionOnly", actionOnly)); diff != "" { + t.Errorf("dnsmasq log does not contain expected DHCP sequence: diff (-got +want):\n%s", diff) + } +}