From 47653f06fbfdc84a5754f24fb33d60b298264b50 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 7 Oct 2026 14:29:29 +0000 Subject: [PATCH] integration: avoid toolchain work and login waits in CI --- .../workflows/integration-test-template.yml | 25 +- cmd/hi/README.md | 24 ++ cmd/hi/docker.go | 109 +++++--- cmd/hi/doctor.go | 17 +- cmd/hi/run.go | 58 +++- cmd/hi/run_test.go | 247 ++++++++++++++++++ flake.nix | 7 +- integration/auth_oidc_test.go | 3 + integration/tsic/login_test.go | 171 ++++++++++++ integration/tsic/tsic.go | 142 ++++++++-- 10 files changed, 715 insertions(+), 88 deletions(-) create mode 100644 cmd/hi/run_test.go create mode 100644 integration/tsic/login_test.go diff --git a/.github/workflows/integration-test-template.yml b/.github/workflows/integration-test-template.yml index 81599ffd9..a92f6d545 100644 --- a/.github/workflows/integration-test-template.yml +++ b/.github/workflows/integration-test-template.yml @@ -61,10 +61,10 @@ jobs: with: name: hi-binary path: /tmp/artifacts - - name: Download Go cache + - name: Download integration test binary uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - name: go-cache + name: integration-test-binary path: /tmp/artifacts - name: Download postgres image if: ${{ inputs.postgres_flag == '--postgres=1' }} @@ -94,7 +94,7 @@ jobs: with: username: ${{ env.DOCKERHUB_USERNAME }} password: ${{ env.DOCKERHUB_TOKEN }} - - name: Load Docker images, Go cache, and prepare binary + - name: Load Docker images and prepare binaries run: | gunzip -c /tmp/artifacts/headscale-image.tar.gz | docker load gunzip -c /tmp/artifacts/tailscale-head-image.tar.gz | docker load @@ -102,33 +102,30 @@ jobs: if [ -f /tmp/artifacts/postgres-image.tar.gz ]; then gunzip -c /tmp/artifacts/postgres-image.tar.gz | docker load fi - chmod +x /tmp/artifacts/hi + chmod +x /tmp/artifacts/hi /tmp/artifacts/integration.test docker images - # Extract Go cache to host directories for bind mounting - mkdir -p /tmp/go-cache - tar -xzf /tmp/artifacts/go-cache.tar.gz -C /tmp/go-cache - ls -la /tmp/go-cache/ /tmp/go-cache/.cache/ - name: Run Integration Test env: HEADSCALE_INTEGRATION_HEADSCALE_IMAGE: headscale:${{ github.sha }} HEADSCALE_INTEGRATION_TAILSCALE_IMAGE: tailscale-head:${{ github.sha }} HEADSCALE_INTEGRATION_POSTGRES_IMAGE: ${{ inputs.postgres_flag == '--postgres=1' && format('postgres:{0}', github.sha) || '' }} - HEADSCALE_INTEGRATION_GO_CACHE: /tmp/go-cache/go - HEADSCALE_INTEGRATION_GO_BUILD_CACHE: /tmp/go-cache/.cache/go-build # Mirror the docker/login-action secrets into env so the # dockertestutil.Credentials resolver picks them up directly # (otherwise it falls back to parsing ~/.docker/config.json, # which works but is one step further from the source). DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_CI_USERNAME }} DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_CI_TOKEN }} - run: /tmp/artifacts/hi run --stats --ts-memory-limit=300 --hs-memory-limit=1500 "^${{ inputs.test }}$" \ - --timeout=120m \ - ${{ inputs.postgres_flag }} + run: | + /tmp/artifacts/hi run --stats --ts-memory-limit=300 --hs-memory-limit=1500 \ + --test-binary=/tmp/artifacts/integration.test \ + --timeout=120m \ + ${{ inputs.postgres_flag }} \ + "^${{ inputs.test }}$" # Sanitize test name for artifact upload (replace invalid characters: " : < > | * ? \ / with -) - name: Sanitize test name for artifacts if: always() id: sanitize - run: echo "name=${TEST_NAME//[\":<>|*?\\\/]/-}" >> $GITHUB_OUTPUT + run: echo "name=${TEST_NAME//[\":<>|*?\\\/]/-}" >> "$GITHUB_OUTPUT" env: TEST_NAME: ${{ inputs.test }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/cmd/hi/README.md b/cmd/hi/README.md index 6c105cfd1..50d5bff15 100644 --- a/cmd/hi/README.md +++ b/cmd/hi/README.md @@ -56,6 +56,7 @@ changing. | `--postgres` | `false` | Use PostgreSQL instead of SQLite | | `--failfast` | `true` | Stop on first test failure | | `--go-version` | auto | Detected from `go.mod` (currently 1.27.0) | +| `--test-binary` | unset | Execute a precompiled integration test binary instead of running `go test` | | `--clean-before` | `true` | Clean stale (stopped/exited) containers before starting | | `--clean-after` | `true` | Clean this run's containers after completion | | `--keep-on-failure` | `false` | Preserve containers for manual inspection on failure | @@ -65,6 +66,29 @@ changing. | `--hs-memory-limit` | `0` | Fail if any headscale container exceeds N MB (0 = disabled) | | `--ts-memory-limit` | `0` | Fail if any tailscale container exceeds N MB | +### Precompiled tests in CI + +CI builds the Headscale server, `hi`, and the integration test binary once +for the worker architecture. Each test job loads the prebuilt images and +runs its own fresh test container: + +```bash +CGO_ENABLED=0 GOOS=linux go test -c ./integration -o integration.test +go run ./cmd/hi run --test-binary=./integration.test "^TestHeadscale$" +``` + +With `--test-binary`, the runner mounts the executable read-only and uses +`HEADSCALE_INTEGRATION_HEADSCALE_IMAGE` as its runtime image when set. +Otherwise it uses the usual Go image. Build the binary with `CGO_ENABLED=0` +for the runner architecture so it needs no compiler or shared Go libraries. +The checkout, test fixtures, Docker access, per-run logs, resource checks, +and cleanup work the same as in source-based runs. CI does not distribute +Go caches to these workers. + +Automatic preflight checks Docker and the checkout. Images are prepared by +their consumers, so ordinary tests do not pull Kubernetes images. The +standalone `doctor` command retains the comprehensive development checks. + ### Timeout guidance The default `120m` is generous for a single test. If you must tune it, diff --git a/cmd/hi/docker.go b/cmd/hi/docker.go index 102ee526e..788081d95 100644 --- a/cmd/hi/docker.go +++ b/cmd/hi/docker.go @@ -73,17 +73,17 @@ func runTestContainer(ctx context.Context, config *RunConfig) error { } } - goTestCmd := buildGoTestCommand(config) + testCmd := buildTestCommand(config) if config.Verbose { - log.Printf("Command: %s", strings.Join(goTestCmd, " ")) + log.Printf("Command: %s", strings.Join(testCmd, " ")) } - imageName := "golang:" + config.GoVersion + imageName := testRunnerImage(config) if err := ensureImageAvailable(ctx, cli, imageName, config.Verbose); err != nil { //nolint:noinlineerr return fmt.Errorf("ensuring image availability: %w", err) } - resp, err := createGoTestContainer(ctx, cli, config, containerName, absLogsDir, goTestCmd) + resp, err := createTestContainer(ctx, cli, config, containerName, absLogsDir, testCmd) if err != nil { return fmt.Errorf("creating container: %w", err) } @@ -202,26 +202,44 @@ func runTestContainer(ctx context.Context, config *RunConfig) error { return nil } -// buildGoTestCommand constructs the go test command arguments. -func buildGoTestCommand(config *RunConfig) []string { +// testRunnerImage reuses the CI server image for the statically built test binary. +// Source-based runs and local runs without a server image use the Go image. +func testRunnerImage(config *RunConfig) string { + if config.TestBinary != "" { + if image := os.Getenv("HEADSCALE_INTEGRATION_HEADSCALE_IMAGE"); image != "" { + return image + } + } + + return "golang:" + config.GoVersion +} + +// buildTestCommand supports both go test and its compiled test binary flags. +func buildTestCommand(config *RunConfig) []string { cmd := []string{"go", "test", "./..."} + flagPrefix := "-" + + if config.TestBinary != "" { + cmd = []string{"/integration.test"} + flagPrefix = "-test." + } if config.TestPattern != "" { - cmd = append(cmd, "-run", config.TestPattern) + cmd = append(cmd, flagPrefix+"run", config.TestPattern) } if config.FailFast { - cmd = append(cmd, "-failfast") + cmd = append(cmd, flagPrefix+"failfast") } - cmd = append(cmd, "-timeout", config.Timeout.String()) - cmd = append(cmd, "-v") + cmd = append(cmd, flagPrefix+"timeout", config.Timeout.String()) + cmd = append(cmd, flagPrefix+"v") return cmd } -// createGoTestContainer creates a Docker container configured for running integration tests. -func createGoTestContainer(ctx context.Context, cli *client.Client, config *RunConfig, containerName, logsDir string, goTestCmd []string) (container.CreateResponse, error) { +// createTestContainer creates a Docker container configured for running integration tests. +func createTestContainer(ctx context.Context, cli *client.Client, config *RunConfig, containerName, logsDir string, testCmd []string) (container.CreateResponse, error) { pwd, err := os.Getwd() if err != nil { return container.CreateResponse{}, fmt.Errorf("getting working directory: %w", err) @@ -254,12 +272,14 @@ func createGoTestContainer(ctx context.Context, cli *client.Client, config *RunC } } - // Set GOCACHE to a known location (used by both bind mount and volume cases) - env = append(env, "GOCACHE=/cache/go-build") + if config.TestBinary == "" { + // Used by both bind mount and volume cases for source-based runs. + env = append(env, "GOCACHE=/cache/go-build") + } containerConfig := &container.Config{ - Image: "golang:" + config.GoVersion, - Cmd: goTestCmd, + Image: testRunnerImage(config), + Cmd: testCmd, Env: env, WorkingDir: projectRoot + "/integration", Tty: true, @@ -286,27 +306,36 @@ func createGoTestContainer(ctx context.Context, cli *client.Client, config *RunC // otherwise fall back to Docker volumes for local development var mounts []mount.Mount - goCache := os.Getenv("HEADSCALE_INTEGRATION_GO_CACHE") - goBuildCache := os.Getenv("HEADSCALE_INTEGRATION_GO_BUILD_CACHE") - - if goCache != "" { - binds = append(binds, goCache+":/go") - } else { + if config.TestBinary != "" { mounts = append(mounts, mount.Mount{ - Type: mount.TypeVolume, - Source: "hs-integration-go-cache", - Target: "/go", + Type: mount.TypeBind, + Source: config.TestBinary, + Target: "/integration.test", + ReadOnly: true, }) - } - - if goBuildCache != "" { - binds = append(binds, goBuildCache+":/cache/go-build") } else { - mounts = append(mounts, mount.Mount{ - Type: mount.TypeVolume, - Source: "hs-integration-go-build-cache", - Target: "/cache/go-build", - }) + goCache := os.Getenv("HEADSCALE_INTEGRATION_GO_CACHE") + goBuildCache := os.Getenv("HEADSCALE_INTEGRATION_GO_BUILD_CACHE") + + if goCache != "" { + binds = append(binds, goCache+":/go") + } else { + mounts = append(mounts, mount.Mount{ + Type: mount.TypeVolume, + Source: "hs-integration-go-cache", + Target: "/go", + }) + } + + if goBuildCache != "" { + binds = append(binds, goBuildCache+":/cache/go-build") + } else { + mounts = append(mounts, mount.Mount{ + Type: mount.TypeVolume, + Source: "hs-integration-go-build-cache", + Target: "/cache/go-build", + }) + } } hostConfig := &container.HostConfig{ @@ -330,8 +359,12 @@ func streamAndWait(ctx context.Context, cli *client.Client, containerID string) } defer out.Close() + logsDone := make(chan struct{}) + go func() { _, _ = io.Copy(os.Stdout, out) + + close(logsDone) }() statusCh, errCh := cli.ContainerWait(ctx, containerID, container.WaitConditionNotRunning) @@ -341,6 +374,14 @@ func streamAndWait(ctx context.Context, cli *client.Client, containerID string) return -1, fmt.Errorf("waiting for container: %w", err) } case status := <-statusCh: + // ContainerWait can finish before the log stream delivers its tail. + // Drain through EOF so fast test exits retain their final summary. + select { + case <-logsDone: + case <-ctx.Done(): + return int(status.StatusCode), fmt.Errorf("waiting for container logs: %w", ctx.Err()) + } + return int(status.StatusCode), nil } diff --git a/cmd/hi/doctor.go b/cmd/hi/doctor.go index 7bd42fb90..172b16496 100644 --- a/cmd/hi/doctor.go +++ b/cmd/hi/doctor.go @@ -53,6 +53,12 @@ func fail(name, message string, suggestions ...string) DoctorResult { // runDoctorCheck performs comprehensive pre-flight checks for integration testing. func runDoctorCheck(ctx context.Context) error { + return runPreflightChecks(ctx, true) +} + +// runPreflightChecks leaves image preparation to the tests that need it. +// The standalone doctor additionally checks development tools and images. +func runPreflightChecks(ctx context.Context, full bool) error { results := []DoctorResult{} // Check 1: Docker binary availability @@ -67,12 +73,17 @@ func runDoctorCheck(ctx context.Context) error { results = append(results, checkDockerContext(ctx)) results = append(results, checkDockerSocket(ctx)) results = append(results, checkDockerHubCredentials()) - results = append(results, checkGolangImage(ctx)) - results = append(results, checkK3sImage(ctx)) + + if full { + results = append(results, checkGolangImage(ctx)) + results = append(results, checkK3sImage(ctx)) + } } // Check 3: Go installation - results = append(results, checkGoInstallation(ctx)) + if full { + results = append(results, checkGoInstallation(ctx)) + } // Check 4: Git repository results = append(results, checkGitRepository(ctx)) diff --git a/cmd/hi/run.go b/cmd/hi/run.go index e0fbd1c04..8c3648125 100644 --- a/cmd/hi/run.go +++ b/cmd/hi/run.go @@ -12,10 +12,15 @@ import ( "github.com/creachadair/command" ) -var ErrTestPatternRequired = errors.New("test pattern is required as first argument or use --test flag") +var ( + ErrTestPatternRequired = errors.New("test pattern is required as first argument or use --test flag") + ErrUnexpectedTestArguments = errors.New("expected a single test pattern; check flag spelling and shell quoting") + ErrInvalidTestBinary = errors.New("test binary must be an executable regular file") +) type RunConfig struct { TestPattern string `flag:"test,Test pattern to run"` + TestBinary string `flag:"test-binary,Path to a precompiled integration test binary"` Timeout time.Duration `flag:"timeout,default=120m,Test timeout"` FailFast bool `flag:"failfast,default=true,Stop on first test failure"` UsePostgres bool `flag:"postgres,default=false,Use PostgreSQL instead of SQLite"` @@ -30,17 +35,47 @@ type RunConfig struct { TSMemoryLimit float64 `flag:"ts-memory-limit,default=0,Fail test if any Tailscale container exceeds this memory limit in MB (0 = disabled)"` } -// runIntegrationTest executes the integration test workflow. -func runIntegrationTest(env *command.Env) error { - args := env.Args - if len(args) > 0 && runConfig.TestPattern == "" { - runConfig.TestPattern = args[0] +func (c *RunConfig) validate(args []string) error { + if len(args) > 1 || (len(args) != 0 && c.TestPattern != "") { + return ErrUnexpectedTestArguments } - if runConfig.TestPattern == "" { + if len(args) == 1 { + c.TestPattern = args[0] + } + + if c.TestPattern == "" { return ErrTestPatternRequired } + if c.TestBinary != "" { + binary, err := filepath.Abs(c.TestBinary) + if err != nil { + return fmt.Errorf("resolving test binary: %w", err) + } + + info, err := os.Stat(binary) + if err != nil { + return fmt.Errorf("reading test binary: %w", err) + } + + if !info.Mode().IsRegular() || info.Mode().Perm()&0o111 == 0 { + return fmt.Errorf("%s: %w", binary, ErrInvalidTestBinary) + } + + c.TestBinary = binary + } + + return nil +} + +// runIntegrationTest executes the integration test workflow. +func runIntegrationTest(env *command.Env) error { + err := runConfig.validate(env.Args) + if err != nil { + return err + } + if runConfig.GoVersion == "" { runConfig.GoVersion = detectGoVersion() } @@ -50,7 +85,7 @@ func runIntegrationTest(env *command.Env) error { log.Printf("Running pre-flight system checks...") } - err := runDoctorCheck(env.Context()) + err = runPreflightChecks(env.Context(), false) if err != nil { return fmt.Errorf("pre-flight checks failed: %w", err) } @@ -62,6 +97,13 @@ func runIntegrationTest(env *command.Env) error { log.Printf("Use PostgreSQL: %t", runConfig.UsePostgres) } + backend := "sqlite" + if runConfig.UsePostgres { + backend = "postgres" + } + + log.Printf("Database backend: %s", backend) + return runTestContainer(env.Context(), &runConfig) } diff --git a/cmd/hi/run_test.go b/cmd/hi/run_test.go new file mode 100644 index 000000000..94c8d3747 --- /dev/null +++ b/cmd/hi/run_test.go @@ -0,0 +1,247 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/creachadair/command" + "github.com/creachadair/flax" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/client" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Exercise the real flag parser: folded YAML shell continuations used to turn +// --postgres into a positional argument, silently selecting SQLite. +func TestRunArguments(t *testing.T) { + tests := []struct { + name string + args []string + postgres bool + wantErr error + }{ + { + name: "postgres before selector", + args: []string{"--postgres=1", "--timeout=15m", "^TestHeadscale$"}, + postgres: true, + }, + { + name: "postgres after selector", + args: []string{"^TestHeadscale$", "--postgres", "--timeout=15m"}, + postgres: true, + }, + { + name: "sqlite", + args: []string{"--postgres=0", "--timeout=15m", "--test=^TestHeadscale$"}, + }, + { + name: "folded shell continuations", + args: []string{"^TestHeadscale$", " --timeout=15m", " --postgres=1"}, + wantErr: ErrUnexpectedTestArguments, + }, + { + name: "extra selector", + args: []string{"^TestHeadscale$", "TestNodeCommand"}, + wantErr: ErrUnexpectedTestArguments, + }, + { + name: "positional selector with test flag", + args: []string{"--test=^TestHeadscale$", "TestNodeCommand"}, + wantErr: ErrUnexpectedTestArguments, + }, + { + name: "missing selector", + wantErr: ErrTestPatternRequired, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var config RunConfig + + cmd := command.C{ + Name: "run", + SetFlags: command.Flags(flax.MustBind, &config), + Run: func(env *command.Env) error { return config.validate(env.Args) }, + } + + err := command.Run(cmd.NewEnv(nil).MergeFlags(true), tt.args) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.postgres, config.UsePostgres) + assert.Equal(t, "^TestHeadscale$", config.TestPattern) + assert.Equal(t, 15*time.Minute, config.Timeout) + }) + } +} + +func TestCompiledTestContainer(t *testing.T) { + t.Setenv("HEADSCALE_INTEGRATION_HEADSCALE_IMAGE", "headscale:ci-test") + // Neither inherited backend settings nor Go cache mounts should override + // the requested backend or leak into a compiled test execution. + t.Setenv("HEADSCALE_INTEGRATION_POSTGRES", "0") + t.Setenv("HEADSCALE_INTEGRATION_GO_CACHE", "/unused/go") + t.Setenv("HEADSCALE_INTEGRATION_GO_BUILD_CACHE", "/unused/build") + + binary := filepath.Join(t.TempDir(), "integration.test") + require.NoError(t, os.WriteFile(binary, []byte("test executable"), 0o700)) //nolint:gosec // The fixture must be executable to validate --test-binary. + config := RunConfig{ + TestPattern: "^TestAutoApproveMultiNetwork/webauth-user.*$", + TestBinary: binary, + UsePostgres: true, + Timeout: 15 * time.Minute, + FailFast: true, + } + require.NoError(t, config.validate(nil)) + + type createRequest struct { + container.Config + + HostConfig container.HostConfig + } + + requests := make(chan createRequest, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request createRequest + + err := json.NewDecoder(r.Body).Decode(&request) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + requests <- request + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"Id":"test-runner","Warnings":[]}`)) + })) + t.Cleanup(server.Close) + cli, err := client.NewClientWithOpts(client.WithHost(server.URL), client.WithVersion("1.44")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, cli.Close()) }) + + const runID = "20261007-120000-abcdef" + + logs := t.TempDir() + _, err = createTestContainer(t.Context(), cli, &config, + "headscale-test-suite-"+runID, logs, buildTestCommand(&config)) + require.NoError(t, err) + + request := <-requests + assert.Equal(t, "headscale:ci-test", request.Image) + assert.Equal(t, []string{ + "/integration.test", "-test.run", config.TestPattern, + "-test.failfast", "-test.timeout", "15m0s", "-test.v", + }, []string(request.Cmd)) + assert.Contains(t, request.Env, "HEADSCALE_INTEGRATION_POSTGRES=1") + assert.NotContains(t, request.Env, "HEADSCALE_INTEGRATION_POSTGRES=0") + assert.Contains(t, request.Env, "HEADSCALE_INTEGRATION_RUN_ID="+runID) + assert.NotContains(t, request.Env, "GOCACHE=/cache/go-build") + assert.Equal(t, runID, request.Labels["hi.run-id"]) + assert.Equal(t, "integration", filepath.Base(request.WorkingDir)) + assert.Contains(t, request.HostConfig.Binds, logs+":/tmp/control") + assert.NotContains(t, request.HostConfig.Binds, "/unused/go:/go") + assert.Equal(t, []mount.Mount{{ + Type: mount.TypeBind, Source: binary, Target: "/integration.test", ReadOnly: true, + }}, request.HostConfig.Mounts) +} + +func TestSourceTestCommand(t *testing.T) { + t.Setenv("HEADSCALE_INTEGRATION_HEADSCALE_IMAGE", "headscale:ci-test") + + config := RunConfig{ + TestPattern: "^TestHeadscale$", GoVersion: "1.27.0", Timeout: time.Minute, + } + assert.Equal(t, []string{"go", "test", "./...", "-run", "^TestHeadscale$", "-timeout", "1m0s", "-v"}, buildTestCommand(&config)) + assert.Equal(t, "golang:1.27.0", testRunnerImage(&config)) + + config.TestBinary = "/integration.test" + + t.Setenv("HEADSCALE_INTEGRATION_HEADSCALE_IMAGE", "") + assert.Equal(t, "golang:1.27.0", testRunnerImage(&config)) +} + +func TestStreamAndWaitDrainsFinalOutput(t *testing.T) { + output, err := os.CreateTemp(t.TempDir(), "test-output") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, output.Close()) }) + + stdout := os.Stdout + os.Stdout = output + + t.Cleanup(func() { os.Stdout = stdout }) + + waitSent := make(chan struct{}) + releaseLogs := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/logs"): + w.Header().Set("Content-Type", "application/vnd.docker.raw-stream") + w.WriteHeader(http.StatusOK) + _ = http.NewResponseController(w).Flush() + + select { + case <-releaseLogs: + _, _ = w.Write([]byte("--- PASS: TestExample (0.01s)\nPASS\n")) + case <-r.Context().Done(): + } + case strings.HasSuffix(r.URL.Path, "/wait"): + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"StatusCode":0}`)) + _ = http.NewResponseController(w).Flush() + + close(waitSent) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + cli, err := client.NewClientWithOpts(client.WithHost(server.URL), client.WithVersion("1.44")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, cli.Close()) }) + + type result struct { + exitCode int + err error + } + + done := make(chan result, 1) + + go func() { + code, err := streamAndWait(t.Context(), cli, "test-runner") + done <- result{code, err} + }() + + <-waitSent + + // Hold back the last log packet after Docker has reported the exit code. + select { + case <-done: + close(releaseLogs) + t.Fatal("returned before the final test output was delivered") + case <-time.After(50 * time.Millisecond): + } + + close(releaseLogs) + + got := <-done + require.NoError(t, got.err) + assert.Zero(t, got.exitCode) + + data, err := os.ReadFile(output.Name()) + require.NoError(t, err) + assert.Equal(t, "--- PASS: TestExample (0.01s)\nPASS\n", string(data)) +} diff --git a/flake.nix b/flake.nix index 4735abd14..a9e83e004 100644 --- a/flake.nix +++ b/flake.nix @@ -198,17 +198,18 @@ goChecks = { build = fc.goBuild (common // { subPackages = [ "cmd/headscale" ]; }); - # The pure unit subset. ./integration (Docker) and + # The pure unit subset. The root ./integration package (Docker) and # ./hscontrol/servertest (slow: 10s+ convergence plus race/stress/HA # property tests — run by the servertest workflow instead) are dropped # from the test set but kept in source so cmd/hi and friends still - # compile; TestPostgres* needs a server (the SQLite equivalents still + # compile. Pure integration helper tests remain included. + # TestPostgres* needs a server (the SQLite equivalents still # run). CGO off matches the build. gotest = fc.goTest ( common // { testExclude = [ - "/integration" + "/integration$" "/hscontrol/servertest" ]; goSkip = [ "TestPostgres" ]; diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go index 7b6b1e7c7..d71ea18c3 100644 --- a/integration/auth_oidc_test.go +++ b/integration/auth_oidc_test.go @@ -938,6 +938,9 @@ func TestOIDCFollowUpUrl(t *testing.T) { _, err = doLoginURL(ts.Hostname(), newUrl) require.NoError(t, err) + err = ts.WaitForRunning(integrationutil.PeerSyncTimeout()) + require.NoError(t, err) + listUsers, err = headscale.ListUsers() require.NoError(t, err) assert.Len(t, listUsers, 1) diff --git a/integration/tsic/login_test.go b/integration/tsic/login_test.go new file mode 100644 index 000000000..552a1bf94 --- /dev/null +++ b/integration/tsic/login_test.go @@ -0,0 +1,171 @@ +package tsic + +import ( + "context" + "fmt" + "io" + "testing" + "time" + + "github.com/juanfont/headscale/hscontrol/util" + "github.com/juanfont/headscale/integration/dockertestutil" + "github.com/ory/dockertest/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Simulate Docker's streamed output without requiring Docker or a shell. +func TestLoginReturnsURLBeforeCLIExit(t *testing.T) { + const wantURL = "https://headscale.test/register/complete-token" + + for _, stream := range []string{"stdout", "stderr"} { + t.Run(stream, func(t *testing.T) { + partialWritten := make(chan struct{}) + completeURL := make(chan struct{}) + authenticate := make(chan struct{}) + + t.Cleanup(func() { close(authenticate) }) + + login := startLogin([]string{"tailscale", "up"}, func(_ []string, opts dockertest.ExecOptions) (int, error) { + output := opts.StdErr + if stream == "stdout" { + output = opts.StdOut + } + + _, _ = io.WriteString(output, "To authenticate, visit:\n\thttps://headscale.test/reg") + + close(partialWritten) + <-completeURL + + _, _ = io.WriteString(output, "ister/complete-token\n\n") + + <-authenticate + // A later URL must not block output draining or replace the first. + _, _ = io.WriteString(output, "https://headscale.test/register/another-token\nSuccess.\n") + + return 0, nil + }) + + <-partialWritten + + select { + case u := <-login.url: + t.Fatalf("returned incomplete URL: %s", u) + default: + } + + close(completeURL) + + u, err := login.waitForURL(time.Second) + require.NoError(t, err) + require.Equal(t, wantURL, u.String()) + + select { + case <-login.done: + t.Fatal("CLI exited before authentication") + default: + } + + // Permit natural completion and verify its exit status is retained. + authenticate <- struct{}{} + + select { + case <-login.done: + require.NoError(t, login.err) + assert.Contains(t, login.output.String(), "Success.") + case <-time.After(time.Second): + t.Fatal("CLI did not complete after authentication") + } + }) + } +} + +func TestLoginCommandFailure(t *testing.T) { + execErr := io.ErrClosedPipe + + for _, tt := range []struct { + name string + output string + exitCode int + err error + wantErr error + }{ + {name: "exec error", err: execErr, wantErr: execErr}, + {name: "CLI failure", output: "invalid login option", exitCode: 7, wantErr: dockertestutil.ErrDockertestCommandFailed}, + {name: "no URL", output: "Success.\n", wantErr: util.ErrNoURLFound}, + {name: "incomplete URL", output: "https://headscale.test/register/incomplete", wantErr: util.ErrNoURLFound}, + } { + t.Run(tt.name, func(t *testing.T) { + login := startLogin(nil, func(_ []string, opts dockertest.ExecOptions) (int, error) { + _, _ = io.WriteString(opts.StdErr, tt.output) + return tt.exitCode, tt.err + }) + _, err := login.waitForURL(time.Second) + require.ErrorIs(t, err, tt.wantErr) + assert.Contains(t, err.Error(), tt.output) + }) + } +} + +func TestLoginFailureAfterURL(t *testing.T) { + finish := make(chan struct{}) + + t.Cleanup(func() { close(finish) }) + + login := startLogin(nil, func(_ []string, opts dockertest.ExecOptions) (int, error) { + _, _ = io.WriteString(opts.StdErr, "https://headscale.test/register/token\n") + + <-finish + + _, _ = io.WriteString(opts.StdErr, "authentication rejected\n") + + return 1, nil + }) + + _, err := login.waitForURL(time.Second) + require.NoError(t, err) + + client := &TailscaleInContainer{hostname: "client", login: login} + // A URL alone is not a completed login. The caller's wait stays bounded. + require.ErrorIs(t, client.WaitForRunning(0), context.DeadlineExceeded) + + finish <- struct{}{} + + select { + case <-login.done: + for range 2 { + // All observers must see the failure, not just the first channel reader. + err = client.WaitForRunning(time.Second) + require.ErrorIs(t, err, dockertestutil.ErrDockertestCommandFailed) + assert.Contains(t, err.Error(), "authentication rejected") + } + case <-time.After(time.Second): + t.Fatal("CLI did not finish") + } +} + +func TestLoginURLDeadlineDoesNotStopCLI(t *testing.T) { + finish := make(chan struct{}) + + t.Cleanup(func() { close(finish) }) + + login := startLogin(nil, func(_ []string, opts dockertest.ExecOptions) (int, error) { + <-finish + + _, _ = fmt.Fprintln(opts.StdErr, "https://headscale.test/register/token") + + return 0, nil + }) + + _, err := login.waitForURL(0) + require.ErrorIs(t, err, dockertestutil.ErrDockertestCommandTimeout) + + finish <- struct{}{} + + select { + case <-login.done: + require.NoError(t, login.err) + case <-time.After(time.Second): + t.Fatal("CLI did not finish after URL deadline") + } +} diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index 1e5dd3f33..ca85c30e7 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -17,6 +17,7 @@ import ( "slices" "strconv" "strings" + "sync" "time" "github.com/cenkalti/backoff/v5" @@ -44,7 +45,9 @@ const ( dockerContextPath = "../." caCertRoot = "/usr/local/share/ca-certificates" dockerExecuteTimeout = 60 * time.Second - tailscaleBin = "tailscale" + // Some OIDC tests deliberately delay authentication for two minutes. + loginTimeout = 5 * time.Minute + tailscaleBin = "tailscale" ) // defaultPingTimeoutVal returns the per-attempt timeout for tailscale ping. @@ -59,17 +62,16 @@ func defaultPingTimeoutVal() time.Duration { } var ( - errTailscalePingFailed = errors.New("ping failed") - errTailscalePingNotDERP = errors.New("ping not via DERP") - errTailscaleNotLoggedIn = errors.New("tailscale not logged in") - errTailscaleWrongPeerCount = errors.New("wrong peer count") - errTailscaleCannotUpWithoutAuthkey = errors.New("cannot up without authkey") - errInvalidClientConfig = errors.New("verifiably invalid client config requested") - errInvalidTailscaleImageFormat = errors.New("invalid HEADSCALE_INTEGRATION_TAILSCALE_IMAGE format, expected repository:tag") - errTailscaleImageRequiredInCI = errors.New("HEADSCALE_INTEGRATION_TAILSCALE_IMAGE must be set in CI for HEAD version") - errContainerNotInitialized = errors.New("container not initialized") - errFQDNNotYetAvailable = errors.New("FQDN not yet available") - errCurlEmptyResponseBody = errors.New("curl returned empty response body") + errTailscalePingFailed = errors.New("ping failed") + errTailscalePingNotDERP = errors.New("ping not via DERP") + errTailscaleNotLoggedIn = errors.New("tailscale not logged in") + errTailscaleWrongPeerCount = errors.New("wrong peer count") + errInvalidClientConfig = errors.New("verifiably invalid client config requested") + errInvalidTailscaleImageFormat = errors.New("invalid HEADSCALE_INTEGRATION_TAILSCALE_IMAGE format, expected repository:tag") + errTailscaleImageRequiredInCI = errors.New("HEADSCALE_INTEGRATION_TAILSCALE_IMAGE must be set in CI for HEAD version") + errContainerNotInitialized = errors.New("container not initialized") + errFQDNNotYetAvailable = errors.New("FQDN not yet available") + errCurlEmptyResponseBody = errors.New("curl returned empty response body") ) const ( @@ -94,6 +96,10 @@ type TailscaleInContainer struct { ips []netip.Addr fqdn string + // Only the current attempt is checked: earlier logins may be deliberately + // abandoned or rejected before the test starts another one. + login *loginAttempt + // optional config caCerts [][]byte headscaleHostname string @@ -718,6 +724,7 @@ func (t *TailscaleInContainer) buildLoginCommand( func (t *TailscaleInContainer) Login( loginServer, authKey string, ) error { + t.login = nil command := t.buildLoginCommand(loginServer, authKey) if _, _, err := t.Execute(command, dockertestutil.ExecuteCommandTimeout(dockerExecuteTimeout)); err != nil { //nolint:noinlineerr @@ -732,34 +739,106 @@ func (t *TailscaleInContainer) Login( return nil } +// loginAttempt streams the first complete URL line while retaining all output. +// done closes only after Docker exec returns; err can then be read repeatedly. +type loginAttempt struct { + mu sync.Mutex + output bytes.Buffer + lineStart int + foundURL bool + url chan *url.URL + done chan struct{} + err error +} + +func (l *loginAttempt) Write(p []byte) (int, error) { + l.mu.Lock() + defer l.mu.Unlock() + + n, _ := l.output.Write(p) + + for !l.foundURL { + line, _, complete := bytes.Cut(l.output.Bytes()[l.lineStart:], []byte{'\n'}) + if !complete { + break + } + + l.lineStart += len(line) + 1 + + u, err := util.ParseLoginURLFromCLILogin(string(line)) + if err == nil { + l.foundURL = true + l.url <- u + } + } + + return n, nil +} + +func startLogin( + command []string, + exec func([]string, dockertest.ExecOptions) (int, error), +) *loginAttempt { + l := &loginAttempt{url: make(chan *url.URL, 1), done: make(chan struct{})} + + go func() { + defer close(l.done) + + exitCode, err := exec(command, dockertest.ExecOptions{StdOut: l, StdErr: l}) + log.Printf("%v finished (exit %d): %s", command, exitCode, l.output.String()) + + switch { + case err != nil: + l.err = fmt.Errorf("tailscale up: %s: %w", l.output.String(), err) + case exitCode != 0: + l.err = fmt.Errorf("tailscale up exited %d: %s: %w", exitCode, l.output.String(), dockertestutil.ErrDockertestCommandFailed) + case !l.foundURL: + l.err = fmt.Errorf("tailscale up: %s: %w", l.output.String(), util.ErrNoURLFound) + } + }() + + return l +} + +func (l *loginAttempt) waitForURL(timeout time.Duration) (*url.URL, error) { + select { + case u := <-l.url: + return u, nil + case <-l.done: + if l.err != nil { + return nil, l.err + } + + return <-l.url, nil + case <-time.After(timeout): + return nil, dockertestutil.ErrDockertestCommandTimeout + } +} + // LoginWithURL runs the login routine on the given Tailscale instance. // This login mechanism uses web + command line flow for authentication. func (t *TailscaleInContainer) LoginWithURL( loginServer string, ) (*url.URL, error) { command := t.buildLoginCommand(loginServer, "") + command = append(command, "--timeout="+loginTimeout.String()) - stdout, stderr, err := t.Execute(command) - if errors.Is(err, errTailscaleNotLoggedIn) { - return nil, errTailscaleCannotUpWithoutAuthkey - } + // Return the URL promptly, but let the CLI finish authentication normally. + // Its own timeout bounds abandoned logins; WaitForRunning checks its exit. + t.login = startLogin(command, t.container.Exec) - defer func() { - if err != nil { - log.Printf("join command: %q", strings.Join(command, " ")) - } - }() - - loginURL, err := util.ParseLoginURLFromCLILogin(stdout + stderr) + u, err := t.login.waitForURL(dockerExecuteTimeout) if err != nil { - return nil, err + return nil, fmt.Errorf("%s fetching login URL: %w", t.hostname, err) } - return loginURL, nil + return u, nil } // Logout runs the logout routine on the given Tailscale instance. func (t *TailscaleInContainer) Logout() error { + t.login = nil + _, _, err := t.Execute([]string{tailscaleBin, "logout"}) if err != nil { return err @@ -1268,7 +1347,7 @@ func (t *TailscaleInContainer) WaitForNeedsLogin(timeout time.Duration) error { } // WaitForRunning blocks until the Tailscale (tailscaled) instance is logged in -// and ready to be used. +// and ready to be used, and the current web login command has exited successfully. func (t *TailscaleInContainer) WaitForRunning(timeout time.Duration) error { return t.waitForBackendState("Running", timeout) } @@ -1280,6 +1359,17 @@ func (t *TailscaleInContainer) waitForBackendState(state string, timeout time.Du ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() + if state == "Running" && t.login != nil { + select { + case <-t.login.done: + if t.login.err != nil { + return fmt.Errorf("%s login failed: %w", t.hostname, t.login.err) + } + case <-ctx.Done(): + return fmt.Errorf("%s waiting for login command: %w", t.hostname, ctx.Err()) + } + } + for { select { case <-ctx.Done():