Luke Kanies | 1 Dec 2010 01:07
Gravatar

Re: [PATCH/puppet 1/2] Maint: Refactor tests to use <class>.indirection.<method>

On Nov 30, 2010, at 10:39 AM, Paul Berry wrote:

On Mon, Nov 29, 2010 at 10:47 PM, Luke Kanies <luke <at> puppetlabs.com> wrote:
What's the reason for this change?

I assume it's related to the default route stuff, but I'd like to understand a bit more about this direction.  In particular, I thought the default route work would actually remove the find etc. methods from the Indirection class, which would make this a bit complicated.

Yeah, it's related to the default route patch that you worked on back in October.  The motivation for the change was to make all the uses of the indirector explicit rather than implicit so that it would be easier to see where it was used.  For example, previously to find all the calls to the indirector you had to grep for ".find", ".search", ".destroy", ".expire", and ".save", and you would get a lot of false hits.  Now you only need to grep for ".indirection" and ".save", and the vast majority of the grep hits are relevant.  In a future commit (probably today) we're planning to make "save" use the same mechanism.

The hope is that once this is done we'll be able to approach other indirector refactorings (such as the default route patch) with a much lower risk of bugs, because we'll be able to easily find all the call sites that are affected by our changes.

Note that the following command seems to have a very high hit rate:

grep -r '[A-Z][a-z]*\.find[ \(]' *

Basically, just grep for capitalized words having 'find' called on them.


--
Be wary of the man who urges an action in which he himself incurs no
risk. -- Joaquin Setanti
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199


--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
Matt Robinson | 1 Dec 2010 01:35
Gravatar

Re: [PATCH/puppet 1/1] (#2495) Better value validation for sshkey

+1

On Sun, Nov 21, 2010 at 12:02 PM, Stefan Schulte
<stefan.schulte <at> taunusstein.net> wrote:
> As mentioned in the ticket it is not obvious that aliases do not belong
> in the resourcename but have to be specified with the property
> "host_aliases". On the puppet-user list I saw someone using this as a
> resource
>
>   <at>  <at> sshkey {"$fqdn,$hostname,$ipaddress":
>    type => rsa,
>    key  => $sshrsakey,
>  }
>
> Puppet will now write a correct entry to the know_hosts file, but when
> it rereads the file, the field $fqdn,$hostname,$ipaddress is split into
> name ($fqdn) and host_aliases ([$hostname,$ipaddress]). Since we dont
> find the resource the user specified, puppet will put the same key in
> the file over and over again. This patch adds a simple validation on
> resourcename.
>
> Signed-off-by: Stefan Schulte <stefan.schulte <at> taunusstein.net>
> ---
>  lib/puppet/type/sshkey.rb     |    7 +++-
>  spec/unit/type/sshkey_spec.rb |   71 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+), 1 deletions(-)
>  create mode 100644 spec/unit/type/sshkey_spec.rb
>
> diff --git a/lib/puppet/type/sshkey.rb b/lib/puppet/type/sshkey.rb
> index b7a1b8a..59a1a12 100755
> --- a/lib/puppet/type/sshkey.rb
> +++ b/lib/puppet/type/sshkey.rb
>  <at>  <at>  -41,7 +41,7  <at>  <at>  module Puppet
>           raise Puppet::Error, "Aliases cannot include whitespace"
>         end
>         if value =~ /,/
> -          raise Puppet::Error, "Aliases cannot include whitespace"
> +          raise Puppet::Error, "Aliases must be provided as an array, not a comma-separated list"
>         end
>       end
>     end
>  <at>  <at>  -50,6 +50,11  <at>  <at>  module Puppet
>       desc "The host name that the key is associated with."
>
>       isnamevar
> +
> +      validate do |value|
> +        raise Puppet::Error, "Resourcename cannot include whitespaces" if value =~ /\s/
> +        raise Puppet::Error, "No comma in resourcename allowed. If you want to specify aliases use the
host_aliases property" if value.include?(',')
> +      end
>     end
>
>     newproperty(:target) do
> diff --git a/spec/unit/type/sshkey_spec.rb b/spec/unit/type/sshkey_spec.rb
> new file mode 100644
> index 0000000..966ca70
> --- /dev/null
> +++ b/spec/unit/type/sshkey_spec.rb
>  <at>  <at>  -0,0 +1,71  <at>  <at> 
> +#!/usr/bin/env ruby
> +
> +require File.dirname(__FILE__) + '/../../spec_helper'
> +
> +sshkey = Puppet::Type.type(:sshkey)
> +
> +describe sshkey do
> +  before do
> +     <at> class = sshkey
> +  end
> +
> +  it "should have :name its namevar" do
> +     <at> class.key_attributes.should == [:name]
> +  end
> +
> +  describe "when validating attributes" do
> +    [:name, :provider].each do |param|
> +      it "should have a #{param} parameter" do
> +         <at> class.attrtype(param).should == :param
> +      end
> +    end
> +
> +    [:host_aliases, :ensure, :key, :type].each do |property|
> +      it "should have a #{property} property" do
> +         <at> class.attrtype(property).should == :property
> +      end
> +    end
> +  end
> +
> +  describe "when validating values" do
> +
> +    it "should support ssh-dss as a type value" do
> +      proc {  <at> class.new(:name => "foo", :type => "ssh-dss") }.should_not raise_error
> +    end
> +
> +    it "should support ssh-rsa as a type value" do
> +      proc {  <at> class.new(:name => "whev", :type => "ssh-rsa") }.should_not raise_error
> +    end
> +
> +    it "should alias :dsa to ssh-dss as a value for type" do
> +      key =  <at> class.new(:name => "whev", :type => :dsa)
> +      key.should(:type).should == :'ssh-dss'
> +    end
> +
> +    it "should alias :rsa to ssh-rsa as a value for type" do
> +      key =  <at> class.new(:name => "whev", :type => :rsa)
> +      key.should(:type).should == :'ssh-rsa'
> +    end
> +
> +    it "should not support values other than ssh-dss, ssh-rsa, dsa, rsa for type" do
> +      proc {  <at> class.new(:name => "whev", :type => :'ssh-dsa') }.should raise_error(Puppet::Error)
> +    end
> +
> +    it "should accept one host_alias" do
> +      proc {  <at> class.new(:name => "foo", :host_aliases => 'foo.bar.tld') }.should_not raise_error
> +    end
> +
> +    it "should accept multiple host_aliases as an array" do
> +      proc {  <at> class.new(:name => "foo", :host_aliases => ['foo.bar.tld','10.0.9.9'])
}.should_not raise_error
> +    end
> +
> +    it "should not accept spaces in any host_alias" do
> +      proc {  <at> class.new(:name => "foo", :host_aliases => ['foo.bar.tld','foo bar']) }.should raise_error(Puppet::Error)
> +    end
> +
> +    it "should not accept aliases in the resourcename" do
> +      proc {  <at> class.new(:name => 'host,host.domain,ip') }.should raise_error(Puppet::Error)
> +    end
> +
> +  end
> +end
> --
> 1.7.3.2
>
> --
> You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
> To post to this group, send email to puppet-dev <at> googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
>
>

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Matt Robinson | 1 Dec 2010 01:53
Gravatar

