Pathname.new blows up when @current_user does not exist and does not have #home (bug was introduced by CHEF-3033)

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 0.10.10
  • Fix Version/s: 0.10.10
  • Component/s: None
  • Labels:
    None

Description

This line of code https://github.com/opscode/chef/commit/530d8305dddfa2b4d39046fd88bd27ccacc4ec5d#L0R132 introduces exception when user has not been created yet.
@current_resource.home is nil in this casse

mhorbul@testbox:~$ irb -rpathname
irb(main):001:0> Pathname.new nil
TypeError: can't dup NilClass

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Max Horbul added a comment -

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

Show
Max Horbul added a comment - https://github.com/opscode/chef/pull/283
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Don't forget to hit resolve when you attach a patch Max.

Show
Bryan McLellan [Opscode] added a comment - Don't forget to hit resolve when you attach a patch Max.
Hide
Permalink
Max Horbul added a comment -

Oops, sorry

Show
Max Horbul added a comment - Oops, sorry
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

We should have a failing test here, one that would have failed before this code change. If you can't provide one quickly I can try.

Show
Bryan McLellan [Opscode] added a comment - We should have a failing test here, one that would have failed before this code change. If you can't provide one quickly I can try.
Hide
Permalink
Max Horbul added a comment -

Which test do you mean ? This output or a unit test ?

[Tue, 01 May 2012 15:21:53 +0000] INFO: Processing group[mhorbul] action create (users::default line 18)
[Tue, 01 May 2012 15:21:53 +0000] INFO: Processing user[mhorbul] action create (users::default line 22)
[Tue, 01 May 2012 15:21:53 +0000] ERROR: user[mhorbul] (users::default line 22) has had an error
[Tue, 01 May 2012 15:21:53 +0000] ERROR: user[mhorbul] (/var/cache/chef/cookbooks/users/definitions/system_user.rb:22:in `from_file') had an error:
user[mhorbul] (users::default line 22) had an error: TypeError: can't dup NilClass
/usr/lib/ruby/1.8/pathname.rb:212:in `dup'
/usr/lib/ruby/1.8/pathname.rb:212:in `initialize'
/usr/lib/ruby/gems/1.8/gems/chef-0.10.9.1/bin/../lib/chef/provider/user/useradd.rb:132:in `new'
/usr/lib/ruby/gems/1.8/gems/chef-0.10.9.1/bin/../lib/chef/provider/user/useradd.rb:132:in `updating_home?'
/usr/lib/ruby/gems/1.8/gems/chef-0.10.9.1/bin/../lib/chef/provider/user/useradd.rb:107:in `universal_options'

Show
Max Horbul added a comment - Which test do you mean ? This output or a unit test ? [Tue, 01 May 2012 15:21:53 +0000] INFO: Processing group [mhorbul] action create (users::default line 18) [Tue, 01 May 2012 15:21:53 +0000] INFO: Processing user [mhorbul] action create (users::default line 22) [Tue, 01 May 2012 15:21:53 +0000] ERROR: user [mhorbul] (users::default line 22) has had an error [Tue, 01 May 2012 15:21:53 +0000] ERROR: user [mhorbul] (/var/cache/chef/cookbooks/users/definitions/system_user.rb:22:in `from_file') had an error: user [mhorbul] (users::default line 22) had an error: TypeError: can't dup NilClass /usr/lib/ruby/1.8/pathname.rb:212:in `dup' /usr/lib/ruby/1.8/pathname.rb:212:in `initialize' /usr/lib/ruby/gems/1.8/gems/chef-0.10.9.1/bin/../lib/chef/provider/user/useradd.rb:132:in `new' /usr/lib/ruby/gems/1.8/gems/chef-0.10.9.1/bin/../lib/chef/provider/user/useradd.rb:132:in `updating_home?' /usr/lib/ruby/gems/1.8/gems/chef-0.10.9.1/bin/../lib/chef/provider/user/useradd.rb:107:in `universal_options'
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

A regression test is one that fails before your fix and succeeds after your fix, to ensure that we don't cause this regression in the future. I've added one in 7336f52b6a3bbed661f5490ee77efed3b9a65ea5 if you want to take a look.

Merged to master, thanks Max.

Show
Bryan McLellan [Opscode] added a comment - A regression test is one that fails before your fix and succeeds after your fix, to ensure that we don't cause this regression in the future. I've added one in 7336f52b6a3bbed661f5490ee77efed3b9a65ea5 if you want to take a look. Merged to master, thanks Max.
Hide
Permalink
Max Horbul added a comment -

I fixed the existing test which failed before the change.

These three lines of code http://goo.gl/JI3YC fix the real state of the "new user" test case.

Without my change test will fail: http://goo.gl/S5CUF

I did not want to test internals to avoid relying on the implementation.

Thank you,
Max

Show
Max Horbul added a comment - I fixed the existing test which failed before the change. These three lines of code http://goo.gl/JI3YC fix the real state of the "new user" test case. Without my change test will fail: http://goo.gl/S5CUF I did not want to test internals to avoid relying on the implementation. Thank you, Max
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Possible regression: CHEF-3082

Show
Bryan McLellan [Opscode] added a comment - Possible regression: CHEF-3082

People

  • Assignee:
    Max Horbul
    Reporter:
    Max Horbul
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: