Merge pull request #8925 from MsysTechnologiesllc/VSingh/MSYS-1109_chefignore_for_respective_cookbook_dir

Fix multiple chefignore file issues

18 files changed, 94 insertions, 40 deletions

diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbook_artifacts_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbook_artifacts_dir.rb
index 273b45a646..c99c689a71 100644
--- a/lib/chef/chef_fs/file_system/chef_server/cookbook_artifacts_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server/cookbook_artifacts_dir.rb
@@ -65,7 +65,7 @@ class Chef
               file_class.symlink other.file_path, proxy_cookbook_path
 
               # Instantiate a proxy loader using the temporary symlink
-              proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path, other.parent.chefignore)
+              proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path, other.chefignore)
               proxy_loader.load_cookbooks
 
               cookbook_to_upload = proxy_loader.cookbook_version
diff --git a/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb
index 0fd1e4acdf..739a42b124 100644
--- a/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb
@@ -71,7 +71,7 @@ class Chef
               file_class.symlink other.file_path, proxy_cookbook_path
 
               # Instantiate a proxy loader using the temporary symlink
-              proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path, other.parent.chefignore)
+              proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path, other.chefignore)
               proxy_loader.load_cookbooks
 
               cookbook_to_upload = proxy_loader.cookbook_version
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_artifact_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_artifact_dir.rb
index 8a6e375fde..a15b47ca73 100644
--- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_artifact_dir.rb
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_artifact_dir.rb
@@ -25,7 +25,7 @@ class Chef
         class ChefRepositoryFileSystemCookbookArtifactDir < ChefRepositoryFileSystemCookbookDir
           # Override from parent
           def cookbook_version
-            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore)
+            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, chefignore)
             cookbook_name, _dash, identifier = name.rpartition("-")
             # KLUDGE: We shouldn't have to use instance_variable_set
             loader.instance_variable_set(:@cookbook_name, cookbook_name)
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb
index 74170cc7a9..4f8368b025 100644
--- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb
@@ -21,6 +21,7 @@ require_relative "../chef_server/cookbook_dir"
 require_relative "../chef_server/versioned_cookbook_dir"
 require_relative "../exceptions"
 require_relative "../../../cookbook/cookbook_version_loader"
+require_relative "../../../cookbook/chefignore"
 
 class Chef
   module ChefFS
@@ -31,6 +32,11 @@ class Chef
         class ChefRepositoryFileSystemCookbookDir < ChefRepositoryFileSystemCookbookEntry
 
           # API Required by Respository::Directory
+          def chefignore
+            @chefignore ||= Chef::Cookbook::Chefignore.new(file_path)
+          rescue Errno::EISDIR, Errno::EACCES
+            # Work around a bug in Chefignore when chefignore is a directory
+          end
 
           def fs_entry_valid?
             return false unless File.directory?(file_path) && name_valid?
@@ -136,7 +142,7 @@ class Chef
           end
 
           def cookbook_version
-            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore)
+            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, chefignore)
             loader.load_cookbooks
             loader.cookbook_version
           end
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_entry.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_entry.rb
index 1708428372..6ae5045284 100644
--- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_entry.rb
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_entry.rb
@@ -68,13 +68,14 @@ class Chef
             end
 
             # Check chefignore
-            ignorer = parent
+            ignorer = self
+
             loop do
-              if ignorer.is_a?(CookbooksDir)
+              if ignorer.is_a?(ChefRepositoryFileSystemCookbookDir)
                 # Grab the path from entry to child
                 path_to_child = name
                 child = self
-                while child.parent != ignorer
+                while child != ignorer
                   path_to_child = PathUtils.join(child.name, path_to_child)
                   child = child.parent
                 end
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb
index 442fa879ad..4d76be579a 100644
--- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb
@@ -25,7 +25,7 @@ class Chef
         class ChefRepositoryFileSystemVersionedCookbookDir < ChefRepositoryFileSystemCookbookDir
           # Override from parent
           def cookbook_version
-            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore)
+            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, chefignore)
             # We need the canonical cookbook name if we are using versioned cookbooks, but we don't
             # want to spend a lot of time adding code to the main Chef libraries
             canonical_name = canonical_cookbook_name(File.basename(file_path))
diff --git a/lib/chef/cookbook/chefignore.rb b/lib/chef/cookbook/chefignore.rb
index 4fed557000..763f5325cc 100644
--- a/lib/chef/cookbook/chefignore.rb
+++ b/lib/chef/cookbook/chefignore.rb
@@ -24,11 +24,9 @@ class Chef
       attr_reader :ignores
 
       def initialize(ignore_file_or_repo)
