Skip to content

config: apply environment overrides before unmarshal - #902

Open
dlanov wants to merge 1 commit into
openconfig:mainfrom
dlanov:fix-833-collector-env
Open

config: apply environment overrides before unmarshal#902
dlanov wants to merge 1 commit into
openconfig:mainfrom
dlanov:fix-833-collector-env

Conversation

@dlanov

@dlanov dlanov commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #833

Nested free-form collector configuration supplied through GNMIC_*
environment variables was not reflected in the unmarshaled Config.

Apply environment-backed free-form configuration before unmarshaling,
while limiting underscore-based nesting to sections whose keys are not
known ahead of time:

  • clustering
  • outputs
  • inputs
  • processors
  • loader
  • actions

Known struct-backed configuration such as cluster-name, log-file,
and format continues to use Viper's AutomaticEnv and environment
key replacer. This avoids treating underscores in variables such as
GNMIC_LOG_FILE as nesting separators while still supporting nested
values such as GNMIC_CLUSTERING_LOCKER_ADDRESS.

Regression coverage verifies:

  • known struct-backed environment variables remain handled by Viper
  • GNMIC_LOG_FILE and GNMIC_CLUSTER_NAME override their fields
  • GNMIC_CLUSTERING_LOCKER_ADDRESS overrides nested free-form config
  • unrelated clustering values from the configuration file are preserved
  • underscore splitting is restricted to the six free-form sections

Testing:

  • focused environment/config regression tests
  • go test ./pkg/config -count=1
  • go test ./... -count=1
  • go vet ./...
  • git diff --check

@karimra karimra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real problem is that _ has two meanings. In log-file it is a dash, and log-file is a struct field. In clustering.locker.address it is a level of nesting, and that key lives inside a map[string]any. envToMap cannot know which meaning to use. Moving the call does not fix this. It only changes a silent failure into a crash at startup.

Could you apply the _ splitting only to the free-form sections (clustering, outputs, inputs, processors, loader, actions)? For the other keys, viper's AutomaticEnv and the key replacer already work, because those keys are struct fields with known names. This way the guessing happens only where the keys are not known in advance.

@dlanov
dlanov force-pushed the fix-833-collector-env branch from 1a74abf to 38c435e Compare August 11, 2026 13:09
@dlanov

dlanov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification. I updated the implementation so underscore splitting is limited to the free-form sections: clustering, outputs, inputs, processors, loader, and actions. Known struct-backed fields continue through Viper AutomaticEnv/key replacement. I also added regression coverage for GNMIC_LOG_FILE and GNMIC_CLUSTER_NAME alongside GNMIC_CLUSTERING_LOCKER_ADDRESS, and updated the existing environment helper tests to reflect the scoped behavior. Full go test ./..., go vet ./..., and git diff --check pass locally.

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.

GNMIC_* environment variables are not interpolated in collector mode

2 participants