[VOTE] Environment variable expansion in configuration files

[Björn Rabenstein]

Prometheus issue #2357 might very well be the one with the most emoji
reactions throughout the whole Prometheus GitHub org. Even if we take
into account that a considerate amount of trolling might be happening
there, it’s quite obvious that a lot of users would like to see
environment variable expansion in configuration files.

However, popular demand alone is not sufficient to introduce a
feature. If the main developers of the codebase think it’s a bad idea,
it ain’t gonna happen. The understanding so far has been that the
topic was discussed in the past, resulting in the members of the
Prometheus team reaching consensus to not introduce environment
variable expansion in configuration files. With no team member raising
an explicit objection, the consensus holds, as defined in the
governance document. However, objections are still being raised by
members of the wider user community, including implications that the
rejection is imposed by selected few rather than the whole team.
Therefore, the recent Prometheus dev summit decided to revisit the
discussion and then make the decision explicit by a majority vote as
described in the governance document.

The best place to read up on the discussion so far is the GH issue
mentioned above.

My attempt at a summary (not only from the issue, but also from other sources):

The main arguments against environment variable expansion in
configuration files:

There should be no ambiguity how to feed configuration into
Prometheus. Reloadable configuration happens in config files,
non-reloadable configuration happens on the command line. Environment
variable expansion in configuration files would open an additional
channel, making configuration more complex, and possibly opening the
floodgates of requesting more and more ways of configuration, slowly
turning Prometheus into a configuration management system.
It is very tempting to use environment variables for secrets. That’s a
very problematic practice, and Prometheus should not encourage it.
Users who really need environment variable expansion can choose from a
number of options to preprocess config files before they are read by
Prometheus. See this simple example on the RP blog.
Environment variable expansion in configuration files can be messy in
practice. There is no general consensus about the syntax: Some prefer
the usual shell syntax ($FOO, ${FOO}, like the popular envsubst tool),
others use a different syntax (e.g. $(FOO) like prominently
Kubernetes). Any syntax might collide with syntactic constructs that
are not meant as variables (happens easily in regular expressions),
and there is again no consensus about escaping options, if they exist
at all.

The main arguments for environment variable expansion in configuration
files (in corresponding order):

Feeding configuration via environment variables is such a common
pattern that its support by Prometheus would not justify the
introduction of other, way less common config mechanisms. It is
natively supported by projects close to Prometheus like Kubernetes and
Thanos (the linked code ironically creates a config file with
variables expanded for consumption by Prometheus).
Prometheus already knows which fields in the config file are secrets.
Environment variable expansion could be disabled for those fields to
not encourage putting secrets in environment variables. (Obviously,
some users want the environment variable expansion precisely for
secrets. They won’t be happy about such a restriction. See voting
options below.)
Even if work-arounds are feasible, many users don’t want to deploy
Prometheus in a special way, creating inconsistency with many other
deployments where they don’t need any work-around.

I hereby call a vote on the following three alternatives:

Keep the status quo, no environment variable expansion in configuration files.
Implement environment variable expansion in configuration files as an
opt-in feature, to be activated via a command-line flag (so that it
doesn’t become a breaking change for existing config files).
Like 2. but environment variable expansion will not happen for fields
that are marked as secrets.

To give enough time for any necessary discussion and to not collide
with the holiday season, the vote will close on 2020-01-06, end of day
UTC.

Note that everybody may participate in the discussion but only
Prometheus team members can vote. According to our governance, the
vote has to be yes or no on each of the three alternatives.

Finally note that this proposal intentionally leaves out
implementation details (which syntax to use, how escaping should work,
which parts of the config files variable expansion should apply to,
how to display the config on the status page, …). We don’t need to
work these out if the vote confirms the rejection of variable
expansion in configuration files. If, however, the vote triggers an
implementation of the feature, and then the devil turns out to be in
the details, we can still rollback by another vote.

--
Björn Rabenstein
[PGP-ID] 0x851C3DA17D748D03
[email] bjo...@rabenste.in

[Krasimir Georgiev]

> Implement environment variable expansion in configuration files as an 

opt-in feature, to be activated via a command-line flag (so that it 

doesn’t become a breaking change for existing config files). 

I don't understand how is this a breaking change without an explicit flag?

Krasi Georgiev Senior Software Engineer Monitoring Team

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAMrVKsyx9fXZsLLBD8jX%2B02MOL_J%2BLkWr0t3BQY4U4Qn%3DJLUgw%40mail.gmail.com.

[Julien Pivotto]

On 18 Dec 10:40, Krasimir Georgiev wrote:
> > Implement environment variable expansion in configuration files as an
> opt-in feature, to be activated via a command-line flag (so that it
> doesn’t become a breaking change for existing config files).
>
> I don't understand how is this a breaking change without an explicit flag?
> Krasi Georgiev Senior Software Engineer
> Monitoring Team

Because you could have e.g. an external label called $MYSYTE for
example. Or, target with labels "$me"..

--
(o- Julien Pivotto
//\ Open-Source Consultant
V_/_ Inuits - https://www.inuits.eu

[Brian Brazil]

On Wed, 18 Dec 2019 at 08:44, Julien Pivotto <roidel...@inuits.eu> wrote:

On 18 Dec 10:40, Krasimir Georgiev wrote:
> > Implement environment variable expansion in configuration files as an
> opt-in feature, to be activated via a command-line flag (so that it
> doesn’t become a breaking change for existing config files).
>
> I don't understand how is this a breaking change without an explicit flag?
> Krasi Georgiev Senior Software Engineer
> Monitoring Team

Because you could have e.g. an external label called $MYSYTE for
example. Or, target with labels "$me"..

Or a bit more plausibly, a relabelling replacement of "$1:9100" or "${1}:9100".

This goes to the argument that templating systems (in the broadest meaning of the term) don't nest well. Even when escaping is properly defined, it's tricky for a user to get things right.

--

Brian Brazil

www.robustperception.io

[Krasimir Georgiev]

Just re-read the whole thread and it seems that everyone has come up with very simple workarounds.

Krasi Georgiev Senior Software Engineer Monitoring Team

[Julien Pivotto]

On 18 Dec 01:54, Björn Rabenstein wrote:

Should we add another alternative which would be usable only in relabeling?

relabel_configs:
- source_env: [HOSTNAME]
target_label: hostname
replacement: $1

[Brian Brazil]

On Wed, 18 Dec 2019 at 23:43, Julien Pivotto <roidel...@inuits.eu> wrote:

On 18 Dec 01:54, Björn Rabenstein wrote:

Should we add another alternative which would be usable only in relabeling?

relabel_configs:
- source_env: [HOSTNAME]
  target_label: hostname
  replacement: $1

That'd be better served by external labels, and even if you wanted it as a target label you wouldn't need an extra way to do things here as you could put $HOSTNAME in the replacement, as you would any other constant string.

I'd expect anything in this space to be pretty dumb texttual substitution, without special casing anything (other than secrets, depending on the vote outcome) or adding new config fields.

-- 

Brian Brazil

www.robustperception.io

[Matt Layher]

> Keep the status quo, no environment variable expansion in configuration files.

YES

> Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files).

