Flags mapped to environment variables

[sc250024]

Feature Request

Hey all, great work on the project. I authored a Kubernetes Helm chart for this repo (https://github.com/helm/charts/pull/7927), and was wondering if it was possible to have each of the CLI flags mapped to an environment variable.

Describe the solution you'd like

Basically, each flag should have an equivalent environment variable. Something like this:

-adapter.send-timeout            => ADAPTER_SEND_TIMEOUT
-log.level                       => LOG_LEVEL
-pg.copy-table                   => PG_COPY_TABLE
-pg.database                     => PG_DATABASE
-pg.db-connect-retries           => PG_DB_CONNECT_RETRIES
-pg.host                         => PG_HOST
-pg.max-idle-conns               => PG_MAX_IDLE_CONNS
-pg.max-open-conns               => PG_MAX_OPEN_CONNS
-pg.password                     => PG_PASSWORD
-pg.port                         => PG_PORT
-pg.prometheus-chunk-interval    => PG_PROMETHEUS_CHUNK_INTERVAL
-pg.prometheus-log-samples       => PG_PROMETHEUS_LOG_SAMPLES
-pg.prometheus-normalized-schema => PG_PROMETHEUS_NORMALIZED_SCHEMA
-pg.schema                       => PG_SCHEMA
-pg.ssl-mode                     => PG_SSL_MODE
-pg.table                        => PG_TABLE
-pg.use-timescaledb              => PG_USE_TIMESCALEDB
-pg.user                         => PG_USER
-read.only                       => READ_ONLY
-web.listen-address              => WEB_LISTEN_ADDRESS
-web.telemetry-path              => WEB_TELEMETRY_PATH

Is your feature request related to a problem? Please describe.

Not a "problem" per se, but the change would make the chart more Kubernetes native. For example, a potential PG_PASSWORD environment variable could be read from a Kubernetes secret as opposed to being passed directly from (and thus visible) from a command line argument.

Describe alternatives you've considered

If you all are fine with only having the CLI flags, that would be alright, just wondering if the environment variable approach is possible since it's (probably) a quick fix.

Teachability, Documentation, Adoption, Migration Strategy

This could be easily documented in a README.md snippet. As long as the mapping between the CLI flags and environment variable are consistent, it would be no problem.

[niksajakovljevic]

@sc250024 I believe that you should be able to use an environment variable as a command line argument? Basically, a Kubernetes secret would become an environment variable...or am I missing something?

[sc250024]

@niksajakovljevic What I mean is, for each command line flag, it should have an equivalent environment variable which, if set, does the same thing as setting the flag.

For example, instead of setting the flag -log.level=debug, I could set LOG_LEVEL environment variable in the container / pod to debug, and it would do the same thing. Basically I searched the codebase of this repo for os.Getenv, and didn't find anything.

This would imply setting a hierarchy, i.e. whether a command-line flag takes precedence over an environment variable, or vice-versa.

[niksajakovljevic]

@sc250024 sure, what I did not understand is why is that needed? In my response above I've explained how you can solve the problem of passing database password

[sc250024]

@niksajakovljevic It's a feature request, so while it's absolutely not required, it's also very good practice and an enhancement. The idea is to make this app more friendly to the 12Factor app standard. I've used the following frameworks before, all have open support for implementing what I described:

• OpenCLI Framework in NodeJS (https://oclif.io/docs/flags.html)
• Cobra / Viper in Golang (https://github.com/spf13/viper#working-with-environment-variables)

All of these have the option to map an environment variable with a CLI flag.

[jcoleman]

@niksajakovljevic Using environment variables written out as command line arguments in a systemd unit file isn't particularly great because they then become visible when doing service prometheus-postgresql-adapter status. That's a pretty serious security flaw.

Either allowing the adapter to read directly from env variables or from a config file would be the correct way to implement this.

[sc250024]

@jcoleman Well said, thank you.

[niksajakovljevic]

@sc250024 Feel free to create a PR for env support if you think it's needed. I'd be happy to see it ;)

@jcoleman Don't you need to be sudo to run service commands? If you're sudo on the machine then I don't see your point. Also I thought that service xxx status only prints the service name and not the whole command? Anyways, I do agree that being able to use env vars is nice is some cases.

[sc250024]

I'll give it a shot. Thank you!

[jcoleman]

@niksajakovljevic sudo is needed to start/restart/stop/etc., but not for status. But more importantly those arguments show up in ps aux output so are globally leaked to any unprivileged user.

[niksajakovljevic]

@jcoleman Sure, but I guess that secure Linux setup should hide processes of other users. That's pretty easy to configure by remounting /proc with hidepid=1 or 2. Anyways, it would be great to see PRs addressing security related problems because that was a bit put on side due to other priorities.

[sc250024]

@niksajakovljevic I found a nifty package which already does the heavy lifting. However, I found a problem with the way this package structures the command-line flags. Here are my examples.

Package repo => https://github.com/jamiealquiza/envy My commit with changes => https://github.com/sc250024/prometheus-postgresql-adapter/commit/7a773b0d841f0e2aaac7d7ce85d7e848303f6ea7

After building, the package I mentioned above nicely creates the following help message with equivalent environment variables named and defined, which is exactly what I would like to achieve:

$ ./prometheus-postgresql-adapter -h
Usage of ./prometheus-postgresql-adapter:
  -adapter.send-timeout duration
        The timeout to use when sending samples to the remote storage. [TIMESCALE_PROMPGADAPTER_ADAPTER.SEND_TIMEOUT] (default 30s)
  -leader-election.pg-advisory-lock-id int
        Unique advisory lock id per adapter high-availability group. Set it if you want to use leader election implementation based on PostgreSQL advisory lock. [TIMESCALE_PROMPGADAPTER_LEADER_ELECTION.PG_ADVISORY_LOCK_ID]
  -leader-election.pg-advisory-lock.prometheus-timeout slack
        Adapter will resign if there are no requests from Prometheus within a given timeout (0 means no timeout). Note: make sure that only one Prometheus instance talks to the adapter. Timeout value should be co-related with Prometheus scrape interval but add enough slack to prevent random flips. [TIMESCALE_PROMPGADAPTER_LEADER_ELECTION.PG_ADVISORY_LOCK.PROMETHEUS_TIMEOUT] (default -1ns)
  -leader-election.rest
        Enable REST interface for the leader election [TIMESCALE_PROMPGADAPTER_LEADER_ELECTION.REST]
  -log.level string
        The log level to use [ "error", "warn", "info", "debug" ]. [TIMESCALE_PROMPGADAPTER_LOG.LEVEL] (default "debug")
  -pg.copy-table string
        Override default table to COPY data to [TIMESCALE_PROMPGADAPTER_PG.COPY_TABLE]
  -pg.database string
        The PostgreSQL database [TIMESCALE_PROMPGADAPTER_PG.DATABASE] (default "postgres")
  -pg.db-connect-retries int
        How many times to retry connecting to the database [TIMESCALE_PROMPGADAPTER_PG.DB_CONNECT_RETRIES]
  -pg.host string
        The PostgreSQL host [TIMESCALE_PROMPGADAPTER_PG.HOST] (default "localhost")
  -pg.max-idle-conns int
        The max number of idle connections to the database [TIMESCALE_PROMPGADAPTER_PG.MAX_IDLE_CONNS] (default 10)
  -pg.max-open-conns int
        The max number of open connections to the database [TIMESCALE_PROMPGADAPTER_PG.MAX_OPEN_CONNS] (default 50)
  -pg.password string
        The PostgreSQL password [TIMESCALE_PROMPGADAPTER_PG.PASSWORD]
  -pg.port int
        The PostgreSQL port [TIMESCALE_PROMPGADAPTER_PG.PORT] (default 5432)
  -pg.prometheus-chunk-interval duration
        The size of a time-partition chunk in TimescaleDB [TIMESCALE_PROMPGADAPTER_PG.PROMETHEUS_CHUNK_INTERVAL] (default 12h0m0s)
  -pg.prometheus-log-samples
        Log raw samples to stdout [TIMESCALE_PROMPGADAPTER_PG.PROMETHEUS_LOG_SAMPLES]
  -pg.prometheus-normalized-schema
        Insert metric samples into normalized schema [TIMESCALE_PROMPGADAPTER_PG.PROMETHEUS_NORMALIZED_SCHEMA] (default true)
  -pg.read-only
        Read-only mode. Don't write to database. Useful when pointing adapter to read replica [TIMESCALE_PROMPGADAPTER_PG.READ_ONLY]
  -pg.schema string
        The PostgreSQL schema [TIMESCALE_PROMPGADAPTER_PG.SCHEMA]
  -pg.ssl-mode string
        The PostgreSQL connection ssl mode [TIMESCALE_PROMPGADAPTER_PG.SSL_MODE] (default "disable")
  -pg.table string
        Override prefix for internal tables. It is also a view name used for querying [TIMESCALE_PROMPGADAPTER_PG.TABLE] (default "metrics")
  -pg.use-timescaledb
        Use timescaleDB [TIMESCALE_PROMPGADAPTER_PG.USE_TIMESCALEDB] (default true)
  -pg.user string
        The PostgreSQL user [TIMESCALE_PROMPGADAPTER_PG.USER] (default "postgres")
  -web.listen-address string
        Address to listen on for web endpoints. [TIMESCALE_PROMPGADAPTER_WEB.LISTEN_ADDRESS] (default ":9201")
  -web.telemetry-path string
        Address to listen on for web endpoints. [TIMESCALE_PROMPGADAPTER_WEB.TELEMETRY_PATH] (default "/metrics")

However, Linux shells don't support environment variable names with . in them. Case in point:

$ export TIMESCALE_PROMPGADAPTER_WEB.TELEMETRY_PATH="/metrics-text"
-bash: export: `TIMESCALE_PROMPGADAPTER_WEB.TELEMETRY_PATH=/metrics-text': not a valid identifier

See the following StackExchange article on why this doesn't work on a shell: https://unix.stackexchange.com/questions/93532/exporting-a-variable-with-dot-in-it

How would you like to proceed with this? Changing the dots (.) to underscores (_) or dashes (-) would do the trick, but not sure if you want to mess up that standard.