Merge pull request #2165 from coderanger/rfc017

RFC 17 implementation

6 files changed, 132 insertions, 36 deletions

diff --git a/lib/chef/cookbook_version.rb b/lib/chef/cookbook_version.rb
index 95af94bdf7..505b403e65 100644
--- a/lib/chef/cookbook_version.rb
+++ b/lib/chef/cookbook_version.rb
@@ -315,13 +315,20 @@ class Chef
       else
         if segment == :files || segment == :templates
           error_message = "Cookbook '#{name}' (#{version}) does not contain a file at any of these locations:\n"
-          error_locations = [
-            "  #{segment}/#{node[:platform]}-#{node[:platform_version]}/#{filename}",
-            "  #{segment}/#{node[:platform]}/#{filename}",
-            "  #{segment}/default/#{filename}",
-          ]
+          error_locations = if filename.is_a?(Array)
+            filename.map{|name| "  #{File.join(segment.to_s, name)}"}
+          else
+            [
+              "  #{segment}/#{node[:platform]}-#{node[:platform_version]}/#{filename}",
+              "  #{segment}/#{node[:platform]}/#{filename}",
+              "  #{segment}/default/#{filename}",
+              "  #{segment}/#{filename}",
+            ]
+          end
           error_message << error_locations.join("\n")
           existing_files = segment_filenames(segment)
+          # Strip the root_dir prefix off all files for readability
+          existing_files.map!{|path| path[root_dir.length+1..-1]} if root_dir
           # Show the files that the cookbook does have. If the user made a typo,
           # hopefully they'll see it here.
           unless existing_files.empty?
@@ -421,38 +428,44 @@ class Chef
     def preferences_for_path(node, segment, path)
       # only files and templates can be platform-specific
       if segment.to_sym == :files || segment.to_sym == :templates
-        begin
-          platform, version = Chef::Platform.find_platform_and_version(node)
-        rescue ArgumentError => e
-          # Skip platform/version if they were not found by find_platform_and_version
-          if e.message =~ /Cannot find a (?:platform|version)/
-            platform = "/unknown_platform/"
-            version = "/unknown_platform_version/"
-          else
-            raise
+        relative_search_path = if path.is_a?(Array)
+          path
+        else
+          begin
+            platform, version = Chef::Platform.find_platform_and_version(node)
+          rescue ArgumentError => e
+            # Skip platform/version if they were not found by find_platform_and_version
+            if e.message =~ /Cannot find a (?:platform|version)/
+              platform = "/unknown_platform/"
+              version = "/unknown_platform_version/"
+            else
+              raise
+            end
           end
-        end
 
-        fqdn = node[:fqdn]
+          fqdn = node[:fqdn]
 
-        # Break version into components, eg: "5.7.1" => [ "5.7.1", "5.7", "5" ]
-        search_versions = []
-        parts = version.to_s.split('.')
+          # Break version into components, eg: "5.7.1" => [ "5.7.1", "5.7", "5" ]
+          search_versions = []
+          parts = version.to_s.split('.')
 
-        parts.size.times do
-          search_versions << parts.join('.')
-          parts.pop
-        end
+          parts.size.times do
+            search_versions << parts.join('.')
+            parts.pop
+          end
 
-        # Most specific to least specific places to find the path
-        search_path = [ File.join(segment.to_s, "host-#{fqdn}", path) ]
-        search_versions.each do |v|
-          search_path << File.join(segment.to_s, "#{platform}-#{v}", path)
-        end
-        search_path << File.join(segment.to_s, platform.to_s, path)
-        search_path << File.join(segment.to_s, "default", path)
+          # Most specific to least specific places to find the path
+          search_path = [ File.join("host-#{fqdn}", path) ]
+          search_versions.each do |v|
+            search_path << File.join("#{platform}-#{v}", path)
+          end
+          search_path << File.join(platform.to_s, path)
+          search_path << File.join("default", path)
+          search_path << path
 
