Made tweaks here and there to get it running better on my local network. I am inches away from that happening. All tests pass.

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

15 files changed, 124 insertions, 99 deletions

diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index 040292b36..509d6a69d 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -127,6 +127,7 @@ module Puppet
                 Puppet.err "Cannot request a certificate without a defined target"
                 return false
             end
+
             Puppet.info "Creating a new certificate request for %s" % @fqdn
             name = OpenSSL::X509::Name.new([["CN", @fqdn]])
 
diff --git a/lib/puppet/metric.rb b/lib/puppet/metric.rb
index f2d754a57..3feb4484c 100644
--- a/lib/puppet/metric.rb
+++ b/lib/puppet/metric.rb
@@ -1,15 +1,12 @@
-#!/usr/local/bin/ruby -w
-
-# $Id$
-
 # included so we can test object types
 require 'puppet'
 
-module Puppet
+module Puppet # :nodoc:
+    # A class for handling metrics.  This is currently ridiculously hackish.
 	class Metric
         def Metric.init
-            @@typemetrics = Hash.new { |typehash,type|
-                typehash[type] = Hash.new(0)
+            @@typemetrics = Hash.new { |typehash,typename|
+                typehash[typename] = Hash.new(0)
             }
 
             @@eventmetrics = Hash.new(0)
@@ -27,9 +24,10 @@ module Puppet
             # first gather stats about all of the types
             Puppet::Type.eachtype { |type|
                 type.each { |instance|
-                    @@typemetrics[type][:total] += 1
+                    hash = @@typemetrics[type]
+                    hash[:total] += 1
                     if instance.managed?
-                        @@typemetrics[type][:managed] += 1
+                        hash[:managed] += 1
                     end
                 }
             }
@@ -46,7 +44,7 @@ module Puppet
                 @@typemetrics[type][:changed] += 1
                 @@typemetrics[type][:totalchanges] += count
             else
-                raise "Unknown metric %s" % metric
+                raise Puppet::DevError, "Unknown metric %s" % metric
             end
         end
 
@@ -58,12 +56,14 @@ module Puppet
             }
         end
 
+        # Iterate across all of the metrics
         def Metric.each
             @@metrics.each { |name,metric|
                 yield metric
             }
         end
 
+        # I'm nearly positive this method is used only for testing
         def Metric.load(ary)
             @@typemetrics = ary[0]
             @@eventmetrics = ary[1]
@@ -246,3 +246,5 @@ module Puppet
         end
     end
 end
+
+# $Id$
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index e076bc1ca..e6a0bdc2b 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -41,6 +41,13 @@ module Puppet
                 # Really, we should stick multiple names in here
                 # but for now just make a simple array
                 names = [client]
+
+                # if the client name is fully qualied (which is normally will be)
+                # add the short name
+                if client =~ /\./
+                    names << client.sub(/\..+/,'')
+                end
+
                 begin
                     if @usenodes
                         unless client
diff --git a/lib/puppet/server.rb b/lib/puppet/server.rb
index 6737b2664..b7ed97799 100644
--- a/lib/puppet/server.rb
+++ b/lib/puppet/server.rb
@@ -98,6 +98,7 @@ module Puppet
     class Server
         # the base class for the different handlers
         class Handler
+            attr_accessor :server
             @subclasses = []
 
             def self.each
diff --git a/lib/puppet/server/authstore.rb b/lib/puppet/server/authstore.rb
index 966bc8372..075f1f9a1 100755
--- a/lib/puppet/server/authstore.rb
+++ b/lib/puppet/server/authstore.rb
@@ -66,6 +66,7 @@ class Server
                 }
             }
 
+            Puppet.info "Defaulting to false for %s" % name
             # default to false
             return false
         end
diff --git a/lib/puppet/server/ca.rb b/lib/puppet/server/ca.rb
index 04096a216..df2614e7c 100644
--- a/lib/puppet/server/ca.rb
+++ b/lib/puppet/server/ca.rb
@@ -19,65 +19,42 @@ class Server
         # FIXME autosign? should probably accept both hostnames and IP addresses
         def autosign?(hostname)
             # simple values are easy
-            asign = Puppet[:autosign]
-            if asign == true or asign == false
-                return asign
+            if @autosign == true or @autosign == false
+                return @autosign
             end
 
             # we only otherwise know how to handle files
-            unless asign =~ /^\//
+            unless @autosign =~ /^\//
                 raise Puppet::Error, "Invalid autosign value %s" %
-                    asign
+                    @autosign
             end
 
