Switch Node#role? to use the attributes expansion instead of the run list.

This means it understands nested roles, avoiding a common footgun.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

2 files changed, 7 insertions, 4 deletions

diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 4391118f06..f03e0ca0b1 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -288,8 +288,11 @@ class Chef
     end
 
     # Returns true if this Node expects a given role, false if not.
+    #
+    # @param role_name [String] Role to check for
+    # @return [Boolean]
     def role?(role_name)
-      run_list.include?("role[#{role_name}]")
+      Array(self[:roles]).include?(role_name)
     end
 
     def primary_runlist
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index c9f3d7d1d2..3a43a155d5 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -1160,16 +1160,16 @@ describe Chef::Node do
 
   describe "roles" do
     it "should allow you to query whether or not it has a recipe applied with role?" do
-      node.run_list << "role[sunrise]"
+      node.automatic["roles"] = %w{sunrise}
       expect(node.role?("sunrise")).to eql(true)
       expect(node.role?("not at home")).to eql(false)
     end
 
     it "should allow you to set roles with arguments" do
-      node.run_list << "role[one]"
-      node.run_list << "role[two]"
+      node.automatic["roles"] = %w{one two}
       expect(node.role?("one")).to eql(true)
       expect(node.role?("two")).to eql(true)
+      expect(node.role?("three")).to eql(false)
     end
   end