-        # Check the 'ignore_file_or_repo' path first and then look in the parent directory
+        # Check the 'ignore_file_or_repo' path first and then look in the parent directories till root
         # to handle both the chef repo cookbook layout and a standalone cookbook
         @ignore_file = find_ignore_file(ignore_file_or_repo)
-        @ignore_file = find_ignore_file(File.dirname(ignore_file_or_repo)) unless readable_file_or_symlink?(@ignore_file)
-
         @ignores = parse_ignore_file
       end
 
@@ -50,27 +48,34 @@ class Chef
 
       def parse_ignore_file
         ignore_globs = []
-        if readable_file_or_symlink?(@ignore_file)
+        if @ignore_file && readable_file_or_symlink?(@ignore_file)
           File.foreach(@ignore_file) do |line|
             ignore_globs << line.strip unless line =~ COMMENTS_AND_WHITESPACE
           end
         else
-          Chef::Log.trace("No chefignore file found at #{@ignore_file} no files will be ignored")
+          Chef::Log.debug("No chefignore file found. No files will be ignored!")
         end
         ignore_globs
       end
 
+      # Lookup of chefignore file till the root dir of the provided path.
+      # If file refer then lookup the parent dir till the root.
+      # eg. path: /var/.chef/cookbook_name
+      # Lookup at '/var/.chef/cookbook_name/chefignore', '/var/.chef/chefignore' '/var/chefignore' and '/chefignore' until exist
       def find_ignore_file(path)
-        if File.basename(path) =~ /chefignore/
-          path
-        else
-          File.join(path, "chefignore")
+        Pathname.new(path).ascend do |dir|
+          next unless dir.directory?
+
+          file = dir.join("chefignore")
+          return file.expand_path.to_s if file.exist?
         end
+
+        nil
       end
 
       def readable_file_or_symlink?(path)
-        File.exist?(@ignore_file) && File.readable?(@ignore_file) &&
-          (File.file?(@ignore_file) || File.symlink?(@ignore_file))
+        File.exist?(path) && File.readable?(path) &&
+          (File.file?(path) || File.symlink?(path))
       end
     end
   end
diff --git a/lib/chef/cookbook/cookbook_version_loader.rb b/lib/chef/cookbook/cookbook_version_loader.rb
index 767196ea0e..f9ec765e47 100644
--- a/lib/chef/cookbook/cookbook_version_loader.rb
+++ b/lib/chef/cookbook/cookbook_version_loader.rb
@@ -189,7 +189,7 @@ class Chef
       end
 
       def chefignore
-        @chefignore ||= Chefignore.new(File.basename(cookbook_path))
+        @chefignore ||= Chefignore.new(cookbook_path)
       end
 
       # Enumerate all the files in a cookbook and assign the resulting list to
diff --git a/lib/chef/cookbook/syntax_check.rb b/lib/chef/cookbook/syntax_check.rb
index 6c1f710f6d..073323da05 100644
--- a/lib/chef/cookbook/syntax_check.rb
+++ b/lib/chef/cookbook/syntax_check.rb
@@ -84,11 +84,13 @@ class Chef
       # If no +cookbook_path+ is given, +Chef::Config.cookbook_path+ is used.
       def self.for_cookbook(cookbook_name, cookbook_path = nil)
         cookbook_path ||= Chef::Config.cookbook_path
-        unless cookbook_path
-          raise ArgumentError, "Cannot find cookbook #{cookbook_name} unless Chef::Config.cookbook_path is set or an explicit cookbook path is given"
+
+        Array(cookbook_path).each do |entry|
+          path = File.expand_path(File.join(entry, cookbook_name.to_s))
+          return new(path) if Dir.exist?(path)
         end
 
-        new(File.join(cookbook_path, cookbook_name.to_s))
+        raise ArgumentError, "Cannot find cookbook #{cookbook_name} unless Chef::Config.cookbook_path is set or an explicit cookbook path is given"
       end
 
       # Create a new SyntaxCheck object
diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb
index b27d751d64..2bafed410d 100644
--- a/lib/chef/cookbook_loader.rb
+++ b/lib/chef/cookbook_loader.rb
@@ -49,8 +49,8 @@ class Chef
 
     # @param repo_paths [Array<String>] the array of repo paths containing cookbook dirs
     def initialize(*repo_paths)
