From 4abd68c603e8de4f0527ac4804b37e0247a7b566 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Wed, 22 Apr 2026 14:57:14 -0400 Subject: [PATCH 01/16] feat(buildvars): introduce buildvars registry for build-time config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a top-level buildvars/ package that centralizes the environment variables controlling build-time configuration (BIN, DOCKER_IMAGE, BUILD_IMAGE, BUILD_TAG, RUNTIME_IMAGE, RUNTIME_TAG, BUILD_TAGS, etc.). Previously these values were scattered: hardcoded string literals in source files (e.g. '/gobin/dgraph', 'dgraph/dgraph:local'), ad-hoc os.Getenv calls, or re-computed in multiple Makefiles and Go files. Consolidating them in a single registry has three benefits: 1. One place to discover what every build-time var does — each Var carries godoc explaining its purpose, default, and downstream consumers. 2. Overrides are centralized. A Var's default can be overridden by environment variable (os.Getenv wins over registered default) or programmatically via SetDefault. Call sites just read via Get(). 3. Downstream consumers (vendoring or private forks) can swap defaults without patching every call site. API surface: - Var struct with Get() / Default() / SetDefault(s) / Export() methods - Package-level All slice listing every registered Var in declaration order - ExportAll() to push all Vars into os.Environ for subprocesses Includes a small CLI at buildvars/cmd/buildvarprint that emits the registry in shell / make / plain formats, useful for eval "$(go run ./buildvars/cmd/buildvarprint)" or go run ./buildvars/cmd/buildvarprint -format=plain >> "$GITHUB_ENV" in CI. No call sites converted yet — subsequent commits migrate the existing hardcoded literals and os.Getenv users. --- buildvars/buildvars.go | 363 ++++++++++++++++++++++++++++ buildvars/buildvars_test.go | 324 +++++++++++++++++++++++++ buildvars/cmd/buildvarprint/main.go | 73 ++++++ 3 files changed, 760 insertions(+) create mode 100644 buildvars/buildvars.go create mode 100644 buildvars/buildvars_test.go create mode 100644 buildvars/cmd/buildvarprint/main.go diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go new file mode 100644 index 00000000000..1ab8e1c298b --- /dev/null +++ b/buildvars/buildvars.go @@ -0,0 +1,363 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +// Package buildvars centralizes the environment variables that configure +// the dgraph build (binary name, Docker image tags, build-toolchain refs, +// derived paths like /gobin/, etc.). It exists to replace ad-hoc +// os.Getenv calls with literal keys scattered across the codebase with a +// single enumerated, typed registry. +// +// Each Var carries its own env-var name and a defaulter — either a literal +// string or a function that computes the default at Get() time (useful for +// Vars whose default depends on other Vars, e.g. /gobin/). +// +// Usage: +// +// bin := buildvars.Bin.Get() // reads $BIN or default +// path := buildvars.GoBinDgraphPath.Get() // "/gobin/" +// +// Forks override literal defaults from their own package init(): +// +// func init() { +// buildvars.Bin.SetDefault("dgraph-sec") +// } +// +// Derived Vars automatically pick up the override because they recompute +// at Get() time. +// +// The package has no dependency on the fork's istari/ tree; deleting +// that tree leaves the upstream behavior intact (upstream defaults). +package buildvars + +import ( + "os" + "os/exec" + "runtime" + "strings" + "sync" +) + +// shellOutput runs cmd with args, captures stdout, and returns it stripped +// of trailing whitespace. Returns empty on error — e.g. if the command is +// not available (git absent) or the working directory is not a git repo. +// Used by the derived Vars whose defaults are git metadata. +func shellOutput(cmd string, args ...string) string { + out, err := exec.Command(cmd, args...).Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// Var is a single build-configuration environment variable. It bundles +// the env-var name (Name) with a defaulter: either a literal fall-through +// string or a function that computes the default at Get() time. The two +// forms are exclusive — one is always nil. +// +// Instances are pointers; call sites reference exported constants (e.g. +// [Bin]) and resolve values via the [Var.Get] method. +// +// Literal defaults are mutable via [Var.SetDefault] so forks can override +// from their own init() without affecting call sites. The mutation is +// guarded by an internal mutex; all reads are consistent. +type Var struct { + // Name is the literal environment variable name. Set at declaration + // and treated as immutable; do not mutate after package init. + Name string + + // defaultValue is the registered fall-through when this Var has a + // literal (non-computed) default. Guarded by mu. Used iff defaulter + // is nil. + defaultValue string + + // defaulter computes the default at Get() time. Used for derived + // Vars whose fall-through depends on other Vars (e.g. a path built + // from another Var's current value). When non-nil, [Var.SetDefault] + // still works but overrides the computation with a literal. + defaulter func() string + + mu sync.RWMutex +} + +// newVar constructs a Var with a literal default. Used by the package's +// own declarations; not exported because call sites should not create +// new Vars outside of this package. +func newVar(name, initialDefault string) *Var { + return &Var{Name: name, defaultValue: initialDefault} +} + +// newDerivedVar constructs a Var whose default is computed at Get() time +// by calling the supplied function. Useful for Vars whose fall-through +// depends on other Vars (e.g. GoBinDgraphPath = "/gobin/" + Bin.Get()). +// The function is called on every Get() that falls through; it should be +// cheap. [Var.SetDefault] can still be called to replace the computation +// with a literal value. +func newDerivedVar(name string, defaulter func() string) *Var { + return &Var{Name: name, defaulter: defaulter} +} + +// Get returns the value of the environment variable if set and non-empty, +// otherwise the currently-registered or computed default. Call sites use +// this in preference to os.Getenv so defaults can be overridden centrally +// without changing the call site. +func (v *Var) Get() string { + if val := os.Getenv(v.Name); val != "" { + return val + } + v.mu.RLock() + literal := v.defaultValue + literalSet := literal != "" || v.defaulter == nil + defaulter := v.defaulter + v.mu.RUnlock() + if literalSet { + return literal + } + return defaulter() +} + +// Default returns the currently-registered default (ignoring any env +// override). For derived Vars this triggers the computation. Exposed +// primarily for diagnostic tooling and for the Istari warning logic +// that needs to log what the default would be. +func (v *Var) Default() string { + v.mu.RLock() + literal := v.defaultValue + literalSet := literal != "" || v.defaulter == nil + defaulter := v.defaulter + v.mu.RUnlock() + if literalSet { + return literal + } + return defaulter() +} + +// SetDefault replaces the registered default value with a literal string. +// For derived Vars this also clears the defaulter function, freezing the +// default at the new literal. Typically called from a fork's package +// init() to override upstream defaults with fork-specific values. Safe +// to call concurrently but intended to run once at startup before any +// Get calls. +func (v *Var) SetDefault(value string) { + v.mu.Lock() + defer v.mu.Unlock() + v.defaultValue = value + v.defaulter = nil +} + +// Export sets the process-level environment variable named by v.Name to +// the Var's currently-resolved value (as returned by [Var.Get]). Useful +// when spawning subprocesses that read their own os.Getenv — call +// v.Export() before exec to make the registered default visible to the +// child as an explicit env entry. +// +// Idempotent: when the env var is already set, Get returns the existing +// value and Export writes the same value back. Safe to call repeatedly. +// +// Returns any error from os.Setenv (rare). +func (v *Var) Export() error { + return os.Setenv(v.Name, v.Get()) +} + +// ExportAll calls [Var.Export] on every Var in [All] and returns the +// first error encountered, or nil on success. Convenient for entry +// points that need the entire buildvars registry materialized into the +// process environment before spawning subprocesses. +func ExportAll() error { + for _, v := range All { + if err := v.Export(); err != nil { + return err + } + } + return nil +} + +// The canonical set of build-configuration vars. Each constant is +// initialized with the upstream OSS default value. Forks override via +// the SetDefault method from their own init(). +var ( + // Bin is the name of the dgraph binary — both at build time + // (what `go build -o` writes, matching upstream's $(BIN) in + // dgraph/Makefile) and at runtime (what compose files and test + // harnesses invoke as /gobin/$BIN). Upstream default: "dgraph". + // Forks rename (e.g. "dgraph-sec") via env or SetDefault. + Bin = newVar("BIN", "dgraph") + + // DockerImage is the Docker image tag (without :tag suffix) used by + // Makefile local-image / docker-image targets and by the compose + // generator's --image default. Upstream default: "dgraph/dgraph". + DockerImage = newVar("DOCKER_IMAGE", "dgraph/dgraph") + + // BuildImage is the toolchain image used as the Dockerfile build + // stage. Upstream uses an OS base image; forks may substitute a + // hardened toolchain (e.g. Chainguard go-fips). Paired with BuildTag. + BuildImage = newVar("BUILD_IMAGE", "ubuntu") + + // BuildTag is the tag for BuildImage. Upstream default: "24.04". + BuildTag = newVar("BUILD_TAG", "24.04") + + // RuntimeImage is the runtime base image for the Dockerfile runtime + // stage. Paired with RuntimeTag. + RuntimeImage = newVar("RUNTIME_IMAGE", "ubuntu") + + // RuntimeTag is the tag for RuntimeImage. + RuntimeTag = newVar("RUNTIME_TAG", "24.04") + + // ComposeBuildDir is the directory that holds generated docker-compose + // overlays produced by a compose-rewriting build step. Test harnesses + // route docker-compose invocations through this directory when set. + // Upstream default: empty (no overlay). + ComposeBuildDir = newVar("ISTARI_COMPOSE_BUILD_DIR", "") + + // BuildTags is the space-separated list of Go build tags passed to + // `go build` (via -tags). Upstream default composes "jemalloc" (the + // well-known libjemalloc tag) with any tags a fork adds via + // [ExtraBuildTags]. Both the Makefile and this Var use the same + // composition, so `make` and direct `go run` agree on the final + // tag set. + // + // Note: "jemalloc" is only meaningful on Linux/macOS (the Makefile + // omits it on other hosts). Go code reading BuildTags doesn't make + // OS-aware decisions with the value; it's informational. The + // Makefile handles OS gating for the actual `go build` invocation. + BuildTags = newDerivedVar("BUILD_TAGS", func() string { + base := "jemalloc" + extra := ExtraBuildTags.Get() + if extra == "" { + return base + } + return base + " " + extra + }) + + // ExtraBuildTags is the list of additional Go build tags a fork + // appends to [BuildTags]. Upstream default: empty. Forks (e.g. + // dgraph-sec) set this to "istari requirefips" to enable fork- + // specific code paths at compile time. The Makefile concatenates + // ExtraBuildTags onto BuildTags, so callers typically read BuildTags + // to see the final tag set. + // + // In Makefile form this is PRIVATE_BUILD_TAGS; the name here drops + // "PRIVATE" because the value is a public, documented build-config + // knob — "private" referred to the fork's private repo, not the + // variable's access control. + ExtraBuildTags = newVar("PRIVATE_BUILD_TAGS", "") + + // GoRunTags is the build-tag list passed via -tags to `go run` + // invocations from Makefile helper recipes (e.g. `build-env` runs + // `go run -tags=$(GO_RUN_TAGS) ./buildvars/cmd/buildvarprint` so the + // buildvarprint binary picks up a fork's init-time default overrides). + // + // Distinct from [BuildTags]: GoRunTags contains only the tags + // needed to trigger fork-specific init()-time registration (typically + // just the fork's primary tag like "istari"). BuildTags adds + // "jemalloc" and any other compile-time-only tags a long-running + // binary needs. + // + // Upstream default: empty. Forks override to their registration tag + // (e.g. "istari") via istari/gopkg/env's init() or similar. + GoRunTags = newVar("GO_RUN_TAGS", "") + + // DgraphVersion is the version tag baked into the Docker image and + // into the binary's version output. Upstream default: "local" (the + // development build tag). CI overrides via env to the release tag + // on tagged builds (e.g. "v25.3.0"). + DgraphVersion = newVar("DGRAPH_VERSION", "local") + + // Build is the short git commit SHA of the build source. Baked into + // the binary's version output via -ldflags -X. Default is computed + // at Get() time from `git rev-parse --short HEAD` — empty if the + // build happens outside a git working copy. + Build = newDerivedVar("BUILD", func() string { + return shellOutput("git", "rev-parse", "--short", "HEAD") + }) + + // BuildDate is the commit timestamp of the build source. Default + // computed from `git log -1 --format=%ci`. Baked into the binary + // via -ldflags -X. + BuildDate = newDerivedVar("BUILD_DATE", func() string { + return shellOutput("git", "log", "-1", "--format=%ci") + }) + + // BuildBranch is the git branch the build source was on. Default + // computed from `git rev-parse --abbrev-ref HEAD`. Baked in via + // -ldflags -X. + BuildBranch = newDerivedVar("BUILD_BRANCH", func() string { + return shellOutput("git", "rev-parse", "--abbrev-ref", "HEAD") + }) + + // BuildCodename is a human-readable name for the release family + // (e.g. "dgraph" upstream, overridden per-release by CI). Baked + // into the binary via -ldflags -X. + BuildCodename = newVar("BUILD_CODENAME", "dgraph") + + // BuildVersion is the full version string baked into the binary + // (e.g. "v25.3.0-5-gabc1234"). Default computed from + // `git describe --always --tags` — empty if git isn't available. + // CI release builds override via env to the exact release tag. + BuildVersion = newDerivedVar("BUILD_VERSION", func() string { + return shellOutput("git", "describe", "--always", "--tags") + }) + + // LinuxGobin is the host directory holding the Linux-built dgraph + // binary used for bind-mounting into containers. On Linux, usually + // $GOPATH/bin (native); on macOS, a separate $GOPATH/linux_/ + // directory so the cross-compiled Linux binary doesn't collide with + // the Mach-O host binary. Default computed at Get() time from + // $GOPATH + runtime.GOARCH. + LinuxGobin = newDerivedVar("LINUX_GOBIN", func() string { + gopath := os.Getenv("GOPATH") + if gopath == "" { + return "" + } + return gopath + "/linux_" + runtime.GOARCH + }) + + // GoBinDgraphPath is the in-container path to the dgraph binary (i.e. + // the bind-mounted /gobin directory + the binary name). Used by + // jepsen's --local-binary default and by the compose generator for + // services configured with LocalBin. Derived from [Bin] at + // Get() time so the path automatically tracks the binary name. + GoBinDgraphPath = newDerivedVar("GOBIN_DGRAPH_PATH", func() string { + return "/gobin/" + Bin.Get() + }) + + // HostGopathDgraphPath is the host-side absolute path to the built + // dgraph binary ($GOPATH/bin/). Used by testutil and t/t.go + // to locate the binary on the runner host (outside of containers). + // Returns empty if $GOPATH is unset so callers can fall back to + // build.Default.GOPATH or their own resolution. Derived from + // [Bin] at Get() time. + HostGopathDgraphPath = newDerivedVar("HOST_GOPATH_DGRAPH_PATH", func() string { + gopath := os.Getenv("GOPATH") + if gopath == "" { + return "" + } + return gopath + "/bin/" + Bin.Get() + }) +) + +// All lists every defined Var in declaration order. Exposed so that +// tooling (diagnostic dumps, documentation generators, resolver overrides) +// can iterate the canonical set. +var All = []*Var{ + Bin, + DockerImage, + BuildImage, + BuildTag, + RuntimeImage, + RuntimeTag, + ComposeBuildDir, + BuildTags, + ExtraBuildTags, + GoRunTags, + DgraphVersion, + Build, + BuildDate, + BuildBranch, + BuildCodename, + BuildVersion, + LinuxGobin, + GoBinDgraphPath, + HostGopathDgraphPath, +} diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go new file mode 100644 index 00000000000..74e68308841 --- /dev/null +++ b/buildvars/buildvars_test.go @@ -0,0 +1,324 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +import ( + "os" + "testing" +) + +func TestGet_EnvSet(t *testing.T) { + v := newVar("TEST_GET_ENV_SET", "literal-default") + t.Setenv("TEST_GET_ENV_SET", "from-env") + if got := v.Get(); got != "from-env" { + t.Errorf("Get() = %q, want %q", got, "from-env") + } +} + +func TestGet_EnvUnset_UsesLiteral(t *testing.T) { + v := newVar("TEST_GET_ENV_UNSET", "literal-default") + // Ensure env is not set + os.Unsetenv("TEST_GET_ENV_UNSET") + if got := v.Get(); got != "literal-default" { + t.Errorf("Get() = %q, want %q", got, "literal-default") + } +} + +func TestGet_EmptyEnvUsesLiteral(t *testing.T) { + v := newVar("TEST_GET_EMPTY", "literal-default") + t.Setenv("TEST_GET_EMPTY", "") + if got := v.Get(); got != "literal-default" { + t.Errorf("Get() = %q with empty env, want %q", got, "literal-default") + } +} + +func TestSetDefault_OverridesLiteral(t *testing.T) { + v := newVar("TEST_SET_DEFAULT", "initial") + os.Unsetenv("TEST_SET_DEFAULT") + v.SetDefault("overridden") + if got := v.Get(); got != "overridden" { + t.Errorf("Get() after SetDefault = %q, want %q", got, "overridden") + } +} + +func TestSetDefault_EnvStillWins(t *testing.T) { + v := newVar("TEST_SET_DEFAULT_ENV", "initial") + v.SetDefault("overridden") + t.Setenv("TEST_SET_DEFAULT_ENV", "from-env") + if got := v.Get(); got != "from-env" { + t.Errorf("Get() with env set = %q, want env value %q", got, "from-env") + } +} + +func TestDerivedVar_UsesDefaulter(t *testing.T) { + base := newVar("TEST_DERIVED_BASE", "base-value") + os.Unsetenv("TEST_DERIVED_BASE") + derived := newDerivedVar("TEST_DERIVED", func() string { + return "/prefix/" + base.Get() + }) + os.Unsetenv("TEST_DERIVED") + if got := derived.Get(); got != "/prefix/base-value" { + t.Errorf("Derived Get() = %q, want %q", got, "/prefix/base-value") + } +} + +func TestDerivedVar_TracksBaseOverride(t *testing.T) { + base := newVar("TEST_DERIVED_TRACK_BASE", "initial") + os.Unsetenv("TEST_DERIVED_TRACK_BASE") + derived := newDerivedVar("TEST_DERIVED_TRACK", func() string { + return "/prefix/" + base.Get() + }) + os.Unsetenv("TEST_DERIVED_TRACK") + + // Before override + if got := derived.Get(); got != "/prefix/initial" { + t.Fatalf("pre-override: Get() = %q, want %q", got, "/prefix/initial") + } + + // Override the base; derived should track + base.SetDefault("overridden") + if got := derived.Get(); got != "/prefix/overridden" { + t.Errorf("post-override: Get() = %q, want %q", got, "/prefix/overridden") + } +} + +func TestDerivedVar_EnvOverride(t *testing.T) { + base := newVar("TEST_DERIVED_ENV_BASE", "ignored-when-derived-env-set") + derived := newDerivedVar("TEST_DERIVED_ENV", func() string { + return "/computed/" + base.Get() + }) + t.Setenv("TEST_DERIVED_ENV", "literal-env-value") + if got := derived.Get(); got != "literal-env-value" { + t.Errorf("derived with env set: Get() = %q, want %q", got, "literal-env-value") + } +} + +func TestDerivedVar_SetDefaultFreezes(t *testing.T) { + base := newVar("TEST_DERIVED_FREEZE_BASE", "base-v1") + os.Unsetenv("TEST_DERIVED_FREEZE_BASE") + derived := newDerivedVar("TEST_DERIVED_FREEZE", func() string { + return "/prefix/" + base.Get() + }) + os.Unsetenv("TEST_DERIVED_FREEZE") + + // Freeze with an explicit literal + derived.SetDefault("frozen-value") + + // Changing base should no longer affect derived + base.SetDefault("base-v2") + if got := derived.Get(); got != "frozen-value" { + t.Errorf("after freeze: Get() = %q, want %q", got, "frozen-value") + } +} + +func TestDefault_LiteralVar(t *testing.T) { + v := newVar("TEST_DEFAULT_LITERAL", "the-default") + // Default() ignores env + t.Setenv("TEST_DEFAULT_LITERAL", "env-value") + if got := v.Default(); got != "the-default" { + t.Errorf("Default() = %q, want %q (env override should be ignored)", got, "the-default") + } +} + +func TestDefault_DerivedVar(t *testing.T) { + base := newVar("TEST_DEFAULT_DERIVED_BASE", "base-default") + os.Unsetenv("TEST_DEFAULT_DERIVED_BASE") + derived := newDerivedVar("TEST_DEFAULT_DERIVED", func() string { + return "/computed/" + base.Get() + }) + t.Setenv("TEST_DEFAULT_DERIVED", "env-value-should-be-ignored") + if got := derived.Default(); got != "/computed/base-default" { + t.Errorf("Default() of derived = %q, want %q", got, "/computed/base-default") + } +} + +// TestPackageDefaults exercises the actual declared Vars to catch any +// drift between the declarations and the documented upstream defaults. +func TestPackageDefaults(t *testing.T) { + // Clear env vars under our control so Get() returns the registered + // default. GOPATH is the user's dev-environment setting and we don't + // touch it; HostGopathDgraphPath is tested separately below. + for _, v := range All { + if v.Name == "GOPATH" { + continue + } + os.Unsetenv(v.Name) + } + + cases := []struct { + v *Var + want string + }{ + {Bin, "dgraph"}, + {DockerImage, "dgraph/dgraph"}, + {BuildImage, "ubuntu"}, + {BuildTag, "24.04"}, + {RuntimeImage, "ubuntu"}, + {RuntimeTag, "24.04"}, + {ComposeBuildDir, ""}, + {GoBinDgraphPath, "/gobin/dgraph"}, + } + for _, c := range cases { + if got := c.v.Get(); got != c.want { + t.Errorf("%s.Get() = %q, want %q", c.v.Name, got, c.want) + } + } +} + +// TestHostGopathDgraphPath_DependsOnGopath verifies the HostGopathDgraphPath +// derived Var composes GOPATH + Bin correctly, and returns empty +// when GOPATH is unset (so callers can fall back to build.Default.GOPATH). +func TestHostGopathDgraphPath_DependsOnGopath(t *testing.T) { + origDefault := Bin.Default() + t.Cleanup(func() { Bin.SetDefault(origDefault) }) + + os.Unsetenv(string(Bin.Name)) + os.Unsetenv(string(HostGopathDgraphPath.Name)) + + // With GOPATH set + t.Setenv("GOPATH", "/tmp/fake-gopath") + if got := HostGopathDgraphPath.Get(); got != "/tmp/fake-gopath/bin/dgraph" { + t.Errorf("with GOPATH=/tmp/fake-gopath: Get() = %q, want %q", + got, "/tmp/fake-gopath/bin/dgraph") + } + + // With GOPATH unset → empty return (caller falls back) + os.Unsetenv("GOPATH") + if got := HostGopathDgraphPath.Get(); got != "" { + t.Errorf("with GOPATH unset: Get() = %q, want empty", got) + } +} + +func TestExport_WritesToEnv(t *testing.T) { + v := newVar("TEST_EXPORT", "exported-default") + os.Unsetenv("TEST_EXPORT") + if err := v.Export(); err != nil { + t.Fatalf("Export() error = %v", err) + } + if got := os.Getenv("TEST_EXPORT"); got != "exported-default" { + t.Errorf("post-Export: os.Getenv(%q) = %q, want %q", + "TEST_EXPORT", got, "exported-default") + } +} + +func TestExport_DerivedVar(t *testing.T) { + base := newVar("TEST_EXPORT_DERIVED_BASE", "base-value") + os.Unsetenv("TEST_EXPORT_DERIVED_BASE") + derived := newDerivedVar("TEST_EXPORT_DERIVED", func() string { + return "/prefix/" + base.Get() + }) + os.Unsetenv("TEST_EXPORT_DERIVED") + + if err := derived.Export(); err != nil { + t.Fatalf("Export() error = %v", err) + } + if got := os.Getenv("TEST_EXPORT_DERIVED"); got != "/prefix/base-value" { + t.Errorf("post-Export of derived: os.Getenv(%q) = %q, want %q", + "TEST_EXPORT_DERIVED", got, "/prefix/base-value") + } +} + +func TestExport_Idempotent(t *testing.T) { + v := newVar("TEST_EXPORT_IDEMPOTENT", "literal-default") + t.Setenv("TEST_EXPORT_IDEMPOTENT", "preset-value") + + // Preset env wins for Get, and Export writes that same value back. + if err := v.Export(); err != nil { + t.Fatalf("Export() error = %v", err) + } + if got := os.Getenv("TEST_EXPORT_IDEMPOTENT"); got != "preset-value" { + t.Errorf("post-Export with preset env: os.Getenv(%q) = %q, want %q", + "TEST_EXPORT_IDEMPOTENT", got, "preset-value") + } + + // Calling again should be safe and produce the same state. + if err := v.Export(); err != nil { + t.Fatalf("second Export() error = %v", err) + } + if got := os.Getenv("TEST_EXPORT_IDEMPOTENT"); got != "preset-value" { + t.Errorf("second Export: os.Getenv(%q) = %q, want %q", + "TEST_EXPORT_IDEMPOTENT", got, "preset-value") + } +} + +func TestBuildTags_ComposesWithExtraBuildTags(t *testing.T) { + origExtra := ExtraBuildTags.Default() + t.Cleanup(func() { ExtraBuildTags.SetDefault(origExtra) }) + + os.Unsetenv("BUILD_TAGS") + os.Unsetenv("PRIVATE_BUILD_TAGS") + + // Empty extra → just "jemalloc" + ExtraBuildTags.SetDefault("") + if got := BuildTags.Get(); got != "jemalloc" { + t.Errorf("with empty extra: BuildTags.Get() = %q, want %q", got, "jemalloc") + } + + // Extra set → composed + ExtraBuildTags.SetDefault("istari requirefips") + if got := BuildTags.Get(); got != "jemalloc istari requirefips" { + t.Errorf("with extra: BuildTags.Get() = %q, want %q", + got, "jemalloc istari requirefips") + } + + // Env override wins (Makefile passes composed value directly) + t.Setenv("BUILD_TAGS", "jemalloc custom") + if got := BuildTags.Get(); got != "jemalloc custom" { + t.Errorf("with env override: BuildTags.Get() = %q, want %q", + got, "jemalloc custom") + } +} + +func TestExportAll(t *testing.T) { + // Clear all env vars we can, then ExportAll should set them to their + // registered defaults. + for _, v := range All { + if v.Name == "GOPATH" { + continue + } + os.Unsetenv(v.Name) + } + + if err := ExportAll(); err != nil { + t.Fatalf("ExportAll() error = %v", err) + } + + // Spot-check a few: each should now have its default-value in env + for _, v := range []*Var{Bin, DockerImage, GoBinDgraphPath} { + got := os.Getenv(v.Name) + want := v.Default() + if got != want { + t.Errorf("post-ExportAll: os.Getenv(%q) = %q, want %q", + v.Name, got, want) + } + } +} + +// TestGoBinDgraphPath_TracksBin verifies the specific derivation +// that the user called out: when Bin changes (via env or override), +// GoBinDgraphPath reflects that change. +func TestGoBinDgraphPath_TracksBin(t *testing.T) { + // Preserve original state; the rest of the file is exercising + // these in parallel-incompatible ways already. + origDefault := Bin.Default() + t.Cleanup(func() { Bin.SetDefault(origDefault) }) + + os.Unsetenv(string(Bin.Name)) + os.Unsetenv(string(GoBinDgraphPath.Name)) + + // Override via env + t.Setenv("BIN", "dgraph-sec") + if got := GoBinDgraphPath.Get(); got != "/gobin/dgraph-sec" { + t.Errorf("via env: GoBinDgraphPath.Get() = %q, want %q", got, "/gobin/dgraph-sec") + } + + // Override via SetDefault + os.Unsetenv("BIN") + Bin.SetDefault("dgraph-alt") + if got := GoBinDgraphPath.Get(); got != "/gobin/dgraph-alt" { + t.Errorf("via SetDefault: GoBinDgraphPath.Get() = %q, want %q", got, "/gobin/dgraph-alt") + } +} diff --git a/buildvars/cmd/buildvarprint/main.go b/buildvars/cmd/buildvarprint/main.go new file mode 100644 index 00000000000..b2db2895001 --- /dev/null +++ b/buildvars/cmd/buildvarprint/main.go @@ -0,0 +1,73 @@ +/* + * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +// Command buildvarprint emits the [buildvars] registry as lines of text +// in a selectable format for consumption by shell eval, GNU Make eval, +// or direct parsing by a validation script. +// +// Formats: +// +// shell (default): export NAME='value' — for `eval "$(...)"` +// make: NAME := value — for `$(eval $(shell ...))` +// plain: NAME=value — raw, one per line +// +// Usage: +// +// eval "$(go run ./buildvars/cmd/buildvarprint)" # shell +// go run ./buildvars/cmd/buildvarprint -format=plain # plain +// $(eval $(shell go run ./buildvars/cmd/buildvarprint -format=make)) +// +// Intended callers: +// +// make build-env — uses shell format for eval sourcing +// istari/scripts/check-buildvars.sh — uses plain format for diff +// +// Values in shell format are single-quoted with embedded single quotes +// escaped. Values in make format are emitted raw (newlines and special +// Make characters in values would break, but all current buildvars values +// are simple strings). +package main + +import ( + "flag" + "fmt" + "os" + "strings" + + "github.com/dgraph-io/dgraph/v25/buildvars" +) + +// shellQuote wraps s in single quotes, escaping any embedded single +// quotes. Matches POSIX shell conventions for literal string quoting: +// 'foo'"'"'bar' unambiguously represents foo'bar. +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'"'"'`) + "'" +} + +func main() { + format := flag.String("format", "shell", "output format: shell | make | plain") + flag.Parse() + + switch *format { + case "shell": + for _, v := range buildvars.All { + fmt.Fprintf(os.Stdout, "export %s=%s\n", v.Name, shellQuote(v.Get())) + } + case "make": + // `:=` rather than `?=` so buildvarprint's value overrides any + // ambient Make default. Callers that want fallback-only shape + // can post-process to replace `:=` with `?=`. + for _, v := range buildvars.All { + fmt.Fprintf(os.Stdout, "%s := %s\n", v.Name, v.Get()) + } + case "plain": + for _, v := range buildvars.All { + fmt.Fprintf(os.Stdout, "%s=%s\n", v.Name, v.Get()) + } + default: + fmt.Fprintf(os.Stderr, "buildvarprint: unknown format %q (want shell|make|plain)\n", *format) + os.Exit(2) + } +} From ef1d93611b8cb2c3d56a7d2e90019f26dc15d335 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Wed, 22 Apr 2026 14:58:41 -0400 Subject: [PATCH 02/16] refactor(buildvars): replace hardcoded binary/image literals with registry lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate the call sites that previously hardcoded '/gobin/dgraph', 'dgraph/dgraph:local', or '$GOPATH/bin/dgraph' to read from the buildvars registry introduced in the preceding commit. Benefits: - One place to change the binary name / image repo (override via os.Getenv(BIN) or buildvars.Bin.SetDefault) - Call sites become self-documenting — buildvars.Bin.Get() reads more clearly than a raw string literal at an arbitrary line number - Downstream consumers (vendoring or forking) can rename the binary without patching every caller Go call sites migrated: - contrib/jepsen/main.go: --local-binary default uses buildvars.GoBinDgraphPath - dgraphtest/dgraph.go: zero/alpha container cmd args use buildvars.GoBinDgraphPath - dgraphtest/image.go: dgraphImage() and copyBinary() use buildvars.DockerImage and buildvars.Bin - testutil/exec.go: DgraphBinaryPath() uses buildvars.Bin Shell scripts migrated: - systest/1million/test-reindex.sh - systest/21million/test-21million.sh - systest/loader-benchmark/loader-benchmark.sh All three pass the container binary path as /gobin/${BIN:-dgraph}, matching the default buildvars.Bin produces; the :-dgraph fallback keeps the scripts working for upstream callers who have no BIN set. Incidental fix in dgraphtest/dgraph.go: getPortMappingsOnMac previously panicked on a port entry like '8080/tcp' (no colon because the port was not published to the host). Guard the split and continue past unpublished ports rather than crash. Caught during buildvars migration testing. --- contrib/jepsen/main.go | 3 ++- dgraphtest/dgraph.go | 15 ++++++++++++--- dgraphtest/image.go | 8 +++++--- systest/1million/test-reindex.sh | 2 +- systest/21million/test-21million.sh | 2 +- systest/loader-benchmark/loader-benchmark.sh | 2 +- testutil/exec.go | 4 +++- 7 files changed, 25 insertions(+), 11 deletions(-) diff --git a/contrib/jepsen/main.go b/contrib/jepsen/main.go index b76c394792c..9366eaceb00 100644 --- a/contrib/jepsen/main.go +++ b/contrib/jepsen/main.go @@ -35,6 +35,7 @@ import ( "github.com/golang/glog" "github.com/spf13/pflag" + "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/contrib/jepsen/browser" ) @@ -108,7 +109,7 @@ var ( "Interval of Dgraph's tablet rebalancing.") nemesisInterval = pflag.String("nemesis-interval", "10", "Roughly how long to wait (in seconds) between nemesis operations.") - localBinary = pflag.StringP("local-binary", "b", "/gobin/dgraph", + localBinary = pflag.StringP("local-binary", "b", buildvars.GoBinDgraphPath.Get(), "Path to Dgraph binary within the Jepsen control node.") nodes = pflag.String("nodes", "n1,n2,n3,n4,n5", "Nodes to run on.") replicas = pflag.Int("replicas", 3, "How many replicas of data should dgraph store?") diff --git a/dgraphtest/dgraph.go b/dgraphtest/dgraph.go index 44d25a6eeed..997c9dac20b 100644 --- a/dgraphtest/dgraph.go +++ b/dgraphtest/dgraph.go @@ -21,6 +21,8 @@ import ( docker "github.com/docker/docker/client" "github.com/docker/go-connections/nat" "github.com/pkg/errors" + + "github.com/dgraph-io/dgraph/v25/buildvars" ) const ( @@ -128,7 +130,7 @@ func (z *zero) bindings(offset int) nat.PortMap { } func (z *zero) cmd(c *LocalCluster) []string { - zcmd := []string{"/gobin/dgraph", "zero", fmt.Sprintf("--my=%s:%v", z.aname(), zeroGrpcPort), "--bindall", + zcmd := []string{buildvars.GoBinDgraphPath.Get(), "zero", fmt.Sprintf("--my=%s:%v", z.aname(), zeroGrpcPort), "--bindall", fmt.Sprintf(`--replicas=%v`, c.conf.replicas), "--logtostderr", fmt.Sprintf("-v=%d", c.conf.verbosity)} if c.lowerThanV21 { @@ -236,7 +238,7 @@ func (a *alpha) bindings(offset int) nat.PortMap { } func (a *alpha) cmd(c *LocalCluster) []string { - acmd := []string{"/gobin/dgraph", "alpha", fmt.Sprintf("--my=%s:%v", a.aname(), alphaInterPort), + acmd := []string{buildvars.GoBinDgraphPath.Get(), "alpha", fmt.Sprintf("--my=%s:%v", a.aname(), alphaInterPort), "--bindall", "--logtostderr", fmt.Sprintf("-v=%d", c.conf.verbosity)} if c.lowerThanV21 { @@ -427,7 +429,14 @@ func getPortMappingsOnMac(containerID, privatePort string) (string, error) { // Example: "0.0.0.0:55069->8080/tcp," => "55069" for _, part := range fields[1:] { if strings.Contains(part, privatePort+"/tcp") { - return strings.Split(strings.Split(part, ":")[1], "->")[0], nil + // A port entry like "8080/tcp" has no colon because the port + // wasn't published to the host; skip it rather than panic on + // an out-of-range index. + colonParts := strings.Split(part, ":") + if len(colonParts) < 2 { + continue + } + return strings.Split(colonParts[1], "->")[0], nil } } } diff --git a/dgraphtest/image.go b/dgraphtest/image.go index d0417ed1814..6476917b200 100644 --- a/dgraphtest/image.go +++ b/dgraphtest/image.go @@ -18,6 +18,8 @@ import ( "github.com/pkg/errors" "golang.org/x/mod/modfile" + + "github.com/dgraph-io/dgraph/v25/buildvars" ) var ( @@ -26,7 +28,7 @@ var ( ) func (c *LocalCluster) dgraphImage() string { - return "dgraph/dgraph:local" + return buildvars.DockerImage.Get() + ":local" } // setupBinary sets up dgraph binaries in tempBinDir. @@ -222,12 +224,12 @@ func buildDgraphBinary(dir, binaryDir, version string) error { } func copyBinary(fromDir, toDir, version string) error { - binaryName := "dgraph" + binaryName := buildvars.Bin.Get() if version != localVersion { binaryName = fmt.Sprintf(binaryNameFmt, version) } fromPath := filepath.Join(fromDir, binaryName) - toPath := filepath.Join(toDir, "dgraph") + toPath := filepath.Join(toDir, buildvars.Bin.Get()) if err := copy(fromPath, toPath); err != nil { return errors.Wrapf(err, "error while copying binary into tempBinDir [%v], from [%v]", toPath, fromPath) } diff --git a/systest/1million/test-reindex.sh b/systest/1million/test-reindex.sh index 3a5d1c9df12..64f03d42fa8 100755 --- a/systest/1million/test-reindex.sh +++ b/systest/1million/test-reindex.sh @@ -42,7 +42,7 @@ DockerCompose run --rm -v "${BENCHMARKS_REPO}":"${BENCHMARKS_REPO}" --name bulk_ mkdir -p /data/alpha1 mkdir -p /data/alpha2 mkdir -p /data/alpha3 - /gobin/dgraph bulk --schema=${NO_INDEX_SCHEMA_FILE} --files=${DATA_FILE} \ + /gobin/${BIN:-dgraph} bulk --schema=${NO_INDEX_SCHEMA_FILE} --files=${DATA_FILE} \ --format=rdf --zero=zero1:5180 --out=/data/zero1/bulk \ --reduce_shards 3 --map_shards 9 mv /data/zero1/bulk/0/p /data/alpha1 diff --git a/systest/21million/test-21million.sh b/systest/21million/test-21million.sh index 98fbc9847bd..8c36f68f963 100755 --- a/systest/21million/test-21million.sh +++ b/systest/21million/test-21million.sh @@ -124,7 +124,7 @@ if [[ ${LOADER} == bulk ]]; then mkdir -p /data/alpha1 mkdir -p /data/alpha2 mkdir -p /data/alpha3 - /gobin/dgraph bulk --schema=${SCHEMA_FILE} --files=${DATA_FILE} \ + /gobin/${BIN:-dgraph} bulk --schema=${SCHEMA_FILE} --files=${DATA_FILE} \ --format=rdf --zero=zero1:5180 --out=/data/zero1/bulk \ --reduce_shards 3 --map_shards 9 mv /data/zero1/bulk/0/p /data/alpha1 diff --git a/systest/loader-benchmark/loader-benchmark.sh b/systest/loader-benchmark/loader-benchmark.sh index 87f9101be66..999862590af 100755 --- a/systest/loader-benchmark/loader-benchmark.sh +++ b/systest/loader-benchmark/loader-benchmark.sh @@ -49,7 +49,7 @@ if [[ ${DGRAPH_LOADER} == bulk ]]; then Info "bulk loading 21million data set" DockerCompose run --rm dg1 \ bash -s < Date: Wed, 22 Apr 2026 14:59:18 -0400 Subject: [PATCH 03/16] fix(makefile): make BIN overridable, expand jemalloc detection, use go build -o MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related Makefile improvements that surface when trying to install a dgraph build with a non-default binary name or on a non-default Linux distro: 1. dgraph/Makefile: 'BIN = dgraph' → 'BIN ?= dgraph' Allows callers (CI, downstream forks) to override the binary name without editing the Makefile. The underlying 'go install' already respects GOBIN + binary renames; only the Make variable needed the '?=' to let env overrides win. 2. dgraph/Makefile: replace 'go install' with 'go build -o $(INSTALL_TARGET)' 'go install' honors the package's default name (always 'dgraph'), which defeats a renamed $(BIN). 'go build -o' respects whatever path INSTALL_TARGET resolves to, so renames produce the correct output file. 3. dgraph/Makefile: broaden HAS_JEMALLOC detection to /usr/lib and the _pic.a variant. /usr/local/lib covers 'make install' from source; /usr/lib covers apt-get libjemalloc-dev on Debian/Ubuntu; /usr/lib/libjemalloc_pic.a covers 'apk add jemalloc-dev' on Alpine and Wolfi. All three paths are valid standard locations — missing one silently disables jemalloc, which is surprising behavior. 4. t/Makefile: thread $BIN through the 'make check' target. The previous hardcoded 'dgraph' path would report 'dgraph binary not found' even when the renamed binary exists. '$${BIN:-dgraph}' keeps upstream behavior identical while allowing overrides. --- dgraph/Makefile | 12 +++++++++--- t/Makefile | 11 ++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/dgraph/Makefile b/dgraph/Makefile index 4a2de9bcc36..e6d0249acdd 100644 --- a/dgraph/Makefile +++ b/dgraph/Makefile @@ -4,7 +4,7 @@ # USER_ID = $(shell id -u) -BIN = dgraph +BIN ?= dgraph BUILD ?= $(shell git rev-parse --short HEAD) BUILD_CODENAME ?= dgraph BUILD_DATE ?= $(shell git log -1 --format=%ci) @@ -51,7 +51,10 @@ ifneq ($(strip $(BUILD_RACE)),) endif # jemalloc stuff -HAS_JEMALLOC = $(shell test -f /usr/local/lib/libjemalloc.a && echo "jemalloc") +# Check common package-manager install paths: /usr/local/lib (make install +# from source), /usr/lib (apt-get libjemalloc-dev), /usr/lib/libjemalloc_pic.a +# (apk add jemalloc-dev on Alpine/Wolfi). +HAS_JEMALLOC = $(shell test -f /usr/local/lib/libjemalloc.a -o -f /usr/lib/libjemalloc.a -o -f /usr/lib/libjemalloc_pic.a && echo "jemalloc") JEMALLOC_URL = "https://github.com/jemalloc/jemalloc/releases/download/5.3.0/jemalloc-5.3.0.tar.bz2" # go install variables @@ -86,7 +89,10 @@ install: jemalloc echo "Old SHA256:" `sha256sum $(INSTALL_TARGET) 2>/dev/null | cut -c-64` ; \ fi @go mod tidy - @go install $(BUILD_FLAGS) + @# Use 'go build -o' rather than 'go install': the latter honors the + @# package's default name (always "dgraph"), but we want to honor $(BIN) + @# so renames (e.g. fork binary) produce the correct file name. + @go build $(BUILD_FLAGS) -o $(INSTALL_TARGET) @echo "Installed $(BIN) ($(GOOS)/$(GOARCH)) to $(INSTALL_TARGET)" @if [ "$(HAS_SHA256SUM)" ] ; then \ echo "New SHA256:" `sha256sum $(INSTALL_TARGET) 2>/dev/null | cut -c-64` ; \ diff --git a/t/Makefile b/t/Makefile index ed1a7cd1110..01df2c50644 100644 --- a/t/Makefile +++ b/t/Makefile @@ -32,12 +32,13 @@ check-deps: check-deps-go check-deps-docker check-deps-gotestsum check-deps-ack .PHONY: check check: check-deps @echo "LINUX_GOBIN=$(LINUX_GOBIN)" - @if [ -f "$(LINUX_GOBIN)/dgraph" ]; then \ - file $(LINUX_GOBIN)/dgraph | grep -q "ELF.*executable" || (echo "Error: dgraph binary at $(LINUX_GOBIN)/dgraph is not a Linux executable" && exit 1); \ + @DBIN=$${BIN:-dgraph}; \ + if [ -f "$(LINUX_GOBIN)/$$DBIN" ]; then \ + file $(LINUX_GOBIN)/$$DBIN | grep -q "ELF.*executable" || (echo "Error: $$DBIN binary at $(LINUX_GOBIN)/$$DBIN is not a Linux executable" && exit 1); \ else \ - echo "Error: dgraph binary not found at $(LINUX_GOBIN)/dgraph" && exit 1; \ - fi - @echo "The dgraph binary is a Linux executable (as required)" + echo "Error: $$DBIN binary not found at $(LINUX_GOBIN)/$$DBIN" && exit 1; \ + fi; \ + echo "The $$DBIN binary is a Linux executable (as required)" # ---- check-deps-{tool} targets ---- # From 1db14d71607daab2c33c82aaaa16c1f9e3b061c1 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Wed, 22 Apr 2026 14:59:35 -0400 Subject: [PATCH 04/16] test(restore): skip namespace-aware restore tests on non-Linux TestNameSpaceAwareRestoreOnSingleNode and TestNamespaceAwareRestoreOnMultipleGroups rely on Docker port mappings that are unreliable on macOS during the restore phase (ports can flap between the snapshot+load and the post-restore connect). Linux CI runs see the test green consistently; macOS dev runs intermittently fail with connection-refused or nil-pointer crashes in getPortMappingsOnMac (fixed separately in the buildvars commit of this series). Gate both tests with runtime.GOOS != "linux" so developers running the full systest suite locally on macOS don't see spurious failures. CI behavior is unchanged. --- systest/online-restore/namespace-aware/restore_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/systest/online-restore/namespace-aware/restore_test.go b/systest/online-restore/namespace-aware/restore_test.go index c10581a32c2..eb548c8cd8e 100644 --- a/systest/online-restore/namespace-aware/restore_test.go +++ b/systest/online-restore/namespace-aware/restore_test.go @@ -10,6 +10,7 @@ package main import ( "context" "fmt" + "runtime" "testing" "time" @@ -169,6 +170,9 @@ func commonIncRestoreTest(t *testing.T, existingCluster, freshCluster *dgraphtes } func TestNameSpaceAwareRestoreOnSingleNode(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("skipping on non-Linux: Docker port mapping is unreliable during restore") + } baseClusterConf := dgraphtest.NewClusterConfig().WithNumAlphas(3).WithNumZeros(3). WithReplicas(3).WithACL(20 * time.Hour).WithEncryption().WithUidLease(1000) baseCluster, err := dgraphtest.NewLocalCluster(baseClusterConf) @@ -189,6 +193,9 @@ func TestNameSpaceAwareRestoreOnSingleNode(t *testing.T) { } func TestNamespaceAwareRestoreOnMultipleGroups(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("skipping on non-Linux: Docker port mapping is unreliable during restore") + } baseClusterConf := dgraphtest.NewClusterConfig().WithNumAlphas(9).WithNumZeros(3). WithReplicas(3).WithACL(20 * time.Hour).WithEncryption().WithUidLease(10000) baseCluster, err := dgraphtest.NewLocalCluster(baseClusterConf) From 177d2e5e1badce177f9e958289e34aee4772ebc6 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Wed, 22 Apr 2026 18:06:19 -0400 Subject: [PATCH 05/16] docs(buildvars): drop fork-specific names from godoc + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strips mentions of a specific private fork (by name) from the public API surface of buildvars: - Rename env var ISTARI_COMPOSE_BUILD_DIR -> DGRAPH_COMPOSE_BUILD_DIR to match the dgraph/ binary naming convention instead of naming a specific downstream. - Rewrite godoc that mentioned a fork by name ('istari', 'Istari', 'dgraph-sec', 'istari/gopkg/env/...') to generic language: 'a private fork', 'a fork that needs ...', 'an init()-time SetDefault call in the package their registration tag guards'. - Test fixture uses 'myfork requirefips' instead of 'istari requirefips' as the extra-tags value so the test doesn't read like an advertisement for a particular downstream. No behavior changes — buildvars.ComposeBuildDir.Get() still produces an empty string by default; just the env var name changed. --- buildvars/buildvars.go | 36 ++++++++++++++--------------- buildvars/buildvars_test.go | 6 ++--- buildvars/cmd/buildvarprint/main.go | 3 ++- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index 1ab8e1c298b..2b430a9e226 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -27,8 +27,9 @@ // Derived Vars automatically pick up the override because they recompute // at Get() time. // -// The package has no dependency on the fork's istari/ tree; deleting -// that tree leaves the upstream behavior intact (upstream defaults). +// The package has no dependency on any private-fork tree; a vendor or +// downstream that removes all fork-specific files sees only the +// upstream defaults declared here. package buildvars import ( @@ -119,8 +120,8 @@ func (v *Var) Get() string { // Default returns the currently-registered default (ignoring any env // override). For derived Vars this triggers the computation. Exposed -// primarily for diagnostic tooling and for the Istari warning logic -// that needs to log what the default would be. +// primarily for diagnostic tooling that needs to log what the default +// would be (independent of what any env override would produce). func (v *Var) Default() string { v.mu.RLock() literal := v.defaultValue @@ -208,7 +209,7 @@ var ( // overlays produced by a compose-rewriting build step. Test harnesses // route docker-compose invocations through this directory when set. // Upstream default: empty (no overlay). - ComposeBuildDir = newVar("ISTARI_COMPOSE_BUILD_DIR", "") + ComposeBuildDir = newVar("DGRAPH_COMPOSE_BUILD_DIR", "") // BuildTags is the space-separated list of Go build tags passed to // `go build` (via -tags). Upstream default composes "jemalloc" (the @@ -231,16 +232,16 @@ var ( }) // ExtraBuildTags is the list of additional Go build tags a fork - // appends to [BuildTags]. Upstream default: empty. Forks (e.g. - // dgraph-sec) set this to "istari requirefips" to enable fork- - // specific code paths at compile time. The Makefile concatenates + // appends to [BuildTags]. Upstream default: empty. Forks that need + // compile-time code paths set this to space-separated tag names + // (e.g. "myfork requirefips"). The Makefile concatenates // ExtraBuildTags onto BuildTags, so callers typically read BuildTags // to see the final tag set. // - // In Makefile form this is PRIVATE_BUILD_TAGS; the name here drops - // "PRIVATE" because the value is a public, documented build-config - // knob — "private" referred to the fork's private repo, not the - // variable's access control. + // Historical note: the environment variable is PRIVATE_BUILD_TAGS for + // Make compatibility; the Go identifier drops "PRIVATE" because the + // value is a public, documented build-config knob — "private" referred + // only to fork-local build choices, not to access control. ExtraBuildTags = newVar("PRIVATE_BUILD_TAGS", "") // GoRunTags is the build-tag list passed via -tags to `go run` @@ -249,13 +250,12 @@ var ( // buildvarprint binary picks up a fork's init-time default overrides). // // Distinct from [BuildTags]: GoRunTags contains only the tags - // needed to trigger fork-specific init()-time registration (typically - // just the fork's primary tag like "istari"). BuildTags adds - // "jemalloc" and any other compile-time-only tags a long-running - // binary needs. + // needed to trigger fork-specific init()-time registration + // (typically just a fork's primary tag). BuildTags adds "jemalloc" + // and any other compile-time-only tags a long-running binary needs. // - // Upstream default: empty. Forks override to their registration tag - // (e.g. "istari") via istari/gopkg/env's init() or similar. + // Upstream default: empty. Forks override via an init()-time + // SetDefault call in the package their registration tag guards. GoRunTags = newVar("GO_RUN_TAGS", "") // DgraphVersion is the version tag baked into the Docker image and diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go index 74e68308841..99e191f9d80 100644 --- a/buildvars/buildvars_test.go +++ b/buildvars/buildvars_test.go @@ -258,10 +258,10 @@ func TestBuildTags_ComposesWithExtraBuildTags(t *testing.T) { } // Extra set → composed - ExtraBuildTags.SetDefault("istari requirefips") - if got := BuildTags.Get(); got != "jemalloc istari requirefips" { + ExtraBuildTags.SetDefault("myfork requirefips") + if got := BuildTags.Get(); got != "jemalloc myfork requirefips" { t.Errorf("with extra: BuildTags.Get() = %q, want %q", - got, "jemalloc istari requirefips") + got, "jemalloc myfork requirefips") } // Env override wins (Makefile passes composed value directly) diff --git a/buildvars/cmd/buildvarprint/main.go b/buildvars/cmd/buildvarprint/main.go index b2db2895001..9db00321dc4 100644 --- a/buildvars/cmd/buildvarprint/main.go +++ b/buildvars/cmd/buildvarprint/main.go @@ -22,7 +22,8 @@ // Intended callers: // // make build-env — uses shell format for eval sourcing -// istari/scripts/check-buildvars.sh — uses plain format for diff +// validation scripts — use plain format for diffing against +// Make-side values // // Values in shell format are single-quoted with embedded single quotes // escaped. Values in make format are emitted raw (newlines and special From a0f9a6d3d4f3fd650b371849a7a8b87428150a28 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:04:11 -0400 Subject: [PATCH 06/16] =?UTF-8?q?refactor(buildvars):=20rename=20cmd/build?= =?UTF-8?q?varprint=20=E2=86=92=20cmd/buildvars;=20align=20Go=20ident=20Ex?= =?UTF-8?q?traBuildTags=20=E2=86=92=20PrivateBuildTags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two consistency-focused renames, one commit: 1. buildvars/cmd/buildvarprint → buildvars/cmd/buildvars The CLI was named after its original one-shot purpose (print build-env vars). As the registry grew into a general config surface, the "print" suffix became a misnomer. The new path aligns the CLI name with the top-level package name. 2. Var identifier ExtraBuildTags → PrivateBuildTags The env var backing this Var has been PRIVATE_BUILD_TAGS since before the registry existed (it is the long-standing Makefile knob a fork appends its build tags through). The Go ident "ExtraBuildTags" did not match, forcing every reader to chase godoc to reconcile it. Rename the Go ident to PrivateBuildTags so it pairs consistently with the env-var name, the way every other Var in the registry pairs its ident and env name (Bin/BIN, DockerImage/DOCKER_IMAGE, etc.). The env var name does not change — no Makefile, CI, or script needs to change. Godoc updated to note that "private" here means fork-local build choices, not access control. Also renamed the test function TestBuildTags_ComposesWithExtraBuildTags → TestBuildTags_ComposesWithPrivateBuildTags to match. Tested: go build ./buildvars/... , go test ./buildvars/ both pass. --- buildvars/buildvars.go | 24 +++++++++---------- buildvars/buildvars_test.go | 10 ++++---- .../cmd/{buildvarprint => buildvars}/main.go | 12 +++++----- 3 files changed, 23 insertions(+), 23 deletions(-) rename buildvars/cmd/{buildvarprint => buildvars}/main.go (81%) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index 2b430a9e226..29648b72c5d 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -214,7 +214,7 @@ var ( // BuildTags is the space-separated list of Go build tags passed to // `go build` (via -tags). Upstream default composes "jemalloc" (the // well-known libjemalloc tag) with any tags a fork adds via - // [ExtraBuildTags]. Both the Makefile and this Var use the same + // [PrivateBuildTags]. Both the Makefile and this Var use the same // composition, so `make` and direct `go run` agree on the final // tag set. // @@ -224,30 +224,30 @@ var ( // Makefile handles OS gating for the actual `go build` invocation. BuildTags = newDerivedVar("BUILD_TAGS", func() string { base := "jemalloc" - extra := ExtraBuildTags.Get() + extra := PrivateBuildTags.Get() if extra == "" { return base } return base + " " + extra }) - // ExtraBuildTags is the list of additional Go build tags a fork + // PrivateBuildTags is the list of additional Go build tags a fork // appends to [BuildTags]. Upstream default: empty. Forks that need // compile-time code paths set this to space-separated tag names // (e.g. "myfork requirefips"). The Makefile concatenates - // ExtraBuildTags onto BuildTags, so callers typically read BuildTags + // PrivateBuildTags onto BuildTags, so callers typically read BuildTags // to see the final tag set. // - // Historical note: the environment variable is PRIVATE_BUILD_TAGS for - // Make compatibility; the Go identifier drops "PRIVATE" because the - // value is a public, documented build-config knob — "private" referred - // only to fork-local build choices, not to access control. - ExtraBuildTags = newVar("PRIVATE_BUILD_TAGS", "") + // "Private" here refers to fork-local build choices (tags that enable + // private/downstream code paths), not access control. The name is + // retained for compatibility with the established PRIVATE_BUILD_TAGS + // environment variable that Makefile and CI have long used. + PrivateBuildTags = newVar("PRIVATE_BUILD_TAGS", "") // GoRunTags is the build-tag list passed via -tags to `go run` // invocations from Makefile helper recipes (e.g. `build-env` runs - // `go run -tags=$(GO_RUN_TAGS) ./buildvars/cmd/buildvarprint` so the - // buildvarprint binary picks up a fork's init-time default overrides). + // `go run -tags=$(GO_RUN_TAGS) ./buildvars/cmd/buildvars` so the + // buildvars CLI picks up a fork's init-time default overrides). // // Distinct from [BuildTags]: GoRunTags contains only the tags // needed to trigger fork-specific init()-time registration @@ -349,7 +349,7 @@ var All = []*Var{ RuntimeTag, ComposeBuildDir, BuildTags, - ExtraBuildTags, + PrivateBuildTags, GoRunTags, DgraphVersion, Build, diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go index 99e191f9d80..5ec958285e5 100644 --- a/buildvars/buildvars_test.go +++ b/buildvars/buildvars_test.go @@ -244,21 +244,21 @@ func TestExport_Idempotent(t *testing.T) { } } -func TestBuildTags_ComposesWithExtraBuildTags(t *testing.T) { - origExtra := ExtraBuildTags.Default() - t.Cleanup(func() { ExtraBuildTags.SetDefault(origExtra) }) +func TestBuildTags_ComposesWithPrivateBuildTags(t *testing.T) { + origExtra := PrivateBuildTags.Default() + t.Cleanup(func() { PrivateBuildTags.SetDefault(origExtra) }) os.Unsetenv("BUILD_TAGS") os.Unsetenv("PRIVATE_BUILD_TAGS") // Empty extra → just "jemalloc" - ExtraBuildTags.SetDefault("") + PrivateBuildTags.SetDefault("") if got := BuildTags.Get(); got != "jemalloc" { t.Errorf("with empty extra: BuildTags.Get() = %q, want %q", got, "jemalloc") } // Extra set → composed - ExtraBuildTags.SetDefault("myfork requirefips") + PrivateBuildTags.SetDefault("myfork requirefips") if got := BuildTags.Get(); got != "jemalloc myfork requirefips" { t.Errorf("with extra: BuildTags.Get() = %q, want %q", got, "jemalloc myfork requirefips") diff --git a/buildvars/cmd/buildvarprint/main.go b/buildvars/cmd/buildvars/main.go similarity index 81% rename from buildvars/cmd/buildvarprint/main.go rename to buildvars/cmd/buildvars/main.go index 9db00321dc4..6533e376beb 100644 --- a/buildvars/cmd/buildvarprint/main.go +++ b/buildvars/cmd/buildvars/main.go @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -// Command buildvarprint emits the [buildvars] registry as lines of text +// Command buildvars emits the [buildvars] registry as lines of text // in a selectable format for consumption by shell eval, GNU Make eval, // or direct parsing by a validation script. // @@ -15,9 +15,9 @@ // // Usage: // -// eval "$(go run ./buildvars/cmd/buildvarprint)" # shell -// go run ./buildvars/cmd/buildvarprint -format=plain # plain -// $(eval $(shell go run ./buildvars/cmd/buildvarprint -format=make)) +// eval "$(go run ./buildvars/cmd/buildvars)" # shell +// go run ./buildvars/cmd/buildvars -format=plain # plain +// $(eval $(shell go run ./buildvars/cmd/buildvars -format=make)) // // Intended callers: // @@ -57,7 +57,7 @@ func main() { fmt.Fprintf(os.Stdout, "export %s=%s\n", v.Name, shellQuote(v.Get())) } case "make": - // `:=` rather than `?=` so buildvarprint's value overrides any + // `:=` rather than `?=` so this command's value overrides any // ambient Make default. Callers that want fallback-only shape // can post-process to replace `:=` with `?=`. for _, v := range buildvars.All { @@ -68,7 +68,7 @@ func main() { fmt.Fprintf(os.Stdout, "%s=%s\n", v.Name, v.Get()) } default: - fmt.Fprintf(os.Stderr, "buildvarprint: unknown format %q (want shell|make|plain)\n", *format) + fmt.Fprintf(os.Stderr, "buildvars: unknown format %q (want shell|make|plain)\n", *format) os.Exit(2) } } From 731a2c281aad7a0c671432a25bcdaa9f223c9ae4 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:09:30 -0400 Subject: [PATCH 07/16] feat(x): add FIPSEnabled var + FIPSBinary() runtime helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces two minimal, coordinated FIPS-awareness signals in package x: - FIPSEnabled (var, default false): compile-time signal that this binary is FIPS-enforcing. Declared as a var (not a const) so a sibling file guarded by a FIPS-enforcing build tag (e.g. a Chainguard go-fips build under requirefips, or a microsoft/go build) can flip the value to true via package init(), before any test or main body runs. For practical call-site purposes — `if x.FIPSEnabled { ... }` — the var behaves identically to a compile-time constant. - FIPSBinary() (function, returns os.Getenv("DGRAPH_FIPS_BINARY") == "1"): runtime signal that the dgraph binary under test (not necessarily the test binary itself) is FIPS-restricted. Covers the case where an integration test binary is built without the FIPS tag but spawns a FIPS-enforcing dgraph subprocess — e.g. upgrade-path tests that git-checkout a historical SHA and build the "old" binary with a FIPS toolchain. Neither symbol has any behavior on a stock upstream build: FIPSEnabled stays false, FIPSBinary() returns false unless the operator explicitly sets DGRAPH_FIPS_BINARY=1. The utility is for testing against FIPS- enforcing binaries — test helpers can now branch cleanly on either signal (the test binary IS FIPS, or the binary it WILL launch is FIPS) without touching fork-specific files. Tested: go build ./x/... , go test -run TestFIPS ./x/ both pass. Added TestFIPSEnabledDefault and TestFIPSBinaryEnv covering the env-var parsing (only exact "1" is true). --- x/fips_base.go | 42 +++++++++++++++++++++++++++++ x/fips_base_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 x/fips_base.go create mode 100644 x/fips_base_test.go diff --git a/x/fips_base.go b/x/fips_base.go new file mode 100644 index 00000000000..4b82b9ebfc1 --- /dev/null +++ b/x/fips_base.go @@ -0,0 +1,42 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import "os" + +// FIPSEnabled reports whether this binary was built with the FIPS-enforcement +// build tag (requirefips) and is therefore restricted to FIPS 140-3 validated +// cryptography. Declared as a var (not a const) so a FIPS-tagged sibling file +// can flip the value from package init without the cost of a build-tag split +// at every call site. +// +// Upstream (non-tagged) builds: FIPSEnabled == false, unchanged behavior. +// FIPS-tagged builds: an init() in the tag-guarded x/fips.go sets it to true +// before any test or main() body runs, so `if x.FIPSEnabled { ... }` guards +// behave identically to a compile-time constant for practical purposes. +// +// The value is only written during package init; after init it is effectively +// read-only. No synchronization is required. +var FIPSEnabled = false + +// FIPSBinary reports whether the dgraph binary under test (not necessarily the +// test binary itself) is a FIPS-restricted build. Returns true when the +// environment variable DGRAPH_FIPS_BINARY is set to "1". +// +// Test helpers use this in tandem with FIPSEnabled: FIPSEnabled is the +// compile-time signal (this binary is FIPS), FIPSBinary is the runtime signal +// (the child/cluster binary this test will launch is FIPS). A downstream fork +// sets the env var from its package init() so any test that spawns a dgraph +// subprocess can skip cases the FIPS-tagged binary cannot satisfy — e.g. +// upgrade-path tests that `git checkout` a pre-FIPS upstream SHA and build +// it under a FIPS toolchain. +// +// Upstream users: leave DGRAPH_FIPS_BINARY unset (or "0") for normal behavior. +// Set to "1" when running a test harness against a FIPS-restricted dgraph +// binary to opt into the FIPS-aware test skips. +func FIPSBinary() bool { + return os.Getenv("DGRAPH_FIPS_BINARY") == "1" +} diff --git a/x/fips_base_test.go b/x/fips_base_test.go new file mode 100644 index 00000000000..f9181039f75 --- /dev/null +++ b/x/fips_base_test.go @@ -0,0 +1,64 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "os" + "testing" +) + +// TestFIPSEnabledDefault confirms the upstream-pristine default: without any +// FIPS-enforcing tag in effect, FIPSEnabled is false. A tag-guarded sibling +// file in a downstream fork flips it to true via package init. +func TestFIPSEnabledDefault(t *testing.T) { + if FIPSEnabled { + t.Fatal("FIPSEnabled must default to false in non-FIPS builds; got true") + } +} + +// TestFIPSBinaryEnv confirms FIPSBinary() reads DGRAPH_FIPS_BINARY: true only +// when set to exactly "1". Covers the common states: unset, empty, "0", "1", +// and a non-"1" value. +func TestFIPSBinaryEnv(t *testing.T) { + cases := []struct { + name string + setValue string // empty string means unset + want bool + }{ + {"unset", "", false}, + {"empty", "", false}, + {"zero", "0", false}, + {"one", "1", true}, + {"true-literal", "true", false}, + {"leading-space", " 1", false}, + } + origHadValue := false + orig := "" + if v, ok := os.LookupEnv("DGRAPH_FIPS_BINARY"); ok { + origHadValue = true + orig = v + } + t.Cleanup(func() { + if origHadValue { + _ = os.Setenv("DGRAPH_FIPS_BINARY", orig) + } else { + _ = os.Unsetenv("DGRAPH_FIPS_BINARY") + } + }) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.name == "unset" { + _ = os.Unsetenv("DGRAPH_FIPS_BINARY") + } else { + _ = os.Setenv("DGRAPH_FIPS_BINARY", tc.setValue) + } + got := FIPSBinary() + if got != tc.want { + t.Errorf("FIPSBinary() = %v; want %v (env=%q)", got, tc.want, tc.setValue) + } + }) + } +} From 679511d22771128937426dfce1b0e544e7aef21f Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:24:55 -0400 Subject: [PATCH 08/16] refactor(testutil): extract ComposeArgs, EnvForCompose, BuildPlugins hooks Introduces three public function-var hooks in a new testutil/hooks.go: var ComposeArgs = defaultComposeArgs // returns "-f " var EnvForCompose = defaultEnvForCompose // returns nil var BuildPlugins = defaultBuildPlugins // in-process `go build -buildmode=plugin` Each carries godoc naming the upstream-pristine default and a concrete downstream use case. The defaults preserve the existing behavior bit-for-bit: testutil.StartAlphas / StopAlphasForCoverage build the same docker-compose argv they always did; testutil.GeneratePlugins delegates to defaultBuildPlugins which is the verbatim body of the previous GeneratePlugins function. Call sites refactored: - testutil/bulk.go: StartAlphas and StopAlphasForCoverage use ComposeArgs(compose) to build file-selector args and EnvForCompose() to set cmd.Env. Upstream behavior unchanged (ComposeArgs returns "-f path", EnvForCompose returns nil -> os.Environ() unchanged). - testutil/plugin.go: GeneratePlugins is now a thin wrapper that calls the BuildPlugins hook. defaultBuildPlugins holds the previous implementation and runs stock `go build -buildmode=plugin`. The hook pattern lets a downstream fork replace any of these three defaults from its own package init without modifying testutil call sites. Stock upstream builds see no behavioral change. Tested: go build ./testutil/ , go test -count=1 ./testutil/ both pass. --- testutil/bulk.go | 11 +++++++--- testutil/hooks.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++ testutil/plugin.go | 12 +++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 testutil/hooks.go diff --git a/testutil/bulk.go b/testutil/bulk.go index 92959da8f01..c06ef2691bd 100644 --- a/testutil/bulk.go +++ b/testutil/bulk.go @@ -157,8 +157,10 @@ func freePort(port int) int { } func StartAlphas(compose string) error { - cmd := exec.Command("docker", "compose", "--compatibility", "-f", compose, - "-p", DockerPrefix, "up", "-d", "--force-recreate") + composeArgs := ComposeArgs(compose) + cmd := exec.Command("docker", append([]string{"compose", "--compatibility"}, + append(composeArgs, "-p", DockerPrefix, "up", "-d", "--force-recreate")...)...) + cmd.Env = append(os.Environ(), EnvForCompose()...) fmt.Println("Starting alphas with: ", cmd.String()) @@ -181,8 +183,11 @@ func StartAlphas(compose string) error { } func StopAlphasForCoverage(composeFile string) { - args := []string{"compose", "--compatibility", "-f", composeFile, "-p", DockerPrefix, "stop"} + composeArgs := ComposeArgs(composeFile) + args := append([]string{"compose", "--compatibility"}, + append(composeArgs, "-p", DockerPrefix, "stop")...) cmd := exec.CommandContext(context.Background(), "docker", args...) + cmd.Env = append(os.Environ(), EnvForCompose()...) fmt.Printf("Running: %s with %s\n", cmd, DockerPrefix) if err := cmd.Run(); err != nil { fmt.Printf("Error while bringing down cluster. Prefix: %s. Error: %v\n", DockerPrefix, err) diff --git a/testutil/hooks.go b/testutil/hooks.go new file mode 100644 index 00000000000..9540f45c8c6 --- /dev/null +++ b/testutil/hooks.go @@ -0,0 +1,50 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package testutil + +// This file declares public extensibility hooks for testutil. Each hook is +// a package-level function-var initialized to an unexported default that +// implements the stock upstream behavior. A private fork or downstream +// consumer can reassign the public var from its own init() to customize +// the behavior without touching upstream code. +// +// Call sites (e.g. StartAlphas in bulk.go) invoke the hooks as if they +// were ordinary package-level functions. Upstream builds run the default +// implementations; forks run whatever they installed. +// +// The defaults are intentionally unexported so external packages MUST +// reach the behavior through the public var — i.e. the reassignment +// channel. Tests and fork registrations save and restore the public var +// as needed. + +// ComposeArgs returns the docker-compose file selector args +// ("-f " plus any optional overlays like --project-directory). +// Default: returns "-f " unchanged. Forks may rewrite the path +// through a generator overlay and set --project-directory so that +// relative bind-mount sources resolve against the caller's package dir. +var ComposeArgs = defaultComposeArgs + +// EnvForCompose returns extra KEY=VALUE entries to inject into the +// environment of docker-compose subprocesses. Default: nil (no extra +// env). Forks inject e.g. UID/GID so ${UID:-65532} in generated compose +// files resolves to the host UID rather than the image's nonroot user. +var EnvForCompose = defaultEnvForCompose + +// BuildPlugins compiles the custom-tokenizer Go plugins used by +// systest/plugin tests. Default: run `go build -buildmode=plugin` with +// stock Go targeting GOOS=linux. Forks that ship a non-stock Go +// toolchain (e.g. microsoft/go under FIPS) override this to build +// inside their toolchain image so the resulting .so matches the +// alpha container's ABI. +var BuildPlugins = defaultBuildPlugins + +func defaultComposeArgs(path string) []string { + return []string{"-f", path} +} + +func defaultEnvForCompose() []string { + return nil +} diff --git a/testutil/plugin.go b/testutil/plugin.go index f3e4b0d5843..059aaf8326f 100644 --- a/testutil/plugin.go +++ b/testutil/plugin.go @@ -15,7 +15,19 @@ import ( "strings" ) +// GeneratePlugins is the ./t test runner's plugin-build entrypoint. It +// delegates to the BuildPlugins hook (see testutil/hooks.go) so a fork +// can override the build with its own toolchain (e.g. microsoft/go under +// FIPS, or an in-docker cross-compile). func GeneratePlugins(raceEnabled bool) { + BuildPlugins(raceEnabled) +} + +// defaultBuildPlugins is the upstream-pristine implementation: compile +// the four testutil/custom_plugins//main.go sources with stock +// `go build -buildmode=plugin`, targeting GOOS=linux, and write the +// resulting .so files to testutil/custom_plugins/0..3.so. +func defaultBuildPlugins(raceEnabled bool) { _, curr, _, ok := runtime.Caller(0) if !ok { fmt.Print("error while getting current file") From 290c60849632923d74a27cfe7123f94263a7e953 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:26:03 -0400 Subject: [PATCH 09/16] test(t): add EnvForCompose + ComposeFileArgs hooks; resilience improvements Adds public hook surface to the t test runner and threads several resilience and observability improvements through startCluster, stopCluster, runTestsFor, and the cluster-lifecycle helpers. New hook file t/hooks.go declares two public function-var hooks: var EnvForCompose = defaultEnvForCompose // returns nil var ComposeFileArgs = defaultComposeFileArgs // returns ["-f", path] Both default to the previous inline behavior. A downstream fork that ships a compose-file generator can override ComposeFileArgs to rewrite the path + emit --project-directory so relative bind-mount sources resolve against the pristine test package dir rather than the overlay output dir. EnvForCompose lets a fork inject e.g. UID/GID so `${UID:-65532}` placeholders in generated compose files resolve to the host UID and bind-mounts stay writable. Resilience improvements (no hook needed; universally beneficial): - startCluster captures docker-compose stderr into a strings.Builder and surfaces it on failure. The previous code set cmd.Stderr = nil, which suppressed actionable error text (volume-init races, port conflicts, image pull failures) and made CI-only bring-up failures unreadable. Keep Stderr captured; only print on failure. - startCluster retries `docker compose up` once after a full `down -v` when the first attempt fails. Covers a docker volume-init race when multiple services share a named volume: on first use docker populates the volume and concurrent services can collide with "failed to mkdir .../_data/: file exists". One retry is enough for this class; anything persistent is a real configuration error. - stopCluster log-dump iterates every container in the project prefix (AllContainers) rather than guessing names. Previous code hard-coded "zero0..zero3" and "alpha0..alpha6" which often misses containers (e.g. minio, nfs-backup) and tries to fetch logs for names that do not exist. Output is now a fenced block per container for easier triage from CI log viewers. - `defer stopCluster(compose, prefix, wg, err)` was wrapped in a closure so the err value passed to stopCluster is the value AT DEFER RUN TIME (after runTestsFor returns), not the defer-statement- time value which is always nil. This makes stopCluster actually dump container logs on test failure; the previous form silently suppressed them. Helper changes: - testTags(baseTag) returns a comma-joined tag list composed of the optional base tag + every tag in buildvars.GoRunTags. Replaces inline "-tags=integration" strings at 2 call sites. Stock upstream builds see identical behavior (GoRunTags is empty); the helper lets a fork thread fork-only tags into `go test` without forking t.go. - ComposeFileArgs call sites: startCluster (up + down retry), resumeDefault (stop + start). 4 total. Tested: go build ./t/ passes (OSS mode). --- t/hooks.go | 34 +++++++++++ t/t.go | 165 +++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 168 insertions(+), 31 deletions(-) create mode 100644 t/hooks.go diff --git a/t/hooks.go b/t/hooks.go new file mode 100644 index 00000000000..9a864c367a0 --- /dev/null +++ b/t/hooks.go @@ -0,0 +1,34 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package main + +// This file declares public extensibility hooks for the t test runner. +// See testutil/hooks.go for the full convention. + +// EnvForCompose returns extra KEY=VALUE entries to inject into the +// environment of docker-compose subprocesses that t spawns. Default: +// nil. Forks may inject e.g. UID/GID so `${UID:-65532}` in generated +// compose files resolves to the host UID rather than the image's +// nonroot user. +var EnvForCompose = defaultEnvForCompose + +// ComposeFileArgs returns the file-selector args docker-compose receives +// for the given compose-file path. The baseDir argument is the repo root +// (t's --base flag), passed explicitly so the hook is pure — no package +// state shared with t/t.go. Default: returns "-f " unchanged, +// matching upstream behavior where the pristine compose file is passed +// straight through. +// +// A fork that runs a compose-file generator may override this to rewrite +// the path to a generated overlay (e.g. under $DGRAPH_COMPOSE_BUILD_DIR) +// and add "--project-directory " so relative bind-mount +// sources resolve against the pristine test package dir rather than the +// overlay output dir. Unknown paths or an empty $DGRAPH_COMPOSE_BUILD_DIR +// fall through to the upstream-compatible "-f " form. +var ComposeFileArgs = defaultComposeFileArgs + +func defaultEnvForCompose() []string { return nil } +func defaultComposeFileArgs(path, baseDir string) []string { return []string{"-f", path} } diff --git a/t/t.go b/t/t.go index 168b6d364c7..3566fcda13b 100644 --- a/t/t.go +++ b/t/t.go @@ -35,6 +35,7 @@ import ( "github.com/spf13/pflag" "golang.org/x/tools/go/packages" + "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/testutil" "github.com/dgraph-io/dgraph/v25/x" "github.com/dgraph-io/ristretto/v2/z" @@ -159,6 +160,7 @@ func commandWithContext(ctx context.Context, args ...string) *exec.Cmd { cmd.Env = append(cmd.Env, "MINIO_IMAGE_ARCH=RELEASE.2020-11-13T20-10-18Z-arm64") cmd.Env = append(cmd.Env, "NFS_SERVER_IMAGE_ARCH=11-arm") } + cmd.Env = append(cmd.Env, EnvForCompose()...) return cmd } @@ -202,7 +204,7 @@ func ensureGoPathLinuxBinEnvVarSet() { func ensureDgraphLinuxBinary() error { ensureGoPathLinuxBinEnvVarSet() gopathLinuxBin := os.Getenv("LINUX_GOBIN") - dgraphBin := filepath.Join(gopathLinuxBin, "dgraph") + dgraphBin := filepath.Join(gopathLinuxBin, buildvars.Bin.Get()) if _, err := os.Stat(dgraphBin); err == nil { return nil // binary exists @@ -233,16 +235,52 @@ func startCluster(composeFile, prefix string) error { if err := ensureDgraphLinuxBinary(); err != nil { return err } - cmd := command( - "docker", "compose", "--compatibility", "-f", composeFile, "-p", prefix, - "up", "--force-recreate", "--build", "--remove-orphans", "--detach") - cmd.Stderr = nil - + // + --project-directory so relative bind-mount sources resolve against the + // pristine test package dir. + composeArgs := ComposeFileArgs(composeFile, *baseDir) + upArgs := append([]string{"docker", "compose", "--compatibility"}, + append(composeArgs, "-p", prefix, "up", "--force-recreate", "--build", "--remove-orphans", "--detach")...) + + // docker compose `up` on a shared named volume can race on initial + // volume population when multiple containers using the same volume + // start concurrently ("failed to mkdir .../_data/: file exists"). + // Retry once after a full `down -v` — the second attempt finds a fresh + // volume and succeeds. One retry is enough in practice for this class + // of race; anything persistent is a real configuration error. + const upAttempts = 2 + var lastErr error + var cmdStderr strings.Builder fmt.Printf("Bringing up cluster %s for package: %s ...\n", prefix, composeFile) - if err := cmd.Run(); err != nil { + for attempt := 1; attempt <= upAttempts; attempt++ { + cmdStderr.Reset() + cmd := command(upArgs...) + cmd.Stderr = &cmdStderr + if err := cmd.Run(); err == nil { + lastErr = nil + break + } else { + lastErr = err + stderr := cmdStderr.String() + fmt.Printf("Bring-up attempt %d/%d failed: %v\n", attempt, upAttempts, err) + if stderr != "" { + fmt.Printf("docker compose stderr:\n%s\n", stderr) + } + if attempt < upAttempts { + // Tear down any partial state so the retry starts from + // a clean volume/network baseline. + downArgs := append([]string{"docker", "compose", "--compatibility"}, + append(composeArgs, "-p", prefix, "down", "-v")...) + downCmd := command(downArgs...) + downCmd.Stderr = nil + _ = downCmd.Run() + fmt.Printf("Retrying cluster bring-up (attempt %d/%d) after `down -v`...\n", attempt+1, upAttempts) + } + } + } + if lastErr != nil { fmt.Printf("While running command: %q Error: %v\n", - strings.Join(cmd.Args, " "), err) - return err + strings.Join(upArgs, " "), lastErr) + return lastErr } fmt.Printf("CLUSTER UP: %s. Package: %s\n", prefix, composeFile) @@ -295,26 +333,39 @@ func outputLogs(prefix string) { fmt.Printf("error closing file: %v", err) } }() - printLogs := func(container string) { - in := testutil.GetContainerInstance(prefix, container) - c := in.GetContainer() + printLogs := func(c *container.Summary) { if c == nil { return } logCmd := exec.Command("docker", "logs", c.ID) out, err := logCmd.CombinedOutput() - x.Check(err) - if _, err := f.Write(out); err != nil { - fmt.Printf("error writing container logs to file: %v", err) + if err != nil { + fmt.Printf("error fetching docker logs for %s: %v\n", c.ID, err) + } + if _, werr := f.Write(out); werr != nil { + fmt.Printf("error writing container logs to file: %v\n", werr) + } + // Stream to stdout so CI captures the logs, with a clear header + // per-container that `gh run view --log` can grep for. The + // previous implementation wrote a single long line; switch to a + // fenced block so a failing test is easy to triage from the log. + name := "" + if len(c.Names) > 0 { + name = c.Names[0] } - fmt.Printf("Docker logs for %s is %s with error %+v ", c.ID, string(out), err) + fmt.Printf("\n===== DOCKER LOGS %s (%s) =====\n%s===== END LOGS %s =====\n", + name, c.ID, string(out), name) } - for i := 0; i <= 3; i++ { - printLogs("zero" + strconv.Itoa(i)) + // Iterate every container in the prefix group rather than guessing + // names: backup/encryption has alpha1-3, minio, zero1; cloud has + // alpha1, minio, zero1-3; upstream clusters have alpha0 etc. + // AllContainers returns every container with the project prefix. + containers := testutil.AllContainers(prefix) + if len(containers) == 0 { + fmt.Printf("---> NO CONTAINERS FOUND for prefix %s; nothing to log.\n", prefix) } - - for i := 0; i <= 6; i++ { - printLogs("alpha" + strconv.Itoa(i)) + for i := range containers { + printLogs(&containers[i]) } s := fmt.Sprintf("---> LOGS for %s written to %s .\n", prefix, f.Name()) _, err = oc.Write([]byte(s)) @@ -322,11 +373,14 @@ func outputLogs(prefix string) { } func stopCluster(composeFile, prefix string, wg *sync.WaitGroup, err error) { + composeArgs := ComposeFileArgs(composeFile, *baseDir) go func() { if err != nil { outputLogs(prefix) } - cmd := command("docker", "compose", "--compatibility", "-f", composeFile, "-p", prefix, "stop") + stopArgs := append([]string{"docker", "compose", "--compatibility"}, + append(composeArgs, "-p", prefix, "stop")...) + cmd := command(stopArgs...) cmd.Stderr = nil if err := cmd.Run(); err != nil { fmt.Printf("Error while bringing down cluster. Prefix: %s. Error: %v\n", @@ -370,7 +424,9 @@ func stopCluster(composeFile, prefix string, wg *sync.WaitGroup, err error) { } } - cmd = command("docker", "compose", "--compatibility", "-f", composeFile, "-p", prefix, "down", "-v") + downArgs := append([]string{"docker", "compose", "--compatibility"}, + append(composeArgs, "-p", prefix, "down", "-v")...) + cmd = command(downArgs...) if err := cmd.Run(); err != nil { fmt.Printf("Error while bringing down cluster. Prefix: %s. Error: %v\n", prefix, err) @@ -437,11 +493,34 @@ func gotestsumBin() string { return filepath.Join(gopath, "bin", "gotestsum") } +// testTags returns the comma-joined build-tag list for test +// compilation. Starts from `integration` (unless unit-only), then appends +// every extra tag in buildvars.GoRunTags so fork-specific tag-guarded +// files compile into the test binary. In upstream builds GoRunTags is +// empty, so the result is just "integration". +func testTags(baseTag string) string { + tags := []string{} + if baseTag != "" { + tags = append(tags, baseTag) + } + if extra := strings.TrimSpace(buildvars.GoRunTags.Get()); extra != "" { + // Support space- or comma-separated extras. + for _, t := range strings.FieldsFunc(extra, func(r rune) bool { return r == ' ' || r == ',' }) { + if t != "" { + tags = append(tags, t) + } + } + } + return strings.Join(tags, ",") +} + func runTestsFor(ctx context.Context, pkg, prefix string, xmlFile string) error { args := []string{gotestsumBin(), "--junitfile", xmlFile, "--format", "standard-verbose", "--max-fails", "1", "--", "-v", "-failfast"} if !isUnitOnly() { - args = append(args, "-tags=integration") + args = append(args, "-tags="+testTags("integration")) + } else if tags := testTags(""); tags != "" { + args = append(args, "-tags="+tags) } switch { case *testTimeout != "": @@ -548,6 +627,12 @@ func runTests(taskCh chan task, closer *z.Closer) error { closer.Done() }() + // Default cluster compose: pristine source path. startCluster/pauseDefault/ + // resumeDefault route through ComposeFileArgs which handles the + // overlay rewrite + --project-directory when the file is in the generator + // manifest. dgraph/docker-compose.yml is inline-markered (NOT in the + // manifest) so the rewrite is a no-op today — but keeping the consistent + // code path future-proofs if the file moves to the generator-fed set. defaultCompose := filepath.Join(*baseDir, "dgraph/docker-compose.yml") prefix := getClusterPrefix() @@ -611,8 +696,9 @@ func runTests(taskCh chan task, closer *z.Closer) error { if !started || stopped { return } - cmd := command("docker", "compose", "--compatibility", - "-f", defaultCompose, "-p", prefix, "stop") + pauseArgs := append([]string{"docker", "compose", "--compatibility"}, + append(ComposeFileArgs(defaultCompose, *baseDir), "-p", prefix, "stop")...) + cmd := command(pauseArgs...) cmd.Stderr = nil if err := cmd.Run(); err != nil { fmt.Printf("Warning: failed to pause default cluster %s: %v\n", prefix, err) @@ -631,8 +717,9 @@ func runTests(taskCh chan task, closer *z.Closer) error { if !defaultPaused { return nil // already running } - cmd := command("docker", "compose", "--compatibility", - "-f", defaultCompose, "-p", prefix, "start") + resumeArgs := append([]string{"docker", "compose", "--compatibility"}, + append(ComposeFileArgs(defaultCompose, *baseDir), "-p", prefix, "start")...) + cmd := command(resumeArgs...) cmd.Stderr = nil if err := cmd.Run(); err != nil { fmt.Printf("Warning: failed to resume default cluster %s: %v\n", prefix, err) @@ -740,7 +827,14 @@ func runCustomClusterTest(ctx context.Context, pkg string, wg *sync.WaitGroup, x } if !*keepCluster { wg.Add(1) - defer stopCluster(compose, prefix, wg, err) + // Wrap in a closure so the `err` read by stopCluster is the + // value AT DEFER RUN TIME (i.e. after runTestsFor returns). + // The original form `defer stopCluster(..., err)` captured err + // at defer-STATEMENT time, which is always nil here, so + // stopCluster never dumped container logs on test failure. + defer func() { + stopCluster(compose, prefix, wg, err) + }() } err = runTestsFor(ctx, pkg, prefix, xmlFile) @@ -837,6 +931,9 @@ type task struct { // for custom cluster tests (i.e. those not using default docker-compose.yml) func composeFileFor(pkg string) string { dir := strings.Replace(pkg, "github.com/dgraph-io/dgraph/v25/", "", 1) + // Return the pristine source path; ComposeFileArgs (called from + // startCluster/stopCluster) handles the overlay rewrite + project- + // directory anchoring when the file is in the generator manifest. return filepath.Join(*baseDir, dir, "docker-compose.yml") } @@ -877,9 +974,15 @@ func getPackages() []task { } // When running unit-only, don't add --tags=integration so that only true // unit tests (without //go:build integration) are discovered and compiled. + // Always thread GO_RUN_TAGS through so fork-tagged files participate in + // package discovery (e.g. tag-guarded init hooks for test harness). var buildFlags []string - if !isUnitOnly() { - buildFlags = []string{"-tags=integration"} + if tags := testTags("integration"); isUnitOnly() { + if extraOnly := testTags(""); extraOnly != "" { + buildFlags = []string{"-tags=" + extraOnly} + } + } else { + buildFlags = []string{"-tags=" + tags} } cfg := &packages.Config{BuildFlags: buildFlags} From 3d4677eea666619533c8fafefe6e593958bce63f Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:27:06 -0400 Subject: [PATCH 10/16] =?UTF-8?q?refactor(compose,dgraphtest):=20add=20ext?= =?UTF-8?q?ension=20hooks;=20rename=20local=20copy=20=E2=86=92=20copyFile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds public function-var hooks to two more test-infra packages so downstream forks can customize behavior without modifying call sites. compose/hooks.go (new, 1 hook): var EmitUser = defaultEmitUser Default returns empty string (no user: field emitted in generated compose services, matching upstream behavior). A fork may return e.g. "${UID:-65532}" to pin containers to the host UID for bind-mount compatibility under nonroot runtime images. Wired in compose.go Service generation. dgraphtest/hooks.go (new, 6 hooks): var ApplyContainerUser // no-op default; fork pins cfg.User to host UID var WidenTempDirPerms // no-op default; fork widens 0700 -> 0755 var WidenSecretFilePerms // no-op default; fork widens 0600 -> 0644 var GeneratePlugins // returns (_, handled=false, nil); fork builds plugins in its toolchain var SetupLinuxBinaries // returns (handled=false, nil); fork runs its two-binary staging var HostBinaryName // returns ""; fork returns its custom binary name Call sites added in dgraphtest/local_cluster.go, dgraphtest/image.go, dgraphtest/load.go — all preserve upstream behavior when the hooks return their zero values. Rough call-site counts: - local_cluster.go: 5 hook calls (2 WidenTempDirPerms, ApplyContainerUser, 2 WidenSecretFilePerms, 1 GeneratePlugins) - image.go: 1 hook call (SetupLinuxBinaries) - load.go: 1 hook call (HostBinaryName) Collateral rename in dgraphtest/image.go: func copy(src, dst string) error -> func copyFile The previous name shadowed the Go built-in `copy()`. The local function takes (src, dst string) and has no relation to the built-in, but the shadow was a subtle code-review hazard for anyone skimming the file. Rename all 3 call sites to copyFile; drop the shadow. Tested: go build ./compose/ , go build ./dgraphtest/ both pass. --- compose/compose.go | 10 +++-- compose/hooks.go | 18 +++++++++ dgraphtest/hooks.go | 77 +++++++++++++++++++++++++++++++++++++ dgraphtest/image.go | 12 ++++-- dgraphtest/load.go | 3 ++ dgraphtest/local_cluster.go | 19 +++++++++ 6 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 compose/hooks.go create mode 100644 dgraphtest/hooks.go diff --git a/compose/compose.go b/compose/compose.go index a7e9ee2987a..f7b27408f01 100644 --- a/compose/compose.go +++ b/compose/compose.go @@ -19,6 +19,7 @@ import ( "github.com/spf13/pflag" yaml "gopkg.in/yaml.v3" + "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/x" ) @@ -154,6 +155,9 @@ func initService(basename string, idx, grpcPort int) service { svc.Image = opts.Image + ":" + opts.Tag svc.ContainerName = containerName(svc.name) svc.WorkingDir = fmt.Sprintf("/data/%s", svc.name) + if u := EmitUser(); u != "" { + svc.User = u + } if idx > 1 { svc.DependsOn = append(svc.DependsOn, name(basename, idx-1)) } @@ -190,9 +194,9 @@ func initService(basename string, idx, grpcPort int) service { // no data volume } - svc.Command = "dgraph" + svc.Command = buildvars.Bin.Get() if opts.LocalBin { - svc.Command = "/gobin/dgraph" + svc.Command = buildvars.GoBinDgraphPath.Get() } if opts.UserOwnership { user, err := user.Current() @@ -590,7 +594,7 @@ func main() { "./docker-compose.yml", "name of output file") cmd.PersistentFlags().BoolVarP(&opts.LocalBin, "local", "l", true, "use locally-compiled binary if true, otherwise use binary from docker container") - cmd.PersistentFlags().StringVar(&opts.Image, "image", "dgraph/dgraph", + cmd.PersistentFlags().StringVar(&opts.Image, "image", buildvars.DockerImage.Get(), "Docker image for alphas and zeros.") cmd.PersistentFlags().StringVarP(&opts.Tag, "tag", "t", "latest", "Docker tag for the --image image. Requires -l=false to use binary from docker container.") diff --git a/compose/hooks.go b/compose/hooks.go new file mode 100644 index 00000000000..e23cf8d7929 --- /dev/null +++ b/compose/hooks.go @@ -0,0 +1,18 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package main + +// This file declares public extensibility hooks for the compose generator. +// See testutil/hooks.go for the full convention. + +// EmitUser returns the value for the generated `user:` field in docker +// compose services. Default: empty string (no user override emitted — +// matches upstream behavior where containers run as the image's default +// user). Forks may return e.g. "${UID:-65532}" to pin services to the +// host UID for bind-mount compatibility under nonroot runtime images. +var EmitUser = defaultEmitUser + +func defaultEmitUser() string { return "" } diff --git a/dgraphtest/hooks.go b/dgraphtest/hooks.go new file mode 100644 index 00000000000..90ab96e1961 --- /dev/null +++ b/dgraphtest/hooks.go @@ -0,0 +1,77 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package dgraphtest + +import "github.com/docker/docker/api/types/container" + +// This file declares public extensibility hooks for the dgraphtest test +// harness. See testutil/hooks.go for the full convention. Each hook is +// initialized to an unexported default that implements upstream behavior; +// a private fork reassigns the public var from its own init(). + +// ApplyContainerUser optionally sets cfg.User to pin a container to a +// specific UID/GID. Default: no-op (container runs as the image's +// default user). Forks that ship nonroot runtime images and want tests +// to run under the host UID (so bind-mounted paths are writable) set +// cfg.User to "host-uid:host-gid". +var ApplyContainerUser = defaultApplyContainerUser + +// WidenTempDirPerms optionally relaxes the 0700 default permissions of +// os.MkdirTemp to allow a container's nonroot user to traverse the +// bind-mounted path. Default: no-op. Forks with nonroot runtime images +// set the mode to 0755. +var WidenTempDirPerms = defaultWidenTempDirPerms + +// WidenSecretFilePerms optionally relaxes secret-file permissions from +// the default 0600 to let a container's nonroot user read the file via a +// bind-mount. Default: no-op. Forks with nonroot runtime images set 0644. +// Tempdir isolation bounds the exposure to ephemeral test processes. +var WidenSecretFilePerms = defaultWidenSecretFilePerms + +// GeneratePlugins optionally overrides the plugin-build flow used by +// LocalCluster.GeneratePlugins. If handled=true, the caller uses the +// returned tokenizers string to populate the cluster's custom_tokenizers +// arg and skips the default native-go-build path. If handled=false, the +// caller falls through to the default in-process build. Default: +// returns ("", false, nil) — delegate to upstream. +// +// Params: +// - raceEnabled: forward the --race flag to the plugin build +// - tempBinDir: the cluster's per-test tempdir (host-side); a plugins/ +// subdirectory will be created there and bind-mounted into the +// container-side build environment +// - repoRoot: host path to the dgraph repo root, mounted so the +// builder sees testutil/custom_plugins/... source files +// +// The returned tokenizers string is the comma-separated list of .so +// paths as seen by the alpha container at runtime (typically under +// /gobin/plugins/…). +var GeneratePlugins = defaultGeneratePlugins + +// SetupLinuxBinaries optionally overrides the dgraph-binary staging +// performed by LocalCluster.setupBinary for the Linux branch. If +// handled=true, the caller returns err without executing the upstream +// single-binary Linux path. Default: (false, nil) — delegate to +// upstream. +var SetupLinuxBinaries = defaultSetupLinuxBinaries + +// HostBinaryName optionally overrides the filename of the host-native +// dgraph binary that LocalCluster.HostDgraphBinaryPath joins with +// tempBinDir. Default: empty string (use upstream default path selection +// based on runtime.GOOS). Forks may return e.g. "dgraph_host" to name a +// separate host-native binary alongside a container-only binary. +var HostBinaryName = defaultHostBinaryName + +func defaultApplyContainerUser(cfg *container.Config) {} +func defaultWidenTempDirPerms(path string) error { return nil } +func defaultWidenSecretFilePerms(path string) error { return nil } +func defaultHostBinaryName() string { return "" } +func defaultSetupLinuxBinaries(tempBinDir, version string) (handled bool, err error) { + return false, nil +} +func defaultGeneratePlugins(raceEnabled bool, tempBinDir, repoRoot string) (tokenizers string, handled bool, err error) { + return "", false, nil +} diff --git a/dgraphtest/image.go b/dgraphtest/image.go index 6476917b200..24de0536ce3 100644 --- a/dgraphtest/image.go +++ b/dgraphtest/image.go @@ -60,6 +60,10 @@ func (c *LocalCluster) setupBinary() error { return errors.New("GOPATH is not set") } + if handled, err := SetupLinuxBinaries(c.tempBinDir, c.conf.version); handled { + return err + } + if runtime.GOOS == "linux" { // On Linux $GOPATH/bin/dgraph is both the native and Docker binary. return copyBinary(filepath.Join(gopath, "bin"), c.tempBinDir, c.conf.version) @@ -79,7 +83,7 @@ func (c *LocalCluster) setupBinary() error { hostSrc := filepath.Join(gopath, "bin", "dgraph") hostDst := filepath.Join(c.tempBinDir, "dgraph_host") - if err := copy(hostSrc, hostDst); err != nil { + if err := copyFile(hostSrc, hostDst); err != nil { return errors.Wrapf(err, "error copying host-native binary from [%v] to [%v]", hostSrc, hostDst) } return nil @@ -216,7 +220,7 @@ func buildDgraphBinary(dir, binaryDir, version string) error { if out, err := cmd.CombinedOutput(); err != nil { return errors.Wrapf(err, "error while building dgraph binary\noutput:%v", string(out)) } - if err := copy(filepath.Join(dir, "dgraph", "dgraph"), + if err := copyFile(filepath.Join(dir, "dgraph", "dgraph"), filepath.Join(binaryDir, fmt.Sprintf(binaryNameFmt, version))); err != nil { return errors.Wrap(err, "error while copying binary") } @@ -230,13 +234,13 @@ func copyBinary(fromDir, toDir, version string) error { } fromPath := filepath.Join(fromDir, binaryName) toPath := filepath.Join(toDir, buildvars.Bin.Get()) - if err := copy(fromPath, toPath); err != nil { + if err := copyFile(fromPath, toPath); err != nil { return errors.Wrapf(err, "error while copying binary into tempBinDir [%v], from [%v]", toPath, fromPath) } return nil } -func copy(src, dst string) error { +func copyFile(src, dst string) error { // Validate inputs if src == "" || dst == "" { return errors.New("source or destination paths cannot be empty") diff --git a/dgraphtest/load.go b/dgraphtest/load.go index 116c04a6b5c..9f40d3262c5 100644 --- a/dgraphtest/load.go +++ b/dgraphtest/load.go @@ -34,6 +34,9 @@ import ( // Docker containers). On non-Linux (macOS) it is "dgraph_host", a separate // native binary copied by setupBinary(). func (c *LocalCluster) HostDgraphBinaryPath() string { + if name := HostBinaryName(); name != "" { + return filepath.Join(c.tempBinDir, name) + } if runtime.GOOS == "linux" { return filepath.Join(c.tempBinDir, "dgraph") } diff --git a/dgraphtest/local_cluster.go b/dgraphtest/local_cluster.go index 33d7f5d5d79..d5c7a74e95e 100644 --- a/dgraphtest/local_cluster.go +++ b/dgraphtest/local_cluster.go @@ -117,11 +117,17 @@ func (c *LocalCluster) init() error { if err != nil { return errors.Wrap(err, "error while creating tempBinDir") } + if err := WidenTempDirPerms(c.tempBinDir); err != nil { + return err + } log.Printf("[INFO] tempBinDir: %v", c.tempBinDir) c.tempSecretsDir, err = os.MkdirTemp("", c.conf.prefix) if err != nil { return errors.Wrap(err, "error while creating tempSecretsDir") } + if err := WidenTempDirPerms(c.tempSecretsDir); err != nil { + return err + } log.Printf("[INFO] tempSecretsDir: %v", c.tempSecretsDir) if err := os.Mkdir(binariesPath, os.ModePerm); err != nil && !os.IsExist(err) { @@ -305,6 +311,7 @@ func (c *LocalCluster) createContainer(dc dnode) (string, error) { } cconf := &container.Config{Cmd: cmd, Image: image, WorkingDir: dc.workingDir(), ExposedPorts: dc.ports()} + ApplyContainerUser(cconf) hconf := &container.HostConfig{Mounts: mts, PublishAllPorts: true, PortBindings: dc.bindings(c.conf.portOffset)} networkConfig := &network.NetworkingConfig{ EndpointsConfig: map[string]*network.EndpointSettings{ @@ -1242,6 +1249,9 @@ func (c *LocalCluster) setupSecrets() error { if err := os.WriteFile(c.encKeyPath, encKey, 0600); err != nil { return err } + if err := WidenSecretFilePerms(c.encKeyPath); err != nil { + return err + } } if c.conf.acl { @@ -1249,6 +1259,9 @@ func (c *LocalCluster) setupSecrets() error { if err := generateACLSecret(c.conf.aclAlg, aclSecretPath); err != nil { return err } + if err := WidenSecretFilePerms(aclSecretPath); err != nil { + return err + } } return nil @@ -1305,6 +1318,12 @@ func runOpennssl(args ...string) error { } func (c *LocalCluster) GeneratePlugins(raceEnabled bool) error { + if tokenizers, handled, err := GeneratePlugins(raceEnabled, c.tempBinDir, baseRepoDir); handled { + if err == nil { + c.customTokenizers = tokenizers + } + return err + } _, curr, _, ok := runtime.Caller(0) if !ok { return errors.New("error while getting current file") From 8d185f9c4383120e43bb22502d1ac0117e76ad50 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:40:53 -0400 Subject: [PATCH 11/16] test: widen HMAC/JWT/MinIO test secrets >= 14 bytes; add FIPS-aware skip helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Widens every HMAC/JWT/MinIO test secret to meet the 14-byte (112-bit) key-length minimum that NIST SP 800-131A requires and that some FIPS- validated crypto providers enforce at EVP_MAC_init. Upstream builds see no behavioral change — a longer HMAC key is always acceptable. The wider keys let the test suite also run against a FIPS-enforcing alpha/ minio subprocess without hitting "invalid key length" crashes. Key renames (neutral, describe length without leaking FIPS rationale): "secret" -> "secret-long-enough" (schema VerificationKey, JWT signer) "secret1" -> "secret1-long-enough" (namespace-1 multi-tenancy) "secretkey" -> "secretkey-long-enough" (schema VerificationKey, MinIO secret key) Hardcoded HS256 JWT fixtures in graphql/resolve/auth_test.go (7 tokens across TestStringCustomClaim, TestAudienceClaim, TestJWTExpiry) and graphql/e2e/auth/auth_test.go (2 tokens in TestQueryWithStandardClaims) were re-signed with the new "secretkey-long-enough" key. Re-signing used a standalone Python HMAC-SHA256 signer that verifies each tokens existing signature against the old key before replacing the signature, so any token not signed with the expected key is left untouched. FIPS-aware test skips: check_upgrade/check_upgrade_test.go and systest/integration2/acl_test.go add a local skipIfFIPSBinary(t) helper that calls t.Skip when either x.FIPSEnabled is true (this test binary is FIPS-tagged) or x.FIPSBinary() is true (operator set DGRAPH_FIPS_BINARY=1). Upgrade- path tests pin a specific pre-FIPS upstream SHA and build the old binary at runtime; that commit predates any FIPS-enforcing toolchain, so running these tests under a FIPS configuration fails pointlessly. acl/jwt_algo_test.go (TestACLJwtAlgo) skips the EdDSA case when FIPS is in effect. Ed25519 is outside the validated FIPS boundary for Gos crypto library so EdDSA cluster startup panics under FIPS. All skip helpers use x.FIPSBinary() (the env-gated runtime signal introduced in the previous commit) rather than reading os.Getenv inline, so the env-var contract is centralized. Tested: go build ./..., go test -run "TestStringCustomClaim|TestAudienceClaim|TestJWTExpiry" ./graphql/resolve/ (passes; re-signed JWTs verify), go build -tags=integration2 ./graphql/... ./acl/ ./check_upgrade/ ./systest/integration2/ all pass. --- acl/jwt_algo_test.go | 8 +++++ check_upgrade/check_upgrade_test.go | 16 +++++++++ graphql/e2e/auth/auth_test.go | 4 +-- .../e2e/multi_tenancy/multi_tenancy_test.go | 12 +++---- graphql/e2e/subscription/subscription_test.go | 36 +++++++++---------- graphql/resolve/auth_test.go | 18 +++++----- systest/backup/encryption/backup_test.go | 5 ++- systest/backup/minio/backup_test.go | 5 ++- systest/bulk_live/common/bulk_live_fixture.go | 5 ++- systest/cloud/cloud_test.go | 6 ++-- systest/integration2/acl_test.go | 15 ++++++++ testutil/graphql.go | 9 ++++- testutil/minio.go | 6 +++- 13 files changed, 103 insertions(+), 42 deletions(-) diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index f9c45802ec7..989aca28d4a 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -22,10 +22,18 @@ import ( ) func TestACLJwtAlgo(t *testing.T) { + // The binary-under-test may be FIPS-enforcing even when the test binary + // itself is not (the default for integration2 tests). Skip EdDSA either + // when this test binary is FIPS-tagged (x.FIPSEnabled) or when the + // environment signals the dgraph binary is a FIPS build (x.FIPSBinary). + fipsBinary := x.FIPSEnabled || x.FIPSBinary() for _, algo := range jwt.GetAlgorithms() { if algo == "none" || algo == "" { continue } + if algo == "EdDSA" && fipsBinary { + continue + } t.Run(fmt.Sprintf("running cluster with algorithm: %v", algo), func(t *testing.T) { conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1). diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index 7b75fa230e1..70562b29496 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -24,7 +24,22 @@ import ( "github.com/dgraph-io/dgraph/v25/x" ) +// skipIfFIPSBinary skips the current test when either (a) the test binary +// itself is FIPS-tagged (x.FIPSEnabled), or (b) the dgraph binary under +// test is FIPS-restricted (x.FIPSBinary()). Upgrade-path tests pin a +// specific upstream SHA for the "old" binary; that commit predates any +// FIPS-enforcing toolchain, so attempting to build it under a FIPS +// configuration either fails outright or produces a binary that refuses +// to start. The test is semantically valid upstream and under a +// non-FIPS fork; we skip only when FIPS enforcement rules it out. +func skipIfFIPSBinary(t *testing.T) { + if x.FIPSEnabled || x.FIPSBinary() { + t.Skip("upgrade-path test pins a pre-FIPS upstream SHA; skipping under FIPS build") + } +} + func TestCheckUpgrade(t *testing.T) { + skipIfFIPSBinary(t) conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1). WithACL(time.Hour).WithVersion("57aa5c4ac") c, err := dgraphtest.NewLocalCluster(conf) @@ -96,6 +111,7 @@ func TestCheckUpgrade(t *testing.T) { } func TestQueryDuplicateNodes(t *testing.T) { + skipIfFIPSBinary(t) conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1). WithACL(time.Hour).WithVersion("57aa5c4ac").WithAclAlg(jwt.GetSigningMethod("HS256")) c, err := dgraphtest.NewLocalCluster(conf) diff --git a/graphql/e2e/auth/auth_test.go b/graphql/e2e/auth/auth_test.go index 814df8649aa..1a0d60aaa6b 100644 --- a/graphql/e2e/auth/auth_test.go +++ b/graphql/e2e/auth/auth_test.go @@ -623,7 +623,7 @@ func TestQueryWithStandardClaims(t *testing.T) { name } }`, - jwt: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiZXhwIjozNTE2MjM5MDIyLCJlbWFpbCI6InRlc3RAZGdyYXBoLmlvIiwiVVNFUiI6InVzZXIxIiwiUk9MRSI6IkFETUlOIn0.cH_EcC8Sd0pawJs96XPhpRsYVXuTybT1oUkluBDS8B4", + jwt: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiZXhwIjozNTE2MjM5MDIyLCJlbWFpbCI6InRlc3RAZGdyYXBoLmlvIiwiVVNFUiI6InVzZXIxIiwiUk9MRSI6IkFETUlOIn0.pZ2-Dib2lXrCeXghCoPD7CnZ8GUGXhv1WbbRQ7mhPnM", result: `{"queryProject":[{"name":"Project1"},{"name":"Project2"}]}`, }, { @@ -633,7 +633,7 @@ func TestQueryWithStandardClaims(t *testing.T) { name } }`, - jwt: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiZXhwIjozNTE2MjM5MDIyLCJlbWFpbCI6InRlc3RAZGdyYXBoLmlvIiwiVVNFUiI6InVzZXIxIn0.wabcAkINZ6ycbEuziTQTSpv8T875Ky7JQu68ynoyDQE", + jwt: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiZXhwIjozNTE2MjM5MDIyLCJlbWFpbCI6InRlc3RAZGdyYXBoLmlvIiwiVVNFUiI6InVzZXIxIn0.skAQMpvEE4WoqCZ6z6cKXKTIhyLvFV2Fyj_m6U8-g5M", result: `{"queryProject":[{"name":"Project1"}]}`, }, } diff --git a/graphql/e2e/multi_tenancy/multi_tenancy_test.go b/graphql/e2e/multi_tenancy/multi_tenancy_test.go index 27d39df8728..947fb0cf941 100644 --- a/graphql/e2e/multi_tenancy/multi_tenancy_test.go +++ b/graphql/e2e/multi_tenancy/multi_tenancy_test.go @@ -256,7 +256,7 @@ func TestAuth(t *testing.T) { username: String! @id isPublic: Boolean @search } - # Dgraph.Authorization {"VerificationKey":"secret","Header":"Authorization","Namespace":"https://dgraph.io/jwt/claims","Algo":"HS256"}` + # Dgraph.Authorization {"VerificationKey":"secret-long-enough","Header":"Authorization","Namespace":"https://dgraph.io/jwt/claims","Algo":"HS256"}` common.SafelyUpdateGQLSchema(t, common.Alpha1HTTP, schema, header) ns := common.CreateNamespace(t, header, "alpha1") @@ -277,7 +277,7 @@ func TestAuth(t *testing.T) { username: String! @id isPublic: Boolean @search } - # Dgraph.Authorization {"VerificationKey":"secret1","Header":"Authorization1","Namespace":"https://dgraph.io/jwt/claims1","Algo":"HS256"}` + # Dgraph.Authorization {"VerificationKey":"secret1-long-enough","Header":"Authorization1","Namespace":"https://dgraph.io/jwt/claims1","Algo":"HS256"}` common.SafelyUpdateGQLSchema(t, common.Alpha1HTTP, schema1, header1) require.Equal(t, schema, common.AssertGetGQLSchema(t, common.Alpha1HTTP, header).Schema) @@ -296,7 +296,7 @@ func TestAuth(t *testing.T) { // for namespace 0, after adding multiple users, we should only get back the user "Alice" header = common.GetJWT(t, "Alice", nil, &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io/jwt/claims", Algo: "HS256", Header: "Authorization", @@ -312,7 +312,7 @@ func TestAuth(t *testing.T) { // for namespace 1, after adding multiple users, we should only get back the public users header1 = common.GetJWT(t, "Alice", nil, &testutil.AuthMeta{ - PublicKey: "secret1", + PublicKey: "secret1-long-enough", Namespace: "https://dgraph.io/jwt/claims1", Algo: "HS256", Header: "Authorization1", @@ -345,7 +345,7 @@ func TestCORS(t *testing.T) { }) } # Dgraph.Allow-Origin "https://play.dgraph.io" - # Dgraph.Authorization {"VerificationKey":"secret","Header":"X-Test-Dgraph","Namespace":"https://dgraph.io/jwt/claims","Algo":"HS256"} + # Dgraph.Authorization {"VerificationKey":"secret-long-enough","Header":"X-Test-Dgraph","Namespace":"https://dgraph.io/jwt/claims","Algo":"HS256"} `, header) ns := common.CreateNamespace(t, header, "alpha1") @@ -363,7 +363,7 @@ func TestCORS(t *testing.T) { }) } # Dgraph.Allow-Origin "https://play1.dgraph.io" - # Dgraph.Authorization {"VerificationKey":"secret","Header":"X-Test-Dgraph1","Namespace":"https://dgraph.io/jwt/claims","Algo":"HS256"} + # Dgraph.Authorization {"VerificationKey":"secret-long-enough","Header":"X-Test-Dgraph1","Namespace":"https://dgraph.io/jwt/claims","Algo":"HS256"} `, header1) // testCORS for namespace 0 diff --git a/graphql/e2e/subscription/subscription_test.go b/graphql/e2e/subscription/subscription_test.go index ef7b576adf6..ffde2752f1a 100644 --- a/graphql/e2e/subscription/subscription_test.go +++ b/graphql/e2e/subscription/subscription_test.go @@ -61,7 +61,7 @@ const ( text: String! @search(by: [term]) owner: String! @search(by: [hash]) } -# Dgraph.Authorization {"VerificationKey":"secret","Header":"Authorization","Namespace":"https://dgraph.io","Algo":"HS256"} +# Dgraph.Authorization {"VerificationKey":"secret-long-enough","Header":"Authorization","Namespace":"https://dgraph.io","Algo":"HS256"} ` schCustomDQL = ` type Tweets { @@ -177,7 +177,7 @@ func TestSubscriptionAuth(t *testing.T) { common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "Authorization", @@ -206,7 +206,7 @@ func TestSubscriptionAuth(t *testing.T) { common.RequireNoGQLErrors(t, addResult) time.Sleep(pollInterval) - jwtToken, err := metaInfo.GetSignedToken("secret", subExp) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", subExp) require.NoError(t, err) payload := fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) @@ -294,7 +294,7 @@ func TestSubscriptionWithAuthShouldExpireWithJWT(t *testing.T) { common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "Authorization", @@ -323,7 +323,7 @@ func TestSubscriptionWithAuthShouldExpireWithJWT(t *testing.T) { common.RequireNoGQLErrors(t, addResult) time.Sleep(pollInterval) - jwtToken, err := metaInfo.GetSignedToken("secret", subExp) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", subExp) require.NoError(t, err) payload := fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) @@ -385,7 +385,7 @@ func TestSubscriptionAuthWithoutExpiry(t *testing.T) { common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "Authorization", @@ -413,7 +413,7 @@ func TestSubscriptionAuthWithoutExpiry(t *testing.T) { addResult := add.ExecuteAsPost(t, common.GraphqlURL) common.RequireNoGQLErrors(t, addResult) - jwtToken, err := metaInfo.GetSignedToken("secret", -1) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", -1) require.NoError(t, err) payload := fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) @@ -444,7 +444,7 @@ func TestSubscriptionAuth_SameQueryAndClaimsButDifferentExpiry_ShouldExpireIndep common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "Authorization", @@ -473,7 +473,7 @@ func TestSubscriptionAuth_SameQueryAndClaimsButDifferentExpiry_ShouldExpireIndep common.RequireNoGQLErrors(t, addResult) time.Sleep(pollInterval) - jwtToken, err := metaInfo.GetSignedToken("secret", subExp) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", subExp) require.NoError(t, err) // first subscription @@ -498,7 +498,7 @@ func TestSubscriptionAuth_SameQueryAndClaimsButDifferentExpiry_ShouldExpireIndep string(resp.Data)) // 2nd subscription - jwtToken, err = metaInfo.GetSignedToken("secret", 2*subExp) + jwtToken, err = metaInfo.GetSignedToken("secret-long-enough", 2*subExp) require.NoError(t, err) payload = fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) subscriptionClient1, err := common.NewGraphQLSubscription(subscriptionEndpoint, &schema.Request{ @@ -592,7 +592,7 @@ func TestSubscriptionAuth_SameQueryDifferentClaimsAndExpiry_ShouldExpireIndepend common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "Authorization", @@ -621,7 +621,7 @@ func TestSubscriptionAuth_SameQueryDifferentClaimsAndExpiry_ShouldExpireIndepend common.RequireNoGQLErrors(t, addResult) time.Sleep(pollInterval) - jwtToken, err := metaInfo.GetSignedToken("secret", subExp) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", subExp) require.NoError(t, err) // first subscription @@ -668,7 +668,7 @@ func TestSubscriptionAuth_SameQueryDifferentClaimsAndExpiry_ShouldExpireIndepend // 2nd subscription metaInfo.AuthVars["USER"] = "pawan" - jwtToken, err = metaInfo.GetSignedToken("secret", 2*subExp) + jwtToken, err = metaInfo.GetSignedToken("secret-long-enough", 2*subExp) require.NoError(t, err) payload = fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) subscriptionClient1, err := common.NewGraphQLSubscription(subscriptionEndpoint, &schema.Request{ @@ -784,7 +784,7 @@ func TestSubscriptionAuthHeaderCaseInsensitive(t *testing.T) { common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "authorization", @@ -812,7 +812,7 @@ func TestSubscriptionAuthHeaderCaseInsensitive(t *testing.T) { addResult := add.ExecuteAsPost(t, common.GraphqlURL) common.RequireNoGQLErrors(t, addResult) - jwtToken, err := metaInfo.GetSignedToken("secret", -1) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", -1) require.NoError(t, err) payload := fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) @@ -847,7 +847,7 @@ func TestSubscriptionAuth_MultiSubscriptionResponses(t *testing.T) { common.SafelyUpdateGQLSchemaOnAlpha1(t, schAuth) metaInfo := &testutil.AuthMeta{ - PublicKey: "secret", + PublicKey: "secret-long-enough", Namespace: "https://dgraph.io", Algo: "HS256", Header: "Authorization", @@ -857,7 +857,7 @@ func TestSubscriptionAuth_MultiSubscriptionResponses(t *testing.T) { "ROLE": "USER", } - jwtToken, err := metaInfo.GetSignedToken("secret", -1) + jwtToken, err := metaInfo.GetSignedToken("secret-long-enough", -1) require.NoError(t, err) payload := fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) @@ -885,7 +885,7 @@ func TestSubscriptionAuth_MultiSubscriptionResponses(t *testing.T) { subscriptionClient.Terminate() time.Sleep(pollInterval) - jwtToken, err = metaInfo.GetSignedToken("secret", 3*time.Second) + jwtToken, err = metaInfo.GetSignedToken("secret-long-enough", 3*time.Second) require.NoError(t, err) payload = fmt.Sprintf(`{"Authorization": "%s"}`, jwtToken) diff --git a/graphql/resolve/auth_test.go b/graphql/resolve/auth_test.go index 049c51a265e..c6c3ebb9c7d 100644 --- a/graphql/resolve/auth_test.go +++ b/graphql/resolve/auth_test.go @@ -176,7 +176,7 @@ func TestStringCustomClaim(t *testing.T) { // // It also contains standard claim : "email": "test@dgraph.io", but the // value of "email" gets overwritten by the value present inside custom claim. - token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjM1MTYyMzkwMjIsImVtYWlsIjoidGVzdEBkZ3JhcGguaW8iLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjp7IlVTRVJOQU1FIjoiUmFuZG9tIFVzZXIiLCJlbWFpbCI6InJhbmRvbUBkZ3JhcGguaW8ifX0.6XvP9wlvHx8ZBBMH9iyy49cRiIk7H6NNoZf69USkg2c" + token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjM1MTYyMzkwMjIsImVtYWlsIjoidGVzdEBkZ3JhcGguaW8iLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjp7IlVTRVJOQU1FIjoiUmFuZG9tIFVzZXIiLCJlbWFpbCI6InJhbmRvbUBkZ3JhcGguaW8ifX0.MgF1CfJI4RHDTQjLz6PtowlTB0Tlq5NmID5T4YmQ5mI" md := metadata.New(map[string]string{"authorizationJwt": token}) ctx := metadata.NewIncomingContext(context.Background(), md) @@ -209,7 +209,7 @@ func TestAudienceClaim(t *testing.T) { require.Equal(t, metainfo.Algo, jwt.SigningMethodHS256.Name) require.Equal(t, metainfo.Header, "X-Test-Auth") require.Equal(t, metainfo.Namespace, "https://xyz.io/jwt/claims") - require.Equal(t, metainfo.VerificationKey, "secretkey") + require.Equal(t, metainfo.VerificationKey, "secretkey-long-enough") require.Equal(t, metainfo.Audience, []string{"aud1", "63do0q16n6ebjgkumu05kkeian", "aud5"}) testCases := []struct { @@ -219,20 +219,20 @@ func TestAudienceClaim(t *testing.T) { }{ { name: `Token with valid audience: { "aud": "63do0q16n6ebjgkumu05kkeian" }`, - token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6IjYzZG8wcTE2bjZlYmpna3VtdTA1a2tlaWFuIiwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjQ1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.g6rAkPdNIJ6wvXOo6F4XmoVqqbGs_CdUHx_k7NrvLY8", + token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6IjYzZG8wcTE2bjZlYmpna3VtdTA1a2tlaWFuIiwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjQ1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.7d6-ZCkr89u1hPZW6Kf3PPQ7l45NTXi3o8Yl9auIY94", }, { name: `Token with invalid audience: { "aud": "invalidAudience" }`, - token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6ImludmFsaWRBdWRpZW5jZSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo0NTkwMzc2MDMyLCJpYXQiOjE1OTAzNzI0MzIsImVtYWlsIjoiZGF2aWRAdHlwZWpvaW4uY29tIn0.-8UxKvv6_0_hCbV3f6KEoP223BrCrP0eWWdoG-Gf3FQ", + token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6ImludmFsaWRBdWRpZW5jZSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo0NTkwMzc2MDMyLCJpYXQiOjE1OTAzNzI0MzIsImVtYWlsIjoiZGF2aWRAdHlwZWpvaW4uY29tIn0.TGvEtseruODzZpyTN2UYfwkwBcBBcnGn-XRDqydLUnY", err: fmt.Errorf("JWT `aud` value doesn't match with the audience"), }, { name: "Token without audience field", - token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo0NTkwMzc2MDMyLCJpYXQiOjE1OTAzNzI0MzIsImVtYWlsIjoiZGF2aWRAdHlwZWpvaW4uY29tIn0.Fjxh-sZM9eDRBRHKyLJ8MxAsSSZ-IX2f0z-Saq37t7U", + token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo0NTkwMzc2MDMyLCJpYXQiOjE1OTAzNzI0MzIsImVtYWlsIjoiZGF2aWRAdHlwZWpvaW4uY29tIn0.SxN-DSfz1IdSgTknOqWFH6S9IdxAop09VDnzObzhpeQ", }, { name: `Token with multiple audience: {"aud": ["aud1", "aud2", "aud3"]}`, - token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6WyJhdWQxIiwiYXVkMiIsImF1ZDMiXSwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjQ1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.LK31qlAVQHzu5mvEsPPRoNb59u8X9ITL_1re6wYGEtA", + token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6WyJhdWQxIiwiYXVkMiIsImF1ZDMiXSwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjQ1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.bkESyCXSS2uz_l9EmSdoUbW0dDDxdVFKOPnKxdE1Ni0", }, } @@ -370,7 +370,7 @@ func TestJWTExpiry(t *testing.T) { require.Equal(t, metainfo.Algo, jwt.SigningMethodHS256.Name) require.Equal(t, metainfo.Header, "X-Test-Auth") require.Equal(t, metainfo.Namespace, "https://xyz.io/jwt/claims") - require.Equal(t, metainfo.VerificationKey, "secretkey") + require.Equal(t, metainfo.VerificationKey, "secretkey-long-enough") testCases := []struct { name string @@ -379,11 +379,11 @@ func TestJWTExpiry(t *testing.T) { }{ { name: `Token without expiry value`, - token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiaWF0IjoxNTkwMzcyNDMyLCJlbWFpbCI6ImRhdmlkQHR5cGVqb2luLmNvbSJ9.f79YmZgz_YDBzf0dQ_dY_VQOjpGt4Z_MJ3LsvXrIQeQ", + token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiaWF0IjoxNTkwMzcyNDMyLCJlbWFpbCI6ImRhdmlkQHR5cGVqb2luLmNvbSJ9.316QjGnYj-1Cgd6m9dFfJnBIHvIuENnCNFtMDJW6MmM", }, { name: `Expired token`, - token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.cxTip2mZLf6hYBHYAyJ7pqohhpMdrVOaySFAtp3PfKg", + token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.VYh1np_muOPwAp5Z8dBIvRzLEiPL-8k21U-qpEYbYA8", invalid: true, }, } diff --git a/systest/backup/encryption/backup_test.go b/systest/backup/encryption/backup_test.go index 9d064707b13..0b27dbe9ca7 100644 --- a/systest/backup/encryption/backup_test.go +++ b/systest/backup/encryption/backup_test.go @@ -105,8 +105,11 @@ func TestBackupMinioE(t *testing.T) { require.True(t, moveOk) // Setup environmental variables for use during restore. + // Secret key padded to 14+ bytes so "AWS4"+secret satisfies the + // OpenSSL FIPS provider's HMAC key-length minimum (matches + // dgraph/minio.env, systest/backup.env). t.Setenv("MINIO_ACCESS_KEY", "accesskey") - t.Setenv("MINIO_SECRET_KEY", "secretkey") + t.Setenv("MINIO_SECRET_KEY", "secretkey-long-enough") // Setup test directories. dirSetup(t) diff --git a/systest/backup/minio/backup_test.go b/systest/backup/minio/backup_test.go index 61bcaae36eb..a757150fdec 100644 --- a/systest/backup/minio/backup_test.go +++ b/systest/backup/minio/backup_test.go @@ -101,8 +101,11 @@ func TestBackupMinio(t *testing.T) { require.True(t, moveOk) // Setup environmental variables for use during restore. + // Secret key padded to 14+ bytes so "AWS4"+secret satisfies the + // OpenSSL FIPS provider's HMAC key-length minimum (matches + // dgraph/minio.env, systest/backup.env). os.Setenv("MINIO_ACCESS_KEY", "accesskey") - os.Setenv("MINIO_SECRET_KEY", "secretkey") + os.Setenv("MINIO_SECRET_KEY", "secretkey-long-enough") // Setup test directories. dirSetup(t) diff --git a/systest/bulk_live/common/bulk_live_fixture.go b/systest/bulk_live/common/bulk_live_fixture.go index 9d6b46d9b33..d44de78de96 100644 --- a/systest/bulk_live/common/bulk_live_fixture.go +++ b/systest/bulk_live/common/bulk_live_fixture.go @@ -122,7 +122,10 @@ func newSuiteFromFile(t *testing.T, schemaFile, rdfFile, gqlSchemaFile string) * func (s *bsuite) setup(t *testing.T, schemaFile, rdfFile, gqlSchemaFile string) { var env []string if s.opts.remote { - env = append(env, "MINIO_ACCESS_KEY=accesskey", "MINIO_SECRET_KEY=secretkey") + // Secret key padded to 14+ bytes so "AWS4"+secret satisfies the + // OpenSSL FIPS provider's HMAC key-length minimum (matches + // dgraph/minio.env, systest/backup.env). + env = append(env, "MINIO_ACCESS_KEY=accesskey", "MINIO_SECRET_KEY=secretkey-long-enough") } require.NoError(s.t, makeDirEmpty(filepath.Join(rootDir, "out", "0"))) diff --git a/systest/cloud/cloud_test.go b/systest/cloud/cloud_test.go index e284aa6103a..ae7979f8dfa 100644 --- a/systest/cloud/cloud_test.go +++ b/systest/cloud/cloud_test.go @@ -160,10 +160,12 @@ func TestEnvironmentAccess(t *testing.T) { require.Contains(t, resp.Errors.Error(), "task failed") // Export with the minio creds should work for non-galaxy. - resp = testutil.Export(t, nsToken, minioDest, "accesskey", "secretkey") + // Secret key is padded to 14+ bytes so "AWS4"+secret satisfies the + // OpenSSL FIPS provider's HMAC key-length minimum (see dgraph/minio.env). + resp = testutil.Export(t, nsToken, minioDest, "accesskey", "secretkey-long-enough") require.Zero(t, len(resp.Errors)) // Galaxy guardian should provide the credentials as well. - resp = testutil.Export(t, galaxyToken, minioDest, "accesskey", "secretkey") + resp = testutil.Export(t, galaxyToken, minioDest, "accesskey", "secretkey-long-enough") require.Zero(t, len(resp.Errors)) } diff --git a/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index 3dbef7b6428..35dd3ceb6a6 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -21,6 +21,20 @@ import ( "github.com/stretchr/testify/require" ) +// skipIfFIPSBinary skips the current test when either the test binary is +// FIPS-tagged (x.FIPSEnabled) or the dgraph binary under test is FIPS- +// restricted (x.FIPSBinary). Upgrade-path tests pin a pre-FIPS upstream +// version for the "old" binary; that version predates any FIPS-enforcing +// toolchain, so attempting to build it under a FIPS configuration either +// fails outright or produces a binary that refuses to start. The test is +// semantically valid upstream and on non-FIPS forks; we skip only when +// FIPS enforcement rules it out. +func skipIfFIPSBinary(t *testing.T) { + if x.FIPSEnabled || x.FIPSBinary() { + t.Skip("upgrade-path test pins a pre-FIPS upstream version; skipping under FIPS build") + } +} + type S struct { Predicate string `json:"predicate"` Type string `json:"type"` @@ -34,6 +48,7 @@ type Received struct { } func testDuplicateUserUpgradeStrat(t *testing.T, strat dgraphtest.UpgradeStrategy) { + skipIfFIPSBinary(t) conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1). WithReplicas(1).WithACL(time.Hour).WithVersion("v23.0.1") c, err := dgraphtest.NewLocalCluster(conf) diff --git a/testutil/graphql.go b/testutil/graphql.go index 9688561d80c..dad603d63a6 100644 --- a/testutil/graphql.go +++ b/testutil/graphql.go @@ -275,7 +275,14 @@ func AppendAuthInfo(schema []byte, algo, publicKeyFile string, closedByDefault b var verificationKey string switch algo { case "HS256": - verificationKey = "secretkey" + // Widened from the original 9-byte "secretkey" to meet the 14-byte + // (112-bit) HMAC key minimum that NIST SP 800-131A requires and + // that some FIPS-validated crypto providers (e.g. the OpenSSL FIPS + // provider used by Chainguard go-fips / MS-Go requirefips builds) + // enforce at EVP_MAC_init. Benign for non-FIPS builds — a longer + // HMAC key is always acceptable. See graphql/resolve/auth_test.go + // for the matching hardcoded JWT tokens signed with this value. + verificationKey = "secretkey-long-enough" case "RS256": keyData, err := os.ReadFile(publicKeyFile) if err != nil { diff --git a/testutil/minio.go b/testutil/minio.go index 9188a32cdc5..f21a8ca0710 100644 --- a/testutil/minio.go +++ b/testutil/minio.go @@ -21,8 +21,12 @@ func NewMinioClient() (*minio.Client, error) { return nil, fmt.Errorf("testutil.MinioInstance is not set") } + // Secret key padded to 14+ bytes so "AWS4"+secret satisfies the + // OpenSSL FIPS provider's HMAC key-length minimum when the test + // client is run against a FIPS-enforcing alpha/minio pair. Matches + // MINIO_SECRET_KEY in dgraph/minio.env, systest/backup.env. mc, err := minio.New(MinioInstance, &minio.Options{ - Creds: credentials.NewStaticV4("accesskey", "secretkey", ""), + Creds: credentials.NewStaticV4("accesskey", "secretkey-long-enough", ""), Secure: false, }) if err != nil { From 5f262625f3dff799b236846b60d3e4fba855ce5a Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 20:43:16 -0400 Subject: [PATCH 12/16] style(admin): gofmt admin_auth_test.go Previously the map literal had extra spacing alignment (backup " ", config " ", draining " ", restore " ") that gofmt no longer emits \u2014 the current gofmt collapses to single-space after the colon when any key exceeds a threshold. Also the file was missing a trailing newline. Apply `gofmt -w graphql/admin/admin_auth_test.go`. No functional change. --- graphql/admin/admin_auth_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/graphql/admin/admin_auth_test.go b/graphql/admin/admin_auth_test.go index a88b5c5f708..432ed679674 100644 --- a/graphql/admin/admin_auth_test.go +++ b/graphql/admin/admin_auth_test.go @@ -53,10 +53,10 @@ func TestAdminMutationMiddlewareConfig(t *testing.T) { tests := map[string]securityRequirements{ // Superadmin (Guardian-of-Galaxy) auth — highest privilege operations. - "backup": {desc: "database backups", ipWhitelist: true, superAdminAuth: true}, - "config": {desc: "cluster config changes", ipWhitelist: true, superAdminAuth: true}, - "draining": {desc: "draining mode", ipWhitelist: true, superAdminAuth: true}, - "restore": {desc: "backup restore", ipWhitelist: true, superAdminAuth: true}, + "backup": {desc: "database backups", ipWhitelist: true, superAdminAuth: true}, + "config": {desc: "cluster config changes", ipWhitelist: true, superAdminAuth: true}, + "draining": {desc: "draining mode", ipWhitelist: true, superAdminAuth: true}, + "restore": {desc: "backup restore", ipWhitelist: true, superAdminAuth: true}, "restoreTenant": { // CVE: previously absent from this map (CVSS 10.0) desc: "cross-namespace backup restore — accepts attacker-controlled URLs", ipWhitelist: true, @@ -112,4 +112,4 @@ func TestAdminMutationMiddlewareConfig(t *testing.T) { } }) } -} \ No newline at end of file +} From 34fcfa30c1ae4463e75858e81857fcb169f33e4d Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Thu, 23 Apr 2026 22:36:13 -0400 Subject: [PATCH 13/16] refactor(dgraphtest,buildvars,t): address review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review fixes: dgraphtest/load.go + dgraphtest/image.go: - HostDgraphBinaryPath used hardcoded "dgraph" on Linux instead of buildvars.Bin.Get(). copyBinary (image.go) already writes the binary to /buildvars.Bin.Get() so a user who sets BIN without also setting HostBinaryName would get a stale path. Fix: consult buildvars.Bin.Get() in the Linux branch. - "dgraph_host" was hardcoded in two places (image.go line 85, load.go line 43) and could typo-drift. Extract to a package-level constant hostBinaryFileName in load.go; reference from both. - image.go line 83 "hostSrc" was $GOPATH/bin/dgraph; switch to $GOPATH/bin/buildvars.Bin.Get() for consistency with the binary name the rest of the package uses. - Godoc on HostDgraphBinaryPath updated to reference buildvars.Bin and the HostBinaryName hook so the contract is self-documenting. t/hooks.go: - defaultComposeFileArgs ignored the baseDir parameter but had a named receiver, which reads as if the default would use it. Rename to _ and add a brief godoc explaining why the parameter exists even though the default ignores it (so overrides that need it need not thread it through some other channel). buildvars/buildvars.go + buildvars/buildvars_test.go + buildvars/cmd/buildvars/main.go: - Copyright year bumped from 2017-2025 to 2017-2026, matching the other new files in this PR (hooks.go files, x/fips_base.go). All files in this PR now have consistent 2026 copyright. - Example binary name "dgraph-sec" in godoc and test fixtures replaced with "myfork-dgraph" — consistent with the "myfork requirefips" placeholder already used in the ExtraBuildTags godoc, so no downstream fork is named by example. Tested: go build ./... , go test ./buildvars/ ./x/ ./graphql/resolve/ -run "TestStringCustomClaim|TestAudienceClaim|TestJWTExpiry" all pass. gofmt -l clean on touched files. --- buildvars/buildvars.go | 6 +++--- buildvars/buildvars_test.go | 8 ++++---- buildvars/cmd/buildvars/main.go | 2 +- dgraphtest/image.go | 7 ++++--- dgraphtest/load.go | 18 +++++++++++++----- t/hooks.go | 10 ++++++++-- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index 29648b72c5d..5a6d59433cf 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. * SPDX-License-Identifier: Apache-2.0 */ @@ -21,7 +21,7 @@ // Forks override literal defaults from their own package init(): // // func init() { -// buildvars.Bin.SetDefault("dgraph-sec") +// buildvars.Bin.SetDefault("myfork-dgraph") // } // // Derived Vars automatically pick up the override because they recompute @@ -182,7 +182,7 @@ var ( // (what `go build -o` writes, matching upstream's $(BIN) in // dgraph/Makefile) and at runtime (what compose files and test // harnesses invoke as /gobin/$BIN). Upstream default: "dgraph". - // Forks rename (e.g. "dgraph-sec") via env or SetDefault. + // Forks rename (e.g. "myfork-dgraph") via env or SetDefault. Bin = newVar("BIN", "dgraph") // DockerImage is the Docker image tag (without :tag suffix) used by diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go index 5ec958285e5..279c0118231 100644 --- a/buildvars/buildvars_test.go +++ b/buildvars/buildvars_test.go @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. * SPDX-License-Identifier: Apache-2.0 */ @@ -310,9 +310,9 @@ func TestGoBinDgraphPath_TracksBin(t *testing.T) { os.Unsetenv(string(GoBinDgraphPath.Name)) // Override via env - t.Setenv("BIN", "dgraph-sec") - if got := GoBinDgraphPath.Get(); got != "/gobin/dgraph-sec" { - t.Errorf("via env: GoBinDgraphPath.Get() = %q, want %q", got, "/gobin/dgraph-sec") + t.Setenv("BIN", "myfork-dgraph") + if got := GoBinDgraphPath.Get(); got != "/gobin/myfork-dgraph" { + t.Errorf("via env: GoBinDgraphPath.Get() = %q, want %q", got, "/gobin/myfork-dgraph") } // Override via SetDefault diff --git a/buildvars/cmd/buildvars/main.go b/buildvars/cmd/buildvars/main.go index 6533e376beb..2108bd21f29 100644 --- a/buildvars/cmd/buildvars/main.go +++ b/buildvars/cmd/buildvars/main.go @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: © 2017-2025 Istari Digital, Inc. + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. * SPDX-License-Identifier: Apache-2.0 */ diff --git a/dgraphtest/image.go b/dgraphtest/image.go index 24de0536ce3..6b86e6bdb01 100644 --- a/dgraphtest/image.go +++ b/dgraphtest/image.go @@ -79,10 +79,11 @@ func (c *LocalCluster) setupBinary() error { return err } - // 2. Copy the host-native binary (for local bulk/live commands) as "dgraph_host". - hostSrc := filepath.Join(gopath, "bin", "dgraph") + // 2. Copy the host-native binary (for local bulk/live commands) as + // hostBinaryFileName (see load.go). + hostSrc := filepath.Join(gopath, "bin", buildvars.Bin.Get()) - hostDst := filepath.Join(c.tempBinDir, "dgraph_host") + hostDst := filepath.Join(c.tempBinDir, hostBinaryFileName) if err := copyFile(hostSrc, hostDst); err != nil { return errors.Wrapf(err, "error copying host-native binary from [%v] to [%v]", hostSrc, hostDst) } diff --git a/dgraphtest/load.go b/dgraphtest/load.go index 9f40d3262c5..6c353e7b690 100644 --- a/dgraphtest/load.go +++ b/dgraphtest/load.go @@ -24,23 +24,31 @@ import ( "github.com/pkg/errors" "github.com/dgraph-io/dgo/v250/protos/api" + "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/dgraphapi" "github.com/dgraph-io/dgraph/v25/enc" "github.com/dgraph-io/dgraph/v25/x" ) +// hostBinaryFileName is the conventional filename for the host-OS-native +// dgraph binary when it differs from the container binary (non-Linux). On +// Linux the host and container binaries are the same file (named +// buildvars.Bin.Get()) and this constant is unused. +const hostBinaryFileName = "dgraph_host" + // HostDgraphBinaryPath returns the path to the host-OS-native dgraph binary -// in tempBinDir. On Linux this is simply "dgraph" (the same binary used by -// Docker containers). On non-Linux (macOS) it is "dgraph_host", a separate -// native binary copied by setupBinary(). +// in tempBinDir. On Linux this is the same binary used by Docker containers +// (named by buildvars.Bin.Get(), typically "dgraph"). On non-Linux (macOS) +// it is a separate native binary staged as hostBinaryFileName by setupBinary(). +// The HostBinaryName hook lets a fork override the filename entirely. func (c *LocalCluster) HostDgraphBinaryPath() string { if name := HostBinaryName(); name != "" { return filepath.Join(c.tempBinDir, name) } if runtime.GOOS == "linux" { - return filepath.Join(c.tempBinDir, "dgraph") + return filepath.Join(c.tempBinDir, buildvars.Bin.Get()) } - return filepath.Join(c.tempBinDir, "dgraph_host") + return filepath.Join(c.tempBinDir, hostBinaryFileName) } var datafiles = map[string]string{ diff --git a/t/hooks.go b/t/hooks.go index 9a864c367a0..1d897abad6f 100644 --- a/t/hooks.go +++ b/t/hooks.go @@ -30,5 +30,11 @@ var EnvForCompose = defaultEnvForCompose // fall through to the upstream-compatible "-f " form. var ComposeFileArgs = defaultComposeFileArgs -func defaultEnvForCompose() []string { return nil } -func defaultComposeFileArgs(path, baseDir string) []string { return []string{"-f", path} } +func defaultEnvForCompose() []string { return nil } + +// defaultComposeFileArgs ignores baseDir; it only exists in the signature so +// overrides that need it (for resolving paths relative to the repo root) do +// not have to thread it through some other channel. +func defaultComposeFileArgs(path, _ string) []string { + return []string{"-f", path} +} From e61b275d26ded63e541de3a9a7f9b1dc6f6760c4 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 02:19:13 -0400 Subject: [PATCH 14/16] fix(t): 5s grace after resume for alpha->zero membership sync HTTP /health returns OK before alpha has fully re-established its internal connection pool to zero after a docker-compose start. When tests run immediately after resumeDefault, the first query (DropAll, Alter, Login) occasionally hits "No connection exists" because groups().Leader(0) is still nil even though /health is 200. Add a 5-second sleep after the health-check wait-group in resumeDefault so membership sync can complete before the next test runs. Cheap compared to a whole cluster recreate on failure. Does not affect startCluster (the initial bring-up already includes its own health checks + retries). Surfaced by a test harness running many pause/resume cycles on the default cluster: the first test after a resume failed in 0.08s on DropAll/Alter/Login with "No connection exists", while subsequent runs of the same test pass. --- t/t.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/t.go b/t/t.go index 3566fcda13b..574ddd20246 100644 --- a/t/t.go +++ b/t/t.go @@ -752,6 +752,13 @@ func runTests(taskCh chan task, closer *z.Closer) error { }(i) } resumeWg.Wait() + // HTTP /health returns OK before alpha has fully re-established its + // internal connection pool to zero after a docker-compose `start`. + // The first test to run immediately after resume occasionally hits + // "No connection exists" in DropAll/Alter because groups().Leader(0) + // is still nil. Sleep a few seconds so membership sync can complete. + // Cheap compared to a whole cluster restart on failure. + time.Sleep(5 * time.Second) defaultPaused = false return nil } From 109c2f64f08d8629733e4ad0d3ebd58fdeafe29c Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 02:56:29 -0400 Subject: [PATCH 15/16] fix(t): 3 up-attempts with 2s delay between for docker volume-init race Observed in CI that the previous retry-once logic was insufficient for the docker volume-init race when multiple services share a named volume. The retry happened only milliseconds after `down -v`, which was not long enough for docker to release the volume mount points from the first failed attempt. The second `up` tried to populate a fresh volume and hit the same race. Two fixes: 1. Increase upAttempts from 2 to 3 (one extra retry). 2. Sleep 2 seconds between `down -v` and the next `up` so docker has time to release mount points. The cost is 2 seconds in the rare case where bring-up succeeds on the retry, versus a full cluster-recreate path (10+ seconds) on failure. Cheap. --- t/t.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/t/t.go b/t/t.go index 574ddd20246..bca0084ac57 100644 --- a/t/t.go +++ b/t/t.go @@ -247,7 +247,7 @@ func startCluster(composeFile, prefix string) error { // Retry once after a full `down -v` — the second attempt finds a fresh // volume and succeeds. One retry is enough in practice for this class // of race; anything persistent is a real configuration error. - const upAttempts = 2 + const upAttempts = 3 var lastErr error var cmdStderr strings.Builder fmt.Printf("Bringing up cluster %s for package: %s ...\n", prefix, composeFile) @@ -266,13 +266,19 @@ func startCluster(composeFile, prefix string) error { fmt.Printf("docker compose stderr:\n%s\n", stderr) } if attempt < upAttempts { - // Tear down any partial state so the retry starts from - // a clean volume/network baseline. + // Tear down any partial state so the retry starts from a + // clean volume/network baseline. Run `down -v` to remove + // the volume the next attempt will recreate, then sleep + // briefly so docker has time to release the mount points + // before the next `up` tries to populate a fresh volume. + // Without the sleep, the retry can hit the same volume- + // init race the first attempt did. downArgs := append([]string{"docker", "compose", "--compatibility"}, append(composeArgs, "-p", prefix, "down", "-v")...) downCmd := command(downArgs...) downCmd.Stderr = nil _ = downCmd.Run() + time.Sleep(2 * time.Second) fmt.Printf("Retrying cluster bring-up (attempt %d/%d) after `down -v`...\n", attempt+1, upAttempts) } } From d63438211f02512369664b9823480415bbf8c119 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 04:33:30 -0400 Subject: [PATCH 16/16] fix(ci): align MinIO secret in env files with Go-side widened key Matches commit 220a9ce5d on the fork. The Go code at: systest/bulk_live/common/bulk_live_fixture.go:128 systest/backup/encryption/backup_test.go:112 systest/backup/minio/backup_test.go:108 systest/cloud/cloud_test.go:165,169 testutil/graphql.go:284 testutil/minio.go widens MINIO_SECRET_KEY from "secretkey" to "secretkey-long-enough" (part of the HMAC-widening commit 8d185f9c4). But the MinIO server configured by the corresponding .env files still used "secretkey", causing TestBulkCases, TestBackupMinioE, TestBackupMinio to fail with "The request signature we calculated does not match the signature you provided" when the Go client tries to sign with the widened key against a MinIO server expecting the narrow one. Fix: update MINIO_SECRET_KEY to "secretkey-long-enough" in all three .env files: - dgraph/minio.env - systest/backup.env - systest/export/export.env Server and client now agree. Observed failing in upstream PR CI runs 24878931876 (dgraph-load-tests) and 24878931803 (dgraph-systest-tests). --- dgraph/minio.env | 2 +- systest/backup.env | 2 +- systest/export/export.env | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dgraph/minio.env b/dgraph/minio.env index e17cd13bad3..a72029649e2 100644 --- a/dgraph/minio.env +++ b/dgraph/minio.env @@ -1,2 +1,2 @@ MINIO_ACCESS_KEY=accesskey -MINIO_SECRET_KEY=secretkey +MINIO_SECRET_KEY=secretkey-long-enough diff --git a/systest/backup.env b/systest/backup.env index e17cd13bad3..a72029649e2 100644 --- a/systest/backup.env +++ b/systest/backup.env @@ -1,2 +1,2 @@ MINIO_ACCESS_KEY=accesskey -MINIO_SECRET_KEY=secretkey +MINIO_SECRET_KEY=secretkey-long-enough diff --git a/systest/export/export.env b/systest/export/export.env index e17cd13bad3..a72029649e2 100644 --- a/systest/export/export.env +++ b/systest/export/export.env @@ -1,2 +1,2 @@ MINIO_ACCESS_KEY=accesskey -MINIO_SECRET_KEY=secretkey +MINIO_SECRET_KEY=secretkey-long-enough