NO

> Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

NO

- Matt

[Tristan Colgate]

Personally I think this is a really bad idea, there are very minimal shells ,with here doc support, that make this trivial to implement.

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/388e1c13-2b1f-4575-8ec4-487bc6e9e952%40googlegroups.com.

[Björn Rabenstein]

My original mail was carefully markdown-formatted, but I messed up sending it out as HTML mail after rendering it as such. I only noticed that now. In case it helps, see the properly formatted text below, with handy links and such…

Prometheus issue #2357 might very well be the one with the most emoji reactions throughout the whole Prometheus GitHub org. Even if we take into account that a considerate amount of trolling might be happening there, it’s quite obvious that a lot of users would like to see environment variable expansion in configuration files.

However, popular demand alone is not sufficient to introduce a feature. If the main developers of the codebase think it’s a bad idea, it ain’t gonna happen. The understanding so far has been that the topic was discussed in the past, resulting in the members of the Prometheus team reaching consensus to not introduce environment variable expansion in configuration files. With no team member raising an explicit objection, the consensus holds, as defined in the governance document. However, objections are still being raised by members of the wider user community, including implications that the rejection is imposed by selected few rather than the whole team. Therefore, the recent Prometheus dev summit decided to revisit the discussion and then make the decision explicit by a majority vote as described in the governance document.

The best place to read up on the discussion so far is the GH issue mentioned above.

My attempt at a summary (not only from the issue, but also from other sources):

The main arguments against environment variable expansion in configuration files:

1. There should be no ambiguity how to feed configuration into Prometheus. Reloadable configuration happens in config files, non-reloadable configuration happens on the command line. Environment variable expansion in configuration files would open an additional channel, making configuration more complex, and possibly opening the floodgates of requesting more and more ways of configuration, slowly turning Prometheus into a configuration management system.
2. It is very tempting to use environment variables for secrets. That’s a very problematic practice, and Prometheus should not encourage it.
3. Users who really need environment variable expansion can choose from a number of options to preprocess config files before they are read by Prometheus. See this simple example on the RP blog.

1. Environment variable expansion in configuration files can be messy in practice. There is no general consensus about the syntax: Some prefer the usual shell syntax ($FOO, ${FOO}, like the popular envsubst tool), others use a different syntax (e.g. $(FOO) like prominently Kubernetes). Any syntax might collide with syntactic constructs that are not meant as variables (happens easily in regular expressions), and there is again no consensus about escaping options, if they exist at all. Yet another point of divergence is the handling of undefined variables: Error out, expand to the empty string, or leave it unexpanded?

The main arguments for environment variable expansion in configuration files (in corresponding order):

1. Feeding configuration via environment variables is such a common pattern that its support by Prometheus would not justify the introduction of other, way less common config mechanisms. It is natively supported by projects close to Prometheus like Kubernetes and Thanos (the linked code ironically creates a config file with variables expanded for consumption by Prometheus).
2. Prometheus already knows which fields in the config file are secrets. Environment variable expansion could be disabled for those fields to not encourage putting secrets in environment variables. (Obviously, some users want the environment variable expansion precisely for secrets. They won’t be happy about such a restriction. See voting options below.)
3. Even if work-arounds are feasible, many users don’t want to deploy Prometheus in a special way, creating inconsistency with many other deployments where they don’t need any work-around.

I hereby call a vote on the following three alternatives:

1. Keep the status quo, no environment variable expansion in configuration files.
2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files).
3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

To give enough time for any necessary discussion and to not collide with the holiday season, the vote will close on 2020-01-06, end of day UTC.

Note that everybody may participate in the discussion but only Prometheus team members can vote. According to our governance, the vote has to be yes or no on each of the three alternatives.

Finally note that this proposal intentionally leaves out implementation details (which syntax to use, how escaping should work, which parts of the config files variable expansion should apply to, how to display the config on the status page, …). We don’t need to work these out if the vote confirms the rejection of variable expansion in configuration files. If, however, the vote triggers an implementation of the feature, and then the devil turns out to be in the details, we can still rollback by another vote.

[Bjoern Rabenstein]

On 18.12.19 08:56, Brian Brazil wrote:
>
> This goes to the argument that templating systems (in the broadest meaning of
> the term) don't nest well. Even when escaping is properly defined, it's tricky
> for a user to get things right.

Which could be used as an argument _for_ the expansion of env
variables as that at least leaves everything to one templating system
(the one included in Prometheus) rather than forcing the user to nest
two templating systmes (e.g. `envsubst` and Prometheus).

[Bjoern Rabenstein]

On 19.12.19 00:43, Julien Pivotto wrote:
>
> Should we add another alternative which would be usable only in relabeling?
>
> relabel_configs:
> - source_env: [HOSTNAME]
> target_label: hostname
> replacement: $1

I agree with what the others said about it. It adds a new option to
the config file, which creates cognitive load for those that don't
need the feature. This being an example better suited for external
labels is a hint that we would need those additional config options in
more than just relabeling.

(In different news: I think it makes sense to have the variable
expansions only in certain fields and not in the full text of the
config file, but we can have that discussion should we decide to have
a variable expansion feature at all.)

[Bjoern Rabenstein]

