diff options
author | danielsdeleo <dan@opscode.com> | 2013-01-14 16:22:44 -0800 |
---|---|---|
committer | danielsdeleo <dan@opscode.com> | 2013-01-14 17:28:02 -0800 |
commit | 136ce0b0c9dff13e69e442f6c22b5edc71981c1c (patch) | |
tree | 82daf40b73658d1e5d627614ba4926ebe818a03e | |
parent | b09dd3272d2557c4722120f89b212bfe72ed7ec3 (diff) | |
download | chef-136ce0b0c9dff13e69e442f6c22b5edc71981c1c.tar.gz |
[CHEF-3497] apply config in the desired order
Takes advantage of new mixlib-cli option to keep default values from the
mixlib-cli DSL separate from user-supplied values. Config settings are
merged:
1. Defaults from mixlib-cli DSL
2. Settings from Chef::Config[:knife]
3. Values from CLI options
-rw-r--r-- | lib/chef/knife.rb | 81 | ||||
-rw-r--r-- | spec/unit/application/knife_spec.rb | 5 | ||||
-rw-r--r-- | spec/unit/knife/bootstrap_spec.rb | 2 | ||||
-rw-r--r-- | spec/unit/knife/ssh_spec.rb | 1 | ||||
-rw-r--r-- | spec/unit/knife_spec.rb | 30 |
5 files changed, 90 insertions, 29 deletions
diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb index 15ad38e25e..421a329f22 100644 --- a/lib/chef/knife.rb +++ b/lib/chef/knife.rb @@ -57,6 +57,11 @@ class Chef attr_accessor :name_args attr_accessor :ui + # Configure mixlib-cli to always separate defaults from user-supplied CLI options + def self.use_separate_defaults? + true + end + def self.ui @ui ||= Chef::Knife::UI.new(STDOUT, STDERR, STDIN, {}) end @@ -274,7 +279,6 @@ class Chef command_name_words = command_name_words.join('_') @name_args.reject! { |name_arg| command_name_words == name_arg } - if config[:help] msg opt_parser exit 1 @@ -293,19 +297,15 @@ class Chef exit(1) end - def allow_auto_knife_config? - false - end - - def configure_from_file_settings! - unless(config[:config_file]) - locate_config_file - end - if(config[:config_file]) - self.class.options.keys.each do |key| - config[key] = Chef::Config[:knife][key] if Chef::Config[:knife].has_key?(key) - end + # Returns a subset of the Chef::Config[:knife] Hash that is relevant to the + # currently executing knife command. This is used by #configure_chef to + # apply settings from knife.rb to the +config+ hash. + def config_file_settings + config_file_settings = {} + self.class.options.keys.each do |key| + config_file_settings[key] = Chef::Config[:knife][key] if Chef::Config[:knife].has_key?(key) end + config_file_settings end def locate_config_file @@ -337,20 +337,26 @@ class Chef end end - def configure_chef - unless config[:config_file] - locate_config_file - end - - # Don't try to load a knife.rb if it doesn't exist. - if config[:config_file] - read_config_file(config[:config_file]) - else - # ...but do log a message if no config was found. - Chef::Config[:color] = config[:color] - ui.warn("No knife configuration file found") - end - + # Apply Config in this order: + # defaults from mixlib-cli + # settings from config file, via Chef::Config[:knife] + # config from command line + def merge_configs + # Apply config file settings on top of mixlib-cli defaults + combined_config = default_config.merge(config_file_settings) + # Apply user-supplied options on top of the above combination + combined_config = combined_config.merge(config) + # replace the config hash from mixlib-cli with our own. + # Need to use the mutate-in-place #replace method instead of assigning to + # the instance variable because other code may have a reference to the + # original config hash object. + config.replace(combined_config) + end + + # Catch-all method that does any massaging needed for various config + # components, such as expanding file paths and converting verbosity level + # into log level. + def apply_computed_config Chef::Config[:color] = config[:color] case Chef::Config[:verbosity] @@ -377,8 +383,6 @@ class Chef Chef::Log.init(Chef::Config[:log_location]) Chef::Log.level(Chef::Config[:log_level] || :error) - Chef::Log.debug("Using configuration from #{config[:config_file]}") - if Chef::Config[:node_name] && Chef::Config[:node_name].bytesize > 90 # node names > 90 bytes only work with authentication protocol >= 1.1 # see discussion in config.rb. @@ -386,6 +390,25 @@ class Chef end end + def configure_chef + unless config[:config_file] + locate_config_file + end + + # Don't try to load a knife.rb if it doesn't exist. + if config[:config_file] + Chef::Log.debug("Using configuration from #{config[:config_file]}") + read_config_file(config[:config_file]) + else + # ...but do log a message if no config was found. + Chef::Config[:color] = config[:color] + ui.warn("No knife configuration file found") + end + + merge_configs + apply_computed_config + end + def read_config_file(file) Chef::Config.from_file(file) rescue SyntaxError => e diff --git a/spec/unit/application/knife_spec.rb b/spec/unit/application/knife_spec.rb index 78a65e7045..9560de8d13 100644 --- a/spec/unit/application/knife_spec.rb +++ b/spec/unit/application/knife_spec.rb @@ -23,6 +23,11 @@ describe Chef::Application::Knife do before(:all) do class NoopKnifeCommand < Chef::Knife + option :opt_with_default, + :short => "-D VALUE", + :long => "-optwithdefault VALUE", + :default => "default-value" + def run end end diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb index 76261d96ab..98955d4f73 100644 --- a/spec/unit/knife/bootstrap_spec.rb +++ b/spec/unit/knife/bootstrap_spec.rb @@ -35,6 +35,8 @@ describe Chef::Knife::Bootstrap do before(:each) do Chef::Log.logger = Logger.new(StringIO.new) @knife = Chef::Knife::Bootstrap.new + # Merge default settings in. + @knife.merge_configs @knife.config[:template_file] = File.expand_path(File.join(CHEF_SPEC_DATA, "bootstrap", "test.erb")) @stdout = StringIO.new @knife.ui.stub!(:stdout).and_return(@stdout) diff --git a/spec/unit/knife/ssh_spec.rb b/spec/unit/knife/ssh_spec.rb index a4c4c33c8b..7f1ed0e321 100644 --- a/spec/unit/knife/ssh_spec.rb +++ b/spec/unit/knife/ssh_spec.rb @@ -34,6 +34,7 @@ describe Chef::Knife::Ssh do before do @knife = Chef::Knife::Ssh.new + @knife.merge_configs @knife.config[:attribute] = "fqdn" @node_foo = Chef::Node.new @node_foo.automatic_attrs[:fqdn] = "foo.example.org" diff --git a/spec/unit/knife_spec.rb b/spec/unit/knife_spec.rb index e8fdc38d2b..d4d721a20d 100644 --- a/spec/unit/knife_spec.rb +++ b/spec/unit/knife_spec.rb @@ -180,6 +180,36 @@ describe Chef::Knife do lambda {Chef::Knife.run(%w{fuuu uuuu fuuuu})}.should raise_error(SystemExit) { |e| e.status.should_not == 0 } end + describe "merging configuration options" do + before do + KnifeSpecs::TestYourself.option(:opt_with_default, + :short => "-D VALUE", + :default => "default-value") + Chef::Config[:knife] = {} + end + + it "prefers the default value if no config or command line value is present" do + knife_command = KnifeSpecs::TestYourself.new([]) #empty argv + knife_command.configure_chef + knife_command.config[:opt_with_default].should == "default-value" + end + + it "prefers a value in Chef::Config[:knife] to the default" do + Chef::Config[:knife][:opt_with_default] = "from-knife-config" + knife_command = KnifeSpecs::TestYourself.new([]) #empty argv + knife_command.configure_chef + knife_command.config[:opt_with_default].should == "from-knife-config" + end + + it "prefers a value from command line over Chef::Config and the default" do + Chef::Config[:knife][:opt_with_default] = "from-knife-config" + knife_command = KnifeSpecs::TestYourself.new(["-D", "from-cli"]) + knife_command.configure_chef + knife_command.config[:opt_with_default].should == "from-cli" + end + end + + end describe "when first created" do |