[PATCH/puppet 2/3] (#5274) New comment property for the hosttype

[Nick Lewis]

From: Stefan Schulte <***@taunusstein.net>

When the parsefile provider for host parses /etc/hosts, it throws any
inlinecomment away. As a result they are also purged in /etc/hosts after
a puppetrun that detects a change. That could be dangerous because you
will lose information even about unmanaged resources.

So if you have something like

192.168.0.1 hostentry_not_managed_by_puppet # Important comment

in /etc/hosts the endresult will be

192.168.0.1\thostentry_not_managed_by_puppet

This patch introduces a new property "comment" for the host type. The
provider is nearly a complete rewrite and a lot shorter and hopefully
easier to understand.

Signed-off-by: Nick Lewis <***@puppetlabs.com>
---
lib/puppet/provider/host/parsed.rb | 76 +++++++++++-------------------------
lib/puppet/type/host.rb | 28 ++++---------
2 files changed, 32 insertions(+), 72 deletions(-)

diff --git a/lib/puppet/provider/host/parsed.rb b/lib/puppet/provider/host/parsed.rb
index 4f15eff..a303c4b 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 8ab7504..1af74d8 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."

[Nick Lewis]

From: Stefan Schulte <***@taunusstein.net>

Just a few additional tests for the new property "comment" of the
host type.

Signed-off-by: Nick Lewis <***@puppetlabs.com>
---
spec/unit/provider/host/parsed_spec.rb | 44 +++++++++++++++++++++++++++
spec/unit/type/host_spec.rb | 4 +--
test/data/providers/host/parsed/valid_hosts | 5 +++
3 files changed, 50 insertions(+), 3 deletions(-)
mode change 100755 => 100644 spec/unit/provider/host/parsed_spec.rb

diff --git a/spec/unit/provider/host/parsed_spec.rb b/spec/unit/provider/host/parsed_spec.rb
old mode 100755
new mode 100644
index 08254e6..239e3bd
--- a/spec/unit/provider/host/parsed_spec.rb
+++ b/spec/unit/provider/host/parsed_spec.rb
@@ -59,6 +59,10 @@ describe provider_class 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
@@ -74,6 +78,10 @@ describe provider_class 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
@@ -109,6 +117,10 @@ describe provider_class 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
@@ -147,6 +159,38 @@ describe provider_class do
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
index 43846b2..12ae2af 100755
--- a/spec/unit/type/host_spec.rb
+++ b/spec/unit/type/host_spec.rb
@@ -21,7 +21,7 @@ describe Puppet::Type.type(:host) do
end
end

- [:ip, :target, :host_aliases, :ensure].each do |property|
+ [:ip, :target, :host_aliases, :comment, :ensure].each do |property|
it "should have a #{property} property" do
@class.attrtype(property).should == :property
end
@@ -61,7 +61,6 @@ describe Puppet::Type.type(:host) do
end

it "should not accept malformed IPv4 addresses like 192.168.0.300" do
- pending
proc { @class.new(:name => "foo", :ip => '192.168.0.300') }.should raise_error
end

@@ -78,7 +77,6 @@ describe Puppet::Type.type(:host) do
end

it "should not accept empty host_aliases" do
- pending
proc { @class.new(:name => "foo", :host_aliases => ['alias1','']) }.should raise_error
end
end
diff --git a/test/data/providers/host/parsed/valid_hosts b/test/data/providers/host/parsed/valid_hosts
index de5caf7..2463629 100644
--- a/test/data/providers/host/parsed/valid_hosts
+++ b/test/data/providers/host/parsed/valid_hosts
@@ -12,3 +12,8 @@
# 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