From 1cc230b43a9b6d625a1b6464e25f8824d3a71f31 Mon Sep 17 00:00:00 2001 From: Nick Vigilante Date: Wed, 8 Jul 2026 11:39:45 -0400 Subject: [PATCH] refactor: extract docgen env prep into a shared package (#26827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What `clidocgen` and the new `configdocgen` (coder/coder#26824) both carried a byte-identical `prepareEnv()` that unsets `CODER_*` and pins `CLIDOCGEN_*` / `TMPDIR` so generated docs don't embed the generating host's home directory. This extracts it to `scripts/docgenenv.Prepare()` and migrates `clidocgen`. ## Why Duplication flagged during review of #26824. `configdocgen` adopts the shared helper in that PR, removing its copy. ## Risk Behavior-preserving: regenerating the CLI reference (`make docs/reference/cli/index.md`) yields no diff, and `make pre-commit` passes (`lint/go`, `lint/ts`, `build`). A focused unit test pins the `Prepare()` contract, and `_test.go` files are excluded from `CLIDOCGEN_INPUTS` so test edits don't mark the generated docs stale.
CI status — blocked by an unrelated main breakage (#24993) All red checks on this PR are inherited from `main`, not caused by these changes. This PR touches only `Makefile` and `scripts/{clidocgen,docgenenv}`; it does not touch Helm. `main` went red at `d0f68cb9b0` ("feat: add listenerset", #24993, merged ~18:26 UTC). The committed `helm/coder/tests/testdata/listenerset*.golden` files don't match what `helm template` renders, so: - **`gen`** regenerates those goldens, and the unstaged-files check fails. - **`test-go-pg` (ubuntu-latest, pg-17) and `test-go-race-pg`** fail only on `TestRenderChart/{coder,default}/listenerset[_redirect]` (golden mismatch; the test prints "Run with -update to update golden files"). The same `test-go-pg` job passes on macOS and Windows, where the Helm render test is skipped, and `scripts/docgenenv` reports `ok` on the failing runners. Base commit `14a61041d9` was green; `main` is red from `d0f68cb9b0` onward. These checks clear once `main` is fixed and this branch is updated. `fmt`, `lint`, `Storybook`, `check-build`, and `test-e2e` are green.
--- 🤖 Opened by Coder Agents on behalf of @nickvigilante. --------- Co-authored-by: Cian Johnston --- Makefile | 1 + scripts/clidocgen/main.go | 30 ++------------------------ scripts/docgenenv/docgenenv.go | 33 +++++++++++++++++++++++++++++ scripts/docgenenv/docgenenv_test.go | 26 +++++++++++++++++++++++ 4 files changed, 62 insertions(+), 28 deletions(-) create mode 100644 scripts/docgenenv/docgenenv.go create mode 100644 scripts/docgenenv/docgenenv_test.go diff --git a/Makefile b/Makefile index 6abc18ee56..d39020b5f0 100644 --- a/Makefile +++ b/Makefile @@ -101,6 +101,7 @@ CLIDOC_SRC_FILES := \ CLIDOCGEN_INPUTS := \ $(wildcard scripts/clidocgen/*.go) \ + $(filter-out %_test.go,$(wildcard scripts/docgenenv/*.go)) \ scripts/clidocgen/command.tpl \ $(CLIDOC_SRC_FILES) diff --git a/scripts/clidocgen/main.go b/scripts/clidocgen/main.go index 47998fca17..9550308cef 100644 --- a/scripts/clidocgen/main.go +++ b/scripts/clidocgen/main.go @@ -5,10 +5,10 @@ import ( "os" "path/filepath" "sort" - "strings" "github.com/coder/coder/v2/enterprise/cli" "github.com/coder/coder/v2/scripts/atomicwrite" + "github.com/coder/coder/v2/scripts/docgenenv" "github.com/coder/flog" "github.com/coder/serpent" ) @@ -29,32 +29,6 @@ type manifest struct { Routes []route `json:"routes,omitempty"` } -func prepareEnv() { - // Unset CODER_ environment variables - for _, env := range os.Environ() { - if strings.HasPrefix(env, "CODER_") { - split := strings.SplitN(env, "=", 2) - if err := os.Unsetenv(split[0]); err != nil { - panic(err) - } - } - } - - // Override default OS values to ensure the same generated results. - err := os.Setenv("CLIDOCGEN_CACHE_DIRECTORY", "~/.cache") - if err != nil { - panic(err) - } - err = os.Setenv("CLIDOCGEN_CONFIG_DIRECTORY", "~/.config/coderv2") - if err != nil { - panic(err) - } - err = os.Setenv("TMPDIR", "/tmp") - if err != nil { - panic(err) - } -} - func deleteEmptyDirs(dir string) error { return filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -79,7 +53,7 @@ func deleteEmptyDirs(dir string) error { } func main() { - prepareEnv() + docgenenv.Prepare() workdir, err := os.Getwd() if err != nil { diff --git a/scripts/docgenenv/docgenenv.go b/scripts/docgenenv/docgenenv.go new file mode 100644 index 0000000000..587e1543a9 --- /dev/null +++ b/scripts/docgenenv/docgenenv.go @@ -0,0 +1,33 @@ +// Package docgenenv normalizes the process environment so documentation +// generators produce host-independent output. +package docgenenv + +import ( + "os" + "strings" +) + +// Prepare clears CODER_* variables and pins the cache, config, and temp +// directories so generated docs don't embed the generating host's home +// directory. +func Prepare() { + for _, env := range os.Environ() { + if !strings.HasPrefix(env, "CODER_") { + continue + } + name, _, _ := strings.Cut(env, "=") + if err := os.Unsetenv(name); err != nil { + panic(err) + } + } + + mustSetenv("CLIDOCGEN_CACHE_DIRECTORY", "~/.cache") + mustSetenv("CLIDOCGEN_CONFIG_DIRECTORY", "~/.config/coderv2") + mustSetenv("TMPDIR", "/tmp") +} + +func mustSetenv(key, value string) { + if err := os.Setenv(key, value); err != nil { + panic(err) + } +} diff --git a/scripts/docgenenv/docgenenv_test.go b/scripts/docgenenv/docgenenv_test.go new file mode 100644 index 0000000000..0fb0a803f4 --- /dev/null +++ b/scripts/docgenenv/docgenenv_test.go @@ -0,0 +1,26 @@ +package docgenenv_test + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/scripts/docgenenv" +) + +//nolint:paralleltest // Prepare mutates the process environment. +func TestPrepare(t *testing.T) { + t.Setenv("CODER_ACCESS_URL", "https://example.com") + t.Setenv("CLIDOCGEN_CACHE_DIRECTORY", "") + t.Setenv("CLIDOCGEN_CONFIG_DIRECTORY", "") + t.Setenv("TMPDIR", "") + + docgenenv.Prepare() + + _, ok := os.LookupEnv("CODER_ACCESS_URL") + require.False(t, ok, "CODER_ prefixed variables should be cleared") + require.Equal(t, "~/.cache", os.Getenv("CLIDOCGEN_CACHE_DIRECTORY")) + require.Equal(t, "~/.config/coderv2", os.Getenv("CLIDOCGEN_CONFIG_DIRECTORY")) + require.Equal(t, "/tmp", os.Getenv("TMPDIR")) +}