Converting transport format to YAML instead of Marshal, and caching the file in a YAML format, also. This required a significant rework of both Transportable classes. Lastly, I am also now caching the list of classes in a class file in /etc/puppet.

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

11 files changed, 232 insertions, 61 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index 22f7c73d0..f0801968c 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -80,7 +80,8 @@ PUPPETVERSION = '0.10.2'
         # then the files},
         :manifestdir    => [:puppetconf,     "manifests"],
         :manifest       => [:manifestdir,    "site.pp"],
-        :localconfig    => [:puppetconf,     "localconfig.ma"],
+        :localconfig    => [:puppetconf,     "localconfig"],
+        :classfile      => [:puppetconf,     "classes.txt"],
         :logfile        => [:logdir,         "puppet.log"],
         :httplogfile    => [:logdir,         "http.log"],
         :masterlog      => [:logdir,         "puppetmaster.log"],
diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index 15c2acdd7..a0fb994d8 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -18,6 +18,7 @@ begin
     require 'cgi'
     require 'xmlrpc/client'
     require 'xmlrpc/server'
+    require 'yaml'
 rescue LoadError => detail
     $noclientnetworking = detail
     raise Puppet::Error, "You must have the Ruby XMLRPC, CGI, and Webrick libraries installed"
@@ -257,25 +258,18 @@ module Puppet
                     end
                 end
 
-                # FIXME this should be in getconfig, not apply
-                container = @objects.to_type
-
-                # Now perform any necessary final actions before we evaluate.
-                Puppet::Type.finalize
-                #if @local
-                #    container = @objects.to_type
-                #else
-                #    container = Marshal::load(@objects).to_type
-                #end
-
+                #Puppet.err :yay
+                #p @objects
+                #Puppet.err :mark
+                #@objects = @objects.to_type
                 # this is a gross hack... but i don't see a good way around it
                 # set all of the variables to empty
                 Puppet::Transaction.init
 
-                # for now we just evaluate the top-level container, but eventually
+                # For now we just evaluate the top-level object, but eventually
                 # there will be schedules and such associated with each object,
-                # and probably with the container itself
-                transaction = container.evaluate
+                # and probably with the container itself.
+                transaction = @objects.evaluate
                 #transaction = Puppet::Transaction.new(objects)
                 transaction.toplevel = true
                 begin
@@ -297,8 +291,9 @@ module Puppet
                 return transaction
             end
 
+            # Retrieve the config from a remote server.  If this fails, then
+            # use the cached copy.
             def getconfig
-                #client.loadproperty('files/sslclient.properties')
                 Puppet.debug("getting config")
 
                 facts = self.class.facts
@@ -311,16 +306,16 @@ module Puppet
 
                 objects = nil
                 if @local
-                    objects = @driver.getconfig(facts)
+                    objects = @driver.getconfig(facts, "yaml")
 
                     if objects == ""
                         raise Puppet::Error, "Could not retrieve configuration"
                     end
                 else
-                    textfacts = CGI.escape(Marshal::dump(facts))
+                    textfacts = CGI.escape(YAML.dump(facts))
 
                     # error handling for this is done in the network client
-                    textobjects = @driver.getconfig(textfacts)
+                    textobjects = @driver.getconfig(textfacts, "yaml")
 
                     unless textobjects == ""
                         begin
@@ -330,23 +325,26 @@ module Puppet
                         end
                     end
 
+                    cachefile = Puppet[:localconfig] + ".yaml"
                     if @cache
                         if textobjects == ""
-                            if FileTest.exists?(Puppet[:localconfig])
-                                textobjects = File.read(Puppet[:localconfig])
+                            if FileTest.exists?(cachefile)
+                                textobjects = File.read(cachefile)
                             else
                                 raise Puppet::Error.new(
                                     "Cannot connect to server and there is no cached configuration"
                                 )
                             end
                         else
-                            # we store the config so that if we can't connect next time, we
-                            # can just run against the most recently acquired copy
+                            # We store the config so that if we can't connect
+                            # next time, we can just run against the most
+                            # recently acquired copy.
+                            Puppet.info "Caching configuration at %s" % cachefile
                             confdir = File.dirname(Puppet[:localconfig])
                             unless FileTest.exists?(confdir)
                                 Puppet.recmkdir(confdir, 0770)
                             end
