[e2e] Let a suite outside this repo use the harness

The harness package documents itself as feature-agnostic — "any suite can ask for
a live management server" — but three details assume the caller lives in this
repo, so the terraform provider's acceptance suite cannot use it and would have to
carry a second harness for the same product instead.

repoRoot walked up from the working directory to the first go.mod and used it as
the Docker build context. From another module that is the caller's own root, where
combined/Dockerfile.multistage does not exist. Require the ancestor to be this
module, and fall back to asking the go tool where this module's source is. For a
dependent that is the extracted module directory of the version it pins, which is
the right context: the server it tests against is then built from the same
revision as the client library it was compiled against.

Geolocation was off unconditionally, in both the container environment and
disableGeoliteUpdate. That is right for agent-network ingest, which does not use
it, but a suite asserting on location-based posture checks needs the database:
management evaluates those rules against it, and a rule it cannot evaluate fails
rather than passing without having been checked. StartCombined now takes options,
with WithGeolocation to keep the download, and WithServerEnv as the general escape
hatch for settings the harness does not model.

StartClient pinned a single network alias and set no hostname, so a second agent
could not start — the alias collides — and the peer's name was whatever the
container got. Both matter to a suite whose fixtures address peers by name:
management records the container hostname at registration, so that is the name the
peer appears under in the API. WithClientName sets alias and hostname together.

All three are additive. StartCombined(ctx) and StartClient(ctx, c, key) still
compile and behave as before, which the existing agent-network suite exercises.

The options configure a container environment and a config file, both assembled
before anything starts, so options_test.go checks them without Docker. A wiring
mistake would otherwise surface as a puzzling failure minutes into a container
run — and the argument order in the config format string is exactly the kind of
thing worth pinning, since a misplaced verb there fails the server's startup.

e2e/harness/client.go

@@ -32,11 +32,35 @@ type Client struct {
 	container testcontainers.Container
 }
 
+// clientOptions is what the ClientOption values assemble.
+type clientOptions struct {
+	name string
+}
+
+// ClientOption adjusts how StartClient runs the agent.
+type ClientOption func(*clientOptions)
+
+// WithClientName names the agent, which sets both its network alias and its
+// container hostname. The hostname matters beyond addressing: the agent reports
+// it to management at registration, so it is the name the peer appears under in
+// the API.
+//
+// Required to run more than one agent against the same server — the default name
+// is shared, and two containers cannot hold the same alias on one network.
+func WithClientName(name string) ClientOption {
+	return func(o *clientOptions) { o.name = name }
+}
+
 // StartClient builds the client image and runs it on the combined server's
 // network, joining via the given setup key. The image entrypoint brings the
 // daemon up automatically; callers wait for connectivity with WaitConnected /
 // WaitProxyPeer.
