How should apply behave under an ENC
76 views
Skip to first unread message
Felix Frank
Oct 5, 2014, 9:30:32 PM10/5/14
to puppe...@googlegroups.com
Hi all,
when we recently fixed a regression that had to do with directory
environments and ENC (PUP-3258) I noticed that the handling is weird
when the ENC actually picks an environment for the local machine.
As far as I can tell, puppet apply ignored that until now. The apply
command used the configured environment, replaced its :manifest setting
with what's chosen on the commandline, and makes that the only available
environment.
https://github.com/puppetlabs/puppet/blob/fea22be6a957e62005cab649537b39af0d0bda74/lib/puppet/application/apply.rb#L185-190
Puppet 3.7.0 is less stringent in that regard. It retains the set of
available environments.
https://github.com/puppetlabs/puppet/blob/master/lib/puppet/application/apply.rb#L193
This apparently causes an error when the ENC returns an environment (any
environment), which happens just below the linked code line in apply.rb.
This environment is looked up, bypassing the override of the :manifest.
I suggested a fix for this: Force the node (external or not) to use the
overridden environment.
https://github.com/ffrank/puppet/commit/39b78a8d25fa96c98f81227cd3ef6b48010fab68
During PR triage (sorry for vanishing suddenly, something came up), we
got some doubts whether this was the right thing to do. It is probably
in keeping with former behavior, but it contradicts a decision that was
made wrt. a classic bug.
http://projects.reductivelabs.com/issues/3910
When using agent, not apply, the puppet master may invoke an ENC. The
environment returned from the classifier takes precedence over any
:environment configuration on the agent (both file and commandline).
This is to allow users to use their ENC in contexts that are relevant
for security.
We're now looking for feedback on whether apply should get the same
semantics, for masterless operation.
There are three alternatives here that I can see:
1. Status quo - ruthlessly override whatever the ENC specifies.
2. Flexible - use the ENC environment, but allow overriding it via --env
on the commandline
3. Strict - always use the ENC environment (except for the overridden
:manifest)
We might even go for a 2a, that would allow config files to override the
ENC as well (if we can easily discern such values from the defaults at
this point in the code).
Personally, I feel that the strict behavior would be very inconvenient.
An attacker could likely circumvent the ENC after all, so the security
aspect doesn't really apply here.
My vote is for the 2nd approach.
Cheers,
Felix
when we recently fixed a regression that had to do with directory
environments and ENC (PUP-3258) I noticed that the handling is weird
when the ENC actually picks an environment for the local machine.
As far as I can tell, puppet apply ignored that until now. The apply
command used the configured environment, replaced its :manifest setting
with what's chosen on the commandline, and makes that the only available
environment.
https://github.com/puppetlabs/puppet/blob/fea22be6a957e62005cab649537b39af0d0bda74/lib/puppet/application/apply.rb#L185-190
Puppet 3.7.0 is less stringent in that regard. It retains the set of
available environments.
https://github.com/puppetlabs/puppet/blob/master/lib/puppet/application/apply.rb#L193
This apparently causes an error when the ENC returns an environment (any
environment), which happens just below the linked code line in apply.rb.
This environment is looked up, bypassing the override of the :manifest.
I suggested a fix for this: Force the node (external or not) to use the
overridden environment.
https://github.com/ffrank/puppet/commit/39b78a8d25fa96c98f81227cd3ef6b48010fab68
During PR triage (sorry for vanishing suddenly, something came up), we
got some doubts whether this was the right thing to do. It is probably
in keeping with former behavior, but it contradicts a decision that was
made wrt. a classic bug.
http://projects.reductivelabs.com/issues/3910
When using agent, not apply, the puppet master may invoke an ENC. The
environment returned from the classifier takes precedence over any
:environment configuration on the agent (both file and commandline).
This is to allow users to use their ENC in contexts that are relevant
for security.
We're now looking for feedback on whether apply should get the same
semantics, for masterless operation.
There are three alternatives here that I can see:
1. Status quo - ruthlessly override whatever the ENC specifies.
2. Flexible - use the ENC environment, but allow overriding it via --env
on the commandline
3. Strict - always use the ENC environment (except for the overridden
:manifest)
We might even go for a 2a, that would allow config files to override the
ENC as well (if we can easily discern such values from the defaults at
this point in the code).
Personally, I feel that the strict behavior would be very inconvenient.
An attacker could likely circumvent the ENC after all, so the security
aspect doesn't really apply here.
My vote is for the 2nd approach.
Cheers,
Felix
John Bollinger
Oct 6, 2014, 4:26:46 PM10/6/14
to puppe...@googlegroups.com
On Sunday, October 5, 2014 8:30:32 PM UTC-5, Felix Frank wrote:
We're now looking for feedback on whether apply should get the same
semantics, for masterless operation.
There are three alternatives here that I can see:
1. Status quo - ruthlessly override whatever the ENC specifies.
2. Flexible - use the ENC environment, but allow overriding it via --env
on the commandline
3. Strict - always use the ENC environment (except for the overridden
:manifest)
We might even go for a 2a, that would allow config files to override the
ENC as well (if we can easily discern such values from the defaults at
this point in the code).
Personally, I feel that the strict behavior would be very inconvenient.
An attacker could likely circumvent the ENC after all, so the security
aspect doesn't really apply here.
My vote is for the 2nd approach.
I am of two minds. Although the security argument is pretty weak in the 'apply' case, the idea that 'apply' should follow the same rules as the agent is pretty appealing to me. On the other hand, the most important question is probably "what do people want to do with the tool"? Were approach (3) selected, I would have great sympathy for hapless admins exclaiming "I know what I'm doing, damnit! Stop getting in my way!"
In fact, I think I've just persuaded myself. I don't think the advantages of option (3) outweigh the value of making 'apply' the most useful tool it can be, which would require providing for convenient environment override. At the same time, I agree that 'apply' should honor the ENC-specific environment by default when there is one. That would appear to put me in the option (2) family of alternatives.
I'm going to think a little more and see what others have to say before I come down on the question of (2) vs. (2a).
In fact, I think I've just persuaded myself. I don't think the advantages of option (3) outweigh the value of making 'apply' the most useful tool it can be, which would require providing for convenient environment override. At the same time, I agree that 'apply' should honor the ENC-specific environment by default when there is one. That would appear to put me in the option (2) family of alternatives.
I'm going to think a little more and see what others have to say before I come down on the question of (2) vs. (2a).
Jeff McCune
Oct 6, 2014, 5:01:30 PM10/6/14
to puppe...@googlegroups.com
On Mon, Oct 6, 2014 at 1:26 PM, John Bollinger <john.bo...@stjude.org> wrote:
I'm going to think a little more and see what others have to say before I come down on the question of (2) vs. (2a).
I think we should do 2a because there should be rough parity between the configuration files and the command line arguments. We should do 2 or 2a because as John says, Puppet should, "Stop getting in my way!"
Specifically, running stand alone tools like Vagrant or a Puppet apply Jenkins jobs would require coordinating the environment for the node across a distributed system, which is complex and non-trivial. Much easier to set the environment using an environment variable or other job parameter and just be done with it. Much more simple and straight-forward.
-Jeff
Britt Gresham
Oct 6, 2014, 10:05:10 PM10/6/14
to puppe...@googlegroups.com
On Mon, Oct 6, 2014 at 2:01 PM, Jeff McCune <je...@puppetlabs.com> wrote:
I think we should do 2a because there should be rough parity between the configuration files and the command line arguments. We should do 2 or 2a because as John says, Puppet should, "Stop getting in my way!"
I think that option 2a is the safest. Allowing command line and configuration files to override an ENC makes the most sense when it comes to running down a configuration issue with puppet. Instead of going to the ENC to find that it is overriding a local configuration change you have a limited number of files to search on a node before consulting the ENC itself.
Even if 2a does not get chosen the command line arguments should rule over any ENC or configuration file because that is what the user is explicitly requesting.
Felix Frank
Oct 7, 2014, 4:48:35 AM10/7/14
to puppe...@googlegroups.com
On 10/07/2014 04:05 AM, Britt Gresham wrote:
> I think we should do 2a because there should be rough parity between
> the configuration files and the command line arguments. We should
> do 2 or 2a because as John says, Puppet should, "Stop getting in my
> way!"
>
>
> I think that option 2a is the safest. Allowing command line and
> configuration files to override an ENC makes the most sense when it
> comes to running down a configuration issue with puppet. Instead of
> going to the ENC to find that it is overriding a local configuration
> change you have a limited number of files to search on a node before
> consulting the ENC itself.
>
> Even if 2a does not get chosen the command line arguments should rule
> over any ENC or configuration file because that is what the user is
> explicitly requesting.
Sounds like a trend. Thanks for the feedback, guys.
> I think we should do 2a because there should be rough parity between
> the configuration files and the command line arguments. We should
> do 2 or 2a because as John says, Puppet should, "Stop getting in my
> way!"
>
>
> I think that option 2a is the safest. Allowing command line and
> configuration files to override an ENC makes the most sense when it
> comes to running down a configuration issue with puppet. Instead of
> going to the ENC to find that it is overriding a local configuration
> change you have a limited number of files to search on a node before
> consulting the ENC itself.
>
> Even if 2a does not get chosen the command line arguments should rule
> over any ENC or configuration file because that is what the user is
> explicitly requesting.
I took a tentative look at the code. While we can easily find out
whether the :environment setting was specified on the command line,
there is no obvious way to find out whether a value was set in the
configuration files rather than left at its default.
I do feel that variant 2 would be much more restrictive and confusing
than 2a by now. So I we may have to patch the settings subsystem to
allow for this.
Am I missing something in the existing tool chain?
Joshua Partlow
Oct 7, 2014, 12:00:57 PM10/7/14
to puppe...@googlegroups.com
No, I don't think so. The closest test is set_by_cli?(param) https://github.com/puppetlabs/puppet/blob/master/lib/puppet/settings.rb#L799
A similar check that lists the layers a parameter is set in would be fairly simple, and if it is set anywhere other than :application_defaults or :overridden_defaults, it would be coming from config file or commandline (except for :memory, though which is a direct override via Puppet[:foo]=).
--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/5433A8DC.9000902%40alumni.tu-berlin.de.
For more options, visit https://groups.google.com/d/optout.
Josh Partlow
Erik Dalén
Oct 9, 2014, 3:02:50 AM10/9/14
to Puppet Developers
I vote for option 2 or 3.
But against 2a. I think differentiating between a config file that specifies the default value (environment=production), and one that leaves it out (to get the default value) seems really odd behaviour.
Note that this whole thing only applies if you have node_terminus=exec in the main or user sections of the config file, and your ENC actually returns a environment. So if you want to ignore your ENC you can also just use --node_terminus=plain on the command line.
--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/5431F0B6.4070700%40Alumni.TU-Berlin.de.
For more options, visit https://groups.google.com/d/optout.
Erik Dalén
R.I.Pienaar
Oct 9, 2014, 5:13:47 AM10/9/14
to puppet-dev
----- Original Message -----
> From: "Erik Dalén" <erik.gus...@gmail.com>
> To: "puppet-dev" <puppe...@googlegroups.com>
> Sent: Thursday, October 9, 2014 8:02:47 AM
> Subject: Re: [Puppet-dev] How should apply behave under an ENC
> I vote for option 2 or 3.
> But against 2a. I think differentiating between a config file that
> specifies the default value (environment=production), and one that leaves
> it out (to get the default value) seems really odd behaviour.
>
> Note that this whole thing only applies if you have node_terminus=exec in
> the main or user sections of the config file, and your ENC actually returns
> a environment. So if you want to ignore your ENC you can also just use
> --node_terminus=plain on the command line.
Configuring an ENC with puppet apply is probably not the most common thing or
something one would enable everywhere apply is used. I found people who use
masterless would have a shell script to do those runs or maybe run puppet with
a specific config file - leaving normal puppet apply test.pp type behaviour as
is. Either way they would enable the masterless mode used to manage the machine
using a special set of steps and those steps would include enabling an ENC
So for me at least if I configure apply to use an ENC then that ENC should be
authoritative.
Felix Frank
Oct 21, 2014, 4:40:19 PM10/21/14
to puppe...@googlegroups.com
Hey everyone,
thanks for your feedback. We appear to see some support for all options
except #1. That one is apparently universally despised.
I'll try and summarize the discussion. Pros get a + plus and cons a - dash.
Option 2: Override ENC environment but only from the commandline.
+ convenient override
+ CLI invocation works as expected (by most)
- config options behave differently from CLI vs. config file, which is
confusing
- explicitly setting the default on the CLI is different from skipping
it or doing it in the config, which is surprising
Option 2a:
+ convenient override
+ configuration options in files and parameters behave identically
- explicitly setting the default in the config has other semantics than
skipping it, which is surprising
Option 3:
+ ENC is important for most masterless folks, who will want it to
override local shenanigans
+ easily overridden via --node_terminus=plain
+ regular use case: ENC is chosen via parameter anyway for masterless,
not for any `apply` run, via dedicated script, and should overrule
environment from the config then
+ apply behaves more like agent (omg consistency)
- will confuse users who pass an environment which won't take effect
All things considered, we find ourselves in a fine position to choose
our poison. There were more votes from the 2/2a camps, but also good
points from RI in favor of 3.
This leads me to believe that whatever we choose, a priority should be
for `apply` to be very vocal about which environment is in effect, and
how it was chosen. I don't think we have so much as a debug message
right now. I propose a notice to always comment on the environment.
Perhaps even a warning when the user is apparently trying to do
something that can't work.
I was just about to suggest just taking 2a for all the votes it received
and for being the closest to current behavior (minus insanity). But then
the list of arguments in favor of 3 is rather impressive. Its negatives
aren't too bad, and can be mitigated through UI design.
Call to the 2/2a supporters - are you at all swayed by those?
Cheers,
Felix
thanks for your feedback. We appear to see some support for all options
except #1. That one is apparently universally despised.
I'll try and summarize the discussion. Pros get a + plus and cons a - dash.
Option 2: Override ENC environment but only from the commandline.
+ convenient override
+ CLI invocation works as expected (by most)
- config options behave differently from CLI vs. config file, which is
confusing
- explicitly setting the default on the CLI is different from skipping
it or doing it in the config, which is surprising
Option 2a:
+ convenient override
+ configuration options in files and parameters behave identically
- explicitly setting the default in the config has other semantics than
skipping it, which is surprising
Option 3:
+ ENC is important for most masterless folks, who will want it to
override local shenanigans
+ easily overridden via --node_terminus=plain
+ regular use case: ENC is chosen via parameter anyway for masterless,
not for any `apply` run, via dedicated script, and should overrule
environment from the config then
+ apply behaves more like agent (omg consistency)
- will confuse users who pass an environment which won't take effect
All things considered, we find ourselves in a fine position to choose
our poison. There were more votes from the 2/2a camps, but also good
points from RI in favor of 3.
This leads me to believe that whatever we choose, a priority should be
for `apply` to be very vocal about which environment is in effect, and
how it was chosen. I don't think we have so much as a debug message
right now. I propose a notice to always comment on the environment.
Perhaps even a warning when the user is apparently trying to do
something that can't work.
I was just about to suggest just taking 2a for all the votes it received
and for being the closest to current behavior (minus insanity). But then
the list of arguments in favor of 3 is rather impressive. Its negatives
aren't too bad, and can be mitigated through UI design.
Call to the 2/2a supporters - are you at all swayed by those?
Cheers,
Felix
John Bollinger
Oct 22, 2014, 2:07:55 PM10/22/14
to puppe...@googlegroups.com
On Tuesday, October 21, 2014 3:40:19 PM UTC-5, Felix Frank wrote:
Hey everyone,
thanks for your feedback. We appear to see some support for all options
except #1. That one is apparently universally despised.
I'll try and summarize the discussion. Pros get a + plus and cons a - dash.
Very nice, Felix, thanks.
[...]
Option 3:
+ ENC is important for most masterless folks, who will want it to
override local shenanigans
+ easily overridden via --node_terminus=plain
Please forgive my ignorance, but is it the case that an ENC does nothing but (maybe) provide an environment under 'puppet apply'? If puppet were going to also apply classes specified by the ENC or to set top-scope variables other than $environment, then disabling the ENC is not really a viable override.
John
John Bollinger
Oct 22, 2014, 4:46:09 PM10/22/14
to puppe...@googlegroups.com
On Tuesday, October 21, 2014 3:40:19 PM UTC-5, Felix Frank wrote:
All things considered, we find ourselves in a fine position to choose
our poison. There were more votes from the 2/2a camps, but also good
points from RI in favor of 3.
This leads me to believe that whatever we choose, a priority should be
for `apply` to be very vocal about which environment is in effect, and
how it was chosen. I don't think we have so much as a debug message
right now. I propose a notice to always comment on the environment.
Perhaps even a warning when the user is apparently trying to do
something that can't work.
I was just about to suggest just taking 2a for all the votes it received
and for being the closest to current behavior (minus insanity). But then
the list of arguments in favor of 3 is rather impressive. Its negatives
aren't too bad, and can be mitigated through UI design.
Call to the 2/2a supporters - are you at all swayed by those?
I'm not sure I'm persuaded, but I am certainly swayed. As I said initially, the consistency afforded by option 3 is very appealing. If my concerns about the viability of the --node-terminus option for overriding the ENC prove unfounded then I wouldn't complain about option 3.
In any case, I agree that 'apply' should be abundantly clear about which environment is chosen, and why.
John
In any case, I agree that 'apply' should be abundantly clear about which environment is chosen, and why.
John
Andy Parker
Nov 5, 2014, 12:46:16 PM11/5/14
to puppe...@googlegroups.com
I get the impression that the general consensus here (and has been expressed in other threads) is in favor of consistency of behavior across the applications even when it might cause some pain for some users because of different usage patterns. That line of reasoning leads in the direction of option 3, I believe, which is what puppet apply currently does.
Does puppet currently emit a warning or other message that it is ignoring one environment in favor of the environment from the ENC? If not maybe that would be enough to clarify what is happening.
John
--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/9d01530b-0dad-4b93-9430-5561d554bb70%40googlegroups.com.
Andrew Parker
Freenode: zaphod42
Twitter: @aparker42
Software DeveloperJoin us at PuppetConf 2015, October 5-9 in Portland, OR - http://2015.puppetconf.com
Register early to save 40%!
Felix Frank
Nov 15, 2014, 6:51:32 PM11/15/14
to puppe...@googlegroups.com
On 11/05/2014 06:46 PM, Andy Parker wrote:
> I get the impression that the general consensus here (and has been
> expressed in other threads) is in favor of consistency of behavior
> across the applications even when it might cause some pain for some
> users because of different usage patterns. That line of reasoning
> leads in the direction of option 3, I believe, which is what puppet
> apply currently does.
>
> Does puppet currently emit a warning or other message that it is
> ignoring one environment in favor of the environment from the ENC? If
> not maybe that would be enough to clarify what is happening.
Well, there is still no clear consensus that I can see, but there has
> I get the impression that the general consensus here (and has been
> expressed in other threads) is in favor of consistency of behavior
> across the applications even when it might cause some pain for some
> users because of different usage patterns. That line of reasoning
> leads in the direction of option 3, I believe, which is what puppet
> apply currently does.
>
> Does puppet currently emit a warning or other message that it is
> ignoring one environment in favor of the environment from the ENC? If
> not maybe that would be enough to clarify what is happening.
been no negative feedback regarding the proposed shift towards option 3.
I shall try and prepare a PR towards implementing it. Please raise any
more concerns or veto before Wednesday, when it will hopefully be triaged.
Thanks,
Felix
Reply all
Reply to author
Forward
0 new messages