Mounts work again, at least with the parsedfile provider. I still need to create a netinfo provider, but it should be short and easy. And, painfully, I still need to port the other six or so subclasses to this new provider.

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

8 files changed, 311 insertions, 208 deletions

diff --git a/lib/puppet/metatype/providers.rb b/lib/puppet/metatype/providers.rb
index b1ce820be..0165da83e 100644
--- a/lib/puppet/metatype/providers.rb
+++ b/lib/puppet/metatype/providers.rb
@@ -41,6 +41,64 @@ class Puppet::Type
         return @defaultprovider
     end
 
+    # Convert a hash, as provided by, um, a provider, into an instance of self.
+    def self.hash2obj(hash)
+        obj = nil
+
+        namevar = self.namevar
+        unless hash.include?(namevar) and hash[namevar]
+            raise Puppet::DevError, "Hash was not passed with namevar"
+        end
+
+        # if the obj already exists with that name...
+        if obj = self[hash[namevar]]
+            # We're assuming here that objects with the same name
+            # are the same object, which *should* be the case, assuming
+            # we've set up our naming stuff correctly everywhere.
+
+            # Mark found objects as present
+            obj.is = [:ensure, :present]
+            obj.state(:ensure).found = :present
+            hash.each { |param, value|
+                if state = obj.state(param)
+                    state.is = value
+                elsif val = obj[param]
+                    obj[param] = val
+                else
+                    # There is a value on disk, but it should go away
+                    obj.is = [param, value]
+                    obj[param] = :absent
+                end
+            }
+        else
+            # create a new obj, since no existing one seems to
+            # match
+            obj = self.create(namevar => hash[namevar])
+
+            # We can't just pass the hash in at object creation time,
+            # because it sets the should value, not the is value.
+            hash.delete(namevar)
+            hash.each { |param, value|
+                obj.is = [param, value]
+            }
+        end
+
+        return obj
+    end
+
+    # Create a list method that just calls our providers.
+    def self.mkprovider_list
+        unless respond_to?(:list)
+            meta_def(:list) do
+                suitableprovider.find_all { |p| p.respond_to?(:list) }.collect { |prov|
+                    prov.list.each { |h| h[:provider] = prov.name }
+                }.flatten.collect do |hash|
+                    hash2obj(hash)
+                end
+            end
+        end
+    end
+
     # Retrieve a provider by name.
     def self.provider(name)
         name = Puppet::Util.symbolize(name)
@@ -67,9 +125,9 @@ class Puppet::Type
     # directly on the type that it's implementing.
     def self.provide(name, options = {}, &block)
         name = Puppet::Util.symbolize(name)
-        model = self
 
         parent = if pname = options[:parent]
+            options.delete(:parent)
             if pname.is_a? Class
                 pname
             else
@@ -85,6 +143,8 @@ class Puppet::Type
             Puppet::Type::Provider
         end
 
+        options[:model] ||= self
+
         self.providify
 
         provider = genclass(name,
@@ -92,9 +152,7 @@ class Puppet::Type
             :hash => @providers,
             :prefix => "Provider",
             :block => block,
-            :attributes => {
-                :model => model
-            }
+            :attributes => options
         )
 
         return provider
@@ -105,6 +163,8 @@ class Puppet::Type
     def self.providify
         return if @paramhash.has_key? :provider
         model = self
+
+        mkprovider_list()
         newparam(:provider) do
             desc "The specific backend for #{self.name.to_s} to use. You will
                 seldom need to specify this -- Puppet will usually discover the
diff --git a/lib/puppet/provider.rb b/lib/puppet/provider.rb
index 36c685990..a5890f131 100644
--- a/lib/puppet/provider.rb
+++ b/lib/puppet/provider.rb
@@ -51,7 +51,11 @@ class Puppet::Provider
             #unless method_defined? name
             unless metaclass.method_defined? name
                 meta_def(name) do |args|
-                    cmd = command(name) + " " + args
+                    if args
+                        cmd = command(name) + " " + args
+                    else
+                        cmd = command(name)
+                    end
                     # This might throw an ExecutionFailure, but the system above
                     # will catch it, if so.
                     return execute(cmd)
