Skip to content

(#15337) Do not merge user and system puppet.conf#1006

Merged
zaphod42 merged 5 commits intopuppetlabs:3.xfrom
jeffmccune:bug/3.x/15337_dont_merge_puppet_conf
Aug 7, 2012
Merged

(#15337) Do not merge user and system puppet.conf#1006
zaphod42 merged 5 commits intopuppetlabs:3.xfrom
jeffmccune:bug/3.x/15337_dont_merge_puppet_conf

Conversation

@jeffmccune
Copy link
Contributor

Puppet 3.x reads settings from both ~/.puppet/puppet.conf and the system
puppet.conf without this patch applied. This is a problem because it
makes it difficult to determine where to load plugins and extensions from
at runtime. Merging the configuration file also makes it more difficult
to explain where a particular setting is taking effect.

This patch makes the intended puppet.conf reading behavior the
following:

1: If provided, use explicit puppet.conf in --confdir
2: If root, use system puppet.conf
3: Otherwise, use ~/.puppet/puppet.conf

This patch also changes the behavior of rack puppet master applications.
We now intended for the rack configuration file, config.ru to explicitly
set --confdir to avoid reading from
~/.puppet/puppet.conf. Please see the example in
ext/rack/files/config.ru for an up to date rack configuration.

Jeff McCune added 5 commits August 5, 2012 10:43
Without this patch I noticed the git log contains both Chris Price and
cprice as separate authors.  This patch fixes the problem by normalizing
them to Chris Price <chris@puppetlabs.com>
The Trollop library is improperly vendored inside of Puppet without this
patch applied.  This is a problem because end users or developers who
have the trollop gem installed will be monkey patched by Puppet since we
open up the top level Trollop module namespace.

This patch fixes the problem by placing Trollop in the namespace of
Puppet::Util::CommandLine::Trollop.  This fixes the problem because the
only place Trollop is used by Puppet is inside the
Puppet::Util::CommandLine class.  Therefore we no longer need to use the
absolute namespace notation of the leading ::.

This also prevents us from monkey patching the real Trollop gem that
lives at ::Trollop if it's been loaded into the process.
This is just slightly more readable.
Without this patch the README_DEVELOPER file displays poorly in my Vim
buffers.  This patch adds some additional markup to separate the code
snippets from the prose.
Puppet 3.x reads settings from both ~/.puppet/puppet.conf and the system
puppet.conf without this patch applied.  This is a problem because it
makes it difficult to determine where to load plugins and extensions
from at runtime.  Merging the configuration file also makes it more
difficult to explain where a particular setting is taking effect.

This patch makes the intended `puppet.conf` reading behavior the
following:

 1: If provided, use explicit puppet.conf in `--confdir`
 2: If root, use system puppet.conf
 3: Otherwise, use ~/.puppet/puppet.conf

This patch also changes the behavior of rack puppet master applications.
We now intended for the rack configuration file, `config.ru` to
explicitly set `--confdir` to avoid reading from
`~/.puppet/puppet.conf`.  Please see the example in
`ext/rack/files/config.ru` for an up to date rack configuration.
@zaphod42
Copy link
Contributor

zaphod42 commented Aug 7, 2012

I got this to pick up the system puppet.conf as root and an explicit puppet.conf as root (via --confdir). A normal user without a .puppet dir used the system conf (which I think is the desired behavior) and it also picked up the .puppet/puppet.confonce it was there. Strangely, as the normal user, I was not able to get the --confdir to override what puppet.conf to read, once the .puppet/puppet.conf was there the --confdir was ignored. Removing the .puppet directory allowed --confdir to work again.

@nigelkersten
Copy link
Contributor

Is that the desired behavior? I don't remember normal users ever falling back to system puppet.conf ?

@zaphod42
Copy link
Contributor

zaphod42 commented Aug 7, 2012

Never mind about my comment about --confdir not working if the .puppet/puppet.conf file existed. I had messed up and wasn't testing with the patch applied

@zaphod42
Copy link
Contributor

zaphod42 commented Aug 7, 2012

Is the desired behavior that if the runmode is not present in the local ~/.puppet/puppet.conf then it will fall back to the system puppet.conf?

[me@pe-centos6 ~]$ puppet master --configprint modulepath 
/etc/puppet/modules:/usr/share/puppet/modules
[me@pe-centos6 ~]$ puppet agent --configprint modulepath 
/in/the/user/puppet/dir
[me@pe-centos6 ~]$ cat .puppet/puppet.conf 
[agent]
modulepath=/in/the/user/puppet/dir
[me@pe-centos6 ~]

@zaphod42
Copy link
Contributor

zaphod42 commented Aug 7, 2012

Arrgh! Never mind again. My brain isn't working today. The behavior in my last comment is correct. The [master] section is coming from the defaults and not the system config file.

zaphod42 added a commit that referenced this pull request Aug 7, 2012
…pet_conf

(#15337) Do not merge user and system puppet.conf
@zaphod42 zaphod42 merged commit 38cba0a into puppetlabs:3.x Aug 7, 2012
@ahpook
Copy link
Contributor

ahpook commented Aug 7, 2012

Andy and I tested this and it looks good.

Added a blurb to the release notes about needing new config.ru -- this will be a breaking change for people who upgrade so we need to make sure it's really visible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants