Merge pull request #5355 from coderanger/solaris-r

Don't use -r for users or groups on Solaris.

4 files changed, 178 insertions, 141 deletions

diff --git a/lib/chef/provider/group/groupadd.rb b/lib/chef/provider/group/groupadd.rb
index bc6b5d0208..162875f9f1 100644
--- a/lib/chef/provider/group/groupadd.rb
+++ b/lib/chef/provider/group/groupadd.rb
@@ -126,7 +126,8 @@ class Chef
 
         def groupadd_options
           opts = ""
-          opts << " -r" if @new_resource.system
+          # Solaris doesn't support system groups.
+          opts << " -r" if @new_resource.system && !node.platform?("solaris2")
           opts << " -o" if @new_resource.non_unique
           opts
         end
diff --git a/lib/chef/provider/user/solaris.rb b/lib/chef/provider/user/solaris.rb
index 8d3df9e68b..3012fcec23 100644
--- a/lib/chef/provider/user/solaris.rb
+++ b/lib/chef/provider/user/solaris.rb
@@ -73,6 +73,19 @@ class Chef
 
         private
 
+        # Override the version from {#Useradd} because Solaris doesn't support
+        # system users and therefore has no `-r` option.
+        #
+        # @since 12.15
+        # @api private
+        # @see Useradd#useradd_options
+        # @return [Array<String>]
+        def useradd_options
+          opts = []
+          opts << "-M" unless managing_home_dir?
+          opts
+        end
+
         def manage_password
           if @current_resource.password != @new_resource.password && @new_resource.password
             Chef::Log.debug("#{@new_resource} setting password to #{@new_resource.password}")
diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb
index 93159dd16e..054d953414 100644
--- a/spec/unit/provider/group/groupadd_spec.rb
+++ b/spec/unit/provider/group/groupadd_spec.rb
@@ -18,156 +18,175 @@
 
 require "spec_helper"
 
-describe Chef::Provider::Group::Groupadd, "set_options" do
-  before do
-    @node = Chef::Node.new
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Group.new("aj")
-    @new_resource.gid(50)
-    @new_resource.members(%w{root aj})
-    @new_resource.system false
-    @new_resource.non_unique false
-    @current_resource = Chef::Resource::Group.new("aj")
-    @current_resource.gid(50)
-    @current_resource.members(%w{root aj})
-    @current_resource.system false
-    @current_resource.non_unique false
-    @provider = Chef::Provider::Group::Groupadd.new(@new_resource, @run_context)
-    @provider.current_resource = @current_resource
-  end
-
-  field_list = {
-    :gid => "-g",
-  }
-
-  field_list.each do |attribute, option|
-    it "should check for differences in #{attribute} between the current and new resources" do
-      expect(@new_resource).to receive(attribute)
-      expect(@current_resource).to receive(attribute)
-      @provider.set_options
+describe Chef::Provider::Group::Groupadd do
+  let(:node) { Chef::Node.new }
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+  let(:new_resource) do
+    Chef::Resource::Group.new("aj").tap do |r|
+      r.gid 50
+      r.members %w{root aj}
+      r.system false
+      r.non_unique false
     end
-    it "should set the option for #{attribute} if the new resources #{attribute} is not null" do
-      allow(@new_resource).to receive(attribute).and_return("wowaweea")
-      expect(@provider.set_options).to eql(" #{option} '#{@new_resource.send(attribute)}' #{@new_resource.group_name}")
+  end
+  let(:current_resource) do
+    Chef::Resource::Group.new("aj").tap do |r|
+      r.gid 50
+      r.members %w{root aj}
+      r.system false
+      r.non_unique false
     end
   end
-
-  it "should combine all the possible options" do
-    match_string = ""
-    field_list.sort { |a, b| a[0] <=> b[0] }.each do |attribute, option|
-      allow(@new_resource).to receive(attribute).and_return("hola")
-      match_string << " #{option} 'hola'"
+  let(:provider) do
+    described_class.new(new_resource, run_context).tap do |p|
+      p.current_resource = current_resource
     end
-    match_string << " aj"
-    expect(@provider.set_options).to eql(match_string)
   end
 
