diff options
author | Tim Smith <tsmith@chef.io> | 2020-11-18 22:17:17 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-11-18 22:17:17 -0800 |
commit | eb4cf191a0483fdb03cab199a8eb360026c4030a (patch) | |
tree | f7f04694ac376437662fb369088860d345d2b071 | |
parent | ab7b15ab9c8fe23fcd2ce861c69aabf77c19dd5a (diff) | |
parent | 23b7bf44a03000481ce2fc125c70434c37247b01 (diff) | |
download | chef-eb4cf191a0483fdb03cab199a8eb360026c4030a.tar.gz |
Merge pull request #10656 from jaymzh/fixuser
Signed-off-by: Tim Smith <tsmith@chef.io>
-rw-r--r-- | lib/chef/provider/group.rb | 4 | ||||
-rw-r--r-- | lib/chef/provider/user.rb | 26 | ||||
-rw-r--r-- | lib/chef/provider/user/mac.rb | 16 | ||||
-rw-r--r-- | lib/chef/provider/user/windows.rb | 13 | ||||
-rw-r--r-- | spec/unit/provider/user_spec.rb | 8 |
5 files changed, 48 insertions, 19 deletions
diff --git a/lib/chef/provider/group.rb b/lib/chef/provider/group.rb index 9e3d7f9a00..0cda3182ba 100644 --- a/lib/chef/provider/group.rb +++ b/lib/chef/provider/group.rb @@ -159,7 +159,7 @@ class Chef converge_by(["manage group #{new_resource.group_name}"] + change_desc) do manage_group - logger.info("#{new_resource} managed") + logger.info("#{new_resource} managed: #{change_desc.join(", ")}") end end @@ -168,7 +168,7 @@ class Chef converge_by(["modify group #{new_resource.group_name}"] + change_desc) do manage_group - logger.info("#{new_resource} modified") + logger.info("#{new_resource} modified: #{change_desc.join(", ")}") end end diff --git a/lib/chef/provider/user.rb b/lib/chef/provider/user.rb index d4a2651378..803d87d820 100644 --- a/lib/chef/provider/user.rb +++ b/lib/chef/provider/user.rb @@ -24,6 +24,7 @@ class Chef class User < Chef::Provider attr_accessor :user_exists, :locked + attr_accessor :change_desc def initialize(new_resource, run_context) super @@ -107,13 +108,20 @@ class Chef # <true>:: If a change is required # <false>:: If the users are identical def compare_user - return true if !new_resource.home.nil? && Pathname.new(new_resource.home).cleanpath != Pathname.new(current_resource.home).cleanpath + @change_desc = [] + if !new_resource.home.nil? && Pathname.new(new_resource.home).cleanpath != Pathname.new(current_resource.home).cleanpath + @change_desc << "change homedir from #{current_resource.home} to #{new_resource.home}" + end %i{comment shell password uid gid}.each do |user_attrib| - return true if !new_resource.send(user_attrib).nil? && new_resource.send(user_attrib).to_s != current_resource.send(user_attrib).to_s + new_val = new_resource.send(user_attrib) + cur_val = current_resource.send(user_attrib) + if !new_val.nil? && new_val.to_s != cur_val.to_s + @change_desc << "change #{user_attrib} from #{cur_val} to #{new_val}" + end end - false + !@change_desc.empty? end action :create do @@ -123,9 +131,9 @@ class Chef logger.info("#{new_resource} created") end elsif compare_user - converge_by("alter user #{new_resource.username}") do + converge_by(["alter user #{new_resource.username}"] + change_desc) do manage_user - logger.info("#{new_resource} altered") + logger.info("#{new_resource} altered, #{change_desc.join(", ")}") end end end @@ -142,18 +150,18 @@ class Chef action :manage do return unless @user_exists && compare_user - converge_by("manage user #{new_resource.username}") do + converge_by(["manage user #{new_resource.username}"] + change_desc) do manage_user - logger.info("#{new_resource} managed") + logger.info("#{new_resource} managed: #{change_desc.join(", ")}") end end action :modify do return unless compare_user - converge_by("modify user #{new_resource.username}") do + converge_by(["modify user #{new_resource.username}"] + change_desc) do manage_user - logger.info("#{new_resource} modified") + logger.info("#{new_resource} modified: #{change_desc.join(", ")}") end end diff --git a/lib/chef/provider/user/mac.rb b/lib/chef/provider/user/mac.rb index a3980aa12f..aef831907a 100644 --- a/lib/chef/provider/user/mac.rb +++ b/lib/chef/provider/user/mac.rb @@ -221,7 +221,17 @@ class Chef end def compare_user - %i{comment shell uid gid salt password admin secure_token hidden}.any? { |m| diverged?(m) } + @change_desc = [] + %i{comment shell uid gid salt password admin secure_token hidden}.each do |attr| + if diverged?(m) + desc = "Update #{attr}" + unless %i{password gid secure_token hidden}.include?(attr) + desc << " from #{current_resource.send(attr)} to #{new_resource.send(attr)}" + end + @change_desc << desc + end + end + !@change_desc.empty? end def manage_user @@ -290,9 +300,7 @@ class Chef end if diverged?(:hidden) - converge_by("alter hidden") do - set_hidden - end + converge_by("alter hidden") { set_hidden } end reload_user_plist diff --git a/lib/chef/provider/user/windows.rb b/lib/chef/provider/user/windows.rb index 6d50e2c8ee..32b2c35264 100644 --- a/lib/chef/provider/user/windows.rb +++ b/lib/chef/provider/user/windows.rb @@ -61,13 +61,20 @@ class Chef # <true>:: If a change is required # <false>:: If the users are identical def compare_user + @change_desc = [] unless @net_user.validate_credentials(new_resource.password) - logger.trace("#{new_resource} password has changed") - return true + @change_desc << "update password" end + %i{uid comment home shell full_name}.any? do |user_attrib| - !new_resource.send(user_attrib).nil? && new_resource.send(user_attrib) != current_resource.send(user_attrib) + new_val = new_resource.send(user_attrib) + cur_val = current_resource.send(user_attrib) + if !new_val.nil? && new_val != cur_val + @change_desc << "change #{user_attrib} from #{cur_val} to #{new_val}" + end end + + !@change_desc.empty? end def create_user diff --git a/spec/unit/provider/user_spec.rb b/spec/unit/provider/user_spec.rb index ac1c943dbf..16428de1a9 100644 --- a/spec/unit/provider/user_spec.rb +++ b/spec/unit/provider/user_spec.rb @@ -259,6 +259,7 @@ describe Chef::Provider::User do it "should call manage_user if the user exists and has mismatched properties" do @provider.user_exists = true allow(@provider).to receive(:compare_user).and_return(true) + allow(@provider).to receive(:change_desc).and_return([ ]) expect(@provider).to receive(:manage_user).and_return(true) @provider.action_create end @@ -266,6 +267,7 @@ describe Chef::Provider::User do it "should set the new_resources updated flag when it creates the user if we call manage_user" do @provider.user_exists = true allow(@provider).to receive(:compare_user).and_return(true) + allow(@provider).to receive(:change_desc).and_return([ ]) allow(@provider).to receive(:manage_user).and_return(true) @provider.action_create @provider.set_updated_status @@ -315,14 +317,16 @@ describe Chef::Provider::User do # @provider.stub(:manage_user).and_return(true) end - it "should run manage_user if the user exists and has mismatched properties" do + it "should call manage_user if the user exists and has mismatched properties" do expect(@provider).to receive(:compare_user).and_return(true) + allow(@provider).to receive(:change_desc).and_return([ ]) expect(@provider).to receive(:manage_user).and_return(true) @provider.action_manage end it "should set the new resources updated flag to true if manage_user is called" do allow(@provider).to receive(:compare_user).and_return(true) + allow(@provider).to receive(:change_desc).and_return([ ]) allow(@provider).to receive(:manage_user).and_return(true) @provider.action_manage @provider.set_updated_status @@ -360,12 +364,14 @@ describe Chef::Provider::User do it "should run manage_user if the user exists and has mismatched properties" do expect(@provider).to receive(:compare_user).and_return(true) + allow(@provider).to receive(:change_desc).and_return([ ]) expect(@provider).to receive(:manage_user).and_return(true) @provider.action_modify end it "should set the new resources updated flag to true if manage_user is called" do allow(@provider).to receive(:compare_user).and_return(true) + allow(@provider).to receive(:change_desc).and_return([ ]) allow(@provider).to receive(:manage_user).and_return(true) @provider.action_modify @provider.set_updated_status |