diff --git a/lib/puppet/provider/mount/parsed.rb b/lib/puppet/provider/mount/parsed.rb
index 086cd5cd9..af67ca6ff 100755
--- a/lib/puppet/provider/mount/parsed.rb
+++ b/lib/puppet/provider/mount/parsed.rb
@@ -1,150 +1,43 @@
 require 'puppet/provider/parsedfile'
 
-Puppet::Type.type(:mount).provide :parsed, :parent => Puppet::Provider::ParsedFile do
-    @filetype = Puppet::FileType.filetype(:flat)
-
-    commands :mountcmd => "mount", :umount => "umount", :df => "df"
-
-    def self.init
-        @platform = Facter["operatingsystem"].value
-        case @platform
-        when "Solaris":
-            @path = "/etc/vfstab"
-            @fields = [:device, :blockdevice, :path, :fstype, :pass, :atboot,
-                       :options]
-            @fielddefaults = [ nil ] * @fields.size
-        when "Darwin":
-            @filetype = Puppet::FileType.filetype(:netinfo)
-            @filetype.format = "fstab"
-            @path = "mounts"
-            @fields = [:device, :path, :fstype, :options, :dump, :pass]
-            @fielddefaults = [ nil ] * @fields.size
-
-            # How to map the dumped table to what we want
-            @fieldnames = {
-                "name" => :device,
-                "dir" => :path,
-                "dump_freq" => :dump,
-                "passno" => :pass,
-                "vfstype" => :fstype,
-                "opts" => :options
-            }
-        else
-            @path = "/etc/fstab"
-            @fields = [:device, :path, :fstype, :options, :dump, :pass]
-            @fielddefaults = [ nil ] * 4 + [ "0", "2" ]
-        end
-
-        # Allow Darwin to override the default filetype
-        unless defined? @filetype
-            @filetype = Puppet::FileType.filetype(:flat)
-        end
-    end
-
-    init
-
-    # XXX This needs to change to being a netinfo provider
-    unless @platform == "Darwin"
-        confine :exists => @path
-    end
-
-    def self.clear
-        init
-        super
-    end
-
-    # Parse a mount tab.
-    #
-    # This method also stores existing comments, and it stores all
-    # mounts in order, mostly so that comments are retained in the
-    # order they were written and in proximity to the same fses.
-    def self.parse(text)
-        # provide a single exception for darwin & netinfo
-        if @filetype == Puppet::FileType.filetype(:netinfo)
-            return parseninfo(text)
-        end
-        count = 0
-
-        instances = []
-        text.chomp.split("\n").each { |line|
-            hash = {}
-            case line
-            when /^#/, /^\s*$/:
-                # add comments and blank lines to the list as they are
-                instances << line
-                comment(line)
-            else
-                values = line.split(/\s+/)
-                if @fields.length < values.length
-                    raise Puppet::Error, "Could not parse line %s" % line
-                end
+fstab = nil
+case Facter.value(:operatingsystem)
+when "Solaris": fstab = "/etc/vfstab"
+else
+    fstab = "/etc/fstab"
+end
 
-                values = @fielddefaults.zip(values).collect { |d, v| v || d }
-                unless @fields.length == values.length
-                    raise Puppet::Error, "Could not parse line %s" % line
-                end
-                
-                @fields.zip(values).each do |field, value|
-                    hash[field] = value
-                end
+Puppet::Type.type(:mount).provide(:parsed,
+    :parent => Puppet::Provider::ParsedFile,
+    :default_target => fstab,
+    :filetype => :flat
+) do
 
-                instances << hash
-                count += 1
-            end
-        }
+    commands :mountcmd => "mount", :umount => "umount", :df => "df"
 
-        return instances
+    @platform = Facter["operatingsystem"].value
+    case @platform
+    when "Solaris":
+        @fields = [:device, :blockdevice, :name, :fstype, :pass, :atboot,
+               :options]
+    else
+        @fields = [:device, :name, :fstype, :options, :dump, :pass]
+        @fielddefaults = [ nil ] * 4 + [ "0", "2" ]
     end
 
