The master and client now successfully speak xmlrpc using the new system.

The server is actually serving REST, but the client can't use
it until we resolve the format and security issues that REST
hasn't yet tackled.

12 files changed, 167 insertions, 172 deletions

diff --git a/bin/puppetd b/bin/puppetd
index f652e6b08..b92773c76 100755
--- a/bin/puppetd
+++ b/bin/puppetd
@@ -317,6 +317,11 @@ if options[:centrallogs]
     Puppet::Util::Log.newdestination(logdest)
 end
 
+# We need to specify a ca location for things to work, but
+# until the REST cert transfers are working, it needs to
+# be local.
+Puppet::SSL::Host.ca_location = :local
+
 # We need tomake the client either way, we just don't start it
 # if --no-client is set.
 client = Puppet::Network::Client.master.new(args)
@@ -338,10 +343,9 @@ if Puppet[:daemonize]
     client.daemonize
 end
 
-unless Puppet::Network::HttpPool.read_cert
-    # If we don't already have the certificate, then create a client to
-    # request one.  Use the special ca stuff, don't use the normal server and port.
-    caclient = Puppet::Network::Client.ca.new()
+caclient = Puppet::Network::Client.ca.new()
+
+unless caclient.read_cert
     if options[:waitforcert] > 0
         begin
             while ! caclient.request_cert do
@@ -360,7 +364,7 @@ unless Puppet::Network::HttpPool.read_cert
     end
 
     # Now read the new cert in.
-    if Puppet::Network::HttpPool.read_cert
+    if caclient.read_cert
         # If we read it in, then get rid of our existing http connection.
         client.recycle_connection
         Puppet.notice "Got signed certificate"
diff --git a/bin/puppetmasterd b/bin/puppetmasterd
index 57fc4bdb4..625b75d52 100755
--- a/bin/puppetmasterd
+++ b/bin/puppetmasterd
@@ -160,6 +160,15 @@ Puppet::Node::Facts.terminus_class = :yaml
 # Cache our nodes in yaml.  Currently not configurable.
 Puppet::Node.cache_class = :yaml
 
+# Configure all of the SSL stuff.
+if Puppet::SSL::CertificateAuthority.ca?
+    Puppet::SSL::Host.ca_location = :local
+    Puppet.settings.use :main, :ssl, :ca
+    Puppet::SSL::CertificateAuthority.instance
+else
+    Puppet::SSL::Host.ca_location = :none
+end
+
 require 'etc'
 
 if Puppet[:parseonly]
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 2a2a81be6..fa604667e 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -60,12 +60,6 @@ module Puppet
                 this directory can be removed without causing harm (although it
                 might result in spurious service restarts)."
         },
