CHEF-4487: support chef_repo_path in all knife commands

6 files changed, 138 insertions, 105 deletions

diff --git a/chef.gemspec b/chef.gemspec
index 0be91849c7..a21eae5982 100644
--- a/chef.gemspec
+++ b/chef.gemspec
@@ -12,7 +12,7 @@ Gem::Specification.new do |s|
   s.email = "adam@opscode.com"
   s.homepage = "http://wiki.opscode.com/display/chef"
 
-  s.add_dependency "mixlib-config", ">= 1.1.2"
+  s.add_dependency "mixlib-config", ">= 2.0.0.rc.1"
   s.add_dependency "mixlib-cli", "~> 1.3.0"
   s.add_dependency "mixlib-log", ">= 1.3.0"
   s.add_dependency "mixlib-authentication", ">= 1.3.0"
diff --git a/lib/chef/chef_fs/config.rb b/lib/chef/chef_fs/config.rb
index ab4cea89f2..6a492dbcc5 100644
--- a/lib/chef/chef_fs/config.rb
+++ b/lib/chef/chef_fs/config.rb
@@ -28,10 +28,16 @@ class Chef
       def initialize(chef_config = Chef::Config, cwd = Dir.pwd)
         @chef_config = chef_config
         @cwd = cwd
-        configure_repo_paths
-      end
 
-      PATH_VARIABLES = %w(acl_path client_path cookbook_path container_path data_bag_path environment_path group_path node_path role_path user_path)
+        # Default to getting *everything* from the server.
+        if !@chef_config[:repo_mode]
+          if @chef_config[:chef_server_url] =~ /\/+organizations\/.+/
+            @chef_config[:repo_mode] = 'hosted_everything'
+          else
+            @chef_config[:repo_mode] = 'everything'
+          end
+        end
+      end
 
       def chef_fs
         @chef_fs ||= create_chef_fs
@@ -125,19 +131,10 @@ class Chef
         server_path
       end
 
-      def require_chef_repo_path
-        if !@chef_config[:chef_repo_path]
-          Chef::Log.error("Must specify either chef_repo_path or cookbook_path in Chef config file")
-          exit(1)
-        end
-      end
-
       private
 
       def object_paths
         @object_paths ||= begin
-          require_chef_repo_path
-
           result = {}
           case @chef_config[:repo_mode]
           when 'static'
@@ -155,51 +152,6 @@ class Chef
           result
         end
       end
-
-      def configure_repo_paths
-        # Smooth out some (for now) inappropriate defaults set by Chef
-        if @chef_config[:cookbook_path] == [ @chef_config.platform_specific_path("/var/chef/cookbooks"),
-                      @chef_config.platform_specific_path("/var/chef/site-cookbooks") ]
-          @chef_config[:cookbook_path] = nil
-        end
-        if @chef_config[:data_bag_path] == @chef_config.platform_specific_path('/var/chef/data_bags')
-          @chef_config[:data_bag_path] = nil
-        end
-        if @chef_config[:node_path] == '/var/chef/node'
-          @chef_config[:node_path] = nil
-        end
-        if @chef_config[:role_path] == @chef_config.platform_specific_path('/var/chef/roles')
-          @chef_config[:role_path] = nil
-        end
-
-        # Infer chef_repo_path from cookbook_path if not speciifed
-        if !@chef_config[:chef_repo_path]
-          if @chef_config[:cookbook_path]
-            @chef_config[:chef_repo_path] = Array(@chef_config[:cookbook_path]).flatten.map { |path| File.expand_path('..', path) }
-          end
-        end
-
-        # Default to getting *everything* from the server.
-        if !@chef_config[:repo_mode]
-          if @chef_config[:chef_server_url] =~ /\/+organizations\/.+/
-            @chef_config[:repo_mode] = 'hosted_everything'
-          else
-            @chef_config[:repo_mode] = 'everything'
-          end
-        end
-
-        # Infer any *_path variables that are not specified
-        if @chef_config[:chef_repo_path]
-          PATH_VARIABLES.each do |variable_name|
-            chef_repo_paths = Array(@chef_config[:chef_repo_path]).flatten
-            variable = variable_name.to_sym
-            if !@chef_config[variable]
-              # cookbook_path -> cookbooks
-              @chef_config[variable] = chef_repo_paths.map { |path| File.join(path, "#{variable_name[0..-6]}s") }
-            end
-          end
-        end
-      end
     end
   end
 end
