From c363b9055d6f79dafc5add21b4b995ca32fe0bd1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 13 Oct 2023 12:29:28 -0700 Subject: [PATCH] tstest/integration: add tests for tun mode (requiring root) Updates #7894 Change-Id: Iff0b07b21ae28c712dd665b12918fa28d6f601d0 Co-authored-by: Maisem Ali Signed-off-by: Brad Fitzpatrick --- .github/workflows/test.yml | 2 + cmd/testwrapper/testwrapper.go | 16 ++++-- tstest/integration/integration.go | 10 ++++ tstest/integration/integration_test.go | 72 +++++++++++++++++++------- wgengine/magicsock/endpoint.go | 4 +- 5 files changed, 80 insertions(+), 24 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 24dc985f3..0b4e5087c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -90,6 +90,8 @@ jobs: sudo apt-get -y install qemu-user - name: build test wrapper run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper + - name: integration tests as root + run: PATH=$PWD/tool:$PATH /tmp/testwrapper --sudo ./tstest/integration/ ${{matrix.buildflags}} - name: test all run: PATH=$PWD/tool:$PATH /tmp/testwrapper ./... ${{matrix.buildflags}} env: diff --git a/cmd/testwrapper/testwrapper.go b/cmd/testwrapper/testwrapper.go index 4878a9448..c85275a51 100644 --- a/cmd/testwrapper/testwrapper.go +++ b/cmd/testwrapper/testwrapper.go @@ -73,11 +73,15 @@ type goTestOutput struct { // It calls close(ch) when it's done. func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []string, ch chan<- *testAttempt) error { defer close(ch) - args := []string{"test", "-json", pt.Pattern} + args := []string{"test", "--json"} + if *flagSudo { + args = append(args, "--exec", "sudo -E") + } + args = append(args, pt.Pattern) args = append(args, otherArgs...) if len(pt.Tests) > 0 { runArg := strings.Join(pt.Tests, "|") - args = append(args, "-run", runArg) + args = append(args, "--run", runArg) } if debug { fmt.Println("running", strings.Join(args, " ")) @@ -177,6 +181,11 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st return nil } +var ( + flagVerbose = flag.Bool("v", false, "verbose") + flagSudo = flag.Bool("sudo", false, "run tests with -exec=sudo") +) + func main() { ctx := context.Background() @@ -187,7 +196,6 @@ func main() { // We run `go test -json` which returns the same information as `go test -v`, // but in a machine-readable format. So this flag is only for testwrapper's // output. - v := flag.Bool("v", false, "verbose") flag.Usage = func() { fmt.Println("usage: testwrapper [testwrapper-flags] [pattern] [build/test flags & test binary flags]") @@ -285,7 +293,7 @@ type nextRun struct { printPkgOutcome(tr.pkg, tr.outcome, thisRun.attempt) continue } - if *v || tr.outcome == "fail" { + if *flagVerbose || tr.outcome == "fail" { io.Copy(os.Stdout, &tr.logs) } if tr.outcome != "fail" { diff --git a/tstest/integration/integration.go b/tstest/integration/integration.go index f5d19e6ee..8077bcea9 100644 --- a/tstest/integration/integration.go +++ b/tstest/integration/integration.go @@ -142,6 +142,16 @@ func findGo() (string, error) { // 2. Look for a go binary in runtime.GOROOT()/bin if runtime.GOROOT() is non-empty. // 3. Look for a go binary in $PATH. + // For tests we want to run as root on GitHub actions, we run with -exec=sudo, + // but that results in this test running with a different PATH and picking the + // wrong Go. So hard code the GitHub Actions case. + if os.Getuid() == 0 && os.Getenv("GITHUB_ACTIONS") == "true" { + const sudoGithubGo = "/home/runner/.cache/tailscale-go/bin/go" + if _, err := os.Stat(sudoGithubGo); err == nil { + return sudoGithubGo, nil + } + } + paths := strings.FieldsFunc(os.Getenv("PATH"), func(r rune) bool { return os.IsPathSeparator(uint8(r)) }) if len(paths) > 0 { candidate := filepath.Join(paths[0], "go"+exe()) diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index c259c29bc..10d8735e8 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -68,6 +68,27 @@ func TestMain(m *testing.M) { os.Exit(0) } +// Tests that tailscaled starts up in TUN mode, and also without data races: +// https://github.com/tailscale/tailscale/issues/7894 +func TestTUNMode(t *testing.T) { + if os.Getuid() != 0 { + t.Skip("skipping when not root") + } + t.Parallel() + env := newTestEnv(t) + env.tunMode = true + n1 := newTestNode(t, env) + d1 := n1.StartDaemon() + + n1.AwaitResponding() + n1.MustUp() + + t.Logf("Got IP: %v", n1.AwaitIP4()) + n1.AwaitRunning() + + d1.MustCleanShutdown(t) +} + func TestOneNodeUpNoAuth(t *testing.T) { t.Parallel() env := newTestEnv(t) @@ -808,9 +829,10 @@ func TestLogoutRemovesAllPeers(t *testing.T) { // testEnv contains the test environment (set of servers) used by one // or more nodes. type testEnv struct { - t testing.TB - cli string - daemon string + t testing.TB + tunMode bool + cli string + daemon string LogCatcher *LogCatcher LogCatcherServer *httptest.Server @@ -899,12 +921,25 @@ func newTestNode(t *testing.T, env *testEnv) *testNode { sockFile = filepath.Join(os.TempDir(), rands.HexString(8)+".sock") t.Cleanup(func() { os.Remove(sockFile) }) } - return &testNode{ + n := &testNode{ env: env, dir: dir, sockFile: sockFile, stateFile: filepath.Join(dir, "tailscale.state"), } + + // Look for a data race. Once we see the start marker, start logging the rest. + var sawRace bool + n.addLogLineHook(func(line []byte) { + if mem.Contains(mem.B(line), mem.S("WARNING: DATA RACE")) { + sawRace = true + } + if sawRace { + t.Logf("%s", line) + } + }) + + return n } func (n *testNode) diskPrefs() *ipn.Prefs { @@ -963,7 +998,7 @@ func (n *testNode) socks5AddrChan() <-chan string { if i == -1 { return } - addr := string(line)[i+len(sub):] + addr := strings.TrimSpace(string(line)[i+len(sub):]) select { case ch <- addr: default: @@ -1010,11 +1045,10 @@ func (op *nodeOutputParser) parseLines() { } line := buf[:nl+1] buf = buf[nl+1:] - lineTrim := bytes.TrimSpace(line) n.mu.Lock() for _, f := range n.onLogLine { - f(lineTrim) + f(line) } n.mu.Unlock() } @@ -1048,8 +1082,8 @@ func (n *testNode) StartDaemon() *Daemon { func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon { t := n.env.t - cmd := exec.Command(n.env.daemon, - "--tun=userspace-networking", + cmd := exec.Command(n.env.daemon) + cmd.Args = append(cmd.Args, "--state="+n.stateFile, "--socket="+n.sockFile, "--socks5-server=localhost:0", @@ -1057,6 +1091,11 @@ func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon { if *verboseTailscaled { cmd.Args = append(cmd.Args, "-verbose=2") } + if !n.env.tunMode { + cmd.Args = append(cmd.Args, + "--tun=userspace-networking", + ) + } cmd.Env = append(os.Environ(), "TS_DEBUG_PERMIT_HTTP_C2N=1", "TS_LOG_TARGET="+n.env.LogCatcherServer.URL, @@ -1067,10 +1106,7 @@ func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon { "TS_NETCHECK_GENERATE_204_URL="+n.env.ControlServer.URL+"/generate_204", ) if version.IsRace() { - const knownBroken = true // TODO(bradfitz,maisem): enable this once we fix all the races :( - if !knownBroken { - cmd.Env = append(cmd.Env, "GORACE=halt_on_error=1") - } + cmd.Env = append(cmd.Env, "GORACE=halt_on_error=1") } cmd.Stderr = &nodeOutputParser{n: n} if *verboseTailscaled { @@ -1143,11 +1179,10 @@ func (n *testNode) AwaitListening() { s := safesocket.DefaultConnectionStrategy(n.sockFile) if err := tstest.WaitFor(20*time.Second, func() (err error) { c, err := safesocket.Connect(s) - if err != nil { - return err + if err == nil { + c.Close() } - c.Close() - return nil + return err }); err != nil { t.Fatal(err) } @@ -1241,7 +1276,8 @@ func (n *testNode) AwaitNeedsLogin() { // Tailscale returns a command that runs the tailscale CLI with the provided arguments. // It does not start the process. func (n *testNode) Tailscale(arg ...string) *exec.Cmd { - cmd := exec.Command(n.env.cli, "--socket="+n.sockFile) + cmd := exec.Command(n.env.cli) + cmd.Args = append(cmd.Args, "--socket="+n.sockFile) cmd.Args = append(cmd.Args, arg...) cmd.Dir = n.dir cmd.Env = append(os.Environ(), diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 88ab6d896..0496ee139 100644 --- a/wgengine/magicsock/endpoint.go +++ b/wgengine/magicsock/endpoint.go @@ -484,9 +484,9 @@ func (p *pingResultAndCallback) reply() bool { return p != nil && p.taken.CompareAndSwap(false, true) } -// discoPing starts a disc-level ping for the "tailscale ping" command (or other +// discoPing starts a disco-level ping for the "tailscale ping" command (or other // callers, such as c2n). res is value to call cb with, already partially -// filled. cb must be called at most once. Once called, ownership of res passes to db. +// filled. cb must be called at most once. Once called, ownership of res passes to cb. func (de *endpoint) discoPing(res *ipnstate.PingResult, size int, cb func(*ipnstate.PingResult)) { de.mu.Lock() defer de.mu.Unlock()