Resending my mail with the proper formatting also exposed that the
first mail I sent contained an earlier version of my draft (thanks to
Gmail's diff-based coloring).

Just for the record: The formatted version had added another sub-item
about variable expansion being poorly standardized: “Yet another point

of divergence is the handling of undefined variables: Error out,

expand to the empty string, or leave it unexpanded?”

Sorry for all the mess...

[Brian Brazil]

On Thu, 19 Dec 2019 at 18:14, Bjoern Rabenstein <bjo...@rabenste.in> wrote:

On 18.12.19 08:56, Brian Brazil wrote:
>
> This goes to the argument that templating systems (in the broadest meaning of
> the term) don't nest well. Even when escaping is properly defined, it's tricky
> for a user to get things right.

Which could be used as an argument _for_ the expansion of env
variables as that at least leaves everything to one templating system
(the one included in Prometheus) rather than forcing the user to nest
two templating systmes (e.g. `envsubst` and Prometheus).

It'd still be layering two templating/interpolation systems: one for regex replacement and the other for env vars. I don't believe anyone is suggesting adding something sophisticated enough to Prometheus so as to have the escaping and other text manipulation functions required for that to begin to be sane.

--

Brian Brazil

www.robustperception.io

[Bjoern Rabenstein]

On 19.12.19 18:31, Brian Brazil wrote:
> On Thu, 19 Dec 2019 at 18:14, Bjoern Rabenstein <bjo...@rabenste.in> wrote:
>
> On 18.12.19 08:56, Brian Brazil wrote:
> >
> > This goes to the argument that templating systems (in the broadest
> meaning of
> > the term) don't nest well. Even when escaping is properly defined, it's
> tricky
> > for a user to get things right.
>
> Which could be used as an argument _for_ the expansion of env
> variables as that at least leaves everything to one templating system
> (the one included in Prometheus) rather than forcing the user to nest
> two templating systmes (e.g. `envsubst` and Prometheus).
>
>
> It'd still be layering two templating/interpolation systems: one for regex
> replacement and the other for env vars.

That's splitting hairs. With that view, every templating system that
expands more than one class of things is multiple templating systems
nested into each other.

> I don't believe anyone is suggesting adding something
> sophisticated enough to Prometheus so as to have the escaping and
> other text manipulation functions required for that to begin to be
> sane.

Should the feature request get accepted, I would suggest exactly
that. Conveniently, the Go `regexp` package already has escaping for
`$`, and the syntax follows that of shell variables. We only need to
define a preference for name collisions.

But I'm getting ahead of myself. We only really need to discuss this
if the feature request is accepted.

[Callum Styan]

1. Keep the status quo, no environment variable expansion in configuration files.

 Yes 

2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files).

no 

3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

no 

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/20191219184547.GV3356%40jahnn.

[Brian Brazil]

On Thu, 19 Dec 2019 at 18:45, Bjoern Rabenstein <bjo...@rabenste.in> wrote:

On 19.12.19 18:31, Brian Brazil wrote:
> On Thu, 19 Dec 2019 at 18:14, Bjoern Rabenstein <bjo...@rabenste.in> wrote:
>
>     On 18.12.19 08:56, Brian Brazil wrote:
>     >
>     > This goes to the argument that templating systems (in the broadest
>     meaning of
>     > the term) don't nest well. Even when escaping is properly defined, it's
>     tricky
>     > for a user to get things right.
>
>     Which could be used as an argument _for_ the expansion of env
>     variables as that at least leaves everything to one templating system
>     (the one included in Prometheus) rather than forcing the user to nest
>     two templating systmes (e.g. `envsubst` and Prometheus).
>
>
> It'd still be layering two templating/interpolation systems: one for regex
> replacement and the other for env vars.

That's splitting hairs. With that view, every templating system that
expands more than one class of things is multiple templating systems
nested into each other.

I don't disagree, the point is more is that the multiple levels of escaping tends to be fun no matter how you categorise them.

An actual templating language can make that largely sane as you say, but an application like Prometheus isn't the place for that.

Such a language belongs as part of configuration management tooling, and everything below that should be plain data structures as far as is practical from my experience.

 

> I don't believe anyone is suggesting adding something
> sophisticated enough to Prometheus so as to have the escaping and
> other text manipulation functions required for that to begin to be
> sane.

Should the feature request get accepted, I would suggest exactly
that. Conveniently, the Go `regexp` package already has escaping for
`$`, and the syntax follows that of shell variables. We only need to
define a preference for name collisions.

I'm not sure if you're talking about a minimal correct implementation of variable substitution (which I'd imagine we'd have similar thoughts on), or something more than that which would expose a language containing things like an escape_regex function to users.

I think any implementation in this area shouldn't be guided by what functions are to hand, rather find some syntax that's the least likely to clash and have some form of correct escaping on offer for when a clash does happen.

Going beyond that is starting to create an actual templating language, rather than "mere" substitution.

Keep in mind that AM configuration and rules files (presuming we ended up adding it to at least one of those) have Go templating exposed to users, so there's more than regexes to consider for escaping. Our rule/notification templates can also themselves contain regexes. This also goes beyond Go, I have had two requests for environment variables for the JMX exporter (though both had a target label use case).

--

Brian Brazil

www.robustperception.io

[Łukasz Mierzwa]

On Wednesday, 18 December 2019 00:54:27 UTC, Björn Rabenstein wrote:

Implement environment variable expansion in configuration files as an
opt-in feature, to be activated via a command-line flag (so that it
doesn’t become a breaking change for existing config files).
Like 2. but environment variable expansion will not happen for fields
that are marked as secrets.

It sounds like a useful feature, but is expanding env variables really enough? It feels like an extreemly simplified case that works for small configuration files with no arrays and it limits what you can actually do. Plus it can be easily achived with tools like https://github.com/docker-infra/reefer.
Isn't there a risk that once implemented it will encourage more feature request to implement even more complete templating syntax?

[Julien Pivotto]

The use case is really expanding $HOSTNAME and other small vars. Not
passing entire config from the env.

[Brian Brazil]

That's basically my argument. If you get to the stage where this comes up, then you already need some form of configuration management tool as you're dealing with some sort of setup where you've already multiple clusters/teams/environments/deployments/something. I see features like this as a bandaid at best for the lack of appropriate tooling in such a scenario.

A lack of such tooling is only something you can get away with when dealing with very low single digits of "things", whereby editing everything by hand (in some form of source control) without any templating or tooling is sufficient configuration management. And if you're already editing things by hand, why not this too?

--

Brian Brazil

www.robustperception.io

[Brian Brazil]

As far as I'm aware the only use case for using in $HOSTNAME is when Prometheus is running as a StatefulSet on Kubernetes, as the hostnames in that case are user determined and static. Thus they can be appropriate for use in external label values (though they may need some trimming first).

If you've already such a non-trivial setup, then you could for example have whatever is producing all that YAML produce a config map per Prometheus, or have it insert a bit of shell to run before Prometheus does to do the substitution, or use a container image that already has that bit of shell, or have your config reloader (k8 can't send a HUP when a config map changes, so you basically have to have a separate config reloader) do substitution - which the Prometheus Operator and Thanos already support, so even if you aren't using the Operator you can borrow their reloader if you need reloads, or find a single container image that runs Prometheus and also does both k8 reloading and the substitution (I'm not aware of one of these existing yet, but wouldn't be difficult to do).

Those would take a varying amount of effort depending on how good your tooling is and how your configuration is factored, but in general I'd expect that inserting shell/choosing a different docker image would be about the same effort as passing the additional command line flag that'd be required to enable this feature. So I don't see how you'd actually be saving anything by having this feature inside Prometheus itself, when you compare to the various alternatives already available for this use case.

--

Brian Brazil

www.robustperception.io

[Łukasz Mierzwa]

On Friday, 20 December 2019 09:52:57 UTC, Julien Pivotto wrote:

The use case is really expanding $HOSTNAME and other small vars. Not
passing entire config from the env.

 I *think* env variables are most commonly related to containers and container ecosystem does have a way of dealing with such problems.