-  describe "when we want to create a system group" do
-    it "should not set groupadd_options '-r' when system is false" do
-      @new_resource.system(false)
-      expect(@provider.groupadd_options).not_to match(/-r/)
+  describe "#set_options" do
+    field_list = {
+      :gid => "-g",
+    }
+
+    field_list.each do |attribute, option|
+      it "should check for differences in #{attribute} between the current and new resources" do
+        expect(new_resource).to receive(attribute)
+        expect(current_resource).to receive(attribute)
+        provider.set_options
+      end
+
+      it "should set the option for #{attribute} if the new resources #{attribute} is not null" do
+        allow(new_resource).to receive(attribute).and_return("wowaweea")
+        expect(provider.set_options).to eql(" #{option} '#{new_resource.send(attribute)}' #{new_resource.group_name}")
+      end
     end
 
-    it "should set groupadd -r if system is true" do
-      @new_resource.system(true)
-      expect(@provider.groupadd_options).to eq(" -r")
+    it "should combine all the possible options" do
+      match_string = ""
+      field_list.sort { |a, b| a[0] <=> b[0] }.each do |attribute, option|
+        allow(new_resource).to receive(attribute).and_return("hola")
+        match_string << " #{option} 'hola'"
+      end
+      match_string << " aj"
+      expect(provider.set_options).to eql(match_string)
     end
-  end
 
-  describe "when we want to create a non_unique gid group" do
-    it "should not set groupadd_options '-o' when non_unique is false" do
-      @new_resource.non_unique(false)
-      expect(@provider.groupadd_options).not_to match(/-o/)
+    describe "when we want to create a system group" do
+      it "should not set groupadd_options '-r' when system is false" do
+        new_resource.system(false)
+        expect(provider.groupadd_options).not_to match(/-r/)
+      end
+
+      it "should set groupadd -r if system is true" do
+        new_resource.system(true)
+        expect(provider.groupadd_options).to eq(" -r")
+      end
+
+      context "on Solaris" do
+        before { node.automatic["platform"] = "solaris2" }
+        it "should not set groupadd -r if system is true" do
+          new_resource.system(true)
+          expect(provider.groupadd_options).not_to match(/-r/)
+        end
+      end
     end
 
-    it "should set groupadd -o if non_unique is true" do
-      @new_resource.non_unique(true)
-      expect(@provider.groupadd_options).to eq(" -o")
-    end
-  end
-end
+    describe "when we want to create a non_unique gid group" do
+      it "should not set groupadd_options '-o' when non_unique is false" do
+        new_resource.non_unique(false)
+        expect(provider.groupadd_options).not_to match(/-o/)
+      end
 
-describe Chef::Provider::Group::Groupadd, "create_group" do
-  before do
-    @node = Chef::Node.new
-    @new_resource = Chef::Resource::Group.new("aj")
-    @provider = Chef::Provider::Group::Groupadd.new(@node, @new_resource)
-    allow(@provider).to receive(:run_command).and_return(true)
-    allow(@provider).to receive(:set_options).and_return(" monkey")
-    allow(@provider).to receive(:groupadd_options).and_return("")
-    allow(@provider).to receive(:modify_group_members).and_return(true)
+      it "should set groupadd -o if non_unique is true" do
+        new_resource.non_unique(true)
+        expect(provider.groupadd_options).to eq(" -o")
+      end
+    end
   end
 
-  it "should run groupadd with the return of set_options" do
-    expect(@provider).to receive(:run_command).with({ :command => "groupadd monkey" }).and_return(true)
-    @provider.create_group
-  end
+  describe "#create_group" do
+    before do
+      allow(provider).to receive(:run_command).and_return(true)
+      allow(provider).to receive(:set_options).and_return(" monkey")
+      allow(provider).to receive(:groupadd_options).and_return("")
+      allow(provider).to receive(:modify_group_members).and_return(true)
+    end
 
-  it "should modify the group members" do
-    expect(@provider).to receive(:modify_group_members).and_return(true)
-    @provider.create_group
-  end
-end
+    it "should run groupadd with the return of set_options" do
+      expect(provider).to receive(:run_command).with({ :command => "groupadd monkey" }).and_return(true)
+      provider.create_group
+    end
 
-describe Chef::Provider::Group::Groupadd do
-  before do
-    @node = Chef::Node.new
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Group.new("aj")
-    @provider = Chef::Provider::Group::Groupadd.new(@new_resource, @run_context)
-    allow(@provider).to receive(:run_command).and_return(true)
-    allow(@provider).to receive(:set_options).and_return(" monkey")
+    it "should modify the group members" do
+      expect(provider).to receive(:modify_group_members).and_return(true)
+      provider.create_group
+    end
   end
 
