There were a number of problems here (duplicated code, meaningless tests, etc.) but the core was that the last definition of target ignored the provided value if there was also a user specified.
Signed-off-by: Markus Roberts <[email protected]> --- lib/puppet/provider/ssh_authorized_key/parsed.rb | 36 +---------- spec/unit/provider/ssh_authorized_key/parsed.rb | 79 ++++++++-------------- 2 files changed, 29 insertions(+), 86 deletions(-) diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb index 6df7f8a..28a68b3 100644 --- a/lib/puppet/provider/ssh_authorized_key/parsed.rb +++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb @@ -36,14 +36,6 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed, :rts => /^\s+/, :match => /^(?:(.+) )?(\d+) (\d+) (\d+)(?: (.+))?$/ - def target - @resource.should(:target) - end - - def user - @resource.should(:user) - end - def dir_perm # Determine correct permission for created directory and file # we can afford more restrictive permissions when the user is known @@ -67,39 +59,13 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed, end def target - if user - File.expand_path("~%s/.ssh/authorized_keys" % user) - elsif target = @resource.should(:target) - target - end + @resource.should(:target) || File.expand_path("~%s/.ssh/authorized_keys" % user) end def user @resource.should(:user) end - def dir_perm - # Determine correct permission for created directory and file - # we can afford more restrictive permissions when the user is known - if target - if user - 0700 - else - 0755 - end - end - end - - def file_perm - if target - if user - 0600 - else - 0644 - end - end - end - def flush # As path expansion had to be moved in the provider, we cannot generate new file # resources and thus have to chown and chmod here. It smells hackish. diff --git a/spec/unit/provider/ssh_authorized_key/parsed.rb b/spec/unit/provider/ssh_authorized_key/parsed.rb index 0f32a6f..ade738b 100755 --- a/spec/unit/provider/ssh_authorized_key/parsed.rb +++ b/spec/unit/provider/ssh_authorized_key/parsed.rb @@ -95,85 +95,61 @@ describe provider_class do File.stubs(:chown) end - describe "and a user has been specified" do + describe "and both a user and a target have been specified" do before :each do - @resource.stubs(:should).with(:user).returns "nobody" - target = File.expand_path("~nobody/.ssh/authorized_keys") + Puppet::Util.stubs(:uid).with("random_bob").returns 12345 + @resource.stubs(:should).with(:user).returns "random_bob" + target = "/tmp/.ssh_dir/place_to_put_authorized_keys" @resource.stubs(:should).with(:target).returns target end it "should create the directory" do - Dir.expects(:mkdir).with(File.expand_path("~nobody/.ssh"), 0700) + File.stubs(:exist?).with("/tmp/.ssh_dir").returns false + Dir.expects(:mkdir).with("/tmp/.ssh_dir", 0700) @provider.flush end it "should chown the directory to the user" do - uid = Puppet::Util.uid("nobody") - File.expects(:chown).with(uid, nil, File.expand_path("~nobody/.ssh")) + uid = Puppet::Util.uid("random_bob") + File.expects(:chown).with(uid, nil, "/tmp/.ssh_dir") @provider.flush end it "should chown the key file to the user" do - uid = Puppet::Util.uid("nobody") - File.expects(:chown).with(uid, nil, File.expand_path("~nobody/.ssh/authorized_keys")) + uid = Puppet::Util.uid("random_bob") + File.expects(:chown).with(uid, nil, "/tmp/.ssh_dir/place_to_put_authorized_keys") @provider.flush end it "should chmod the key file to 0600" do - File.expects(:chmod).with(0600, File.expand_path("~nobody/.ssh/authorized_keys")) - @provider.flush - end - end - - describe "and a target has been specified" do - before :each do - @resource.stubs(:should).with(:user).returns nil - @resource.stubs(:should).with(:target).returns "/tmp/.ssh/authorized_keys" - end - - it "should make the directory" do - Dir.expects(:mkdir).with("/tmp/.ssh", 0755) - @provider.flush - end - - it "should chmod the key file to 0644" do - File.expects(:chmod).with(0644, "/tmp/.ssh/authorized_keys") + File.expects(:chmod).with(0600, "/tmp/.ssh_dir/place_to_put_authorized_keys") @provider.flush end end - end -end - -describe provider_class do - before :each do - @resource = stub("resource", :name => "foo") - @resource.stubs(:[]).returns "foo" - @provider = provider_class.new(@resource) - end - - describe "when flushing" do - before :each do - # Stub file and directory operations - Dir.stubs(:mkdir) - File.stubs(:chmod) - File.stubs(:chown) - end - - describe "and a user has been specified" do + describe "and a user has been specified with no target" do before :each do @resource.stubs(:should).with(:user).returns "nobody" @resource.stubs(:should).with(:target).returns nil + # + # I'd like to use random_bob here and something like + # + # File.stubs(:expand_path).with("~random_bob/.ssh").returns "/users/r/random_bob/.ssh" + # + # but mocha objects strenuously to stubbing File.expand_path + # so I'm left with using nobody. + @dir = File.expand_path("~nobody/.ssh") end it "should create the directory" do - Dir.expects(:mkdir).with(File.expand_path("~nobody/.ssh"), 0700) + File.stubs(:exist?).with(@dir).returns false + Dir.expects(:mkdir).with(@dir,0700) @provider.flush end it "should chown the directory to the user" do uid = Puppet::Util.uid("nobody") - File.expects(:chown).with(uid, nil, File.expand_path("~nobody/.ssh")) + File.expects(:chown).with(uid, nil, @dir) @provider.flush end @@ -189,19 +165,20 @@ describe provider_class do end end - describe "and a target has been specified" do + describe "and a target has been specified with no user" do before :each do @resource.stubs(:should).with(:user).returns nil - @resource.stubs(:should).with(:target).returns "/tmp/.ssh/authorized_keys" + @resource.stubs(:should).with(:target).returns("/tmp/.ssh_dir/place_to_put_authorized_keys") end it "should make the directory" do - Dir.expects(:mkdir).with("/tmp/.ssh", 0755) + File.stubs(:exist?).with("/tmp/.ssh_dir").returns false + Dir.expects(:mkdir).with("/tmp/.ssh_dir", 0755) @provider.flush end it "should chmod the key file to 0644" do - File.expects(:chmod).with(0644, "/tmp/.ssh/authorized_keys") + File.expects(:chmod).with(0644, "/tmp/.ssh_dir/place_to_put_authorized_keys") @provider.flush end end -- 1.6.4 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
