diff options
13 files changed, 534 insertions, 137 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c79dcb1c5..c72caa1633 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * [**Nate Walck**](https://github.com/natewalck) [pr#4078](https://github.com/chef/chef/pull/4078) Add `osx_profile` HWRP for OS X * [pr#4369](https://github.com/chef/chef/pull/4396) Import omnibus-chef chef project definition and history +* [pr#4399](https://github.com/chef/chef/pull/4399) Correctly save `policy_name` and `policy_group` with `knife node edit` ## 12.6.0 diff --git a/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb b/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb index d2e0e64afe..38abd60668 100644 --- a/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb +++ b/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb @@ -21,6 +21,7 @@ require 'chef/chef_fs/file_system/chef_server/acls_dir' require 'chef/chef_fs/file_system/base_fs_dir' require 'chef/chef_fs/file_system/chef_server/rest_list_dir' require 'chef/chef_fs/file_system/chef_server/cookbooks_dir' +require 'chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir' require 'chef/chef_fs/file_system/chef_server/data_bags_dir' require 'chef/chef_fs/file_system/chef_server/nodes_dir' require 'chef/chef_fs/file_system/chef_server/org_entry' @@ -141,7 +142,7 @@ class Chef @children ||= begin result = [ # /cookbooks - CookbooksDir.new("cookbooks", self), + versioned_cookbooks ? VersionedCookbooksDir.new("cookbooks", self) : CookbooksDir.new("cookbooks", self), # /data_bags DataBagsDir.new("data_bags", self, "data"), # /environments diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb index b888b6de22..433f1a009d 100644 --- a/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb +++ b/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb @@ -28,23 +28,23 @@ class Chef module ChefFS module FileSystem module ChefServer + # Unversioned cookbook. + # + # /cookbooks/NAME + # + # Children look like: + # + # - metadata.rb + # - attributes/ + # - libraries/ + # - recipes/ + # class CookbookDir < BaseFSDir def initialize(name, parent, options = {}) super(name, parent) @exists = options[:exists] - # If the name is apache2-1.0.0 and versioned_cookbooks is on, we know - # the actual cookbook_name and version. - if root.versioned_cookbooks - if name =~ VALID_VERSIONED_COOKBOOK_NAME - @cookbook_name = $1 - @version = $2 - else - @exists = false - end - else - @cookbook_name = name - @version = root.cookbook_version # nil unless --cookbook-version specified in download/diff - end + @cookbook_name = name + @version = root.cookbook_version # nil unless --cookbook-version specified in download/diff end attr_reader :cookbook_name, :version @@ -61,10 +61,6 @@ class Chef :root_files => { }, } - # See Erchef code - # https://github.com/opscode/chef_objects/blob/968a63344d38fd507f6ace05f73d53e9cd7fb043/src/chef_regex.erl#L94 - VALID_VERSIONED_COOKBOOK_NAME = /^([.a-zA-Z0-9_-]+)-(\d+\.\d+\.\d+)$/ - def add_child(child) @children << child end diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb index 28d88ea330..5fd8111d7a 100644 --- a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb +++ b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb @@ -29,6 +29,15 @@ class Chef module ChefFS module FileSystem module ChefServer + # cookbooks/ + # apache2/ + # mysql/ + # cookbook_artifacts/ + # apache2/ + # 1.0.0/ + # 1.0.1/ + # mysql/ + # 2.0.5/ class CookbooksDir < RestListDir include Chef::Mixin::FileClass @@ -40,16 +49,7 @@ class Chef def children @children ||= begin - if root.versioned_cookbooks - result = [] - root.get_json("#{api_path}/?num_versions=all").each_pair do |cookbook_name, cookbooks| - cookbooks['versions'].each do |cookbook_version| - result << CookbookDir.new("#{cookbook_name}-#{cookbook_version['version']}", self, :exists => true) - end - end - else - result = root.get_json(api_path).keys.map { |cookbook_name| CookbookDir.new(cookbook_name, self, :exists => true) } - end + result = root.get_json(api_path).keys.map { |cookbook_name| CookbookDir.new(cookbook_name, self, exists: true) } result.sort_by(&:name) end end @@ -60,7 +60,7 @@ class Chef end def upload_cookbook_from(other, options = {}) - root.versioned_cookbooks ? upload_versioned_cookbook(other, options) : upload_unversioned_cookbook(other, options) + upload_cookbook(other, options) rescue Timeout::Error => e raise Chef::ChefFS::FileSystem::OperationFailedError.new(:write, self, e, "Timeout writing: #{e}") rescue Net::HTTPServerException => e @@ -74,53 +74,13 @@ class Chef raise Chef::ChefFS::FileSystem::CookbookFrozenError.new(:write, self, e, "Cookbook #{other.name} is frozen") end - # Knife currently does not understand versioned cookbooks - # Cookbook Version uploader also requires a lot of refactoring - # to make this work. So instead, we make a temporary cookbook - # symlinking back to real cookbook, and upload the proxy. - def upload_versioned_cookbook(other, options) - cookbook_name = Chef::ChefFS::FileSystem::Repository::ChefRepositoryFileSystemCookbookDir.canonical_cookbook_name(other.name) - - Dir.mktmpdir do |temp_cookbooks_path| - proxy_cookbook_path = "#{temp_cookbooks_path}/#{cookbook_name}" - - # Make a symlink - 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.load_cookbooks - - cookbook_to_upload = proxy_loader.cookbook_version - cookbook_to_upload.freeze_version if options[:freeze] - - # Instantiate a new uploader based on the proxy loader - uploader = Chef::CookbookUploader.new(cookbook_to_upload, :force => options[:force], :rest => root.chef_rest) - - with_actual_cookbooks_dir(temp_cookbooks_path) do - upload_cookbook!(uploader) - end - - # - # When the temporary directory is being deleted on - # windows, the contents of the symlink under that - # directory is also deleted. So explicitly remove - # the symlink without removing the original contents if we - # are running on windows - # - if Chef::Platform.windows? - Dir.rmdir proxy_cookbook_path - end - end - end - - def upload_unversioned_cookbook(other, options) + def upload_cookbook(other, options) cookbook_to_upload = other.chef_object cookbook_to_upload.freeze_version if options[:freeze] uploader = Chef::CookbookUploader.new(cookbook_to_upload, :force => options[:force], :rest => root.chef_rest) with_actual_cookbooks_dir(other.parent.file_path) do - upload_cookbook!(uploader) + uploader.upload_cookbooks end end @@ -134,18 +94,8 @@ class Chef Chef::Config.cookbook_path = old_cookbook_path end - def upload_cookbook!(uploader, options = {}) - if uploader.respond_to?(:upload_cookbook) - uploader.upload_cookbook - else - uploader.upload_cookbooks - end - end - def can_have_child?(name, is_dir) - return false if !is_dir - return false if root.versioned_cookbooks && name !~ Chef::ChefFS::FileSystem::ChefServer::CookbookDir::VALID_VERSIONED_COOKBOOK_NAME - return true + is_dir end end end diff --git a/lib/chef/chef_fs/file_system/chef_server/versioned_cookbook_dir.rb b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbook_dir.rb new file mode 100644 index 0000000000..25b3fa38cc --- /dev/null +++ b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbook_dir.rb @@ -0,0 +1,45 @@ +# +# Author:: John Keiser (<jkeiser@opscode.com>) +# Copyright:: Copyright (c) 2012 Opscode, 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 'chef/chef_fs/file_system/chef_server/versioned_cookbook_dir' + +class Chef + module ChefFS + module FileSystem + module ChefServer + class VersionedCookbookDir < CookbookDir + # See Erchef code + # https://github.com/opscode/chef_objects/blob/968a63344d38fd507f6ace05f73d53e9cd7fb043/src/chef_regex.erl#L94 + VALID_VERSIONED_COOKBOOK_NAME = /^([.a-zA-Z0-9_-]+)-(\d+\.\d+\.\d+)$/ + + def initialize(name, parent, options = {}) + super(name, parent) + # If the name is apache2-1.0.0 and versioned_cookbooks is on, we know + # the actual cookbook_name and version. + if name =~ VALID_VERSIONED_COOKBOOK_NAME + @cookbook_name = $1 + @version = $2 + else + @exists = false + end + end + end + end + end + end +end 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 new file mode 100644 index 0000000000..bccb34bb37 --- /dev/null +++ b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb @@ -0,0 +1,100 @@ +# +# Author:: John Keiser (<jkeiser@opscode.com>) +# Copyright:: Copyright (c) 2012 Opscode, 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 'chef/chef_fs/file_system/chef_server/cookbooks_dir' +require 'chef/chef_fs/file_system/chef_server/versioned_cookbook_dir' + +class Chef + module ChefFS + module FileSystem + module ChefServer + # /cookbooks + # + # Its children look like: + # + # - apache2-1.0.0 + # - apache2-1.0.1 + # - mysql-2.0.5 + # + class VersionedCookbooksDir < CookbooksDir + + def make_child_entry(name) + result = @children.select { |child| child.name == name }.first if @children + result || VersionedCookbookDir.new(name, self) + end + + def children + @children ||= begin + result = [] + root.get_json("#{api_path}/?num_versions=all").each_pair do |cookbook_name, cookbooks| + cookbooks['versions'].each do |cookbook_version| + result << VersionedCookbookDir.new("#{cookbook_name}-#{cookbook_version['version']}", self) + end + end + result.sort_by(&:name) + end + end + + # Knife currently does not understand versioned cookbooks + # Cookbook Version uploader also requires a lot of refactoring + # to make this work. So instead, we make a temporary cookbook + # symlinking back to real cookbook, and upload the proxy. + def upload_cookbook(other, options) + cookbook_name = Chef::ChefFS::FileSystem::Repository::ChefRepositoryFileSystemCookbookDir.canonical_cookbook_name(other.name) + + Dir.mktmpdir do |temp_cookbooks_path| + proxy_cookbook_path = "#{temp_cookbooks_path}/#{cookbook_name}" + + # Make a symlink + 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.load_cookbooks + + cookbook_to_upload = proxy_loader.cookbook_version + cookbook_to_upload.freeze_version if options[:freeze] + + # Instantiate a new uploader based on the proxy loader + uploader = Chef::CookbookUploader.new(cookbook_to_upload, :force => options[:force], :rest => root.chef_rest) + + with_actual_cookbooks_dir(temp_cookbooks_path) do + uploader.upload_cookbooks + end + + # + # When the temporary directory is being deleted on + # windows, the contents of the symlink under that + # directory is also deleted. So explicitly remove + # the symlink without removing the original contents if we + # are running on windows + # + if Chef::Platform.windows? + Dir.rmdir proxy_cookbook_path + end + end + end + + def can_have_child?(name, is_dir) + is_dir && name =~ Chef::ChefFS::FileSystem::ChefServer::VersionedCookbookDir::VALID_VERSIONED_COOKBOOK_NAME + end + end + end + end + end +end 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 91b8652de3..e6557bd05d 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 @@ -18,6 +18,7 @@ require 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_entry' require 'chef/chef_fs/file_system/chef_server/cookbook_dir' +require 'chef/chef_fs/file_system/chef_server/versioned_cookbook_dir' require 'chef/chef_fs/file_system/not_found_error' require 'chef/cookbook/chefignore' require 'chef/cookbook/cookbook_version_loader' @@ -27,25 +28,9 @@ class Chef module FileSystem module Repository class ChefRepositoryFileSystemCookbookDir < ChefRepositoryFileSystemCookbookEntry - def initialize(name, parent, file_path = nil) - super(name, parent, file_path) - end - def chef_object begin - loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.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 - if root.versioned_cookbooks - canonical_name = canonical_cookbook_name(File.basename(file_path)) - raise "When versioned_cookbooks mode is on, cookbook #{file_path} must match format <cookbook_name>-x.y.z" unless canonical_name - - # KLUDGE: We shouldn't have to use instance_variable_set - loader.instance_variable_set(:@cookbook_name, canonical_name) - end - - loader.load_cookbooks - cb = loader.cookbook_version + cb = cookbook_version if !cb Chef::Log.error("Cookbook #{file_path} empty.") raise "Cookbook #{file_path} empty." @@ -74,7 +59,7 @@ class Chef # Exposed as a class method so that it can be used elsewhere def self.canonical_cookbook_name(entry_name) - name_match = Chef::ChefFS::FileSystem::ChefServer::CookbookDir::VALID_VERSIONED_COOKBOOK_NAME.match(entry_name) + name_match = Chef::ChefFS::FileSystem::ChefServer::VersionedCookbookDir::VALID_VERSIONED_COOKBOOK_NAME.match(entry_name) return nil if name_match.nil? return name_match[1] end @@ -97,6 +82,12 @@ class Chef segment_info = Chef::ChefFS::FileSystem::ChefServer::CookbookDir::COOKBOOK_SEGMENT_INFO[child_name.to_sym] || {} ChefRepositoryFileSystemCookbookEntry.new(child_name, self, nil, segment_info[:ruby_only], segment_info[:recursive]) end + + def cookbook_version + loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore) + loader.load_cookbooks + cb = loader.cookbook_version + end end end end diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb index c189323c9a..2e9eeea0ee 100644 --- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb +++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb @@ -20,6 +20,7 @@ require 'chef/chef_fs/file_system/base_fs_dir' require 'chef/chef_fs/file_system/repository/chef_repository_file_system_entry' require 'chef/chef_fs/file_system/repository/chef_repository_file_system_acls_dir' require 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbooks_dir' +require 'chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir' require 'chef/chef_fs/file_system/repository/chef_repository_file_system_data_bags_dir' require 'chef/chef_fs/file_system/multiplexed_dir' require 'chef/chef_fs/data_handler/client_data_handler' @@ -162,7 +163,11 @@ class Chef end case name when 'cookbooks' - dirs = paths.map { |path| ChefRepositoryFileSystemCookbooksDir.new(name, self, path) } + if versioned_cookbooks + dirs = paths.map { |path| ChefRepositoryFileSystemVersionedCookbooksDir.new(name, self, path) } + else + dirs = paths.map { |path| ChefRepositoryFileSystemCookbooksDir.new(name, self, path) } + end when 'data_bags' dirs = paths.map { |path| ChefRepositoryFileSystemDataBagsDir.new(name, self, path) } when 'acls' 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 new file mode 100644 index 0000000000..cf9378dbf9 --- /dev/null +++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb @@ -0,0 +1,42 @@ +# +# Author:: John Keiser (<jkeiser@opscode.com>) +# Copyright:: Copyright (c) 2013 Opscode, 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 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir' + +class Chef + module ChefFS + module FileSystem + module Repository + class ChefRepositoryFileSystemVersionedCookbookDir < ChefRepositoryFileSystemCookbookDir + # Override from parent + def cookbook_version + loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.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)) + raise "When versioned_cookbooks mode is on, cookbook #{file_path} must match format <cookbook_name>-x.y.z" unless canonical_name + # KLUDGE: We shouldn't have to use instance_variable_set + loader.instance_variable_set(:@cookbook_name, canonical_name) + loader.load_cookbooks + loader.cookbook_version + end + end + end + end + end +end diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir.rb new file mode 100644 index 0000000000..322944fc82 --- /dev/null +++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir.rb @@ -0,0 +1,34 @@ +# +# Author:: John Keiser (<jkeiser@opscode.com>) +# Copyright:: Copyright (c) 2013 Opscode, 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 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbooks_dir' +require 'chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir' + +class Chef + module ChefFS + module FileSystem + module Repository + class ChefRepositoryFileSystemVersionedCookbooksDir < ChefRepositoryFileSystemCookbooksDir + def make_child_entry(child_name) + ChefRepositoryFileSystemVersionedCookbookDir.new(child_name, self) + end + end + end + end + end +end diff --git a/lib/chef/environment.rb b/lib/chef/environment.rb index aedccbb08d..45491e0d72 100644 --- a/lib/chef/environment.rb +++ b/lib/chef/environment.rb @@ -246,7 +246,7 @@ class Chef if Chef::Config[:solo] load_from_file(name) else - chef_server_rest.get("environments/#{name}") + self.from_hash(chef_server_rest.get("environments/#{name}")) end end diff --git a/lib/chef/knife/core/node_editor.rb b/lib/chef/knife/core/node_editor.rb index fe14e18d9d..7b7cb72df9 100644 --- a/lib/chef/knife/core/node_editor.rb +++ b/lib/chef/knife/core/node_editor.rb @@ -1,6 +1,7 @@ # # Author:: Daniel DeLeo (<dan@opscode.com>) -# Copyright:: Copyright (c) 2011 Opscode, Inc. +# Author:: Jordan Running (<jr@chef.io>) +# Copyright:: Copyright (c) 2011-2016 Chef Software, Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -18,81 +19,101 @@ require 'chef/json_compat' require 'chef/node' -require 'tempfile' class Chef class Knife class NodeEditor + attr_reader :node, :ui, :config + private :node, :ui, :config - attr_reader :node - attr_reader :ui - attr_reader :config - + # @param node [Chef::Node] + # @param ui [Chef::Knife::UI] + # @param config [Hash] def initialize(node, ui, config) @node, @ui, @config = node, ui, config end + # Opens the node data (as JSON) in the user's editor and returns a new + # {Chef::Node} reflecting the user's changes. + # + # @return [Chef::Node] def edit_node abort "You specified the --disable_editing option, nothing to edit" if config[:disable_editing] assert_editor_set! - updated_node_data = @ui.edit_data(view) + updated_node_data = ui.edit_data(view) apply_updates(updated_node_data) @updated_node end + # Returns an array of the names of properties that have been changed or + # +false+ if none were changed. + # + # @return [Array<String>] if any properties have been changed. + # @return [false] if no properties have been changed. def updated? + return false if @updated_node.nil? + pristine_copy = Chef::JSONCompat.parse(Chef::JSONCompat.to_json(node)) updated_copy = Chef::JSONCompat.parse(Chef::JSONCompat.to_json(@updated_node)) - unless pristine_copy == updated_copy - updated_properties = %w{name normal chef_environment run_list default override automatic}.reject do |key| - pristine_copy[key] == updated_copy[key] - end + + updated_properties = %w[ + name + chef_environment + automatic + default + normal + override + policy_name + policy_group + run_list + ].reject do |key| + pristine_copy[key] == updated_copy[key] end - ( pristine_copy != updated_copy ) && updated_properties - end - private + updated_properties.any? && updated_properties + end + # @api private def view - result = {} - result["name"] = node.name - result["chef_environment"] = node.chef_environment - result["normal"] = node.normal_attrs - result["run_list"] = node.run_list + result = { + "name" => node.name, + "chef_environment" => node.chef_environment, + "normal" => node.normal_attrs, + "policy_name" => node.policy_name, + "policy_group" => node.policy_group, + "run_list" => node.run_list, + } if config[:all_attributes] result["default"] = node.default_attrs result["override"] = node.override_attrs result["automatic"] = node.automatic_attrs end + result end + # @api private def apply_updates(updated_data) if node.name and node.name != updated_data["name"] ui.warn "Changing the name of a node results in a new node being created, #{node.name} will not be modified or removed." - confirm = ui.confirm "Proceed with creation of new node" + ui.confirm "Proceed with creation of new node" end - @updated_node = Node.new.tap do |n| - n.name( updated_data["name"] ) - n.chef_environment( updated_data["chef_environment"] ) - n.run_list( updated_data["run_list"]) - n.normal_attrs = updated_data["normal"] - - if config[:all_attributes] - n.default_attrs = updated_data["default"] - n.override_attrs = updated_data["override"] - n.automatic_attrs = updated_data["automatic"] - else - n.default_attrs = node.default_attrs - n.override_attrs = node.override_attrs - n.automatic_attrs = node.automatic_attrs - end + data = updated_data.dup + + unless config[:all_attributes] + data['automatic'] = node.automatic_attrs + data['default'] = node.default_attrs + data['override'] = node.override_attrs end + + @updated_node = Node.from_hash(data) end + private + def abort(message) ui.error(message) exit 1 diff --git a/spec/unit/knife/core/node_editor_spec.rb b/spec/unit/knife/core/node_editor_spec.rb new file mode 100644 index 0000000000..52e3aeadd5 --- /dev/null +++ b/spec/unit/knife/core/node_editor_spec.rb @@ -0,0 +1,211 @@ +# +# Author:: Jordan Running (<jr@chef.io>) +# Copyright:: Copyright (c) 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/core/node_editor' + +describe Chef::Knife::NodeEditor do + let(:node_data) do + { 'name' => 'test_node', + 'chef_environment' => 'production', + 'automatic' => { 'foo' => 'bar' }, + 'default' => { 'alpha' => { 'bravo' => 'charlie', 'delta' => 'echo' } }, + 'normal' => { 'alpha' => { 'bravo' => 'hotel' }, 'tags' => [] }, + 'override' => { 'alpha' => { 'bravo' => 'foxtrot', 'delta' => 'golf' } }, + 'policy_name' => nil, + 'policy_group' => nil, + 'run_list' => %w(role[comedy] role[drama] recipe[mystery]) + } + end + + let(:node) { Chef::Node.from_hash(node_data) } + + let(:ui) { double 'ui' } + let(:base_config) { { editor: 'cat' } } + let(:config) { base_config.merge(all_attributes: false) } + + subject { described_class.new(node, ui, config) } + + describe '#view' do + it 'returns a Hash with only the name, chef_environment, normal, ' + + 'policy_name, policy_group, and run_list properties' do + expected = node_data.select do |key,| + %w[ name chef_environment normal + policy_name policy_group run_list ].include?(key) + end + + expect(subject.view).to eq(expected) + end + + context 'when config[:all_attributes] == true' do + let(:config) { base_config.merge(all_attributes: true) } + + it 'returns a Hash with all of the node\'s properties' do + expect(subject.view).to eq(node_data) + end + end + end + + describe '#apply_updates' do + context 'when the node name is changed' do + before(:each) do + allow(ui).to receive(:warn) + allow(ui).to receive(:confirm).and_return(true) + end + + it 'emits a warning and prompts for confirmation' do + data = subject.view.merge('name' => 'foo_new_name_node') + updated_node = subject.apply_updates(data) + + expect(ui).to have_received(:warn) + .with 'Changing the name of a node results in a new node being ' + + 'created, test_node will not be modified or removed.' + + expect(ui).to have_received(:confirm) + .with('Proceed with creation of new node') + + expect(updated_node).to be_a(Chef::Node) + end + end + + context 'when config[:all_attributes] == false' do + let(:config) { base_config.merge(all_attributes: false) } + + let(:updated_data) do + subject.view.merge( + 'normal' => { 'alpha' => { 'bravo' => 'hotel2' }, 'tags' => [ 'xyz' ] }, + 'policy_name' => 'mypolicy', + 'policy_group' => 'prod', + 'run_list' => %w(role[drama] recipe[mystery]) + ) + end + + it 'returns a node with run_list and normal_attrs changed' do + updated_node = subject.apply_updates(updated_data) + expect(updated_node).to be_a(Chef::Node) + + # Expected to have been changed + expect(updated_node.chef_environment).to eql(updated_data['chef_environment']) + expect(updated_node.normal_attrs).to eql(updated_data['normal']) + expect(updated_node.policy_name).to eql(updated_data['policy_name']) + expect(updated_node.policy_group).to eql(updated_data['policy_group']) + expect(updated_node.run_list.map(&:to_s)).to eql(updated_data['run_list']) + + # Expected not to have changed + expect(updated_node.default_attrs).to eql(node.default_attrs) + expect(updated_node.override_attrs).to eql(node.override_attrs) + expect(updated_node.automatic_attrs).to eql(node.automatic_attrs) + end + end + + context 'when config[:all_attributes] == true' do + let(:config) { base_config.merge(all_attributes: true) } + + let(:updated_data) do + subject.view.merge( + 'default' => { 'alpha' => { 'bravo' => 'charlie2', 'delta' => 'echo2' } }, + 'normal' => { 'alpha' => { 'bravo' => 'hotel2' }, 'tags' => [ 'xyz' ] }, + 'override' => { 'alpha' => { 'bravo' => 'foxtrot2', 'delta' => 'golf2' } }, + 'policy_name' => 'mypolicy', + 'policy_group' => 'prod', + 'run_list' => %w(role[drama] recipe[mystery]) + ) + end + + it 'returns a node with all editable properties changed' do + updated_node = subject.apply_updates(updated_data) + expect(updated_node).to be_a(Chef::Node) + + expect(updated_node.chef_environment).to eql(updated_data['chef_environment']) + expect(updated_node.automatic_attrs).to eql(updated_data['automatic']) + expect(updated_node.normal_attrs).to eql(updated_data['normal']) + expect(updated_node.default_attrs).to eql(updated_data['default']) + expect(updated_node.override_attrs).to eql(updated_data['override']) + expect(updated_node.policy_name).to eql(updated_data['policy_name']) + expect(updated_node.policy_group).to eql(updated_data['policy_group']) + expect(updated_node.run_list.map(&:to_s)).to eql(updated_data['run_list']) + end + end + end + + describe '#updated?' do + context 'before the node has been edited' do + it 'returns false' do + expect(subject.updated?).to be false + end + end + + context 'after the node has been edited' do + context 'and changes were made' do + let(:updated_data) do + subject.view.merge( + 'default' => { 'alpha' => { 'bravo' => 'charlie2', 'delta' => 'echo2' } }, + 'normal' => { 'alpha' => { 'bravo' => 'hotel2' }, 'tags' => [ 'xyz' ] }, + 'override' => { 'alpha' => { 'bravo' => 'foxtrot2', 'delta' => 'golf2' } }, + 'policy_name' => 'mypolicy', + 'policy_group' => 'prod', + 'run_list' => %w(role[drama] recipe[mystery]) + ) + end + + context 'and changes affect only editable properties' do + before(:each) do + allow(ui).to receive(:edit_data) + .with(subject.view) + .and_return(updated_data) + + subject.edit_node + end + + it 'returns an array of the changed property names' do + expect(subject.updated?).to eql %w[ normal policy_name policy_group run_list ] + end + end + + context 'and the changes include non-editable properties' do + before(:each) do + data = updated_data.merge('bad_property' => 'bad_value') + + allow(ui).to receive(:edit_data) + .with(subject.view) + .and_return(data) + + subject.edit_node + end + + it 'returns an array of property names that doesn\'t include ' + + 'the non-editable properties' do + expect(subject.updated?).to eql %w[ normal policy_name policy_group run_list ] + end + end + end + + context 'and changes were not made' do + before(:each) do + allow(ui).to receive(:edit_data) + .with(subject.view) + .and_return(subject.view.dup) + + subject.edit_node + end + + it { is_expected.not_to be_updated } + end + end + end +end |