+1. I had some style quibbles but I've talked myself out of them. On Mon, Oct 11, 2010 at 9:28 PM, Markus Roberts <[email protected]> wrote:
> This is intended to be a minimal fix, with tests, to prevent chage from > running > unless needed. > > Signed-off-by: Markus Roberts <[email protected]> > --- > lib/puppet/provider/nameservice.rb | 4 ++- > lib/puppet/provider/user/user_role_add.rb | 4 ++- > lib/puppet/provider/user/useradd.rb | 11 +++---- > spec/unit/provider/user/useradd_spec.rb | 42 > +++++++++++++++++++++++++++++ > 4 files changed, 53 insertions(+), 8 deletions(-) > > diff --git a/lib/puppet/provider/nameservice.rb > b/lib/puppet/provider/nameservice.rb > index 9830fab..d57052b 100644 > --- a/lib/puppet/provider/nameservice.rb > +++ b/lib/puppet/provider/nameservice.rb > @@ -165,7 +165,9 @@ class Puppet::Provider::NameService < Puppet::Provider > > begin > execute(self.addcmd) > - execute(self.passcmd) if self.feature? :manages_password_age > + if feature?(:manages_password_age) && (cmd = passcmd) > + execute(cmd) > + end > rescue Puppet::ExecutionFailure => detail > raise Puppet::Error, "Could not create #[email protected]} #{@ > resource.name}: #{detail}" > end > diff --git a/lib/puppet/provider/user/user_role_add.rb > b/lib/puppet/provider/user/user_role_add.rb > index 7e7ad78..b835895 100644 > --- a/lib/puppet/provider/user/user_role_add.rb > +++ b/lib/puppet/provider/user/user_role_add.rb > @@ -81,7 +81,9 @@ Puppet::Type.type(:user).provide :user_role_add, :parent > => :useradd, :source => > run(transition("normal"), "transition role to") > else > run(addcmd, "create") > - run(passcmd, "change password policy for") > + if cmd = passcmd > + run(cmd, "change password policy for") > + end > end > # added to handle case when password is specified > self.password = @resource[:password] if @resource[:password] > diff --git a/lib/puppet/provider/user/useradd.rb > b/lib/puppet/provider/user/useradd.rb > index 9a62db4..5a163f3 100644 > --- a/lib/puppet/provider/user/useradd.rb > +++ b/lib/puppet/provider/user/useradd.rb > @@ -70,13 +70,12 @@ Puppet::Type.type(:user).provide :useradd, :parent => > Puppet::Provider::NameServ > end > > def passcmd > - cmd = [command(:password)] > - [:password_min_age, :password_max_age].each do |property| > - if value = @resource.should(property) > - cmd << flag(property) << value > - end > + age_limits = [:password_min_age, :password_max_age].select { > |property| @resource.should(property) } > + if age_limits.empty? > + nil > + else > + [command(:password),age_limits.collect { |property| [flag(property), > @resource.should(property)]}, @resource[:name]].flatten > end > - cmd << @resource[:name] > end > > def min_age > diff --git a/spec/unit/provider/user/useradd_spec.rb > b/spec/unit/provider/user/useradd_spec.rb > index 26367c5..9ebba59 100755 > --- a/spec/unit/provider/user/useradd_spec.rb > +++ b/spec/unit/provider/user/useradd_spec.rb > @@ -131,4 +131,46 @@ describe provider_class do > @provider.addcmd.must == ["useradd", "-G", "somegroup", "-o", "-m", > "someuser"] > end > end > + > + describe "when calling passcmd" do > + before do > + @resource.stubs(:allowdupe?).returns true > + @resource.stubs(:managehome?).returns true > + end > + > + it "should call command with :pass" do > + @provider.expects(:command).with(:password) > + @provider.passcmd > + end > + > + it "should return nil if neither min nor max is set" do > + @resource.stubs(:should).with(:password_min_age).returns nil > + @resource.stubs(:should).with(:password_max_age).returns nil > + @provider.passcmd.must == nil > + end > + > + it "should return a chage command array with -m <value> and the user > name if password_min_age is set" do > + @provider.stubs(:command).with(:password).returns("chage") > + @resource.stubs(:[]).with(:name).returns("someuser") > + @resource.stubs(:should).with(:password_min_age).returns 123 > + @resource.stubs(:should).with(:password_max_age).returns nil > + @provider.passcmd.must == ['chage','-m',123,'someuser'] > + end > + > + it "should return a chage command array with -M <value> if > password_max_age is set" do > + @provider.stubs(:command).with(:password).returns("chage") > + @resource.stubs(:[]).with(:name).returns("someuser") > + @resource.stubs(:should).with(:password_min_age).returns nil > + @resource.stubs(:should).with(:password_max_age).returns 999 > + @provider.passcmd.must == ['chage','-M',999,'someuser'] > + end > + > + it "should return a chage command array with -M <value> -m <value> if > both password_min_age and password_max_age are set" do > + @provider.stubs(:command).with(:password).returns("chage") > + @resource.stubs(:[]).with(:name).returns("someuser") > + @resource.stubs(:should).with(:password_min_age).returns 123 > + @resource.stubs(:should).with(:password_max_age).returns 999 > + @provider.passcmd.must == ['chage','-m',123,'-M',999,'someuser'] > + end > + end > end > -- > 1.7.0.4 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > 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 [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
