Creating user with no home directory set fails

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Unknown Unknown
  • Resolution: Fixed
  • Affects Version/s: 0.10.10
  • Fix Version/s: 0.10.10
  • Component/s: Chef Solo
  • Labels:
  • Environment:
    1. uname -a
      Linux text-12 3.2.0-24-generic #37-Ubuntu SMP Wed Apr 25 08:43:22 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux
    2. chef-solo -v
      Chef: 0.10.10.rc.3

Description

The following recipe:

user node[:redis][:user] do
system true
shell "/bin/false"
end

Produces this error:

[Fri, 04 May 2012 13:55:13 +0000] DEBUG: Chef::Exceptions::Exec: user[redis] (redis::default line 5) had an error: Chef::Exceptions::Exec: useradd -s '/bin/false' -d '' -r redis returned 3, expected 0
---- Begin output of useradd -s '/bin/false' -d '' -r redis ----
STDOUT: STDERR: useradd: invalid home directory ''
---- End output of useradd -s '/bin/false' -d '' -r redis ----

I'm still digging but seems to stem from one of the following commits:

using Pathname#cleanpath on paths before comparing them
https://github.com/opscode/chef/commit/530d8305dddfa2b4d39046fd88bd27ccacc4ec5d#chef/lib/chef/provider/user/useradd.rb

or

CHEF-3076: do not clear path when @current_user does not have #home
https://github.com/opscode/chef/commit/5255a4611baa8cd605a600db28368de85e6aaebd#chef/lib/chef/provider/user/useradd.rb

in 0.10.8 the method looked like this and things were working as expected:

def updating_home?
@current_resource.home != @new_resource.home && @new_resource.home
end

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Mike Glenn added a comment -

Just some more of the debug level out put:

