[PUP-4744] Yumrepo doesn't recognize whitespace delimited reposdir settings in /etc/yum.conf Created: 2015/06/11 Updated: 2016/10/19 Resolved: 2016/01/13
|Component/s:||Types and Providers|
|Affects Version/s:||PUP 3.8.1, PUP 4.1.0|
|Fix Version/s:||PUP 4.3.2|
|Reporter:||Charlie Sharpsteen||Assignee:||Steve Barlow|
|Labels:||customer, puppethack, support|
|Remaining Estimate:||Not Specified|
|Time Spent:||Not Specified|
|Original Estimate:||Not Specified|
osfamily => RedHat
|Sprint:||Client 2015-12-30, Client 2016-01-13|
|Release Notes:||Bug Fix|
|Release Notes Summary:||The reposdir setting for yum allows commas, whitespace including newlines, but puppet's yumrepo type only recognized commas and would add the repo definitions to /etc/yum.conf. This fix updates puppet to also recognize whitespace and newlines, creating repo definitions in /etc/yum.conf.d as expected.|
The inifile provider for the Yumrepo type searches various directories when deciding where to create repository definitions. One source of directories is the reposdir setting of /etc/yum.conf. Yum allows this setting to contain a list of directories delimited by either commas or whitespace, however Puppet only uses commas when parsing the setting.
If the reposdir setting contains a whitespace delimited list of directories, then Puppet will almost silently (a DEBUG level message is logged) fall back to creating definitions directly in /etc/yum.conf.
The definition is added to /etc/yum.conf instead of a new file under /tmp/:
The definition is created under /tmp/ as it's the last reposdir specified
|Comment by Josh Cooper [ 2015/12/10 ]|
Probably just need to change: https://github.com/puppetlabs/puppet/blob/4.3.1/lib/puppet/provider/yumrepo/inifile.rb#L77
|Comment by Kylo Ginsberg [ 2015/12/28 ]|
Specifically, it chooses the last, not the first, directory in the reposdir setting. I.e. given the arrangement in the description, it will create /tmp/foo.repo, not /etc/yumrepos.d/foo.repo. I think this is the appropriate thing to do, e.g. see the reasoning in the comment at https://github.com/puppetlabs/puppet/blob/4.2.2/lib/puppet/provider/yumrepo/inifile.rb#L169-L171.
If it is the appropriate thing to do, we might want to flesh out the docstring at https://github.com/puppetlabs/puppet/blob/4.2.2/lib/puppet/provider/yumrepo/inifile.rb#L17-L21 to call out this case.
If not, then we should discuss what the desired behavior is.
|Comment by Kylo Ginsberg [ 2016/01/11 ]|
Last call for watchers on this. See my comment above. Please respond with objections to the behavior by Tuesday Jan 12. If I don't here otherwise I'll update the description to match the behavior (which, again, I think actually makes sense). Thanks all!!
|Comment by Josh Cooper [ 2016/01/11 ]|
The behavior seems to be intentional due to
|Comment by Adrien Thebo [ 2016/01/11 ]|
In Puppet 3.5.0/3.5.1 the yumrepo type was split out into a distinct type and provider, which was a massive overhaul. Moreover that code had almost no effective tests beforehand so we regressed on more than a handful of features during the refactor. It looks like this line (https://github.com/puppetlabs/puppet/blob/3.2.3/lib/puppet/type/yumrepo.rb#L112) originally made either spaces or commas work, but this intent wasn't clearly called out so I suspect that it was accidentally dropped.
+1 for treating commas and spaces as equivalent values for the reposdir setting.
(edit: it's worth noting that per http://linux.die.net/man/5/yum.conf the path separator character isn't specified in the documentation for yum.conf, at least as far as I can tell. We should definitely document this behavior in the code + add tests so that we don't regress on this again.)
(edit 2: it looks like some settings like reposdir and tsflags take a comma or space separated list, while settings like exclude only take space separated lists.)
|Comment by Kylo Ginsberg [ 2016/01/12 ]|
Thanks for chiming in folks!
Moving this along to Ready for Review, since I did the verification up above.