From de7c66c544c7eca101e803afb6ee2b471d027ce5 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Wed, 6 May 2020 14:47:52 -0700 Subject: Chef 15 backport of Chef 16 knife cleanup Backport of some of #9646 to make it so Chef-16 style knife plugins can run on Chef-15. Signed-off-by: Lamont Granquist --- chef-config/lib/chef-config/config.rb | 27 --------------- lib/chef/knife.rb | 46 +++++++++++++++----------- lib/chef/knife/bootstrap/chef_vault_handler.rb | 20 ++++++----- lib/chef/knife/bootstrap/client_builder.rb | 42 ++++++++++++----------- lib/chef/knife/ssh.rb | 2 +- 5 files changed, 62 insertions(+), 75 deletions(-) diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb index d784096081..5a88251dcc 100644 --- a/chef-config/lib/chef-config/config.rb +++ b/chef-config/lib/chef-config/config.rb @@ -851,33 +851,6 @@ module ChefConfig # knife configuration data config_context :knife do - # XXX: none of these default values are applied to knife (and would create a backcompat - # break in knife if this bug was fixed since many of the defaults below are wrong). this appears - # to be the start of an attempt to be able to use config_strict_mode true? if so, this approach - # is fraught with peril because this namespace is used by every knife plugin in the wild and - # we would need to validate every cli option in every knife attribute out there and list them all here. - # - # based on the way that people may define `knife[:foobar] = "something"` for the knife-foobar - # gem plugin i'm pretty certain we can never turn on anything like config_string_mode since - # any config value may be a typo or it may be in some gem in some knife plugin we don't know about. - # - # we do still need to maintain at least one of these so that the knife config hash gets - # created. - # - # this whole situation is deeply unsatisfying. - default :ssh_port, nil - default :ssh_user, nil - default :ssh_attribute, nil - default :ssh_gateway, nil - default :ssh_gateway_identity, nil - default :bootstrap_version, nil - default :bootstrap_proxy, nil - default :bootstrap_template, nil - default :secret, nil - default :secret_file, nil - default :host_key_verify, nil - default :forward_agent, nil - default :sort_status_reverse, nil default :hints, {} end diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb index a8aa97eb3e..2d8256a093 100644 --- a/lib/chef/knife.rb +++ b/lib/chef/knife.rb @@ -345,31 +345,35 @@ class Chef exit(1) end - # keys from mixlib-cli options - def cli_keys - self.class.options.keys + # This is all set and default mixlib-config values. We only need the default + # values here (the set values are explicitly mixed in again later), but there is + # no mixlib-config API to get a Hash back with only the default values. + # + # Assumption: since config_file_defaults is the lowest precedence it doesn't matter + # that we include the set values here, but this is a hack and makes the name of the + # method a lie. FIXME: make the name not a lie by adding an API to mixlib-config. + # + # @api private + # + def config_file_defaults + Chef::Config[:knife].save(true) # this is like "dup" to a (real) Hash, and includes default values (and user set values) end - # extracts the settings from the Chef::Config[:knife] sub-hash that correspond - # to knife cli options -- in preparation for merging config values with cli values + # This is only the user-set mixlib-config values. We do not include the defaults + # here so that the config defaults do not override the cli defaults. + # + # @api private # - # NOTE: due to weirdness in mixlib-config #has_key? is only true if the value has - # been set by the user -- the Chef::Config defaults return #has_key?() of false and - # this code DEPENDS on that functionality since applying the default values in - # Chef::Config[:knife] would break the defaults in the cli that we would otherwise - # overwrite. def config_file_settings - cli_keys.each_with_object({}) do |key, memo| - if Chef::Config[:knife].key?(key) - memo[key] = Chef::Config[:knife][key] - end - end + Chef::Config[:knife].save(false) # this is like "dup" to a (real) Hash, and does not include default values (just user set values) end # config is merged in this order (inverse of precedence) - # default_config - mixlib-cli defaults (accessor from the mixin) - # config_file_settings - Chef::Config[:knife] sub-hash - # config - mixlib-cli settings (accessor from the mixin) + # config_file_defaults - Chef::Config[:knife] defaults from chef-config (XXX: this also includes the settings, but they get overwritten) + # default_config - mixlib-cli defaults (accessor from mixlib-cli) + # config_file_settings - Chef::Config[:knife] user settings from the client.rb file + # config - mixlib-cli settings (accessor from mixlib-cli) + # def merge_configs # Update our original_config - if someone has created a knife command # instance directly, they are likely ot have set cmd.config values directly @@ -377,7 +381,7 @@ class Chef @original_config = config.dup # other code may have a handle to the config object, so use Hash#replace to deliberately # update-in-place. - config.replace(default_config.merge(config_file_settings).merge(config)) + config.replace(config_file_defaults.merge(default_config).merge(config_file_settings).merge(config)) end # @@ -387,8 +391,9 @@ class Chef # @return [Symbol,NilClass] return the source of the config key, # one of: # - :cli - this was explicitly provided on the CLI - # - :config - this came from Chef::Config[:knife] + # - :config - this came from Chef::Config[:knife] explicitly being set # - :cli_default - came from a declared CLI `option`'s `default` value. + # - :config_default - this came from Chef::Config[:knife]'s defaults # - nil - if the key could not be found in any source. # This can happen when it is invalid, or has been # set directly into #config without then calling #merge_config @@ -396,6 +401,7 @@ class Chef return :cli if @original_config.include? key return :config if config_file_settings.key? key return :cli_default if default_config.include? key + return :config_default if config_file_defaults.key? key # must come after :config check nil end diff --git a/lib/chef/knife/bootstrap/chef_vault_handler.rb b/lib/chef/knife/bootstrap/chef_vault_handler.rb index 7007e1403b..e189839857 100644 --- a/lib/chef/knife/bootstrap/chef_vault_handler.rb +++ b/lib/chef/knife/bootstrap/chef_vault_handler.rb @@ -21,7 +21,7 @@ class Chef class ChefVaultHandler # @return [Hash] knife merged config, typically @config - attr_accessor :knife_config + attr_accessor :config # @return [Chef::Knife::UI] ui object for output attr_accessor :ui @@ -29,11 +29,15 @@ class Chef # @return [Chef::ApiClient] vault client attr_reader :client - # @param knife_config [Hash] knife merged config, typically @config + # @param config [Hash] knife merged config, typically @config # @param ui [Chef::Knife::UI] ui object for output - def initialize(knife_config: {}, ui: nil) - @knife_config = knife_config - @ui = ui + def initialize(config: {}, knife_config: nil, ui: nil) + @config = config + unless knife_config.nil? + # the knife_config argument becomes deprecated in Chef-16, don't use it + @config = knife_config + end + @ui = ui end # Updates the chef vault items for the newly created client. @@ -85,17 +89,17 @@ class Chef # @return [String] string with serialized JSON representing the chef vault items def bootstrap_vault_json - knife_config[:bootstrap_vault_json] + config[:bootstrap_vault_json] end # @return [String] JSON text in a file representing the chef vault items def bootstrap_vault_file - knife_config[:bootstrap_vault_file] + config[:bootstrap_vault_file] end # @return [Hash] Ruby object representing the chef vault items to create def bootstrap_vault_item - knife_config[:bootstrap_vault_item] + config[:bootstrap_vault_item] end # Helper to return a ruby object represeting all the data bags and items diff --git a/lib/chef/knife/bootstrap/client_builder.rb b/lib/chef/knife/bootstrap/client_builder.rb index d557cc1feb..5e0f60fa67 100644 --- a/lib/chef/knife/bootstrap/client_builder.rb +++ b/lib/chef/knife/bootstrap/client_builder.rb @@ -28,7 +28,7 @@ class Chef class ClientBuilder # @return [Hash] knife merged config, typically @config - attr_accessor :knife_config + attr_accessor :config # @return [Hash] chef config object attr_accessor :chef_config # @return [Chef::Knife::UI] ui object for output @@ -36,13 +36,17 @@ class Chef # @return [Chef::ApiClient] client saved on run attr_reader :client - # @param knife_config [Hash] Hash of knife config settings + # @param config [Hash] Hash of knife config settings # @param chef_config [Hash] Hash of chef config settings # @param ui [Chef::Knife::UI] UI object for output - def initialize(knife_config: {}, chef_config: {}, ui: nil) - @knife_config = knife_config - @chef_config = chef_config - @ui = ui + def initialize(config: {}, knife_config: nil, chef_config: {}, ui: nil) + @config = config + unless knife_config.nil? + # the knife_config argument becomes deprecated in Chef-16, don't use it + @config = knife_config + end + @chef_config = chef_config + @ui = ui end # Main entry. Prompt the user to clean up any old client or node objects. Then create @@ -77,34 +81,34 @@ class Chef private - # @return [String] node name from the knife_config + # @return [String] node name from the config def node_name - knife_config[:chef_node_name] + config[:chef_node_name] end - # @return [String] enviroment from the knife_config + # @return [String] enviroment from the config def environment - knife_config[:environment] + config[:environment] end - # @return [String] run_list from the knife_config + # @return [String] run_list from the config def run_list - knife_config[:run_list] + config[:run_list] end - # @return [String] policy_name from the knife_config + # @return [String] policy_name from the config def policy_name - knife_config[:policy_name] + config[:policy_name] end - # @return [String] policy_group from the knife_config + # @return [String] policy_group from the config def policy_group - knife_config[:policy_group] + config[:policy_group] end - # @return [Hash,Array] Object representation of json first-boot attributes from the knife_config + # @return [Hash,Array] Object representation of json first-boot attributes from the config def first_boot_attributes - knife_config[:first_boot_attributes] + config[:first_boot_attributes] end # @return [String] chef server url from the Chef::Config @@ -154,7 +158,7 @@ class Chef node.environment(environment) if environment node.policy_name = policy_name if policy_name node.policy_group = policy_group if policy_group - (knife_config[:tags] || []).each do |tag| + (config[:tags] || []).each do |tag| node.tags << tag end node diff --git a/lib/chef/knife/ssh.rb b/lib/chef/knife/ssh.rb index a959dc887d..01ddd37146 100644 --- a/lib/chef/knife/ssh.rb +++ b/lib/chef/knife/ssh.rb @@ -547,7 +547,7 @@ class Chef end def get_stripped_unfrozen_value(value) - return nil if value.nil? + return nil unless value value.strip end -- cgit v1.2.1