-        search_path
+          search_path
+        end
+        relative_search_path.map {|relative_path| File.join(segment.to_s, relative_path)}
       else
         [File.join(segment, path)]
       end
@@ -666,7 +679,13 @@ class Chef
         # Check if path is actually under root_path
         next if parts[0] == '..'
         if segment == :templates || segment == :files
-          return [ pathname.to_s, parts[1] ]
+          # Check if pathname looks like files/foo or templates/foo (unscoped)
+          if pathname.each_filename.to_a.length == 2
+            # Use root_default in case the same path exists at root_default and default
+            return [ pathname.to_s, 'root_default' ]
+          else
+            return [ pathname.to_s, parts[1] ]
+          end
         else
           return [ pathname.to_s, 'default' ]
         end
diff --git a/lib/chef/resource/cookbook_file.rb b/lib/chef/resource/cookbook_file.rb
index de758aef71..2709cf64f4 100644
--- a/lib/chef/resource/cookbook_file.rb
+++ b/lib/chef/resource/cookbook_file.rb
@@ -40,7 +40,7 @@ class Chef
       end
 
       def source(source_filename=nil)
-        set_or_return(:source, source_filename, :kind_of => String)
+        set_or_return(:source, source_filename, :kind_of => [ String, Array ])
       end
 
       def cookbook(cookbook_name=nil)
diff --git a/lib/chef/resource/template.rb b/lib/chef/resource/template.rb
index 9cba6f1c38..8473f5b677 100644
--- a/lib/chef/resource/template.rb
+++ b/lib/chef/resource/template.rb
@@ -50,7 +50,7 @@ class Chef
         set_or_return(
           :source,
           file,
-          :kind_of => [ String ]
+          :kind_of => [ String, Array ]
         )
       end
 
diff --git a/spec/data/cb_version_cookbooks/cookbook2/files/test.txt b/spec/data/cb_version_cookbooks/cookbook2/files/test.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/spec/data/cb_version_cookbooks/cookbook2/files/test.txt
diff --git a/spec/data/cb_version_cookbooks/cookbook2/templates/test.erb b/spec/data/cb_version_cookbooks/cookbook2/templates/test.erb
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/spec/data/cb_version_cookbooks/cookbook2/templates/test.erb
diff --git a/spec/unit/cookbook_version_spec.rb b/spec/unit/cookbook_version_spec.rb
index 25bc936569..8436e5c480 100644
--- a/spec/unit/cookbook_version_spec.rb
+++ b/spec/unit/cookbook_version_spec.rb
@@ -115,14 +115,13 @@ describe Chef::CookbookVersion do
       @cookbook[:provider_filenames]    = Dir[File.join(@cookbook_root, 'providers', '**', '*.rb')]
       @cookbook[:root_filenames]        = Array(File.join(@cookbook_root, 'README.rdoc'))
       @cookbook[:metadata_filenames]    = Array(File.join(@cookbook_root, 'metadata.json'))
-
     end
 
     describe "and a cookbook with the same name" do
       before do
         # Currently the cookbook loader finds all the files then tells CookbookVersion
         # where they are.
-        @cookbook_version = Chef::CookbookVersion.new("tatft", @cookbook_root)
+        @cookbook_version = Chef::CookbookVersion.new('tatft', @cookbook_root)
 
         @cookbook_version.attribute_filenames  = @cookbook[:attribute_filenames]
         @cookbook_version.definition_filenames = @cookbook[:definition_filenames]
@@ -350,6 +349,84 @@ describe Chef::CookbookVersion do
         readme["specificity"].should == "default"
       end
     end