[Fri, 04 May 2012 13:55:13 +0000] INFO: Processing user[redis] action create (redis::default line 5)
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: user[redis] user does not exist
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: user[redis] setting shell to /bin/false
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: user[redis] setting home to
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: Executing useradd -s '/bin/false' -d '' -r redis
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: ---- Begin output of useradd -s '/bin/false' -d '' -r redis ----
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: STDOUT:
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: STDERR: useradd: invalid home directory ''
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: ---- End output of useradd -s '/bin/false' -d '' -r redis ----
[Fri, 04 May 2012 13:55:13 +0000] DEBUG: Ran useradd -s '/bin/false' -d '' -r redis returned 3
[Fri, 04 May 2012 13:55:13 +0000] ERROR: user[redis] (redis::default line 5) has had an error
[Fri, 04 May 2012 13:55:13 +0000] ERROR: user[redis] (/var/chef/cookbooks/redis/recipes/default.rb:5:in `from_file') had an error:
user[redis] (redis::default line 5) had an error: Chef::Exceptions::Exec: useradd -s '/bin/false' -d '' -r redis returned 3, expected 0
---- Begin output of useradd -s '/bin/false' -d '' -r redis ----
STDOUT: STDERR: useradd: invalid home directory ''
---- End output of useradd -s '/bin/false' -d '' -r redis ----

Show
Mike Glenn added a comment - Just some more of the debug level out put: [Fri, 04 May 2012 13:55:13 +0000] INFO: Processing user [redis] action create (redis::default line 5) [Fri, 04 May 2012 13:55:13 +0000] DEBUG: user [redis] user does not exist [Fri, 04 May 2012 13:55:13 +0000] DEBUG: user [redis] setting shell to /bin/false [Fri, 04 May 2012 13:55:13 +0000] DEBUG: user [redis] setting home to [Fri, 04 May 2012 13:55:13 +0000] DEBUG: Executing useradd -s '/bin/false' -d '' -r redis [Fri, 04 May 2012 13:55:13 +0000] DEBUG: ---- Begin output of useradd -s '/bin/false' -d '' -r redis ---- [Fri, 04 May 2012 13:55:13 +0000] DEBUG: STDOUT: [Fri, 04 May 2012 13:55:13 +0000] DEBUG: STDERR: useradd: invalid home directory '' [Fri, 04 May 2012 13:55:13 +0000] DEBUG: ---- End output of useradd -s '/bin/false' -d '' -r redis ---- [Fri, 04 May 2012 13:55:13 +0000] DEBUG: Ran useradd -s '/bin/false' -d '' -r redis returned 3 [Fri, 04 May 2012 13:55:13 +0000] ERROR: user [redis] (redis::default line 5) has had an error [Fri, 04 May 2012 13:55:13 +0000] ERROR: user [redis] (/var/chef/cookbooks/redis/recipes/default.rb:5:in `from_file') had an error: user [redis] (redis::default line 5) had an error: Chef::Exceptions::Exec: useradd -s '/bin/false' -d '' -r redis returned 3, expected 0 ---- Begin output of useradd -s '/bin/false' -d '' -r redis ---- STDOUT: STDERR: useradd: invalid home directory '' ---- End output of useradd -s '/bin/false' -d '' -r redis ----
Hide
Permalink
Mike Glenn added a comment -

The new logic here is definitely whats causing the issue:

 
def updating_home?
  # will return false if paths are equivalent
  # Pathname#cleanpath does a better job than ::File::expand_path (on both unix and windows)
  # ::File.expand_path("///tmp") == ::File.expand_path("/tmp") => false
  # ::File.expand_path("\\tmp") => "C:/tmp"
  @current_resource.home.nil? or
    (@new_resource.home and Pathname.new(@current_resource.home).cleanpath != Pathname.new(@new_resource.home).cleanpath)
end

The first check of @current_resource.home.nil? is always nil for a non-existing user, this short circuits the logic and forces the code into the section to include home directory options when creating a user.

I can submit a patch but I'm a bit confused as to if the current logic should even be checking these parameters since manage_home and supports[:manage_home] are false by default. This says to me that there is no need to compare the current and new home directories.

If someone with a bit more context on how this logic should flow can let me know what the design intent is I'll be happy to submit a pull request.

Show
Mike Glenn added a comment - The new logic here is definitely whats causing the issue: def updating_home? # will return false if paths are equivalent # Pathname#cleanpath does a better job than ::File::expand_path (on both unix and windows) # ::File.expand_path("///tmp") == ::File.expand_path("/tmp") => false # ::File.expand_path("\\tmp") => "C:/tmp" @current_resource.home.nil? or (@new_resource.home and Pathname.new(@current_resource.home).cleanpath != Pathname.new(@new_resource.home).cleanpath) end The first check of @current_resource.home.nil? is always nil for a non-existing user, this short circuits the logic and forces the code into the section to include home directory options when creating a user. I can submit a patch but I'm a bit confused as to if the current logic should even be checking these parameters since manage_home and supports [:manage_home] are false by default. This says to me that there is no need to compare the current and new home directories. If someone with a bit more context on how this logic should flow can let me know what the design intent is I'll be happy to submit a pull request.
Hide
Permalink
Mike Glenn added a comment -

I took a shot at sorting out the logic to make it match the docs and the principle of least astonishment.

https://github.com/opscode/chef/pull/286

Show
Mike Glenn added a comment - I took a shot at sorting out the logic to make it match the docs and the principle of least astonishment. https://github.com/opscode/chef/pull/286
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Thank you for taking the time to help make Chef better! We require a Contributor License Agreement (CLA) from contributors to help protect all Chef users. It is easy to complete one online. Instructions to do so are on our wiki, which is linked below. Once your account is updated, you will have access to 'resolve' tickets that indicates to the code review team that your contribution is ready for review.

http://wiki.opscode.com/display/chef/How+to+Contribute

Show
Bryan McLellan [Opscode] added a comment - Thank you for taking the time to help make Chef better! We require a Contributor License Agreement (CLA) from contributors to help protect all Chef users. It is easy to complete one online. Instructions to do so are on our wiki, which is linked below. Once your account is updated, you will have access to 'resolve' tickets that indicates to the code review team that your contribution is ready for review. http://wiki.opscode.com/display/chef/How+to+Contribute
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

I get a lot of failing unit tests from on this branch:

Failures:

  1) Chef::Provider::User::Useradd when setting option when we want to create a system user should set useradd -r
     Failure/Error: @provider.useradd_options.should == " -r"
     NameError:
       Cannot find a resource for useradd_options on  version 
     # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node'
     # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:106

  2) Chef::Provider::User::Useradd when setting option when the resource has a different home directory and supports home directory management should set -d /homedir -m
     Failure/Error: @provider.universal_options.should == " -d '/wowaweea'"
       expected: " -d '/wowaweea'"
            got: "" (using ==)
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:118

  3) Chef::Provider::User::Useradd when setting option when the resource has a different home directory and supports home directory management (using real attributes) should set -d /homedir -m
     Failure/Error: @provider.universal_options.should eql(" -d '/wowaweea'")
       
       expected: " -d '/wowaweea'"
            got: ""
       
       (compared using eql?)
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:131

  4) Chef::Provider::User::Useradd when creating a user runs useradd with the computed command options
     Failure/Error: @provider.create_user
       #<Chef::Provider::User::Useradd:0x7f6308304158> received :run_command with unexpected arguments
         expected: ({:command=>"useradd -c 'Adam Jacob' -g '23' -p 'abracadabra' -s '/usr/bin/zsh' -u '1000' -d '/Users/mud' -m adam"})
              got: ({:command=>"useradd -c 'Adam Jacob' -g '23' -p 'abracadabra' -s '/usr/bin/zsh' -u '1000' -m -d '/Users/mud' adam"})
     # /home/btm/src/chef/chef/spec/../lib/chef/provider/user/useradd.rb:34:in `create_user'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:172

  5) Chef::Provider::User::Useradd when managing a user runs usermod with the computed command options
     Failure/Error: @provider.manage_user
       #<Chef::Provider::User::Useradd:0x7f63082f7d18> received :run_command with unexpected arguments
         expected: ({:command=>"usermod -g '23' -d '/Users/mud' adam"})
              got: ({:command=>"usermod -g '23'-m -d '/Users/mud' adam"})
     # /home/btm/src/chef/chef/spec/../lib/chef/provider/user/useradd.rb:42:in `manage_user'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:186

  6) Chef::Provider::User::Useradd when managing a user does not set the -r option to usermod
     Failure/Error: @provider.manage_user
       #<Chef::Provider::User::Useradd:0x7f63082eb360> received :run_command with unexpected arguments
         expected: ({:command=>"usermod -g '23' -d '/Users/mud' adam"})
              got: ({:command=>"usermod -g '23'-m -d '/Users/mud' adam"})
     # /home/btm/src/chef/chef/spec/../lib/chef/provider/user/useradd.rb:42:in `manage_user'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:192

  7) Chef::Provider::User::Useradd when checking if home needs updating should return false if home matches
     Failure/Error: @provider.updating_home?.should == home_check["expected_result"]
     NameError:
       Cannot find a resource for updating_home? on  version 
     # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node'
     # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349

  8) Chef::Provider::User::Useradd when checking if home needs updating should return true if home doesn't match
     Failure/Error: @provider.updating_home?.should == home_check["expected_result"]
     NameError:
       Cannot find a resource for updating_home? on  version 
     # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node'
     # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349

  9) Chef::Provider::User::Useradd when checking if home needs updating should return false if home only differs by trailing slash
     Failure/Error: @provider.updating_home?.should == home_check["expected_result"]
     NameError:
       Cannot find a resource for updating_home? on  version 
     # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node'
     # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349

  10) Chef::Provider::User::Useradd when checking if home needs updating should return false if home is an equivalent path
     Failure/Error: @provider.updating_home?.should == home_check["expected_result"]
     NameError:
       Cannot find a resource for updating_home? on  version 
     # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node'
     # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349

  11) Chef::Provider::User::Useradd when checking if home needs updating should return false if the current home is not known
     Failure/Error: @provider.updating_home?.should == true
     NameError:
       Cannot find a resource for updating_home? on  version 
     # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform'
     # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node'
     # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing'
     # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:360