-      @repo_paths = repo_paths.flatten.map { |p| File.expand_path(p) }
-      raise ArgumentError, "You must specify at least one cookbook repo path" if repo_paths.empty?
+      @repo_paths = repo_paths.flatten.compact.map { |p| File.expand_path(p) }
+      raise ArgumentError, "You must specify at least one cookbook repo path" if @repo_paths.empty?
     end
 
     # The primary function of this class is to build this Mash mapping cookbook names as a string to
@@ -171,7 +171,7 @@ class Chef
         begin
           mash = Mash.new
           all_directories_in_repo_paths.each do |cookbook_path|
-            loader = Cookbook::CookbookVersionLoader.new(cookbook_path, chefignore(File.dirname(cookbook_path)))
+            loader = Cookbook::CookbookVersionLoader.new(cookbook_path, chefignore(cookbook_path))
             cookbook_name = loader.cookbook_name
             if mash.key?(cookbook_name)
               raise Chef::Exceptions::CookbookMergingError, "Cookbook merging is no longer supported, the cookbook named #{cookbook_name} can only appear once in the cookbook_path"
diff --git a/lib/chef/cookbook_uploader.rb b/lib/chef/cookbook_uploader.rb
index 31fec8cfd1..ffc4040194 100644
--- a/lib/chef/cookbook_uploader.rb
+++ b/lib/chef/cookbook_uploader.rb
@@ -139,7 +139,7 @@ class Chef
 
     def validate_cookbooks
       cookbooks.each do |cb|
-        syntax_checker = Chef::Cookbook::SyntaxCheck.for_cookbook(cb.name)
+        syntax_checker = Chef::Cookbook::SyntaxCheck.new(cb.root_dir)
         Chef::Log.info("Validating ruby files")
         exit(1) unless syntax_checker.validate_ruby_files
         Chef::Log.info("Validating templates")
diff --git a/spec/data/cookbooks/starter/chefignore b/spec/data/cookbooks/starter/chefignore
new file mode 100644
index 0000000000..b9d6c768c2
--- /dev/null
+++ b/spec/data/cookbooks/starter/chefignore
@@ -0,0 +1,8 @@
+#
+# The ignore file allows you to skip files in cookbooks with the same name that appear
+# later in the search path.
+#
+
+recipes/default.rb
+  # comments can be indented
+ignored
diff --git a/spec/data/cookbooks/starter/files/sample.txt b/spec/data/cookbooks/starter/files/sample.txt
new file mode 100644
index 0000000000..e635a0f018
--- /dev/null
+++ b/spec/data/cookbooks/starter/files/sample.txt
@@ -0,0 +1 @@
+This is a Chef cookbook file. It is used to copy content verbatim on to a server.
\ No newline at end of file
diff --git a/spec/data/cookbooks/starter/metadata.rb b/spec/data/cookbooks/starter/metadata.rb
new file mode 100644
index 0000000000..fbd288e9c4
--- /dev/null
+++ b/spec/data/cookbooks/starter/metadata.rb
@@ -0,0 +1,2 @@
+name "starter"
+version "1.0.0"
diff --git a/spec/data/cookbooks/starter/recipes/default.rb b/spec/data/cookbooks/starter/recipes/default.rb
new file mode 100644
index 0000000000..c48b9a4fca
--- /dev/null
+++ b/spec/data/cookbooks/starter/recipes/default.rb
@@ -0,0 +1,4 @@
+# This is a Chef recipe file. It can be used to specify resources which will
+# apply configuration to a server.
+
+# For more information, see the documentation: https://docs.chef.io/essentials_cookbook_recipes.html
diff --git a/spec/unit/cookbook/chefignore_spec.rb b/spec/unit/cookbook/chefignore_spec.rb
index 95b9295f50..606e8d8007 100644
--- a/spec/unit/cookbook/chefignore_spec.rb
+++ b/spec/unit/cookbook/chefignore_spec.rb
@@ -18,32 +18,52 @@
 require "spec_helper"
 
 describe Chef::Cookbook::Chefignore do
-  before do
-    @chefignore = Chef::Cookbook::Chefignore.new(File.join(CHEF_SPEC_DATA, "cookbooks"))
-  end
+  let(:chefignore) { described_class.new(File.join(CHEF_SPEC_DATA, "cookbooks")) }
 
   it "loads the globs in the chefignore file" do
-    expect(@chefignore.ignores).to match_array(%w{recipes/ignoreme.rb ignored})
+    expect(chefignore.ignores).to match_array(%w{recipes/ignoreme.rb ignored})
   end
 
   it "removes items from an array that match the ignores" do
     file_list = %w{ recipes/ignoreme.rb recipes/dontignoreme.rb }
