Skip to content

fix(env,config): replace envsubst with built-in expander - #64

Open
kooksee wants to merge 9 commits into
v2from
fix/env-subst-numeric
Open

kooksee wants to merge 9 commits into
v2from
fix/env-subst-numeric

Conversation

@kooksee

@kooksee kooksee commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Removes three unmaintained/problematic external dependencies and fixes long-standing bugs in environment variable substitution:

  • github.com/a8m/envsubst → replaced by a built-in expander (env/expand.go), fixing default-value and digit-key resolution bugs.
  • github.com/ettle/strcase (upstream unmaintained) → vendored into internal/strcase with its MIT license preserved.
  • github.com/iancoleman/strcase → removed; the codegen plugin now uses the vendored internal/strcase.

Changes

envsubst removal & substitution fixes (5c29c29)

  • Fix ${VAR:default} silently resolving to empty. envsubst implements bash parameter expansion, where the single-colon form is a substring offset, not a default value. ${PORT:8080} with PORT unset resolved to an empty string, so numeric defaults like port: ${PORT:8080} never applied (loaded as 0).
  • Fix keys containing digits failing to resolve. keyHandler renames keys at digit-to-uppercase boundaries on env.Set/Reload (T2HOST is stored as T2_HOST, SHA256SUM as SHA256_SUM), while substitution looked up the raw name only, so declared values were never found.
  • New substitution semantics (env/expand.go):
    • ${NAME} / $NAME — env value, empty when unset
    • ${NAME:default} / ${NAME:-default} — default when unset or empty; everything after the first : is the default (inner colons kept, surrounding quotes stripped)
    • declared patch_envs values keep priority over inline defaults (existing behavior preserved)

env key-resolution hardening (b9072ef)

  • Unified 3-candidate lookup (raw name → keyHandler(name) → keyHandler(ToUpper(name))) shared by Expand, Get/getVal and Lookup, so lowercase references of renamed digit keys resolve too (${sha256sum} finds SHA256_SUM).
  • Delete unsets every stored spelling of a key, so variables saved under a renamed key are actually removed.
  • New typed getters GetBoolOr / GetIntOr / GetFloatOr — explicit defaults on unset/empty/parse-error instead of -1/false sentinels.
  • Documented Reload() blast radius: it rewrites the whole process environment in place; warning added to doc comment and READMEs.
  • Fixed documented-but-unimplemented ${env.VAR} syntax in config docs (real syntax is the CEL env("VAR") function).

resty lint fix (0f496e3)

  • Drop deprecated net.Dialer.DualStack in component/resty (staticcheck SA1019, pre-existing CI lint failure).

strcase vendoring (a54c794)

  • Copy the used subset of github.com/ettle/strcase v0.2.0 into internal/strcase (caser, split, convert, initialisms, unicode helpers) with upstream tests kept as the behavioral spec; MIT license text and copyright preserved in internal/strcase/LICENSE.
  • Only importer env/util.go switched to the internal path; go mod tidy drops the dependency. No behavioral change (key normalization verified unchanged).

iancoleman/strcase removal (4f18b58)

  • cmds/protoc-gen-go-errors2 and the protoutils test only used ToCamel/ToSnake; switched to the vendored internal/strcase (ToPascal/ToSnake). Output is identical for conventional proto naming; names with digits or acronyms now produce more idiomatic Go (APIKey -> ApiKey instead of Apikey).

Also carries refactor: adopt modern Go stdlib APIs across packages (b82d7e2): mechanical reflect.TypeOf → reflect.TypeFor modernization committed on this branch.

Supersedes #65.

Behavior changes

  • ${VAR:default} now applies the default when the variable is unset or empty (previously silently empty). Bash advanced expansions (:?, #len, substring, // replace) are no longer supported by env.Expand — none were used in the repo or documented.
  • Get/Lookup/Delete resolve additional key spellings (raw name first); keys stored under exactly one spelling keep their values, so existing callers are unaffected unless a raw and a normalized key coexist.
  • Deliberately not changed: keyHandler's renaming itself (case-asymmetric) — deferred to v3.

Testing

  • Vendored upstream strcase suite passes (go test ./internal/strcase/)
  • env/expand_test.go (24 cases), env/api_test.go (raw fallback, typed getters, LoadFiles, Expand×Reload), config/expand_test.go (4 end-to-end regressions)
  • go test -race ./env/ ./config/ and full suite go test ./... pass; golden config tests unchanged
  • go vet clean; golangci-lint run repo-wide: 0 issues

- add env/expand.go implementing ${NAME}, ${NAME:default} and
  ${NAME:-default} substitution with quote-stripped defaults
- fix ${VAR:default} silently resolving to empty: envsubst treated the
  single-colon form as a bash substring offset, so numeric defaults
  like ${PORT:8080} never applied
- fix keys containing digits failing to resolve: lookup falls back from
  the raw name to the keyHandler-normalized name (T2HOST -> T2_HOST)
- drop github.com/a8m/envsubst dependency
…d Or getters

- centralize env key resolution in lookupEnv: raw name, keyHandler-normalized
  name, then normalized uppercase name; fixes lowercase references of
  digit-renamed keys (${sha256sum} now finds SHA256_SUM)
- Get/Lookup now resolve raw-exported keys without Reload, matching Expand;
  Delete unsets every stored spelling so renamed vars are actually removed
- add GetBoolOr/GetIntOr/GetFloatOr with explicit defaults instead of -1/false
  sentinels
- document Reload's process-wide rewriting side effect in doc comment and
  READMEs
- fix documented-but-unimplemented ${env.VAR} syntax in config docs (real
  syntax is the CEL env("VAR") function)
Fast Fallback has been enabled by default since Go 1.12; the field is
deprecated and fails staticcheck SA1019 on CI.
Upstream github.com/ettle/strcase (v0.2.0) is no longer maintained.
Copy the sources the repo actually uses into internal/strcase, keeping
the package API frozen; upstream MIT license text and copyright notice
are preserved in internal/strcase/LICENSE as the license requires.

Only env/util.go imported the package; behavior is verified by the
vendored upstream test suite (caser/strcase/unicode tests) plus the
existing env/config tests.
The codegen plugin and protoutils test only used ToCamel/ToSnake; switch
to the vendored internal/strcase (ToPascal/ToSnake). Outputs are
identical for conventional proto naming; for names with digits or
acronyms the new output is more idiomatic (APIKey -> ApiKey instead of
Apikey).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant