Merge branch 'ticket/next/5274' into next

5 files changed, 330 insertions, 72 deletions

diff --git a/lib/puppet/provider/host/parsed.rb b/lib/puppet/provider/host/parsed.rb
index 4f15eff3f..a303c4bcf 100644
--- a/lib/puppet/provider/host/parsed.rb
+++ b/lib/puppet/provider/host/parsed.rb
@@ -8,66 +8,36 @@ else
 end
 
 
-      Puppet::Type.type(:host).provide(
-        :parsed,
-  :parent => Puppet::Provider::ParsedFile,
-  :default_target => hosts,
-        
-  :filetype => :flat
-) do
+Puppet::Type.type(:host).provide(:parsed,:parent => Puppet::Provider::ParsedFile,
+  :default_target => hosts,:filetype => :flat) do
   confine :exists => hosts
 
   text_line :comment, :match => /^#/
   text_line :blank, :match => /^\s*$/
 
-  record_line :parsed, :fields => %w{ip name host_aliases},
-    :optional => %w{host_aliases},
-    :rts => true do |line|
-    hash = {}
-    if line.sub!(/^(\S+)\s+(\S+)\s*/, '')
-      hash[:ip] = $1
-      hash[:name] = $2
-
-      if line.empty?
-        hash[:host_aliases] = []
+  record_line :parsed, :fields => %w{ip name host_aliases comment},
+    :optional => %w{host_aliases comment},
+    :match    => /^(\S+)\s+(\S+)\s*(.*?)?(?:\s*#\s*(.*))?$/,
+    :post_parse => proc { |hash|
+      # An absent comment should match "comment => ''"
+      hash[:comment] = '' if hash[:comment].nil? or hash[:comment] == :absent
+      unless hash[:host_aliases].nil? or hash[:host_aliases] == :absent
+        hash[:host_aliases] = hash[:host_aliases].split(/\s+/)
       else
-        line.sub!(/\s*/, '')
-        line.sub!(/^([^#]+)\s*/) do |value|
-          aliases = $1
-          unless aliases =~ /^\s*$/
-            hash[:host_aliases] = aliases.split(/\s+/)
-          end
-
-          ""
-        end
+        hash[:host_aliases] = []
       end
-    else
-      raise Puppet::Error, "Could not match '#{line}'"
-    end
-
-    hash[:host_aliases] = [] if hash[:host_aliases] == ""
-
-    return hash
-  end
-
-  # Convert the current object into a host-style string.
-  def self.to_line(hash)
-    return super unless hash[:record_type] == :parsed
-    [:ip, :name].each do |n|
-      raise ArgumentError, "#{n} is a required attribute for hosts" unless hash[n] and hash[n] != :absent
-    end
-
-    str = "#{hash[:ip]}\t#{hash[:name]}"
-
-    if hash.include? :host_aliases and !hash[:host_aliases].empty?
-      if hash[:host_aliases].is_a? Array
+    },
+    :to_line  => proc { |hash|
+      [:ip, :name].each do |n|
+        raise ArgumentError, "#{n} is a required attribute for hosts" unless hash[n] and hash[n] != :absent
+      end
+      str = "#{hash[:ip]}\t#{hash[:name]}"
+      if hash.include? :host_aliases and !hash[:host_aliases].empty?
         str += "\t#{hash[:host_aliases].join("\t")}"
-      else
-        raise ArgumentError, "Host aliases must be specified as an array"
       end
-    end
-
-    str
-  end
+      if hash.include? :comment and !hash[:comment].empty?
+        str += "\t# #{hash[:comment]}"
+      end
+      str
+    }
 end
-
diff --git a/lib/puppet/type/host.rb b/lib/puppet/type/host.rb
index 8ab750459..1af74d886 100755
--- a/lib/puppet/type/host.rb
+++ b/lib/puppet/type/host.rb
@@ -5,11 +5,11 @@ module Puppet
     newproperty(:ip) do
       desc "The host's IP address, IPv4 or IPv6."
 
-    validate do |value|
-      unless value =~ /((([0-9a-fA-F]+:){7}[0-9a-fA-F]+)|(([0-9a-fA-F]+:)*[0-9a-fA-F]+)?::(([0-9a-fA-F]+:)*[0-9a-fA-F]+)?)|((25[0-5]|2[0-4][\d]|[0-1]?\d?\d)(\.(25[0-5]|2[0-4]\d|[0-1]?\d?\d)){3})/
-      raise Puppet::Error, "Invalid IP address"
+      validate do |value|
+        unless value =~ /^((([0-9a-fA-F]+:){7}[0-9a-fA-F]+)|(([0-9a-fA-F]+:)*[0-9a-fA-F]+)?::(([0-9a-fA-F]+:)*[0-9a-fA-F]+)?)|((25[0-5]|2[0-4][\d]|[0-1]?\d?\d)(\.(25[0-5]|2[0-4]\d|[0-1]?\d?\d)){3})$/
+          raise Puppet::Error, "Invalid IP address"
+        end
       end
-    end
 
     end
 
@@ -26,21 +26,6 @@ module Puppet
         currentvalue.join(" ")
       end
 
-      def retrieve
-        is = super
-        case is
-        when String
-          is = is.split(/\s*,\s*/)
-        when Symbol
-          is = [is]
-        when Array
-          # nothing
-        else
-          raise Puppet::DevError, "Invalid @is type #{is.class}"
-        end
-        is
-      end
-
       # We actually want to return the whole array here, not just the first
       # value.
       def should
@@ -61,9 +46,14 @@ module Puppet
 
       validate do |value|
         raise Puppet::Error, "Host aliases cannot include whitespace" if value =~ /\s/
+        raise Puppet::Error, "Host alias cannot be an empty string. Use an empty array to delete all host_aliases " if value =~ /^\s*$/
       end
     end
 
+    newproperty(:comment) do
+      desc "A comment that will be attached to the line with a # character"
+    end
+
     newproperty(:target) do
       desc "The file in which to store service information.  Only used by
         those providers that write to disk."
diff --git a/spec/unit/provider/host/parsed_spec.rb b/spec/unit/provider/host/parsed_spec.rb
new file mode 100644
index 000000000..239e3bd86
--- /dev/null
+++ b/spec/unit/provider/host/parsed_spec.rb
@@ -0,0 +1,196 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet_spec/files'
+require 'puppettest/support/utils'
+require 'puppettest/fileparsing'
+
+provider_class = Puppet::Type.type(:host).provider(:parsed)
+
+describe provider_class do
+  include PuppetSpec::Files
+  extend  PuppetTest::Support::Utils
+  include PuppetTest::FileParsing
+
+  before do
+    @host_class = Puppet::Type.type(:host)
+    @provider = @host_class.provider(:parsed)
+    @hostfile = tmpfile('hosts')
+    @provider.any_instance.stubs(:target).returns @hostfile
+  end
+
+  after :each do
+    @provider.initvars
+  end
+
+  def mkhost(args)
+    hostresource = Puppet::Type::Host.new(:name => args[:name])
+    hostresource.stubs(:should).with(:target).returns @hostfile
+
+    # Using setters of provider
+    host = @provider.new(hostresource)
+    args.each do |property,value|
+      host.send("#{property}=", value)
+    end
+    host
+  end
+
+  def genhost(host)
+    @provider.stubs(:filetype).returns(Puppet::Util::FileType::FileTypeRam)
+    File.stubs(:chown)
+    File.stubs(:chmod)
+    Puppet::Util::SUIDManager.stubs(:asuser).yields
+    host.flush
+    @provider.target_object(@hostfile).read
+  end
+
+  describe "when parsing a line with ip and hostname" do
+
+    it "should parse an ipv4 from the first field" do
+      @provider.parse_line("127.0.0.1    localhost")[:ip].should == "127.0.0.1"
+    end
+
+    it "should parse an ipv6 from the first field" do
+      @provider.parse_line("::1     localhost")[:ip].should == "::1"
+    end
+
+    it "should parse the name from the second field" do
+      @provider.parse_line("::1     localhost")[:name].should == "localhost"
+    end
+
+    it "should set an empty comment" do
+      @provider.parse_line("::1     localhost")[:comment].should == ""
+    end
+
+  end
+
+  describe "when parsing a line with ip, hostname and comment" do
+    before do
+      @testline = "127.0.0.1   localhost # A comment with a #-char"
+    end
+
+    it "should parse the ip from the first field" do
+      @provider.parse_line(@testline)[:ip].should == "127.0.0.1"
+    end
+
+    it "should parse the hostname from the second field" do
+      @provider.parse_line(@testline)[:name].should == "localhost"
+    end
+
+    it "should parse the comment after the first '#' character" do
+      @provider.parse_line(@testline)[:comment].should == 'A comment with a #-char'
+    end
+
+  end
+
+  describe "when parsing a line with ip, hostname and aliases" do
+
+    it "should parse alias from the third field" do
+      @provider.parse_line("127.0.0.1   localhost   localhost.localdomain")[:host_aliases].should == ["localhost.localdomain"]
+    end
+
+    it "should parse multiple aliases" do
+      @provider.parse_line("127.0.0.1 host alias1 alias2")[:host_aliases].should == ['alias1', 'alias2']
+      @provider.parse_line("127.0.0.1 host alias1\talias2")[:host_aliases].should == ['alias1', 'alias2']
+      @provider.parse_line("127.0.0.1 host alias1\talias2   alias3")[:host_aliases].should == ['alias1', 'alias2', 'alias3']
+    end
+
+  end
+
+  describe "when parsing a line with ip, hostname, aliases and comment" do
+
+    before do
+      # Just playing with a few different delimiters
+      @testline = "127.0.0.1\t   host  alias1\talias2   alias3   #   A comment with a #-char"
+    end
+
+    it "should parse the ip from the first field" do
+      @provider.parse_line(@testline)[:ip].should == "127.0.0.1"
+    end
+
+    it "should parse the hostname from the second field" do
+      @provider.parse_line(@testline)[:name].should == "host"
+    end
+
+    it "should parse all host_aliases from the third field" do
+      @provider.parse_line(@testline)[:host_aliases].should == ['alias1' ,'alias2', 'alias3']
+    end
+
+    it "should parse the comment after the first '#' character" do
+      @provider.parse_line(@testline)[:comment].should == 'A comment with a #-char'
+    end
+
+  end
+
+  describe "when operating on /etc/hosts like files" do
+    fakedata("data/providers/host/parsed","valid*").each do |file|
+      it "should be able to parse #{file}" do
+       fakedataparse(file)
+      end
+    end
+
+    it "should be able to generate a simple hostfile entry" do
+      host = mkhost(
+        :name   => 'localhost',
+        :ip     => '127.0.0.1',
+        :ensure => :present
+      )
+      genhost(host).should == "127.0.0.1\tlocalhost\n"
+    end
+
+    it "should be able to generate an entry with one alias" do
+      host = mkhost(
+        :name   => 'localhost.localdomain',
+        :ip     => '127.0.0.1',
+        :host_aliases => ['localhost'],
+        :ensure => :present
+      )
+      genhost(host).should == "127.0.0.1\tlocalhost.localdomain\tlocalhost\n"
+    end
+
+    it "should be able to generate an entry with more than one alias" do
+      host = mkhost(
+        :name       => 'host',
+        :ip         => '192.0.0.1',
+        :host_aliases => [ 'a1','a2','a3','a4' ],
+        :ensure     => :present
+      )
+      genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\n"
+    end
+
+    it "should be able to generate a simple hostfile entry with comments" do
+      host = mkhost(
+        :name    => 'localhost',
+        :ip      => '127.0.0.1',
+        :comment => 'Bazinga!',
+        :ensure  => :present
+      )
+      genhost(host).should == "127.0.0.1\tlocalhost\t# Bazinga!\n"
+    end
+
+    it "should be able to generate an entry with one alias and a comment" do
+      host = mkhost(
+        :name   => 'localhost.localdomain',
+        :ip     => '127.0.0.1',
+        :host_aliases => ['localhost'],
+        :comment => 'Bazinga!',
+        :ensure => :present
+      )
+      genhost(host).should == "127.0.0.1\tlocalhost.localdomain\tlocalhost\t# Bazinga!\n"
+    end
+
+    it "should be able to generate an entry with more than one alias and a comment" do
+      host = mkhost(
+        :name         => 'host',
+        :ip           => '192.0.0.1',
+        :host_aliases => [ 'a1','a2','a3','a4' ],
+        :comment      => 'Bazinga!',
+        :ensure       => :present
+      )
+      genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\t# Bazinga!\n"
+    end
+
+  end
+
+end
diff --git a/spec/unit/type/host_spec.rb b/spec/unit/type/host_spec.rb
new file mode 100755
index 000000000..12ae2af08
--- /dev/null
+++ b/spec/unit/type/host_spec.rb
@@ -0,0 +1,83 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key)
+
+describe Puppet::Type.type(:host) do
+  before do
+    @class = Puppet::Type.type(:host)
+    @catalog = Puppet::Resource::Catalog.new
+  end
+
+  it "should have :name be its namevar" do
+    @class.key_attributes.should == [:name]
+  end
+
+  describe "when validating attributes" do
+    [:name, :provider ].each do |param|
+      it "should have a #{param} parameter" do
+        @class.attrtype(param).should == :param
+      end
+    end
+
+    [:ip, :target, :host_aliases, :comment, :ensure].each do |property|
+      it "should have a #{property} property" do
+        @class.attrtype(property).should == :property
+      end
+    end
+  end
+
+  describe "when validating values" do
+    it "should support present as a value for ensure" do
+      proc { @class.new(:name => "foo", :ensure => :present) }.should_not raise_error
+    end
+
+    it "should support absent as a value for ensure" do
+      proc { @class.new(:name => "foo", :ensure => :absent) }.should_not raise_error
+    end
+
+    it "should accept IPv4 addresses" do
+      proc { @class.new(:name => "foo", :ip => '10.96.0.1') }.should_not raise_error
+    end
+
+    it "should accept long IPv6 addresses" do
+      # Taken from wikipedia article about ipv6
+      proc { @class.new(:name => "foo", :ip => '2001:0db8:85a3:08d3:1319:8a2e:0370:7344') }.should_not raise_error
+    end
+
+    it "should accept one host_alias" do
+      proc { @class.new(:name => "foo", :host_aliases => 'alias1') }.should_not raise_error
+    end
+
+    it "should accept multiple host_aliases" do
+      proc { @class.new(:name => "foo", :host_aliases => [ 'alias1', 'alias2' ]) }.should_not raise_error
+    end
+
+    it "should accept shortened IPv6 addresses" do
+      proc { @class.new(:name => "foo", :ip => '2001:db8:0:8d3:0:8a2e:70:7344') }.should_not raise_error
+      proc { @class.new(:name => "foo", :ip => '::ffff:192.0.2.128') }.should_not raise_error
+      proc { @class.new(:name => "foo", :ip => '::1') }.should_not raise_error
+    end
+
+    it "should not accept malformed IPv4 addresses like 192.168.0.300" do
+      proc { @class.new(:name => "foo", :ip => '192.168.0.300') }.should raise_error
+    end
+
+    it "should not accept malformed IP addresses like 2001:0dg8:85a3:08d3:1319:8a2e:0370:7344" do
+      proc { @class.new(:name => "foo", :ip => '2001:0dg8:85a3:08d3:1319:8a2e:0370:7344') }.should raise_error
+    end
+
+    it "should not accept spaces in resourcename" do
+      proc { @class.new(:name => "foo bar") }.should raise_error
+    end
+
+    it "should not accept host_aliases with spaces" do
+      proc { @class.new(:name => "foo", :host_aliases => [ 'well_formed', 'not wellformed' ]) }.should raise_error
+    end
+
+    it "should not accept empty host_aliases" do
+      proc { @class.new(:name => "foo", :host_aliases => ['alias1','']) }.should raise_error
+    end
+  end
+end
diff --git a/test/data/providers/host/parsed/valid_hosts b/test/data/providers/host/parsed/valid_hosts
new file mode 100644
index 000000000..24636295d
--- /dev/null
+++ b/test/data/providers/host/parsed/valid_hosts
@@ -0,0 +1,19 @@
+# Some leading comment, that should be ignored
+# The next line is empty so it should be ignored
+
+::1 localhost
+
+# We now try another delimiter: Several tabs
+127.0.0.1         localhost
+
+# No test trailing spaces
+10.0.0.1         host1         
+
+# Ok its time to test aliases
+2001:252:0:1::2008:8 ipv6host alias1
+192.168.0.1 ipv4host alias2 alias3
+
+# Testing inlinecomments now
+192.168.0.2 host3 # This is host3
+192.168.0.3 host4 alias10 # This is host4
+192.168.0.4 host5   alias11             alias12      #     This is host5