Show
Bryan McLellan [Opscode] added a comment - I get a lot of failing unit tests from on this branch: Failures: 1) Chef::Provider::User::Useradd when setting option when we want to create a system user should set useradd -r Failure/Error: @provider.useradd_options.should == " -r" NameError: Cannot find a resource for useradd_options on version # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node' # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:106 2) Chef::Provider::User::Useradd when setting option when the resource has a different home directory and supports home directory management should set -d /homedir -m Failure/Error: @provider.universal_options.should == " -d '/wowaweea'" expected: " -d '/wowaweea'" got: "" (using ==) # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:118 3) Chef::Provider::User::Useradd when setting option when the resource has a different home directory and supports home directory management (using real attributes) should set -d /homedir -m Failure/Error: @provider.universal_options.should eql( " -d '/wowaweea'" ) expected: " -d '/wowaweea'" got: "" (compared using eql?) # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:131 4) Chef::Provider::User::Useradd when creating a user runs useradd with the computed command options Failure/Error: @provider.create_user #<Chef::Provider::User::Useradd:0x7f6308304158> received :run_command with unexpected arguments expected: ({:command=> "useradd -c 'Adam Jacob' -g '23' -p 'abracadabra' -s '/usr/bin/zsh' -u '1000' -d '/Users/mud' -m adam" }) got: ({:command=> "useradd -c 'Adam Jacob' -g '23' -p 'abracadabra' -s '/usr/bin/zsh' -u '1000' -m -d '/Users/mud' adam" }) # /home/btm/src/chef/chef/spec/../lib/chef/provider/user/useradd.rb:34:in `create_user' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:172 5) Chef::Provider::User::Useradd when managing a user runs usermod with the computed command options Failure/Error: @provider.manage_user #<Chef::Provider::User::Useradd:0x7f63082f7d18> received :run_command with unexpected arguments expected: ({:command=> "usermod -g '23' -d '/Users/mud' adam" }) got: ({:command=> "usermod -g '23'-m -d '/Users/mud' adam" }) # /home/btm/src/chef/chef/spec/../lib/chef/provider/user/useradd.rb:42:in `manage_user' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:186 6) Chef::Provider::User::Useradd when managing a user does not set the -r option to usermod Failure/Error: @provider.manage_user #<Chef::Provider::User::Useradd:0x7f63082eb360> received :run_command with unexpected arguments expected: ({:command=> "usermod -g '23' -d '/Users/mud' adam" }) got: ({:command=> "usermod -g '23'-m -d '/Users/mud' adam" }) # /home/btm/src/chef/chef/spec/../lib/chef/provider/user/useradd.rb:42:in `manage_user' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:192 7) Chef::Provider::User::Useradd when checking if home needs updating should return false if home matches Failure/Error: @provider.updating_home?.should == home_check[ "expected_result" ] NameError: Cannot find a resource for updating_home? on version # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node' # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349 8) Chef::Provider::User::Useradd when checking if home needs updating should return true if home doesn't match Failure/Error: @provider.updating_home?.should == home_check[ "expected_result" ] NameError: Cannot find a resource for updating_home? on version # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node' # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349 9) Chef::Provider::User::Useradd when checking if home needs updating should return false if home only differs by trailing slash Failure/Error: @provider.updating_home?.should == home_check[ "expected_result" ] NameError: Cannot find a resource for updating_home? on version # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node' # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349 10) Chef::Provider::User::Useradd when checking if home needs updating should return false if home is an equivalent path Failure/Error: @provider.updating_home?.should == home_check[ "expected_result" ] NameError: Cannot find a resource for updating_home? on version # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node' # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:349 11) Chef::Provider::User::Useradd when checking if home needs updating should return false if the current home is not known Failure/Error: @provider.updating_home?.should == true NameError: Cannot find a resource for updating_home? on version # /home/btm/src/chef/chef/spec/../lib/chef/resource_platform_map.rb:129:in `get' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:667:in `resource_for_platform' # /home/btm/src/chef/chef/spec/../lib/chef/resource.rb:684:in `resource_for_node' # /home/btm/src/chef/chef/spec/../lib/chef/mixin/recipe_definition_dsl_core.rb:58:in `method_missing' # /home/btm/src/chef/chef/spec/unit/provider/user/useradd_spec.rb:360
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

The cause of this regression is 5255a4611baa8cd605a600db28368de85e6aaebd CHEF-3076

Show
Bryan McLellan [Opscode] added a comment - The cause of this regression is 5255a4611baa8cd605a600db28368de85e6aaebd CHEF-3076
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

fixed in dcb83e6c635cba1c4c6939aa3df458d76a95cbf3

Show
Bryan McLellan [Opscode] added a comment - fixed in dcb83e6c635cba1c4c6939aa3df458d76a95cbf3
Hide
Permalink
Max Horbul added a comment -

Good catch.
I have added one more test which covers the user case described in this ticket.
https://github.com/mhorbul/chef/commit/57d078f308b2d375fdb542492c031f6224fe2d8d

Let me know if you want me to create a pull request so you could merge it in.

Show
Max Horbul added a comment - Good catch. I have added one more test which covers the user case described in this ticket. https://github.com/mhorbul/chef/commit/57d078f308b2d375fdb542492c031f6224fe2d8d Let me know if you want me to create a pull request so you could merge it in.
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Got it, thanks Max.

Show
Bryan McLellan [Opscode] added a comment - Got it, thanks Max.

People

  • Assignee:
    Unassigned
    Reporter:
    Mike Glenn
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: