Setting pluginsync default to false, and (hopefully) fixing autosign problem when the file exists and autosign is set to true (#180). The problem was that the puppetmasterd script was redundantly setting autosign in the CA, when the CA already knows how to deal with autosigning, which meant that autosign was being set before the config was parsed. Thus, there was no bug when autosign was set on the command line but there was when it was set in the config.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1304 980ebf18-57e1-0310-9a29-db15c13687c0

6 files changed, 62 insertions, 21 deletions

diff --git a/bin/puppetmasterd b/bin/puppetmasterd
index feaec5dbc..ff00d0c8f 100755
--- a/bin/puppetmasterd
+++ b/bin/puppetmasterd
@@ -164,8 +164,6 @@ rescue GetoptLong::InvalidOption => detail
     #end
     exit(1)
 end
-ca[:autosign] = Puppet[:autosign]
-
 
 # Handle the logging settings.
 if options[:debug] or options[:verbose]
diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index cadabe33b..70037729f 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -27,7 +27,7 @@ class Puppet::Client::MasterClient < Puppet::Client
             "From where to retrieve plugins.  The standard Puppet ``file`` type
              is used for retrieval, so anything that is a valid file source can
              be used here."],
-        :pluginsync => [true,
+        :pluginsync => [false,
             "Whether plugins should be synced with the central server."]
     )
 
diff --git a/lib/puppet/server/ca.rb b/lib/puppet/server/ca.rb
index 5c2d739b1..0aa31d937 100644
--- a/lib/puppet/server/ca.rb
+++ b/lib/puppet/server/ca.rb
@@ -16,28 +16,36 @@ class Server
             iface.add_method("array getcert(csr)")
         }
 
+        def autosign
+            if defined? @autosign
+                @autosign
+            else
+                Puppet[:autosign]
+            end
+        end
+
         # FIXME autosign? should probably accept both hostnames and IP addresses
         def autosign?(hostname)
             # simple values are easy
-            if @autosign == true or @autosign == false
-                return @autosign
+            if autosign == true or autosign == false
+                return autosign
             end
 
             # we only otherwise know how to handle files
-            unless @autosign =~ /^\//
+            unless autosign =~ /^\//
                 raise Puppet::Error, "Invalid autosign value %s" %
-                    @autosign.inspect
+                    autosign.inspect
             end
 
-            unless FileTest.exists?(@autosign)
+            unless FileTest.exists?(autosign)
                 unless defined? @@warnedonautosign
                     @@warnedonautosign = true
-                    Puppet.info "Autosign is enabled but %s is missing" % @autosign
+                    Puppet.info "Autosign is enabled but %s is missing" % autosign
                 end
                 return false
             end
             auth = Puppet::Server::AuthStore.new
-            File.open(@autosign) { |f|
+            File.open(autosign) { |f|
                 f.each { |line|
                     auth.allow(line.chomp)
                 }
@@ -49,7 +57,10 @@ class Server
 
         def initialize(hash = {})
             Puppet.config.use(:puppet, :certificates, :ca)
-            @autosign = hash[:autosign] || Puppet[:autosign]
+            if hash.include? :autosign
+                @autosign = hash[:autosign]
+            end
+
             @ca = Puppet::SSLCertificates::CA.new(hash)
         end
 
diff --git a/test/executables/puppetmasterd.rb b/test/executables/puppetmasterd.rb
index cbf8ac02a..92a8c6b84 100755
--- a/test/executables/puppetmasterd.rb
+++ b/test/executables/puppetmasterd.rb
@@ -74,14 +74,10 @@ class TestPuppetMasterD < Test::Unit::TestCase
             )
         }
         assert_nothing_raised() {
-            #Puppet.notice "calling status"
-            #retval = client.call("status.status", "")
             retval = client.getconfig
         }
 
         objects = nil
-        stopmasterd
-        sleep(1)
     end
 
     # verify that we can run puppetmasterd in parse-only mode
diff --git a/test/puppettest.rb b/test/puppettest.rb
index c65c95f49..6bdc41a75 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -462,6 +462,11 @@ module ExeTest
         assert($? == 0, "Puppetmasterd exit status was %s" % $?)
         sleep(1)
 
+        cleanup do
+            stopmasterd
+            sleep(1)
+        end
+
         return manifest
     end
 
@@ -483,7 +488,7 @@ module ExeTest
                 if line =~ /ruby.+puppetmasterd/
                     next if line =~ /\.rb/ # skip the test script itself
                     ary = line.sub(/^\s+/, '').split(/\s+/)
-                    runningpid = ary[1].to_i
+                    pid = ary[1].to_i
                 end
             }
 
diff --git a/test/server/ca.rb b/test/server/ca.rb
index 0009b3d32..b7bad5cd6 100644
--- a/test/server/ca.rb
+++ b/test/server/ca.rb
@@ -21,11 +21,6 @@ end
 
 class TestCA < Test::Unit::TestCase
 	include ServerTest
-    def teardown
-        super
-        #print "\n\n" if Puppet[:debug]
-    end
-
     # Verify that we're autosigning.  We have to autosign a "different" machine,
     # since we always autosign the CA server's certificate.
     def test_autocertgeneration
@@ -205,4 +200,40 @@ class TestCA < Test::Unit::TestCase
             )
         }
     end
+
+    # Make sure true/false causes the file to be ignored.
+    def test_autosign_true_beats_file
+        caserv = nil
+        assert_nothing_raised {
+            caserv = Puppet::Server::CA.new()
+        }
+
+        host = "hostname.domain.com"
+
+        # Create an autosign file
+        file = tempfile()
+        Puppet[:autosign] = file
+
+        File.open(file, "w") { |f|
+            f.puts host
+        }
+
+        # Start with "false"
+        Puppet[:autosign] = false
+
+        assert(! caserv.autosign?(host), "Host was incorrectly autosigned")
+
+        # Then set it to true
+        Puppet[:autosign] = true
+        assert(caserv.autosign?(host), "Host was not autosigned")
+        # And try a different host
+        assert(caserv.autosign?("other.yay.com"), "Host was not autosigned")
+
+        # And lastly the file
+        Puppet[:autosign] = file
+        assert(caserv.autosign?(host), "Host was not autosigned")
+
+        # And try a different host
+        assert(! caserv.autosign?("other.yay.com"), "Host was autosigned")
+    end
 end