From 6ce61b9c3086d5213cba4689f42f37937df6942f Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:55:57 -0400 Subject: [PATCH 01/24] feat(buildvars): introduce typed build-config registry Replace hardcoded binary/image literals with lookups through a typed registry populated at build time. Introduces buildvars package, renames cmd/buildvarprint -> cmd/buildvars, aligns Go identifier ExtraBuildTags -> PrivateBuildTags. Migrates hardcoded call sites in contrib/jepsen, testutil/exec, dgraphtest/dgraph, and systest scripts. --- buildvars/buildvars.go | 363 +++++++++++++++++++ buildvars/buildvars_test.go | 324 +++++++++++++++++ buildvars/cmd/buildvars/main.go | 74 ++++ contrib/jepsen/main.go | 3 +- dgraphtest/dgraph.go | 15 +- systest/1million/test-reindex.sh | 2 +- systest/21million/test-21million.sh | 2 +- systest/loader-benchmark/loader-benchmark.sh | 2 +- testutil/exec.go | 4 +- 9 files changed, 781 insertions(+), 8 deletions(-) create mode 100644 buildvars/buildvars.go create mode 100644 buildvars/buildvars_test.go create mode 100644 buildvars/cmd/buildvars/main.go diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go new file mode 100644 index 00000000000..5a6d59433cf --- /dev/null +++ b/buildvars/buildvars.go @@ -0,0 +1,363 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 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("myfork-dgraph") +// } +// +// Derived Vars automatically pick up the override because they recompute +// at Get() time. +// +// 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 ( + "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 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 + 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. "myfork-dgraph") 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("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 + // well-known libjemalloc tag) with any tags a fork adds via + // [PrivateBuildTags]. 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 := PrivateBuildTags.Get() + if extra == "" { + return base + } + return base + " " + extra + }) + + // 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 + // PrivateBuildTags onto BuildTags, so callers typically read BuildTags + // to see the final tag set. + // + // "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/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 + // (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 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 + // 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, + PrivateBuildTags, + 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..279c0118231 --- /dev/null +++ b/buildvars/buildvars_test.go @@ -0,0 +1,324 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 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_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" + PrivateBuildTags.SetDefault("") + if got := BuildTags.Get(); got != "jemalloc" { + t.Errorf("with empty extra: BuildTags.Get() = %q, want %q", got, "jemalloc") + } + + // Extra set → composed + 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") + } + + // 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", "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 + 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/buildvars/main.go b/buildvars/cmd/buildvars/main.go new file mode 100644 index 00000000000..2108bd21f29 --- /dev/null +++ b/buildvars/cmd/buildvars/main.go @@ -0,0 +1,74 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +// 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. +// +// 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/buildvars)" # shell +// go run ./buildvars/cmd/buildvars -format=plain # plain +// $(eval $(shell go run ./buildvars/cmd/buildvars -format=make)) +// +// Intended callers: +// +// make build-env — uses shell format for eval sourcing +// 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 +// 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 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 { + 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, "buildvars: unknown format %q (want shell|make|plain)\n", *format) + os.Exit(2) + } +} 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 bc7fdcea707..9c004ac6895 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 ( @@ -133,7 +135,7 @@ func (z *zero) cmd(c *LocalCluster) []string { if z.myAddrOverride != "" { myAddr = z.myAddrOverride } - zcmd := []string{"/gobin/dgraph", "zero", fmt.Sprintf("--my=%s", myAddr), "--bindall", + zcmd := []string{buildvars.GoBinDgraphPath.Get(), "zero", fmt.Sprintf("--my=%s", myAddr), "--bindall", fmt.Sprintf(`--replicas=%v`, c.conf.replicas), "--logtostderr", fmt.Sprintf("-v=%d", c.conf.verbosity)} if c.lowerThanV21 { @@ -241,7 +243,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 { @@ -432,7 +434,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/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: Fri, 24 Apr 2026 13:56:05 -0400 Subject: [PATCH 02/24] fix(makefile): make BIN overridable, expand jemalloc detection, use go build -o Allow BIN path override via environment. Expand jemalloc detection to handle both install prefixes. Switch dgraph and t Makefiles to 'go build -o' for explicit output paths. --- 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 2cb32eefc6cfd32de7ea7699ac509baa39ac3c28 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:56:11 -0400 Subject: [PATCH 03/24] test(restore): skip namespace-aware restore tests on non-Linux Namespace-aware restore depends on Linux-only kernel features. Guard the test file with //go:build linux so Darwin CI does not fail. --- 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 bfb7e3d988d02f0cc600c3b0e9cd17a0959bae4d Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:56:17 -0400 Subject: [PATCH 04/24] feat(x): add FIPSEnabled var + FIPSBinary() runtime helper Add untagged package-level FIPSEnabled var so non-FIPS builds can still reference x.FIPSEnabled. Add FIPSBinary() returning whether the dgraph binary under test is a FIPS build (via DGRAPH_FIPS_BINARY env var). Enables FIPS-aware test skips without tag-guarded symbols. --- 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 7d5e22ba6658214ff0028d5010dc899afabc43af Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:56:29 -0400 Subject: [PATCH 05/24] refactor(testutil): extract ComposeArgs, EnvForCompose, BuildPlugins hooks Relocate environment-assembly and plugin-build logic behind extension hooks so downstream forks can layer FIPS-aware variants without patching upstream helpers. Hooks default to the existing behavior. --- 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 4fcbf6ef06c705c36d2023e4cacc4dee49bb1305 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:56:37 -0400 Subject: [PATCH 06/24] =?UTF-8?q?refactor(compose,dgraphtest):=20add=20ext?= =?UTF-8?q?ension=20hooks;=20rename=20copy=20=E2=86=92=20copyFile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract compose-file selection and cluster image-resolution behind hooks so downstream forks can insert FIPS- and registry-aware variants without forking the files. Rename the local 'copy' helper to 'copyFile' to avoid shadowing the standard library name. --- compose/compose.go | 10 +++-- compose/hooks.go | 18 +++++++++ dgraphtest/hooks.go | 77 +++++++++++++++++++++++++++++++++++++ dgraphtest/image.go | 27 ++++++++----- dgraphtest/load.go | 21 +++++++--- dgraphtest/local_cluster.go | 19 +++++++++ 6 files changed, 154 insertions(+), 18 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 d0417ed1814..6b86e6bdb01 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. @@ -58,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) @@ -73,11 +79,12 @@ 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") - if err := copy(hostSrc, hostDst); err != nil { + 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) } return nil @@ -214,7 +221,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") } @@ -222,19 +229,19 @@ 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") - if err := copy(fromPath, toPath); err != nil { + toPath := filepath.Join(toDir, buildvars.Bin.Get()) + 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..6c353e7b690 100644 --- a/dgraphtest/load.go +++ b/dgraphtest/load.go @@ -24,20 +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/dgraphtest/local_cluster.go b/dgraphtest/local_cluster.go index 5e102acdf84..5d7ffb18587 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{ @@ -1311,6 +1318,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 { @@ -1318,6 +1328,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 @@ -1374,6 +1387,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 f68112de3ecd6e63bd0ce12d703cd571e3ee47f8 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:56:48 -0400 Subject: [PATCH 07/24] test: widen HMAC/JWT/MinIO test secrets + add FIPS-aware skip helpers FIPS crypto policies require secret keys of at least 14 bytes. Widen shared test secrets in HMAC, JWT, and MinIO fixtures, and align the env files used by docker-compose test harnesses. Replace x.FIPSEnabled truthiness with x.FIPSBinary() calls at test skip sites so non-FIPS compilations can still reference FIPS awareness without tag-guarded symbols. --- acl/jwt_algo_test.go | 8 +++++ check_upgrade/check_upgrade_test.go | 16 +++++++++ dgraph/minio.env | 2 +- 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.env | 2 +- 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/export/export.env | 2 +- systest/integration2/acl_test.go | 15 ++++++++ testutil/graphql.go | 9 ++++- testutil/minio.go | 6 +++- 16 files changed, 106 insertions(+), 45 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/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/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.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/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/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 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 1252e9c2e52af6adb6748ac3506f53771e2c8b67 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:56:57 -0400 Subject: [PATCH 08/24] refactor(t): extract EnvForCompose + ComposeFileArgs hooks; add resilience Pull compose environment assembly and compose-file argument selection behind hooks so downstream forks can supply FIPS- and registry-aware variants without touching upstream t/t.go. Resilience improvements bundled in the same file: - 3 up-attempts with 2s delay between in startCluster for docker volume-init races (300ms was insufficient) - 5s grace after resumeDefault's health-check wait for alpha->zero connection pool re-establishment Discovered during CI stabilization; universally beneficial. --- t/hooks.go | 40 ++++++++++++ t/t.go | 178 +++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 187 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..1d897abad6f --- /dev/null +++ b/t/hooks.go @@ -0,0 +1,40 @@ +/* + * 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 } + +// 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} +} diff --git a/t/t.go b/t/t.go index 168b6d364c7..bca0084ac57 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,58 @@ 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 = 3 + 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. 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) + } + } + } + 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 +339,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 +379,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 +430,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 +499,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 +633,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 +702,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 +723,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) @@ -665,6 +758,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 } @@ -740,7 +840,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 +944,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 +987,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 76986ee084c9a5a54c4696784f1a1464648b8326 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Fri, 24 Apr 2026 13:57:02 -0400 Subject: [PATCH 09/24] style(admin): gofmt admin_auth_test.go --- 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 c106991d6c053ebb6fdaf8eb91cf281b76aae7fb Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 13:44:28 -0400 Subject: [PATCH 10/24] fix(docker): retry apt-get on transient mirror sync failures archive.ubuntu.com publishes index updates non-atomically; CI builds intermittently fail with 'File has unexpected size... Mirror sync in progress?' when the runner fetches Packages.gz mid-publish. Retry the install once after a 15-second pause and a fresh apt list, which is sufficient to ride out the transient. While here, drop a duplicate 'htop' line in the package list (was listed twice). Same retry pattern is used in the Chainguard go-fips image's apk calls; this makes contrib/Dockerfile equivalently resilient. --- contrib/Dockerfile | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/contrib/Dockerfile b/contrib/Dockerfile index d7ebe448fbf..e8ca1683c74 100644 --- a/contrib/Dockerfile +++ b/contrib/Dockerfile @@ -14,21 +14,30 @@ RUN rm -rf /var/lib/apt/lists/* # use cache busting to avoid old versions # remove /var/lib/apt/lists/* to reduce image size. # see: https://docs.docker.com/develop/develop-images/dockerfile_best-practices -RUN apt-get update && apt-get install -y --no-install-recommends \ - ca-certificates \ - htop \ - curl \ - htop \ - iputils-ping \ - jq \ - libjemalloc-dev \ - less \ - sysstat \ - gnupg \ - gnupg2 \ - tar \ - libpam0g \ - && rm -rf /var/lib/apt/lists/* +# +# Retry the install once on transient apt-get errors (most commonly the +# `File has unexpected size... Mirror sync in progress?` race against +# archive.ubuntu.com mid-publish, which surfaces frequently in CI). The +# rm + sleep between attempts gives the mirror time to settle and forces +# a fresh package-list fetch. +RUN set -eu; \ + install_pkgs() { \ + apt-get update && apt-get install -y --no-install-recommends \ + ca-certificates \ + htop \ + curl \ + iputils-ping \ + jq \ + libjemalloc-dev \ + less \ + sysstat \ + gnupg \ + gnupg2 \ + tar \ + libpam0g; \ + }; \ + install_pkgs || (sleep 15 && rm -rf /var/lib/apt/lists/* && install_pkgs); \ + rm -rf /var/lib/apt/lists/* ADD linux /usr/local/bin From 352ff5afe41e25d4085ab70e82535a6d69ed48b2 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 16:30:06 -0400 Subject: [PATCH 11/24] address review feedback - buildvars: rename PrivateBuildTags -> CustomBuildTags; PRIVATE_BUILD_TAGS -> CUSTOM_BUILD_TAGS. 'Private' read as access-control; 'Custom' is clearer for the consumer-extensibility intent. - compose/compose.go: explain EmitUser hook (default "" inherits image USER; downstream non-root compose tests override to set service user). - dgraphtest/load.go: tighten hostBinaryFileName grammar. - dgraphtest/local_cluster.go: add hook-call comments at the four override points (WidenTempDirPerms x2, ApplyContainerUser, WidenSecretFilePerms, GeneratePlugins) explaining each is a public hook with a no-op default and the downstream-override scenario. - graphql/e2e/auth/auth_test.go: explain why JWT fixtures were re-signed (NIST SP 800-131A 14-byte HMAC minimum + FIPS provider enforcement). - systest/{1million,21million,loader-benchmark}/*.sh: prefer the canonical buildvars path ${GOBIN_DGRAPH_PATH:-/gobin/dgraph} over the synthetic /gobin/${BIN:-dgraph}; both resolve the same way but the former matches the registry's named Var. --- buildvars/buildvars.go | 25 ++++++++---------- buildvars/buildvars_test.go | 12 ++++----- compose/compose.go | 7 +++++ dgraphtest/load.go | 4 +-- dgraphtest/local_cluster.go | 27 ++++++++++++++++++++ graphql/e2e/auth/auth_test.go | 9 +++++++ systest/1million/test-reindex.sh | 2 +- systest/21million/test-21million.sh | 2 +- systest/loader-benchmark/loader-benchmark.sh | 2 +- 9 files changed, 64 insertions(+), 26 deletions(-) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index 5a6d59433cf..f2ee2d1072c 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 - // [PrivateBuildTags]. Both the Makefile and this Var use the same + // [CustomBuildTags]. Both the Makefile and this Var use the same // composition, so `make` and direct `go run` agree on the final // tag set. // @@ -224,25 +224,20 @@ var ( // Makefile handles OS gating for the actual `go build` invocation. BuildTags = newDerivedVar("BUILD_TAGS", func() string { base := "jemalloc" - extra := PrivateBuildTags.Get() + extra := CustomBuildTags.Get() if extra == "" { return base } return base + " " + extra }) - // 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 - // PrivateBuildTags onto BuildTags, so callers typically read BuildTags - // to see the final tag set. - // - // "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", "") + // CustomBuildTags is the list of additional Go build tags a fork or + // downstream consumer appends to [BuildTags]. Upstream default: empty. + // Consumers that need compile-time code paths set this to space- + // separated tag names (e.g. "myfork requirefips"). The Makefile + // concatenates CustomBuildTags onto BuildTags, so callers typically + // read BuildTags to see the final tag set. + CustomBuildTags = newVar("CUSTOM_BUILD_TAGS", "") // GoRunTags is the build-tag list passed via -tags to `go run` // invocations from Makefile helper recipes (e.g. `build-env` runs @@ -349,7 +344,7 @@ var All = []*Var{ RuntimeTag, ComposeBuildDir, BuildTags, - PrivateBuildTags, + CustomBuildTags, GoRunTags, DgraphVersion, Build, diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go index 279c0118231..33f863221a7 100644 --- a/buildvars/buildvars_test.go +++ b/buildvars/buildvars_test.go @@ -244,21 +244,21 @@ func TestExport_Idempotent(t *testing.T) { } } -func TestBuildTags_ComposesWithPrivateBuildTags(t *testing.T) { - origExtra := PrivateBuildTags.Default() - t.Cleanup(func() { PrivateBuildTags.SetDefault(origExtra) }) +func TestBuildTags_ComposesWithCustomBuildTags(t *testing.T) { + origExtra := CustomBuildTags.Default() + t.Cleanup(func() { CustomBuildTags.SetDefault(origExtra) }) os.Unsetenv("BUILD_TAGS") - os.Unsetenv("PRIVATE_BUILD_TAGS") + os.Unsetenv("CUSTOM_BUILD_TAGS") // Empty extra → just "jemalloc" - PrivateBuildTags.SetDefault("") + CustomBuildTags.SetDefault("") if got := BuildTags.Get(); got != "jemalloc" { t.Errorf("with empty extra: BuildTags.Get() = %q, want %q", got, "jemalloc") } // Extra set → composed - PrivateBuildTags.SetDefault("myfork requirefips") + CustomBuildTags.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/compose/compose.go b/compose/compose.go index f7b27408f01..1d123a93994 100644 --- a/compose/compose.go +++ b/compose/compose.go @@ -155,6 +155,13 @@ 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) + // EmitUser is a public hook (compose/hooks.go); the upstream default + // returns "" so the generated compose service inherits the image's + // own USER directive. Downstream consumers that run dgraph as a + // non-root user under compose can override the hook to return a + // "uid:gid" string (e.g. "${UID:-65532}"), which is then emitted as + // the service's `user:` field. Bind-mounted host paths must be + // readable by that uid for the container to start cleanly. if u := EmitUser(); u != "" { svc.User = u } diff --git a/dgraphtest/load.go b/dgraphtest/load.go index 6c353e7b690..dd500861b48 100644 --- a/dgraphtest/load.go +++ b/dgraphtest/load.go @@ -32,8 +32,8 @@ import ( // 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. +// Linux the host and container binaries are the same file name (returned +// by buildvars.Bin.Get()) and this constant is unused. const hostBinaryFileName = "dgraph_host" // HostDgraphBinaryPath returns the path to the host-OS-native dgraph binary diff --git a/dgraphtest/local_cluster.go b/dgraphtest/local_cluster.go index 6e7abcba041..0018cb604fe 100644 --- a/dgraphtest/local_cluster.go +++ b/dgraphtest/local_cluster.go @@ -118,6 +118,11 @@ func (c *LocalCluster) init() error { if err != nil { return errors.Wrap(err, "error while creating tempBinDir") } + // WidenTempDirPerms is a public hook (dgraphtest/hooks.go) — default + // is no-op. Downstream consumers running dgraph as a non-root user + // inside compose-test containers override it to widen perms on + // host-side temp dirs that are bind-mounted into containers, so the + // in-container uid can read/write those paths. if err := WidenTempDirPerms(c.tempBinDir); err != nil { return err } @@ -126,6 +131,7 @@ func (c *LocalCluster) init() error { if err != nil { return errors.Wrap(err, "error while creating tempSecretsDir") } + // Same hook, applied to the secrets temp dir. if err := WidenTempDirPerms(c.tempSecretsDir); err != nil { return err } @@ -312,6 +318,12 @@ func (c *LocalCluster) createContainer(dc dnode) (string, error) { } cconf := &container.Config{Cmd: cmd, Image: image, WorkingDir: dc.workingDir(), ExposedPorts: dc.ports()} + // ApplyContainerUser is a public hook (dgraphtest/hooks.go) — default + // is no-op. Downstream consumers that run dgraph as a non-root user + // inside the test container override it to set cconf.User to the + // host's uid:gid, so files the container writes are readable on the + // host (and bind-mounted host paths are readable inside the + // container). ApplyContainerUser(cconf) hconf := &container.HostConfig{Mounts: mts, PublishAllPorts: true, PortBindings: dc.bindings(c.conf.portOffset)} networkConfig := &network.NetworkingConfig{ @@ -1312,6 +1324,13 @@ func (c *LocalCluster) inspectContainer(containerID string) (string, error) { } func (c *LocalCluster) setupSecrets() error { + // WidenSecretFilePerms is a public hook (dgraphtest/hooks.go) — + // default is no-op. Secret files are written with mode 0600 + // (owner-only) which is correct upstream. Downstream consumers + // running the dgraph container as a non-root user that differs from + // the host owner override the hook to widen perms (e.g. add group- + // or world-read) so the in-container uid can read the bind-mounted + // secret files. if c.conf.encryption { // use this key because some of the data is already encrypted using this key. encKey := []byte("1234567890123456") @@ -1388,6 +1407,14 @@ func runOpennssl(args ...string) error { } func (c *LocalCluster) GeneratePlugins(raceEnabled bool) error { + // GeneratePlugins is a public hook (dgraphtest/hooks.go) — the + // upstream default returns (nil, false, nil), so the fallback path + // below (host-side `go build -buildmode=plugin`) runs. Downstream + // consumers whose toolchain isn't directly invokable on the host + // (e.g. forks pinning a Docker-only build image) override the hook + // to compile plugins inside that image and return their .so paths; + // when handled=true the override's outcome wins and the host-side + // fallback is skipped. if tokenizers, handled, err := GeneratePlugins(raceEnabled, c.tempBinDir, baseRepoDir); handled { if err == nil { c.customTokenizers = tokenizers diff --git a/graphql/e2e/auth/auth_test.go b/graphql/e2e/auth/auth_test.go index 1a0d60aaa6b..834ed6a9410 100644 --- a/graphql/e2e/auth/auth_test.go +++ b/graphql/e2e/auth/auth_test.go @@ -615,6 +615,15 @@ func TestQueryWithStandardClaims(t *testing.T) { if metaInfo.Algo == "RS256" { t.Skip() } + // JWTs below are pre-signed with the same secret used by the schema's + // VerificationKey ("secret-long-enough"). Both the secret and these + // fixture JWTs were re-signed when the test secrets were widened to + // 14+ bytes — the minimum HMAC key length NIST SP 800-131A requires + // and that some FIPS-validated crypto providers enforce at + // EVP_MAC_init. Shorter keys are valid under stock HMAC but rejected + // at signing time on FIPS builds. The signatures here were verified + // against the previous secret before being re-signed against the new + // one, so claim payloads are unchanged. testCases := []TestCase{ { query: ` diff --git a/systest/1million/test-reindex.sh b/systest/1million/test-reindex.sh index 64f03d42fa8..cf4e531b6ff 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/${BIN:-dgraph} bulk --schema=${NO_INDEX_SCHEMA_FILE} --files=${DATA_FILE} \ + ${GOBIN_DGRAPH_PATH:-/gobin/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 8c36f68f963..907853623ad 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/${BIN:-dgraph} bulk --schema=${SCHEMA_FILE} --files=${DATA_FILE} \ + ${GOBIN_DGRAPH_PATH:-/gobin/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 999862590af..1f74217e7bf 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: Mon, 4 May 2026 16:34:56 -0400 Subject: [PATCH 12/24] remove unhelpful comment on defaultCompose The comment was paraphrasing what ComposeFileArgs (t/hooks.go) already documents at its declaration, with extra fork-specific path references that don't belong in upstream. The variable name is self-documenting; the rewrite mechanism is explained on the hook. --- t/t.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/t/t.go b/t/t.go index bca0084ac57..21045ff24bb 100644 --- a/t/t.go +++ b/t/t.go @@ -633,12 +633,6 @@ 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() From e1427ca34018c7e810c0b4a4ec357b9420a9499d Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 16:37:14 -0400 Subject: [PATCH 13/24] rename buildvars.Bin -> buildvars.BinaryName MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Var's Go identifier was 'Bin' which reads as an abbreviation. The binary-naming concept benefits from a descriptive symbol; reads more naturally as buildvars.BinaryName.Get() at every call site (compose, dgraphtest, testutil, t). The env-var name stays as BIN — Makefiles, shell scripts, and CI configs reference $(BIN) / ${BIN} directly, and those are left unchanged. The Go identifier rename is the only intent. --- buildvars/buildvars.go | 33 +++++++++++++++++++-------------- buildvars/buildvars_test.go | 26 +++++++++++++------------- compose/compose.go | 2 +- dgraphtest/image.go | 6 +++--- dgraphtest/load.go | 6 +++--- t/t.go | 2 +- testutil/exec.go | 2 +- 7 files changed, 41 insertions(+), 36 deletions(-) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index f2ee2d1072c..259d69e57b7 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -11,17 +11,17 @@ // // 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/). +// Vars whose default depends on other Vars, e.g. /gobin/). // // Usage: // -// bin := buildvars.Bin.Get() // reads $BIN or default -// path := buildvars.GoBinDgraphPath.Get() // "/gobin/" +// bin := buildvars.BinaryName.Get() // reads $BIN or default +// path := buildvars.GoBinDgraphPath.Get() // "/gobin/" // // Forks override literal defaults from their own package init(): // // func init() { -// buildvars.Bin.SetDefault("myfork-dgraph") +// buildvars.BinaryName.SetDefault("myfork-dgraph") // } // // Derived Vars automatically pick up the override because they recompute @@ -58,7 +58,7 @@ func shellOutput(cmd string, args ...string) string { // 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. +// [BinaryName]) 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 @@ -91,7 +91,7 @@ func newVar(name, initialDefault string) *Var { // 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()). +// depends on other Vars (e.g. GoBinDgraphPath = "/gobin/" + BinaryName.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. @@ -178,12 +178,17 @@ func ExportAll() error { // 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 + // BinaryName 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. "myfork-dgraph") via env or SetDefault. - Bin = newVar("BIN", "dgraph") + // + // The env-var name is kept as BIN for backward compatibility with + // shell scripts, Makefiles, and CI configs that reference $(BIN) / + // ${BIN} directly. The Go identifier is BinaryName because Go + // callers benefit from the more descriptive symbol. + BinaryName = newVar("BIN", "dgraph") // DockerImage is the Docker image tag (without :tag suffix) used by // Makefile local-image / docker-image targets and by the compose @@ -311,24 +316,24 @@ var ( // 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 + // services configured with LocalBin. Derived from [BinaryName] at // Get() time so the path automatically tracks the binary name. GoBinDgraphPath = newDerivedVar("GOBIN_DGRAPH_PATH", func() string { - return "/gobin/" + Bin.Get() + return "/gobin/" + BinaryName.Get() }) // HostGopathDgraphPath is the host-side absolute path to the built - // dgraph binary ($GOPATH/bin/). Used by testutil and t/t.go + // 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. + // [BinaryName] at Get() time. HostGopathDgraphPath = newDerivedVar("HOST_GOPATH_DGRAPH_PATH", func() string { gopath := os.Getenv("GOPATH") if gopath == "" { return "" } - return gopath + "/bin/" + Bin.Get() + return gopath + "/bin/" + BinaryName.Get() }) ) @@ -336,7 +341,7 @@ var ( // tooling (diagnostic dumps, documentation generators, resolver overrides) // can iterate the canonical set. var All = []*Var{ - Bin, + BinaryName, DockerImage, BuildImage, BuildTag, diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go index 33f863221a7..16bc7fee2bd 100644 --- a/buildvars/buildvars_test.go +++ b/buildvars/buildvars_test.go @@ -152,7 +152,7 @@ func TestPackageDefaults(t *testing.T) { v *Var want string }{ - {Bin, "dgraph"}, + {BinaryName, "dgraph"}, {DockerImage, "dgraph/dgraph"}, {BuildImage, "ubuntu"}, {BuildTag, "24.04"}, @@ -169,13 +169,13 @@ func TestPackageDefaults(t *testing.T) { } // TestHostGopathDgraphPath_DependsOnGopath verifies the HostGopathDgraphPath -// derived Var composes GOPATH + Bin correctly, and returns empty +// derived Var composes GOPATH + BinaryName 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) }) + origDefault := BinaryName.Default() + t.Cleanup(func() { BinaryName.SetDefault(origDefault) }) - os.Unsetenv(string(Bin.Name)) + os.Unsetenv(string(BinaryName.Name)) os.Unsetenv(string(HostGopathDgraphPath.Name)) // With GOPATH set @@ -287,7 +287,7 @@ func TestExportAll(t *testing.T) { } // Spot-check a few: each should now have its default-value in env - for _, v := range []*Var{Bin, DockerImage, GoBinDgraphPath} { + for _, v := range []*Var{BinaryName, DockerImage, GoBinDgraphPath} { got := os.Getenv(v.Name) want := v.Default() if got != want { @@ -297,16 +297,16 @@ func TestExportAll(t *testing.T) { } } -// TestGoBinDgraphPath_TracksBin verifies the specific derivation -// that the user called out: when Bin changes (via env or override), +// TestGoBinDgraphPath_TracksBinaryName verifies the specific derivation +// that the user called out: when BinaryName changes (via env or override), // GoBinDgraphPath reflects that change. -func TestGoBinDgraphPath_TracksBin(t *testing.T) { +func TestGoBinDgraphPath_TracksBinaryName(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) }) + origDefault := BinaryName.Default() + t.Cleanup(func() { BinaryName.SetDefault(origDefault) }) - os.Unsetenv(string(Bin.Name)) + os.Unsetenv(string(BinaryName.Name)) os.Unsetenv(string(GoBinDgraphPath.Name)) // Override via env @@ -317,7 +317,7 @@ func TestGoBinDgraphPath_TracksBin(t *testing.T) { // Override via SetDefault os.Unsetenv("BIN") - Bin.SetDefault("dgraph-alt") + BinaryName.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/compose/compose.go b/compose/compose.go index 1d123a93994..ab98fcadf2b 100644 --- a/compose/compose.go +++ b/compose/compose.go @@ -201,7 +201,7 @@ func initService(basename string, idx, grpcPort int) service { // no data volume } - svc.Command = buildvars.Bin.Get() + svc.Command = buildvars.BinaryName.Get() if opts.LocalBin { svc.Command = buildvars.GoBinDgraphPath.Get() } diff --git a/dgraphtest/image.go b/dgraphtest/image.go index 6b86e6bdb01..55b26c560fa 100644 --- a/dgraphtest/image.go +++ b/dgraphtest/image.go @@ -81,7 +81,7 @@ func (c *LocalCluster) setupBinary() error { // 2. Copy the host-native binary (for local bulk/live commands) as // hostBinaryFileName (see load.go). - hostSrc := filepath.Join(gopath, "bin", buildvars.Bin.Get()) + hostSrc := filepath.Join(gopath, "bin", buildvars.BinaryName.Get()) hostDst := filepath.Join(c.tempBinDir, hostBinaryFileName) if err := copyFile(hostSrc, hostDst); err != nil { @@ -229,12 +229,12 @@ func buildDgraphBinary(dir, binaryDir, version string) error { } func copyBinary(fromDir, toDir, version string) error { - binaryName := buildvars.Bin.Get() + binaryName := buildvars.BinaryName.Get() if version != localVersion { binaryName = fmt.Sprintf(binaryNameFmt, version) } fromPath := filepath.Join(fromDir, binaryName) - toPath := filepath.Join(toDir, buildvars.Bin.Get()) + toPath := filepath.Join(toDir, buildvars.BinaryName.Get()) if err := copyFile(fromPath, toPath); err != nil { return errors.Wrapf(err, "error while copying binary into tempBinDir [%v], from [%v]", toPath, fromPath) } diff --git a/dgraphtest/load.go b/dgraphtest/load.go index dd500861b48..1112edb5ceb 100644 --- a/dgraphtest/load.go +++ b/dgraphtest/load.go @@ -33,12 +33,12 @@ import ( // 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 name (returned -// by buildvars.Bin.Get()) and this constant is unused. +// by buildvars.BinaryName.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 the same binary used by Docker containers -// (named by buildvars.Bin.Get(), typically "dgraph"). On non-Linux (macOS) +// (named by buildvars.BinaryName.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 { @@ -46,7 +46,7 @@ func (c *LocalCluster) HostDgraphBinaryPath() string { return filepath.Join(c.tempBinDir, name) } if runtime.GOOS == "linux" { - return filepath.Join(c.tempBinDir, buildvars.Bin.Get()) + return filepath.Join(c.tempBinDir, buildvars.BinaryName.Get()) } return filepath.Join(c.tempBinDir, hostBinaryFileName) } diff --git a/t/t.go b/t/t.go index 21045ff24bb..93458ef319c 100644 --- a/t/t.go +++ b/t/t.go @@ -204,7 +204,7 @@ func ensureGoPathLinuxBinEnvVarSet() { func ensureDgraphLinuxBinary() error { ensureGoPathLinuxBinEnvVarSet() gopathLinuxBin := os.Getenv("LINUX_GOBIN") - dgraphBin := filepath.Join(gopathLinuxBin, buildvars.Bin.Get()) + dgraphBin := filepath.Join(gopathLinuxBin, buildvars.BinaryName.Get()) if _, err := os.Stat(dgraphBin); err == nil { return nil // binary exists diff --git a/testutil/exec.go b/testutil/exec.go index f91a8f275bd..fa4c56a0194 100644 --- a/testutil/exec.go +++ b/testutil/exec.go @@ -124,7 +124,7 @@ func DgraphBinaryPath() string { gopath = build.Default.GOPATH } - return filepath.Join(gopath, "bin", buildvars.Bin.Get()) + return filepath.Join(gopath, "bin", buildvars.BinaryName.Get()) } func DetectRaceInZeros(prefix string) bool { From e39f7089587f138742e2b6a2b4ea4250359410ae Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 18:38:26 -0400 Subject: [PATCH 14/24] move FIPS-awareness surface to buildvars; collapse two-piece API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FIPS-awareness surface lived in package x as two separate symbols: x.FIPSEnabled (var, compile-time signal flipped by tag-guarded init) and x.FIPSBinary() (function, runtime signal reading DGRAPH_FIPS_BINARY). Every call site combined them as 'if x.FIPSEnabled || x.FIPSBinary()'. Two improvements in one move: 1. Relocate to package buildvars. The FIPS-awareness surface is build- time configuration, alongside BinaryName, BuildImage, BUILD_TAGS, etc. It belongs in the registry, not in package x's grab-bag of low-level helpers. 2. Collapse to a single caller-facing helper. New buildvars.FIPSEnabled() ORs the two underlying signals internally; callers don't need to know about the compile-time-vs-runtime distinction. The two-piece API was honest about implementation but pushed boilerplate to every site. Layout: buildvars/fips.go — FIPSEnabled(), untagged buildvars/fips_requirefips.go — init() flips internal flag under -tags=requirefips buildvars/fips_test.go — exercises both signals Migrated call sites: acl/jwt_algo_test.go x.FIPSEnabled || x.FIPSBinary() -> buildvars.FIPSEnabled() check_upgrade/check_upgrade_test.go same systest/integration2/acl_test.go same Removed: x/fips_base.go — FIPSEnabled var + FIPSBinary func x/fips_base_test.go — moved to buildvars/fips_test.go --- acl/jwt_algo_test.go | 17 +++++-- buildvars/fips.go | 36 ++++++++++++++ buildvars/fips_requirefips.go | 16 +++++++ buildvars/fips_test.go | 74 +++++++++++++++++++++++++++++ check_upgrade/check_upgrade_test.go | 19 ++++---- systest/integration2/acl_test.go | 15 +++--- x/fips_base.go | 42 ---------------- x/fips_base_test.go | 64 ------------------------- 8 files changed, 156 insertions(+), 127 deletions(-) create mode 100644 buildvars/fips.go create mode 100644 buildvars/fips_requirefips.go create mode 100644 buildvars/fips_test.go delete mode 100644 x/fips_base.go delete mode 100644 x/fips_base_test.go diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index 989aca28d4a..beacf6974a6 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -16,17 +16,21 @@ import ( "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" + "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/dgraphapi" "github.com/dgraph-io/dgraph/v25/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" ) 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() + // EdDSA (Ed25519) is outside Go's FIPS validation boundary; the + // FIPS-restricted dgraph binary rejects it at JWT-signing setup. + // buildvars.FIPSEnabled() returns true under either signal — this + // test binary is FIPS-tagged, or the cluster's dgraph subprocess + // reports FIPS via DGRAPH_FIPS_BINARY=1 — so a single guard covers + // both. The remaining algorithms in jwt.GetAlgorithms() are + // FIPS-approvable and run in both modes. + fipsBinary := buildvars.FIPSEnabled() for _, algo := range jwt.GetAlgorithms() { if algo == "none" || algo == "" { continue @@ -34,6 +38,9 @@ func TestACLJwtAlgo(t *testing.T) { if algo == "EdDSA" && fipsBinary { 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/buildvars/fips.go b/buildvars/fips.go new file mode 100644 index 00000000000..a1376549e59 --- /dev/null +++ b/buildvars/fips.go @@ -0,0 +1,36 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +import "os" + +// FIPSEnabled reports whether the dgraph binary in scope is running under +// FIPS 140-3 enforcement. It returns true if either: +// +// - this binary was compiled with `-tags=requirefips` (the tag-guarded +// init() in buildvars/fips_requirefips.go flips an internal flag at +// package load), or +// - the environment variable DGRAPH_FIPS_BINARY is set to "1" (the +// runtime signal a test harness uses when its own binary isn't +// FIPS-tagged but the dgraph subprocess it launches is). +// +// Test code typically calls this in a single guard: +// +// if buildvars.FIPSEnabled() { +// t.Skip("test requires features unavailable under FIPS") +// } +// +// Upstream (non-tagged) builds with the env var unset always return false; +// no behavior change. +func FIPSEnabled() bool { + return fipsEnabledCompiled || os.Getenv("DGRAPH_FIPS_BINARY") == "1" +} + +// fipsEnabledCompiled is the compile-time component of [FIPSEnabled]. +// Default false; the tag-guarded init() in buildvars/fips_requirefips.go +// sets it to true when -tags=requirefips is in effect. Read-only after +// package init. +var fipsEnabledCompiled = false diff --git a/buildvars/fips_requirefips.go b/buildvars/fips_requirefips.go new file mode 100644 index 00000000000..ea7f0103688 --- /dev/null +++ b/buildvars/fips_requirefips.go @@ -0,0 +1,16 @@ +//go:build requirefips + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +// init flips the compile-time component of [FIPSEnabled] to true whenever +// the `requirefips` build tag is in effect. Runs at package load before +// any caller's main() or test body, so [FIPSEnabled] reads consistently +// from the start. +func init() { + fipsEnabledCompiled = true +} diff --git a/buildvars/fips_test.go b/buildvars/fips_test.go new file mode 100644 index 00000000000..8c99e86edcb --- /dev/null +++ b/buildvars/fips_test.go @@ -0,0 +1,74 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +import ( + "os" + "testing" +) + +// TestFIPSEnabled_DefaultFalse confirms the upstream-pristine default: +// without -tags=requirefips and with DGRAPH_FIPS_BINARY unset, FIPSEnabled +// returns false. Under -tags=requirefips, the tag-guarded init in +// buildvars/fips_requirefips.go flips fipsEnabledCompiled to true and the +// untagged build of this test isn't compiled — so it's safe to assert +// false here unconditionally. +func TestFIPSEnabled_DefaultFalse(t *testing.T) { + saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") + if FIPSEnabled() { + t.Fatal("FIPSEnabled() must return false in non-FIPS builds with " + + "DGRAPH_FIPS_BINARY unset; got true") + } +} + +// TestFIPSEnabled_ReadsDgraphFIPSBinary confirms the runtime signal: +// FIPSEnabled() returns true only when DGRAPH_FIPS_BINARY is set to +// exactly "1". Covers the common edge cases: unset, empty, "0", "1", +// and other non-"1" values. +func TestFIPSEnabled_ReadsDgraphFIPSBinary(t *testing.T) { + saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") + 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}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.name == "unset" { + _ = os.Unsetenv("DGRAPH_FIPS_BINARY") + } else { + t.Setenv("DGRAPH_FIPS_BINARY", tc.setValue) + } + if got := FIPSEnabled(); got != tc.want { + t.Errorf("FIPSEnabled() = %v; want %v (env=%q)", + got, tc.want, tc.setValue) + } + }) + } +} + +// saveAndUnsetEnv is a small test helper that captures the current value +// of name (if any), unsets it, and registers a cleanup to restore the +// original state. Avoids leaking test-set env values across cases. +func saveAndUnsetEnv(t *testing.T, name string) { + t.Helper() + orig, had := os.LookupEnv(name) + t.Cleanup(func() { + if had { + _ = os.Setenv(name, orig) + } else { + _ = os.Unsetenv(name) + } + }) + _ = os.Unsetenv(name) +} diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index 70562b29496..88e0895200b 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -19,21 +19,22 @@ import ( "github.com/stretchr/testify/require" "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/dgraphtest" "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. +// skipIfFIPSBinary skips the current test when buildvars.FIPSEnabled() +// reports the test binary or the dgraph binary it spawns is FIPS-tagged. +// 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() { + if buildvars.FIPSEnabled() { t.Skip("upgrade-path test pins a pre-FIPS upstream SHA; skipping under FIPS build") } } diff --git a/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index 35dd3ceb6a6..76d58e34f99 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -14,6 +14,7 @@ import ( "time" "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/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" @@ -21,16 +22,16 @@ 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 +// skipIfFIPSBinary skips the current test when buildvars.FIPSEnabled() +// reports the test binary or the dgraph binary it spawns is FIPS-tagged. +// 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() { + if buildvars.FIPSEnabled() { t.Skip("upgrade-path test pins a pre-FIPS upstream version; skipping under FIPS build") } } diff --git a/x/fips_base.go b/x/fips_base.go deleted file mode 100644 index 4b82b9ebfc1..00000000000 --- a/x/fips_base.go +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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 deleted file mode 100644 index f9181039f75..00000000000 --- a/x/fips_base_test.go +++ /dev/null @@ -1,64 +0,0 @@ -/* - * 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 45e78f2d10f0961e4699b3e3642652e2403bc3ee Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 18:47:45 -0400 Subject: [PATCH 15/24] Revert "move FIPS-awareness surface to buildvars; collapse two-piece API" This reverts commit e39f7089587f138742e2b6a2b4ea4250359410ae. --- acl/jwt_algo_test.go | 17 ++----- buildvars/fips.go | 36 -------------- buildvars/fips_requirefips.go | 16 ------- buildvars/fips_test.go | 74 ----------------------------- check_upgrade/check_upgrade_test.go | 19 ++++---- systest/integration2/acl_test.go | 15 +++--- x/fips_base.go | 42 ++++++++++++++++ x/fips_base_test.go | 64 +++++++++++++++++++++++++ 8 files changed, 127 insertions(+), 156 deletions(-) delete mode 100644 buildvars/fips.go delete mode 100644 buildvars/fips_requirefips.go delete mode 100644 buildvars/fips_test.go create mode 100644 x/fips_base.go create mode 100644 x/fips_base_test.go diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index beacf6974a6..989aca28d4a 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -16,21 +16,17 @@ import ( "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" - "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/dgraphapi" "github.com/dgraph-io/dgraph/v25/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" ) func TestACLJwtAlgo(t *testing.T) { - // EdDSA (Ed25519) is outside Go's FIPS validation boundary; the - // FIPS-restricted dgraph binary rejects it at JWT-signing setup. - // buildvars.FIPSEnabled() returns true under either signal — this - // test binary is FIPS-tagged, or the cluster's dgraph subprocess - // reports FIPS via DGRAPH_FIPS_BINARY=1 — so a single guard covers - // both. The remaining algorithms in jwt.GetAlgorithms() are - // FIPS-approvable and run in both modes. - fipsBinary := buildvars.FIPSEnabled() + // 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 @@ -38,9 +34,6 @@ func TestACLJwtAlgo(t *testing.T) { if algo == "EdDSA" && fipsBinary { 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/buildvars/fips.go b/buildvars/fips.go deleted file mode 100644 index a1376549e59..00000000000 --- a/buildvars/fips.go +++ /dev/null @@ -1,36 +0,0 @@ -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -import "os" - -// FIPSEnabled reports whether the dgraph binary in scope is running under -// FIPS 140-3 enforcement. It returns true if either: -// -// - this binary was compiled with `-tags=requirefips` (the tag-guarded -// init() in buildvars/fips_requirefips.go flips an internal flag at -// package load), or -// - the environment variable DGRAPH_FIPS_BINARY is set to "1" (the -// runtime signal a test harness uses when its own binary isn't -// FIPS-tagged but the dgraph subprocess it launches is). -// -// Test code typically calls this in a single guard: -// -// if buildvars.FIPSEnabled() { -// t.Skip("test requires features unavailable under FIPS") -// } -// -// Upstream (non-tagged) builds with the env var unset always return false; -// no behavior change. -func FIPSEnabled() bool { - return fipsEnabledCompiled || os.Getenv("DGRAPH_FIPS_BINARY") == "1" -} - -// fipsEnabledCompiled is the compile-time component of [FIPSEnabled]. -// Default false; the tag-guarded init() in buildvars/fips_requirefips.go -// sets it to true when -tags=requirefips is in effect. Read-only after -// package init. -var fipsEnabledCompiled = false diff --git a/buildvars/fips_requirefips.go b/buildvars/fips_requirefips.go deleted file mode 100644 index ea7f0103688..00000000000 --- a/buildvars/fips_requirefips.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build requirefips - -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -// init flips the compile-time component of [FIPSEnabled] to true whenever -// the `requirefips` build tag is in effect. Runs at package load before -// any caller's main() or test body, so [FIPSEnabled] reads consistently -// from the start. -func init() { - fipsEnabledCompiled = true -} diff --git a/buildvars/fips_test.go b/buildvars/fips_test.go deleted file mode 100644 index 8c99e86edcb..00000000000 --- a/buildvars/fips_test.go +++ /dev/null @@ -1,74 +0,0 @@ -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -import ( - "os" - "testing" -) - -// TestFIPSEnabled_DefaultFalse confirms the upstream-pristine default: -// without -tags=requirefips and with DGRAPH_FIPS_BINARY unset, FIPSEnabled -// returns false. Under -tags=requirefips, the tag-guarded init in -// buildvars/fips_requirefips.go flips fipsEnabledCompiled to true and the -// untagged build of this test isn't compiled — so it's safe to assert -// false here unconditionally. -func TestFIPSEnabled_DefaultFalse(t *testing.T) { - saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") - if FIPSEnabled() { - t.Fatal("FIPSEnabled() must return false in non-FIPS builds with " + - "DGRAPH_FIPS_BINARY unset; got true") - } -} - -// TestFIPSEnabled_ReadsDgraphFIPSBinary confirms the runtime signal: -// FIPSEnabled() returns true only when DGRAPH_FIPS_BINARY is set to -// exactly "1". Covers the common edge cases: unset, empty, "0", "1", -// and other non-"1" values. -func TestFIPSEnabled_ReadsDgraphFIPSBinary(t *testing.T) { - saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") - 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}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if tc.name == "unset" { - _ = os.Unsetenv("DGRAPH_FIPS_BINARY") - } else { - t.Setenv("DGRAPH_FIPS_BINARY", tc.setValue) - } - if got := FIPSEnabled(); got != tc.want { - t.Errorf("FIPSEnabled() = %v; want %v (env=%q)", - got, tc.want, tc.setValue) - } - }) - } -} - -// saveAndUnsetEnv is a small test helper that captures the current value -// of name (if any), unsets it, and registers a cleanup to restore the -// original state. Avoids leaking test-set env values across cases. -func saveAndUnsetEnv(t *testing.T, name string) { - t.Helper() - orig, had := os.LookupEnv(name) - t.Cleanup(func() { - if had { - _ = os.Setenv(name, orig) - } else { - _ = os.Unsetenv(name) - } - }) - _ = os.Unsetenv(name) -} diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index 88e0895200b..70562b29496 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -19,22 +19,21 @@ import ( "github.com/stretchr/testify/require" "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/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" ) -// skipIfFIPSBinary skips the current test when buildvars.FIPSEnabled() -// reports the test binary or the dgraph binary it spawns is FIPS-tagged. -// 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. +// 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 buildvars.FIPSEnabled() { + if x.FIPSEnabled || x.FIPSBinary() { t.Skip("upgrade-path test pins a pre-FIPS upstream SHA; skipping under FIPS build") } } diff --git a/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index 76d58e34f99..35dd3ceb6a6 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -14,7 +14,6 @@ import ( "time" "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/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" @@ -22,16 +21,16 @@ import ( "github.com/stretchr/testify/require" ) -// skipIfFIPSBinary skips the current test when buildvars.FIPSEnabled() -// reports the test binary or the dgraph binary it spawns is FIPS-tagged. -// 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 +// 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 buildvars.FIPSEnabled() { + if x.FIPSEnabled || x.FIPSBinary() { t.Skip("upgrade-path test pins a pre-FIPS upstream version; skipping under FIPS build") } } 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 ce09235fbd428253ca3f93b4328a0a60bb1753f6 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 18:50:42 -0400 Subject: [PATCH 16/24] consolidate FIPS-context API in x; rename build tag requirefips -> fips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes that need to land together: 1. Keep FIPS-awareness in package x (revert the brief move to buildvars). x is the right home for the FIPS-domain helpers (TLS cipher list, JWT algorithms, RSA key size, OpenSSL provider validation) — those are TLS/JWT-domain concerns the buildvars registry deliberately doesn't pull in. The compile-time + runtime detection naturally sits next to them. 2. Collapse the two-piece x.FIPSEnabled (var) + x.FIPSBinary() (func) into a single x.FIPSEnabled() function that ORs both signals internally. Every prior call site was 'if x.FIPSEnabled || x.FIPSBinary()'; the helper hides the compile-time-vs-runtime distinction. x.FIPSBinary stays exported for the rare caller wanting the runtime-only signal. 3. Rename the build tag itself: 'requirefips' -> 'fips'. The 'require' prefix was inherited from microsoft/go's tag name; for our purposes it's noise. Internally we now own the tag name and the shorter form reads better at every -tags=fips invocation. Files: x/fips_base.go (was: var FIPSEnabled + func FIPSBinary) -> func FIPSEnabled() bool (ORs both signals) func FIPSBinary() bool (runtime-only, kept for direct access) var fipsEnabledCompiled (internal compile-time flag) x/fips_base_test.go: assertions adapted to new API. Migrated call sites: acl/jwt_algo_test.go x.FIPSEnabled || x.FIPSBinary() -> x.FIPSEnabled() check_upgrade/check_upgrade_test.go same systest/integration2/acl_test.go same Build-tag references in docs and example strings: buildvars/{buildvars,buildvars_test}.go: 'myfork requirefips' -> 'myfork fips' testutil/graphql.go: prose mention reworded. Note: the actual //go:build fips file (x/fips.go with TLS/JWT/RSA helpers and ValidateFIPSMode) lives in the fork tree, not in this OSS PR. The upstream-mergeable surface stops at x/fips_base.go's untagged primitives. --- acl/jwt_algo_test.go | 13 +++--- buildvars/buildvars.go | 2 +- buildvars/buildvars_test.go | 6 +-- check_upgrade/check_upgrade_test.go | 18 ++++---- systest/integration2/acl_test.go | 14 +++--- testutil/graphql.go | 2 +- x/fips_base.go | 58 +++++++++++++----------- x/fips_base_test.go | 69 ++++++++++++++++++----------- 8 files changed, 103 insertions(+), 79 deletions(-) diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index 989aca28d4a..7f5c269eb72 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -22,11 +22,14 @@ 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() + // EdDSA (Ed25519) is outside Go's FIPS validation boundary; the + // FIPS-restricted dgraph binary rejects it at JWT-signing setup. + // x.FIPSEnabled() returns true under either signal — this test binary + // is fips-tagged, or the cluster's dgraph subprocess reports FIPS via + // DGRAPH_FIPS_BINARY=1 — so a single guard covers both. The remaining + // algorithms in jwt.GetAlgorithms() are FIPS-approvable and run in + // both modes. + fipsBinary := x.FIPSEnabled() for _, algo := range jwt.GetAlgorithms() { if algo == "none" || algo == "" { continue diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index 259d69e57b7..cde4cff110c 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -239,7 +239,7 @@ var ( // CustomBuildTags is the list of additional Go build tags a fork or // downstream consumer appends to [BuildTags]. Upstream default: empty. // Consumers that need compile-time code paths set this to space- - // separated tag names (e.g. "myfork requirefips"). The Makefile + // separated tag names (e.g. "myfork fips"). The Makefile // concatenates CustomBuildTags onto BuildTags, so callers typically // read BuildTags to see the final tag set. CustomBuildTags = newVar("CUSTOM_BUILD_TAGS", "") diff --git a/buildvars/buildvars_test.go b/buildvars/buildvars_test.go index 16bc7fee2bd..a75bbb78dc0 100644 --- a/buildvars/buildvars_test.go +++ b/buildvars/buildvars_test.go @@ -258,10 +258,10 @@ func TestBuildTags_ComposesWithCustomBuildTags(t *testing.T) { } // Extra set → composed - CustomBuildTags.SetDefault("myfork requirefips") - if got := BuildTags.Get(); got != "jemalloc myfork requirefips" { + CustomBuildTags.SetDefault("myfork fips") + if got := BuildTags.Get(); got != "jemalloc myfork fips" { t.Errorf("with extra: BuildTags.Get() = %q, want %q", - got, "jemalloc myfork requirefips") + got, "jemalloc myfork fips") } // Env override wins (Makefile passes composed value directly) diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index 70562b29496..7bdae0ae3c6 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -24,16 +24,16 @@ 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. +// skipIfFIPSBinary skips the current test when x.FIPSEnabled() reports +// the test binary or the dgraph binary it spawns is fips-tagged. +// 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() { + if x.FIPSEnabled() { t.Skip("upgrade-path test pins a pre-FIPS upstream SHA; skipping under FIPS build") } } diff --git a/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index 35dd3ceb6a6..c831e4d8b9f 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -21,16 +21,16 @@ 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 +// skipIfFIPSBinary skips the current test when x.FIPSEnabled() reports +// the test binary or the dgraph binary it spawns is fips-tagged. +// 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() { + if x.FIPSEnabled() { t.Skip("upgrade-path test pins a pre-FIPS upstream version; skipping under FIPS build") } } diff --git a/testutil/graphql.go b/testutil/graphql.go index dad603d63a6..1370cbd9845 100644 --- a/testutil/graphql.go +++ b/testutil/graphql.go @@ -278,7 +278,7 @@ func AppendAuthInfo(schema []byte, algo, publicKeyFile string, closedByDefault b // 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) + // provider used by Chainguard go-fips / Microsoft Go FIPS-mode 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. diff --git a/x/fips_base.go b/x/fips_base.go index 4b82b9ebfc1..213817d8c8f 100644 --- a/x/fips_base.go +++ b/x/fips_base.go @@ -7,36 +7,42 @@ 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. +// FIPSEnabled reports whether the dgraph binary in scope is running under +// FIPS 140-3 enforcement. It returns true if either: // -// 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. +// - this binary was compiled with `-tags=fips` (the tag-guarded init() +// in x/fips.go flips fipsEnabledCompiled to true at package load), or +// - the runtime DGRAPH_FIPS_BINARY env var is set to "1" (the signal a +// test harness uses when its own binary isn't fips-tagged but the +// dgraph subprocess it launches is). // -// 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 code typically calls this in a single guard: // -// 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. +// if x.FIPSEnabled() { +// t.Skip("test requires features unavailable under FIPS") +// } // -// 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. +// Upstream (non-tagged) builds with the env var unset always return false; +// no behavior change. +func FIPSEnabled() bool { + return fipsEnabledCompiled || FIPSBinary() +} + +// FIPSBinary is the runtime-only component of [FIPSEnabled]. Exposed for the +// rare caller that wants to distinguish "the dgraph subprocess we're about +// to launch is FIPS" from "this test binary itself is FIPS-tagged" — most +// call sites should prefer [FIPSEnabled], which ORs both signals. +// +// Returns true when DGRAPH_FIPS_BINARY is set to exactly "1". 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. func FIPSBinary() bool { return os.Getenv("DGRAPH_FIPS_BINARY") == "1" } + +// fipsEnabledCompiled is the compile-time component of [FIPSEnabled]. +// Default false; the tag-guarded init() in x/fips.go (under //go:build fips) +// sets it to true when -tags=fips is in effect. Read-only after package init. +var fipsEnabledCompiled = false diff --git a/x/fips_base_test.go b/x/fips_base_test.go index f9181039f75..699a20ad5e8 100644 --- a/x/fips_base_test.go +++ b/x/fips_base_test.go @@ -10,19 +10,25 @@ import ( "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") +// TestFIPSEnabled_DefaultFalse confirms the upstream-pristine default: +// without -tags=fips and with DGRAPH_FIPS_BINARY unset, FIPSEnabled +// returns false. Under -tags=fips, the tag-guarded init in x/fips.go +// flips fipsEnabledCompiled to true and the untagged build of this test +// isn't compiled — so it's safe to assert false here unconditionally. +func TestFIPSEnabled_DefaultFalse(t *testing.T) { + saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") + if FIPSEnabled() { + t.Fatal("FIPSEnabled() must return false in non-FIPS builds with " + + "DGRAPH_FIPS_BINARY unset; 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) { +// TestFIPSEnabled_ReadsDgraphFIPSBinary confirms the runtime signal: +// FIPSEnabled() returns true when DGRAPH_FIPS_BINARY is set to exactly +// "1", regardless of the compile-time tag. Covers the common edge cases: +// unset, empty, "0", "1", and other non-"1" values. +func TestFIPSEnabled_ReadsDgraphFIPSBinary(t *testing.T) { + saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") cases := []struct { name string setValue string // empty string means unset @@ -35,30 +41,39 @@ func TestFIPSBinaryEnv(t *testing.T) { {"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) + t.Setenv("DGRAPH_FIPS_BINARY", tc.setValue) + } + if got := FIPSEnabled(); got != tc.want { + t.Errorf("FIPSEnabled() = %v; want %v (env=%q)", + got, tc.want, tc.setValue) } - got := FIPSBinary() - if got != tc.want { - t.Errorf("FIPSBinary() = %v; want %v (env=%q)", got, tc.want, tc.setValue) + // FIPSBinary should agree with the runtime-only side + // regardless of compile-time tag — verify directly. + if got := FIPSBinary(); got != tc.want { + t.Errorf("FIPSBinary() = %v; want %v (env=%q)", + got, tc.want, tc.setValue) } }) } } + +// saveAndUnsetEnv captures the current value of name (if any), unsets it, +// and registers a cleanup to restore the original state. Avoids leaking +// test-set env values across cases. +func saveAndUnsetEnv(t *testing.T, name string) { + t.Helper() + orig, had := os.LookupEnv(name) + t.Cleanup(func() { + if had { + _ = os.Setenv(name, orig) + } else { + _ = os.Unsetenv(name) + } + }) + _ = os.Unsetenv(name) +} From c4a08305426abe6602e8516830c29209c6a90726 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 19:01:37 -0400 Subject: [PATCH 17/24] move FIPS-tag flag to buildvars; drop runtime DGRAPH_FIPS_BINARY signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FIPS-awareness surface previously had two parts: - x.FIPSEnabled (compile-time signal flipped by tag-guarded init) - x.FIPSBinary() (runtime signal reading DGRAPH_FIPS_BINARY env var) The runtime signal was never actually independent. The fork ships 'istari' and 'fips' build tags coupled in CUSTOM_BUILD_TAGS; if the test binary is built with one, it's built with both. There's no realistic config where the test binary is non-FIPS but the dgraph subprocess is. The runtime env var was redundant boilerplate. This commit: 1. Moves the flag to package buildvars as plain 'var FIPSEnabled bool'. The tag-guarded init() in buildvars/fips_on.go (//go:build fips) flips it to true. A simple bool, not a function, not a *Var — it's a compile-time fact, not an env-driven config. 2. Drops x.FIPSBinary() and the DGRAPH_FIPS_BINARY env var entirely. Call sites read buildvars.FIPSEnabled directly. 3. Renames helpers: skipIfFIPSBinary -> skipIfFIPS (the binary-vs-test distinction was never meaningful given coupled tags). Layout: buildvars/fips.go — var FIPSEnabled (default false) buildvars/fips_on.go — //go:build fips; init() flips to true buildvars/fips_test.go — //go:build !fips; default-false test buildvars/fips_on_test.go — //go:build fips; flipped-to-true test Migrated call sites: acl/jwt_algo_test.go x.FIPSEnabled || x.FIPSBinary() -> buildvars.FIPSEnabled check_upgrade/check_upgrade_test.go same systest/integration2/acl_test.go same Removed: x/fips_base.go (FIPSEnabled var + FIPSBinary func) x/fips_base_test.go (tests for the dropped surface; default-false test moved to buildvars/fips_test.go) --- acl/jwt_algo_test.go | 12 ++--- buildvars/fips.go | 20 ++++++++ buildvars/fips_on.go | 15 ++++++ buildvars/fips_on_test.go | 19 +++++++ buildvars/fips_test.go | 20 ++++++++ check_upgrade/check_upgrade_test.go | 22 ++++---- systest/integration2/acl_test.go | 20 ++++---- x/fips_base.go | 48 ------------------ x/fips_base_test.go | 79 ----------------------------- 9 files changed, 98 insertions(+), 157 deletions(-) create mode 100644 buildvars/fips.go create mode 100644 buildvars/fips_on.go create mode 100644 buildvars/fips_on_test.go create mode 100644 buildvars/fips_test.go delete mode 100644 x/fips_base.go delete mode 100644 x/fips_base_test.go diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index 7f5c269eb72..f2a614f3d57 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -16,6 +16,7 @@ import ( "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" + "github.com/dgraph-io/dgraph/v25/buildvars" "github.com/dgraph-io/dgraph/v25/dgraphapi" "github.com/dgraph-io/dgraph/v25/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" @@ -24,17 +25,14 @@ import ( func TestACLJwtAlgo(t *testing.T) { // EdDSA (Ed25519) is outside Go's FIPS validation boundary; the // FIPS-restricted dgraph binary rejects it at JWT-signing setup. - // x.FIPSEnabled() returns true under either signal — this test binary - // is fips-tagged, or the cluster's dgraph subprocess reports FIPS via - // DGRAPH_FIPS_BINARY=1 — so a single guard covers both. The remaining - // algorithms in jwt.GetAlgorithms() are FIPS-approvable and run in - // both modes. - fipsBinary := x.FIPSEnabled() + // Skip the EdDSA iteration under -tags=fips. The remaining algorithms + // in jwt.GetAlgorithms() are FIPS-approvable and run in both modes. + fipsBuild := buildvars.FIPSEnabled for _, algo := range jwt.GetAlgorithms() { if algo == "none" || algo == "" { continue } - if algo == "EdDSA" && fipsBinary { + if algo == "EdDSA" && fipsBuild { continue } diff --git a/buildvars/fips.go b/buildvars/fips.go new file mode 100644 index 00000000000..9dccecd95b5 --- /dev/null +++ b/buildvars/fips.go @@ -0,0 +1,20 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +// FIPSEnabled reports whether this binary was built with -tags=fips and +// is therefore restricted to FIPS 140-3 validated cryptography. Default +// false; the //go:build fips init() in buildvars/fips_on.go sets it to +// true at package load before any caller's main() or test body runs. +// +// Test code uses it to skip cases the FIPS-tagged binary cannot satisfy: +// +// if buildvars.FIPSEnabled { +// t.Skip("test requires features unavailable under FIPS") +// } +// +// Read-only after package init. +var FIPSEnabled = false diff --git a/buildvars/fips_on.go b/buildvars/fips_on.go new file mode 100644 index 00000000000..44f6dd98b5e --- /dev/null +++ b/buildvars/fips_on.go @@ -0,0 +1,15 @@ +//go:build fips + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +// init flips [FIPSEnabled] to true under -tags=fips. Runs at package +// load before any caller's main() or test body, so reads are consistent +// from the start. +func init() { + FIPSEnabled = true +} diff --git a/buildvars/fips_on_test.go b/buildvars/fips_on_test.go new file mode 100644 index 00000000000..339d2dec669 --- /dev/null +++ b/buildvars/fips_on_test.go @@ -0,0 +1,19 @@ +//go:build fips + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +import "testing" + +// TestFIPSEnabled_TrueUnderFIPSTag confirms the //go:build fips init in +// buildvars/fips_on.go flipped FIPSEnabled to true. The companion +// fips_test.go (//go:build !fips) asserts the default-false case. +func TestFIPSEnabled_TrueUnderFIPSTag(t *testing.T) { + if !FIPSEnabled { + t.Fatal("FIPSEnabled must be true under -tags=fips; got false") + } +} diff --git a/buildvars/fips_test.go b/buildvars/fips_test.go new file mode 100644 index 00000000000..1150e5774a2 --- /dev/null +++ b/buildvars/fips_test.go @@ -0,0 +1,20 @@ +//go:build !fips + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +import "testing" + +// TestFIPSEnabled_DefaultFalse confirms the default: without -tags=fips, +// FIPSEnabled is false. Tagged !fips because the //go:build fips init +// in buildvars/fips_on.go flips the flag; the FIPS-on assertion lives +// in the corresponding buildvars/fips_on_test.go (also //go:build fips). +func TestFIPSEnabled_DefaultFalse(t *testing.T) { + if FIPSEnabled { + t.Fatal("FIPSEnabled must be false in non-FIPS builds; got true") + } +} diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index 7bdae0ae3c6..a6dedeaa745 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -19,27 +19,25 @@ import ( "github.com/stretchr/testify/require" "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/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" ) -// skipIfFIPSBinary skips the current test when x.FIPSEnabled() reports -// the test binary or the dgraph binary it spawns is fips-tagged. -// 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() { +// skipIfFIPS skips the current test under -tags=fips. 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. Semantically valid in non-FIPS builds. +func skipIfFIPS(t *testing.T) { + if buildvars.FIPSEnabled { t.Skip("upgrade-path test pins a pre-FIPS upstream SHA; skipping under FIPS build") } } func TestCheckUpgrade(t *testing.T) { - skipIfFIPSBinary(t) + skipIfFIPS(t) conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1). WithACL(time.Hour).WithVersion("57aa5c4ac") c, err := dgraphtest.NewLocalCluster(conf) @@ -111,7 +109,7 @@ func TestCheckUpgrade(t *testing.T) { } func TestQueryDuplicateNodes(t *testing.T) { - skipIfFIPSBinary(t) + skipIfFIPS(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/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index c831e4d8b9f..668326cf477 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -14,6 +14,7 @@ import ( "time" "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/dgraphtest" "github.com/dgraph-io/dgraph/v25/x" @@ -21,16 +22,13 @@ import ( "github.com/stretchr/testify/require" ) -// skipIfFIPSBinary skips the current test when x.FIPSEnabled() reports -// the test binary or the dgraph binary it spawns is fips-tagged. -// 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() { +// skipIfFIPS skips the current test under -tags=fips. 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. Semantically valid in non-FIPS builds. +func skipIfFIPS(t *testing.T) { + if buildvars.FIPSEnabled { t.Skip("upgrade-path test pins a pre-FIPS upstream version; skipping under FIPS build") } } @@ -48,7 +46,7 @@ type Received struct { } func testDuplicateUserUpgradeStrat(t *testing.T, strat dgraphtest.UpgradeStrategy) { - skipIfFIPSBinary(t) + skipIfFIPS(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/x/fips_base.go b/x/fips_base.go deleted file mode 100644 index 213817d8c8f..00000000000 --- a/x/fips_base.go +++ /dev/null @@ -1,48 +0,0 @@ -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package x - -import "os" - -// FIPSEnabled reports whether the dgraph binary in scope is running under -// FIPS 140-3 enforcement. It returns true if either: -// -// - this binary was compiled with `-tags=fips` (the tag-guarded init() -// in x/fips.go flips fipsEnabledCompiled to true at package load), or -// - the runtime DGRAPH_FIPS_BINARY env var is set to "1" (the signal a -// test harness uses when its own binary isn't fips-tagged but the -// dgraph subprocess it launches is). -// -// Test code typically calls this in a single guard: -// -// if x.FIPSEnabled() { -// t.Skip("test requires features unavailable under FIPS") -// } -// -// Upstream (non-tagged) builds with the env var unset always return false; -// no behavior change. -func FIPSEnabled() bool { - return fipsEnabledCompiled || FIPSBinary() -} - -// FIPSBinary is the runtime-only component of [FIPSEnabled]. Exposed for the -// rare caller that wants to distinguish "the dgraph subprocess we're about -// to launch is FIPS" from "this test binary itself is FIPS-tagged" — most -// call sites should prefer [FIPSEnabled], which ORs both signals. -// -// Returns true when DGRAPH_FIPS_BINARY is set to exactly "1". 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. -func FIPSBinary() bool { - return os.Getenv("DGRAPH_FIPS_BINARY") == "1" -} - -// fipsEnabledCompiled is the compile-time component of [FIPSEnabled]. -// Default false; the tag-guarded init() in x/fips.go (under //go:build fips) -// sets it to true when -tags=fips is in effect. Read-only after package init. -var fipsEnabledCompiled = false diff --git a/x/fips_base_test.go b/x/fips_base_test.go deleted file mode 100644 index 699a20ad5e8..00000000000 --- a/x/fips_base_test.go +++ /dev/null @@ -1,79 +0,0 @@ -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package x - -import ( - "os" - "testing" -) - -// TestFIPSEnabled_DefaultFalse confirms the upstream-pristine default: -// without -tags=fips and with DGRAPH_FIPS_BINARY unset, FIPSEnabled -// returns false. Under -tags=fips, the tag-guarded init in x/fips.go -// flips fipsEnabledCompiled to true and the untagged build of this test -// isn't compiled — so it's safe to assert false here unconditionally. -func TestFIPSEnabled_DefaultFalse(t *testing.T) { - saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") - if FIPSEnabled() { - t.Fatal("FIPSEnabled() must return false in non-FIPS builds with " + - "DGRAPH_FIPS_BINARY unset; got true") - } -} - -// TestFIPSEnabled_ReadsDgraphFIPSBinary confirms the runtime signal: -// FIPSEnabled() returns true when DGRAPH_FIPS_BINARY is set to exactly -// "1", regardless of the compile-time tag. Covers the common edge cases: -// unset, empty, "0", "1", and other non-"1" values. -func TestFIPSEnabled_ReadsDgraphFIPSBinary(t *testing.T) { - saveAndUnsetEnv(t, "DGRAPH_FIPS_BINARY") - 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}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if tc.name == "unset" { - _ = os.Unsetenv("DGRAPH_FIPS_BINARY") - } else { - t.Setenv("DGRAPH_FIPS_BINARY", tc.setValue) - } - if got := FIPSEnabled(); got != tc.want { - t.Errorf("FIPSEnabled() = %v; want %v (env=%q)", - got, tc.want, tc.setValue) - } - // FIPSBinary should agree with the runtime-only side - // regardless of compile-time tag — verify directly. - if got := FIPSBinary(); got != tc.want { - t.Errorf("FIPSBinary() = %v; want %v (env=%q)", - got, tc.want, tc.setValue) - } - }) - } -} - -// saveAndUnsetEnv captures the current value of name (if any), unsets it, -// and registers a cleanup to restore the original state. Avoids leaking -// test-set env values across cases. -func saveAndUnsetEnv(t *testing.T, name string) { - t.Helper() - orig, had := os.LookupEnv(name) - t.Cleanup(func() { - if had { - _ = os.Setenv(name, orig) - } else { - _ = os.Unsetenv(name) - } - }) - _ = os.Unsetenv(name) -} From e355fbcdd49e4f6f5059b6fed7003c3b154de59f Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 19:05:01 -0400 Subject: [PATCH 18/24] buildvars FIPS: harmonize file naming to match Go stdlib boring convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the 4-file split (fips.go untagged + fips_on.go tagged + 2 tests) with a 2-file pair following crypto/internal/boring's pattern: fips.go (//go:build fips) -> var FIPSEnabled = true nofips.go (//go:build !fips) -> var FIPSEnabled = false fips_test.go (//go:build fips) -> TestFIPSEnabled_TrueUnderFIPSTag nofips_test.go (//go:build !fips) -> TestFIPSEnabled_FalseUnderNoFIPSTag Each compiled build sees exactly one declaration; no init() needed. The 'no' prefix for the complement file matches Go stdlib's crypto/internal/boring/notboring.go style and reads better than the prior _on/_off pair. No call-site changes — buildvars.FIPSEnabled is still a plain bool read. --- buildvars/fips.go | 12 +++++++----- buildvars/fips_on.go | 15 --------------- buildvars/fips_on_test.go | 19 ------------------- buildvars/fips_test.go | 15 +++++++-------- buildvars/nofips.go | 13 +++++++++++++ buildvars/nofips_test.go | 19 +++++++++++++++++++ 6 files changed, 46 insertions(+), 47 deletions(-) delete mode 100644 buildvars/fips_on.go delete mode 100644 buildvars/fips_on_test.go create mode 100644 buildvars/nofips.go create mode 100644 buildvars/nofips_test.go diff --git a/buildvars/fips.go b/buildvars/fips.go index 9dccecd95b5..4436aea67c1 100644 --- a/buildvars/fips.go +++ b/buildvars/fips.go @@ -1,3 +1,5 @@ +//go:build fips + /* * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. * SPDX-License-Identifier: Apache-2.0 @@ -6,9 +8,7 @@ package buildvars // FIPSEnabled reports whether this binary was built with -tags=fips and -// is therefore restricted to FIPS 140-3 validated cryptography. Default -// false; the //go:build fips init() in buildvars/fips_on.go sets it to -// true at package load before any caller's main() or test body runs. +// is therefore restricted to FIPS 140-3 validated cryptography. // // Test code uses it to skip cases the FIPS-tagged binary cannot satisfy: // @@ -16,5 +16,7 @@ package buildvars // t.Skip("test requires features unavailable under FIPS") // } // -// Read-only after package init. -var FIPSEnabled = false +// Companion declaration in buildvars/nofips.go (//go:build !fips) sets +// the var to false; only one of the two files compiles into any given +// build, matching Go's stdlib boring/notboring convention. +var FIPSEnabled = true diff --git a/buildvars/fips_on.go b/buildvars/fips_on.go deleted file mode 100644 index 44f6dd98b5e..00000000000 --- a/buildvars/fips_on.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build fips - -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -// init flips [FIPSEnabled] to true under -tags=fips. Runs at package -// load before any caller's main() or test body, so reads are consistent -// from the start. -func init() { - FIPSEnabled = true -} diff --git a/buildvars/fips_on_test.go b/buildvars/fips_on_test.go deleted file mode 100644 index 339d2dec669..00000000000 --- a/buildvars/fips_on_test.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build fips - -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -import "testing" - -// TestFIPSEnabled_TrueUnderFIPSTag confirms the //go:build fips init in -// buildvars/fips_on.go flipped FIPSEnabled to true. The companion -// fips_test.go (//go:build !fips) asserts the default-false case. -func TestFIPSEnabled_TrueUnderFIPSTag(t *testing.T) { - if !FIPSEnabled { - t.Fatal("FIPSEnabled must be true under -tags=fips; got false") - } -} diff --git a/buildvars/fips_test.go b/buildvars/fips_test.go index 1150e5774a2..9009f2ce58c 100644 --- a/buildvars/fips_test.go +++ b/buildvars/fips_test.go @@ -1,4 +1,4 @@ -//go:build !fips +//go:build fips /* * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. @@ -9,12 +9,11 @@ package buildvars import "testing" -// TestFIPSEnabled_DefaultFalse confirms the default: without -tags=fips, -// FIPSEnabled is false. Tagged !fips because the //go:build fips init -// in buildvars/fips_on.go flips the flag; the FIPS-on assertion lives -// in the corresponding buildvars/fips_on_test.go (also //go:build fips). -func TestFIPSEnabled_DefaultFalse(t *testing.T) { - if FIPSEnabled { - t.Fatal("FIPSEnabled must be false in non-FIPS builds; got true") +// TestFIPSEnabled_TrueUnderFIPSTag confirms FIPSEnabled is true under +// -tags=fips (set by buildvars/fips.go's declaration). Companion test +// in buildvars/nofips_test.go covers the default-false case. +func TestFIPSEnabled_TrueUnderFIPSTag(t *testing.T) { + if !FIPSEnabled { + t.Fatal("FIPSEnabled must be true under -tags=fips; got false") } } diff --git a/buildvars/nofips.go b/buildvars/nofips.go new file mode 100644 index 00000000000..33487615967 --- /dev/null +++ b/buildvars/nofips.go @@ -0,0 +1,13 @@ +//go:build !fips + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +// FIPSEnabled is false in non-FIPS builds (the upstream-pristine +// default). Companion declaration in buildvars/fips.go (//go:build fips) +// sets it to true; see that file for the docstring. +var FIPSEnabled = false diff --git a/buildvars/nofips_test.go b/buildvars/nofips_test.go new file mode 100644 index 00000000000..465fc72c63d --- /dev/null +++ b/buildvars/nofips_test.go @@ -0,0 +1,19 @@ +//go:build !fips + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package buildvars + +import "testing" + +// TestFIPSEnabled_FalseUnderNoFIPSTag confirms the upstream-pristine +// default: without -tags=fips, FIPSEnabled is false. Companion test in +// buildvars/fips_test.go covers the FIPS-on case. +func TestFIPSEnabled_FalseUnderNoFIPSTag(t *testing.T) { + if FIPSEnabled { + t.Fatal("FIPSEnabled must be false in non-FIPS builds; got true") + } +} From 509045a8a9f7d6be02a2671e895370ec4517fd20 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 19:07:05 -0400 Subject: [PATCH 19/24] buildvars FIPS: drop tag-guarded variant, leave only the false default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit (e355fbcdd) wrongly added a //go:build fips file declaring 'var FIPSEnabled = true' to the upstream PR. The upstream project never wants FIPS enabled — it's a downstream fork concern. The upstream PR's job is to expose the extension point (a public bool that forks can flip via a tag-guarded sibling), not to ship the flipping half itself. Now: a single untagged buildvars/fips.go declaring FIPSEnabled = false, plus a test asserting that default. Forks that need the FIPS-on side ship their own tag-guarded sibling file (//go:build fips, or whatever tag they prefer) that reassigns the var via init(). --- buildvars/fips.go | 15 +++++++-------- buildvars/fips_test.go | 14 ++++++-------- buildvars/nofips.go | 13 ------------- buildvars/nofips_test.go | 19 ------------------- 4 files changed, 13 insertions(+), 48 deletions(-) delete mode 100644 buildvars/nofips.go delete mode 100644 buildvars/nofips_test.go diff --git a/buildvars/fips.go b/buildvars/fips.go index 4436aea67c1..b1c15d25a4c 100644 --- a/buildvars/fips.go +++ b/buildvars/fips.go @@ -1,5 +1,3 @@ -//go:build fips - /* * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. * SPDX-License-Identifier: Apache-2.0 @@ -7,8 +5,11 @@ package buildvars -// FIPSEnabled reports whether this binary was built with -tags=fips and -// is therefore restricted to FIPS 140-3 validated cryptography. +// FIPSEnabled reports whether this binary was built with FIPS 140-3 +// enforcement and is therefore restricted to validated cryptography. +// Default false. A downstream fork ships a tag-guarded sibling file in +// this package (e.g. //go:build fips) that flips it to true at package +// load before any caller's main() or test body runs. // // Test code uses it to skip cases the FIPS-tagged binary cannot satisfy: // @@ -16,7 +17,5 @@ package buildvars // t.Skip("test requires features unavailable under FIPS") // } // -// Companion declaration in buildvars/nofips.go (//go:build !fips) sets -// the var to false; only one of the two files compiles into any given -// build, matching Go's stdlib boring/notboring convention. -var FIPSEnabled = true +// Read-only after package init. +var FIPSEnabled = false diff --git a/buildvars/fips_test.go b/buildvars/fips_test.go index 9009f2ce58c..482f96de4b9 100644 --- a/buildvars/fips_test.go +++ b/buildvars/fips_test.go @@ -1,5 +1,3 @@ -//go:build fips - /* * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. * SPDX-License-Identifier: Apache-2.0 @@ -9,11 +7,11 @@ package buildvars import "testing" -// TestFIPSEnabled_TrueUnderFIPSTag confirms FIPSEnabled is true under -// -tags=fips (set by buildvars/fips.go's declaration). Companion test -// in buildvars/nofips_test.go covers the default-false case. -func TestFIPSEnabled_TrueUnderFIPSTag(t *testing.T) { - if !FIPSEnabled { - t.Fatal("FIPSEnabled must be true under -tags=fips; got false") +// TestFIPSEnabled_DefaultFalse confirms the upstream-pristine default: +// FIPSEnabled is false. Forks that flip it via a tag-guarded sibling +// file run their own assertion under that tag. +func TestFIPSEnabled_DefaultFalse(t *testing.T) { + if FIPSEnabled { + t.Fatal("FIPSEnabled must be false in upstream builds; got true") } } diff --git a/buildvars/nofips.go b/buildvars/nofips.go deleted file mode 100644 index 33487615967..00000000000 --- a/buildvars/nofips.go +++ /dev/null @@ -1,13 +0,0 @@ -//go:build !fips - -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -// FIPSEnabled is false in non-FIPS builds (the upstream-pristine -// default). Companion declaration in buildvars/fips.go (//go:build fips) -// sets it to true; see that file for the docstring. -var FIPSEnabled = false diff --git a/buildvars/nofips_test.go b/buildvars/nofips_test.go deleted file mode 100644 index 465fc72c63d..00000000000 --- a/buildvars/nofips_test.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build !fips - -/* - * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package buildvars - -import "testing" - -// TestFIPSEnabled_FalseUnderNoFIPSTag confirms the upstream-pristine -// default: without -tags=fips, FIPSEnabled is false. Companion test in -// buildvars/fips_test.go covers the FIPS-on case. -func TestFIPSEnabled_FalseUnderNoFIPSTag(t *testing.T) { - if FIPSEnabled { - t.Fatal("FIPSEnabled must be false in non-FIPS builds; got true") - } -} From 23d9f3fa82d799f193283d213a5f2d720958d7a1 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 19:19:34 -0400 Subject: [PATCH 20/24] buildvars FIPS: keep docstring agnostic about fork's tag name Three OSS PR sites referenced '-tags=fips' in comments. The OSS PR is upstream-mergeable; it shouldn't presume any particular fork's build- tag name. Switch to 'when buildvars.FIPSEnabled is true' (the visible behavior callers can reason about) and the buildvars/fips.go docstring to a generic 'tag-guarded init() under whatever tag the fork uses' form. Sites: acl/jwt_algo_test.go under -tags=fips -> when buildvars.FIPSEnabled is true check_upgrade/check_upgrade_test.go same systest/integration2/acl_test.go same buildvars/fips.go '//go:build fips' example -> generic tag-agnostic phrasing --- acl/jwt_algo_test.go | 2 +- buildvars/fips.go | 7 ++++--- check_upgrade/check_upgrade_test.go | 2 +- systest/integration2/acl_test.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index f2a614f3d57..d33afa74f3a 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -25,7 +25,7 @@ import ( func TestACLJwtAlgo(t *testing.T) { // EdDSA (Ed25519) is outside Go's FIPS validation boundary; the // FIPS-restricted dgraph binary rejects it at JWT-signing setup. - // Skip the EdDSA iteration under -tags=fips. The remaining algorithms + // Skip the EdDSA iteration when buildvars.FIPSEnabled is true. The remaining algorithms // in jwt.GetAlgorithms() are FIPS-approvable and run in both modes. fipsBuild := buildvars.FIPSEnabled for _, algo := range jwt.GetAlgorithms() { diff --git a/buildvars/fips.go b/buildvars/fips.go index b1c15d25a4c..689643be5ef 100644 --- a/buildvars/fips.go +++ b/buildvars/fips.go @@ -7,9 +7,10 @@ package buildvars // FIPSEnabled reports whether this binary was built with FIPS 140-3 // enforcement and is therefore restricted to validated cryptography. -// Default false. A downstream fork ships a tag-guarded sibling file in -// this package (e.g. //go:build fips) that flips it to true at package -// load before any caller's main() or test body runs. +// Default false. A downstream fork that runs FIPS-enforced builds flips +// this var to true from a tag-guarded init() (the tag is whatever the +// fork uses to gate its FIPS-enforcing code paths) before any caller's +// main() or test body runs. // // Test code uses it to skip cases the FIPS-tagged binary cannot satisfy: // diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index a6dedeaa745..ab7a22ef04b 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -25,7 +25,7 @@ import ( "github.com/dgraph-io/dgraph/v25/x" ) -// skipIfFIPS skips the current test under -tags=fips. Upgrade-path +// skipIfFIPS skips the current test when buildvars.FIPSEnabled is true. 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 diff --git a/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index 668326cf477..cac7407d7c0 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -22,7 +22,7 @@ import ( "github.com/stretchr/testify/require" ) -// skipIfFIPS skips the current test under -tags=fips. Upgrade-path +// skipIfFIPS skips the current test when buildvars.FIPSEnabled is true. 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 From 87e445d773b05fe7027ebf907165b9e759f238a5 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 22:33:42 -0400 Subject: [PATCH 21/24] t/Makefile: explain DBIN recipe-local alias Carry forward the explanatory comment from the fork PR. DBIN exists because BIN is a common shell variable name; rebinding it via := inside a recipe would leak to subsequent commands and subprocesses. A function/recipe-local alias is bounded-scope, semantics-equivalent. --- t/Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/Makefile b/t/Makefile index 01df2c50644..44c155a5eca 100644 --- a/t/Makefile +++ b/t/Makefile @@ -32,6 +32,12 @@ check-deps: check-deps-go check-deps-docker check-deps-gotestsum check-deps-ack .PHONY: check check: check-deps @echo "LINUX_GOBIN=$(LINUX_GOBIN)" + @# DBIN is a recipe-local alias for "${BIN:-dgraph}" — BIN with the + @# upstream-pristine default applied. The alias keeps the rest of the + @# recipe's path expressions short and parallels the same pattern in + @# compose/run.sh. We don't rebind $$BIN itself because BIN is a + @# common shell variable name; using a fork-local alias avoids any + @# accidental leakage into commands the recipe later invokes. @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); \ From 7916c1b7a25ae043cc405ca3ac6ca125c8c59a73 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 22:36:35 -0400 Subject: [PATCH 22/24] buildvars: drop unjustified mutex; correct the docstring Var had an internal sync.RWMutex on the defaultValue/defaulter pair, guarding a race that doesn't occur. SetDefault is documented and used exclusively during package init(): the buildvars package's own init sets the upstream defaults, then a tag-guarded fork init() runs after (via the import-ordering of istari/gopkg/env). All of that is single- threaded by Go's init contract; user code, including the goroutines that read via Get(), starts after init completes. The mutex didn't add safety; the docstring claim 'guarded by an internal mutex; all reads are consistent' was technically true but implied a concurrency story that isn't actually exercised. Drop the mutex, drop the import, and rewrite the docstring to say what's actually going on: 'SetDefault is expected to run during package init before any goroutines exist; no synchronization is performed because no concurrent reader/writer scenario is reachable in practice.' Default() docstring now states the real callers (tests + diagnostics) rather than just diagnostics. SetDefault docstring drops the misleading 'safe to call concurrently' claim. --- buildvars/buildvars.go | 50 +++++++++++++++--------------------------- 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index cde4cff110c..b2bb8adb0a1 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -37,7 +37,6 @@ import ( "os/exec" "runtime" "strings" - "sync" ) // shellOutput runs cmd with args, captures stdout, and returns it stripped @@ -61,16 +60,17 @@ func shellOutput(cmd string, args ...string) string { // [BinaryName]) 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. +// from their own init() without affecting call sites. SetDefault is +// expected to run during package init() before any goroutines exist; no +// synchronization is performed because no concurrent reader/writer +// scenario is reachable in practice. 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. + // literal (non-computed) default. Used iff defaulter is nil. defaultValue string // defaulter computes the default at Get() time. Used for derived @@ -78,8 +78,6 @@ type Var struct { // 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 @@ -107,42 +105,30 @@ 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 + if v.defaultValue != "" || v.defaulter == nil { + return v.defaultValue } - return defaulter() + return v.defaulter() } // Default returns the currently-registered default (ignoring any env -// override). For derived Vars this triggers the computation. Exposed -// primarily for diagnostic tooling that needs to log what the default -// would be (independent of what any env override would produce). +// override). For derived Vars this triggers the computation. Exposed for +// tests that mutate via [Var.SetDefault] and need to capture the prior +// value to restore later, and for diagnostic tooling that wants 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 - literalSet := literal != "" || v.defaulter == nil - defaulter := v.defaulter - v.mu.RUnlock() - if literalSet { - return literal + if v.defaultValue != "" || v.defaulter == nil { + return v.defaultValue } - return defaulter() + return v.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. +// default at the new literal. Intended to run once during package init() +// before any goroutines exist; no synchronization is performed. func (v *Var) SetDefault(value string) { - v.mu.Lock() - defer v.mu.Unlock() v.defaultValue = value v.defaulter = nil } From 1656f209b7e488a70c5712945903e86525efaf53 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 22:37:52 -0400 Subject: [PATCH 23/24] buildvars: replace 'iff' jargon with plain explanation of the two-mode invariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment on Var.defaultValue said 'Used iff defaulter is nil', which read as logic-textbook shorthand and was also slightly wrong (it described one direction of the relationship; the actual invariant is that defaultValue and defaulter are mutually exclusive storage for the default — exactly one supplies the answer at any time). Spell it out: defaultValue is the literal-default storage, defaulter is the computed-default storage, SetDefault freezes computed → literal. --- buildvars/buildvars.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index b2bb8adb0a1..dabd88e544f 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -69,14 +69,19 @@ type Var struct { // 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. Used iff defaulter is nil. + // defaultValue is the registered literal fall-through when no env + // override is set. Mutually exclusive with defaulter: if defaulter + // is non-nil it computes the default and defaultValue is empty; + // if defaulter is nil, defaultValue holds the answer (possibly the + // empty string, which is a valid default for vars like + // ComposeBuildDir). 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, when non-nil, 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). [Var.SetDefault] + // freezes the result by clearing defaulter and writing the literal + // to defaultValue. defaulter func() string } From 9199611f12c2426157b33b4b0d9027dd840ae812 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Mon, 4 May 2026 22:47:54 -0400 Subject: [PATCH 24/24] copyedit: tighten comments and docstrings (Strunk pass) Apply Elements-of-Style rules to the prose this PR added or modified. Focus on the heavy-comment files: buildvars package docs, hook declarations across compose/dgraphtest/t/testutil, hook-call comments in dgraphtest/local_cluster.go, FIPS-aware skip helpers in acl/jwt_algo_test.go and check_upgrade/check_upgrade_test.go and systest/integration2/acl_test.go, plus the misc one-off comments in dgraph/Makefile, t/Makefile, contrib/Dockerfile, dgraphtest/load.go, and graphql/e2e/auth/auth_test.go. Specific changes: - Active voice over passive where natural (Rule 10). - Positive form over negated (Rule 11): 'are unavailable' for 'isn't available', 'lacks' for 'does not have', etc. - Concrete over abstract (Rule 12): name the actual value or pattern in the example rather than 'various' or 'some'. - Drop needless words (Rule 13): cut hedging (essentially, basically, typically), pleonasms (actually, really, very), 'in order to' -> 'to', 'owing to the fact that' -> 'because', and 'is a function which' / 'is initialized to' constructions. - Em-dash discipline: at most one rhetorical em-dash per sentence; replace pile-on parentheticals with semicolons or split sentences. - Keep technical content unchanged. No code edits, only prose. 17 files; +277/-268 (net +9 from line wrapping). --- acl/jwt_algo_test.go | 7 +- buildvars/buildvars.go | 238 ++++++++++++++-------------- buildvars/cmd/buildvars/main.go | 18 +-- buildvars/fips.go | 10 +- check_upgrade/check_upgrade_test.go | 11 +- compose/compose.go | 14 +- compose/hooks.go | 9 +- contrib/Dockerfile | 10 +- dgraph/Makefile | 13 +- dgraphtest/hooks.go | 45 +++--- dgraphtest/load.go | 17 +- dgraphtest/local_cluster.go | 50 +++--- graphql/e2e/auth/auth_test.go | 16 +- systest/integration2/acl_test.go | 11 +- t/Makefile | 13 +- t/hooks.go | 23 ++- testutil/hooks.go | 40 ++--- 17 files changed, 277 insertions(+), 268 deletions(-) diff --git a/acl/jwt_algo_test.go b/acl/jwt_algo_test.go index d33afa74f3a..db5955ed652 100644 --- a/acl/jwt_algo_test.go +++ b/acl/jwt_algo_test.go @@ -23,10 +23,11 @@ import ( ) func TestACLJwtAlgo(t *testing.T) { - // EdDSA (Ed25519) is outside Go's FIPS validation boundary; the + // EdDSA (Ed25519) lies outside Go's FIPS validation boundary; the // FIPS-restricted dgraph binary rejects it at JWT-signing setup. - // Skip the EdDSA iteration when buildvars.FIPSEnabled is true. The remaining algorithms - // in jwt.GetAlgorithms() are FIPS-approvable and run in both modes. + // Skip the EdDSA iteration when buildvars.FIPSEnabled is true. The + // remaining algorithms in jwt.GetAlgorithms() are FIPS-approvable + // and run in both modes. fipsBuild := buildvars.FIPSEnabled for _, algo := range jwt.GetAlgorithms() { if algo == "none" || algo == "" { diff --git a/buildvars/buildvars.go b/buildvars/buildvars.go index dabd88e544f..fa065995eb0 100644 --- a/buildvars/buildvars.go +++ b/buildvars/buildvars.go @@ -4,14 +4,15 @@ */ // 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. +// the dgraph build: binary name, Docker image tags, build-toolchain refs, +// and derived paths such as /gobin/. It replaces ad-hoc os.Getenv +// calls 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/). +// Each Var carries its env-var name and a defaulter: either a literal +// string or a function that computes the default at Get() time. The +// function form supports Vars whose default depends on other Vars, such +// as /gobin/. // // Usage: // @@ -27,9 +28,9 @@ // Derived Vars automatically pick up the override because they recompute // at Get() time. // -// 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. +// The package depends on no private-fork tree. A vendor or downstream +// that removes all fork-specific files sees only the upstream defaults +// declared here. package buildvars import ( @@ -40,9 +41,9 @@ import ( ) // 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. +// of trailing whitespace. Returns empty on error: for example, when the +// command is missing (git absent) or the working directory lacks 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 { @@ -54,58 +55,58 @@ func shellOutput(cmd string, args ...string) string { // 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. +// forms are exclusive; one is always nil. // -// Instances are pointers; call sites reference exported constants (e.g. -// [BinaryName]) and resolve values via the [Var.Get] method. +// Instances are pointers. Call sites reference exported constants such as +// [BinaryName] and resolve values via [Var.Get]. // // Literal defaults are mutable via [Var.SetDefault] so forks can override -// from their own init() without affecting call sites. SetDefault is -// expected to run during package init() before any goroutines exist; no -// synchronization is performed because no concurrent reader/writer -// scenario is reachable in practice. +// from their own init() without affecting call sites. SetDefault must run +// during package init() before any goroutines exist; the type performs no +// synchronization because no concurrent reader/writer scenario is +// reachable in practice. 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 literal fall-through when no env - // override is set. Mutually exclusive with defaulter: if defaulter - // is non-nil it computes the default and defaultValue is empty; - // if defaulter is nil, defaultValue holds the answer (possibly the - // empty string, which is a valid default for vars like - // ComposeBuildDir). + // override is set. Mutually exclusive with defaulter: when + // defaulter is non-nil, it computes the default and defaultValue + // is empty; when defaulter is nil, defaultValue holds the answer + // (possibly the empty string, which is a valid default for vars + // like ComposeBuildDir). defaultValue string // defaulter, when non-nil, 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). [Var.SetDefault] - // freezes the result by clearing defaulter and writing the literal - // to defaultValue. + // Used for derived Vars whose fall-through depends on other Vars, + // such as a path built from another Var's current value. + // [Var.SetDefault] freezes the result by clearing defaulter and + // writing the literal to defaultValue. defaulter func() string } // 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. +// own declarations; unexported because call sites must not create new +// Vars outside 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/" + BinaryName.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. +// depends on other Vars, such as GoBinDgraphPath = "/gobin/" + BinaryName.Get(). +// The function runs on every Get() that falls through and should be +// cheap. [Var.SetDefault] can still 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. +// Get returns the value of the environment variable when set and +// non-empty; otherwise the currently registered or computed default. +// Call sites prefer Get 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 @@ -116,12 +117,11 @@ func (v *Var) Get() string { return v.defaulter() } -// Default returns the currently-registered default (ignoring any env -// override). For derived Vars this triggers the computation. Exposed for -// tests that mutate via [Var.SetDefault] and need to capture the prior -// value to restore later, and for diagnostic tooling that wants to log -// what the default would be (independent of what any env override would -// produce). +// Default returns the currently registered default and ignores any env +// override. For derived Vars it triggers the computation. Exposed for +// tests that mutate via [Var.SetDefault] and must capture the prior +// value to restore later, and for diagnostic tooling that logs the +// default independent of any env override. func (v *Var) Default() string { if v.defaultValue != "" || v.defaulter == nil { return v.defaultValue @@ -130,17 +130,17 @@ func (v *Var) Default() string { } // 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. Intended to run once during package init() -// before any goroutines exist; no synchronization is performed. +// For derived Vars it also clears the defaulter function, freezing the +// default at the new literal. Run once during package init() before any +// goroutines exist; the type performs no synchronization. func (v *Var) SetDefault(value string) { 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 +// 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. // @@ -154,7 +154,7 @@ func (v *Var) Export() error { // 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 +// points that must materialize the entire buildvars registry into the // process environment before spawning subprocesses. func ExportAll() error { for _, v := range All { @@ -165,30 +165,31 @@ func ExportAll() error { return nil } -// The canonical set of build-configuration vars. Each constant is -// initialized with the upstream OSS default value. Forks override via +// The canonical set of build-configuration vars. Each constant +// initializes with the upstream OSS default value. Forks override via // the SetDefault method from their own init(). var ( - // BinaryName is the name of the dgraph binary — both at build time - // (what `go build -o` writes, matching upstream's $(BIN) in + // BinaryName is the name of the dgraph binary 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. "myfork-dgraph") via env or SetDefault. + // Forks rename via env or SetDefault, e.g. "myfork-dgraph". // - // The env-var name is kept as BIN for backward compatibility with - // shell scripts, Makefiles, and CI configs that reference $(BIN) / - // ${BIN} directly. The Go identifier is BinaryName because Go + // The env-var name remains BIN for backward compatibility with + // shell scripts, Makefiles, and CI configs that reference $(BIN) + // or ${BIN} directly. The Go identifier is BinaryName because Go // callers benefit from the more descriptive symbol. BinaryName = 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". + // the Makefile local-image and 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 is the toolchain image for the Dockerfile build stage. + // Upstream uses an OS base image; forks may substitute a hardened + // toolchain such as Chainguard go-fips. Paired with BuildTag. BuildImage = newVar("BUILD_IMAGE", "ubuntu") // BuildTag is the tag for BuildImage. Upstream default: "24.04". @@ -201,22 +202,22 @@ var ( // 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. + // ComposeBuildDir 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("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 - // well-known libjemalloc tag) with any tags a fork adds via - // [CustomBuildTags]. Both the Makefile and this Var use the same + // `go build` via -tags. The upstream default composes "jemalloc" + // (the well-known libjemalloc tag) with any tags a fork adds via + // [CustomBuildTags]. The Makefile and this Var share 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 + // "jemalloc" is meaningful only on Linux and macOS; the Makefile + // omits it on other hosts. Go code reading BuildTags treats the + // value as informational and makes no OS-aware decisions. The // Makefile handles OS gating for the actual `go build` invocation. BuildTags = newDerivedVar("BUILD_TAGS", func() string { base := "jemalloc" @@ -229,20 +230,20 @@ var ( // CustomBuildTags is the list of additional Go build tags a fork or // downstream consumer appends to [BuildTags]. Upstream default: empty. - // Consumers that need compile-time code paths set this to space- - // separated tag names (e.g. "myfork fips"). The Makefile - // concatenates CustomBuildTags onto BuildTags, so callers typically - // read BuildTags to see the final tag set. + // Consumers needing compile-time code paths set this to + // space-separated tag names, e.g. "myfork fips". The Makefile + // concatenates CustomBuildTags onto BuildTags, so callers read + // BuildTags to see the final tag set. CustomBuildTags = newVar("CUSTOM_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/buildvars` so the - // buildvars CLI picks up a fork's init-time default overrides). + // invocations from Makefile helper recipes. For example, `build-env` + // runs `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 - // (typically just a fork's primary tag). BuildTags adds "jemalloc" + // 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 via an init()-time @@ -250,42 +251,43 @@ var ( 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"). + // 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 is the short git commit SHA of the build source, baked + // into the binary's version output via -ldflags -X. The default + // computes at Get() time from `git rev-parse --short HEAD` and is + // empty when 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 is the commit timestamp of the build source, baked + // into the binary via -ldflags -X. The default computes from + // `git log -1 --format=%ci`. 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 is the git branch of the build source, baked in via + // -ldflags -X. The default computes from + // `git rev-parse --abbrev-ref HEAD`. 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 is a human-readable name for the release family, + // baked into the binary via -ldflags -X. Upstream default: "dgraph"; + // CI overrides per release. 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 is the full version string baked into the binary, + // e.g. "v25.3.0-5-gabc1234". The default computes from + // `git describe --always --tags` and is empty when git is + // unavailable. CI release builds override via env to the exact + // release tag. BuildVersion = newDerivedVar("BUILD_VERSION", func() string { return shellOutput("git", "describe", "--always", "--tags") }) @@ -293,9 +295,9 @@ var ( // 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. + // directory so the cross-compiled Linux binary does not collide + // with the Mach-O host binary. The default computes at Get() time + // from $GOPATH + runtime.GOARCH. LinuxGobin = newDerivedVar("LINUX_GOBIN", func() string { gopath := os.Getenv("GOPATH") if gopath == "" { @@ -304,21 +306,21 @@ var ( 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 [BinaryName] at - // Get() time so the path automatically tracks the binary name. + // GoBinDgraphPath is the in-container path to the dgraph binary: + // the bind-mounted /gobin directory joined with the binary name. + // Used by jepsen's --local-binary default and by the compose + // generator for services configured with LocalBin. Derived from + // [BinaryName] at Get() time so the path tracks the binary name. GoBinDgraphPath = newDerivedVar("GOBIN_DGRAPH_PATH", func() string { return "/gobin/" + BinaryName.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 - // [BinaryName] at Get() time. + // dgraph binary, $GOPATH/bin/. Used by testutil and + // t/t.go to locate the binary on the runner host outside + // containers. Returns empty when $GOPATH is unset so callers can + // fall back to build.Default.GOPATH or their own resolution. + // Derived from [BinaryName] at Get() time. HostGopathDgraphPath = newDerivedVar("HOST_GOPATH_DGRAPH_PATH", func() string { gopath := os.Getenv("GOPATH") if gopath == "" { @@ -328,9 +330,9 @@ var ( }) ) -// All lists every defined Var in declaration order. Exposed so that -// tooling (diagnostic dumps, documentation generators, resolver overrides) -// can iterate the canonical set. +// All lists every defined Var in declaration order. Exposed so tooling +// such as diagnostic dumps, documentation generators, and resolver +// overrides can iterate the canonical set. var All = []*Var{ BinaryName, DockerImage, diff --git a/buildvars/cmd/buildvars/main.go b/buildvars/cmd/buildvars/main.go index 2108bd21f29..27bea0793e9 100644 --- a/buildvars/cmd/buildvars/main.go +++ b/buildvars/cmd/buildvars/main.go @@ -3,9 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -// 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. +// 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. // // Formats: // @@ -25,10 +25,10 @@ // 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 -// Make characters in values would break, but all current buildvars values -// are simple strings). +// The shell format single-quotes values and escapes embedded single +// quotes. The make format emits values raw; newlines and special Make +// characters would break it, but all current buildvars values are +// simple strings. package main import ( @@ -40,7 +40,7 @@ import ( "github.com/dgraph-io/dgraph/v25/buildvars" ) -// shellQuote wraps s in single quotes, escaping any embedded single +// shellQuote wraps s in single quotes and escapes any embedded single // quotes. Matches POSIX shell conventions for literal string quoting: // 'foo'"'"'bar' unambiguously represents foo'bar. func shellQuote(s string) string { @@ -58,7 +58,7 @@ func main() { } case "make": // `:=` rather than `?=` so this command's value overrides any - // ambient Make default. Callers that want fallback-only shape + // ambient Make default. Callers wanting 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()) diff --git a/buildvars/fips.go b/buildvars/fips.go index 689643be5ef..0dd914df2c1 100644 --- a/buildvars/fips.go +++ b/buildvars/fips.go @@ -6,11 +6,11 @@ package buildvars // FIPSEnabled reports whether this binary was built with FIPS 140-3 -// enforcement and is therefore restricted to validated cryptography. -// Default false. A downstream fork that runs FIPS-enforced builds flips -// this var to true from a tag-guarded init() (the tag is whatever the -// fork uses to gate its FIPS-enforcing code paths) before any caller's -// main() or test body runs. +// enforcement and is restricted to validated cryptography. Default +// false. A downstream fork running FIPS-enforced builds sets this var +// to true from a tag-guarded init(), where the tag is whatever the fork +// uses to gate its FIPS-enforcing code paths. The init() runs before +// any caller's main() or test body. // // Test code uses it to skip cases the FIPS-tagged binary cannot satisfy: // diff --git a/check_upgrade/check_upgrade_test.go b/check_upgrade/check_upgrade_test.go index ab7a22ef04b..358c4d10a4f 100644 --- a/check_upgrade/check_upgrade_test.go +++ b/check_upgrade/check_upgrade_test.go @@ -25,11 +25,12 @@ import ( "github.com/dgraph-io/dgraph/v25/x" ) -// skipIfFIPS skips the current test when buildvars.FIPSEnabled is true. 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. Semantically valid in non-FIPS builds. +// skipIfFIPS skips the current test when buildvars.FIPSEnabled is true. +// Upgrade-path tests pin a specific upstream SHA for the "old" binary; +// that commit predates any FIPS-enforcing toolchain, so building it +// under a FIPS configuration fails outright or produces a binary that +// refuses to start. The test remains semantically valid in non-FIPS +// builds. func skipIfFIPS(t *testing.T) { if buildvars.FIPSEnabled { t.Skip("upgrade-path test pins a pre-FIPS upstream SHA; skipping under FIPS build") diff --git a/compose/compose.go b/compose/compose.go index ab98fcadf2b..1f19740bd3e 100644 --- a/compose/compose.go +++ b/compose/compose.go @@ -155,13 +155,13 @@ 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) - // EmitUser is a public hook (compose/hooks.go); the upstream default - // returns "" so the generated compose service inherits the image's - // own USER directive. Downstream consumers that run dgraph as a - // non-root user under compose can override the hook to return a - // "uid:gid" string (e.g. "${UID:-65532}"), which is then emitted as - // the service's `user:` field. Bind-mounted host paths must be - // readable by that uid for the container to start cleanly. + // EmitUser is a public hook in compose/hooks.go. The upstream + // default returns "" so the generated compose service inherits the + // image's own USER directive. Downstream consumers that run dgraph + // as a non-root user under compose override the hook to return a + // "uid:gid" string such as "${UID:-65532}", which is emitted as the + // service's `user:` field. Bind-mounted host paths must be readable + // by that uid for the container to start cleanly. if u := EmitUser(); u != "" { svc.User = u } diff --git a/compose/hooks.go b/compose/hooks.go index e23cf8d7929..f68c2f6a58e 100644 --- a/compose/hooks.go +++ b/compose/hooks.go @@ -9,10 +9,11 @@ package main // 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. +// compose services. Default: empty string, emitting no user override +// and matching 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/contrib/Dockerfile b/contrib/Dockerfile index e8ca1683c74..95a177527ea 100644 --- a/contrib/Dockerfile +++ b/contrib/Dockerfile @@ -15,11 +15,11 @@ RUN rm -rf /var/lib/apt/lists/* # remove /var/lib/apt/lists/* to reduce image size. # see: https://docs.docker.com/develop/develop-images/dockerfile_best-practices # -# Retry the install once on transient apt-get errors (most commonly the -# `File has unexpected size... Mirror sync in progress?` race against -# archive.ubuntu.com mid-publish, which surfaces frequently in CI). The -# rm + sleep between attempts gives the mirror time to settle and forces -# a fresh package-list fetch. +# Retry the install once on transient apt-get errors. The most common +# is the `File has unexpected size... Mirror sync in progress?` race +# against archive.ubuntu.com mid-publish, which surfaces frequently in +# CI. The rm + sleep between attempts gives the mirror time to settle +# and forces a fresh package-list fetch. RUN set -eu; \ install_pkgs() { \ apt-get update && apt-get install -y --no-install-recommends \ diff --git a/dgraph/Makefile b/dgraph/Makefile index f3570a3287c..1c2b103c5d7 100644 --- a/dgraph/Makefile +++ b/dgraph/Makefile @@ -51,9 +51,9 @@ ifneq ($(strip $(BUILD_RACE)),) endif # jemalloc stuff -# 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). +# Check common package-manager install paths: /usr/local/lib (make +# install from source), /usr/lib (apt-get libjemalloc-dev), and +# /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.1/jemalloc-5.3.1.tar.bz2" @@ -89,9 +89,10 @@ install: jemalloc echo "Old SHA256:" `sha256sum $(INSTALL_TARGET) 2>/dev/null | cut -c-64` ; \ fi @go mod tidy - @# 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. + @# Use 'go build -o' rather than 'go install': 'go install' honors + @# the package's default name (always "dgraph"), but we want to + @# honor $(BIN) so renames such as a 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 \ diff --git a/dgraphtest/hooks.go b/dgraphtest/hooks.go index 90ab96e1961..90b4a23ab63 100644 --- a/dgraphtest/hooks.go +++ b/dgraphtest/hooks.go @@ -8,61 +8,62 @@ 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; +// harness. See testutil/hooks.go for the full convention. Each hook +// initializes 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 +// specific UID/GID. Default: no-op, so the 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 +// os.MkdirTemp so a container's nonroot user can 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. +// the default 0600 so a container's nonroot user can 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 +// LocalCluster.GeneratePlugins. When 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. +// arg and skips the default native-go-build path. When handled=false, +// the caller falls through to the default in-process build. Default: +// returns ("", false, nil) and delegates 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 +// - tempBinDir: the cluster's per-test tempdir (host-side). The hook +// creates a plugins/ subdirectory there and bind-mounts it 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/…). +// paths the alpha container sees 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 +// LocalCluster.setupBinary performs for the Linux branch. When // handled=true, the caller returns err without executing the upstream -// single-binary Linux path. Default: (false, nil) — delegate to +// single-binary Linux path. Default: (false, nil) and delegates 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. +// tempBinDir. Default: empty string, which selects the upstream default +// path 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) {} diff --git a/dgraphtest/load.go b/dgraphtest/load.go index 1112edb5ceb..ef2aca4513c 100644 --- a/dgraphtest/load.go +++ b/dgraphtest/load.go @@ -31,16 +31,17 @@ import ( ) // 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 name (returned -// by buildvars.BinaryName.Get()) and this constant is unused. +// dgraph binary when it differs from the container binary (non-Linux). +// On Linux the host and container binaries share the same file name, +// returned by buildvars.BinaryName.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 the same binary used by Docker containers -// (named by buildvars.BinaryName.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. +// HostDgraphBinaryPath returns the path to the host-OS-native dgraph +// binary in tempBinDir. On Linux this is the same binary used by Docker +// containers, named by buildvars.BinaryName.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) diff --git a/dgraphtest/local_cluster.go b/dgraphtest/local_cluster.go index 0018cb604fe..8fc65602b0a 100644 --- a/dgraphtest/local_cluster.go +++ b/dgraphtest/local_cluster.go @@ -118,11 +118,11 @@ func (c *LocalCluster) init() error { if err != nil { return errors.Wrap(err, "error while creating tempBinDir") } - // WidenTempDirPerms is a public hook (dgraphtest/hooks.go) — default - // is no-op. Downstream consumers running dgraph as a non-root user - // inside compose-test containers override it to widen perms on - // host-side temp dirs that are bind-mounted into containers, so the - // in-container uid can read/write those paths. + // WidenTempDirPerms is a public hook in dgraphtest/hooks.go; the + // default is no-op. Downstream consumers running dgraph as a + // non-root user inside compose-test containers override it to + // widen perms on host-side temp dirs bind-mounted into containers, + // so the in-container uid can read and write those paths. if err := WidenTempDirPerms(c.tempBinDir); err != nil { return err } @@ -318,12 +318,12 @@ func (c *LocalCluster) createContainer(dc dnode) (string, error) { } cconf := &container.Config{Cmd: cmd, Image: image, WorkingDir: dc.workingDir(), ExposedPorts: dc.ports()} - // ApplyContainerUser is a public hook (dgraphtest/hooks.go) — default - // is no-op. Downstream consumers that run dgraph as a non-root user - // inside the test container override it to set cconf.User to the - // host's uid:gid, so files the container writes are readable on the - // host (and bind-mounted host paths are readable inside the - // container). + // ApplyContainerUser is a public hook in dgraphtest/hooks.go; the + // default is no-op. Downstream consumers that run dgraph as a + // non-root user inside the test container override it to set + // cconf.User to the host's uid:gid, so files the container writes + // are readable on the host and bind-mounted host paths are readable + // inside the container. ApplyContainerUser(cconf) hconf := &container.HostConfig{Mounts: mts, PublishAllPorts: true, PortBindings: dc.bindings(c.conf.portOffset)} networkConfig := &network.NetworkingConfig{ @@ -1324,12 +1324,12 @@ func (c *LocalCluster) inspectContainer(containerID string) (string, error) { } func (c *LocalCluster) setupSecrets() error { - // WidenSecretFilePerms is a public hook (dgraphtest/hooks.go) — - // default is no-op. Secret files are written with mode 0600 - // (owner-only) which is correct upstream. Downstream consumers - // running the dgraph container as a non-root user that differs from - // the host owner override the hook to widen perms (e.g. add group- - // or world-read) so the in-container uid can read the bind-mounted + // WidenSecretFilePerms is a public hook in dgraphtest/hooks.go; the + // default is no-op. Secret files use mode 0600 (owner-only), which + // is correct upstream. Downstream consumers running the dgraph + // container as a non-root user that differs from the host owner + // override the hook to widen perms — for example, adding group- or + // world-read — so the in-container uid can read the bind-mounted // secret files. if c.conf.encryption { // use this key because some of the data is already encrypted using this key. @@ -1407,14 +1407,14 @@ func runOpennssl(args ...string) error { } func (c *LocalCluster) GeneratePlugins(raceEnabled bool) error { - // GeneratePlugins is a public hook (dgraphtest/hooks.go) — the - // upstream default returns (nil, false, nil), so the fallback path - // below (host-side `go build -buildmode=plugin`) runs. Downstream - // consumers whose toolchain isn't directly invokable on the host - // (e.g. forks pinning a Docker-only build image) override the hook - // to compile plugins inside that image and return their .so paths; - // when handled=true the override's outcome wins and the host-side - // fallback is skipped. + // GeneratePlugins is a public hook in dgraphtest/hooks.go. The + // upstream default returns (nil, false, nil), so the host-side + // `go build -buildmode=plugin` fallback below runs. Downstream + // consumers whose toolchain is not directly invokable on the host + // — for example, forks pinning a Docker-only build image — + // override the hook to compile plugins inside that image and + // return their .so paths. When handled=true the override's outcome + // wins and the host-side fallback is skipped. if tokenizers, handled, err := GeneratePlugins(raceEnabled, c.tempBinDir, baseRepoDir); handled { if err == nil { c.customTokenizers = tokenizers diff --git a/graphql/e2e/auth/auth_test.go b/graphql/e2e/auth/auth_test.go index 834ed6a9410..c997eb24e3e 100644 --- a/graphql/e2e/auth/auth_test.go +++ b/graphql/e2e/auth/auth_test.go @@ -615,15 +615,15 @@ func TestQueryWithStandardClaims(t *testing.T) { if metaInfo.Algo == "RS256" { t.Skip() } - // JWTs below are pre-signed with the same secret used by the schema's - // VerificationKey ("secret-long-enough"). Both the secret and these - // fixture JWTs were re-signed when the test secrets were widened to - // 14+ bytes — the minimum HMAC key length NIST SP 800-131A requires + // The JWTs below are pre-signed with the secret the schema's + // VerificationKey uses ("secret-long-enough"). Both the secret and + // these fixture JWTs were re-signed when the test secrets grew to + // 14+ bytes: the minimum HMAC key length NIST SP 800-131A requires // and that some FIPS-validated crypto providers enforce at - // EVP_MAC_init. Shorter keys are valid under stock HMAC but rejected - // at signing time on FIPS builds. The signatures here were verified - // against the previous secret before being re-signed against the new - // one, so claim payloads are unchanged. + // EVP_MAC_init. Shorter keys are valid under stock HMAC but + // rejected at signing time on FIPS builds. The signatures here + // were verified against the previous secret before re-signing + // against the new one, so claim payloads are unchanged. testCases := []TestCase{ { query: ` diff --git a/systest/integration2/acl_test.go b/systest/integration2/acl_test.go index cac7407d7c0..27f0a500d43 100644 --- a/systest/integration2/acl_test.go +++ b/systest/integration2/acl_test.go @@ -22,11 +22,12 @@ import ( "github.com/stretchr/testify/require" ) -// skipIfFIPS skips the current test when buildvars.FIPSEnabled is true. 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. Semantically valid in non-FIPS builds. +// skipIfFIPS skips the current test when buildvars.FIPSEnabled is true. +// Upgrade-path tests pin a pre-FIPS upstream version for the "old" +// binary; that version predates any FIPS-enforcing toolchain, so +// building it under a FIPS configuration fails outright or produces a +// binary that refuses to start. The test remains semantically valid in +// non-FIPS builds. func skipIfFIPS(t *testing.T) { if buildvars.FIPSEnabled { t.Skip("upgrade-path test pins a pre-FIPS upstream version; skipping under FIPS build") diff --git a/t/Makefile b/t/Makefile index 44c155a5eca..5f7c8be1ac1 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)" - @# DBIN is a recipe-local alias for "${BIN:-dgraph}" — BIN with the - @# upstream-pristine default applied. The alias keeps the rest of the - @# recipe's path expressions short and parallels the same pattern in - @# compose/run.sh. We don't rebind $$BIN itself because BIN is a - @# common shell variable name; using a fork-local alias avoids any - @# accidental leakage into commands the recipe later invokes. + @# DBIN is a recipe-local alias for "${BIN:-dgraph}": BIN with the + @# upstream-pristine default applied. The alias keeps the rest of + @# the recipe's path expressions short and parallels the same + @# pattern in compose/run.sh. The recipe does not rebind $$BIN + @# itself because BIN is a common shell variable name; a + @# fork-local alias avoids accidental leakage into commands the + @# recipe later invokes. @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); \ diff --git a/t/hooks.go b/t/hooks.go index 1d897abad6f..7958d519fdc 100644 --- a/t/hooks.go +++ b/t/hooks.go @@ -10,19 +10,18 @@ package main // 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. +// nil. Forks may inject 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. +// for the given compose-file path. The baseDir argument is the repo +// root (t's --base flag), passed explicitly so the hook is pure and +// shares no package state with t/t.go. Default: returns "-f " +// unchanged, matching upstream behavior where the pristine compose file +// passes straight through. // -// A fork that runs a compose-file generator may override this to rewrite +// A fork running 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 @@ -32,9 +31,9 @@ var ComposeFileArgs = defaultComposeFileArgs 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. +// defaultComposeFileArgs ignores baseDir; it exists in the signature so +// overrides that need it (to resolve 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} } diff --git a/testutil/hooks.go b/testutil/hooks.go index 9540f45c8c6..72a6a3c3bed 100644 --- a/testutil/hooks.go +++ b/testutil/hooks.go @@ -5,40 +5,40 @@ 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. +// This file declares public extensibility hooks for testutil. Each hook +// is a package-level function-var initialized to an unexported default +// that implements stock upstream behavior. A private fork or downstream +// consumer reassigns the public var from its own init() to customize +// 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 +// Call sites such as StartAlphas in bulk.go invoke the hooks as 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. +// The defaults are deliberately unexported so external packages must +// reach the behavior through the public var: 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). +// ComposeArgs returns the docker-compose file selector args: +// "-f " plus any optional overlays such as --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. +// through a generator overlay and set --project-directory so 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. +// environment of docker-compose subprocesses. Default: nil. Forks +// inject 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. +// inside their toolchain image so the resulting .so matches the alpha +// container's ABI. var BuildPlugins = defaultBuildPlugins func defaultComposeArgs(path string) []string {