Merge branch 'chef-dk-227' into 11-stable

2 files changed, 90 insertions, 1 deletions

diff --git a/lib/chef/knife/core/subcommand_loader.rb b/lib/chef/knife/core/subcommand_loader.rb
index 0489c726b3..a2a12b5121 100644
--- a/lib/chef/knife/core/subcommand_loader.rb
+++ b/lib/chef/knife/core/subcommand_loader.rb
@@ -21,6 +21,9 @@ class Chef
   class Knife
     class SubcommandLoader
 
+      MATCHES_CHEF_GEM = %r{/chef-[\d]+\.[\d]+\.[\d]+}.freeze
+      MATCHES_THIS_CHEF_GEM = %r{/chef-#{Chef::VERSION}/}.freeze
+
       attr_reader :chef_config_dir
       attr_reader :env
 
@@ -121,6 +124,14 @@ class Chef
         subcommand_files = {}
         files.each do |file|
           rel_path = file[/(#{Regexp.escape File.join('chef', 'knife', '')}.*)\.rb/, 1]
+
+          # When not installed as a gem (ChefDK/appbundler in particular), AND
+          # a different version of Chef is installed via gems, `files` will
+          # include some files from the 'other' Chef install. If this contains
+          # a knife command that doesn't exist in this version of Chef, we will
+          # get a LoadError later when we try to require it.
+          next if from_different_chef_version?(file)
+
           subcommand_files[rel_path] = file
         end
 
@@ -184,6 +195,19 @@ class Chef
 
         Dir[glob].map { |f| f.untaint }
       end
+
+      def from_different_chef_version?(path)
+        matches_any_chef_gem?(path) && !matches_this_chef_gem?(path)
+      end
+
+      def matches_any_chef_gem?(path)
+        path =~ MATCHES_CHEF_GEM
+      end
+
+      def matches_this_chef_gem?(path)
+        path =~ MATCHES_THIS_CHEF_GEM
+      end
+
     end
   end
 end
diff --git a/spec/unit/knife/core/subcommand_loader_spec.rb b/spec/unit/knife/core/subcommand_loader_spec.rb
index 033649bbc2..62a4a5cbb1 100644
--- a/spec/unit/knife/core/subcommand_loader_spec.rb
+++ b/spec/unit/knife/core/subcommand_loader_spec.rb
@@ -73,7 +73,72 @@ describe Chef::Knife::SubcommandLoader do
     @loader.site_subcommands.should include(expected_command)
   end
 
-  describe "finding 3rd party plugins" do 
+  # https://github.com/opscode/chef-dk/issues/227
+  #
+  # `knife` in ChefDK isn't from a gem install, it's directly run from a clone
+  # of the source, but there can be one or more versions of chef also installed
+  # as a gem. If the gem install contains a command that doesn't exist in the
+  # source tree of the "primary" chef install, it can be loaded and cause an
+  # error. We also want to ensure that we only load builtin commands from the
+  # "primary" chef install.
+  context "when a different version of chef is also installed as a gem" do
+
+    let(:all_found_commands) do
+      [
+        "/opt/chefdk/embedded/apps/chef/lib/chef/knife/bootstrap.rb",
+        "/opt/chefdk/embedded/apps/chef/lib/chef/knife/client_bulk_delete.rb",
+        "/opt/chefdk/embedded/apps/chef/lib/chef/knife/client_create.rb",
+        # We use the fake version 1.0.0 because that version doesn't exist,
+        # which ensures it won't ever equal "chef-#{Chef::VERSION}"
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-1.0.0/lib/chef/knife/bootstrap.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-1.0.0/lib/chef/knife/client_bulk_delete.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-1.0.0/lib/chef/knife/client_create.rb",
+        # This command is "extra" compared to what's in the embedded/apps/chef install:
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-1.0.0/lib/chef/knife/data_bag_secret_options.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-vault-2.2.4/lib/chef/knife/decrypt.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/knife-spork-1.4.1/lib/chef/knife/spork-bump.rb",
+        # These are fake commands that have names designed to test that the
+        # regex is strict enough
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-foo-#{Chef::VERSION}/lib/chef/knife/chef-foo.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/foo-chef-#{Chef::VERSION}/lib/chef/knife/foo-chef.rb",
+        # In a real scenario, we'd use rubygems APIs to only select the most
+        # recent gem, but for this test we want to check that we're doing the
+        # right thing both when the plugin version matches and does not match
+        # the current chef version. Looking at
+        # `SubcommandLoader::MATCHES_THIS_CHEF_GEM` and
+        # `SubcommandLoader::MATCHES_CHEF_GEM` should make it clear why we want
+        # to test these two cases.
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-foo-1.0.0/lib/chef/knife/chef-foo.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/foo-chef-1.0.0/lib/chef/knife/foo-chef.rb"
+      ]
+    end
+
+    let(:expected_valid_commands) do
+      [
+        "/opt/chefdk/embedded/apps/chef/lib/chef/knife/bootstrap.rb",
+        "/opt/chefdk/embedded/apps/chef/lib/chef/knife/client_bulk_delete.rb",
+        "/opt/chefdk/embedded/apps/chef/lib/chef/knife/client_create.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-vault-2.2.4/lib/chef/knife/decrypt.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/knife-spork-1.4.1/lib/chef/knife/spork-bump.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-foo-#{Chef::VERSION}/lib/chef/knife/chef-foo.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/foo-chef-#{Chef::VERSION}/lib/chef/knife/foo-chef.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/chef-foo-1.0.0/lib/chef/knife/chef-foo.rb",
+        "/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/foo-chef-1.0.0/lib/chef/knife/foo-chef.rb"
+      ]
+    end
+
+    before do
+      expect(@loader).to receive(:find_files_latest_gems).with("chef/knife/*.rb").and_return(all_found_commands)
+      expect(@loader).to receive(:find_subcommands_via_dirglob).and_return({})
+    end
+
+    it "ignores commands from the non-matching gem install" do
+      expect(@loader.find_subcommands_via_rubygems.values).to eq(expected_valid_commands)
+    end
+
+  end
+
+  describe "finding 3rd party plugins" do
     let(:env_home) { "/home/alice" }
     let(:manifest_path) { env_home + "/.chef/plugin_manifest.json" }