Are there use cases where someone runs Prometheus without a container, yet feeds _some_ config options by setting env variables rather then editing configuration?

Maybe this problem (if it's really only related to docker) could be solved by providing an official image that includes some tool like https://github.com/docker-infra/reefer, so users who want to have such simplified env variable expansion can pull that one?

[Brian Brazil]

On Sat, 21 Dec 2019 at 10:32, Łukasz Mierzwa <l.mi...@gmail.com> wrote:

On Friday, 20 December 2019 09:52:57 UTC, Julien Pivotto wrote:

The use case is really expanding $HOSTNAME and other small vars. Not
passing entire config from the env.

 I *think* env variables are most commonly related to containers and container ecosystem does have a way of dealing with such problems.
Are there use cases where someone runs Prometheus without a container, yet feeds _some_ config options by setting env variables rather then editing configuration?

Offhand the only one mentioned in the various threads was HTTP_PROXY.

Maybe this problem (if it's really only related to docker) could be solved by providing an official image that includes some tool like https://github.com/docker-infra/reefer, so users who want to have such simplified env variable expansion can pull that one?

I'm not sure we should provide something like that officially (it being on the Prometheus project to provide every potential combination of Prometheus+Tool as a docker image isn't scalable), but given the interest in this feature it shouldn't be hard to find someone in the community to provide such a thing as part of the broader ecosystem.

--

Brian Brazil

www.robustperception.io

[Goutham Veeramachaneni]

I've just gone through the entire Github issue and noted the usecases that were reported were environment variables will help: external_labels, http_proxy, remote_write_urls, and secrets (mainly secrets?).

What irks me is that $SOMETHING is a legit thing to use in the config. Like one can have an external label whose value is "$APP_01". But I am also super sympathetic to those who want to sent an external label via environment variables. Infact we have 2 different statefulsets with two different configmaps so that we have different labels in the HA pairs. We have a templating system that makes this trivial, but I don't think it is the case with others.

Having said that, I think the Prometheus operator should be enough for most usecases on k8s and it handles everything mentioned above easily. For those who are looking to do things directly, they could always have a init container / sidecar from operator to achieve the same. It is additional complexity, but the alternative is the operator which just works.

I am not super sure about the non-container usecases. But from my very cursory look adding a envsubst preprocessing step looks like just a few lines of code in ansible/chef/puppet. I don't think the overhead for the operators is too high. Finally, regarding secrets, given how we already have the basic auth secrets pointing to a file, can we have the other secrets in files too? Would that be sufficient to handle the secrets usecase?

Given how I cannot see a sane way to do the substitution, specially in the external labels usecase, I am drifting towards a no for supporting it. I will cast my vote after a few days, once I have done more research. But in the meantime, could folks help me see a sane way to satisfy the external labels usecase?

Thanks,

Goutham.

[Brian Brazil]

On Sun, 29 Dec 2019 at 16:12, 'Goutham Veeramachaneni' via Prometheus Developers <prometheus...@googlegroups.com> wrote:

I've just gone through the entire Github issue and noted the usecases that were reported were environment variables will help: external_labels, http_proxy, remote_write_urls, and secrets (mainly secrets?).

What irks me is that $SOMETHING is a legit thing to use in the config. Like one can have an external label whose value is "$APP_01". But I am also super sympathetic to those who want to sent an external label via environment variables. Infact we have 2 different statefulsets with two different configmaps so that we have different labels in the HA pairs. We have a templating system that makes this trivial, but I don't think it is the case with others.

Having said that, I think the Prometheus operator should be enough for most usecases on k8s and it handles everything mentioned above easily. For those who are looking to do things directly, they could always have a init container / sidecar from operator to achieve the same. It is additional complexity, but the alternative is the operator which just works.

I am not super sure about the non-container usecases. But from my very cursory look adding a envsubst preprocessing step looks like just a few lines of code in ansible/chef/puppet. I don't think the overhead for the operators is too high.

If you're using ansible/chef/puppet you shouldn't even need envsubst, the in-built templating will generally cover your use case.

Finally, regarding secrets, given how we already have the basic auth secrets pointing to a file, can we have the other secrets in files too?

We have that for bearer tokens and TLS too. Basic auth was actually added the last of all those.

Would that be sufficient to handle the secrets usecase?

It's important to distinguish between not checking secrets into source control, and not having secrets in the config file that Prometheus reads. 

For not checking secrets into source control, I believe that substituting in the secret is the duty of configuration management to handle before the file gets to Prometheus.

Given that, having secrets in a separate file only makes sense where there's some form of automatic secret rotation going on that's not tied to the general Prometheus reload lifecycle. Bearer tokens on k8 are the canonical example of this, when token changes we want to use start using it on the next http request. Otherwise you're not gaining anything, as the secrets are still on the filesystem no matter which specific file they're in. I'm not aware of any requests for secrets files due to secrets changing underneath us beyond what we already have, but if they come up we can evaluate them on a case-by-case basis as usual.

The same logic and arguments apply to AM, but it's more complicated due to routing and templating. Thankfully all those secrets are very very static, so we can ignore that rabbit hole.

Brian

Given how I cannot see a sane way to do the substitution, specially in the external labels usecase, I am drifting towards a no for supporting it. I will cast my vote after a few days, once I have done more research. But in the meantime, could folks help me see a sane way to satisfy the external labels usecase?

Thanks,

Goutham.

On Saturday, December 21, 2019 at 1:05:03 PM UTC, Brian Brazil wrote:

On Sat, 21 Dec 2019 at 10:32, Łukasz Mierzwa <l.mi...@gmail.com> wrote:

On Friday, 20 December 2019 09:52:57 UTC, Julien Pivotto wrote:

The use case is really expanding $HOSTNAME and other small vars. Not
passing entire config from the env.

 I *think* env variables are most commonly related to containers and container ecosystem does have a way of dealing with such problems.
Are there use cases where someone runs Prometheus without a container, yet feeds _some_ config options by setting env variables rather then editing configuration?

Offhand the only one mentioned in the various threads was HTTP_PROXY.

Maybe this problem (if it's really only related to docker) could be solved by providing an official image that includes some tool like https://github.com/docker-infra/reefer, so users who want to have such simplified env variable expansion can pull that one?

I'm not sure we should provide something like that officially (it being on the Prometheus project to provide every potential combination of Prometheus+Tool as a docker image isn't scalable), but given the interest in this feature it shouldn't be hard to find someone in the community to provide such a thing as part of the broader ecosystem.

--

Brian Brazil

www.robustperception.io

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/98e8037e-2ea7-4354-8b18-527a481de411%40googlegroups.com.

--

Brian Brazil

www.robustperception.io

[Matthias Rampke]

1. Keep the status quo, no environment variable expansion in configuration files.

YES

2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files)

