Hosts now work again, and it should be straightforward to create a netinfo provider, too.

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

10 files changed, 226 insertions, 183 deletions

diff --git a/lib/puppet/provider/host/parsed.rb b/lib/puppet/provider/host/parsed.rb
index c606562a2..e2e0786dc 100644
--- a/lib/puppet/provider/host/parsed.rb
+++ b/lib/puppet/provider/host/parsed.rb
@@ -1,62 +1,57 @@
 require 'puppet/provider/parsedfile'
 
-Puppet::Type.type(:host).provide :parsed, :parent => Puppet::Provider::ParsedFile do
-    @path = "/etc/hosts"
-    @filetype = Puppet::FileType.filetype(:flat)
+hosts = nil
+case Facter.value(:operatingsystem)
+when "Solaris": hosts = "/etc/inet/hosts"
+else
+    hosts = "/etc/hosts"
+end
 
-    confine :exists => @path
+Puppet::Type.type(:host).provide(:parsed,
+    :parent => Puppet::Provider::ParsedFile,
+    :default_target => hosts,
+    :filetype => :flat
+) do
+    confine :exists => hosts
 
-    # Parse a host file
-    #
-    # This method also stores existing comments, and it stores all host
-    # jobs in order, mostly so that comments are retained in the order
-    # they were written and in proximity to the same jobs.
-    def self.parse(text)
-        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 
-            else
-                if line.sub!(/^(\S+)\s+(\S+)\s*/, '')
-                    hash[:ip] = $1
-                    hash[:name] = $2
+    text_line :comment, :match => /^#/ 
+    text_line :blank, :match => /^\s*$/ 
 
