(#2128) Add support for setting node name based on a fact

This adds the node_name_fact setting, which specifies a fact to use to
determine the node name. This allows dynamically determining the node name
without having to modify puppet.conf or command line options.

Using this setting requires modifying auth.conf to allow nodes to request
catalogs not matching their certnames.

For example, this would allow any authenticated node to retrieve any catalog:

  # $confdir/auth.conf
  path ~ /catalog/.+
  allow *

The node_name_fact and node_name_value options are mutually exclusive, because
it is ambiguous which setting should take precedence.

Paired-With: Jacob Helwig <jacob@puppetlabs.com>

9 files changed, 150 insertions, 3 deletions

diff --git a/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb b/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb
new file mode 100644
index 000000000..2e89d765f
--- /dev/null
+++ b/acceptance/tests/allow_arbitrary_node_name_fact_for_agent.rb
@@ -0,0 +1,46 @@
+test_name "node_name_fact should be used to determine the node name for puppet agent"
+
+success_message = "node_name_fact setting was correctly used to determine the node name"
+
+node_names = []
+
+on agents, facter('kernel') do
+  node_names << stdout.chomp
+end
+
+node_names.uniq!
+
+authfile = "/tmp/auth.conf-2128-#{$$}"
+authconf = node_names.map do |node_name|
+  %Q[
+path /catalog/#{node_name}
+auth yes
+allow *
+]
+end.join("\n")
+
+manifest_file = "/tmp/node_name_value-test-#{$$}.pp"
+manifest = %Q[
+  Exec { path => "/usr/bin:/bin" }
+  node default {
+    exec { "false": }
+  }
+]
+manifest << node_names.map do |node_name|
+  %Q[
+    node "#{node_name}" {
+      exec { "echo #{success_message}": }
+    }
+  ]
+end.join("\n")
+
+create_remote_file master, authfile, authconf
+create_remote_file master, manifest_file, manifest
+
+on master, "chmod 644 #{authfile} #{manifest_file}"
+
+with_master_running_on(master, "--rest_authconfig #{authfile} --manifest #{manifest_file} --daemonize --autosign true") do
+  run_agent_on(agents, "--no-daemonize --verbose --onetime --node_name_fact kernel --server #{master}") do
+    assert_match(success_message, stdout)
+  end
+end
diff --git a/acceptance/tests/allow_arbitrary_node_name_fact_for_apply.rb b/acceptance/tests/allow_arbitrary_node_name_fact_for_apply.rb
new file mode 100644
index 000000000..d7cf2dcac
--- /dev/null
+++ b/acceptance/tests/allow_arbitrary_node_name_fact_for_apply.rb
@@ -0,0 +1,30 @@
+test_name "node_name_fact should be used to determine the node name for puppet apply"
+
+success_message = "node_name_fact setting was correctly used to determine the node name"
+
+node_names = []
+
+on agents, facter('kernel') do
+  node_names << stdout.chomp
+end
+
+node_names.uniq!
+
+manifest = %Q[
+  Exec { path => "/usr/bin:/bin" }
+  node default {
+    exec { "false": }
+  }
+]
+
+node_names.each do |node_name|
+  manifest << %Q[
+    node "#{node_name}" {
+      exec { "echo #{success_message}": }
+    }
+  ]
+end
+
+on agents, puppet_apply("--verbose --node_name_fact kernel"), :stdin => manifest do
+  assert_match(success_message, stdout)
+end
diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 3f423a4c4..717935640 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -89,6 +89,11 @@ class Puppet::Application::Apply < Puppet::Application
       raise "Could not find facts for #{Puppet[:node_name_value]}"
     end
 
+    unless Puppet[:node_name_fact].empty?
+      Puppet[:node_name_value] = facts.values[Puppet[:node_name_fact]]
+      facts.name = Puppet[:node_name_value]
+    end
+
     # Find our Node
     unless node = Puppet::Node.find(Puppet[:node_name_value])
       raise "Could not find node #{Puppet[:node_name_value]}"
diff --git a/lib/puppet/configurer/fact_handler.rb b/lib/puppet/configurer/fact_handler.rb
index 4d80e17b6..77bd1e5f1 100644
--- a/lib/puppet/configurer/fact_handler.rb
+++ b/lib/puppet/configurer/fact_handler.rb
@@ -16,7 +16,12 @@ module Puppet::Configurer::FactHandler
     # compile them and then "cache" them on the server.
     begin
       reload_facter
-      Puppet::Node::Facts.find(Puppet[:node_name_value])
+      facts = Puppet::Node::Facts.find(Puppet[:node_name_value])
+      unless Puppet[:node_name_fact].empty?
+        Puppet[:node_name_value] = facts.values[Puppet[:node_name_fact]]
+        facts.name = Puppet[:node_name_value]
+      end
+      facts
     rescue SystemExit,NoMemoryError
       raise
     rescue Exception => detail
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 17c2850e5..4502dae16 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -487,6 +487,14 @@ module Puppet
 
   setdefaults(:agent,
     :node_name_value => ["$certname", "The name of the node."],
+    :node_name_fact => { :default => "",
+        :desc => "The fact to use as the node name.",
+        :hook => proc do |value|
+          if !value.empty? and Puppet[:node_name_value] != Puppet[:certname]
+            raise "Cannot specify both the node_name_value and node_name_fact settings"
+          end
+        end
+      },
     :localconfig => { :default => "$statedir/localconfig",
       :owner => "root",
       :mode => 0660,
diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb
index 572d98cde..3178fca11 100755
--- a/spec/integration/defaults_spec.rb
+++ b/spec/integration/defaults_spec.rb
@@ -30,6 +30,21 @@ describe "Puppet defaults" do
     end
   end
 
+  describe "when setting the :node_name_fact" do
+    it "should fail when also setting :node_name_value" do
+      lambda do
+        Puppet.settings[:node_name_value] = "some value"
+        Puppet.settings[:node_name_fact] = "some_fact"
+      end.should raise_error("Cannot specify both the node_name_value and node_name_fact settings")
+    end
+
+    it "should not fail when using the default for :node_name_value" do
+      lambda do
+        Puppet.settings[:node_name_fact] = "some_fact"
+      end.should_not raise_error
+    end
+  end
+
   describe "when configuring the :crl" do
     it "should warn if :cacrl is set to false" do
       Puppet.expects(:warning)
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index 83a5ded7f..cf9d44ca2 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -258,10 +258,25 @@ describe Puppet::Application::Apply do
         @apply.main
       end
 
-      it "should collect the node facts" do
-        Puppet::Node::Facts.expects(:find).returns(@facts)
+      it "should set the facts name based on the node_name_fact" do
+        @facts = Puppet::Node::Facts.new(Puppet[:node_name_value], 'my_name_fact' => 'other_node_name')
+        @facts.save
+        Puppet::Node.new('other_node_name').save
+        Puppet[:node_name_fact] = 'my_name_fact'
 
         @apply.main
+
+        @facts.name.should == 'other_node_name'
+      end
+
+      it "should set the node_name_value based on the node_name_fact" do
+        Puppet::Node::Facts.new(Puppet[:node_name_value], 'my_name_fact' => 'other_node_name').save
+        Puppet::Node.new('other_node_name').save
+        Puppet[:node_name_fact] = 'my_name_fact'
+
+        @apply.main
+
+        Puppet[:node_name_value].should == 'other_node_name'
       end
 
       it "should raise an error if we can't find the facts" do
diff --git a/spec/unit/configurer/fact_handler_spec.rb b/spec/unit/configurer/fact_handler_spec.rb
index ddb541174..ced743ea6 100755
--- a/spec/unit/configurer/fact_handler_spec.rb
+++ b/spec/unit/configurer/fact_handler_spec.rb
@@ -61,6 +61,22 @@ describe Puppet::Configurer::FactHandler do
       @facthandler.find_facts.should == bar_facts
     end
 
+    it "should set the facts name based on the node_name_fact" do
+      Puppet::Node::Facts.new(Puppet[:node_name_value], 'my_name_fact' => 'other_node_name').save
+      Puppet[:node_name_fact] = 'my_name_fact'
+
+      @facthandler.find_facts.name.should == 'other_node_name'
+    end
+
+    it "should set the node_name_value based on the node_name_fact" do
+      Puppet::Node::Facts.new(Puppet[:node_name_value], 'my_name_fact' => 'other_node_name').save
+      Puppet[:node_name_fact] = 'my_name_fact'
+
+      @facthandler.find_facts
+
+      Puppet[:node_name_value].should == 'other_node_name'
+    end
+
     it "should reload Facter before finding facts" do
       @facthandler.expects(:reload_facter)
 
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index d454b5fce..b825e3404 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -103,6 +103,13 @@ describe Puppet::Configurer do
       @agent.run
     end
 
+    it "should respect node_name_fact when setting the host on a report" do
+      Puppet[:node_name_fact] = 'my_name_fact'
+      @facts.values = {'my_name_fact' => 'node_name_from_fact'}
+
+      @agent.run.host.should == 'node_name_from_fact'
+    end
+
     it "should pass the new report to the catalog" do
       report = Puppet::Transaction::Report.new("apply")
       Puppet::Transaction::Report.stubs(:new).returns report