NO

3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

NOOO

In detail:

1. I think the status quo is a perfectly viable situation. As a long-running service with dynamic configuration, Prometheus requires the environment around it to be able to supply configuration anyway.

2. I could live with this but I think it would cause too many awkward mismatches: environment variables can only change on restart, while the configuration itself is dynamically loaded. "Why didn't this change?" is going to be a problem here. The expansion syntax colliding with regex references is another one like this, although we could lean into this ambiguity (use the same syntax for env variables + named captures with clear precedence rules).

3. We should not patronize users only because a pattern that was the preferred way until recently has fallen out of fashion.

/MR

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLqYXJk3MkTRZ63srdEj%3DmBF4mJwZE6Gh79z%2BcmqPicRSw%40mail.gmail.com.

[Bjoern Rabenstein]

On 29.12.19 19:55, 'Matthias Rampke' via Prometheus Developers wrote:
> The expansion syntax colliding with regex references is
> another one like this, although we could lean into this ambiguity (use the same
> syntax for env variables + named captures with clear precedence rules).

Yeah, this is an interesting point. I hinted in that direction
earlier.

Essentially, all the valid concerns about escaping and ambiguity
issues are ironically swaying me towards _supporting_ env variable
expansion, mostly because that gives us the possibility to limit it to
certain fields and define sane escaping and precedence rules, like the
one you have mentioned above for regexp captures. Also note that the
regexp templating already has a way to create a literal "$" by using
"$$" in the template. We could use the same consistently for the
external labels, i.e. "$APPNAME" would expand to the environment
variable, while "$$APPNAME" would resust in a literal "$APPNAME" in
the label value.

_However_, a solution like
https://www.robustperception.io/environment-substitution-with-docker
will ruthlessly expand everything in the files fed to `envsubst`, so
now you have the escaping problem _everywhere_, not just in particular
fields on which a built-in variable expansion would be acting on
selectively and in a well defined manner.

`envsubst` in particular, just as a side note, doesn't even have any
escaping mechanism, which is a common problem with hacky
work-arounds (like defining a `DOLLAR='$'` variable and such).

[Matthias Rampke]

I think envsubst may not be a particularly good solution. In my mental model of the problem, the base solution that always works is a POSIX shell script with a heredoc. The escaping on that is not particularly pretty but it is well defined.

/MR

[Brian Brazil]

On Sun, 29 Dec 2019 at 23:27, 'Matthias Rampke' via Prometheus Developers <prometheus...@googlegroups.com> wrote:

I think envsubst may not be a particularly good solution. In my mental model of the problem, the base solution that always works is a POSIX shell script with a heredoc. The escaping on that is not particularly pretty but it is well defined.

It looks like you can whitelist which variables to substitute. The docs aren't the clearest, echo '$PWD ${USER} ${PATH}' | envsubst '$PWD ${USER}' shows the behaviour. The relevant point is more that there are a selection of options out there.

Brian

 

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5VBBBSfs5TW4OgEmExhdtRc-BT3BPWwGXLBAan7fYkB8A%40mail.gmail.com.

--

Brian Brazil

www.robustperception.io

[Simon Pasquier]

1. Keep the status quo, no environment variable expansion in
configuration files.
yes

2. Implement environment variable expansion in configuration files as

an opt-in feature, to be activated via a command-line flag (so that it
doesn’t become a breaking change for existing config files).

no

3. Like 2. but environment variable expansion will not happen for

fields that are marked as secrets.

no

> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAMrVKszdOaHPLkKTQ8d1x4a2nRdaFT_YBDauGjCpg36v6zRx5w%40mail.gmail.com.

[Chris Marchbanks]

1. Keep the status quo, no environment variable expansion in

configuration files.

Yes

2. Implement environment variable expansion in configuration files as
an opt-in feature, to be activated via a command-line flag (so that it
doesn’t become a breaking change for existing config files).

No

3. Like 2. but environment variable expansion will not happen for
fields that are marked as secrets.

No

That said, I understand the frustration around not being able to configure
certain fields via environment variables, especially external labels. I don't

think allowing env var expansion is the correct way forward, but I could be

open to allowing env vars to configure very specific fields such as external

labels separately from expansion. Something like how Grafana has very

specific syntax for using env vars, but do not expand them in their config.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAMrVKsyx9fXZsLLBD8jX%2B02MOL_J%2BLkWr0t3BQY4U4Qn%3DJLUgw%40mail.gmail.com.

[Julien Pivotto]

On 18 Dec 08:56, Brian Brazil wrote:
> On Wed, 18 Dec 2019 at 08:44, Julien Pivotto <roidel...@inuits.eu> wrote:

>
> > On 18 Dec 10:40, Krasimir Georgiev wrote:
> > > > Implement environment variable expansion in configuration files as an
> > > opt-in feature, to be activated via a command-line flag (so that it
> > > doesn’t become a breaking change for existing config files).
> > >

> > > I don't understand how is this a breaking change without an explicit
> > flag?
> > > Krasi Georgiev Senior Software Engineer
> > > Monitoring Team
> >
> > Because you could have e.g. an external label called $MYSYTE for
> > example. Or, target with labels "$me"..
>
>
> Or a bit more plausibly, a relabelling replacement of "$1:9100" or
> "${1}:9100".

Possibly using ${env.HOSTNAME} would be a harmless way to introduce this
without adding an extra flag.

[Brian Brazil]

That's still a valid string though currently, so there could be a collision (maybe you're using that as label value to signal out to an AM webhook?)

--

Brian Brazil

www.robustperception.io

[Bartłomiej Płotka]

Thanks for bringing this up. My vote, (along the lines what Chris mentioned):

4. Implement environment variable expansion ONLY for the external labels section (and potentially only for label values) as it is a very common use case like replica label when running Prometheus in HA when running on K8s. There is so collision issue there as label value has a strict format. 

Kind Regards,

Bartek

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLqcSVJtW0Z4FROErbN2hwyHSxGaqWV326hOzDz69kNsXw%40mail.gmail.com.

[Brian Brazil]

On Mon, 6 Jan 2020 at 10:32, Bartłomiej Płotka <bwpl...@gmail.com> wrote:

Thanks for bringing this up. My vote, (along the lines what Chris mentioned):

It's not clear how you're voting on the 3 options, so this is not a valid vote. Can you clarify?

4. Implement environment variable expansion ONLY for the external labels section (and potentially only for label values) as it is a very common use case like replica label when running Prometheus in HA when running on K8s. There is so collision issue there as label value has a strict format. 

Label values are full UTF-8, so there can be collisions with any potential syntax. The string "$HOSTNAME" is a perfectly valid label value for example.

