Fix bootstrapping windows-from-linux

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

5 files changed, 53 insertions, 73 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index ee25df6954..1acf2b7da2 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -60,49 +60,44 @@ module ChefConfig
     #   platform_specific_path("/etc/chef/client.pem") #=> "C:\\chef\\client.pem"
     # @param path [String] The unix path to convert to a platform specific path
     # @return [String] a platform specific path
-    def self.platform_specific_path(path)
-      path = PathHelper.cleanpath(path)
-      if ChefUtils.windows?
+    def self.platform_specific_path(path, windows: ChefUtils.windows?)
+      path = PathHelper.cleanpath(path, windows: windows)
+      if windows
         # turns \etc\chef\client.rb and \var\chef\client.rb into C:/chef/client.rb
         # Some installations will be on different drives so use the drive that
         # the expanded path to __FILE__ is found.
         drive = windows_installation_drive
         if drive && path[0] == '\\' && path.split('\\')[2] == "chef"
-          path = PathHelper.join(drive, path.split('\\', 3)[2])
+          path = PathHelper.join(drive, path.split('\\', 3)[2], windows: windows)
         end
       end
       path
     end
 
     # On *nix, /etc/chef
-    def self.etc_chef_dir(is_windows = ChefUtils.windows?)
-      path = is_windows ? c_chef_dir : PathHelper.join("/etc", ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
+    def self.etc_chef_dir(windows: ChefUtils.windows?)
+      path = windows ? c_chef_dir : PathHelper.join("/etc", ChefConfig::Dist::DIR_SUFFIX)
     end
 
     # On *nix, /var/chef
-    def self.var_chef_dir(is_windows = ChefUtils.windows?)
-      path = is_windows ? c_chef_dir : PathHelper.join("/var", ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
+    def self.var_chef_dir(windows: ChefUtils.windows?)
+      path = windows ? c_chef_dir : PathHelper.join("/var", ChefConfig::Dist::DIR_SUFFIX)
     end
 
     # On *nix, the root of /var/, used to test if we can create and write in /var/chef
-    def self.var_root_dir(is_windows = ChefUtils.windows?)
-      path = is_windows ? c_chef_dir : "/var"
-      PathHelper.cleanpath(path)
+    def self.var_root_dir(windows: ChefUtils.windows?)
+      path = windows ? c_chef_dir : "/var"
     end
 
     # On windows, C:/chef/
     def self.c_chef_dir
       drive = windows_installation_drive || "C:"
       path = PathHelper.join(drive, ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
     end
 
     def self.c_opscode_dir
       drive = windows_installation_drive || "C:"
       path = PathHelper.join(drive, ChefConfig::Dist::LEGACY_CONF_DIR, ChefConfig::Dist::DIR_SUFFIX)
-      PathHelper.cleanpath(path)
     end
 
     # the drive where Chef is installed on a windows host. This is determined
diff --git a/chef-config/lib/chef-config/path_helper.rb b/chef-config/lib/chef-config/path_helper.rb
index 19e7cce26a..b3f44d802a 100644
--- a/chef-config/lib/chef-config/path_helper.rb
+++ b/chef-config/lib/chef-config/path_helper.rb
@@ -26,14 +26,14 @@ module ChefConfig
     # Maximum characters in a standard Windows path (260 including drive letter and NUL)
     WIN_MAX_PATH = 259
 
-    def self.dirname(path)
-      if ChefUtils.windows?
+    def self.dirname(path, windows: ChefUtils.windows?)
+      if windows
         # Find the first slash, not counting trailing slashes
         end_slash = path.size
         loop do
-          slash = path.rindex(/[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator)}]/, end_slash - 1)
+          slash = path.rindex(/[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator(windows: windows))}]/, end_slash - 1)
           if !slash
-            return end_slash == path.size ? "." : path_separator
+            return end_slash == path.size ? "." : path_separator(windows: windows)
           elsif slash == end_slash - 1
             end_slash = slash
           else
@@ -47,9 +47,9 @@ module ChefConfig
 
     BACKSLASH = '\\'.freeze
 
-    def self.path_separator
-      if ChefUtils.windows?
-        File::ALT_SEPARATOR || BACKSLASH
+    def self.path_separator(windows: ChefUtils.windows?)
+      if windows
+        BACKSLASH
       else
         File::SEPARATOR
       end
