From 0363617b4d03bec33b0f7861500d044432c5684c Mon Sep 17 00:00:00 2001 From: Arturo Peroni Date: Fri, 11 Sep 2026 10:13:02 +0200 Subject: [PATCH 1/2] feat(env): alias-first TRACEBLOC_ENV stage var, normalize toward RFC-0076 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normalize this repo's owned stage-selecting env var toward the RFC-0076 canon (backend#3391), alias-first — read new-or-old, never break an existing deployment. TRACEBLOC_ENV is now the canonical name; the legacy CLIENT_ENV is read as a fallback (remove_by: 2026-12-31). - api.ResolveEnv: --env flag, then $TRACEBLOC_ENV, then legacy $CLIENT_ENV, then prod. Alias precedence lives in one place (stageFromEnv). - doctor: the cluster's stage, read off the jobs-manager Deployment spec, is now read alias-first (stageFromClusterSpec) — a consumer-side alias so the edge chart can adopt the canonical key on its own S3-edge timeline. - auth login / status --check help + the unknown-env error name the canonical var (legacy noted); goldens regenerated. - env-resolution guard registers TRACEBLOC_ENV as a needle so a new read of either name lands in the allowlist; TestMain clears both stage vars so the higher-precedence canonical name can't make CLIENT_ENV-only isolation flaky. - VERSION 0.10.24 -> 0.10.25 (version-bump-gate: env reads are packaged paths). The other RFC-0076 config keys (registry REGISTRY_URL, telemetry, boolean gates) have no owned, unprefixed occurrence here: TRACEBLOC_ALLOW_UNVERIFIED already carries the prefix and no REGISTRY_URL/SKIP_TELEMETRY var exists. Co-Authored-By: Claude Opus 4.8 --- VERSION | 2 +- internal/api/client.go | 33 +++++++++++++---- internal/api/client_test.go | 25 ++++++++++++- internal/cli/auth.go | 6 ++-- internal/cli/env_resolution_test.go | 23 ++++++------ internal/cli/main_test.go | 21 +++++++++++ internal/cli/telemetry_test.go | 4 +++ internal/cli/testdata/golden/07-login.golden | 4 +-- .../cli/testdata/golden/zz-all-strings.golden | 2 +- internal/doctor/doctor.go | 30 ++++++++++++---- internal/doctor/doctor_test.go | 36 ++++++++++++++++++- 11 files changed, 153 insertions(+), 33 deletions(-) create mode 100644 internal/cli/main_test.go diff --git a/VERSION b/VERSION index 211f7aaf..dbca4f35 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.10.24 +0.10.25 diff --git a/internal/api/client.go b/internal/api/client.go index e295189e..88a241c6 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -21,13 +21,22 @@ import ( "time" ) -// Backend environments (mirror CLIENT_ENV). +// Backend environments (mirror the stage env var — $TRACEBLOC_ENV, legacy $CLIENT_ENV). const ( EnvDev = "dev" EnvStg = "stg" EnvProd = "prod" ) +// Stage-selecting environment variables (RFC-0076 settings-naming, backend#3391). +// TRACEBLOC_ENV is the canonical name; CLIENT_ENV is the legacy alias, read as a +// fallback so existing installs and $CLIENT_ENV exports keep working. Alias-first: +// read new-or-old, never break a deployment. Legacy alias remove_by: 2026-12-31. +const ( + StageEnvVar = "TRACEBLOC_ENV" + LegacyStageEnvVar = "CLIENT_ENV" +) + const defaultTimeout = 30 * time.Second // ── User-Agent: minimum-CLI-version handshake (RFC-0001 §13 / §14 R11 / C.1) ── @@ -80,9 +89,9 @@ func (t userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) return t.base.RoundTrip(req) } -// BaseURL maps a CLIENT_ENV value to the backend base URL — kept in lock-step -// with the installer's `_backend_url` and client-runtime's CLIENT_ENV→backend -// mapping. Unknown / empty → prod. +// BaseURL maps a stage value to the backend base URL — kept in lock-step with the +// installer's `_backend_url` and client-runtime's stage→backend mapping. Unknown / +// empty → prod. func BaseURL(env string) string { switch strings.ToLower(env) { case EnvDev: @@ -94,18 +103,28 @@ func BaseURL(env string) string { } } -// ResolveEnv picks the backend env: an explicit value (a --env flag) wins, -// then $CLIENT_ENV, then prod. +// ResolveEnv picks the backend env: an explicit value (a --env flag) wins, then +// the stage env var — canonical $TRACEBLOC_ENV, else legacy $CLIENT_ENV — then prod. func ResolveEnv(explicit string) string { if explicit != "" { return strings.ToLower(explicit) } - if e := os.Getenv("CLIENT_ENV"); e != "" { + if e := stageFromEnv(); e != "" { return strings.ToLower(e) } return EnvProd } +// stageFromEnv reads the deploy stage from the process environment, preferring the +// canonical TRACEBLOC_ENV over the legacy CLIENT_ENV alias (remove_by 2026-12-31). +// The single reader of both names, so the alias precedence lives in one place. +func stageFromEnv() string { + if e := os.Getenv(StageEnvVar); e != "" { + return e + } + return os.Getenv(LegacyStageEnvVar) +} + // IsKnownEnv reports whether env is one of the recognized backends (dev/stg/prod, // case-insensitively). Callers that let a human PICK the env (e.g. `login`) use it // to reject a typo up front — BaseURL deliberately falls unknown values back to diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 2a1f107a..2d3a104e 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -28,12 +28,14 @@ func TestBaseURL(t *testing.T) { } func TestResolveEnv(t *testing.T) { + // Isolate both stage vars: the canonical name and the legacy alias. + t.Setenv("TRACEBLOC_ENV", "") t.Setenv("CLIENT_ENV", "stg") if got := ResolveEnv("dev"); got != "dev" { t.Errorf("explicit should win: got %q", got) } if got := ResolveEnv(""); got != "stg" { - t.Errorf("CLIENT_ENV should be used: got %q", got) + t.Errorf("legacy $CLIENT_ENV should be used as the fallback: got %q", got) } t.Setenv("CLIENT_ENV", "") if got := ResolveEnv(""); got != "prod" { @@ -41,6 +43,27 @@ func TestResolveEnv(t *testing.T) { } } +// TestResolveEnvStageAlias pins the RFC-0076 alias precedence (backend#3391): the +// canonical $TRACEBLOC_ENV is preferred, the legacy $CLIENT_ENV is read only as a +// fallback, and an explicit --env still beats both. +func TestResolveEnvStageAlias(t *testing.T) { + // Canonical alone is honoured. + t.Setenv("TRACEBLOC_ENV", "dev") + t.Setenv("CLIENT_ENV", "") + if got := ResolveEnv(""); got != "dev" { + t.Errorf("canonical $TRACEBLOC_ENV should be used: got %q", got) + } + // Canonical wins over the legacy alias when both are set. + t.Setenv("CLIENT_ENV", "prod") + if got := ResolveEnv(""); got != "dev" { + t.Errorf("canonical $TRACEBLOC_ENV should beat legacy $CLIENT_ENV: got %q", got) + } + // Explicit --env still wins over both. + if got := ResolveEnv("stg"); got != "stg" { + t.Errorf("explicit --env should beat the environment: got %q", got) + } +} + func TestIsKnownEnv(t *testing.T) { known := []string{"dev", "stg", "prod", "DEV", "Prod"} // case-insensitive for _, env := range known { diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 8adee687..42f7aa3f 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -40,7 +40,7 @@ machine. Honors HTTP(S)_PROXY / NO_PROXY for corporate-proxy networks.`, }, } cmd.Flags().StringVar(&envFlag, "env", "", - "backend environment: dev|stg|prod (default: $CLIENT_ENV, then prod)") + "backend environment: dev|stg|prod (default: $TRACEBLOC_ENV, then legacy $CLIENT_ENV, then prod)") return cmd } @@ -65,7 +65,7 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { if !api.IsKnownEnv(env) { return &exitError{code: exitFailure, err: fmt.Errorf( "unknown backend environment %q — valid values are dev, stg, prod (default). "+ - "Check --env / $CLIENT_ENV", env)} + "Check --env / $TRACEBLOC_ENV (or legacy $CLIENT_ENV)", env)} } client := newAPIClient(env) p.Detailf("backend %s — requesting a device code …", client.BaseURL) @@ -461,7 +461,7 @@ func newAuthStatusCmd() *cobra.Command { cmd.Flags().BoolVar(&check, "check", false, "exit 0 only if signed in with a backend-valid token, else 1; silent unless --verbose") cmd.Flags().StringVar(&envFlag, "env", "", - "backend environment the check targets: dev|stg|prod (default: $CLIENT_ENV, then prod)") + "backend environment the check targets: dev|stg|prod (default: $TRACEBLOC_ENV, then legacy $CLIENT_ENV, then prod)") return cmd } diff --git a/internal/cli/env_resolution_test.go b/internal/cli/env_resolution_test.go index 8449ec53..b049c473 100644 --- a/internal/cli/env_resolution_test.go +++ b/internal/cli/env_resolution_test.go @@ -428,9 +428,9 @@ func TestKnownSessionEnvGatesUnknownButKeepsKnown(t *testing.T) { // internal/ — i.e. blind exactly where a new site is most likely to land, in a // package written by someone who never reads internal/cli (Lukas on #551). var resolutionSites = map[string]string{ - // The primitive: ResolveEnv is the --env/$CLIENT_ENV/prod chain, and the only - // os.Getenv("CLIENT_ENV") in the module. - "internal/api/client.go": "api.ResolveEnv — the primitive chain, and the only $CLIENT_ENV read", + // The primitive: ResolveEnv is the --env/$TRACEBLOC_ENV/legacy $CLIENT_ENV/prod + // chain, and the only os.Getenv of the stage var in the module. + "internal/api/client.go": "api.ResolveEnv — the primitive chain, and the only $TRACEBLOC_ENV/$CLIENT_ENV read", // The --env FLAG, a different question: the env the human/installer NAMED, // which login persists (the one cfg.CurrentEnv WRITE) and `auth status --check` // validates against the session. @@ -441,9 +441,10 @@ var resolutionSites = map[string]string{ // Storage. Profiles are keyed by the RAW stored string, so this layer must not // normalise; it hands the raw value out and sessionEnv normalises it. "internal/config/config.go": "the on-disk current_env field, its accessors, and the v1 migration", - // The CLUSTER's CLIENT_ENV, read off the jobs-manager Deployment — a - // deliberately different question from this CLI's session env. - "internal/doctor/doctor.go": "the cluster's own CLIENT_ENV, for the egress probe's target host", + // The CLUSTER's stage (chart-written $TRACEBLOC_ENV, legacy $CLIENT_ENV), read + // off the jobs-manager Deployment — a deliberately different question from this + // CLI's session env. + "internal/doctor/doctor.go": "the cluster's own stage var, for the egress probe's target host", // internal/cli/telemetry.go is deliberately ABSENT, and the staleness check // below is what keeps it that way: telemetryEnv/signedInEnv delegate the whole // chain to sessionEnv and name no needle, so an entry for it would be inert — @@ -455,10 +456,12 @@ var resolutionSites = map[string]string{ // api.BaseURL/IsKnownEnv are pure mappings over an argument and are deliberately // absent — they resolve nothing. // -// Deliberately BROAD (bare identifiers, and CLIENT_ENV unquoted so it matches -// help text too): a false positive is a loud line in a diff, a false negative is -// the bug this guard exists to catch. Fail closed. -var envNeedles = []string{"CurrentEnv", "ResolveEnv", "CLIENT_ENV"} +// Deliberately BROAD (bare identifiers, and the stage-var names unquoted so they +// match help text too): a false positive is a loud line in a diff, a false negative +// is the bug this guard exists to catch. Fail closed. TRACEBLOC_ENV is the RFC-0076 +// canonical stage var; CLIENT_ENV is its legacy alias (remove_by 2026-12-31) — both +// are needles so a new read of EITHER name lands in the allowlist. +var envNeedles = []string{"CurrentEnv", "ResolveEnv", "TRACEBLOC_ENV", "CLIENT_ENV"} // matchesAnyNeedle is THE matcher, called from both directions — the detection // sweep and the allowlist audit. One function on purpose: two copies of "does diff --git a/internal/cli/main_test.go b/internal/cli/main_test.go new file mode 100644 index 00000000..3f57416f --- /dev/null +++ b/internal/cli/main_test.go @@ -0,0 +1,21 @@ +package cli + +import ( + "os" + "testing" +) + +// TestMain clears BOTH stage-selecting env vars before the package's tests run, so +// no test inherits a developer's or CI runner's ambient stage from the process +// environment. This is load-bearing after RFC-0076 (backend#3391) made the reads +// alias-first: api.ResolveEnv now consults the canonical $TRACEBLOC_ENV BEFORE the +// legacy $CLIENT_ENV, so a test that pins the ambient stage by setting only one of +// the two names would be silently overridden by an ambient value of the other. A +// package-wide clean baseline fixes that for every current AND future site — the +// same class-not-instance guarantee the env-resolution guard in this package is +// built around — rather than relying on each test to neutralise both names. +func TestMain(m *testing.M) { + os.Unsetenv("TRACEBLOC_ENV") + os.Unsetenv("CLIENT_ENV") + os.Exit(m.Run()) +} diff --git a/internal/cli/telemetry_test.go b/internal/cli/telemetry_test.go index 05056775..9194fd02 100644 --- a/internal/cli/telemetry_test.go +++ b/internal/cli/telemetry_test.go @@ -32,6 +32,10 @@ func testBuildInfo() BuildInfo { func isolateConfig(t *testing.T) { t.Helper() t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + // Pin the ambient stage to prod, clearing the canonical name too: it outranks + // the legacy $CLIENT_ENV set below (api.stageFromEnv), so leaving it set would + // let it override this pin. + t.Setenv("TRACEBLOC_ENV", "") t.Setenv("CLIENT_ENV", api.EnvProd) } diff --git a/internal/cli/testdata/golden/07-login.golden b/internal/cli/testdata/golden/07-login.golden index d08e9359..db8da7e0 100644 --- a/internal/cli/testdata/golden/07-login.golden +++ b/internal/cli/testdata/golden/07-login.golden @@ -22,7 +22,7 @@ Usage: tracebloc login [flags] Flags: - --env string backend environment: dev|stg|prod (default: $CLIENT_ENV, then prod) + --env string backend environment: dev|stg|prod (default: $TRACEBLOC_ENV, then legacy $CLIENT_ENV, then prod) -h, --help help for login Global Flags: @@ -50,7 +50,7 @@ Usage: Flags: --check exit 0 only if signed in with a backend-valid token, else 1; silent unless --verbose - --env string backend environment the check targets: dev|stg|prod (default: $CLIENT_ENV, then prod) + --env string backend environment the check targets: dev|stg|prod (default: $TRACEBLOC_ENV, then legacy $CLIENT_ENV, then prod) -h, --help help for status Global Flags: diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index af957237..e4ccd851 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -809,7 +809,7 @@ screen. %s/%d are runtime placeholders. "tracebloc-doctor-%s.txt" "tracebloc-stage-%s-%s" "unavailable" -"unknown backend environment %q — valid values are dev, stg, prod (default). Check --env / $CLIENT_ENV" +"unknown backend environment %q — valid values are dev, stg, prod (default). Check --env / $TRACEBLOC_ENV (or legacy $CLIENT_ENV)" "unknown command %q for %q" "unparseable MemTotal %q: %w" "unparseable NCPU %q: %w" diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index e2670cbe..6ed3a4fe 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -570,7 +570,7 @@ func checkProxy(env map[string]string) Result { // the backend at all. func checkBackendEgress(ctx context.Context, env map[string]string, probe func(context.Context, string) error) Result { const name = "Backend egress (from this machine)" - host := backendHost(env["CLIENT_ENV"]) + host := backendHost(stageFromClusterSpec(env)) url := "https://" + host + "/" if err := probe(ctx, url); err != nil { return Result{ @@ -583,20 +583,36 @@ func checkBackendEgress(ctx context.Context, env map[string]string, probe func(c return Result{Name: name, Status: StatusOK, Detail: host + " reachable"} } -// backendHost maps CLIENT_ENV to the backend API host, mirroring the edge +// stageFromClusterSpec picks the deploy stage out of the jobs-manager container's +// env, preferring the canonical TRACEBLOC_ENV over the legacy CLIENT_ENV alias. +// These keys are written by the edge CHART (this CLI is a consumer here, not the +// owner): reading both lets the chart adopt the canonical name on its own S3-edge +// timeline without a flag-day, and today — while the chart still writes CLIENT_ENV +// — the fallback keeps behaviour identical. Legacy alias remove_by: 2026-12-31. +// The literals are spelled out (not api.StageEnvVar) both because these are the +// chart's key names, not this CLI's process-env names, and so the env-resolution +// guard still sees this file as a sanctioned stage-var read site. +func stageFromClusterSpec(env map[string]string) string { + if v := env["TRACEBLOC_ENV"]; v != "" { + return v + } + return env["CLIENT_ENV"] +} + +// backendHost maps a stage value to the backend API host, mirroring the edge // runtime's own mapping (controller.py). Unset/unknown defaults to prod, the -// chart's CLIENT_ENV default. +// chart's stage default. // // DERIVED FROM api.BaseURL, not restated. The env→host mapping used to be a // second copy of BaseURL's switch living in this package, which is how the two // drift: the same three hosts written down twice, with nothing that fails when // only one of them is edited. api.BaseURL already lower-cases, so TrimSpace is -// the only normalisation this adds — a CLIENT_ENV read off a container spec can +// the only normalisation this adds — a stage value read off a container spec can // carry surrounding whitespace that a --env flag cannot. // -// The input is the CLUSTER's CLIENT_ENV (read off the jobs-manager Deployment), -// not this CLI's session env — a deliberately different question, which is why -// this takes a string rather than calling into the session resolution. +// The input is the CLUSTER's stage (read off the jobs-manager Deployment), not +// this CLI's session env — a deliberately different question, which is why this +// takes a string rather than calling into the session resolution. func backendHost(clientEnv string) string { u, err := url.Parse(api.BaseURL(strings.TrimSpace(clientEnv))) if err != nil || u.Host == "" { diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index ec9de8bb..490e13cb 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -16,6 +16,7 @@ import ( "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" + "github.com/tracebloc/cli/internal/api" "github.com/tracebloc/cli/internal/cluster" ) @@ -404,13 +405,46 @@ func TestCheckBackendEgress(t *testing.T) { failProbe := func(context.Context, string) error { return errors.New("dns failure") } if r := checkBackendEgress(bg(), map[string]string{"CLIENT_ENV": "dev"}, okProbe); r.Status != StatusOK || !strings.Contains(r.Detail, "dev-api.tracebloc.io") { - t.Fatalf("reachable dev => %v / %q", r.Status, r.Detail) + t.Fatalf("reachable dev (legacy CLIENT_ENV) => %v / %q", r.Status, r.Detail) + } + // Canonical stage key on the cluster spec resolves the same host (RFC-0076). + if r := checkBackendEgress(bg(), map[string]string{"TRACEBLOC_ENV": "dev"}, okProbe); r.Status != StatusOK || !strings.Contains(r.Detail, "dev-api.tracebloc.io") { + t.Fatalf("reachable dev (canonical TRACEBLOC_ENV) => %v / %q", r.Status, r.Detail) } if r := checkBackendEgress(bg(), map[string]string{}, failProbe); r.Status != StatusFail || !strings.Contains(r.Detail, "api.tracebloc.io") { t.Fatalf("unreachable default => %v / %q", r.Status, r.Detail) } } +// TestStageFromClusterSpec pins the alias-first read of the chart-written stage +// key: canonical TRACEBLOC_ENV preferred, legacy CLIENT_ENV as the fallback. +func TestStageFromClusterSpec(t *testing.T) { + // stageFromClusterSpec reads the key LITERALS (spelled out for the cli-package + // env-resolution guard), so nothing compile-couples them to the api consts. Pin + // the two names in lock-step here: if api's canonical/legacy stage-var names ever + // change, this fails instead of the doctor silently reading a stale key off the + // jobs-manager spec and probing the wrong host. + if api.StageEnvVar != "TRACEBLOC_ENV" || api.LegacyStageEnvVar != "CLIENT_ENV" { + t.Fatalf("stage-var names drifted from the literals stageFromClusterSpec reads: "+ + "canonical api.StageEnvVar=%q, legacy api.LegacyStageEnvVar=%q", api.StageEnvVar, api.LegacyStageEnvVar) + } + tests := []struct { + name string + env map[string]string + want string + }{ + {"canonical only", map[string]string{"TRACEBLOC_ENV": "dev"}, "dev"}, + {"legacy only", map[string]string{"CLIENT_ENV": "stg"}, "stg"}, + {"canonical wins", map[string]string{"TRACEBLOC_ENV": "dev", "CLIENT_ENV": "prod"}, "dev"}, + {"neither", map[string]string{}, ""}, + } + for _, tc := range tests { + if got := stageFromClusterSpec(tc.env); got != tc.want { + t.Errorf("%s: stageFromClusterSpec(%v) = %q, want %q", tc.name, tc.env, got, tc.want) + } + } +} + func TestBackendHost(t *testing.T) { tests := map[string]string{ "dev": "dev-api.tracebloc.io", From 6fbbb5afd9dd1e0ba3c0a03d2b49cf8b0dbc6182 Mon Sep 17 00:00:00 2001 From: Arturo Peroni Date: Fri, 11 Sep 2026 10:37:42 +0200 Subject: [PATCH 2/2] fix(cli): errcheck TestMain os.Unsetenv + bump VERSION 0.10.26 after develop merge (cli#656) Co-Authored-By: Claude Opus 4.8 --- VERSION | 2 +- internal/cli/main_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index dbca4f35..61012ac6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.10.25 +0.10.26 diff --git a/internal/cli/main_test.go b/internal/cli/main_test.go index 3f57416f..7274f960 100644 --- a/internal/cli/main_test.go +++ b/internal/cli/main_test.go @@ -15,7 +15,7 @@ import ( // same class-not-instance guarantee the env-resolution guard in this package is // built around — rather than relying on each test to neutralise both names. func TestMain(m *testing.M) { - os.Unsetenv("TRACEBLOC_ENV") - os.Unsetenv("CLIENT_ENV") + _ = os.Unsetenv("TRACEBLOC_ENV") + _ = os.Unsetenv("CLIENT_ENV") os.Exit(m.Run()) }