diff options
| -rw-r--r-- | lib/chef/node_map.rb | 61 | ||||
| -rw-r--r-- | spec/unit/node_map_spec.rb | 5 | ||||
| -rw-r--r-- | spec/unit/platform_spec.rb | 23 | ||||
| -rw-r--r-- | spec/unit/recipe_spec.rb | 16 | ||||
| -rw-r--r-- | spec/unit/resource_spec.rb | 2 |
5 files changed, 38 insertions, 69 deletions
diff --git a/lib/chef/node_map.rb b/lib/chef/node_map.rb index 4e07f93593..bfa884d6d7 100644 --- a/lib/chef/node_map.rb +++ b/lib/chef/node_map.rb @@ -34,11 +34,11 @@ class Chef # @yield [node] Arbitrary node filter as a block which takes a node argument # @return [NodeMap] Returns self for possible chaining # - def set(key, value, platform: nil, platform_family: nil, os: nil, on_platform: nil, on_platforms: nil, &block) + def set(key, value, platform: nil, platform_version: nil, platform_family: nil, os: nil, on_platform: nil, on_platforms: nil, &block) Chef::Log.deprecation "The on_platform option to node_map has been deprecated" if on_platform Chef::Log.deprecation "The on_platforms option to node_map has been deprecated" if on_platforms platform ||= on_platform || on_platforms - filters = { platform: platform, platform_family: platform_family, os: os } + filters = { platform: platform, platform_version: platform_version, platform_family: platform_family, os: os } new_matcher = { filters: filters, block: block, value: value } @map[key] ||= [] # Decide where to insert the matcher; the new value is preferred over @@ -110,47 +110,32 @@ class Chef specificity end - # @todo: this works fine, but is probably hard to understand - def negative_match(filter, param) - # We support strings prefaced by '!' to mean 'not'. In particular, this is most useful - # for os matching on '!windows'. - negative_matches = filter.select { |f| f[0] == '!' } - return true if !negative_matches.empty? && negative_matches.include?('!' + param) + # + # Succeeds if: + # - no negative matches (!value) + # - at least one positive match (value or :all), or no positive filters + # + def matches_black_white_list?(node, filters, attribute) + # It's super common for the filter to be nil. Catch that so we don't + # spend any time here. + return true if !filters[attribute] + filter_values = Array(filters[attribute]) + value = node[attribute] + + # Split the blacklist and whitelist + blacklist, whitelist = filter_values.partition { |v| v.is_a?(String) && v.start_with?('!') } - # We support the symbol :all to match everything, for backcompat, but this can and should - # simply be ommitted. - positive_matches = filter.reject { |f| f[0] == '!' || f == :all } - return true if !positive_matches.empty? && !positive_matches.include?(param) + # If any blacklist value matches, we don't match + return false if blacklist.any? { |v| v[1..-1] == value } - # sorry double-negative: this means we pass this filter. - false + # If the whitelist is empty, or anything matches, we match. + whitelist.empty? || whitelist.any? { |v| v == :all || v == value } end def filters_match?(node, filters) - return true if filters.empty? - - # each filter is applied in turn. if any fail, then it shortcuts and returns false. - # if it passes or does not exist it succeeds and continues on. so multiple filters are - # effectively joined by 'and'. all filters can be single strings, or arrays which are - # effectively joined by 'or'. - - os_filter = [ filters[:os] ].flatten.compact - unless os_filter.empty? - return false if negative_match(os_filter, node[:os]) - end - - platform_family_filter = [ filters[:platform_family] ].flatten.compact - unless platform_family_filter.empty? - return false if negative_match(platform_family_filter, node[:platform_family]) - end - - # :on_platform and :on_platforms here are synonyms which are deprecated - platform_filter = [ filters[:platform] || filters[:on_platform] || filters[:on_platforms] ].flatten.compact - unless platform_filter.empty? - return false if negative_match(platform_filter, node[:platform]) - end - - return true + matches_black_white_list?(node, filters, :os) && + matches_black_white_list?(node, filters, :platform_family) && + matches_black_white_list?(node, filters, :platform) end def block_matches?(node, block) diff --git a/spec/unit/node_map_spec.rb b/spec/unit/node_map_spec.rb index fe7372961b..9b5ff5e8c6 100644 --- a/spec/unit/node_map_spec.rb +++ b/spec/unit/node_map_spec.rb @@ -134,6 +134,10 @@ describe Chef::NodeMap do end describe "resource back-compat testing" do + before :each do + Chef::Config[:treat_deprecation_warnings_as_errors] = false + end + it "should handle :on_platforms => :all" do node_map.set(:chef_gem, :foo, :on_platforms => :all) allow(node).to receive(:[]).with(:platform).and_return("windows") @@ -152,4 +156,3 @@ describe Chef::NodeMap do end end - diff --git a/spec/unit/platform_spec.rb b/spec/unit/platform_spec.rb index 0a4e9655d0..36325d5411 100644 --- a/spec/unit/platform_spec.rb +++ b/spec/unit/platform_spec.rb @@ -18,29 +18,6 @@ require 'spec_helper' -describe "Chef::Platform supports" do - [ - :freebsd, - :ubuntu, - :debian, - :centos, - :fedora, - :suse, - :opensuse, - :redhat, - :oracle, - :gentoo, - :arch, - :solaris, - :gcel, - :ibm_powerkvm - ].each do |platform| - it "#{platform}" do - expect(Chef::Platform.platforms).to have_key(platform) - end - end -end - describe Chef::Platform do context "while testing with fake data" do diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb index a4c0af882f..618c742998 100644 --- a/spec/unit/recipe_spec.rb +++ b/spec/unit/recipe_spec.rb @@ -122,7 +122,7 @@ describe Chef::Recipe do it "locate resource for particular platform" do ShaunTheSheep = Class.new(Chef::Resource) ShaunTheSheep.use_automatic_resource_name - ShaunTheSheep.provides :laughter, :on_platforms => ["television"] + ShaunTheSheep.provides :laughter, :platform => ["television"] node.automatic[:platform] = "television" node.automatic[:platform_version] = "123" res = recipe.laughter "timmy" @@ -144,10 +144,8 @@ describe Chef::Recipe do node.automatic[:platform] = "nbc_sports" Sounders = Class.new(Chef::Resource) Sounders.use_automatic_resource_name - Sounders.provides :football, platform: "nbc_sports" TottenhamHotspur = Class.new(Chef::Resource) TottenhamHotspur.use_automatic_resource_name - TottenhamHotspur.provides :football, platform: "nbc_sports" end after do @@ -155,9 +153,12 @@ describe Chef::Recipe do Object.send(:remove_const, :TottenhamHotspur) end - it "selects one if it is given priority" do + it "selects one if it is the last declared" do expect(Chef::Log).not_to receive(:warn) - Chef::Platform::ResourcePriorityMap.instance.send(:priority, :football, TottenhamHotspur, platform: "nbc_sports") + + Sounders.provides :football, platform: "nbc_sports" + TottenhamHotspur.provides :football, platform: "nbc_sports" + res1 = recipe.football "club world cup" expect(res1.name).to eql("club world cup") expect(res1).to be_a_kind_of(TottenhamHotspur) @@ -165,7 +166,10 @@ describe Chef::Recipe do it "selects the other one if it is given priority" do expect(Chef::Log).not_to receive(:warn) - Chef::Platform::ResourcePriorityMap.instance.send(:priority, :football, Sounders, platform: "nbc_sports") + + TottenhamHotspur.provides :football, platform: "nbc_sports" + Sounders.provides :football, platform: "nbc_sports" + res1 = recipe.football "club world cup" expect(res1.name).to eql("club world cup") expect(res1).to be_a_kind_of(Sounders) diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb index 95de09f5ce..1d20fcf604 100644 --- a/spec/unit/resource_spec.rb +++ b/spec/unit/resource_spec.rb @@ -864,7 +864,7 @@ describe Chef::Resource do @node.name("bumblebee") @node.automatic[:platform] = "autobots" @node.automatic[:platform_version] = "6.1" - klz2.provides :dinobot, :on_platforms => ['autobots'] + klz2.provides :dinobot, :platform => ['autobots'] Object.const_set('Grimlock', klz2) klz2.provides :grimlock end |