-                            File.open(Puppet[:localconfig], "w", 0660) { |f|
+                            File.open(cachefile, "w", 0660) { |f|
                                 f.print textobjects
                             }
                         end
@@ -355,17 +353,54 @@ module Puppet
                     end
 
                     begin
-                        objects = Marshal::load(textobjects)
+                        objects = YAML.load(textobjects)
                     rescue => detail
-                        raise Puppet::Error.new("Could not understand configuration")
+                        raise Puppet::Error,
+                            "Could not understand configuration: %s" %
+                            detail.to_s
                     end
                 end
-                if objects.is_a?(Puppet::TransBucket)
-                    @objects = objects
-                else
+
+                unless objects.is_a?(Puppet::TransBucket)
                     raise NetworkClientError,
                         "Invalid returned objects of type %s" % objects.class
                 end
+
+                if classes = objects.classes
+                    self.setclasses(classes)
+                else
+                    Puppet.info "No classes"
+                end
+
+                # Clear all existing objects, so we can recreate our stack.
+                @objects = nil
+                if defined? @objects
+                    Puppet::Type.allclear
+                end
+
+                # Now convert the objects to real Puppet objects
+                @objects = objects.to_type
+
+                if @objects.nil?
+                    raise Puppet::Error, "Configuration could not be processed"
+                end
+                #@objects = objects
+
+                # and perform any necessary final actions before we evaluate.
+                Puppet::Type.finalize
+
+                return @objects
+            end
+
+            def setclasses(ary)
+                begin
+                    File.open(Puppet[:classfile], "w") { |f|
+                        f.puts ary.join("\n")
+                    }
+                rescue => detail
+                    Puppet.err "Could not create class file %s: %s" %
+                        [Puppet[:classfile], detail]
+                end
             end
         end
 
diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb
index 7d4b750c7..6b7aa506d 100644
--- a/lib/puppet/log.rb
+++ b/lib/puppet/log.rb
@@ -216,9 +216,9 @@ module Puppet # :nodoc:
                         begin
                             #puts "would have sent %s" % msg
                             #puts "would have sent %s" %
-                            #    CGI.escape(Marshal::dump(msg))
+                            #    CGI.escape(YAML.dump(msg))
                             begin
-                                tmp = CGI.escape(Marshal::dump(msg))
+                                tmp = CGI.escape(YAML.dump(msg))
                             rescue => detail
                                 puts "Could not dump: %s" % detail.to_s
                                 return
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 9d6a8504c..bb15b3320 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -150,7 +150,11 @@ module Puppet
             if self.is_a?(Puppet::State)
                 return self.should
             else
-                return @value
+                if defined? @value
+                    return @value
+                else
+                    return nil
+                end
             end
         end
 
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index 344863ec0..96555faa1 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -61,6 +61,9 @@ module Puppet
 
                         # We've already evaluated the AST, in this case
                         retval = @scope.evalnode(names, facts)
+                        if classes = @scope.classlist
+                            retval.classes = classes
+                        end
                         return retval
                     else
                         # We've already evaluated the AST, in this case
@@ -68,7 +71,11 @@ module Puppet
                         @scope.interp = self
                         @scope.type = "puppet"
                         @scope.name = "top"
-                        return @scope.evaluate(@ast, facts, @classes)
+                        retval = @scope.evaluate(@ast, facts, @classes)
+                        if classes = @scope.classlist
+                            retval.classes = classes + @classes
+                        end
+                        return retval
                     end
                     #@ast.evaluate(@scope)
                 rescue Puppet::DevError, Puppet::Error, Puppet::ParseError => except
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index a2d05e7d4..598c67b5e 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -149,6 +149,14 @@ module Puppet
                 @level == 1
             end
 
+            # Return a list of all of the defined classes.
+            def classlist
+                unless defined? @classtable
+                    raise Puppet::DevError, "Scope did not receive class table"
+                end
+                return @classtable.keys
+            end
+
             # Yield each child scope in turn
             def each
                 @children.reject { |child|
@@ -714,7 +722,7 @@ module Puppet
 
                     # it'd be nice not to have to do this...
                     results.each { |result|
-                        #Puppet.debug "Result type is %s" % result.class
+                        #Puppet.warning "Result type is %s" % result.class
                         bucket.push(result)
                     }
                     if defined? @type
diff --git a/lib/puppet/server/logger.rb b/lib/puppet/server/logger.rb
index d5feb0320..ae57370d8 100755
--- a/lib/puppet/server/logger.rb
+++ b/lib/puppet/server/logger.rb
@@ -1,3 +1,5 @@
+require 'yaml'
+
 module Puppet
 class Server # :nodoc:
     class LoggerError < RuntimeError; end
@@ -18,11 +20,11 @@ class Server # :nodoc:
             # if the client is set, then we're not local
             if client
                 begin
-                    message = Marshal::load(CGI.unescape(message))
+                    message = YAML.load(CGI.unescape(message))
                     #message = message
                 rescue => detail
                     raise XMLRPC::FaultException.new(
-                        1, "Could not unMarshal log message from %s" % client
+                        1, "Could not unYAML log message from %s" % client
                     )
                 end
             end
diff --git a/lib/puppet/server/master.rb b/lib/puppet/server/master.rb
index e86663d0f..8d4ddbfde 100644
--- a/lib/puppet/server/master.rb
+++ b/lib/puppet/server/master.rb
@@ -3,6 +3,7 @@ require 'puppet'
 require 'puppet/parser/interpreter'
 require 'puppet/sslcertificates'
 require 'xmlrpc/server'
+require 'yaml'
 
 module Puppet
 class Server
@@ -57,7 +58,7 @@ class Server
             end
         end
 
-        def getconfig(facts, client = nil, clientip = nil)
+        def getconfig(facts, format = "marshal", client = nil, clientip = nil)
             if @local
                 # we don't need to do anything, since we should already
                 # have raw objects
@@ -66,11 +67,26 @@ class Server
                 Puppet.debug "Our client is remote"
 
                 # XXX this should definitely be done in the protocol, somehow
-                begin
-                    facts = Marshal::load(CGI.unescape(facts))
-                rescue => detail
+                case format
+                when "marshal":
+                    begin
+                        facts = Marshal::load(CGI.unescape(facts))
+                    rescue => detail
+                        raise XMLRPC::FaultException.new(
+                            1, "Could not rebuild facts"
+                        )
+                    end
+                when "yaml":
+                    begin
+                        facts = YAML.load(CGI.unescape(facts))
+                    rescue => detail
+                        raise XMLRPC::FaultException.new(
+                            1, "Could not rebuild facts"
+                        )
+                    end
+                else
                     raise XMLRPC::FaultException.new(
-                        1, "Could not rebuild facts"
+                        1, "Unavailable config format %s" % format
                     )
                 end
             end
@@ -95,7 +111,18 @@ class Server
             if @local
                 return retobjects
             else
-                return CGI.escape(Marshal::dump(retobjects))
+                str = nil
+                case format
+                when "marshal":
+                    str = Marshal::dump(retobjects)
+                when "yaml":
+                    str = YAML.dump(retobjects)
+                else
+                    raise XMLRPC::FaultException.new(
+                        1, "Unavailable config format %s" % format
+                    )
+                end
+                return CGI.escape(str)
             end
         end
     end
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index e81401c41..129e80bac 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -1,17 +1,32 @@
 require 'puppet'
+require 'yaml'
 
 module Puppet
     # The transportable objects themselves.  Basically just a hash with some
-    # metadata and a few extra methods.
-    class TransObject < Hash
+    # metadata and a few extra methods.  I used to have the object actually
+    # be a subclass of Hash, but I could never correctly dump them using
+    # YAML.
+    class TransObject
+        include Enumerable
         attr_accessor :type, :name, :file, :line
 
         attr_writer :tags
 
+        %w{has_key? include? length delete empty? << [] []=}.each { |method|
+            define_method(method) do |*args|
+                @params.send(method, *args)
+            end
+        }
+
+        def each
+            @params.each { |p,v| yield p, v }
+        end
+
         def initialize(name,type)
-            self[:name] = name
             @type = type
             @name = name
+            @params = {"name" => name}
+            #Puppet.info @params.inspect
             #self.class.add(self)
         end
 
@@ -23,10 +38,20 @@ module Puppet
             return @tags
         end
 
+        def to_hash
+            @params.dup
+        end
+
         def to_s
             return "%s(%s) => %s" % [@type,self[:name],super]
         end
 
+        def to_yaml_properties
+            instance_variables
+            #%w{ @type @name @file @line @tags }.find_all { |v|
+            #}
+        end
+
         def to_type(parent = nil)
             if parent
                 self[:parent] = parent
@@ -34,34 +59,55 @@ module Puppet
             retobj = nil
             if type = Puppet::Type.type(self.type)
                 unless retobj = type.create(self)
+                    Puppet.info "Could not create object"
                     return nil
                 end
-                retobj.file = @file
-                retobj.line = @line
+                #retobj.file = @file
+                #retobj.line = @line
             else
                 raise Puppet::Error.new("Could not find object type %s" % self.type)
             end
 
-            if defined? @tags and @tags
-                #Puppet.debug "%s(%s) tags: %s" % [@type, @name, @tags.join(" ")]
-                retobj.tags = @tags
-            end
+            #if defined? @tags and @tags
+            #    #Puppet.debug "%s(%s) tags: %s" % [@type, @name, @tags.join(" ")]
+            #    retobj.tags = @tags
+            #end
 
             if parent
                 parent.push retobj
             end
 
+            Puppet.info retobj.class
+
             return retobj
         end
     end
-    #------------------------------------------------------------
 
-    #------------------------------------------------------------
-    # just a linear container for objects
-    class TransBucket < Array
-        attr_accessor :name, :type, :file, :line
+    # Just a linear container for objects.  Behaves mostly like an array, except
+    # that YAML will correctly dump them even with their instance variables.
+    class TransBucket
+        include Enumerable
+
+        attr_accessor :name, :type, :file, :line, :classes
+
+        %w{delete shift include? length empty? << []}.each { |method|
+            define_method(method) do |*args|
+                #Puppet.warning "Calling %s with %s" % [method, args.inspect]
+                @children.send(method, *args)
+                #Puppet.warning @params.inspect
+            end
+        }
+
+        def each
+            @children.each { |c| yield c }
+        end
+
+        def initialize
+            @children = []
+        end
 
         def push(*args)
+            #Puppet.warning "calling push"
             args.each { |arg|
                 case arg
                 when Puppet::TransBucket, Puppet::TransObject
@@ -72,7 +118,12 @@ module Puppet
                             arg.class
                 end
             }
-            super
+            @children += args
+            #Puppet.warning @children.inspect
+        end
+
+        def to_yaml_properties
+            instance_variables
         end
 
         def to_type(parent = nil)
@@ -103,6 +154,7 @@ module Puppet
                 hash[:parent] = parent
             end
             container = Puppet.type(:component).create(hash)
+            #Puppet.info container.inspect
 
             if parent
                 parent.push container
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index dfef5d07a..2623fd6c5 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -95,6 +95,19 @@ class Type < Puppet::Element
         end
     end
 
+    def inspect
+        str = "Type(%s)" % self.name
+        if defined? @states
+            str += " States(" + @states.inspect + ")"
+        end
+        if defined? @parameters
+            str += " Parameters(" + @parameters.inspect + ")"
+        end
+        if defined? @metaparams
+            str += " Metaparams(" + @metaparams.inspect + ")"
+        end
+    end
+
     # Create @@typehash from @@typeary.  This is meant to be run
     # multiple times -- whenever it is discovered that the two
     # objects have differents lengths.
@@ -531,7 +544,8 @@ class Type < Puppet::Element
         when @@metaparamhash.include?(name): return :meta
         when @validstates.include?(name): return :state
         else
-            raise Puppet::DevError, "Invalid parameter %s" % [name]
+            raise Puppet::DevError, "Invalid attribute '%s' for class '%s'" %
+                [name, self.name]
         end
     end
 
@@ -1042,6 +1056,21 @@ class Type < Puppet::Element
             self.initvars
         end
 
+        # If we got passed a transportable object, we just pull a bunch of info
+        # directly from it.
+        if hash.is_a?(Puppet::TransObject)
+            #self[:name] = hash[:name]
+            [:file, :line, :tags].each { |getter|
+                if hash.respond_to?(getter)
+                    setter = getter.to_s + "="
+                    if val = hash.send(getter)
+                        self.send(setter, val)
+                    end
+                end
+            }
+            hash = hash.to_hash
+        end
+
         # Before anything else, set our parent if it was included
         if hash.include?(:parent)
             @parent = hash[:parent]
@@ -1217,9 +1246,11 @@ class Type < Puppet::Element
 
     # fix any namevar => param translations
     def argclean(hash)
-        # we have to set the name of our object before anything else,
-        # because it might be used in creating the other states
-        hash = hash.dup
+        # We have to set the name of our object before anything else,
+        # because it might be used in creating the other states.  We dup and
+        # then convert to a hash here because TransObjects behave strangely
+        # here.
+        hash = hash.dup.to_hash
 
         if hash.include?(:parent)
             hash.delete(:parent)
diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
index 59d51e680..1603dba34 100755
--- a/lib/puppet/type/tidy.rb
+++ b/lib/puppet/type/tidy.rb
@@ -151,6 +151,10 @@ module Puppet
             end
         end
 
+        # Erase PFile's validate method
+        validate do
+        end
+
         @depthfirst = true
 
         def initialize(hash)