I think it's also a bit weird to single out one particular field for one particular deployment model, why this one and not all the others?

Brian

--

Brian Brazil

www.robustperception.io

[Frederic Branczyk]

1. Keep the status quo, no environment variable expansion in configuration files.

yes

2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files)

no

3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

no

---

While I understand the desire for these things, I agree with the general sentiment of this thread, taking a particular opinion here is only going to help those whose system is opinionated in the same way. I do believe that having the canonical source of configuration be the config file and the config file only in Prometheus is a great benefit, and the more I work with it, the more I wish all systems work that way. 

On Monday, 6 January 2020 11:41:50 UTC+1, Brian Brazil wrote:

On Mon, 6 Jan 2020 at 10:32, Bartłomiej Płotka <bwpl...@gmail.com> wrote:

Thanks for bringing this up. My vote, (along the lines what Chris mentioned):

It's not clear how you're voting on the 3 options, so this is not a valid vote. Can you clarify?

4. Implement environment variable expansion ONLY for the external labels section (and potentially only for label values) as it is a very common use case like replica label when running Prometheus in HA when running on K8s. There is so collision issue there as label value has a strict format. 

Label values are full UTF-8, so there can be collisions with any potential syntax. The string "$HOSTNAME" is a perfectly valid label value for example.

I think it's also a bit weird to single out one particular field for one particular deployment model, why this one and not all the others?

Brian

 

Kind Regards,

Bartek

On Mon, 6 Jan 2020 at 10:26, Brian Brazil <brian...@robustperception.io> wrote:

On Mon, 6 Jan 2020 at 10:15, Julien Pivotto <roidel...@inuits.eu> wrote:

On 18 Dec 08:56, Brian Brazil wrote:
> On Wed, 18 Dec 2019 at 08:44, Julien Pivotto <roidel...@inuits.eu> wrote:
>
> > On 18 Dec 10:40, Krasimir Georgiev wrote:
> > > > Implement environment variable expansion in configuration files as an
> > > opt-in feature, to be activated via a command-line flag (so that it
> > > doesn’t become a breaking change for existing config files).
> > >
> > > I don't understand how is this a breaking change without an explicit
> > flag?
> > > Krasi Georgiev Senior Software Engineer
> > > Monitoring Team
> >
> > Because you could have e.g. an external label called $MYSYTE for
> > example. Or, target with labels "$me"..
>
>
> Or a bit more plausibly, a relabelling replacement of "$1:9100" or
> "${1}:9100".

Possibly using ${env.HOSTNAME} would be a harmless way to introduce this
without adding an extra flag.

That's still a valid string though currently, so there could be a collision (maybe you're using that as label value to signal out to an AM webhook?)

--

Brian Brazil

www.robustperception.io

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-developers+unsub...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLqcSVJtW0Z4FROErbN2hwyHSxGaqWV326hOzDz69kNsXw%40mail.gmail.com.

--

Brian Brazil

www.robustperception.io

[Bartłomiej Płotka]

Fair point, it's the label name that has strict formatting. 

The reason behind allowing envvar only in those fields is that it limits the problematic edge cases like user want to substitute envvar in once place but not in another.  However given the number of issues here it looks like separate tooling is the best option, so:

1. Keep the status quo, no environment variable expansion in configuration files: yes

2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files): no

3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets: no

Kind Regards,

Bartek

Kind Regards,

Bartek

To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLqcSVJtW0Z4FROErbN2hwyHSxGaqWV326hOzDz69kNsXw%40mail.gmail.com.

--

Brian Brazil

www.robustperception.io

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/618207e7-2035-4972-b308-4a2f0b89780f%40googlegroups.com.

[Goutham Veeramachaneni]

1. Keep the status quo, no environment variable expansion in configuration files.

Yes

2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files)

No

3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

No

I do this mainly because I don't see a non-confusing way to implement this. For all the deployment options I saw (puppet, chef, ansible, k8s) I could see easy workarounds. They are either built in or have a sidecar component in the wild that people could use.

Thanks

Goutham

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAMssQwaxjDAyNjNcyN-UQq1jEqhbHjVPc3MsFGKzdPZLT6t76A%40mail.gmail.com.

[Johannes Ziemke]

I'll vote for 2. (Implement environment variable expansion).

Arguably using puppet/chef/ansible isn't real "cloud native" and I haven't been using these for greenfield things since ~5 years now. Kubernetes doesn't provide means for rendering configuration either. You could argue that's Kubernetes ecosystem's problem but it forces people to use hacks even for minor configuration.

For the record, I've built docker-reefer precisely because of the lack of env var configuration in Prometheus. And it, and similar options, prevent you from using configmaps and auto-reload the config without having to restart Prometheus. In general, I consider it a hack.

If the implicitness of env vars are a concern, we could also not interpolate env vars as they are but support setting config variables on the command line, similar to what terraform and jsonnet is doing: `prometheus -var hostname=$HOSTNAME`.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAK6hLk1PvRsFUt1vDDhqzx9%3D7U-q%2B4bdP8VDRnGPCqT5aT9_Og%40mail.gmail.com.

[Bjoern Rabenstein]

On 06.01.20 13:06, Johannes Ziemke wrote:
> I'll vote for 2. (Implement environment variable expansion).

I'll take that as a "no" for (1) and (3) and a "yes" for (2).

(For better or worth, the governance needs us to say "yes" or "no" for
each individual choice. In principcle, it would be better to just rank
the choices and then use one of the established voting methods to pick
the winner. We discussed this shortly during the governance creation
and then went for the simpler approach we have now, I assume mostly
because the ranking method can create "stalemates", and how to resolve
those is a somewhat contentious topic among voting method researchers.)

[Bjoern Rabenstein]

On 03.01.20 09:18, Chris Marchbanks wrote:
>
> That said, I understand the frustration around not being able to configure
> certain fields via environment variables, especially external labels. I don't
> think allowing env var expansion is the correct way forward, but I could be
> open to allowing env vars to configure very specific fields such as external
> labels separately from expansion. Something like how Grafana has very
> specific syntax for using env vars, but do not expand them in their config.

That works nicely in Grafana because the whole ini-style config file
has only entries that can be addressed with
`<SectionName>_<KeyName>`. That enables the environment variables with
tha naming pattern `GF_<SectionName>_<KeyName>`, see
https://grafana.com/docs/grafana/latest/installation/configuration/#using-environment-variables

It would be more complicated with the Prometheus YAML config file, as
they allow repetition and deeper nesting.

Having said that, I think it makes sense to "whitelist" env var
expansion for very specific fields only. As I said before, this is
superior to external preprocessing of config files because those tools
either don't understand the structure of the Prometheus config files
(acting just on the plain text) or have to know about which fields
they should touch (assuming they understand YAML). Also, we have exact
knowledge then where env var expansion applies and can come up with
preference and escaping that makes sense in that context rather than
letting the user solve that problem in a makeshift way.

