Merge pull request #759 from bensomers/glob_roles

[CHEF-4193] Enabling storage of roles in subdirectories

3 files changed, 61 insertions, 38 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 438055acd1..22fafaa4dc 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -64,6 +64,7 @@ class Chef
     class PrivateKeyMissing < RuntimeError; end
     class CannotWritePrivateKey < RuntimeError; end
     class RoleNotFound < RuntimeError; end
+    class DuplicateRole < RuntimeError; end
     class ValidationFailed < ArgumentError; end
     class InvalidPrivateKey < ArgumentError; end
     class ConfigurationError < ArgumentError; end
diff --git a/lib/chef/role.rb b/lib/chef/role.rb
index 6ad58b816d..64952c6ab2 100644
--- a/lib/chef/role.rb
+++ b/lib/chef/role.rb
@@ -231,21 +231,25 @@ class Chef
     end
 
     # Load a role from disk - prefers to load the JSON, but will happily load
-    # the raw rb files as well.
-    def self.from_disk(name, force=nil)
+    # the raw rb files as well. Can search within directories in the role_path.
+    def self.from_disk(name)
       paths = Array(Chef::Config[:role_path])
+      paths.each do |path|
+        roles_files = Dir.glob(File.join(path, "**", "**"))
+        js_files = roles_files.select { |file| file.match /#{name}\.json$/ }
+        rb_files = roles_files.select { |file| file.match /#{name}\.rb$/ }
+        if js_files.count > 1 or rb_files.count > 1
+          raise Chef::Exceptions::DuplicateRole, "Multiple roles of same type found named #{name}"
+        end
+        js_path, rb_path = js_files.first, rb_files.first
 
-      paths.each do |p|
-        js_file = File.join(p, "#{name}.json")
-        rb_file = File.join(p, "#{name}.rb")
-
-        if File.exists?(js_file) || force == "json"
+        if js_path && File.exists?(js_path)
           # from_json returns object.class => json_class in the JSON.
-          return Chef::JSONCompat.from_json(IO.read(js_file))
-        elsif File.exists?(rb_file) || force == "ruby"
+          return Chef::JSONCompat.from_json(IO.read(js_path))
+        elsif rb_path && File.exists?(rb_path)
           role = Chef::Role.new
           role.name(name)
-          role.from_file(rb_file)
+          role.from_file(rb_path)
           return role
         end
       end
diff --git a/spec/unit/role_spec.rb b/spec/unit/role_spec.rb
index 7410b3a0c6..9ff75566b4 100644
--- a/spec/unit/role_spec.rb
+++ b/spec/unit/role_spec.rb
@@ -256,26 +256,46 @@ EOR
     end
 
     it "should return a Chef::Role object from JSON" do
-      File.should_receive(:exists?).with(File.join(Chef::Config[:role_path], 'lolcat.json')).exactly(1).times.and_return(true)
-      IO.should_receive(:read).with(File.join(Chef::Config[:role_path], 'lolcat.json')).and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
+      Dir.should_receive(:glob).and_return(["#{Chef::Config[:role_path]}/memes", "#{Chef::Config[:role_path]}/memes/lolcat.json"])
+      file_path = File.join(Chef::Config[:role_path], 'memes/lolcat.json')
+      File.should_receive(:exists?).with(file_path).exactly(1).times.and_return(true)
+      IO.should_receive(:read).with(file_path).and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
       @role.should be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
     end
 
     it "should return a Chef::Role object from a Ruby DSL" do
-      File.should_receive(:exists?).with(File.join(Chef::Config[:role_path], 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join(Chef::Config[:role_path], 'lolcat.rb')).exactly(2).times.and_return(true)
-      File.should_receive(:readable?).with(File.join(Chef::Config[:role_path], 'lolcat.rb')).exactly(1).times.and_return(true)
-      IO.should_receive(:read).with(File.join(Chef::Config[:role_path], 'lolcat.rb')).and_return(ROLE_DSL)
+      Dir.should_receive(:glob).and_return(["#{Chef::Config[:role_path]}/memes", "#{Chef::Config[:role_path]}/memes/lolcat.rb"])
+      rb_path = File.join(Chef::Config[:role_path], 'memes/lolcat.rb')
+      File.should_receive(:exists?).with(rb_path).exactly(2).times.and_return(true)
+      File.should_receive(:readable?).with(rb_path).exactly(1).times.and_return(true)
+      IO.should_receive(:read).with(rb_path).and_return(ROLE_DSL)
+      @role.should be_a_kind_of(Chef::Role)
+      @role.class.from_disk("lolcat")
+    end
+
+    it "should prefer a Chef::Role Object from JSON over one from a Ruby DSL" do
+      Dir.should_receive(:glob).and_return(["#{Chef::Config[:role_path]}/memes", "#{Chef::Config[:role_path]}/memes/lolcat.json", "#{Chef::Config[:role_path]}/memes/lolcat.rb"])
+      js_path = File.join(Chef::Config[:role_path], 'memes/lolcat.json')
+      rb_path = File.join(Chef::Config[:role_path], 'memes/lolcat.rb')
+      File.should_receive(:exists?).with(js_path).exactly(1).times.and_return(true)
+      File.should_not_receive(:exists?).with(rb_path)
+      IO.should_receive(:read).with(js_path).and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
       @role.should be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
     end
 
     it "should raise an exception if the file does not exist" do
-      File.should_receive(:exists?).with(File.join(Chef::Config[:role_path], 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join(Chef::Config[:role_path], 'lolcat.rb')).exactly(1).times.and_return(false)
+      Dir.should_receive(:glob).and_return(["#{Chef::Config[:role_path]}/meme.rb"])
+      File.should_not_receive(:exists?)
       lambda {@role.class.from_disk("lolcat")}.should raise_error(Chef::Exceptions::RoleNotFound)
     end
+
+    it "should raise an exception if two files exist with the same name" do
+      Dir.should_receive(:glob).and_return(["#{Chef::Config[:role_path]}/memes/lolcat.rb", "#{Chef::Config[:role_path]}/lolcat.rb"])
+      File.should_not_receive(:exists?)
+      lambda {@role.class.from_disk("lolcat")}.should raise_error(Chef::Exceptions::DuplicateRole)
+    end
   end
 
   describe "when loading from disk and role_path is an array" do
@@ -285,46 +305,44 @@ EOR
     end
 
     it "should return a Chef::Role object from JSON" do
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.json')).exactly(1).times.and_return(true)
-      IO.should_receive(:read).with(File.join('/path1', 'lolcat.json')).and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
+      Dir.should_receive(:glob).with(File.join('/path1', '**', '**')).exactly(1).times.and_return(['/path1/lolcat.json'])
+      File.should_receive(:exists?).with('/path1/lolcat.json').exactly(1).times.and_return(true)
+      IO.should_receive(:read).with('/path1/lolcat.json').and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
       @role.should be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
     end
 
     it "should return a Chef::Role object from JSON when role is in the second path" do
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.rb')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path/path2', 'lolcat.json')).exactly(1).times.and_return(true)
-      IO.should_receive(:read).with(File.join('/path/path2', 'lolcat.json')).and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
+      Dir.should_receive(:glob).with(File.join('/path1', '**', '**')).exactly(1).times.and_return([])
+      Dir.should_receive(:glob).with(File.join('/path/path2', '**', '**')).exactly(1).times.and_return(['/path/path2/lolcat.json'])
+      File.should_receive(:exists?).with('/path/path2/lolcat.json').exactly(1).times.and_return(true)
+      IO.should_receive(:read).with('/path/path2/lolcat.json').and_return('{"name": "ceiling_cat", "json_class": "Chef::Role" }')
       @role.should be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
     end
 
     it "should return a Chef::Role object from a Ruby DSL" do
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.rb')).exactly(2).times.and_return(true)
-      File.should_receive(:readable?).with(File.join('/path1', 'lolcat.rb')).exactly(1).times.and_return(true)
-      IO.should_receive(:read).with(File.join('/path1', 'lolcat.rb')).and_return(ROLE_DSL)
+      Dir.should_receive(:glob).with(File.join('/path1', '**', '**')).exactly(1).times.and_return(['/path1/lolcat.rb'])
+      File.should_receive(:exists?).with('/path1/lolcat.rb').exactly(2).times.and_return(true)
+      File.should_receive(:readable?).with('/path1/lolcat.rb').and_return(true)
+      IO.should_receive(:read).with('/path1/lolcat.rb').exactly(1).times.and_return(ROLE_DSL)
       @role.should be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
     end
 
     it "should return a Chef::Role object from a Ruby DSL when role is in the second path" do
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.rb')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path/path2', 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path/path2', 'lolcat.rb')).exactly(2).times.and_return(true)
-      File.should_receive(:readable?).with(File.join('/path/path2', 'lolcat.rb')).exactly(1).times.and_return(true)
-      IO.should_receive(:read).with(File.join('/path/path2', 'lolcat.rb')).and_return(ROLE_DSL)
+      Dir.should_receive(:glob).with(File.join('/path1', '**', '**')).exactly(1).times.and_return([])
+      Dir.should_receive(:glob).with(File.join('/path/path2', '**', '**')).exactly(1).times.and_return(['/path/path2/lolcat.rb'])
+      File.should_receive(:exists?).with('/path/path2/lolcat.rb').exactly(2).times.and_return(true)
+      File.should_receive(:readable?).with('/path/path2/lolcat.rb').and_return(true)
+      IO.should_receive(:read).with('/path/path2/lolcat.rb').exactly(1).times.and_return(ROLE_DSL)
       @role.should be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
     end
 
     it "should raise an exception if the file does not exist" do
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path1', 'lolcat.rb')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path/path2', 'lolcat.json')).exactly(1).times.and_return(false)
-      File.should_receive(:exists?).with(File.join('/path/path2', 'lolcat.rb')).exactly(1).times.and_return(false)
+      Dir.should_receive(:glob).with(File.join('/path1', '**', '**')).exactly(1).times.and_return([])
+      Dir.should_receive(:glob).with(File.join('/path/path2', '**', '**')).exactly(1).times.and_return([])
       lambda {@role.class.from_disk("lolcat")}.should raise_error(Chef::Exceptions::RoleNotFound)
     end