From d7c9c765dbf28df3631e709832c44c343569cb53 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Wed, 10 Aug 2011 16:20:00 -0700 Subject: (#8740) Do not enumerate files in the root directory. Previously the command 'puppet resource file' would enumerate all files in the root directory, and generate an exception if the file type was not a directory, file, or link. Worse, it would also do this when a file or directory was specified, e.g. 'puppet resource file /etc/hosts'. Ideally, the find method of the ral terminus should not need to call the type's instances class method, instead just creating an instance of the type with the specified name and parameters. However, some types, like package, depend on this behavior. The type walks all providers and all instances that they provide, checking to see if the provider provides an instance with that name, and also warning if another provider provides an instance with the same name. Also, ideally, puppet should not blow up when encountering an unsupported file type, e.g. Unix domain socket, but that would be too big of a change for 2.6.x. This commit changes 'puppet resource file' to return a message saying that the operation is not supported: Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc The change is bit of a hack, as ideally, the file type's instances method could raise an exception when called in a 'search' context, but return an empty array in a 'find' context. But that also would be too big of a change for 2.6.x. This commit also adds spec tests for the resource application and file type, as well as an acceptance test, which creates a Unix domain socket in the root directory, while running 'puppet resource file'. Paired-with: Nick Lewis Reviewed-by: Jacob Helwig --- ...ket_8740_should_not_enumerate_root_directory.rb | 32 ++++++++++++++++++++++ lib/puppet/application/resource.rb | 3 ++ lib/puppet/type/file.rb | 4 +-- spec/unit/application/resource_spec.rb | 25 +++++++++++++++++ spec/unit/type/file_spec.rb | 6 ++++ 5 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb diff --git a/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb b/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb new file mode 100644 index 000000000..5763a3c98 --- /dev/null +++ b/acceptance/tests/resource/file/ticket_8740_should_not_enumerate_root_directory.rb @@ -0,0 +1,32 @@ +test_name "#8740: should not enumerate root directory" + +target = "/test-socket-#{$$}" + +step "clean up the system before we begin" +on(agents, "rm -f #{target}") + +step "create UNIX domain socket" +on(agents, %Q{ruby -e "require 'socket'; UNIXServer::new('#{target}').close"}) + +step "query for all files, which should return nothing" +on(agents, puppet_resource('file'), :acceptable_exit_codes => [1]) do + assert_match(%r{Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc}, stderr) +end + +["/", "/etc"].each do |file| + step "query '#{file}' directory, which should return single entry" + on(agents, puppet_resource('file', file)) do + files = stdout.scan(/^file \{ '([^']+)'/).flatten + + assert_equal(1, files.size, "puppet returned multiple files: #{files.join(', ')}") + assert_match(file, files[0], "puppet did not return file") + end +end + +step "query file that does not exist, which should report the file is absent" +on(agents, puppet_resource('file', '/this/does/notexist')) do + assert_match(/ensure\s+=>\s+'absent'/, stdout) +end + +step "remove UNIX domain socket" +on(agents, "rm -f #{target}") diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb index f55caa58a..bc4faf5e4 100644 --- a/lib/puppet/application/resource.rb +++ b/lib/puppet/application/resource.rb @@ -81,6 +81,9 @@ class Puppet::Application::Resource < Puppet::Application [ Puppet::Resource.new( type, name, :parameters => params ).save( key ) ] end else + if type == "file" + raise "Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc" + end Puppet::Resource.search( key, {} ) end.map(&format).join("\n") diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 630ebe5de..e91929cf8 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -310,8 +310,8 @@ Puppet::Type.newtype(:file) do super(path.gsub(/\/+/, '/').sub(/\/$/, '')) end - def self.instances(base = '/') - return self.new(:name => base, :recurse => true, :recurselimit => 1, :audit => :all).recurse_local.values + def self.instances + return [] end @depthfirst = false diff --git a/spec/unit/application/resource_spec.rb b/spec/unit/application/resource_spec.rb index b6c52b11e..9059a1fc5 100755 --- a/spec/unit/application/resource_spec.rb +++ b/spec/unit/application/resource_spec.rb @@ -230,4 +230,29 @@ describe Puppet::Application::Resource do end end + + describe "when handling file type" do + before :each do + Facter.stubs(:loadfacts) + @resource.preinit + end + + it "should raise an exception if no file specified" do + @resource.command_line.stubs(:args).returns(['file']) + + lambda { @resource.main }.should raise_error(RuntimeError, /Listing all file instances is not supported/) + end + + it "should output a file resource when given a file path" do + res = Puppet::Type.type(:file).new(:path => "/etc").to_resource + Puppet::Resource.expects(:find).returns(res) + + @resource.command_line.stubs(:args).returns(['file', '/etc']) + @resource.expects(:puts).with do |args| + args.should =~ /file \{ '\/etc'/m + end + + @resource.main + end + end end diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb index 90f3daf09..81ccee821 100755 --- a/spec/unit/type/file_spec.rb +++ b/spec/unit/type/file_spec.rb @@ -1190,4 +1190,10 @@ describe Puppet::Type.type(:file) do @file[:checksum].should be :md5lite end end + + describe ".instances" do + it 'should return an empty array' do + Puppet::Type::File.instances.should == [] + end + end end -- cgit