@@ -60,16 +60,16 @@ module ChefConfig
     TRAILING_SLASHES_REGEX = /[#{path_separator_regex}]+$/.freeze
     LEADING_SLASHES_REGEX = /^[#{path_separator_regex}]+/.freeze
 
-    def self.join(*args)
+    def self.join(*args, windows: ChefUtils.windows?)
       args.flatten.inject do |joined_path, component|
         joined_path = joined_path.sub(TRAILING_SLASHES_REGEX, "")
         component = component.sub(LEADING_SLASHES_REGEX, "")
-        joined_path + "#{path_separator}#{component}"
+        joined_path + "#{path_separator(windows: windows)}#{component}"
       end
     end
 
-    def self.validate_path(path)
-      if ChefUtils.windows?
+    def self.validate_path(path, windows: ChefUtils.windows?)
+      if windows
         unless printable?(path)
           msg = "Path '#{path}' contains non-printable characters. Check that backslashes are escaped with another backslash (e.g. C:\\\\Windows) in double-quoted strings."
           ChefConfig.logger.error(msg)
@@ -108,14 +108,14 @@ module ChefConfig
     end
 
     # Produces a comparable path.
-    def self.canonical_path(path, add_prefix = true)
+    def self.canonical_path(path, add_prefix = true, windows: ChefUtils.windows?)
       # First remove extra separators and resolve any relative paths
       abs_path = File.absolute_path(path)
 
-      if ChefUtils.windows?
+      if windows
         # Add the \\?\ API prefix on Windows unless add_prefix is false
         # Downcase on Windows where paths are still case-insensitive
-        abs_path.gsub!(::File::SEPARATOR, path_separator)
+        abs_path.gsub!(::File::SEPARATOR, path_separator(windows: windows))
         if add_prefix && abs_path !~ /^\\\\?\\/
           abs_path.insert(0, "\\\\?\\")
         end
@@ -137,25 +137,25 @@ module ChefConfig
     #
     # Generally, if the user isn't going to be seeing it, you should be
     # using Pathname#cleanpath instead of this function.
-    def self.cleanpath(path)
+    def self.cleanpath(path, windows: ChefUtils.windows?)
       path = Pathname.new(path).cleanpath.to_s
       # ensure all forward slashes are backslashes
-      if ChefUtils.windows?
-        path = path.gsub(File::SEPARATOR, path_separator)
+      if windows
+        path = path.gsub(File::SEPARATOR, path_separator(windows: windows))
       end
       path
     end
 
-    def self.paths_eql?(path1, path2)
-      canonical_path(path1) == canonical_path(path2)
+    def self.paths_eql?(path1, path2, windows: ChefUtils.windows?)
+      canonical_path(path1, windows: windows) == canonical_path(path2, windows: windows)
     end
 
     # @deprecated this method is deprecated. Please use escape_glob_dirs
     # Paths which may contain glob-reserved characters need
     # to be escaped before globbing can be done.
     # http://stackoverflow.com/questions/14127343
-    def self.escape_glob(*parts)
-      path = cleanpath(join(*parts))
+    def self.escape_glob(*parts, windows: ChefUtils.windows?)
+      path = cleanpath(join(*parts, windows: windows), windows: windows)
       path.gsub(/[\\\{\}\[\]\*\?]/) { |x| "\\" + x }
     end
 
@@ -166,8 +166,8 @@ module ChefConfig
       path.gsub(/[\\\{\}\[\]\*\?]/) { |x| "\\" + x }
     end
 
-    def self.relative_path_from(from, to)
-      Pathname.new(cleanpath(to)).relative_path_from(Pathname.new(cleanpath(from)))
+    def self.relative_path_from(from, to, windows: ChefUtils.windows?)
+      Pathname.new(cleanpath(to, windows: windows)).relative_path_from(Pathname.new(cleanpath(from, windows: windows)))
     end
 
     # Set the project-specific home directory environment variable.
@@ -213,11 +213,11 @@ module ChefConfig
     #
     # The return is a list of all the returned values from each block invocation or a list of paths
     # if no block is provided.
-    def self.all_homes(*args)
+    def self.all_homes(*args, windows: ChefUtils.windows?)
       paths = []
       paths << ENV[@@per_tool_home_environment] if defined?(@@per_tool_home_environment) && @@per_tool_home_environment && ENV[@@per_tool_home_environment]
       paths << ENV["CHEF_HOME"] if ENV["CHEF_HOME"]
-      if ChefUtils.windows?
+      if windows
         # By default, Ruby uses the the following environment variables to determine Dir.home:
         # HOME
         # HOMEDRIVE HOMEPATH
@@ -246,7 +246,7 @@ module ChefConfig
       # Note: Maybe this is a bad idea on some unixy systems where \ might be a valid character depending on
       # the particular brand of kool-aid you consume.  This code assumes that \ and / are both
       # path separators on any system being used.
-      paths = paths.map { |home_path| home_path.gsub(path_separator, ::File::SEPARATOR) if home_path }
+      paths = paths.map { |home_path| home_path.gsub(path_separator(windows: windows), ::File::SEPARATOR) if home_path }
 
       # Filter out duplicate paths and paths that don't exist.
       valid_paths = paths.select { |home_path| home_path && Dir.exist?(home_path.force_encoding("utf-8")) }
diff --git a/chef-config/spec/unit/path_helper_spec.rb b/chef-config/spec/unit/path_helper_spec.rb
index f416bb1826..60bc20d3f3 100644
--- a/chef-config/spec/unit/path_helper_spec.rb
+++ b/chef-config/spec/unit/path_helper_spec.rb
@@ -232,14 +232,14 @@ RSpec.describe ChefConfig::PathHelper do
 
   describe "paths_eql?" do
     it "returns true if the paths are the same" do
-      allow(path_helper).to receive(:canonical_path).with("bandit").and_return("c:/bandit/bandit")
-      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      allow(path_helper).to receive(:canonical_path).with("bandit", windows: ChefUtils.windows?).and_return("c:/bandit/bandit")
+      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit", windows: ChefUtils.windows?).and_return("c:/bandit/bandit")
       expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_truthy
     end
 
     it "returns false if the paths are different" do
-      allow(path_helper).to receive(:canonical_path).with("bandit").and_return("c:/Bo/Bandit")
-      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      allow(path_helper).to receive(:canonical_path).with("bandit", windows: ChefUtils.windows?).and_return("c:/Bo/Bandit")
+      allow(path_helper).to receive(:canonical_path).with("../bandit/bandit", windows: ChefUtils.windows?).and_return("c:/bandit/bandit")
       expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_falsey
     end
   end
@@ -259,8 +259,6 @@ RSpec.describe ChefConfig::PathHelper do
                        else
                          "this/\\*path/\\[needs\\]/escaping\\?"
                        end
-        expect(path_helper).to receive(:join).with(*args).and_call_original
-        expect(path_helper).to receive(:cleanpath).and_call_original
         expect(path_helper.escape_glob(*args)).to eq(escaped_path)
       end
     end
diff --git a/lib/chef/knife/core/windows_bootstrap_context.rb b/lib/chef/knife/core/windows_bootstrap_context.rb
index 8469879022..b7e72151f2 100644
--- a/lib/chef/knife/core/windows_bootstrap_context.rb
+++ b/lib/chef/knife/core/windows_bootstrap_context.rb
@@ -41,20 +41,6 @@ class Chef
           super(config, run_list, chef_config, secret)
         end
 
-        # This is a duplicate of ChefConfig::PathHelper.cleanpath, however
-        # this presumes Windows so we can avoid changing the method definitions
-        # across Chef, ChefConfig, and ChefUtils for the circumstance where
-        # the methods are being run for a system other than the one Ruby is
-        # executing on.
-        #
-        # We only need to cleanpath the paths that we are passing to cmd.exe,
-        # anything written to a configuration file or passed as an argument
-        # will be interpreted by ruby later and do the right thing.
-        def cleanpath(path)
-          path = Pathname.new(path).cleanpath.to_s
-          path.gsub(File::SEPARATOR, '\\')
-        end
-
         def validation_key
           if File.exist?(File.expand_path(chef_config[:validation_key]))
             IO.read(File.expand_path(chef_config[:validation_key]))
@@ -75,9 +61,9 @@ class Chef
           client_rb = <<~CONFIG
             chef_server_url  "#{chef_config[:chef_server_url]}"
             validation_client_name "#{chef_config[:validation_client_name]}"
-            file_cache_path   "#{ChefConfig::Config.var_chef_dir(true)}/cache"
-            file_backup_path  "#{ChefConfig::Config.var_chef_dir(true)}/backup"
-            cache_options     ({:path => "#{ChefConfig::Config.etc_chef_dir(true)}/cache/checksums", :skip_expires => true})
+            file_cache_path   "#{ChefConfig::Config.var_chef_dir(windows: true)}/cache"
+            file_backup_path  "#{ChefConfig::Config.var_chef_dir(windows: true)}/backup"
+            cache_options     ({:path => "#{ChefConfig::Config.etc_chef_dir(windows: true)}/cache/checksums", :skip_expires => true})
           CONFIG
 
           unless chef_config[:chef_license].nil?
@@ -90,8 +76,8 @@ class Chef
             client_rb << "# Using default node name (fqdn)\n"
           end
 
-          if chef_config[:config_log_level]
-            client_rb << %Q{log_level :#{chef_config[:config_log_level]}\n}
+          if config[:config_log_level]
+            client_rb << %Q{log_level :#{config[:config_log_level]}\n}
           else
             client_rb << "log_level        :auto\n"
           end
@@ -140,11 +126,11 @@ class Chef
           end
 
           if config[:secret]
-            client_rb << %Q{encrypted_data_bag_secret "#{ChefConfig::Config.etc_chef_dir(true)}/encrypted_data_bag_secret"\n}
+            client_rb << %Q{encrypted_data_bag_secret "#{ChefConfig::Config.etc_chef_dir(windows: true)}/encrypted_data_bag_secret"\n}
           end
 
           unless trusted_certs_script.empty?
-            client_rb << %Q{trusted_certs_dir "#{ChefConfig::Config.etc_chef_dir(true)}/trusted_certs"\n}
+            client_rb << %Q{trusted_certs_dir "#{ChefConfig::Config.etc_chef_dir(windows: true)}/trusted_certs"\n}
           end
 
           if chef_config[:fips]
@@ -173,9 +159,10 @@ class Chef
         end
 
         def start_chef
+          c_opscode_dir = ChefConfig::PathHelper.cleanpath(ChefConfig::Config.c_opscode_dir, windows: true)
           bootstrap_environment_option = bootstrap_environment.nil? ? "" : " -E #{bootstrap_environment}"
-          start_chef = "SET \"PATH=%SystemRoot%\\system32;%SystemRoot%;%SystemRoot%\\System32\\Wbem;%SYSTEMROOT%\\System32\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;#{ChefConfig::Config.c_opscode_dir}\\bin;#{ChefConfig::Config.c_opscode_dir}\\embedded\\bin\;%PATH%\"\n"
-          start_chef << "#{Chef::Dist::CLIENT} -c #{ChefConfig::Config.etc_chef_dir(true)}/client.rb -j #{ChefConfig::Config.etc_chef_dir(true)}/first-boot.json#{bootstrap_environment_option}\n"
+          start_chef = "SET \"PATH=%SYSTEM32%;%SystemRoot%;%SYSTEM32%\\Wbem;%SYSTEM32%\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;#{c_opscode_dir}\\bin;#{c_opscode_dir}\\embedded\\bin\;%PATH%\"\n"
+          start_chef << "#{Chef::Dist::CLIENT} -c c:/chef/client.rb -j #{ChefConfig::Config.etc_chef_dir(windows: true)}/first-boot.json#{bootstrap_environment_option}\n"
         end
 
         def win_wget
@@ -276,7 +263,7 @@ class Chef
         end
 
         def bootstrap_directory
-          cleanpath(ChefConfig::Config.etc_chef_dir(true))
+          ChefConfig::Config.etc_chef_dir(windows: true)
         end
 
         def local_download_path
diff --git a/spec/unit/knife/core/windows_bootstrap_context_spec.rb b/spec/unit/knife/core/windows_bootstrap_context_spec.rb
index 6b041c5311..77e16f1a4a 100644
--- a/spec/unit/knife/core/windows_bootstrap_context_spec.rb
+++ b/spec/unit/knife/core/windows_bootstrap_context_spec.rb
@@ -165,7 +165,7 @@ describe Chef::Knife::Core::WindowsBootstrapContext do
         echo.file_backup_path  "C:/chef/backup"
         echo.cache_options     ^({:path =^> "C:/chef/cache/checksums", :skip_expires =^> true}^)
         echo.# Using default node name ^(fqdn^)
-        echo.log_level :info
+        echo.log_level        :auto
         echo.log_location       STDOUT
       EXPECTED
       expect(bootstrap_context.config_content).to eq expected
@@ -183,7 +183,7 @@ describe Chef::Knife::Core::WindowsBootstrapContext do
 
   describe "#start_chef" do
     it "returns the expected string" do
-      expect(bootstrap_context.start_chef).to match(%r{SET \"PATH=%SystemRoot%\\system32;%SystemRoot%;%SystemRoot%\\System32\\Wbem;%SYSTEMROOT%\\System32\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;C:\/opscode\/chef\\bin;C:\/opscode\/chef\\embedded\\bin\;%PATH%\"\n})
+      expect(bootstrap_context.start_chef).to match(/SET "PATH=%SYSTEM32%;%SystemRoot%;%SYSTEM32%\\Wbem;%SYSTEM32%\\WindowsPowerShell\\v1.0\\;C:\\ruby\\bin;C:\\opscode\\chef\\bin;C:\\opscode\\chef\\embedded\\bin;%PATH%"/)
     end
   end