diff --git a/lib/chef/chef_fs/knife.rb b/lib/chef/chef_fs/knife.rb
index 5900c29f61..68bfb70e34 100644
--- a/lib/chef/chef_fs/knife.rb
+++ b/lib/chef/chef_fs/knife.rb
@@ -60,7 +60,7 @@ class Chef
         # --chef-repo-path overrides all other paths
         if config[:chef_repo_path]
           Chef::Config[:chef_repo_path] = config[:chef_repo_path]
-          Chef::ChefFS::Config::PATH_VARIABLES.each do |variable_name|
+          Chef::Config::PATH_VARIABLES.each do |variable_name|
             Chef::Config[variable_name.to_sym] = chef_repo_paths.map { |path| File.join(path, "#{variable_name[0..-6]}s") }
           end
         end
@@ -96,7 +96,6 @@ class Chef
         args.map do |arg|
           if !@chef_fs_config.base_path && !Chef::ChefFS::PathUtils.is_absolute?(arg)
             # Check if chef repo path is specified to give a better error message
-            @chef_fs_config.require_chef_repo_path
             ui.error("Attempt to use relative path '#{arg}' when current directory is outside the repository path")
             exit(1)
           end
diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index e3211d232e..6fcc5b9f7b 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -29,6 +29,8 @@ class Chef
 
     extend Mixlib::Config
 
+    config_strict_mode false
+
     # Manages the chef secret session key
     # === Returns
     # <newkey>:: A new or retrieved session key
@@ -50,6 +52,14 @@ class Chef
       configuration.inspect
     end
 
+    def self.platform_path_separator
+      if RUBY_PLATFORM =~ /mswin|mingw|windows/
+        File::ALT_SEPARATOR || '\\'
+      else
+        File::SEPARATOR
+      end
+    end
+
     def self.platform_specific_path(path)
       if RUBY_PLATFORM =~ /mswin|mingw|windows/
         # turns /etc/chef/client.rb into C:/chef/client.rb
@@ -108,10 +118,89 @@ class Chef
         rescue Errno::ENOENT
           raise Chef::Exceptions::ConfigurationError, "Failed to open or create log file at #{location.to_str}"
         end
-          f
+        f
+      end
+    end
+
+    # The root where all local chef object data is stored.  cookbooks, data bags,
+    # environments are all assumed to be in separate directories under this.
+    # chef-solo uses these directories for input data.  knife commands
+    # that upload or download files (such as knife upload, knife role from file,
+    # etc.) work.
+    default :chef_repo_path do
+      if self.configuration[:cookbook_path]
+        if self.configuration[:cookbook_path].kind_of?(String)
+          File.expand_path('..', self.configuration[:cookbook_path])
+        else
+          self.configuration[:cookbook_path].map do |path|
+            File.expand_path('..', path)
+          end
+        end
+      else
+        platform_specific_path("/var/chef")
+      end
+    end
+
+    def self.derive_path_from_chef_repo_path(child_path)
+      if chef_repo_path.kind_of?(String)
+        "#{chef_repo_path}#{platform_path_separator}#{child_path}"
+      else
+        chef_repo_path.map { |path| "#{path}#{platform_path_separator}#{child_path}"}
+      end
+    end
+
+    # Location of acls on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/acls.
+    # Only applies to Enterprise Chef commands.
+    default(:acl_path) { derive_path_from_chef_repo_path('acls') }
+
+    # Location of clients on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/acls.
+    default(:client_path) { derive_path_from_chef_repo_path('clients') }
+
+    # Location of cookbooks on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/cookbooks.  If chef_repo_path
+    # is not specified, this is set to [/var/chef/cookbooks, /var/chef/site-cookbooks]).
+    default(:cookbook_path) do
+      if self.configuration[:chef_repo_path]
+        derive_path_from_chef_repo_path('cookbooks')
+      else
+        Array(derive_path_from_chef_repo_path('cookbooks')).flatten + 
+          Array(derive_path_from_chef_repo_path('site-cookbooks')).flatten
       end
     end
 
+    # Location of containers on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/containers.
+    # Only applies to Enterprise Chef commands.
+    default(:container_path) { derive_path_from_chef_repo_path('containers') }
+
+    # Location of data bags on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/data_bags.
+    default(:data_bag_path) { derive_path_from_chef_repo_path('data_bags') }
+
+    # Location of environments on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/environments.
+    default(:environment_path) { derive_path_from_chef_repo_path('environments') }
+
+    # Location of groups on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/groups.
+    # Only applies to Enterprise Chef commands.
+    default(:group_path) { derive_path_from_chef_repo_path('groups') }
+
+    # Location of nodes on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/nodes.
+    default(:node_path) { derive_path_from_chef_repo_path('nodes') }
+
+    # Location of roles on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/roles.
+    default(:role_path) { derive_path_from_chef_repo_path('roles') }
+
+    # Location of users on disk. String or array of strings.
+    # Defaults to <chef_repo_path>/users.
+    # Does not apply to Enterprise Chef commands.
+    default(:user_path) { derive_path_from_chef_repo_path('users') }
+
     # Turn on "path sanity" by default. See also: http://wiki.opscode.com/display/chef/User+Environment+PATH+Sanity
     enforce_path_sanity(true)
 
