Merge pull request #5243 from chef/lcg/rewrite-linux-user-provider

Rewrite linux useradd provider

3 files changed, 27 insertions, 23 deletions

diff --git a/spec/functional/resource/user/useradd_spec.rb b/spec/functional/resource/user/useradd_spec.rb
index fb16a2b858..dadf696185 100644
--- a/spec/functional/resource/user/useradd_spec.rb
+++ b/spec/functional/resource/user/useradd_spec.rb
@@ -94,6 +94,7 @@ describe Chef::Provider::User::Useradd, metadata do
 
     ["cf-test"].each do |u|
       r = resource_for_platform("DELETE USER", run_context)
+      r.manage_home true
       r.username("cf-test")
       r.run_action(:remove)
     end
@@ -257,6 +258,14 @@ describe Chef::Provider::User::Useradd, metadata do
             expect(File).to exist(home)
           end
         end
+
+        context "and manage_home is the default" do
+          let(:manage_home) { nil }
+
+          it "does not create the home dir without `manage_home'" do
+            expect(File).not_to exist(home)
+          end
+        end
       end
 
       context "when a password is specified" do
diff --git a/spec/support/shared/unit/provider/useradd_based_user_provider.rb b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
index de851c4ad4..fc12b2d5b6 100644
--- a/spec/support/shared/unit/provider/useradd_based_user_provider.rb
+++ b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
@@ -100,7 +100,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
 
       it "should set useradd -r" do
         @new_resource.system(true)
-        expect(provider.useradd_options).to eq([ "-r" ])
+        expect(provider.useradd_options).to eq([ "-r", "-m" ])
       end
     end
 
@@ -111,21 +111,21 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
       end
 
       it "should set -m -d /homedir" do
-        expect(provider.universal_options).to eq(%w{-d /wowaweea -m})
-        expect(provider.useradd_options).to eq([])
+        expect(provider.universal_options).to eq(%w{-d /wowaweea})
+        expect(provider.usermod_options).to eq(%w{-m})
       end
     end
 
     describe "when the resource has a different home directory and supports home directory management (using real attributes)" do
       before do
-        allow(@new_resource).to receive(:home).and_return("/wowaweea")
-        allow(@new_resource).to receive(:manage_home).and_return(true)
-        allow(@new_resource).to receive(:non_unique).and_return(false)
+        @new_resource.home("/wowaweea")
+        @new_resource.manage_home true
+        @new_resource.non_unique false
       end
 
       it "should set -m -d /homedir" do
-        expect(provider.universal_options).to eql(%w{-d /wowaweea -m})
-        expect(provider.useradd_options).to eq([])
+        expect(provider.universal_options).to eq(%w{-d /wowaweea})
+        expect(provider.usermod_options).to eq(%w{-m})
       end
     end
 
@@ -179,7 +179,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
         command.concat(["-p", "abracadabra"]) if supported_useradd_options.key?("password")
         command.concat([ "-s", "/usr/bin/zsh",
                          "-u", "1000",
-                         "-r",
+                         "-r", "-m",
                          "adam" ])
         expect(provider).to receive(:shell_out!).with(*command).and_return(true)
         provider.create_user
@@ -220,7 +220,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
     end
 
     it "CHEF-3429: does not set -m if we aren't changing the home directory" do
-      expect(provider).to receive(:updating_home?).and_return(false)
+      expect(provider).to receive(:updating_home?).at_least(:once).and_return(false)
       command = ["usermod",
                   "-g", "23",
                   "adam" ]
diff --git a/spec/unit/provider/user/linux_spec.rb b/spec/unit/provider/user/linux_spec.rb
index ef12cec234..ac94592859 100644
--- a/spec/unit/provider/user/linux_spec.rb
+++ b/spec/unit/provider/user/linux_spec.rb
@@ -39,16 +39,6 @@ describe Chef::Provider::User::Linux do
 
   include_examples "a useradd-based user provider", supported_useradd_options
 
-  describe "manage_user" do
-    # CHEF-5247: Chef::Provider::User::Solaris subclasses Chef::Provider::User::Useradd, but does not use usermod to change passwords.
-    # Thus, a call to Solaris#manage_user calls Solaris#manage_password and Useradd#manage_user, but the latter should be a no-op.
-    it "should not run the command if universal_options is an empty array" do
-      allow(provider).to receive(:universal_options).and_return([])
-      expect(provider).not_to receive(:shell_out!)
-      provider.manage_user
-    end
-  end
-
   describe "manage_home behavior" do
     before(:each) do
       @new_resource = Chef::Resource::User::LinuxUser.new("adam", @run_context)
@@ -66,16 +56,21 @@ describe Chef::Provider::User::Linux do
     end
 
     it "defaults manage_home to true" do
-      expect( @new_resource.manage_home ).to be true
+      expect( @new_resource.manage_home ).to be false
     end
 
-    it "by default manage_home is true and we do not -M" do
-      expect( provider.useradd_options ).to eql([])
+    it "by default manage_home is false and we use -M" do
+      expect( provider.useradd_options ).to eql(["-M"])
     end
 
     it "setting manage_home to false includes -M" do
       @new_resource.manage_home false
       expect( provider.useradd_options ).to eql(["-M"])
     end
+
+    it "setting manage_home to true includes -m" do
+      @new_resource.manage_home true
+      expect( provider.useradd_options ).to eql(["-m"])
+    end
   end
 end