[Brian Brazil]

On Mon, 6 Jan 2020 at 14:10, Bjoern Rabenstein <bjo...@rabenste.in> wrote:

On 03.01.20 09:18, Chris Marchbanks wrote:
>
> That said, I understand the frustration around not being able to configure
> certain fields via environment variables, especially external labels. I don't
> think allowing env var expansion is the correct way forward, but I could be
> open to allowing env vars to configure very specific fields such as external
> labels separately from expansion. Something like how Grafana has very
> specific syntax for using env vars, but do not expand them in their config.

That works nicely in Grafana because the whole ini-style config file
has only entries that can be addressed with
`<SectionName>_<KeyName>`. That enables the environment variables with
tha naming pattern `GF_<SectionName>_<KeyName>`, see
https://grafana.com/docs/grafana/latest/installation/configuration/#using-environment-variables

 

It would be more complicated with the Prometheus YAML config file, as
they allow repetition and deeper nesting.

Repetition is why certain Alertmanager configuration was moved from environment variables, many years back.

Having said that, I think it makes sense to "whitelist" env var
expansion for very specific fields only. As I said before, this is
superior to external preprocessing of config files because those tools
either don't understand the structure of the Prometheus config files
(acting just on the plain text) or have to know about which fields
they should touch (assuming they understand YAML). Also, we have exact
knowledge then where env var expansion applies and can come up with
preference and escaping that makes sense in that context rather than
letting the user solve that problem in a makeshift way.

That is indeed a consideration, but also takes us pretty far from dumb textual substitution. That'd end up being a first-class feature in and of itself, with a fair bit of complexity - at that point we're effectively starting to develop the beginnings of something like jsonnet as that's the class of solution you need to properly model such a configuration.

I think there's a variant of Greenspun's tenth rule that applies here in relation to configuration management. Let's leave jsonnet-class problems to jsonnet-class solutions, rather than layering on another templating/interpolation system for the user to fight. I very much agree with Frederic's sentiments that a simple file makes life much easier, having had the experience of attempting to do proper configuration management on top of other approaches over the years.

> For better or worth, the governance needs us to say "yes" or "no" for each individual choice.

It doesn't by my reading of it, however the intention was clear so it's a valid vote anyway imho.

--

Brian Brazil

www.robustperception.io

[Bjoern Rabenstein]

On 19.12.19 19:10, Björn Rabenstein wrote:
> 1. Keep the status quo, no environment variable expansion in configuration
> files.

No.

> 2. Implement environment variable expansion in configuration files as an
> opt-in feature, to be activated via a command-line flag (so that it doesn’t

> become a breaking change for existing config files).

No.

> 3. Like 2. but environment variable expansion will not happen for fields that
> are marked as secrets.

Yes.


Lengthy, multi-faceted explanation:

First of all, I see an issue with the governance-mandating method to
vote for multiple mutually exclusive options. As I've just written in
another mail in this thread: “For better or worth, the governance

needs us to say "yes" or "no" for each individual choice. In
principcle, it would be better to just rank the choices and then use
one of the established voting methods to pick the winner. We discussed
this shortly during the governance creation and then went for the
simpler approach we have now, I assume mostly because the ranking
method can create "stalemates", and how to resolve those is a somewhat

contentious topic among voting method researchers.”

If I ranked the proposals, it would be (3), (1), (2).

I'm still fine with the status quo. I've never felt strongly against
environment variable expansion either, but I've been aware of the many
hairy issues connected to it. Interestingly, the discussion we had on
this thread convinced me to take (3) as my first preference, precisely
_because_ of all the good points brought up about the difficulty of
environment variable expansion. How so?

What I would like to implement concretely is environment variable
expansion only for carefully selected fields (with the option to
extend it to more fields over time, which is possible for as long as
we mark the feature as experimental). With the limited amount of
fields, we can contain the amount of surprises. Creating sane escaping
and preference rules is much easier on a limited set of fields.

Putting myself into the shoes of somebody who needs environment
variable expansion, what would I do in the current situation? I could
restructure my whole deployment system to remove the need for
environment variable expansion, but that's often not an option, as the
required changes are too invasive or the deployment system is not even
under my control (common in larger organizations). I assume most of
the many angry people that have left reactions on issue #2357 are in
exactly that situation. So what do I do? I follow the recommendation
of using `envsubst` or a shell here doc or docker-reefer or one of the
many other more or less simple possibilities. Now all the concerns
brought up against environment variable expansion hit me hard: I have
to come up with escaping rules for the whole file (not just the
white-listed fields where environment variable expansion makes
sense). I am now literally nesting different templating
systems. Nobody keeps me from using the environment variable expansion
for secrets. Also, because of the many possible tools, there won't be
one clearly preferred way, so the amount of people I can share
experiences and solutions with is limited due to fragmentation of the
community.

With the solution I would prefer by now, all of these problems are
circumvented:

- If you don't like environment variable expansion, you just never
activate the flag and completely ignore the whole issue.

- For both users and the Prometheus developers that implement the
environment variable expansion, the blast radius of hairy issues is
limited by carefully picking the fields to be whitelisted. With a
focus on the most needed fields, many users can be made happy with a
reasonable amount of effort.

- The escaping and preference rules will be solved in a reasonably
rule by the Prometheus developers rather than in a makeshift and
error-prone way separately by each user who has to come up with an
external wrapping solution.

- There will be one obviously preferred way of environment variable
expansion (the built-in one), enabling sharing experiences and
solutions.

- I totally get the "patronizing" issue MR mentioned about secrets in
environment variables. My preferred solution will alleviate this
aspect a bit by not stressing the negative aspects of banning secret
fields, but being focused on the positive aspect of allowing
selected few fields (of which (not quite) coincidentally none
happens to be a secret field).

[Bjoern Rabenstein]

Reminder: The vote closes end of today (UTC). No votes have been cast
(to my knowledge) by the following team members:

Ben Kochie
Brian Brazil
Fabian Reinartz
Ganesh Vernekar
Julius Volz
Krasi Georgiev
Max Inden
Richard Hartmann
Steve Durrheimer
Stuart Nelson
Tobias Schmidt
Tom Wilkie

[Johannes Ziemke]

Sorry about that. My vote would be:

1: No

2: Yes (preferred)

3: Yes

(2 more cents on this: While I believe in safe defaults, I'd consider preventing using env vars in secrets nannying. Not all secrets or environments are born equal and until quite recently using secrets in env vars were considered perfectly fine.)

[Julius Volz]

After reading the entire discussion, I'm still not sure. I started out being somewhat in favor of env var support, but now I'm leaning more against it. So I guess:

1) yes

2) no

3) no

But willing to revisit in the future if things change...

