summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTyler Ball <tyler-ball@users.noreply.github.com>2016-03-04 11:45:35 -0700
committerTyler Ball <tyler-ball@users.noreply.github.com>2016-03-04 11:45:35 -0700
commitee53ee1aba16ee2ad7c1fdde18869af971dcba67 (patch)
tree5c4e9ba97d90a855cde83b5ff9ea3b5d485cd7f8
parenta19b1d07d1fa416b8591681f1a04ef673aa25882 (diff)
parent68b188dbd933a5ec97e5540d22541c57480bdad3 (diff)
downloadchef-ee53ee1aba16ee2ad7c1fdde18869af971dcba67.tar.gz
Merge pull request #4651 from chef/tball/rehash_error
Always rehash from gem source and not existing hash file
-rw-r--r--lib/chef/knife.rb5
-rw-r--r--lib/chef/knife/core/hashed_command_loader.rb25
-rw-r--r--lib/chef/knife/core/subcommand_loader.rb6
-rw-r--r--lib/chef/knife/rehash.rb5
-rw-r--r--spec/functional/knife/rehash_spec.rb39
-rw-r--r--spec/unit/knife/core/hashed_command_loader_spec.rb16
-rw-r--r--spec/unit/knife/core/subcommand_loader_spec.rb6
-rw-r--r--spec/unit/knife_spec.rb7
8 files changed, 105 insertions, 4 deletions
diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb
index 657216bcf0..8f7232d927 100644
--- a/lib/chef/knife.rb
+++ b/lib/chef/knife.rb
@@ -145,6 +145,11 @@ class Chef
end
def self.subcommand_class_from(args)
+ if args.size == 1 and args[0].strip.downcase == "rehash"
+ # To prevent issues with the rehash file not pointing to the correct plugins,
+ # we always use the glob loader when regenerating the rehash file
+ @subcommand_loader = Chef::Knife::SubcommandLoader.gem_glob_loader(chef_config_dir)
+ end
subcommand_loader.command_class_from(args) || subcommand_not_found!(args)
end
diff --git a/lib/chef/knife/core/hashed_command_loader.rb b/lib/chef/knife/core/hashed_command_loader.rb
index 7b6c1c4c08..1165af6eb2 100644
--- a/lib/chef/knife/core/hashed_command_loader.rb
+++ b/lib/chef/knife/core/hashed_command_loader.rb
@@ -40,9 +40,29 @@ class Chef
def list_commands(pref_category = nil)
if pref_category || manifest[KEY]["plugins_by_category"].key?(pref_category)
- { pref_category => manifest[KEY]["plugins_by_category"][pref_category] }
+ commands = { pref_category => manifest[KEY]["plugins_by_category"][pref_category] }
else
- manifest[KEY]["plugins_by_category"]
+ commands = manifest[KEY]["plugins_by_category"]
+ end
+ # If any of the specified plugins in the manifest dont have a valid path we will
+ # eventually get an error and the user will need to rehash - instead, lets just
+ # print out 1 error here telling them to rehash
+ errors = {}
+ commands.collect { |k, v| v }.flatten.each do |command|
+ paths = manifest[KEY]["plugins_paths"][command]
+ if paths && paths.is_a?(Array)
+ # It is only an error if all the paths don't exist
+ if paths.all? { |sc| !File.exists?(sc) }
+ errors[command] = paths
+ end
+ end
+ end
+ if errors.empty?
+ commands
+ else
+ Chef::Log.error "There are files specified in the manifest that are missing. Please rehash to update the subcommands cache. If you see this error after rehashing delete the cache at #{Chef::Knife::SubcommandLoader.plugin_manifest_path}"
+ Chef::Log.error "Missing files:\n\t#{errors.values.flatten.join("\n\t")}"
+ {}
end
end
@@ -59,7 +79,6 @@ class Chef
if File.exists?(sc)
Kernel.load sc
else
- Chef::Log.error "The file #{sc} is missing for subcommand '#{subcommand_for_args(args)}'. Please rehash to update the subcommands cache."
return false
end
end
diff --git a/lib/chef/knife/core/subcommand_loader.rb b/lib/chef/knife/core/subcommand_loader.rb
index 95ab219c80..dc0b0cc39c 100644
--- a/lib/chef/knife/core/subcommand_loader.rb
+++ b/lib/chef/knife/core/subcommand_loader.rb
@@ -58,6 +58,12 @@ class Chef
end
end
+ # There are certain situations where we want to shortcut the loader selection
+ # in self.for_config and force using the GemGlobLoader
+ def self.gem_glob_loader(chef_config_dir)
+ Knife::SubcommandLoader::GemGlobLoader.new(chef_config_dir)
+ end
+
def self.plugin_manifest?
plugin_manifest_path && File.exist?(plugin_manifest_path)
end
diff --git a/lib/chef/knife/rehash.rb b/lib/chef/knife/rehash.rb
index 3e7bab7e0f..79286368f8 100644
--- a/lib/chef/knife/rehash.rb
+++ b/lib/chef/knife/rehash.rb
@@ -35,7 +35,10 @@ class Chef
end
def reload_plugins
- Chef::Knife::SubcommandLoader::GemGlobLoader.new(@@chef_config_dir).load_commands
+ # The subcommand_loader for this knife command should _always_ be the GemGlobLoader. The GemGlobLoader loads
+ # plugins from disc and ensures the hash we write is always correct. By this point it should also already have
+ # loaded plugins and `load_commands` shouldn't have an effect.
+ Chef::Knife.subcommand_loader.load_commands
end
def generate_hash
diff --git a/spec/functional/knife/rehash_spec.rb b/spec/functional/knife/rehash_spec.rb
new file mode 100644
index 0000000000..22ffa125fa
--- /dev/null
+++ b/spec/functional/knife/rehash_spec.rb
@@ -0,0 +1,39 @@
+#
+# Author:: Daniel DeLeo (<dan@chef.io>)
+# Copyright:: Copyright 2010-2016, Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "spec_helper"
+
+require "chef/knife/rehash"
+require "chef/knife/core/subcommand_loader"
+
+describe "knife rehash" do
+ before do
+ allow(Chef::Knife::SubcommandLoader).to receive(:load_commands)
+ end
+
+ after do
+ # We need to clean up the generated manifest or else is messes with later tests
+ FileUtils.rm_f(Chef::Knife::SubcommandLoader.plugin_manifest_path)
+ end
+
+ it "writes the loaded plugins to disc" do
+ knife_rehash = Chef::Knife::Rehash.new
+ knife_rehash.run
+ expect(File.read(Chef::Knife::SubcommandLoader.plugin_manifest_path)).to match(/node_list.rb/)
+ end
+end
diff --git a/spec/unit/knife/core/hashed_command_loader_spec.rb b/spec/unit/knife/core/hashed_command_loader_spec.rb
index 0135868bf5..081f9deae5 100644
--- a/spec/unit/knife/core/hashed_command_loader_spec.rb
+++ b/spec/unit/knife/core/hashed_command_loader_spec.rb
@@ -46,6 +46,10 @@ describe Chef::Knife::SubcommandLoader::HashedCommandLoader do
plugin_manifest)}
describe "#list_commands" do
+ before do
+ allow(File).to receive(:exists?).and_return(true)
+ end
+
it "lists all commands by category when no argument is given" do
expect(loader.list_commands).to eq({ "cool" => ["cool_a"], "cooler" => ["cooler_b"] })
end
@@ -53,6 +57,18 @@ describe Chef::Knife::SubcommandLoader::HashedCommandLoader do
it "lists only commands in the given category when a category is given" do
expect(loader.list_commands("cool")).to eq({ "cool" => ["cool_a"] })
end
+
+ context "when the plugin path is invalid" do
+ before do
+ expect(File).to receive(:exists?).with("/file/for/plugin/b").and_return(false)
+ end
+
+ it "lists all commands by category when no argument is given" do
+ expect(Chef::Log).to receive(:error).with(/There are files specified in the manifest that are missing/)
+ expect(Chef::Log).to receive(:error).with("Missing files:\n\t/file/for/plugin/b")
+ expect(loader.list_commands).to eq({})
+ end
+ end
end
describe "#subcommand_files" do
diff --git a/spec/unit/knife/core/subcommand_loader_spec.rb b/spec/unit/knife/core/subcommand_loader_spec.rb
index 6cda244132..b235102a0b 100644
--- a/spec/unit/knife/core/subcommand_loader_spec.rb
+++ b/spec/unit/knife/core/subcommand_loader_spec.rb
@@ -61,4 +61,10 @@ describe Chef::Knife::SubcommandLoader do
end
end
end
+
+ describe "#gem_glob_loader" do
+ it "always creates a GemGlobLoader" do
+ expect(Chef::Knife::SubcommandLoader.gem_glob_loader(config_dir)).to be_a Chef::Knife::SubcommandLoader::GemGlobLoader
+ end
+ end
end
diff --git a/spec/unit/knife_spec.rb b/spec/unit/knife_spec.rb
index 698ae205f1..ec1e59d863 100644
--- a/spec/unit/knife_spec.rb
+++ b/spec/unit/knife_spec.rb
@@ -23,6 +23,7 @@ end
require "spec_helper"
require "uri"
+require "chef/knife/core/gem_glob_loader"
describe Chef::Knife do
@@ -148,6 +149,12 @@ describe Chef::Knife do
expect(Chef::Knife.subcommand_class_from(%w{cookbook site vendor --help foo bar baz})).to eq(:CookbookSiteVendor)
end
+ it "special case sets the subcommand_loader to GemGlobLoader when running rehash" do
+ Chef::Knife.subcommands["rehash"] = :Rehash
+ expect(Chef::Knife.subcommand_class_from(%w{rehash })).to eq(:Rehash)
+ expect(Chef::Knife.subcommand_loader).to be_a(Chef::Knife::SubcommandLoader::GemGlobLoader)
+ end
+
end
describe "the headers include X-Remote-Request-Id" do