-                    unless line == ""
-                        line.sub!(/\s*/, '')
-                        line.sub!(/^([^#]+)\s*/) do |value|
-                            aliases = $1
-                            unless aliases =~ /^\s*$/
-                                hash[:alias] = aliases.split(/\s+/)
-                            end
+    record_line :parsed, :fields => %w{ip name alias},
+        :optional => %w{alias},
+        :rts => true do |line|
+        hash = {}
+        if line.sub!(/^(\S+)\s+(\S+)\s*/, '')
+            hash[:ip] = $1
+            hash[:name] = $2
 
-                            ""
-                        end
+            unless line == ""
+                line.sub!(/\s*/, '')
+                line.sub!(/^([^#]+)\s*/) do |value|
+                    aliases = $1
+                    unless aliases =~ /^\s*$/
+                        hash[:alias] = aliases.split(/\s+/)
                     end
-                else
-                    raise Puppet::Error, "Could not match '%s'" % line
-                end
 
-                if hash[:alias] == ""
-                    hash.delete(:alias)
+                    ""
                 end
-
-                instances << hash
-
-                count += 1
             end
-        }
+        else
+            raise Puppet::Error, "Could not match '%s'" % line
+        end
+
+        if hash[:alias] == ""
+            hash.delete(:alias)
+        end
 
-        return instances
+        return hash
     end
 
     # Convert the current object into a host-style string.
-    def self.to_record(hash)
+    def self.to_line(hash)
+        return super unless hash[:record_type] == :parsed
         [:ip, :name].each do |n|
-            unless hash.has_key? n
+            unless hash[n] and hash[n] != :absent
                 raise ArgumentError, "%s is a required attribute for hosts" % n
             end
         end
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index 40b977314..cdcc49b20 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -224,15 +224,6 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         end
     end
 
-    # Write out the file.
-    def self.store(records)
-        if records.empty?
-            Puppet.notice "No %s records for %s" % [self.name, @path]
-        else
-            target_object.write(self.to_file(instances))
-        end
-    end
-
     # Initialize the object if necessary.
     def self.target_object(target)
         @target_objects[target] ||= @filetype.new(target)
@@ -273,11 +264,13 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
             end
         end
         self.class.modified(@state_hash[:target] || self.class.default_target)
+        return (@model.class.name.to_s + "_created").intern
     end
 
     def destroy
         # We use the method here so it marks the target as modified.
         self.ensure = :absent
+        return (@model.class.name.to_s + "_deleted").intern
     end
 
     def exists?
diff --git a/lib/puppet/provider/port/parsed.rb b/lib/puppet/provider/port/parsed.rb
index 510b461df..62905bcea 100755
--- a/lib/puppet/provider/port/parsed.rb
+++ b/lib/puppet/provider/port/parsed.rb
@@ -24,7 +24,9 @@ Puppet::Type.type(:port).provide(:parsed,
     text_line :funky_darwin, :match => /^\s+\d+\//
 
     # We have to manually parse the line, since it's so darn complicated.
-    record_line :parsed, :fields => %w{name port protocols alias} do |line|
+    record_line :parsed, :fields => %w{name port protocols alias description},
+        :optional => %w{alias description}
+    do |line|
         if line =~ /\/ddp/
             raise "missed ddp in %s" % line
         end
diff --git a/lib/puppet/type/host.rb b/lib/puppet/type/host.rb
index b2ed848fc..49a3144b8 100755
--- a/lib/puppet/type/host.rb
+++ b/lib/puppet/type/host.rb
@@ -1,7 +1,9 @@
 require 'puppet/type/parsedtype'
 
 module Puppet
-    newtype(:host, Puppet::Type::ParsedType) do
+    newtype(:host) do
+        ensurable
+
         newstate(:ip) do
             desc "The host's IP address, IPv4 or IPv6."
         end
@@ -75,6 +77,18 @@ module Puppet
             end
         end
 
+        newstate(:target) do
+            desc "The file in which to store service information.  Only used by
+                those providers that write to disk (i.e., not NetInfo)."
+
+            defaultto { if @parent.class.defaultprovider.ancestors.include?(Puppet::Provider::ParsedFile)
+                    @parent.class.defaultprovider.default_target
+                else
+                    nil
+                end
+            }
+        end
+
         newparam(:name) do
             desc "The host name."
 
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index 9faa6d8ec..9b96b909f 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -190,7 +190,6 @@ class State < Puppet::Parameter
     end
 
     # Figure out which event to return.
-    # not specified.
     def event(name, event = nil)
         if value_event = self.class.value_option(name, :event)
             return value_event
diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb
index 4056f2fff..390648404 100644
--- a/lib/puppet/util/fileparsing.rb
+++ b/lib/puppet/util/fileparsing.rb
@@ -145,7 +145,18 @@ module Puppet::Util::FileParsing
         return nil
     end
 
-    # Define a new type of record.  These lines get split into hashes.
+    # Define a new type of record.  These lines get split into hashes.  Valid
+    # options are:
+    # * <tt>:absent</tt>: What to use when a field is absent.  Defaults to "".
+    # * <tt>:fields</tt>: The list of fields, as an array.  By default, all
+    #   fields are considered required.
+    # * <tt>:joiner</tt>: How to join fields together.  Defaults to '\t'.
+    # * <tt>:optional</tt>: Which fields are optional.  If these are missing,
+    #   you'll just get the 'absent' value instead of an ArgumentError.
+    # * <tt>:rts</tt>: Whether to remove trailing whitespace.  Defaults to false.
+    #   If true, whitespace will be removed; if a regex, then whatever matches
+    #   the regex will be removed.
+    # * <tt>:separator</tt>: The record separator.  Defaults to /\s+/.
     def record_line(name, options, &block)
         unless options.include?(:fields)
             raise ArgumentError, "Must include a list of fields"
@@ -162,6 +173,12 @@ module Puppet::Util::FileParsing
 
         options[:absent] ||= ""
 
+        if options[:optional]
+            options[:optional] = options[:optional].collect { |f| symbolize(f) }
+        else
+            options[:optional] = []
+        end
+
         options[:separator] ||= /\s+/
 
         # Unless they specified a string-based joiner, just use a single
@@ -218,14 +235,28 @@ module Puppet::Util::FileParsing
         else
             joinchar = type[:joiner] || type[:separator]
 
-            return type[:fields].collect { |field|
+            line = type[:fields].collect { |field|
                 # If the field is marked absent, use the appropriate replacement
-                if details[field] == :absent
-                    type[:absent]
+                if details[field] == :absent or details[field].nil?
+                    if type[:optional].include?(field)
+                        type[:absent]
+                    else
+                        raise ArgumentError, "Field %s is required" % field
+                    end
                 else
                     details[field].to_s
                 end
-            }.join(joinchar)
+            }.reject { |c| c.nil?}.join(joinchar)
+
+            if regex = type[:rts]
+                # If they say true, then use whitespace; else, use their regex.
+                if regex == true
+                    regex = /\s+$/
+                end
+                return line.sub(regex,'')
+            else
+                return line
+            end
         end
     end
 
diff --git a/test/data/types/hosts/solaris b/test/data/types/hosts/solaris
new file mode 100644
index 000000000..be3b97f2a
--- /dev/null
+++ b/test/data/types/hosts/solaris
@@ -0,0 +1,5 @@
+#
+# Internet host table
+#
+127.0.0.1	localhost
+192.168.0.50	sol10b
diff --git a/test/providers/parsedhost.rb b/test/providers/parsedhost.rb
index 9836b90d1..6f7d36b8c 100755
--- a/test/providers/parsedhost.rb
+++ b/test/providers/parsedhost.rb
@@ -22,6 +22,7 @@ class TestParsedHostProvider < Test::Unit::TestCase
     def teardown
         Puppet::FileType.filetype(:ram).clear
         @provider.filetype = @oldfiletype
+        @provider.clear
         super
     end
 
@@ -45,7 +46,8 @@ class TestParsedHostProvider < Test::Unit::TestCase
         return {
             :name => "fakehost%s" % @hcount,
             :ip => "192.168.27.%s" % @hcount,
-            :alias => ["alias%s" % @hcount]
+            :alias => ["alias%s" % @hcount],
+            :ensure => :present
         }
     end
 
@@ -56,21 +58,21 @@ class TestParsedHostProvider < Test::Unit::TestCase
 
         host = @provider.new(fakemodel)
 
+        assert(host, "Could not create provider host")
         hash.each do |name, val|
-            fakemodel[name] = val
+            host.send(name.to_s + "=", val)
         end
-        assert(host, "Could not create provider host")
 
         return host
     end
 
     # Make sure we convert both directlys correctly using a simple host.
     def test_basic_isomorphism
-        hash = {:name => "myhost", :ip => "192.168.43.56", :alias => %w{another host}}
+        hash = {:record_type => :parsed, :name => "myhost", :ip => "192.168.43.56", :alias => %w{another host}}
 
         str = nil
         assert_nothing_raised do
-            str = @provider.to_record(hash)
+            str = @provider.to_line(hash)
         end
 
         assert_equal("192.168.43.56\tmyhost\tanother\thost", str)
@@ -100,51 +102,30 @@ class TestParsedHostProvider < Test::Unit::TestCase
         end
 
         assert_equal([
-            "# comment one",
-            "",
-            {:name => "myhost", :ip => "192.168.43.56", :alias => %w{another host}},
-            "    ",
-            "# another comment",
-            {:name => "anotherhost", :ip => "192.168.43.57"}
+            {:record_type => :comment, :line => "# comment one"},
+            {:record_type => :blank, :line => ""},
+            {:record_type => :parsed, :name => "myhost", :ip => "192.168.43.56", :alias => %w{another host}},
+            {:record_type => :blank, :line => "    "},
+            {:record_type => :comment, :line => "# another comment"},
+            {:record_type => :parsed, :name => "anotherhost", :ip => "192.168.43.57"}
         ], instances)
 
-        assert_nothing_raised do
-            @provider.store(instances)
-        end
         newtext = nil
         assert_nothing_raised do
-            newtext = @provider.fileobj.read
+            newtext = @provider.to_file(instances)
         end
 
         assert_equal(text, newtext)
     end
 
-    def test_empty_and_absent_hashes_are_not_written
-        mkfaketype()
-
-        instances = [
-            {:name => "myhost", :ip => "192.168.43.56", :alias => %w{another host}},
-            {},
-            {:ensure => :absent, :name => "anotherhost", :ip => "192.168.43.57"}
-        ]
-
-        assert_nothing_raised do
-            newtext = @provider.store(instances)
-        end
-        newtext = nil
-        assert_nothing_raised do
-            newtext = @provider.fileobj.read
-        end
-        text = "192.168.43.56\tmyhost\tanother\thost\n"
-
-        assert_equal(text, newtext)
-    end
-
     def test_simplehost
         mkfaketype
+        @provider.default_target = :yayness
+        file = @provider.target_object(:yayness)
+
         # Start out with no content.
         assert_nothing_raised {
-            assert_equal([], @provider.retrieve)
+            assert_equal([], @provider.parse(file.read))
         }
 
         # Now create a provider
@@ -155,40 +136,42 @@ class TestParsedHostProvider < Test::Unit::TestCase
 
         # Make sure we're still empty
         assert_nothing_raised {
-            assert_equal([], @provider.retrieve)
+            assert_equal([], @provider.parse(file.read))
         }
 
-        hash = host.model.to_hash
-
         # Try storing it
         assert_nothing_raised do
-            host.store(hash)
+            host.flush
         end
 
         # Make sure we get the host back
         assert_nothing_raised {
-            assert_equal([hash], @provider.retrieve)
+            assert(file.read.include?(host.name),
+                "Did not flush host to disk")
         }
 
         # Remove a single field and make sure it gets tossed
-        hash.delete(:alias)
+        name = host.alias
+        host.alias = [:absent]
 
         assert_nothing_raised {
-            host.store(hash)
-            assert_equal([hash], @provider.retrieve)
+            host.flush
+            assert(! file.read.include?(name[0]),
+                "Did not remove alias from disk")
         }
 
         # Make sure it throws up if we remove a required field
-        hash.delete(:ip)
+        host.ip = :absent
 
         assert_raise(ArgumentError) {
-            host.store(hash)
+            host.flush
         }
 
         # Now remove the whole object
+        host.ensure = :absent
         assert_nothing_raised {
-            host.store({})
-            assert_equal([], @provider.retrieve)
+            host.flush
+            assert_equal([], @provider.parse(file.read))
         }
     end
 
@@ -201,7 +184,9 @@ class TestParsedHostProvider < Test::Unit::TestCase
     # get taken into account.
     def test_modifyingfile
         hostfile = tempfile()
-        @provider.path = hostfile
+        @provider.default_target = hostfile
+
+        file = @provider.target_object(hostfile)
 
         hosts = []
         3.times {
@@ -210,33 +195,38 @@ class TestParsedHostProvider < Test::Unit::TestCase
         }
 
         hosts.each do |host|
-            host.store
+            host.flush
         end
 
         newhost = mkhost()
         hosts << newhost
 
         # Now store our new host
-        newhost.store()
+        newhost.flush()
 
         # Verify we can retrieve that info
         assert_nothing_raised("Could not retrieve after second write") {
-            newhost.hash
+            @provider.prefetch
         }
 
-        text = @provider.fileobj.read
+        text = file.read
 
-        instances = @provider.retrieve
+        instances = @provider.parse(text)
 
         # And verify that we have data for everything
         hosts.each { |host|
             name = host.model[:name]
             assert(text.include?(name), "Host %s is not in file" % name)
-            hash = host.hash
+            hash = host.state_hash
             assert(! hash.empty?, "Could not find host %s" % name)
             assert(hash[:ip], "Could not find ip for host %s" % name)
         }
     end
+
+    def test_mountsparse
+        files = fakedata("data/types/hosts")
+        fakedataparse(*files)
+    end
 end
 
 # $Id$
diff --git a/test/types/host.rb b/test/types/host.rb
index 148b923de..3c9d28434 100755
--- a/test/types/host.rb
+++ b/test/types/host.rb
@@ -23,11 +23,12 @@ class TestHost < Test::Unit::TestCase
 
         cleanup do @hosttype.defaultprovider = nil end
 
-        oldpath = @provider.path
+        @default_file = @provider.default_target
         cleanup do
-            @provider.path = oldpath
+            @provider.default_target = @default_file
         end
-        @provider.path = tempfile()
+        @target = tempfile()
+        @provider.default_target = @target
     end
 
     def mkhost
@@ -48,18 +49,21 @@ class TestHost < Test::Unit::TestCase
         return host
     end
 
-    def test_simplehost
-        host = nil
-        assert_nothing_raised {
-            Puppet.type(:host).defaultprovider.retrieve
+    def test_list
+        assert_nothing_raised do
+            @hosttype.defaultprovider.prefetch
+        end
 
-            count = 0
-            @hosttype.each do |h|
-                count += 1
-            end
+        count = 0
+        @hosttype.each do |h|
+            count += 1
+        end
 
-            assert_equal(0, count, "Found hosts in empty file somehow")
-        }
+        assert_equal(0, count, "Found hosts in empty file somehow")
+    end
+
+    def test_simplehost
+        host = nil
 
         assert_nothing_raised {
             host = Puppet.type(:host).create(
@@ -68,11 +72,19 @@ class TestHost < Test::Unit::TestCase
             )
         }
 
-        assert_apply(host)
+        assert_events([:host_created], host)
 
         assert_nothing_raised { host.retrieve }
 
         assert_equal(:present, host.is(:ensure))
+
+        host[:ensure] = :absent
+
+        assert_events([:host_deleted], host)
+
+        assert_nothing_raised { host.retrieve }
+
+        assert_equal(:absent, host.is(:ensure))
     end
 
     def test_moddinghost
@@ -88,6 +100,10 @@ class TestHost < Test::Unit::TestCase
         host[:alias] = %w{madstop kirby yayness}
 
         assert_events([:host_changed], host)
+
+        host.retrieve
+
+        assert_equal(%w{madstop kirby yayness}, host.is(:alias))
     end
 
     def test_aliasisstate
@@ -111,56 +127,6 @@ class TestHost < Test::Unit::TestCase
         same = host.class["testing"]
         assert(same, "Could not retrieve by alias")
     end
-
-    def test_removal
-        host = mkhost()
-        assert_nothing_raised {
-            host[:ensure] = :present
-        }
-        assert_events([:host_created], host)
-
-        assert(host.exists?, "Host is not considered in sync")
-
-        assert_equal(:present, host.is(:ensure))
-
-        assert_nothing_raised {
-            host[:ensure] = :absent
-        }
-        assert_events([:host_removed], host)
-
-        text = host.provider.class.fileobj.read
-
-        assert(! text.include?(host[:name]), "Host is still in text")
-        host.retrieve
-        assert_events([], host)
-    end
-
-    def test_modifyingfile
-        hosts = []
-        names = []
-        3.times {
-            h = mkhost()
-            hosts << h
-            names << h.name
-        }
-        assert_apply(*hosts)
-        hosts.clear
-        Puppet.type(:host).clear
-        newhost = mkhost()
-        names << newhost.name
-        assert_apply(newhost)
-        # Verify we can retrieve that info
-        assert_nothing_raised("Could not retrieve after second write") {
-            newhost.retrieve
-        }
-
-        text = newhost.provider.class.fileobj.read
-
-        # And verify that we have data for everything
-        names.each { |name|
-            assert(text.include?(name), "Host is not in file")
-        }
-    end
 end
 
 # $Id$
diff --git a/test/util/fileparsing.rb b/test/util/fileparsing.rb
index 71b0393d1..bd142d2d0 100755
--- a/test/util/fileparsing.rb
+++ b/test/util/fileparsing.rb
@@ -464,17 +464,18 @@ billy three four\n"
 
     # Make sure we correctly handle optional fields.  We'll skip this
     # functionality until we really know we need it.
-    def disabled_test_optional_fields
+    def test_optional_fields
         parser = FParser.new
 
         assert_nothing_raised do
             parser.record_line :record,
                 :fields => %w{one two three four},
                 :optional => %w{three four},
+                :absent => "*",
                 :separator => " " # A single space
         end
 
-        ["a b c d", "a b  d", "a b", "a b  ", "a b c"].each do |line|
+        ["a b c d", "a b * d", "a b * *", "a b c *"].each do |line|
             record = nil
             assert_nothing_raised do
                 record = parser.parse_line(line)
@@ -489,6 +490,53 @@ billy three four\n"
             # And make sure they're equal
             assert_equal(line, newline)
         end
+
+        # Now make sure it pukes if we don't provide the required fields
+        assert_raise(ArgumentError) do
+            parser.to_line(:record_type => :record, :one => "yay")
+        end
+    end
+
+    def test_record_rts
+        parser = FParser.new
+
+        # Start with the default
+        assert_nothing_raised do
+            parser.record_line :record,
+                :fields => %w{one two three four},
+                :optional => %w{three four}
+        end
+
+        assert_equal("a b  ",
+            parser.to_line(:record_type => :record, :one => "a", :two => "b")
+        )
+
+        # Now say yes to removing
+        parser.clear_records
+        assert_nothing_raised do
+            parser.record_line :record,
+                :fields => %w{one two three four},
+                :optional => %w{three four},
+                :rts => true
+        end
+
+        assert_equal("a b",
+            parser.to_line(:record_type => :record, :one => "a", :two => "b")
+        )
+
+        # Lastly, try a regex
+        parser.clear_records
+        assert_nothing_raised do
+            parser.record_line :record,
+                :fields => %w{one two three four},
+                :optional => %w{three four},
+                :absent => "*",
+                :rts => /[ *]+$/
+        end
+
+        assert_equal("a b",
+            parser.to_line(:record_type => :record, :one => "a", :two => "b")
+        )
     end
 end