On Wed, Dec 18, 2019 at 1:54 AM Björn Rabenstein <bjo...@rabenste.in> wrote:

Prometheus issue #2357 might very well be the one with the most emoji
reactions throughout the whole Prometheus GitHub org. Even if we take
into account that a considerate amount of trolling might be happening
there, it’s quite obvious that a lot of users would like to see

environment variable expansion in configuration files.

Keep the status quo, no environment variable expansion in configuration files.

Implement environment variable expansion in configuration files as an
opt-in feature, to be activated via a command-line flag (so that it
doesn’t become a breaking change for existing config files).

Like 2. but environment variable expansion will not happen for fields
that are marked as secrets.

To give enough time for any necessary discussion and to not collide
with the holiday season, the vote will close on 2020-01-06, end of day
UTC.

Note that everybody may participate in the discussion but only
Prometheus team members can vote. According to our governance, the
vote has to be yes or no on each of the three alternatives.

Finally note that this proposal intentionally leaves out
implementation details (which syntax to use, how escaping should work,
which parts of the config files variable expansion should apply to,
how to display the config on the status page, …). We don’t need to
work these out if the vote confirms the rejection of variable
expansion in configuration files. If, however, the vote triggers an
implementation of the feature, and then the devil turns out to be in
the details, we can still rollback by another vote.

--
Björn Rabenstein
[PGP-ID] 0x851C3DA17D748D03
[email] bjo...@rabenste.in

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAMrVKsyx9fXZsLLBD8jX%2B02MOL_J%2BLkWr0t3BQY4U4Qn%3DJLUgw%40mail.gmail.com.

[Brian Brazil]

>  1. Keep the status quo, no environment variable expansion in configuration
>     files.

Yes

>  2. Implement environment variable expansion in configuration files as an

>     opt-in feature, to be activated via a command-line flag (so that it doesn’t
>     become a breaking change for existing config files).

No

>  3. Like 2. but environment variable expansion will not happen for fields that
>     are marked as secrets.

No.

My preferences would be 1->2->3. 3 is better from a security standpoint, however it would be much more complex than 2. I'm also not a big fan of adding business logic purely to prevent bad practice.

> What I would like to implement concretely is environment variable

expansion only for carefully selected fields (with the option to
extend it to more fields over time, which is possible for as long as
we mark the feature as experimental). With the limited amount of
fields, we can contain the amount of surprises. Creating sane escaping
and preference rules is much easier on a limited set of fields.

This conveniently shows why 3 is complex. A config file format, escaping, and semantics that can have breaking changes from release to release, plus the eternal debates around what fields to allow it for, is a lot of effort and user impact for something that is not a core function. Simple string substitution on the entire file is the most I'd probably do.

> I see an issue with the governance-mandating method to vote for multiple mutually exclusive options.

Yeah, the approval voting is weird. We can always change if it actually matters at some point, hopefully multi-choice votes will continue to be a rare occurrence though.

Brian

--

Brian Brazil

www.robustperception.io

[Tobias Schmidt]

1. Keep the status quo, no environment variable expansion in configuration files.

YES

2. Implement environment variable expansion in configuration files as an opt-in feature, to be activated via a command-line flag (so that it doesn’t become a breaking change for existing config files).

YES

3. Like 2. but environment variable expansion will not happen for fields that are marked as secrets.

NO

---

After reading all of the discussion again the last hour, I'm still very unsure about allowing ENV variables, but I haven't attempted deploying the standard docker container without additional modifications for a long while. I feel this should work out-of-the-box, but then most deployments will likely need some custom code anyway. In order to form a clear opinion, I'd need more time to see the pain points users describe first hand, which sadly I don't have. I don't think the project will suffer from neither implementing nor not implementing ENV support, so I'm voting for both options.

I do feel strongly not to get into a continuous discussions about on which parts of the config to allow the ENV solution. Also, while secrets in environment variables in many environments can be an issue, I don't agree that they are unconditionally dangerous if used correctly, and I don't agree that everyone has to suffer to protect the masses. A NO to option 3.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLoLsctJXx3z-3MBE8pvrx-CcpjO16JF8r-G-8RQX-GGgg%40mail.gmail.com.

[Bjoern Rabenstein]

And here is the result (in ASCII art, fixed-width font required):

.---------- no 11: 2
/ .------ all fields 2:11
| / .-- not in secrets 2:11
| | /
Bartłomiej Płotka Y N N
Ben Kochie
Björn Rabenstein N N Y
Brian Brazil Y N N
Callum Styan Y N N
Chris Marchbanks Y N N
Fabian Reinartz
Frederic Branczyk Y N N
Ganesh Vernekar
Goutham Veeramachaneni Y N N
Johannes Ziemke N Y Y
Julius Volz Y N N
Krasi Georgiev
Matt Layher Y N N
Matthias Rampke Y N N
Max Inden
Richard Hartmann
Simon Pasquier Y N N
Steve Durrheimer
Stuart Nelson
Tobias Schmidt Y Y N
Tom Wilkie

Empty lines are team members that haven't cast a vote. If I have
missed or misread a mail, please let me know.

However, the result is very clear in any case.

[Matthias Rampke]

Thank you Björn for setting up and running the vote, and thanks to everyone for the thoughtful discussion!

/MR

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/20200107004052.GB5133%40jahnn.

[Julius Volz]

Yeah, thanks! It was good to have this discussion in detail again. Remarkable how many people still feel unsure now, but in the end it's still a very clear majority for just keeping the status quo.

I think a lot of times people feel (and emoji-react) negatively on GitHub issues not mainly because they feel super strongly about an issue per se, but because they feel unheard or brushed off too quickly. Having more votes with lengthy, reasoned discussions like here is a good way to counter-act that.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5WYS5YqiU7Zjkv153qSm3pfZ4CwJXqSUP1nV3N%3D0BFY6Q%40mail.gmail.com.

[Bjoern Rabenstein]

On 07.01.20 12:08, Julius Volz wrote:
>
> I think a lot of times people feel (and emoji-react) negatively on GitHub
> issues not mainly because they feel super strongly about an issue per se, but
> because they feel unheard or brushed off too quickly. Having more votes with
> lengthy, reasoned discussions like here is a good way to counter-act that.

My thoughts, too. It would have been much better, though, if more of
the prometheus-team members had voted. 9 of 22 team members did not
cast a vote. I'd feel much better if we could say that an overwhelming
majority of the team members oppose this feature. Now we can only say
that 50% oppose the feature, while most others apparently don't care.

[Julius Volz]

Yeah. TBH I almost didn't vote, not because I don't care, but because I was so unsure. Maybe others felt similar, not sure. 

[Brian Brazil]

True, but it's still a much better turnout than the last public vote.

--

Brian Brazil

www.robustperception.io