-  describe "manage group" do
+  describe "#manage_group" do
+    before do
+      allow(provider).to receive(:run_command).and_return(true)
+      allow(provider).to receive(:set_options).and_return(" monkey")
+    end
 
     it "should run groupmod with the return of set_options" do
-      allow(@provider).to receive(:modify_group_members).and_return(true)
-      expect(@provider).to receive(:run_command).with({ :command => "groupmod monkey" }).and_return(true)
-      @provider.manage_group
+      allow(provider).to receive(:modify_group_members).and_return(true)
+      expect(provider).to receive(:run_command).with({ :command => "groupmod monkey" }).and_return(true)
+      provider.manage_group
     end
 
     it "should modify the group members" do
-      expect(@provider).to receive(:modify_group_members).and_return(true)
-      @provider.manage_group
+      expect(provider).to receive(:modify_group_members).and_return(true)
+      provider.manage_group
     end
   end
 
-  describe "remove_group" do
+  describe "#remove_group" do
+    before do
+      allow(provider).to receive(:run_command).and_return(true)
+      allow(provider).to receive(:set_options).and_return(" monkey")
+    end
 
     it "should run groupdel with the new resources group name" do
-      expect(@provider).to receive(:run_command).with({ :command => "groupdel aj" }).and_return(true)
-      @provider.remove_group
+      expect(provider).to receive(:run_command).with({ :command => "groupdel aj" }).and_return(true)
+      provider.remove_group
     end
   end
 
   [:add_member, :remove_member, :set_members].each do |m|
     it "should raise an error when calling #{m}" do
-      expect { @provider.send(m, [ ]) }.to raise_error(Chef::Exceptions::Group, "you must override #{m} in #{@provider}")
+      expect { provider.send(m, [ ]) }.to raise_error(Chef::Exceptions::Group, "you must override #{m} in #{provider}")
     end
   end
 
-  describe "load_current_resource" do
+  describe "#load_current_resource" do
+    before do
+      allow(provider).to receive(:run_command).and_return(true)
+      allow(provider).to receive(:set_options).and_return(" monkey")
+    end
+
     before do
       allow(File).to receive(:exists?).and_return(false)
-      @provider.define_resource_requirements
+      provider.define_resource_requirements
     end
+
     it "should raise an error if the required binary /usr/sbin/groupadd doesn't exist" do
       expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(false)
-      expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
+      expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
+
     it "should raise an error if the required binary /usr/sbin/groupmod doesn't exist" do
       expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(true)
       expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(false)
-      expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
+      expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
+
     it "should raise an error if the required binary /usr/sbin/groupdel doesn't exist" do
       expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(true)
       expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(true)
       expect(File).to receive(:exists?).with("/usr/sbin/groupdel").and_return(false)
-      expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
+      expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
   end
diff --git a/spec/unit/provider/user/solaris_spec.rb b/spec/unit/provider/user/solaris_spec.rb
index 860c9e41dd..d6c0b886fc 100644
--- a/spec/unit/provider/user/solaris_spec.rb
+++ b/spec/unit/provider/user/solaris_spec.rb
@@ -29,26 +29,31 @@ describe Chef::Provider::User::Solaris do
     Struct.new(:stdout, :stderr, :exitstatus)
   end
 
-  subject(:provider) do
-    p = described_class.new(@new_resource, @run_context)
-    p.current_resource = @current_resource
+  let(:node) do
+    Chef::Node.new.tap do |node|
+      node.automatic["platform"] = "solaris2"
+    end
+  end
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+  let(:new_resource) do
+    Chef::Resource::User::SolarisUser.new("adam", @run_context)
+  end
+  let(:current_resource) do
+    Chef::Resource::User::SolarisUser.new("adam", @run_context)
+  end
 
-    # Prevent the useradd-based provider tests from trying to write /etc/shadow
-    allow(p).to receive(:write_shadow_file)
-    p
+  subject(:provider) do
+    described_class.new(new_resource, run_context).tap do |p|
+      p.current_resource = current_resource
+      # Prevent the useradd-based provider tests from trying to write /etc/shadow
+      allow(p).to receive(:write_shadow_file)
+    end
   end
 
   describe "when we want to set a password" do
     before(:each) do