@@ -121,19 +210,17 @@ class Chef
     # The number of times the client should retry when registering with the server
     client_registration_retries 5
 
-    # Where the cookbooks are located. Meaning is somewhat context dependent between
-    # knife, chef-client, and chef-solo.
-    cookbook_path [ platform_specific_path("/var/chef/cookbooks"),
-                    platform_specific_path("/var/chef/site-cookbooks") ]
-
     # An array of paths to search for knife exec scripts if they aren't in the current directory
     script_path []
 
     # Where cookbook files are stored on the server (by content checksum)
-    checksum_path "/var/chef/checksums"
+    checksum_path '/var/chef/checksums'
 
     # Where chef's cache files should be stored
-    file_cache_path platform_specific_path("/var/chef/cache")
+    file_cache_path platform_specific_path('/var/chef/cache')
+
+    # Where backups of chef-managed files should go
+    file_backup_path platform_specific_path('/var/chef/backup')
 
     # By default, chef-client (or solo) creates a lockfile in
     # `file_cache_path`/chef-client-running.pid
@@ -144,9 +231,6 @@ class Chef
     # '/tmp/chef-client-running.pid'
     lockfile nil
 
-    # Where backups of chef-managed files should go
-    file_backup_path platform_specific_path("/var/chef/backup")
-
     ## Daemonization Settings ##
     # What user should Chef run as?
     user nil
@@ -206,13 +290,6 @@ class Chef
     ssl_ca_path nil
     ssl_ca_file nil
 
-    # Where should chef-solo look for role files?
-    role_path platform_specific_path("/var/chef/roles")
-
-    data_bag_path platform_specific_path("/var/chef/data_bags")
-
-    environment_path platform_specific_path("/var/chef/environments")
-
     # Where should chef-solo download recipes from?
     recipe_url nil
 
diff --git a/spec/integration/knife/chef_repo_path_spec.rb b/spec/integration/knife/chef_repo_path_spec.rb
index 11989933a1..1825c5a00a 100644
--- a/spec/integration/knife/chef_repo_path_spec.rb
+++ b/spec/integration/knife/chef_repo_path_spec.rb
@@ -171,7 +171,7 @@ EOM
       context 'when only chef_repo_path is set to its alternate' do
         before :each do
           %w(client cookbook data_bag environment node role user).each do |object_name|
-            Chef::Config["#{object_name}_path".to_sym] = nil
+            Chef::Config.delete("#{object_name}_path".to_sym)
           end
           Chef::Config.chef_repo_path = File.join(Chef::Config.chef_repo_path, 'chef_repo2')
         end
@@ -439,7 +439,7 @@ EOM
       context 'when when chef_repo_path is set to both places and no other _path is set' do
         before :each do
           %w(client cookbook data_bag environment node role user).each do |object_name|
