Add --datasource.* CLI flags as alternatives to DATA_SOURCE_* env vars - #1375
Add --datasource.* CLI flags as alternatives to DATA_SOURCE_* env vars#1375pujitha24 wants to merge 1 commit into
Conversation
Motivation
exporter/datasource.go's GetDataSources() only reads Postgres connection
config from DATA_SOURCE_NAME, DATA_SOURCE_URI(_FILE), DATA_SOURCE_USER(_FILE)
and DATA_SOURCE_PASS(_FILE) environment variables, with no CLI flag path.
This is part of a broader effort to let CLI flags eventually replace env-var
configuration across the exporter, and the datasource config was the one
remaining piece with no flag equivalent at all.
Approach
Add seven new kingpin flags to cmd/postgres_exporter/main.go: --datasource.dsn,
--datasource.uri, --datasource.uri-file, --datasource.user,
--datasource.user-file, --datasource.pass, --datasource.pass-file. They carry
no Envar() binding since GetDataSources() already owns the env fallback
logic.
GetDataSources() now takes a DataSourceOpts struct instead of no arguments.
For each setting, an explicit flag value takes precedence; when a flag is
empty, behavior falls back exactly to the previous env-var-only logic
(DATA_SOURCE_NAME still short-circuits everything else, file-based secrets
still win over plain values). All new flags default to "", so deployments
that only ever set env vars see no behavior change.
Updated README.md to document the new flags and cross-reference them from
the Environment Variables section, matching how prior flag additions in this
repo have been documented.
Validation
- go build ./... — clean
- go test ./... — all four packages pass; exporter package goes from 9 to
11 passing cases (added TestFlagSettingWithDsn, confirming the DSN flag
wins over DATA_SOURCE_NAME, and TestFlagSettingWithUserPassUri, confirming
a DSN can be built from --datasource.user/--datasource.pass/--datasource.uri
alone with no env vars set); the three pre-existing env-var tests were
updated to call GetDataSources(DataSourceOpts{}) and still pass unchanged,
confirming no behavior change for env-var-only usage
- golangci-lint run ./exporter/... ./cmd/... (version v2.12.2, matching this
repo's CI) — 0 issues
- gofmt -l on all changed files — clean
Report: prometheus-community#1372
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5 (via Claude Code)
ArthurSens
left a comment
There was a problem hiding this comment.
Same feedback we gave to #1376: I don't think it's a good idea to have sensitive information like database secrets in CLI flags. This can be easily exploited because CLI flags are written to files in the /proc filesystem.
|
Same concern got raised on #1376 (sysadmind's comment, then nicolastakashi and you discussing whether secrets should move to a config file — maybe extending auth-modules.yml — instead of flags). I'd rather see that design settle there than push a guess here. The file-path flags (uri-file, user-file, pass-file) aren't really the problem since they just point at a file on disk; dsn and pass as raw flag values are exactly what you're calling out. I'll hold off on further changes to this PR until there's a direction on #1376, and rework it once that's clearer. |
|
Awesome, then let's focus on that PR and we can avoid duplicated work here :) |
Motivation
exporter/datasource.go's GetDataSources() only reads Postgres connection
config from DATA_SOURCE_NAME, DATA_SOURCE_URI(_FILE), DATA_SOURCE_USER(_FILE)
and DATA_SOURCE_PASS(_FILE) environment variables, with no CLI flag path.
This is part of a broader effort to let CLI flags eventually replace env-var
configuration across the exporter, and the datasource config was the one
remaining piece with no flag equivalent at all.
Approach
Add seven new kingpin flags to cmd/postgres_exporter/main.go: --datasource.dsn,
--datasource.uri, --datasource.uri-file, --datasource.user,
--datasource.user-file, --datasource.pass, --datasource.pass-file. They carry
no Envar() binding since GetDataSources() already owns the env fallback
logic.
GetDataSources() now takes a DataSourceOpts struct instead of no arguments.
For each setting, an explicit flag value takes precedence; when a flag is
empty, behavior falls back exactly to the previous env-var-only logic
(DATA_SOURCE_NAME still short-circuits everything else, file-based secrets
still win over plain values). All new flags default to "", so deployments
that only ever set env vars see no behavior change.
Updated README.md to document the new flags and cross-reference them from
the Environment Variables section, matching how prior flag additions in this
repo have been documented.
Validation
11 passing cases (added TestFlagSettingWithDsn, confirming the DSN flag
wins over DATA_SOURCE_NAME, and TestFlagSettingWithUserPassUri, confirming
a DSN can be built from --datasource.user/--datasource.pass/--datasource.uri
alone with no env vars set); the three pre-existing env-var tests were
updated to call GetDataSources(DataSourceOpts{}) and still pass unchanged,
confirming no behavior change for env-var-only usage
repo's CI) — 0 issues
Report: #1372
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)
Fixes #1372