diff --git a/e2e/harness/client.go b/e2e/harness/client.go index f53d0ea64..0dac79c07 100644 --- a/e2e/harness/client.go +++ b/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, diff --git a/e2e/harness/combined.go b/e2e/harness/combined.go index b2f0d89d2..a1045a9a6 100644 --- a/e2e/harness/combined.go +++ b/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") }, diff --git a/e2e/harness/config.go b/e2e/harness/config.go index 71b3656c5..f0952b18c 100644 --- a/e2e/harness/config.go +++ b/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: diff --git a/e2e/harness/options_test.go b/e2e/harness/options_test.go new file mode 100644 index 000000000..0e55ac462 --- /dev/null +++ b/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) + } + } +} diff --git a/e2e/harness/paths.go b/e2e/harness/paths.go index d7df6bbfa..550b8ad49 100644 --- a/e2e/harness/paths.go +++ b/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 }