From 9f4e52f5c8067904c275cdf2f9fb0ec290fc6470 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Mon, 19 Sep 2016 18:53:43 -0700 Subject: Solaris doesn't support -r for users or group. Signed-off-by: Noah Kantrowitz --- lib/chef/provider/group/groupadd.rb | 3 ++- lib/chef/provider/user/useradd.rb | 3 ++- 2 files changed, 4 insertions(+), 2 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/useradd.rb b/lib/chef/provider/user/useradd.rb index 68b62812a7..c0dcafac4a 100644 --- a/lib/chef/provider/user/useradd.rb +++ b/lib/chef/provider/user/useradd.rb @@ -140,7 +140,8 @@ class Chef def useradd_options opts = [] - opts << "-r" if new_resource.system + # Solaris doesn't support system users. + opts << "-r" if new_resource.system && !node.platform?("solaris2") opts << "-M" unless managing_home_dir? opts end -- cgit v1.2.1 From f823d2624d821180a74cf376001e95d920688693 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Mon, 19 Sep 2016 18:53:58 -0700 Subject: Refactor these tests to be less repeat-y. Signed-off-by: Noah Kantrowitz --- spec/unit/provider/user/solaris_spec.rb | 77 +++++++++++++++------------------ 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/spec/unit/provider/user/solaris_spec.rb b/spec/unit/provider/user/solaris_spec.rb index 860c9e41dd..3e2ecb440f 100644 --- a/spec/unit/provider/user/solaris_spec.rb +++ b/spec/unit/provider/user/solaris_spec.rb @@ -29,26 +29,32 @@ 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 - # Prevent the useradd-based provider tests from trying to write /etc/shadow - allow(p).to receive(:write_shadow_file) - p + 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 + + 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,7 +74,7 @@ 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 @@ -76,45 +82,34 @@ describe Chef::Provider::User::Solaris do 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 - end 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 +118,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 +126,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 -- cgit v1.2.1 From 7db14d05be621a52a5dea3abf2bc2c7313ca4d00 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Mon, 19 Sep 2016 19:45:04 -0700 Subject: Add a test for create_user to make sure we have no `-r`s. Signed-off-by: Noah Kantrowitz --- spec/unit/provider/user/solaris_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/unit/provider/user/solaris_spec.rb b/spec/unit/provider/user/solaris_spec.rb index 3e2ecb440f..be8ad15916 100644 --- a/spec/unit/provider/user/solaris_spec.rb +++ b/spec/unit/provider/user/solaris_spec.rb @@ -81,6 +81,16 @@ describe Chef::Provider::User::Solaris do end end + 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 -- cgit v1.2.1 From 571cd1db65c8e79ae04237f250fd58873f420f41 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Tue, 20 Sep 2016 15:20:08 -0700 Subject: Refactor groupadd tests to a more normal RSpec layout. Signed-off-by: Noah Kantrowitz --- spec/unit/provider/group/groupadd_spec.rb | 203 ++++++++++++++++-------------- 1 file changed, 107 insertions(+), 96 deletions(-) diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb index 93159dd16e..f2c57cf38e 100644 --- a/spec/unit/provider/group/groupadd_spec.rb +++ b/spec/unit/provider/group/groupadd_spec.rb @@ -18,156 +18,167 @@ 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/) - 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/) + end - it "should set groupadd -o if non_unique is true" do - @new_resource.non_unique(true) - expect(@provider.groupadd_options).to eq(" -o") + it "should set groupadd -r if system is true" do + new_resource.system(true) + expect(provider.groupadd_options).to eq(" -r") + end end - end -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) - 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 - 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 + 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 modify the group members" do - expect(@provider).to receive(:modify_group_members).and_return(true) - @provider.create_group - end -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 -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 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 + + 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 + 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 + 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 + 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 -- cgit v1.2.1 From 1c990b07cdcf22444b68b0d26d06c1c0ffc5fa3f Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Tue, 20 Sep 2016 15:21:50 -0700 Subject: Add a test for the new Solaris behavior. Signed-off-by: Noah Kantrowitz --- spec/unit/provider/group/groupadd_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb index f2c57cf38e..8f6a3884b3 100644 --- a/spec/unit/provider/group/groupadd_spec.rb +++ b/spec/unit/provider/group/groupadd_spec.rb @@ -82,6 +82,14 @@ describe Chef::Provider::Group::Groupadd 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 describe "when we want to create a non_unique gid group" do -- cgit v1.2.1 From c58f0ff0c51bc919447bbbd4b049cc2579c5ed2a Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Tue, 20 Sep 2016 16:05:03 -0700 Subject: Style fixes. Signed-off-by: Noah Kantrowitz --- spec/unit/provider/group/groupadd_spec.rb | 24 ++++++++++++------------ spec/unit/provider/user/solaris_spec.rb | 1 - 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb index 8f6a3884b3..054d953414 100644 --- a/spec/unit/provider/group/groupadd_spec.rb +++ b/spec/unit/provider/group/groupadd_spec.rb @@ -24,18 +24,18 @@ describe Chef::Provider::Group::Groupadd do 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 + r.gid 50 + r.members %w{root aj} + r.system false + r.non_unique false end 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 + r.gid 50 + r.members %w{root aj} + r.system false + r.non_unique false end end let(:provider) do @@ -105,7 +105,7 @@ describe Chef::Provider::Group::Groupadd do end end - describe "create_group" do + describe "#create_group" do before do allow(provider).to receive(:run_command).and_return(true) allow(provider).to receive(:set_options).and_return(" monkey") @@ -124,7 +124,7 @@ describe Chef::Provider::Group::Groupadd do 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") @@ -142,7 +142,7 @@ describe Chef::Provider::Group::Groupadd do 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") @@ -160,7 +160,7 @@ describe Chef::Provider::Group::Groupadd do 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") diff --git a/spec/unit/provider/user/solaris_spec.rb b/spec/unit/provider/user/solaris_spec.rb index be8ad15916..d6c0b886fc 100644 --- a/spec/unit/provider/user/solaris_spec.rb +++ b/spec/unit/provider/user/solaris_spec.rb @@ -29,7 +29,6 @@ describe Chef::Provider::User::Solaris do Struct.new(:stdout, :stderr, :exitstatus) end - let(:node) do Chef::Node.new.tap do |node| node.automatic["platform"] = "solaris2" -- cgit v1.2.1 From f57fc526f3c779d530170517dd2f08a50554b22d Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Wed, 21 Sep 2016 16:49:00 -0700 Subject: Fully override the method because we are planning to phase out the shared base class anyway so the slight code duplication is fine. Signed-off-by: Noah Kantrowitz --- lib/chef/provider/user/solaris.rb | 13 +++++++++++++ lib/chef/provider/user/useradd.rb | 3 +-- 2 files changed, 14 insertions(+), 2 deletions(-) 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] + 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/lib/chef/provider/user/useradd.rb b/lib/chef/provider/user/useradd.rb index c0dcafac4a..68b62812a7 100644 --- a/lib/chef/provider/user/useradd.rb +++ b/lib/chef/provider/user/useradd.rb @@ -140,8 +140,7 @@ class Chef def useradd_options opts = [] - # Solaris doesn't support system users. - opts << "-r" if new_resource.system && !node.platform?("solaris2") + opts << "-r" if new_resource.system opts << "-M" unless managing_home_dir? opts end -- cgit v1.2.1