+  end
+
+  describe 'with a cookbook directory named cookbook2 that has unscoped files' do
+    before do
+      @cookbook = Hash.new { |hash, key| hash[key] = [] }
+
+      @cookbook_root = File.join(CHEF_SPEC_DATA, 'cb_version_cookbooks', 'cookbook2')
+
+      # Dunno if the paths here are representitive of what is set by CookbookLoader...
+      @cookbook[:attribute_filenames]   = Dir[File.join(@cookbook_root, 'attributes', '**', '*.rb')]
+      @cookbook[:definition_filenames]  = Dir[File.join(@cookbook_root, 'definitions', '**', '*.rb')]
+      @cookbook[:file_filenames]        = Dir[File.join(@cookbook_root, 'files', '**', '*.*')]
+      @cookbook[:recipe_filenames]      = Dir[File.join(@cookbook_root, 'recipes', '**', '*.rb')]
+      @cookbook[:template_filenames]    = Dir[File.join(@cookbook_root, 'templates', '**', '*.*')]
+      @cookbook[:library_filenames]     = Dir[File.join(@cookbook_root, 'libraries', '**', '*.rb')]
+      @cookbook[:resource_filenames]    = Dir[File.join(@cookbook_root, 'resources', '**', '*.rb')]
+      @cookbook[:provider_filenames]    = Dir[File.join(@cookbook_root, 'providers', '**', '*.rb')]
+      @cookbook[:root_filenames]        = Array(File.join(@cookbook_root, 'README.rdoc'))
+      @cookbook[:metadata_filenames]    = Array(File.join(@cookbook_root, 'metadata.json'))
+
+      @cookbook_version = Chef::CookbookVersion.new('cookbook2', @cookbook_root)
+      @cookbook_version.attribute_filenames  = @cookbook[:attribute_filenames]
+      @cookbook_version.definition_filenames = @cookbook[:definition_filenames]
+      @cookbook_version.recipe_filenames     = @cookbook[:recipe_filenames]
+      @cookbook_version.template_filenames   = @cookbook[:template_filenames]
+      @cookbook_version.file_filenames       = @cookbook[:file_filenames]
+      @cookbook_version.library_filenames    = @cookbook[:library_filenames]
+      @cookbook_version.resource_filenames   = @cookbook[:resource_filenames]
+      @cookbook_version.provider_filenames   = @cookbook[:provider_filenames]
+      @cookbook_version.root_filenames       = @cookbook[:root_filenames]
+      @cookbook_version.metadata_filenames   = @cookbook[:metadata_filenames]
+
+      # Used to test file-specificity related file lookups
+      @node = Chef::Node.new
+      @node.set[:platform] = "ubuntu"
+      @node.set[:platform_version] = "13.04"
+      @node.name("testing")
+    end
+
+    it "should see a template" do
+      @cookbook_version.should have_template_for_node(@node, "test.erb")
+    end
+
+    it "should see a template using an array lookup" do
+      @cookbook_version.should have_template_for_node(@node, ["test.erb"])
+    end
+
+    it "should see a template using an array lookup with non-existant elements" do
+      @cookbook_version.should have_template_for_node(@node, ["missing.txt", "test.erb"])
+    end
+
+    it "should see a file" do
+      @cookbook_version.should have_cookbook_file_for_node(@node, "test.txt")
+    end
+
+    it "should see a file using an array lookup" do
+      @cookbook_version.should have_cookbook_file_for_node(@node, ["test.txt"])
+    end
+
+    it "should see a file using an array lookup with non-existant elements" do
+      @cookbook_version.should have_cookbook_file_for_node(@node, ["missing.txt", "test.txt"])
+    end
+
+    it "should not see a non-existant template" do
+      @cookbook_version.should_not have_template_for_node(@node, "missing.erb")
+    end
+
+    it "should not see a non-existant template using an array lookup" do
+      @cookbook_version.should_not have_template_for_node(@node, ["missing.erb"])
+    end
+
+    it "should not see a non-existant file" do
+      @cookbook_version.should_not have_cookbook_file_for_node(@node, "missing.txt")
+    end
+
+    it "should not see a non-existant file using an array lookup" do
+      @cookbook_version.should_not have_cookbook_file_for_node(@node, ["missing.txt"])
+    end
 
   end