-      @node = Chef::Node.new
-      @events = Chef::EventDispatch::Dispatcher.new
-      @run_context = Chef::RunContext.new(@node, {}, @events)
-
-      @new_resource = Chef::Resource::User::SolarisUser.new("adam", @run_context)
-      @current_resource = Chef::Resource::User::SolarisUser.new("adam", @run_context)
-
-      @new_resource.password "hocus-pocus"
-
+      new_resource.password "hocus-pocus"
     end
 
     it "should use its own shadow file writer to set the password" do
@@ -68,53 +73,52 @@ describe Chef::Provider::User::Solaris do
       # may not be able to write to /etc for tests...
       temp_file = Tempfile.new("shadow")
       allow(Tempfile).to receive(:new).with("shadow", "/etc").and_return(temp_file)
-      @new_resource.password "verysecurepassword"
+      new_resource.password "verysecurepassword"
       provider.manage_user
       expect(::File.open(password_file.path, "r").read).to match(/adam:verysecurepassword:/)
       password_file.unlink
     end
   end
 
-  describe "when managing user locked status" do
-    before(:each) do
-      @node = Chef::Node.new
-      @events = Chef::EventDispatch::Dispatcher.new
-      @run_context = Chef::RunContext.new(@node, {}, @events)
-
-      @new_resource = Chef::Resource::User::SolarisUser.new("dave")
-      @current_resource = @new_resource.dup
-
-      @provider = Chef::Provider::User::Solaris.new(@new_resource, @run_context)
-      @provider.current_resource = @current_resource
+  describe "#create_user" do
+    context "with a system user" do
+      before { new_resource.system(true) }
+      it "should not pass -r" do
+        expect(provider).to receive(:shell_out!).with("useradd", "-M", "adam")
+        provider.create_user
+      end
     end
+  end
+
+  describe "when managing user locked status" do
     describe "when determining if the user is locked" do
 
       # locked shadow lines
       [
-        "dave:LK:::::::",
-        "dave:*LK*:::::::",
-        "dave:*LK*foobar:::::::",
-        "dave:*LK*bahamas10:::::::",
-        "dave:*LK*L....:::::::",
+        "adam:LK:::::::",
+        "adam:*LK*:::::::",
+        "adam:*LK*foobar:::::::",
+        "adam:*LK*bahamas10:::::::",
+        "adam:*LK*L....:::::::",
       ].each do |shadow|
         it "should return true if user is locked with #{shadow}" do
           shell_return = shellcmdresult.new(shadow + "\n", "", 0)
-          expect(provider).to receive(:shell_out!).with("getent", "shadow", @new_resource.username).and_return(shell_return)
+          expect(provider).to receive(:shell_out!).with("getent", "shadow", "adam").and_return(shell_return)
           expect(provider.check_lock).to eql(true)
         end
       end
 
       # unlocked shadow lines
       [
-        "dave:NP:::::::",
-        "dave:*NP*:::::::",
-        "dave:foobar:::::::",
-        "dave:bahamas10:::::::",
-        "dave:L...:::::::",
+        "adam:NP:::::::",
+        "adam:*NP*:::::::",
+        "adam:foobar:::::::",
+        "adam:bahamas10:::::::",
+        "adam:L...:::::::",
       ].each do |shadow|
         it "should return false if user is unlocked with #{shadow}" do
           shell_return = shellcmdresult.new(shadow + "\n", "", 0)
-          expect(provider).to receive(:shell_out!).with("getent", "shadow", @new_resource.username).and_return(shell_return)
+          expect(provider).to receive(:shell_out!).with("getent", "shadow", "adam").and_return(shell_return)
           expect(provider.check_lock).to eql(false)
         end
       end
@@ -123,7 +127,7 @@ describe Chef::Provider::User::Solaris do
     describe "when locking the user" do
       it "should run passwd -l with the new resources username" do
         shell_return = shellcmdresult.new("", "", 0)
-        expect(provider).to receive(:shell_out!).with("passwd", "-l", @new_resource.username).and_return(shell_return)
+        expect(provider).to receive(:shell_out!).with("passwd", "-l", "adam").and_return(shell_return)
         provider.lock_user
       end
     end
@@ -131,7 +135,7 @@ describe Chef::Provider::User::Solaris do
     describe "when unlocking the user" do
       it "should run passwd -u with the new resources username" do
         shell_return = shellcmdresult.new("", "", 0)
-        expect(provider).to receive(:shell_out!).with("passwd", "-u", @new_resource.username).and_return(shell_return)
+        expect(provider).to receive(:shell_out!).with("passwd", "-u", "adam").and_return(shell_return)
         provider.unlock_user
       end
     end