Please review pull request #199: (#7484) Domain Fact should handle TLD opened by (hkenney)
Description:
The domain fact executed "hostname" on all platforms,
however the default on Linux is that "hostname" does not return FQDN.
This causes problems when the machine only has a TLD, rather than being
in a second level domain. For example, ns01.tld vs ns01.puppetlabs.com.
This patch makes 'hostname -f' to be opt-in to prevent problems
on certain operating systems such as Solaris and HP-UX.
- Opened: Wed May 09 17:44:39 UTC 2012
- Based on: puppetlabs:master (1bd98fd94b5e577216b80789ee9f933dd5cc74f8)
- Requested merge: hkenney:ticket/master/7484_domain_fact_should_handle_tld (3168927e5184ff04d564fdd8757648f973927993)
Diff follows:
diff --git a/lib/facter/domain.rb b/lib/facter/domain.rb
index 2ae909a..306ad0d 100644
--- a/lib/facter/domain.rb
+++ b/lib/facter/domain.rb
@@ -23,7 +23,17 @@
# Get the domain from various sources; the order of these
# steps is important
- if name = Facter::Util::Resolution.exec('hostname') \
+ # In some OS 'hostname -f' will change the hostname to '-f'
+ # We know that Solaris and HP-UX exhibit this behavior
+ # On good OS, 'hostname -f' will return the FQDN which is preferable
+ # Due to dangerous behavior of 'hostname -f' on old OS, we will explicitly opt-in
+ # 'hostname -f' --hkenney May 9, 2012
+ hostname_command = 'hostname'
+ can_do_hostname_f = Regexp.union /Linux/i, /FreeBSD/i, /Darwin/i
+ hostname_command = 'hostname -f' if Facter.value(:kernel) =~ can_do_hostname_f
+
+
+ if name = Facter::Util::Resolution.exec(hostname_command) \
and name =~ /.*?\.(.+$)/
$1
diff --git a/spec/unit/domain_spec.rb b/spec/unit/domain_spec.rb
index 07195ca..9e34802 100755
--- a/spec/unit/domain_spec.rb
+++ b/spec/unit/domain_spec.rb
@@ -4,98 +4,119 @@
describe "Domain name facts" do
- describe "on linux" do
- before do
- Facter.fact(:kernel).stubs(:value).returns("Linux")
- FileTest.stubs(:exists?).with("/etc/resolv.conf").returns(true)
- end
-
- it "should use the hostname binary" do
- Facter::Util::Resolution.expects(:exec).with("hostname").returns "test.example.com"
- Facter.fact(:domain).value.should == "example.com"
- end
-
- it "should fall back to the dnsdomainname binary" do
- Facter::Util::Resolution.expects(:exec).with("hostname").returns("myhost")
- Facter::Util::Resolution.expects(:exec).with("dnsdomainname").returns("example.com")
- Facter.fact(:domain).value.should == "example.com"
- end
-
-
- it "should fall back to /etc/resolv.conf" do
- Facter::Util::Resolution.expects(:exec).with("hostname").at_least_once.returns("myhost")
- Facter::Util::Resolution.expects(:exec).with("dnsdomainname").at_least_once.returns("")
- File.expects(:open).with('/etc/resolv.conf').at_least_once
- Facter.fact(:domain).value
- end
-
- it "should attempt to resolve facts in a specific order" do
- seq = sequence('domain')
- Facter::Util::Resolution.stubs(:exec).with("hostname").in_sequence(seq).at_least_once
- Facter::Util::Resolution.stubs(:exec).with("dnsdomainname").in_sequence(seq).at_least_once
- File.expects(:open).with('/etc/resolv.conf').in_sequence(seq).at_least_once
- Facter.fact(:domain).value
- end
-
- describe "when using /etc/resolv.conf" do
+ { :linux => {:kernel => "Linux", :hostname_command => "hostname -f"},
+ :solaris => {:kernel => "SunOS", :hostname_command => "hostname"},
+ :darwin => {:kernel => "Darwin", :hostname_command => "hostname -f"},
+ :freebsd => {:kernel => "FreeBSD", :hostname_command => "hostname -f"},
+ :hpux => {:kernel => "HP-UX", :hostname_command => "hostname"},
+ }.each do |key, nested_hash|
+
+ describe "on #{key}" do
before do
- Facter::Util::Resolution.stubs(:exec).with("hostname")
- Facter::Util::Resolution.stubs(:exec).with("dnsdomainname")
- @mock_file = mock()
- File.stubs(:open).with("/etc/resolv.conf").yields(@mock_file)
+ Facter.fact(:kernel).stubs(:value).returns(nested_hash[:kernel])
+ FileTest.stubs(:exists?).with("/etc/resolv.conf").returns(true)
end
-
- it "should use the domain field over the search field" do
- lines = [
- "nameserver 4.2.2.1",
- "search example.org",
- "domain example.com",
- ]
- @mock_file.expects(:each).multiple_yields(*lines)
- Facter.fact(:domain).value.should == 'example.com'
+
+ let :hostname_command do
+ nested_hash[:hostname_command]
+ end
+
+ it "should use the hostname binary" do
+ Facter::Util::Resolution.expects(:exec).with(hostname_command).returns "test.example.com"
+ Facter.fact(:domain).value.should == "example.com"
end
-
- it "should fall back to the search field" do
- lines = [
- "nameserver 4.2.2.1",
- "search example.org",
- ]
- @mock_file.expects(:each).multiple_yields(*lines)
- Facter.fact(:domain).value.should == 'example.org'
+
+ it "should fall back to the dnsdomainname binary" do
+ Facter::Util::Resolution.expects(:exec).with(hostname_command).returns("myhost")
+ Facter::Util::Resolution.expects(:exec).with("dnsdomainname").returns("example.com")
+ Facter.fact(:domain).value.should == "example.com"
end
-
- it "should use the first domain in the search field" do
- lines = [
- "search example.org example.net",
- ]
- @mock_file.expects(:each).multiple_yields(*lines)
- Facter.fact(:domain).value.should == 'example.org'
+
+
+ it "should fall back to /etc/resolv.conf" do
+ Facter::Util::Resolution.expects(:exec).with(hostname_command).at_least_once.returns("myhost")
+ Facter::Util::Resolution.expects(:exec).with("dnsdomainname").at_least_once.returns("")
+ File.expects(:open).with('/etc/resolv.conf').at_least_once
+ Facter.fact(:domain).value
+ end
+
+ it "should attempt to resolve facts in a specific order" do
+ seq = sequence('domain')
+ Facter::Util::Resolution.stubs(:exec).with(hostname_command).in_sequence(seq).at_least_once
+ Facter::Util::Resolution.stubs(:exec).with("dnsdomainname").in_sequence(seq).at_least_once
+ File.expects(:open).with('/etc/resolv.conf').in_sequence(seq).at_least_once
+ Facter.fact(:domain).value
end
+
+ describe "Top level domain" do
+ it "should find the domain name" do
+ Facter::Util::Resolution.expects(:exec).with(hostname_command).returns "ns01.tld"
+ Facter::Util::Resolution.expects(:exec).with("dnsdomainname").never
+ File.expects(:exists?).with('/etc/resolv.conf').never
+ Facter.fact(:domain).value.should == "tld"
+ end
+ end
+
+ describe "when using /etc/resolv.conf" do
+ before do
+ Facter::Util::Resolution.stubs(:exec).with(hostname_command)
+ Facter::Util::Resolution.stubs(:exec).with("dnsdomainname")
+ @mock_file = mock()
+ File.stubs(:open).with("/etc/resolv.conf").yields(@mock_file)
+ end
+
+ it "should use the domain field over the search field" do
+ lines = [
+ "nameserver 4.2.2.1",
+ "search example.org",
+ "domain example.com",
+ ]
+ @mock_file.expects(:each).multiple_yields(*lines)
+ Facter.fact(:domain).value.should == 'example.com'
+ end
- # Test permutations of domain and search
- [
- ["domain domain", "domain"],
- ["domain search", "search"],
- ["search domain", "domain"],
- ["search search", "search"],
- ["search domain notdomain", "domain"],
- [["#search notdomain","search search"], "search"],
- [["# search notdomain","search search"], "search"],
- [["#domain notdomain","domain domain"], "domain"],
- [["# domain notdomain","domain domain"], "domain"],
- ].each do |tuple|
- field = tuple[0]
- expect = tuple[1]
- it "should return #{expect} from \"#{field}\"" do
+ it "should fall back to the search field" do
lines = [
- field
- ].flatten
+ "nameserver 4.2.2.1",
+ "search example.org",
+ ]
@mock_file.expects(:each).multiple_yields(*lines)
- Facter.fact(:domain).value.should == expect
+ Facter.fact(:domain).value.should == 'example.org'
+ end
+
+ it "should use the first domain in the search field" do
+ lines = [
+ "search example.org example.net",
+ ]
+ @mock_file.expects(:each).multiple_yields(*lines)
+ Facter.fact(:domain).value.should == 'example.org'
+ end
+
+ # Test permutations of domain and search
+ [
+ ["domain domain", "domain"],
+ ["domain search", "search"],
+ ["search domain", "domain"],
+ ["search search", "search"],
+ ["search domain notdomain", "domain"],
+ [["#search notdomain","search search"], "search"],
+ [["# search notdomain","search search"], "search"],
+ [["#domain notdomain","domain domain"], "domain"],
+ [["# domain notdomain","domain domain"], "domain"],
+ ].each do |tuple|
+ field = tuple[0]
+ expect = tuple[1]
+ it "should return #{expect} from \"#{field}\"" do
+ lines = [
+ field
+ ].flatten
+ @mock_file.expects(:each).multiple_yields(*lines)
+ Facter.fact(:domain).value.should == expect
+ end
end
end
end
- end
+ end
describe "on Windows" do
before(:each) do
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" 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-dev?hl=en.