Re: [PATCH/puppet 4/4] (#5274) Using Propery::OrderedList for host_alias

Stefan,
This 4th commit didn't get in with your previous change set since it
was added after we initially looked at the code.  Also, it really
doesn't have anything to do with ticket #5274 which is about inline
comments, so this commit should really be part of another ticket.
We've opened a new ticket, #5427 which is was unfortunately generated
with a number that is very similar and thus a tad confusing.  We've
added you as a watcher for that ticket and have some comments there,
so check that out.

Thanks
Matt

On Thu, Nov 18, 2010 at 2:54 PM, Stefan Schulte
<stefan.schulte <at> taunusstein.net> wrote:
> This uses the propertyclass Puppet::Property::OrderedList to represent
> the list of host_aliases. This lets us remove the in_sync, shoult_to_s
> etc overrides. The new membership paramater can even be used to just
> specify a minimal list of host_aliases (but default is inclusive).
>
> In the provider class the list is represented by a string (=no array)
> so there were a few changes necessary as well.
>
> Because Puppet::Property::List uses the specified delimiter when
> converting should values to strings, I changed the delimiter to a simple
> space instead a tab. This keeps messages produced by puppet in a nice
> format.
>
> The tests had to be changed to work with the new behaviour of
> host_aliases. There are a few additional tests as well.
>
> Signed-off-by: Stefan Schulte <stefan.schulte <at> taunusstein.net>
> ---
>  lib/puppet/provider/host/parsed.rb     |    8 ++---
>  lib/puppet/type/host.rb                |   41 ++++++++++------------------
>  spec/unit/provider/host/parsed_spec.rb |   28 ++++++++++++-------
>  spec/unit/type/host_spec.rb            |   46 +++++++++++++++++++++++++++++---
>  4 files changed, 78 insertions(+), 45 deletions(-)
>
> diff --git a/lib/puppet/provider/host/parsed.rb b/lib/puppet/provider/host/parsed.rb
> index a303c4b..2ba01a4 100644
> --- a/lib/puppet/provider/host/parsed.rb
> +++ b/lib/puppet/provider/host/parsed.rb
>  <at>  <at>  -22,9 +22,7  <at>  <at>  Puppet::Type.type(:host).provide(:parsed,:parent => Puppet::Provider::ParsedFile
>       # 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
> -        hash[:host_aliases] = []
> +        hash[:host_aliases].gsub!(/\s+/,' ') # Change delimiter
>       end
>     },
>     :to_line  => proc { |hash|
>  <at>  <at>  -32,8 +30,8  <at>  <at>  Puppet::Type.type(:host).provide(:parsed,:parent => Puppet::Provider::ParsedFile
>         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")}"
> +      if hash.include? :host_aliases and !hash[:host_aliases].nil? and hash[:host_aliases] != :absent
> +        str += "\t#{hash[:host_aliases]}"
>       end
>       if hash.include? :comment and !hash[:comment].empty?
>         str += "\t# #{hash[:comment]}"
> diff --git a/lib/puppet/type/host.rb b/lib/puppet/type/host.rb
> index 1af74d8..3520ae2 100755
> --- a/lib/puppet/type/host.rb
> +++ b/lib/puppet/type/host.rb
>  <at>  <at>  -1,3 +1,5  <at>  <at> 
> +require 'puppet/property/ordered_list'
> +
>  module Puppet
>   newtype(:host) do
>     ensurable
>  <at>  <at>  -13,41 +15,28  <at>  <at>  module Puppet
>
>     end
>
> -    newproperty(:host_aliases) do
> +    # for now we use OrderedList to indicate that the order does matter.
> +    newproperty(:host_aliases, :parent => Puppet::Property::OrderedList) do
>       desc "Any aliases the host might have.  Multiple values must be
>         specified as an array."
>
> -      def insync?(is)
> -        is ==  <at> should
> +      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
>
> -      def is_to_s(currentvalue =  <at> is)
> -        currentvalue = [currentvalue] unless currentvalue.is_a? Array
> -        currentvalue.join(" ")
> +      def delimiter
> +        " "
>       end
>
> -      # We actually want to return the whole array here, not just the first
> -      # value.
> -      def should
> -        if defined?( <at> should)
> -          if  <at> should == [:absent]
> -            return :absent
> -          else
> -            return  <at> should
> -          end
> -        else
> -          return nil
> -        end
> -      end
> +    end
>
> -      def should_to_s(newvalue =  <at> should)
> -        newvalue.join(" ")
> -      end
> +    newparam(:membership) do
> +      desc "This paramter decides if the specified host_aliases should be the only aliases for the
> +        host or if host_aliases should only specify the minimal subset."
>
> -      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
> +      newvalues :inclusive, :minimum
> +      defaultto :inclusive
>     end
>
>     newproperty(:comment) do
> diff --git a/spec/unit/provider/host/parsed_spec.rb b/spec/unit/provider/host/parsed_spec.rb
> index 239e3bd..0bc9651 100644
> --- a/spec/unit/provider/host/parsed_spec.rb
> +++ b/spec/unit/provider/host/parsed_spec.rb
>  <at>  <at>  -28,9 +28,13  <at>  <at>  describe provider_class do
>     hostresource = Puppet::Type::Host.new(:name => args[:name])
>     hostresource.stubs(:should).with(:target).returns  <at> hostfile
>
> -    # Using setters of provider
> +    # Using setters of provider to build our testobject
> +    # Note: We already proved, that in case of host_aliases
> +    # the provider setter "host_aliases=(value)" will be
> +    # called with the joined array, so we just simulate that
>     host =  <at> provider.new(hostresource)
>     args.each do |property,value|
> +      value = value.join(" ") if property == :host_aliases and value.is_a?(Array)
>       host.send("#{property}=", value)
>     end
>     host
>  <at>  <at>  -63,6 +67,10  <at>  <at>  describe provider_class do
>        <at> provider.parse_line("::1     localhost")[:comment].should == ""
>     end
>
> +    it "should set host_aliases to :absent" do
> +       <at> provider.parse_line("::1     localhost")[:host_aliases].should == :absent
> +    end
> +
>   end
>
>   describe "when parsing a line with ip, hostname and comment" do
>  <at>  <at>  -87,13 +95,13  <at>  <at>  describe provider_class do
>   describe "when parsing a line with ip, hostname and aliases" do
>
>     it "should parse alias from the third field" do
> -       <at> provider.parse_line("127.0.0.1   localhost  
localhost.localdomain")[:host_aliases].should == ["localhost.localdomain"]
> +       <at> provider.parse_line("127.0.0.1   localhost  
localhost.localdomain")[:host_aliases].should == "localhost.localdomain"
>     end
>
>     it "should parse multiple aliases" do
> -       <at> provider.parse_line("127.0.0.1 host alias1 alias2")[:host_aliases].should ==
['alias1', 'alias2']
> -       <at> provider.parse_line("127.0.0.1 host alias1\talias2")[:host_aliases].should ==
['alias1', 'alias2']
> -       <at> provider.parse_line("127.0.0.1 host alias1\talias2   alias3")[:host_aliases].should
== ['alias1', 'alias2', 'alias3']
> +       <at> provider.parse_line("127.0.0.1 host alias1 alias2")[:host_aliases].should == 'alias1 alias2'
> +       <at> provider.parse_line("127.0.0.1 host alias1\talias2")[:host_aliases].should == 'alias1 alias2'
> +       <at> provider.parse_line("127.0.0.1 host alias1\talias2   alias3")[:host_aliases].should
== 'alias1 alias2 alias3'
>     end
>
>   end
>  <at>  <at>  -114,7 +122,7  <at>  <at>  describe provider_class do
>     end
>
>     it "should parse all host_aliases from the third field" do
> -       <at> provider.parse_line( <at> testline)[:host_aliases].should == ['alias1' ,'alias2', 'alias3']
> +       <at> provider.parse_line( <at> testline)[:host_aliases].should == 'alias1 alias2 alias3'
>     end
>
>     it "should parse the comment after the first '#' character" do
>  <at>  <at>  -143,7 +151,7  <at>  <at>  describe provider_class do
>       host = mkhost(
>         :name   => 'localhost.localdomain',
>         :ip     => '127.0.0.1',
> -        :host_aliases => ['localhost'],
> +        :host_aliases => 'localhost',
>         :ensure => :present
>       )
>       genhost(host).should == "127.0.0.1\tlocalhost.localdomain\tlocalhost\n"
>  <at>  <at>  -156,7 +164,7  <at>  <at>  describe provider_class do
>         :host_aliases => [ 'a1','a2','a3','a4' ],
>         :ensure     => :present
>       )
> -      genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\n"
> +      genhost(host).should == "192.0.0.1\thost\ta1 a2 a3 a4\n"
>     end
>
>     it "should be able to generate a simple hostfile entry with comments" do
>  <at>  <at>  -173,7 +181,7  <at>  <at>  describe provider_class do
>       host = mkhost(
>         :name   => 'localhost.localdomain',
>         :ip     => '127.0.0.1',
> -        :host_aliases => ['localhost'],
> +        :host_aliases => 'localhost',
>         :comment => 'Bazinga!',
>         :ensure => :present
>       )
>  <at>  <at>  -188,7 +196,7  <at>  <at>  describe provider_class do
>         :comment      => 'Bazinga!',
>         :ensure       => :present
>       )
> -      genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\t# Bazinga!\n"
> +      genhost(host).should == "192.0.0.1\thost\ta1 a2 a3 a4\t# Bazinga!\n"
>     end
>
>   end
> diff --git a/spec/unit/type/host_spec.rb b/spec/unit/type/host_spec.rb
> index 12ae2af..4c15f76 100755
> --- a/spec/unit/type/host_spec.rb
> +++ b/spec/unit/type/host_spec.rb
>  <at>  <at>  -2,12 +2,14  <at>  <at> 
>
>  require File.dirname(__FILE__) + '/../../spec_helper'
>
> -ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key)
> +host = Puppet::Type.type(:host)
>
> -describe Puppet::Type.type(:host) do
> +describe host do
>   before do
> -     <at> class = Puppet::Type.type(:host)
> +     <at> class = host
>      <at> catalog = Puppet::Resource::Catalog.new
> +     <at> provider = stub 'provider'
> +     <at> resource = stub 'resource', :resource => nil, :provider =>  <at> provider
>   end
>
>   it "should have :name be its namevar" do
>  <at>  <at>  -15,7 +17,7  <at>  <at>  describe Puppet::Type.type(:host) do
>   end
>
>   describe "when validating attributes" do
> -    [:name, :provider ].each do |param|
> +    [:name, :provider, :membership ].each do |param|
>       it "should have a #{param} parameter" do
>          <at> class.attrtype(param).should == :param
>       end
>  <at>  <at>  -26,6 +28,11  <at>  <at>  describe Puppet::Type.type(:host) do
>          <at> class.attrtype(property).should == :property
>       end
>     end
> +
> +    it "should have a list host_aliases" do
> +       <at> class.attrclass(:host_aliases).ancestors.should be_include(Puppet::Property::OrderedList)
> +    end
> +
>   end
>
>   describe "when validating values" do
>  <at>  <at>  -80,4 +87,35  <at>  <at>  describe Puppet::Type.type(:host) do
>       proc {  <at> class.new(:name => "foo", :host_aliases => ['alias1','']) }.should raise_error
>     end
>   end
> +
> +  describe "when syncing" do
> +
> +    it "should send the first value to the provider for ip property" do
> +       <at> ip =  <at> class.attrclass(:ip).new(:resource =>  <at> resource, :should => %w{192.168.0.1 192.168.0.2})
> +       <at> provider.expects(:ip=).with '192.168.0.1'
> +       <at> ip.sync
> +    end
> +
> +    it "should send the first value to the provider for comment property" do
> +       <at> comment =  <at> class.attrclass(:comment).new(:resource =>  <at> resource, :should => %w{Bazinga Notme})
> +       <at> provider.expects(:comment=).with 'Bazinga'
> +       <at> comment.sync
> +    end
> +
> +    it "should send the joined array to the provider for host_alias" do
> +       <at> host_aliases =  <at> class.attrclass(:host_aliases).new(:resource =>  <at> resource, :should =>
%w{foo bar})
> +       <at> resource.stubs(:[]).with(:membership).returns :inclusive
> +       <at> provider.expects(:host_aliases=).with 'foo bar'
> +       <at> host_aliases.sync
> +    end
> +
> +    it "should also use the specified delimiter for joining" do
> +       <at> host_aliases =  <at> class.attrclass(:host_aliases).new(:resource =>  <at> resource, :should =>
%w{foo bar})
> +       <at> resource.stubs(:[]).with(:membership).returns :inclusive
> +       <at> host_aliases.stubs(:delimiter).returns "\t"
> +       <at> provider.expects(:host_aliases=).with "foo\tbar"
> +       <at> host_aliases.sync
> +    end
> +
> +  end
>  end
> --
> 1.7.3.2
>
> --
> You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
> To post to this group, send email to puppet-dev <at> googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
>
>

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Luke Kanies | 1 Dec 2010 02:40
Gravatar

Re: Best way to parse /etc/services

On Nov 23, 2010, at 10:35 AM, Stefan Schulte wrote:

> On Tue, Nov 23, 2010 at 12:36:07AM -0600, Luke Kanies wrote:
>> If you invert it, it works better:
>> 
>> port { '22/udp': label => 'telnet' }
>> 
> 
> Havent thought of that an it looks pretty good for me. The duplication
> doesnt really bothers me, because I can easily write a define for that.
> 
> But what I dont like about that approach is, that while it is good for
> adding entries its just near to useless when you're trying to detect
> errors. Because I think you rareley want to change the
> label of a port but maybe you want to check, if the specified port is
> actually right. (At least I want to do that). So in the example abvoe:
> If I already have
> 
>  telnet 20/udp # someone copied&pasted but didnt changed port
> 
> in my file, puppet will happily adding another telnet entry.

Hmm, you're right - in that case, telnet would look up to multiple values, which isn't acceptable.  It's
really all three pieces that are primary - the name, number, and protocol.

Yuck.

-- 
If computers get too powerful, we can organize them into a committee --
that will do them in.    -- Bradley's Bromide
---------------------------------------------------------------------
Luke Kanies  -|-   http://puppetlabs.com   -|-   +1(615)594-8199

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

James Turnbull | 1 Dec 2010 04:15

Re: [PATCH/puppet 1/1] Bug #5423: This moves the home directory property before the uid property, thus minimizing room for damage when usermod is in use.

James Turnbull wrote:
> From: Jonathan Boyett <jonathan <at> failingservers.com>
> 

Jonathan

I don't see why the order of the properties would make any difference to
the way this applies. They are not executed in sequence from what I
understand.

Regards

James Turnbull

-- 
Puppet Labs - http://www.puppetlabs.com
C: 503-734-8571

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

James Turnbull | 1 Dec 2010 07:08

Re: Bug #5423: This moves the home directory property before the uid property, thus minimizing room for damage when usermod is in use.

Jonathan wrote:
> James-
> 
> I initially thought the same thing. Here's some steps to reproduce the
> issue:
> 
> 1. Make a simple manifest, similar to the one below:
> 
> user { "testy":
>         ensure => present,
>         uid => "2345",
>         gid => "2345",
>         groups => ["testy"],
>         shell => "/bin/bash",
>         home => "/tmp/poop",
>         managehome => "true",
>     }
> 
> group {"testy":
>         ensure => present,
>         gid => 2345,
> }
> 
> 2. Run puppet and ensure that the user and homedir have been created and
> check the permissions
> 3. Create a second directory anywhere on the filesystem, say /var/test
> 4. Open /etc/passwd and change the home dir for your test user to /
> 5. Open your test manifest and change the uid and gid of your test user
> to something else
> 6. Puppet changes the uid first, via usermod, so everything in / which
> was owned by the old uid is now owned by the new uid. Puppet changes the
> home dir after:
> 
> notice: //User[testy]/uid: uid changed '12345' to '2345'
> notice: //User[testy]/home: home changed '/' to '/tmp/poop'
> 
> You can verify by testing you second directory, wherever it may be.
> 
> This can cause some very bad things to happen. Let me know if you need
> more info.
> 

Jonathan

I understand the issue you're having... I just don't understand how your
fix can work - order of the properties in the type should have no impact
on how Puppet manages the resource.

Regards

James Turnbull

-- 
Puppet Labs - http://www.puppetlabs.com
C: 503-734-8571

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

James Turnbull | 1 Dec 2010 07:19

Re: Bug #5423: This moves the home directory property before the uid property, thus minimizing room for damage when usermod is in use.

Jonathan wrote:
> James-
> 
> I literally said those exact words. But on a whim I changed then order
> and it works correctly, every time. I haven't had a chance to dig deeper
> yet, so I'm not sure if the issue is in the namespace provider, but the
> patch definitely fixes it.
> 
> Try out the patch using the same steps below. It'll work, I swear :-)

It's still not right way to fix it ... even if it does.

I'd like to dig a bit deeper I think.

Regards

James Turnbull

-- 
Puppet Labs - http://www.puppetlabs.com
C: 503-734-8571

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Héctor Rivas Gándara | 1 Dec 2010 08:45
Picon
Gravatar

Re: Class ProviderAixuseradd is already defined in Puppet::Type::User

On Tue, Nov 30, 2010 at 8:33 PM, Luke Kanies <luke <at> puppetlabs.com> wrote:

>> I do not require directly any puppet module.
>
> You're directly requiring the aixobject parent class, which shouldn't be necessary.  Can you try
removing that and see if it fixes it?

It started to work again without change, but I will remove that require.

Thank you.

--
Atentamente
Héctor Rivas

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

James Turnbull | 1 Dec 2010 09:10
Gravatar

[PATCH/puppet 1/1] Reverted a portion of #4149 because it broke --parseonly totally.

See #5081 for further information.

This probably isn't the right fix but the current fix has broken
numerous people's parseonly hooks etc.

Signed-off-by: James Turnbull <james <at> lovedthanlost.net>
---
 lib/puppet/application/apply.rb |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 59a95d3..701d510 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
 <at>  <at>  -65,7 +65,8  <at>  <at>  class Puppet::Application::Apply < Puppet::Application
       Puppet[:manifest] = command_line.args.shift
     end
     begin
-      Puppet::Node::Environment.new(Puppet[:environment]).known_resource_types
+      Puppet::Resource::TypeCollection.new(Puppet[:environment]).perform_initial_import
+      #Puppet::Node::Environment.new(Puppet[:environment]).known_resource_types
     rescue => detail
       Puppet.err detail
       exit 1
-- 
1.7.2.3

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

James Turnbull | 1 Dec 2010 09:13
Gravatar

[PATCH/puppet 1/1] Reverted a portion of #4149 because it broke --parseonly totally.

See #5081 for further information.

This probably isn't the right fix but the current fix has broken
numerous people's parseonly hooks etc.

Signed-off-by: James Turnbull <james <at> lovedthanlost.net>
---
 lib/puppet/application/apply.rb |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 59a95d3..7494b27 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
 <at>  <at>  -65,7 +65,7  <at>  <at>  class Puppet::Application::Apply < Puppet::Application
       Puppet[:manifest] = command_line.args.shift
     end
     begin
-      Puppet::Node::Environment.new(Puppet[:environment]).known_resource_types
+      Puppet::Resource::TypeCollection.new(Puppet[:environment]).perform_initial_import
     rescue => detail
       Puppet.err detail
       exit 1
-- 
1.7.2.3

--

-- 
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev <at> googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscribe <at> googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.


Gmane