Issue #17710 has been updated by Alex Harvey.
Disclaimer - more experienced developers on the project may completely disagree
with any or all of this. But for what it's worth the issues with the IP facts
are as I see them -
1. A number of methods use case statements without default cases such that the
code will need modification for each new kernel version that gets added. If
the case statement is required at all (and I suspect it is) then perhaps the
linux and BSD kernels ought to fall into the default case.
This applies to the REGEX_MAP, and the get_all_interface_output and
get_single_interface_output methods, in lib/facter/util/ip.rb; to
Facter.add(:ipaddress) blocks in lib/facter/ipaddress.rb; to
Facter.add("netmask") blocks in lib/facter/netmask.rb; and to
Facter.add(:ipaddress6) blocks in lib/facter/ipaddress6.rb.
2. This is a general problem with the facter code; there doesn't seem to be a
clear rule about when to specify full paths to external commands and when not
to. Or perhaps Luke had a clear rule that wasn't subsequently followed. In
any case if we unset the path we can see that facter pushes /usr/sbin:/sbin
onto the path internally -
<pre>
# PATH= /usr/local/bin/facter 2>/dev/null |grep path
path => :/usr/sbin:/sbin
</pre>
This makes the code vulnerable both to external commands like ifconfig being in
unexpected locations (when the full path is hardcoded) and to code failing when
$PATH isn't set in an expected way (i.e. to include /bin and /usr/bin).
If developers agree with this I would be happy to go through the code and make
this change.
3. Going back to the REGEX_MAP - is this really required at all? I think we
could write a single regex for :ipaddress, :ipaddress6, :macaddress, and
:netmask that handles all the known platforms. We could also avoid the problem
of having no default case that way. We could then stub a whole range of
ifconfig examples from all our platforms and properly test these using RSpec.
I'd be happy to work on this too if developers agree.
4. All of the calls to external commands apparently would need to be rewritten
using delegate methods so that their outputs can be stubbed without stubbing
exec.
5. Finally, all the tests in spec/unit/util/ip_spec.rb that stub
:get_all_interface_output should be changed so that the outputs of the system
commands instead are stubbed.
----------------------------------------
Refactor #17710: parts of lib/factor/util/ip.rb to be rewritten with delegate
methods to allow proper tests to be written
https://projects.puppetlabs.com/issues/17710#change-76840
Author: Alex Harvey
Status: Unreviewed
Priority: Normal
Assignee: Jeff McCune
Category:
Target version:
Keywords:
Branch:
Affected Facter version: 1.6.14
--
You have received this notification because you have either subscribed to it,
or are involved in it.
To change your notification preferences, please click here:
http://projects.puppetlabs.com/my/account
--
You received this message because you are subscribed to the Google Groups
"Puppet Bugs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-bugs?hl=en.