-            Chef::Config["#{object_name}_path".to_sym] = nil
+            Chef::Config.delete("#{object_name}_path".to_sym)
           end
           Chef::Config.chef_repo_path = [
             Chef::Config.chef_repo_path,
@@ -541,10 +541,10 @@ EOM
       context 'when cookbook_path is set and nothing else' do
         before :each do
           %w(client data_bag environment node role user).each do |object_name|
-            Chef::Config["#{object_name}_path".to_sym] = nil
+            Chef::Config.delete("#{object_name}_path".to_sym)
           end
-          Chef::Config.cookbook_path = File.join(Chef::Config.chef_repo_path, 'chef_repo2', 'cookbooks')
-          Chef::Config.chef_repo_path = nil
+          Chef::Config.delete(:chef_repo_path)
+          Chef::Config.cookbook_path = File.join(@repository_dir, 'chef_repo2', 'cookbooks')
         end
 
         context 'when cwd is at the top level' do
@@ -599,13 +599,13 @@ EOM
       context 'when cookbook_path is set to multiple places and nothing else is set' do
         before :each do
           %w(client data_bag environment node role user).each do |object_name|
-            Chef::Config["#{object_name}_path".to_sym] = nil
+            Chef::Config.delete("#{object_name}_path".to_sym)
           end
+          Chef::Config.delete(:chef_repo_path)
           Chef::Config.cookbook_path = [
-            File.join(Chef::Config.chef_repo_path, 'cookbooks'),
-            File.join(Chef::Config.chef_repo_path, 'chef_repo2', 'cookbooks')
+            File.join(@repository_dir, 'cookbooks'),
+            File.join(@repository_dir, 'chef_repo2', 'cookbooks')
           ]
-          Chef::Config.chef_repo_path = nil
         end
 
         context 'when cwd is at the top level' do
@@ -702,7 +702,7 @@ EOM
       context 'when data_bag_path and chef_repo_path are set, and nothing else' do
         before :each do
           %w(client cookbook  environment node role user).each do |object_name|
-            Chef::Config["#{object_name}_path".to_sym] = nil
+            Chef::Config.delete("#{object_name}_path".to_sym)
           end
           Chef::Config.data_bag_path = File.join(Chef::Config.chef_repo_path, 'data_bags')
           Chef::Config.chef_repo_path = File.join(Chef::Config.chef_repo_path, 'chef_repo2')
@@ -760,24 +760,34 @@ EOM
       context 'when data_bag_path is set and nothing else' do
         before :each do
           %w(client cookbook  environment node role user).each do |object_name|
-            Chef::Config["#{object_name}_path".to_sym] = nil
+            Chef::Config.delete("#{object_name}_path".to_sym)
           end
-          Chef::Config.data_bag_path = File.join(Chef::Config.chef_repo_path, 'data_bags')
-          Chef::Config.chef_repo_path = nil
+          Chef::Config.delete(:chef_repo_path)
+          Chef::Config.data_bag_path = File.join(@repository_dir, 'data_bags')
         end
 
-        it 'knife list --local -Rfp / fails' do
-          knife('list --local -Rfp /').should_fail("ERROR: Must specify either chef_repo_path or cookbook_path in Chef config file\n")
+        it 'knife list --local -Rfp / lists data bags' do
+          knife('list --local -Rfp /').should_succeed <<EOM
+/data_bags/
+/data_bags/bag/
+/data_bags/bag/item.json
+EOM
         end
 
-        it 'knife list --local -Rfp /data_bags fails' do
-          knife('list --local -Rfp /data_bags').should_fail("ERROR: Must specify either chef_repo_path or cookbook_path in Chef config file\n")
+        it 'knife list --local -Rfp /data_bags lists data bags' do
+          knife('list --local -Rfp /data_bags').should_succeed <<EOM
+/data_bags/bag/
+/data_bags/bag/item.json
+EOM
         end
 
         context 'when cwd is inside the data_bags directory' do
           cwd 'data_bags'
-          it 'knife list --local -Rfp fails' do
-            knife('list --local -Rfp').should_fail("ERROR: Must specify either chef_repo_path or cookbook_path in Chef config file\n")
+          it 'knife list --local -Rfp lists data bags' do
+            knife('list --local -Rfp').should_succeed <<EOM
+bag/
+bag/item.json
+EOM
           end
         end
       end
diff --git a/spec/support/shared/integration/integration_helper.rb b/spec/support/shared/integration/integration_helper.rb
index b7b377ff6a..359072a197 100644
--- a/spec/support/shared/integration/integration_helper.rb
+++ b/spec/support/shared/integration/integration_helper.rb
@@ -46,12 +46,9 @@ module IntegrationSupport
       before :each do
         raise "Can only create one directory per test" if @repository_dir
         @repository_dir = Dir.mktmpdir('chef_repo')
-        @old_chef_repo_path = Chef::Config.chef_repo_path
-        @old_paths = {}
         Chef::Config.chef_repo_path = @repository_dir
         %w(client cookbook data_bag environment node role user).each do |object_name|
-          @old_paths[object_name] = Chef::Config["#{object_name}_path".to_sym]
-          Chef::Config["#{object_name}_path".to_sym] = nil
+          Chef::Config.delete("#{object_name}_path".to_sym)
         end
       end
 
@@ -59,13 +56,11 @@ module IntegrationSupport
         if @repository_dir
           begin
             %w(client cookbook data_bag environment node role user).each do |object_name|
-              Chef::Config["#{object_name}_path".to_sym] = @old_paths[object_name]
+              Chef::Config.delete("#{object_name}_path".to_sym)
             end
-            Chef::Config.chef_repo_path = @old_chef_repo_path
+            Chef::Config.delete(:chef_repo_path)
             FileUtils.remove_entry_secure(@repository_dir)
           ensure
-            @old_chef_repo_path = nil
-            @old_paths = nil
             @repository_dir = nil
           end
         end