-func StartClient(ctx context.Context, c *Combined, setupKey string) (*Client, error) {
+func StartClient(ctx context.Context, c *Combined, setupKey string, opts ...ClientOption) (*Client, error) {
+	o := clientOptions{name: clientAlias}
+	for _, opt := range opts {
+		opt(&o)
+	}
+
 	root, err := repoRoot()
 	if err != nil {
 		return nil, err
@@ -47,9 +71,13 @@ func StartClient(ctx context.Context, c *Combined, setupKey string) (*Client, er
 	}
 
 	req := testcontainers.ContainerRequest{
-		Image:          clientImage,
+		Image: clientImage,
+		// The agent reports the container's hostname to management, so this is
+		// the name the peer is addressable by in the API as well as on the
+		// network. The entrypoint takes no hostname flag of its own.
+		Hostname:       o.name,
 		Networks:       []string{c.network.Name},
-		NetworkAliases: map[string][]string{c.network.Name: {clientAlias}},
+		NetworkAliases: map[string][]string{c.network.Name: {o.name}},
 		Env: map[string]string{
 			"NB_MANAGEMENT_URL": combinedExposedURL,
 			"NB_SETUP_KEY":      setupKey,

e2e/harness/combined.go

@@ -61,10 +61,67 @@ type Combined struct {
 	workDir string
 }
 
+// combinedOptions is what the CombinedOption values assemble.
+type combinedOptions struct {
+	geolocation bool
+	env         map[string]string
+}
+
+// CombinedOption adjusts how StartCombined boots the server. The defaults suit a
+// suite that only drives the API; the options exist for the ones that need more
+// of the product than that.
+type CombinedOption func(*combinedOptions)
+
+// WithGeolocation leaves the GeoLite database download enabled. It is off by
+// default because the download adds startup latency that most suites get nothing
+// for. A suite asserting on location-based posture checks needs it: management
+// evaluates those rules against the database, and without it the rule fails
+// instead of passing without having been checked.
+func WithGeolocation() CombinedOption {
+	return func(o *combinedOptions) { o.geolocation = true }
+}
+
+// WithServerEnv adds environment variables to the combined container, overriding
+// the defaults on a key collision. For settings this harness does not model
+// directly, so a suite needing one does not have to fork the harness to get it.
+func WithServerEnv(env map[string]string) CombinedOption {
+	return func(o *combinedOptions) {
+		if o.env == nil {
+			o.env = map[string]string{}
+		}
+		for k, v := range env {
+			o.env[k] = v
+		}
+	}
+}
+
+// combinedEnv is the combined container's environment: setup-PAT enabled so the
+// caller can mint an admin token through /api/setup, geolocation off unless the
+// suite asked for it, and whatever the suite added on top.
+func combinedEnv(o combinedOptions) map[string]string {
+	env := map[string]string{
+		"NB_SETUP_PAT_ENABLED": "true",
+	}
+	if !o.geolocation {
+		// Skip the GeoLite DB download — it blocks startup and agent-network
+		// ingest doesn't use geolocation.
+		env["NB_DISABLE_GEOLOCATION"] = "true"
+	}
+	for k, v := range o.env {
+		env[k] = v
+	}
+	return env
+}
+
 // StartCombined builds the combined server from its multistage Dockerfile and
 // boots it with setup-PAT enabled on a fresh shared network, returning once the
 // API is serving. The caller still owns minting the admin PAT via Bootstrap.
-func StartCombined(ctx context.Context) (*Combined, error) {
+func StartCombined(ctx context.Context, opts ...CombinedOption) (*Combined, error) {
+	var o combinedOptions
+	for _, opt := range opts {
+		opt(&o)
+	}
+
 	root, err := repoRoot()
 	if err != nil {
 		return nil, err
@@ -88,7 +145,7 @@ func StartCombined(ctx context.Context) (*Combined, error) {
 		return nil, fmt.Errorf("create work dir: %w", err)
 	}
 
-	cfg := fmt.Sprintf(combinedConfigYAML, combinedExposedURL, containerIssuer)
+	cfg := fmt.Sprintf(combinedConfigYAML, combinedExposedURL, !o.geolocation, containerIssuer)
 	if err := os.WriteFile(filepath.Join(workDir, "config.yaml"), []byte(cfg), 0o644); err != nil { //nolint:gosec // non-secret config, bind-mounted and read by the container
 		_ = net.Remove(ctx)
 		return nil, fmt.Errorf("write combined config: %w", err)
@@ -112,13 +169,8 @@ func StartCombined(ctx context.Context) (*Combined, error) {
 		ExposedPorts:   []string{combinedHTTPPort},
 		Networks:       []string{net.Name},
 		NetworkAliases: map[string][]string{net.Name: {combinedAlias}},
-		Env: map[string]string{
-			"NB_SETUP_PAT_ENABLED": "true",
-			// Skip the GeoLite DB download — it blocks startup and agent-network
-			// ingest doesn't use geolocation.
-			"NB_DISABLE_GEOLOCATION": "true",
-		},
-		Cmd: []string{"--config", "/nb/config.yaml"},
+		Env:            combinedEnv(o),
+		Cmd:            []string{"--config", "/nb/config.yaml"},
 		HostConfigModifier: func(hc *container.HostConfig) {
 			hc.Binds = append(hc.Binds, workDir+":/nb")
 		},

e2e/harness/config.go

@@ -15,6 +15,11 @@ package harness
 // server is required to load it — a broken path or malformed file fails startup
 // rather than silently falling back to the compiled-in rates, and TestMain then
 // fails with the container logs.
+//
+// disableGeoliteUpdate is a parameter rather than a fixed true because a suite
+// that exercises geolocation needs the database: management can only evaluate a
+// location rule with GeoLite loaded, and a rule it cannot evaluate fails rather
+// than passing vacuously. See WithGeolocation.
 const combinedConfigYAML = `server:
   listenAddress: ":8080"
   exposedAddress: "%s"
@@ -25,7 +30,7 @@ const combinedConfigYAML = `server:
   authSecret: "e2e-relay-secret"
   dataDir: "/nb/data"
   disableAnonymousMetrics: true
-  disableGeoliteUpdate: true
+  disableGeoliteUpdate: %t
   auth:
     issuer: "%s"
   store:

e2e/harness/options_test.go

@@ -0,0 +1,136 @@
+//go:build e2e
+
+package harness
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// The options exist so a suite can ask for a deployment this harness would not
+// otherwise give it. What they configure is a container environment and a config
+// file, both assembled before anything is started, so they are checkable without
+// Docker — which is the point: a wiring mistake here would otherwise only show up
+// as a puzzling failure minutes into a container run.
+
+func TestCombinedEnvGeolocation(t *testing.T) {
+	var off combinedOptions
+	if got := combinedEnv(off)["NB_DISABLE_GEOLOCATION"]; got != "true" {
+		t.Errorf("geolocation should be off by default, NB_DISABLE_GEOLOCATION = %q", got)
+	}
+
+	var on combinedOptions
+	WithGeolocation()(&on)
+	if _, set := combinedEnv(on)["NB_DISABLE_GEOLOCATION"]; set {
+		t.Error("WithGeolocation must leave NB_DISABLE_GEOLOCATION unset, so the server downloads the database")
+	}
+	if combinedEnv(on)["NB_SETUP_PAT_ENABLED"] != "true" {
+		t.Error("the setup PAT must stay enabled whatever else is configured; Bootstrap depends on it")
+	}
+}
+
+// The config file carries the same decision as the environment variable, and the
+// server needs both to agree: disableGeoliteUpdate suppresses the download even
+// when geolocation itself is enabled.
+func TestCombinedConfigGeolocation(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		opts []CombinedOption
+		want string
+	}{
+		{name: "default", want: "disableGeoliteUpdate: true"},
+		{name: "with geolocation", opts: []CombinedOption{WithGeolocation()}, want: "disableGeoliteUpdate: false"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var o combinedOptions
+			for _, opt := range tc.opts {
+				opt(&o)
+			}
+			cfg := fmt.Sprintf(combinedConfigYAML, combinedExposedURL, !o.geolocation, containerIssuer)
+			if !strings.Contains(cfg, tc.want) {
+				t.Errorf("config should contain %q, got:\n%s", tc.want, cfg)
+			}
+			// The issuer is the last verb; a mis-ordered argument list would put
+			// the boolean here instead and the server would fail to start.
+			if !strings.Contains(cfg, `issuer: "`+containerIssuer+`"`) {
+				t.Errorf("issuer not rendered, got:\n%s", cfg)
+			}
+		})
+	}
+}
+
+func TestWithServerEnvOverrides(t *testing.T) {
+	var o combinedOptions
+	WithServerEnv(map[string]string{"NB_LOG_LEVEL": "debug"})(&o)
+	WithServerEnv(map[string]string{"NB_SETUP_PAT_ENABLED": "false"})(&o)
+
+	env := combinedEnv(o)
+	if env["NB_LOG_LEVEL"] != "debug" {
+		t.Errorf("added variable missing, NB_LOG_LEVEL = %q", env["NB_LOG_LEVEL"])
+	}
+	if env["NB_SETUP_PAT_ENABLED"] != "false" {
+		t.Errorf("a suite must be able to override a default, NB_SETUP_PAT_ENABLED = %q", env["NB_SETUP_PAT_ENABLED"])
+	}
+}
+
+// Two agents on one network cannot share an alias, so the name has to reach both
+// the alias and the hostname. The hostname is the one management records, so it is
+// also what the peer is addressable by through the API.
+func TestWithClientName(t *testing.T) {
+	o := clientOptions{name: clientAlias}
+	if o.name != "client" {
+		t.Fatalf("unexpected default client name %q", o.name)
+	}
+
+	WithClientName("peer2")(&o)
+	if o.name != "peer2" {
+		t.Errorf("WithClientName did not take, name = %q", o.name)
+	}
+}
+
+// repoRoot has to recognise this module rather than merely finding a go.mod, or a
+// suite in another module gets its own root and a build context without the
+// component Dockerfiles in it.
+func TestIsModule(t *testing.T) {
+	dir := t.TempDir()
+	other := filepath.Join(dir, "go.mod")
+	if err := os.WriteFile(other, []byte("module example.com/other\n\ngo 1.25\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if isModule(other, modulePath) {
+		t.Error("another module's go.mod must not be taken for this repo")
+	}
+
+	ours := filepath.Join(dir, "ours.mod")
+	if err := os.WriteFile(ours, []byte("// a comment\n\nmodule "+modulePath+"\n\ngo 1.25\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if !isModule(ours, modulePath) {
+		t.Error("this repo's go.mod was not recognised")
+	}
+
+	if isModule(filepath.Join(dir, "absent.mod"), modulePath) {
+		t.Error("a missing go.mod must not report a match")
+	}
+}
+
+// Running from inside the repo, repoRoot finds it by walking up — the module
+// lookup is only the fallback, and this asserts the walk still wins so an
+// in-repo run never depends on the module cache.
+func TestRepoRootFindsThisRepo(t *testing.T) {
+	root, err := repoRoot()
+	if err != nil {
+		t.Fatalf("repoRoot: %v", err)
+	}
+	if !isModule(filepath.Join(root, "go.mod"), modulePath) {
+		t.Errorf("repoRoot returned %s, which is not this module", root)
+	}
+	for _, f := range []string{combinedDockerfile, clientDockerfile} {
+		if _, err := os.Stat(filepath.Join(root, f)); err != nil {
+			t.Errorf("%s is not present under the reported root %s: %v", f, root, err)
+		}
+	}
+}

e2e/harness/paths.go

@@ -3,27 +3,75 @@
 package harness
 
 import (
+	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"strings"
 )
 
-// repoRoot walks up from the working directory to the module root (the
-// directory holding go.mod), so the Docker build context is correct no matter
-// which package the test runs from.
+// modulePath is this module, used both to recognise the repo when walking up
+// from the working directory and to locate it when the suite lives elsewhere.
+const modulePath = "github.com/netbirdio/netbird"
+
+// repoRoot returns the directory the component Dockerfiles are built from.
+//
+// Walking up from the working directory finds it for any test inside this repo,
+// no matter which package it runs from. A suite in another module gets a
+// different answer that way — its own module root, where combined/Dockerfile
+// does not exist — so the ancestor has to be this module and not merely some
+// module. When it is not, the build context is the extracted module directory of
+// whichever version that suite depends on, which is the right one: the server it
+// tests against is then built from the same revision as the client library it
+// was compiled with.
 func repoRoot() (string, error) {
 	dir, err := os.Getwd()
 	if err != nil {
 		return "", err
 	}
 	for {
-		if _, statErr := os.Stat(filepath.Join(dir, "go.mod")); statErr == nil {
+		if isModule(filepath.Join(dir, "go.mod"), modulePath) {
 			return dir, nil
 		}
 		parent := filepath.Dir(dir)
 		if parent == dir {
-			return "", fmt.Errorf("go.mod not found above %s", dir)
+			break
 		}
 		dir = parent
 	}
+	return moduleDir()
+}
+
+// isModule reports whether the go.mod at path declares the given module.
+func isModule(path, want string) bool {
+	b, err := os.ReadFile(path)
+	if err != nil {
+		return false
+	}
+	for _, line := range strings.Split(string(b), "\n") {
+		if rest, ok := strings.CutPrefix(strings.TrimSpace(line), "module "); ok {
+			return strings.TrimSpace(rest) == want
+		}
+	}
+	return false
+}
+
+// moduleDir asks the go tool where this module's source is, which for a
+// dependent module is its extracted copy in the module cache. The cache is
+// read-only, and a Docker build context is only ever read.
+func moduleDir() (string, error) {
+	cmd := exec.CommandContext(context.Background(), "go", "list", "-m", "-f", "{{.Dir}}", modulePath)
+	out, err := cmd.Output()
+	if err != nil {
+		return "", fmt.Errorf("locate %s: %w", modulePath, err)
+	}
+	dir := strings.TrimSpace(string(out))
+	if dir == "" {
+		return "", fmt.Errorf("locate %s: the go tool reported no directory; run `go mod download %s`", modulePath, modulePath)
+	}
+	if _, err := os.Stat(dir); err != nil {
+		return "", fmt.Errorf("locate %s: %w", modulePath, err)
+	}
+	return dir, nil
 }