-        :ssldir => {
-            :default => "$confdir/ssl",
-            :mode => 0771,
-            :owner => "root",
-            :desc => "Where SSL certificates are kept."
-        },
         :rundir => { 
             :default => rundir,
             :mode => 01777,
@@ -172,7 +166,7 @@ module Puppet
         fqdn = hostname
     end
 
-    Puppet.setdefaults(:ssl,
+    Puppet.setdefaults(:main,
         :certname => [fqdn, "The name to use when handling certificates.  Defaults
             to the fully qualified domain name."],
         :certdnsnames => ['', "The DNS names on the Server certificate as a colon-separated list.
@@ -181,6 +175,12 @@ module Puppet
         :certdir => ["$ssldir/certs", "The certificate directory."],
         :crl => [true, "Whether to use a certificate revocation list.  If this is set to true and the CRL does not exist,
             you will get a failure."],
+        :ssldir => {
+            :default => "$confdir/ssl",
+            :mode => 0771,
+            :owner => "root",
+            :desc => "Where SSL certificates are kept."
+        },
         :publickeydir => ["$ssldir/public_keys", "The public key directory."],
         :requestdir => ["$ssldir/certificate_requests", "Where host certificate requests are stored."],
         :privatekeydir => { :default => "$ssldir/private_keys",
@@ -286,7 +286,7 @@ module Puppet
         :serial => { :default => "$cadir/serial",
             :owner => "$user",
             :group => "$group",
-            :mode => 0600,
+            :mode => 0644,
             :desc => "Where the serial number for certificates is stored."
         },
         :autosign => { :default => "$confdir/autosign.conf",
@@ -319,7 +319,7 @@ module Puppet
     self.setdefaults(self.settings[:name],
         :config => ["$confdir/puppet.conf",
             "The configuration file for #{Puppet[:name]}."],
-        :pidfile => ["", "The pid file"],
+        :pidfile => ["$rundir/$name.pid", "The pid file"],
         :bindaddress => ["", "The address to bind to.  Mongrel servers
             default to 127.0.0.1 and WEBrick defaults to 0.0.0.0."],
         :servertype => ["webrick", "The type of server to use.  Currently supported
diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb
index 0329f5354..5054c1dbe 100644
--- a/lib/puppet/ssl/certificate_authority.rb
+++ b/lib/puppet/ssl/certificate_authority.rb
@@ -16,11 +16,16 @@ class Puppet::SSL::CertificateAuthority
 
     require 'puppet/ssl/certificate_authority/interface'
 
+    def self.ca?
+        return false unless Puppet[:ca]
+        return false unless Puppet[:name] == "puppetmasterd"
+        return true
+    end
+
     # If this process can function as a CA, then return a singleton
     # instance.
     def self.instance
-        return nil unless Puppet[:ca]
-        return nil unless Puppet[:name] == "puppetmasterd"
+        return nil unless ca?
 
         unless defined?(@instance) and @instance
             @instance = new
@@ -177,11 +182,17 @@ class Puppet::SSL::CertificateAuthority
     # file so this one is considered used.
     def next_serial
         serial = nil
+
+        # This is slightly odd.  If the file doesn't exist, our readwritelock creates
+        # it, but with a mode we can't actually read in some cases.  So, use
+        # a default before the lock.
+        unless FileTest.exist?(Puppet[:serial])
+            serial = 0x0
+        end
+
         Puppet.settings.readwritelock(:serial) { |f|
             if FileTest.exist?(Puppet[:serial])
-                serial = File.read(Puppet.settings[:serial]).chomp.hex
-            else
-                serial = 0x0
+                serial ||= File.read(Puppet.settings[:serial]).chomp.hex
             end
 
             # We store the next valid serial, not the one we just used.
diff --git a/lib/puppet/ssl/certificate_factory.rb b/lib/puppet/ssl/certificate_factory.rb
index 4b1669804..41155fd41 100644
--- a/lib/puppet/ssl/certificate_factory.rb
+++ b/lib/puppet/ssl/certificate_factory.rb
@@ -115,7 +115,7 @@ class Puppet::SSL::CertificateFactory
         dnsnames = Puppet[:certdnsnames]
         name = @name.to_s.sub(%r{/CN=},'')
         if dnsnames != ""
-            dnsnames.split(':').each { |d| subject_alt_name << 'DNS:' + d }
+            dnsnames.split(':').each { |d| @subject_alt_name << 'DNS:' + d }
             @subject_alt_name << 'DNS:' + name # Add the fqdn as an alias
         elsif name == Facter.value(:fqdn) # we're a CA server, and thus probably the server
             @subject_alt_name << 'DNS:' + "puppet" # Add 'puppet' as an alias
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index 09bba5b59..1b953c95e 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -699,13 +699,19 @@ Generated on #{Time.now}.
                             [file]
                     end
 
-                    writesub(default, tmpfile, *args, &bloc)
+                    # If there's a failure, remove our tmpfile
+                    begin
+                        writesub(default, tmpfile, *args, &bloc)
+                    rescue
+                        File.unlink(tmpfile) if FileTest.exist?(tmpfile)
+                        raise
+                    end
 
                     begin
                         File.rename(tmpfile, file)
                     rescue => detail
-                        Puppet.err "Could not rename %s to %s: %s" %
-                            [file, tmpfile, detail]
+                        Puppet.err "Could not rename %s to %s: %s" % [file, tmpfile, detail]
+                        File.unlink(tmpfile) if FileTest.exist?(tmpfile)
                     end
                 end
             end
diff --git a/spec/integration/bin/puppetmasterd.rb b/spec/integration/bin/puppetmasterd.rb
new file mode 100755
index 000000000..447344472
--- /dev/null
+++ b/spec/integration/bin/puppetmasterd.rb
@@ -0,0 +1,109 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+describe "puppetmasterd" do
+    before do
+        # Get a safe temporary file
+        file = Tempfile.new("puppetmaster_integration_testing")
+        @dir = file.path
+        file.delete
+
+        Dir.mkdir(@dir)
+
+        Puppet.settings[:confdir] = @dir
+        Puppet.settings[:vardir] = @dir
+        Puppet[:certdnsnames] = "localhost"
+
+        @@port = 12345
+    end
+
+    after {
+        stop
+
+        Puppet::SSL::Host.ca_location = :none
+
+        system("rm -rf %s" % @dir)
+        Puppet.settings.clear
+    }
+
+    def arguments
+        rundir = File.join(Puppet[:vardir], "run")
+        @pidfile = File.join(rundir, "testing.pid")
+        args = ""
+        args += " --confdir %s" % Puppet[:confdir]
+        args += " --rundir %s" % rundir
+        args += " --pidfile %s" % @pidfile
+        args += " --vardir %s" % Puppet[:vardir]
+        args += " --certdnsnames %s" % Puppet[:certdnsnames]
+        args += " --masterport %s" % @@port
+        args += " --user %s" % Puppet::Util::SUIDManager.uid
+        args += " --group %s" % Puppet::Util::SUIDManager.gid
+        args += " --autosign true"
+    end
+
+    def start(addl_args = "")
+        Puppet.settings.mkdir(:manifestdir)
+        Puppet.settings.write(:manifest) do |f|
+            f.puts { "notify { testing: }" }
+        end
+
+        args = arguments + addl_args
+
+        output = %x{puppetmasterd #{args}}.chomp
+    end
+
+    def stop
+        if @pidfile and FileTest.exist?(@pidfile)
+            pid = File.read(@pidfile).chomp.to_i
+            Process.kill(:TERM, pid)
+        end
+    end
+
+    it "should create a PID file" do
+        start
+
+        FileTest.exist?(@pidfile).should be_true
+    end
+
+    it "should be serving status information over REST"
+
+    it "should be serving status information over xmlrpc" do
+        start
+
+        sleep 0.5
+
+        client = Puppet::Network::Client.status.new(:Server => "localhost", :Port => @@port)
+
+        FileUtils.mkdir_p(File.dirname(Puppet[:autosign]))
+        File.open(Puppet[:autosign], "w") { |f|
+            f.puts Puppet[:certname]
+        }
+
+        client.cert
+        retval = client.status
+
+        retval.should == 1
+    end
+
+    it "should exit with return code 0 after parsing if --parseonly is set and there are no errors" do
+        start(" --parseonly > /dev/null")
+        sleep(1)
+
+        ps = Facter["ps"].value || "ps -ef"
+        pid = nil
+        %x{#{ps}}.chomp.split(/\n/).each { |line|
+            next if line =~ /^puppet/ # skip normal master procs
+            if line =~ /puppetmasterd.+--manifest/
+                ary = line.split(" ")
+                pid = ary[1].to_i
+            end
+        }
+
+        $?.should == 0
+
+        pid.should be_nil
+    end
+
+    it "should exit with return code 1 after parsing if --parseonly is set and there are errors"
+end
diff --git a/spec/integration/network/server/webrick.rb b/spec/integration/network/server/webrick.rb
index ab9e94605..f2b55ef92 100755
--- a/spec/integration/network/server/webrick.rb
+++ b/spec/integration/network/server/webrick.rb
@@ -28,6 +28,8 @@ describe Puppet::Network::Server do
             @tmpfile.delete
             Puppet.settings.clear
 
+            system("rm -rf %s" % @dir)
+
             # This is necessary so the terminus instances don't lie around.
             Puppet::SSL::Key.indirection.clear_cache
             Puppet::SSL::Certificate.indirection.clear_cache
diff --git a/spec/integration/ssl/host.rb b/spec/integration/ssl/host.rb
index 5e457bded..d4834c341 100755
--- a/spec/integration/ssl/host.rb
+++ b/spec/integration/ssl/host.rb
@@ -32,6 +32,8 @@ describe Puppet::SSL::Host do
 
         # This is necessary so the terminus instances don't lie around.
         Puppet::SSL::Key.indirection.clear_cache
+        Puppet::SSL::Certificate.indirection.clear_cache
+        Puppet::SSL::CertificateRevocationList.indirection.clear_cache
         Puppet::SSL::CertificateRequest.indirection.clear_cache
     }
 
diff --git a/spec/unit/ssl/certificate_authority.rb b/spec/unit/ssl/certificate_authority.rb
index b0be0e450..a4581fb21 100755
--- a/spec/unit/ssl/certificate_authority.rb
+++ b/spec/unit/ssl/certificate_authority.rb
@@ -293,7 +293,7 @@ describe Puppet::SSL::CertificateAuthority do
             end
 
             it "should return the current content of the serial file" do
-                FileTest.expects(:exist?).with(@path).returns true
+                FileTest.stubs(:exist?).with(@path).returns true
                 File.expects(:read).with(@path).returns "0002"
 
                 @ca.next_serial.should == 2
diff --git a/test/executables/puppetmasterd.rb b/test/executables/puppetmasterd.rb
deleted file mode 100755
index 6d4ddf56f..000000000
--- a/test/executables/puppetmasterd.rb
+++ /dev/null
@@ -1,147 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../lib/puppettest'
-
-require 'puppet'
-require 'puppet/network/client'
-require 'puppettest'
-require 'socket'
-
-class TestPuppetMasterD < Test::Unit::TestCase
-    include PuppetTest::ExeTest
-    def setup
-        super
-        Puppet[:certdnsnames] = "localhost"
-    end
-
-    def getcerts
-        include Puppet::Daemon
-        if self.readcerts
-            return [@cert, @key, @cacert, @cacertfile]
-        else
-            raise "Couldn't read certs"
-        end
-    end
-
-    # start the daemon and verify it responds and such
-    def test_normalstart
-        startmasterd
-
-        pidfile = File.join(Puppet[:vardir], "run", "puppetmasterd.pid")
-        assert(FileTest.exists?(pidfile), "PID file does not exist")
-
-        sleep(1)
-        assert_nothing_raised {
-            socket = TCPSocket.new("127.0.0.1", @@port)
-            socket.close
-        }
-
-        client = nil
-        assert_nothing_raised() {
-            client = Puppet::Network::Client.status.new(
-                :Server => "localhost",
-                :Port => @@port
-            )
-        }
-
-        # set our client up to auto-sign
-        assert(Puppet[:autosign] =~ /^#{File::SEPARATOR}/,
-            "Autosign is set to %s, not a file" % Puppet[:autosign])
-
-        FileUtils.mkdir_p(File.dirname(Puppet[:autosign]))
-        File.open(Puppet[:autosign], "w") { |f|
-            f.puts Puppet[:certname]
-        }
-
-        retval = nil
-
-        # init the client certs
-        assert_nothing_raised() {
-            client.cert
-        }
-
-        # call status
-        assert_nothing_raised() {
-            retval = client.status
-        }
-        assert_equal(1, retval, "Status.status return value was %s" % retval)
-
-        # this client shoulduse the same certs
-        assert_nothing_raised() {
-            client = Puppet::Network::Client.master.new(
-                :Server => "localhost",
-                :Port => @@port
-            )
-        }
-        assert_nothing_raised() {
-            retval = client.getconfig
-        }
-
-        objects = nil
-    end
-
-    # verify that we can run puppetmasterd in parse-only mode
-    def test_parseonly
-        startmasterd("--parseonly > /dev/null")
-        sleep(1)
-
-        pid = nil
-        ps = Facter["ps"].value || "ps -ef"
-        %x{#{ps}}.chomp.split(/\n/).each { |line|
-            next if line =~ /^puppet/ # skip normal master procs
-            if line =~ /puppetmasterd.+--manifest/
-                ary = line.split(" ")
-                pid = ary[1].to_i
-            end
-        }
-
-        assert($? == 0, "Puppetmasterd ended with non-zero exit status")
-
-        assert_nil(pid, "Puppetmasterd is still running after parseonly")
-    end
-
-    def disabled_test_sslconnection
-        #file = File.join(exampledir, "code", "head")
-        #startmasterd("--manifest #{file}")
-
-        #assert_nothing_raised {
-        #    socket = TCPSocket.new("127.0.0.1", Puppet[:masterport])
-        #    socket.close
-        #}
-
-        client = nil
-        cert, key, cacert, cacertfile = getcerts()
-
-        assert_nothing_raised() {
-            client = Net::HTTP.new("localhost", Puppet[:masterport])
-            client.cert = cert
-            client.key = key
-            client.ca_file = cacertfile
-            client.use_ssl = true
-            client.start_immediately = true
-        }
-        retval = nil
-
-        assert_nothing_raised() {
-            retval = client.nothing
-        }
-        assert_equal(1, retval, "return value was %s" % retval)
-        facts = {}
-        Facter.each { |p,v|
-            facts[p] = v
-        }
-        textfacts = CGI.escape(YAML.dump(facts))
-        assert_nothing_raised() {
-            #Puppet.notice "calling status"
-            #retval = client.call("status.status", "")
-            retval = client.call("puppetmaster.getconfig", textfacts, "yaml")
-        }
-
-        objects = nil
-        assert_nothing_raised {
-            YAML.load(CGI.unescape(retval))
-        }
-        #stopmasterd
-    end
-end
-
diff --git a/test/lib/puppettest/exetest.rb b/test/lib/puppettest/exetest.rb
index 05de56c0f..0d66c5a07 100644
--- a/test/lib/puppettest/exetest.rb
+++ b/test/lib/puppettest/exetest.rb
@@ -58,7 +58,6 @@ module PuppetTest::ExeTest
         args += " --masterport %s" % @@port
         args += " --user %s" % Puppet::Util::SUIDManager.uid
         args += " --group %s" % Puppet::Util::SUIDManager.gid
-        args += " --nonodes"
         args += " --autosign true"
 
         #if Puppet[:debug]