summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/chef/node_map.rb61
-rw-r--r--spec/unit/node_map_spec.rb5
-rw-r--r--spec/unit/platform_spec.rb23
-rw-r--r--spec/unit/recipe_spec.rb16
-rw-r--r--spec/unit/resource_spec.rb2
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