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.

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"

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]

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

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.

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

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

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

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

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
     }
 

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

test/executables/puppetmasterd.rb

@@ -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
-

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]