-    expect(@chefignore.remove_ignores_from(file_list)).to eq(%w{recipes/dontignoreme.rb})
+    expect(chefignore.remove_ignores_from(file_list)).to eq(%w{recipes/dontignoreme.rb})
   end
 
   it "determines if a file is ignored" do
-    expect(@chefignore.ignored?("ignored")).to be_truthy
-    expect(@chefignore.ignored?("recipes/ignoreme.rb")).to be_truthy
-    expect(@chefignore.ignored?("recipes/dontignoreme.rb")).to be_falsey
+    expect(chefignore.ignored?("ignored")).to be_truthy
+    expect(chefignore.ignored?("recipes/ignoreme.rb")).to be_truthy
+    expect(chefignore.ignored?("recipes/dontignoreme.rb")).to be_falsey
   end
 
   context "when using the single cookbook pattern" do
-    before do
-      @chefignore = Chef::Cookbook::Chefignore.new(File.join(CHEF_SPEC_DATA, "standalone_cookbook"))
+    let(:chefignore) { described_class.new(File.join(CHEF_SPEC_DATA, "cookbooks/starter")) }
+
+    it "loads the globs in the chefignore file" do
+      expect(chefignore.ignores).to match_array(%w{recipes/default.rb ignored})
+    end
+  end
+
+  context "when cookbook has it's own chefignore" do
+    let(:chefignore) { described_class.new(File.join(CHEF_SPEC_DATA, "cookbooks/starter")) }
+
+    it "loads the globs in the chefignore file" do
+      expect(chefignore.ignores).to match_array(%w{recipes/default.rb ignored})
     end
+  end
+
+  context "when cookbook don't have own chefignore" do
+    let(:chefignore) { described_class.new(File.join(CHEF_SPEC_DATA, "cookbooks/apache2")) }
+
+    it "loads the globs in the chefignore file of cookbooks dir" do
+      expect(chefignore.ignores).to match_array(%w{recipes/ignoreme.rb ignored})
+    end
+  end
+
+  context "when using the single cookbook pattern" do
+    let(:chefignore) { described_class.new(File.join(CHEF_SPEC_DATA, "standalone_cookbook")) }
 
     it "loads the globs in the chefignore file" do
-      expect(@chefignore.ignores).to match_array(%w{recipes/ignoreme.rb ignored vendor/bundle/*})
+      expect(chefignore.ignores).to match_array(%w{recipes/ignoreme.rb ignored vendor/bundle/*})
     end
   end
 end
diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb
index c747f14dd0..a09bbd043f 100644
--- a/spec/unit/cookbook_loader_spec.rb
+++ b/spec/unit/cookbook_loader_spec.rb
@@ -101,7 +101,7 @@ describe Chef::CookbookLoader do
         cookbook_loader.each_key do |cookbook_name|
           seen << cookbook_name
         end
-        expect(seen).to eq %w{angrybash apache2 borken ignorken irssi java name-mismatch openldap preseed supports-platform-constraints wget}
+        expect(seen).to eq %w{angrybash apache2 borken ignorken irssi java name-mismatch openldap preseed starter supports-platform-constraints wget}
       end
     end
 
diff --git a/spec/unit/cookbook_uploader_spec.rb b/spec/unit/cookbook_uploader_spec.rb
index 2adecfaa82..07ff303274 100644
--- a/spec/unit/cookbook_uploader_spec.rb
+++ b/spec/unit/cookbook_uploader_spec.rb
@@ -21,9 +21,10 @@ require "spec_helper"
 describe Chef::CookbookUploader do
 
   let(:http_client) { double("Chef::ServerAPI") }
+  let(:cookbook_path) { File.join(CHEF_SPEC_DATA, "cookbooks") }
 
   let(:cookbook_loader) do
-    loader = Chef::CookbookLoader.new(File.join(CHEF_SPEC_DATA, "cookbooks"))
+    loader = Chef::CookbookLoader.new(cookbook_path)
     loader.load_cookbooks
     loader.cookbooks_by_name["apache2"].identifier = apache2_identifier
     loader.cookbooks_by_name["java"].identifier = java_identifier
@@ -55,6 +56,10 @@ describe Chef::CookbookUploader do
 
   let(:uploader) { described_class.new(cookbooks_to_upload, rest: http_client, policy_mode: policy_mode) }
 
+  before do
+    allow(Chef::Config).to receive(:cookbook_path) { cookbook_path }
+  end
+
   it "defaults to not enabling policy mode" do
     expect(described_class.new(cookbooks_to_upload, rest: http_client).policy_mode?).to be(false)
   end