-            unless FileTest.exists?(asign)
-                Puppet.warning "Autosign is enabled but %s is missing" % asign
+            unless FileTest.exists?(@autosign)
+                Puppet.info "Autosign is enabled but %s is missing" % @autosign
                 return false
             end
             auth = Puppet::Server::AuthStore.new
-            File.open(asign) { |f|
+            File.open(@autosign) { |f|
                 f.each { |line|
                     auth.allow(line.chomp)
-#                    if line =~ /^[.\w-]+$/ and line == hostname
-#                        Puppet.info "%s exactly matched %s" % [hostname, line]
-#                        return true
-#                    else
-#                        begin
-#                            rx = Regexp.new(line)
-#                        rescue => detail
-#                            Puppet.err(
-#                                "Could not create regexp out of autosign line %s: %s" %
-#                                [line, detail]
-#                            )
-#                            next
-#                        end
-#
-#                        if hostname =~ rx
-#                            Puppet.info "%s matched %s" % [hostname, line]
-#                            return true
-#                        end
-#                    end
                 }
             }
 
             # for now, just cheat and pass a fake IP address to allowed?
-            return auth.allowed?(hostname, "127.0.0.1")
+            return auth.allowed?(hostname, "127.1.1.1")
         end
 
         def initialize(hash = {})
+            @autosign = hash[:autosign] || Puppet[:autosign]
             @ca = Puppet::SSLCertificates::CA.new()
         end
 
         # our client sends us a csr, and we either store it for later signing,
         # or we sign it right away
         def getcert(csrtext, client = nil, clientip = nil)
-            # okay, i need to retrieve the hostname from the csr, and then
-            # verify that i get the same hostname through reverse lookup or
-            # something
-
-            Puppet.info "Someone's trying for a cert"
             csr = OpenSSL::X509::Request.new(csrtext)
 
+            # Use the hostname from the CSR, not from the network.
             subject = csr.subject
 
             nameary = subject.to_a.find { |ary|
@@ -85,7 +62,9 @@ class Server
             }
 
             if nameary.nil?
-                Puppet.err "Invalid certificate request"
+                Puppet.err(
+                    "Invalid certificate request: could not retrieve server name"
+                )
                 return "invalid"
             end
 
@@ -129,10 +108,13 @@ class Server
             cert, cacert = ca.getclientcert(hostname)
             if cert and cacert
                 Puppet.info "Retrieving existing certificate for %s" % hostname
-                Puppet.info "Cert: %s; Cacert: %s" % [cert.class, cacert.class]
+                #Puppet.info "Cert: %s; Cacert: %s" % [cert.class, cacert.class]
                 return [cert.to_pem, cacert.to_pem]
             elsif @ca
-                if self.autosign?(hostname)
+                if self.autosign?(hostname) or client.nil?
+                    if client.nil?
+                        Puppet.info "Signing certificate for CA server"
+                    end
                     # okay, we don't have a signed cert
                     # if we're a CA and autosign is turned on, then go ahead and sign
                     # the csr and return the results
diff --git a/lib/puppet/server/servlet.rb b/lib/puppet/server/servlet.rb
index ce962b4ea..703e984b0 100644
--- a/lib/puppet/server/servlet.rb
+++ b/lib/puppet/server/servlet.rb
@@ -66,13 +66,18 @@ class Server
 
             @loadedhandlers = []
             handlers.each { |handler|
-                Puppet.debug "adding handler for %s" % handler.class
+                #Puppet.debug "adding handler for %s" % handler.class
                 self.add_handler(handler.class.interface, handler)
             }
 
+            # Initialize these to nil, but they will get set to values
+            # by the 'service' method.  These have to instance variables
+            # because I don't have a clear line from the service method to
+            # the service hook.
             @request = nil
             @client = nil
             @clientip = nil
+
             self.set_service_hook { |obj, *args|
                 #raise "crap!"
                 if @client and @clientip
@@ -81,6 +86,8 @@ class Server
                 end
                 begin
                     obj.call(*args)
+                rescue XMLRPC::FaultException
+                    raise
                 rescue Puppet::Server::AuthorizationError => detail
                     #Puppet.warning obj.inspect
                     #Puppet.warning args.inspect
@@ -99,13 +106,17 @@ class Server
                     #Puppet.warning obj.inspect
                     #Puppet.warning args.inspect
                     Puppet.err "Could not call: %s" % detail.to_s
-                    raise error
+                    raise XMLRPC::FaultException.new(1, detail.to_s)
                 end
             }
         end
 
+        # Handle the actual request.  This does some basic collection of
+        # data, and then just calls the parent method.
         def service(request, response)
             @request = request
+
+            # The only way that @client can be nil is if the request is local.
             if peer = request.peeraddr
                 @client = peer[2]
                 @clientip = peer[3]
@@ -120,17 +131,17 @@ class Server
             # then we get the hostname from the cert, instead of via IP
             # info
             if cert = request.client_cert
-                name = cert.subject
-                #Puppet.info name.inspect
-                if name.to_s =~ /CN=(\w+)/
-                    Puppet.info "Overriding %s with cert name %s" %
-                        [@client, $1]
-                    @client = $1
+                nameary = cert.subject.to_a.find { |ary|
+                    ary[0] == "CN"
+                }   
+
+                if nameary.nil?
+                    Puppet.warning "Could not retrieve server name from cert"
                 else
-                    Puppet.warning "Could not match against %s(%s)" %
-                        [name, name.class]
+                    Puppet.debug "Overriding %s with cert name %s" %
+                        [@client, nameary[1]]
+                    @client = nameary[1]
                 end
-                #Puppet.info "client cert is %s" % request.client_cert
             end
             #if request.server_cert
             #    Puppet.info "server cert is %s" % @request.server_cert
diff --git a/lib/puppet/sslcertificates.rb b/lib/puppet/sslcertificates.rb
index bc3382ab6..6dcd25656 100755
--- a/lib/puppet/sslcertificates.rb
+++ b/lib/puppet/sslcertificates.rb
@@ -207,7 +207,7 @@ module SSLCertificates
             :serial         => [:cadir,          "serial"],
             :privatedir     => [:ssldir,         "private"],
             :passfile       => [:privatedir,     "password"],
-            :autosign       => [:ssldir,         "autosign.conf"],
+            :autosign       => [:puppetconf,     "autosign.conf"],
             :ca_crl_days    => 365,
             :ca_days        => 1825,
             :ca_md          => "md5",
@@ -443,7 +443,8 @@ module SSLCertificates
 
             certfile = host2certfile(host)
             if File.exists?(certfile)
-                Puppet.notice "Overwriting signed certificate for %s" % host
+                Puppet.notice "Overwriting signed certificate %s for %s" %
+                    [certfile, host]
             end
 
             File.open(certfile, "w", 0660) { |f|
diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb
index e1b59e019..902f2ee04 100644
--- a/lib/puppet/statechange.rb
+++ b/lib/puppet/statechange.rb
@@ -36,8 +36,8 @@ module Puppet
             end
 
             if @state.noop
-                @state.parent.log "%s should be %s" %
-                    [@state, state.should_to_s]
+                @state.parent.log "%s is %s, should be %s" %
+                    [@state, state.is_to_s, state.should_to_s]
                 #Puppet.debug "%s is noop" % @state
                 return nil
             end
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index d86e775cd..23bfecee3 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -43,7 +43,7 @@ module Puppet
             end
 
             if defined? @tags and @tags
-                Puppet.warning "%s(%s) tags: %s" % [@type, @name, @tags.join(" ")]
+                Puppet.debug "%s(%s) tags: %s" % [@type, @name, @tags.join(" ")]
             end
 
             return retobj
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index ab5b797c6..296e8af54 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -25,10 +25,8 @@ module Puppet
             # Override the parent method, because we've got all kinds of
             # funky definitions of 'in sync'.
             def insync?
-                Puppet.debug "is: %s" % @is
                 # Iterate across all of the should values, and see how they turn out.
                 @should.each { |should|
-                    Puppet.debug "should: %s" % should
                     case should
                     when :installed
                         unless @is == :notinstalled
@@ -39,7 +37,7 @@ module Puppet
                         if @is == latest
                             return true
                         else
-                            Puppet.warning "latest is %s" % latest
+                            Puppet.debug "latest is %s" % latest
                         end
                     when :notinstalled
                         if @is == :notinstalled
@@ -65,6 +63,7 @@ module Puppet
                 case value
                 when "latest":
                     unless @parent.respond_to?(:latest)
+                        Puppet.err @parent.inspect
                         raise Puppet::Error,
                             "Package type %s does not support querying versions" %
                             @parent[:type]
@@ -326,6 +325,9 @@ module Puppet
             # be set in 'should', or through comparing against the system, in which
             # case the hash's values should be set in 'is'
             def initialize(hash)
+                type = hash["type"] || hash[:type] || self.class.default
+                self.type2module(type)
+
                 super
 
                 unless @states.include?(:install)
@@ -338,18 +340,6 @@ module Puppet
                 end
             end
 
-            # Set the package type parameter.  Looks up the corresponding
-            # module and then extends the 'install' state.
-            def paramtype=(typename)
-                if type = self.class.pkgtype(typename)
-                    Puppet.debug "Extending %s with %s" % [self.name, type]
-                    self.extend(type)
-                    @parameters[:type] = type
-                else
-                    raise Puppet::Error, "Invalid package type %s" % typename
-                end
-            end
-
             def retrieve
                 if hash = self.query
                     hash.each { |param, value|
@@ -367,6 +357,16 @@ module Puppet
                     }
                 end
             end
+
+            # Extend the package with the appropriate package type.
+            def type2module(typename)
+                if type = self.class.pkgtype(typename)
+                    Puppet.debug "Extending to package type %s" % [type]
+                    self.extend(type)
+                else
+                    raise Puppet::Error, "Invalid package type %s" % typename
+                end
+            end
         end # Puppet::Type::Package
     end
 
diff --git a/lib/puppet/type/pfile/group.rb b/lib/puppet/type/pfile/group.rb
index f38860259..3444d2ec3 100755
--- a/lib/puppet/type/pfile/group.rb
+++ b/lib/puppet/type/pfile/group.rb
@@ -54,15 +54,14 @@ module Puppet
                         "Could not retrieve gid for %s" % @parent.name)
                 end
 
-                # now make sure the user is allowed to change to that group
-                unless Process.uid == 0
-                    groups = %x{groups}.chomp.split(/\s/)
-                    unless groups.include?(gname)
-                        Puppet.notice "Cannot chgrp: not in group %s" % gname
-                        raise Puppet::Error.new(
-                            "Cannot chgrp: not in group %s" % gname)
-                    end
-                end
+                #unless Process.uid == 0
+                #    groups = %x{groups}.chomp.split(/\s/)
+                #    unless groups.include?(gname)
+                #        Puppet.notice "Cannot chgrp: not in group %s" % gname
+                #        raise Puppet::Error.new(
+                #            "Cannot chgrp: not in group %s" % gname)
+                #    end
+                #end
 
                 if gid.nil?
                     raise Puppet::Error.new(
@@ -76,15 +75,18 @@ module Puppet
             # we'll just let it fail, but we should probably set things up so
             # that users get warned if they try to change to an unacceptable group.
             def sync
-                #unless Process.uid == 0
-                #    unless defined? @@notifiedgroup
-                #        Puppet.notice(  
-                #            "Cannot manage group ownership unless running as root"
-                #        )
-                #        @@notifiedgroup = true
-                #    end
-                #    return nil
-                #end
+                # now make sure the user is allowed to change to that group
+                # We don't do this in the should section, so it can still be used
+                # for noop.
+                unless Process.uid == 0
+                    unless defined? @@notifiedgroup
+                        Puppet.notice(  
+                            "Cannot manage group ownership unless running as root"
+                        )
+                        @@notifiedgroup = true
+                    end
+                    return nil
+                end
 
                 if @is == :notfound
                     @parent.stat(true)
diff --git a/lib/puppet/type/pfile/source.rb b/lib/puppet/type/pfile/source.rb
index ec4f7107b..db2055172 100755
--- a/lib/puppet/type/pfile/source.rb
+++ b/lib/puppet/type/pfile/source.rb
@@ -125,7 +125,7 @@ module Puppet
                         @parent.state(:create).retrieve
                     end
                     # we'll let the :create state do our work
-                    @should = nil
+                    @should.clear
                     @is = true
                 # FIXME We should at least support symlinks, I would think...
                 else
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index df3609af3..49da3ce00 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -179,8 +179,6 @@ module Puppet
                     end
                 end
 
-                Puppet.err @defsvctype
-
                 return @defsvctype
             end
 
@@ -236,6 +234,11 @@ module Puppet
 
                 super
 
+                unless @parameters.include?(:pattern)
+                    # default to using the service name as the pattern
+                    self[:pattern] = self.name
+                end
+
                 # and then see if it needs to be checked
                 if self.respond_to?(:configchk)
                     self.configchk
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index 7f74f7344..34ffd63e7 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -56,6 +56,11 @@ class State < Puppet::Element
             raise Puppet::DevError, "%s's should is not array" % self.class.name
         end
 
+        # an empty array is analogous to no should values
+        if @should.empty?
+            return true
+        end
+
         # Look for a matching value
         @should.each { |val|
             if @is == val
@@ -93,7 +98,16 @@ class State < Puppet::Element
 
     # Only return the first value
     def should
-        return @should[0]
+        if defined? @should
+            unless @should.is_a?(Array)
+                Puppet.warning @should.inspect
+                raise Puppet::DevError, "should for %s on %s is not an array" %
+                    [self.class.name, @parent.name]
+            end
+            return @should[0]
+        else
+            return nil
+        end
     end
 
     # Set the should value.