-    # Parse a netinfo table.
-    def self.parseninfo(text)
-        array = @fileobj.to_array(text)
+    text_line :comment, :match => /^\s*#/
+    text_line :blank, :match => /^\s*$/
 
-        instances = []
-        array.each do |record|
-            hash = {}
-            @fieldnames.each do |name, field|
-                if value = record[name]
-                    if field == :options
-                        hash[field] = value.join(",")
-                    else
-                        hash[field] = value[0]
-                    end
-                else
-                    raise ArgumentError, "Field %s was not provided" % [name]
-                end
-            end
-
-            instances << hash
-        end
-
-        return instances
-    end
-
-    # Convert the current object into an fstab-style string.
-    def self.to_record(hash)
-        self.fields.collect do |field|
-            if value = hash[field]
-                value
-            else
-                raise Puppet::Error,
-                    "Could not retrieve value for %s" % field
-            end
-        end.join("\t")
-    end
+    record_line self.name, :fields => @fields, :separator => /\s+/, :joiner => "\t"
 
     # This only works when the mount point is synced to the fstab.
     def mount
-        mountcmd @model[:path]
+        mountcmd @model[:name]
     end
 
     # This only works when the mount point is synced to the fstab.
     def unmount
-        output = %x{#{command(:umount)} #{@model[:path]}}
-
-        unless $? == 0
-            raise Puppet::Error, "Could not unmount %s" % @model[:path]
-        end
+        umount @model[:name]
     end
 
     # Is the mount currently mounted?
@@ -156,13 +49,12 @@ Puppet::Type.type(:mount).provide :parsed, :parent => Puppet::Provider::ParsedFi
         when "Solaris": df = "#{command(:df)} -k"
         end
         %x{#{df}}.split("\n").find do |line|
-            p line
             fs = line.split(/\s+/)[-1]
             if platform == "Darwin"
-                fs == "/private/var/automount" + @model[:path] or
-                    fs == @model[:path]
+                fs == "/private/var/automount" + @model[:name] or
+                    fs == @model[:name]
             else
-                fs == @model[:path]
+                fs == @model[:name]
             end
         end
     end
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index f9e98d96a..5539679d0 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -13,17 +13,34 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     extend Puppet::Util::FileParsing
 
     class << self
-        attr_reader :filetype
         attr_accessor :default_target
     end
 
     attr_accessor :state_hash
 
+    def self.clean(hash)
+        newhash = hash.dup
+        [:record_type, :on_disk].each do |p|
+            if newhash.include?(p)
+                newhash.delete(p)
+            end
+        end
+
+        return newhash
+    end
+
     def self.clear
         @target_objects.clear
         @records.clear
     end
 
+    def self.filetype
+        unless defined? @filetype
+            @filetype = Puppet::FileType.filetype(:flat)
+        end
+        return @filetype
+    end
+
     def self.filetype=(type)
         if type.is_a?(Class)
             @filetype = type
@@ -34,25 +51,37 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         end
     end
 
-    # Flush all of the targets for which there are modified records.
+    # Flush all of the targets for which there are modified records.  The only
+    # reason we pass a record here is so that we can add it to the stack if
+    # necessary -- it's passed from the instance calling 'flush'.
     def self.flush(record)
-        return unless defined?(@modified) and ! @modified.empty?
-
         # Make sure this record is on the list to be flushed.
         unless record[:on_disk]
             record[:on_disk] = true
             @records << record
+
+            # If we've just added the record, then make sure our
+            # target will get flushed.
+            modified(record[:target] || default_target)
         end
 
+        return unless defined?(@modified) and ! @modified.empty?
+
+        flushed = []
         @modified.sort { |a,b| a.to_s <=> b.to_s }.uniq.each do |target|
             Puppet.debug "Flushing %s provider target %s" % [@model.name, target]
             flush_target(target)
+            flushed << target
         end
+
+        @modified.reject! { |t| flushed.include?(t) }
     end
 
     # Flush all of the records relating to a specific target.
     def self.flush_target(target)
-        target_object(target).write(to_file(target_records(target)))
+        target_object(target).write(to_file(target_records(target).reject { |r|
+            r[:ensure] == :absent
+        }))
     end
 
     # Return the header placed at the top of each generated file, warning
@@ -66,20 +95,43 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     # Add another type var.
     def self.initvars
         @records = []
+        @target_objects = {}
 
         # Default to flat files
         @filetype = Puppet::FileType.filetype(:flat)
         super
     end
 
+    # Return a list of all of the records we can find.
+    def self.list
+        prefetch()
+        @records.find_all { |r| r[:record_type] == self.name }.collect { |r|
+            clean(r)
+        }
+    end
+
+    def self.list_by_name
+        list.collect { |r| r[:name] }
+    end
+
     # Create attribute methods for each of the model's non-metaparam attributes.
     def self.model=(model)
         [model.validstates, model.parameters].flatten.each do |attr|
-            define_method(attr) do @state_hash[attr] end
+            attr = symbolize(attr)
+            define_method(attr) do
+                # If it's not a valid field for this record type (which can happen
+                # when different platforms support different fields), then just
+                # return the should value, so the model shuts up.
+                if @state_hash[attr] or self.class.valid_attr?(self.class.name, attr)
+                    @state_hash[attr] || :absent
+                else
+                    @model.should(attr)
+                end
+            end
 
             define_method(attr.to_s + "=") do |val|
                 # Mark that this target was modified.
-                modeltarget = @model[:target]
+                modeltarget = @model[:target] || self.class.default_target
 
                 # If they're the same, then just mark that one as modified
                 if @state_hash[:target] and @state_hash[:target] == modeltarget
@@ -94,7 +146,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
                     end
                     @state_hash[:target] = modeltarget
                 end
-                @state_hash[attr] = val.to_s
+                @state_hash[attr] = val
             end
         end
         @model = model
@@ -104,7 +156,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     # used within the attr= methods.
     def self.modified(target)
         @modified ||= []
-        @modified << target
+        @modified << target unless @modified.include?(target)
     end
 
     # Retrieve all of the data from disk.  There are three ways to know
@@ -127,9 +179,10 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         @records += retrieve(target).each do |r|
             r[:on_disk] = true
             r[:target] = target
+            r[:ensure] = :present
         end
 
-        # Retrieve the current state of this target.
+        # Set current state on any existing resource instances.
         target_records(target).find_all { |i| i.is_a?(Hash) }.each do |record|
             # Find any model instances whose names match our instances.
             if instance = self.model[record[:name]]
@@ -140,11 +193,14 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
                     instance.provider.state_hash = record
                 end
             end
-            # Any unmatched records are assumed to be unmanaged, so
-            # we just leave them alone.
         end
     end
 
+    # Is there an existing record with this name?
+    def self.record?(name)
+        @records.find { |r| r[:name] == name }
+    end
+
     # Retrieve the text for the file. Returns nil in the unlikely
     # event that it doesn't exist.
     def self.retrieve(path)
@@ -169,7 +225,6 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
 
     # Initialize the object if necessary.
     def self.target_object(target)
-        @target_objects ||= {}
         @target_objects[target] ||= @filetype.new(target)
 
         @target_objects[target]
@@ -198,55 +253,56 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
             targets << model[:target]
         end
 
-        targets.uniq
+        targets.uniq.reject { |t| t.nil? }
     end
 
-    # Write our data to disk.
-    def flush
-        # Make sure we've got a target and name set.
-        @state_hash[:target] ||= @model[:target]
-        @state_hash[:name] ||= @model[:name]
-
-        self.class.flush(@state_hash)
+    def create
+        @model.class.validstates.each do |state|
+            if value = @model.should(state)
+                @state_hash[state] = value
+            end
+        end
+        self.class.modified(@state_hash[:target] || self.class.default_target)
     end
 
-    def initialize(model)
-        super
-
-        # Initialize our data hash with the record type.  The record type
-        # in the subclass has to create a record matching its name.
-        @state_hash = {:record_type => self.class.name}
+    def destroy
+        # We use the method here so it marks the target as modified.
+        self.ensure = :absent
     end
 
-    # Have we been modified since the last flush?
-    def modified?
-        @state[:flush_needed]
+    def exists?
+        if @state_hash[:ensure] == :absent or @state_hash[:ensure].nil?
+            return false
+        else
+            return true
+        end
     end
 
-    def store(hash = nil)
-        hash ||= self.model.to_hash
+    # Write our data to disk.
+    def flush
+        # Make sure we've got a target and name set.
 
-        unless @instances
-            self.hash
+        # If the target isn't set, then this is our first modification, so
+        # mark it for flushing.
+        unless @state_hash[:target]
+            @state_hash[:target] = @model[:target] || self.class.default_target
+            self.class.modified(@state_hash[:target])
         end
+        @state_hash[:name] ||= @model.name
 
-        if hash.empty? 
-            @me.clear
-        else
-            hash.each do |name, value|
-                if @me[name] != hash[name]
-                    @me[name] = hash[name]
-                end
-            end
+        self.class.flush(@state_hash)
+    end
 
-            @me.each do |name, value|
-                unless hash.has_key? name
-                    @me.delete(name)
-                end
-            end
-        end
+    def initialize(model)
+        super
 
-        self.class.store(@instances)
+        # See if there's already a matching state_hash in the records list;
+        # else, use a default value.
+        # We provide a default for 'ensure' here, because the provider will
+        # override it if the thing exists, but it won't touch it if it doesn't
+        # exist.
+        @state_hash = self.class.record?(model[:name]) ||
+            {:record_type => self.class.name, :ensure => :absent}
     end
 end
 
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 8df86ee81..1adeb1366 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -363,9 +363,11 @@ class Type < Puppet::Element
     end
 
     # Convert to a transportable object
-    def to_trans
-        # Collect all of the "is" values
-        retrieve()
+    def to_trans(ret = true)
+        # Retrieve the object, if they tell use to.
+        if ret
+            retrieve()
+        end
 
         trans = TransObject.new(self.title, self.class.name)
 
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index 518f21884..a6be65ae6 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -12,22 +12,30 @@ module Puppet
                   currently mounted, if it is ``mounted``, the filesystem 
                   is entered into the mount table and mounted."
 
-            newvalue(:present, :call => :after) do
+            newvalue(:present) do
                 if provider.mounted?
                     provider.unmount
                     return :mount_unmounted
                 else
+                    provider.create
                     return :mount_created
                 end
             end
+            aliasvalue :unmounted, :present
 
-            newvalue(:absent, :event => :mount_deleted, :call => :after) do
+            newvalue(:absent, :event => :mount_deleted) do
                 if provider.mounted?
                     provider.unmount
                 end
+
+                provider.destroy
             end
 
-            newvalue(:mounted, :event => :mount_mounted, :call => :after) do
+            newvalue(:mounted, :event => :mount_mounted) do
+                # Create the mount point if it does not already exist.
+                if self.is == :absent or self.is.nil?
+                    provider.create
+                end
                 # We have to flush any changes to disk.
                 @parent.flush
 
@@ -43,13 +51,10 @@ module Puppet
             end
 
             def retrieve
-                fail "called retrieve"
-                Puppet.warning @is.inspect
                 if provider.mounted?
                     @is = :mounted
                 else
-                    val = super()
-                    @is = val
+                    @is = super()
                 end
 
             end
@@ -107,12 +112,29 @@ module Puppet
                 support this."
         end
 
-        newparam(:path) do
+        newstate(:target) do
+            desc "The file in which to store the mount table.  Only used by
+                those providers that write to disk (i.e., not NetInfo)."
+
+            defaultto { @parent.class.defaultprovider.default_target }
+        end
+
+        newparam(:name) do
             desc "The mount path for the mount."
 
             isnamevar
         end
 
+        newparam(:path) do
+            desc "The deprecated name for the mount point.  Please use ``name`` now."
+
+            def should=(value)
+                warning "'path' is deprecated for mounts.  Please use 'name'."
+                @parent[:name] = value
+                super
+            end
+        end
+
         @doc = "Manages mounted mounts, including putting mount
             information into the mount table. The actual behavior depends 
             on the value of the 'ensure' parameter."
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index c639b671b..9faa6d8ec 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -272,7 +272,7 @@ class State < Puppet::Parameter
 
         # Look for a matching value
         @should.each { |val|
-            if @is == val
+            if @is == val or @is == val.to_s
                 return true
             end
         }
@@ -457,11 +457,27 @@ class State < Puppet::Parameter
 
         def self.defaultvalues
             newvalue(:present) do
-                @parent.create
+                if @parent.provider and @parent.provider.respond_to?(:create)
+                    @parent.provider.create
+                else
+                    @parent.create
+                end
             end
 
             newvalue(:absent) do
-                @parent.destroy
+                if @parent.provider and @parent.provider.respond_to?(:destroy)
+                    @parent.provider.destroy
+                else
+                    @parent.destroy
+                end
+            end
+
+            defaultto do
+                if @parent.managed?
+                    :present
+                else
+                    nil
+                end
             end
 
             # This doc will probably get overridden
@@ -498,7 +514,15 @@ class State < Puppet::Parameter
             # to get checked, which means that those other states do not have
             # @is values set.  This seems to be the source of quite a few bugs,
             # although they're mostly logging bugs, not functional ones.
-            if @parent.exists?
+            if prov = @parent.provider and prov.respond_to?(:exists?)
+                result = prov.exists?
+            elsif @parent.respond_to?(:exists?)
+                result = @parent.exists?
+            else
+                raise Puppet::DevError, "No ability to determine if %s exists" %
+                    @parent.class.name
+            end
+            if result
                 @is = :present
             else
                 @is = :absent
diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb
index 705192b11..91a70c214 100644
--- a/lib/puppet/util/fileparsing.rb
+++ b/lib/puppet/util/fileparsing.rb
@@ -34,6 +34,15 @@ module Puppet::Util::FileParsing
         @record_order.clear
     end
 
+    def fields(type)
+        type = symbolize(type)
+        if @record_types.include?(type)
+            @record_types[type][:fields].dup
+        else
+            nil
+        end
+    end
+
     # Try to match a specific text line.
     def handle_text_line(line, hash)
         if line =~ hash[:match]
@@ -46,10 +55,22 @@ module Puppet::Util::FileParsing
     # Try to match a record.
     def handle_record_line(line, hash)
         if hash[:match]
+            raise "No provision yet for matching whole lines"
         else
             ret = {}
-            hash[:fields].zip(line.split(hash[:separator])) do |param, value|
-                ret[param] = value
+            sep = hash[:separator]
+
+            # String "helpfully" replaces ' ' with /\s+/ in splitting, so we
+            # have to work around it.
+            if sep == " "
+                sep = / /
+            end
+            hash[:fields].zip(line.split(sep)) do |param, value|
+                if value and value != ""
+                    ret[param] = value
+                else
+                    ret[param] = :absent
+                end
             end
             ret[:record_type] = hash[:name]
             return ret
@@ -117,6 +138,8 @@ module Puppet::Util::FileParsing
             r
         end
 
+        options[:absent] ||= ""
+
         options[:separator] ||= /\s+/
 
         # Unless they specified a string-based joiner, just use a single
@@ -164,7 +187,14 @@ module Puppet::Util::FileParsing
         else
             joinchar = type[:joiner] || type[:separator]
 
-            return type[:fields].collect { |field| details[field].to_s }.join(joinchar)
+            return type[:fields].collect { |field|
+                # If the field is marked absent, use the appropriate replacement
+                if details[field] == :absent
+                    type[:absent]
+                else
+                    details[field].to_s
+                end
+            }.join(joinchar)
         end
     end
 
@@ -177,6 +207,19 @@ module Puppet::Util::FileParsing
         end
     end
 
+    def valid_attr?(type, attr)
+        type = symbolize(type)
+        if @record_types[type] and @record_types[type][:fields].include?(symbolize(attr))
+            return true
+        else
+            if symbolize(attr) == :ensure
+                return true
+            else
+                false
+            end
+        end
+    end
+
     private
     # Define a new type of record.
     def new_line_type(name, type, options)