chef / knife-windows

Plugin for Chef's knife tool for working with Windows nodes
Apache License 2.0
152 stars 110 forks source link

bootstrap windows does not copy contents of trusted_certs from a Windows workstation #404

Closed codereflection closed 7 years ago

codereflection commented 8 years ago

When bootstrapping a node from a Windows workstation, using knife-windows 1.7.0, the contents of the workstation's trusted_certs folder are not copied to the trusted_certs folder on the node due to a bug in the trusted_certs_content method. I have verified that this does work properly on a Mac. The issue is that Dir.glob does not take backslashes, yet PathHelper.escape_glob(@chef_config[:trusted_certs_dir]) returns double backslashes on a Windows machine.

The existing trusted_certs_content method: https://github.com/chef/knife-windows/blob/master/lib/chef/knife/core/windows_bootstrap_context.rb

        # Returns a string for copying the trusted certificates on the workstation to the system being bootstrapped
        # This string should contain both the commands necessary to both create the files, as well as their content
        def trusted_certs_content
          content = ""
          if @chef_config[:trusted_certs_dir]
            Dir.glob(File.join(PathHelper.escape_glob(@chef_config[:trusted_certs_dir]), "*.{crt,pem}")).each do |cert|
              content << "> #{bootstrap_directory}/trusted_certs/#{File.basename(cert)} (\n" +
                         escape_and_echo(IO.read(File.expand_path(cert))) + "\n)\n"
            end
          end
          content
        end

The fix is simple. Just add .gsub('\\','/') so we do this instead:

        # Returns a string for copying the trusted certificates on the workstation to the system being bootstrapped
        # This string should contain both the commands necessary to both create the files, as well as their content
        def trusted_certs_content
          content = ""
          if @chef_config[:trusted_certs_dir]
            Dir.glob(File.join(PathHelper.escape_glob(@chef_config[:trusted_certs_dir]), "*.{crt,pem}").gsub('\\','/')).each do |cert|
              puts "cert: '#{File.basename(cert)}'"
              content << "> #{bootstrap_directory}/trusted_certs/#{File.basename(cert)} (\n" +
                         escape_and_echo(IO.read(File.expand_path(cert))) + "\n)\n"
            end
          end
          content
        end
jdpc02 commented 7 years ago

Any chance this can be incorporated in the next build? Encountered this issue this week but it was working previously. I think it was after I updated to chefDK 0.19.6 from an older 0.12 version. I have temporarily instructed my windows admins to use this fix for our currently rollout.

Roviluca commented 7 years ago

I have the same issue but that 0.15.16 was my working version before update.

codereflection commented 7 years ago

I just realized that this fix will probably break the process on Mac / *nix systems. Needs to be tested out. I'll try and do that today.

codereflection commented 7 years ago

Confirmed that the fix works on Mac, because gsub is looking for backslashes, which PathHelper.escape_glob only returns on Windows machines. So the fix using .gsub('\\','/') seems good on both OS's.

mwrock commented 7 years ago

Thanks for all this !! This will likely get pulled in faster if submitted via PR. No obligation of course but it helps move it along more quickly.

codereflection commented 7 years ago

Can do, but wouldn't this require the CLA have to been signed? If not, you'll have a PR for this today.

mwrock commented 7 years ago

We've moved away from CLA to to DCO: https://blog.chef.io/2016/09/19/introducing-developer-certificate-of-origin/

codereflection commented 7 years ago

Sweet, PR incoming today. Thanks @mwrock!

coderjoe commented 7 years ago

I started hacking on this yesterday when I discovered it independent of this issue. I think I have PR ready code I can throw up if it's welcome.

mwrock commented 7 years ago

it is absolutely welcome!

coderjoe commented 7 years ago

It's in the hands of the build queue gods now. Let me know if there's anything I need to change or anything I can do to help get this pushed through. I work at a windows shop and this is confusing all of our employees on relatively new ChefDK versions (after around 0.15.x I believe)

I'm generally available to respond business hours US Eastern, but I'll be up a little late tonight. 😪 💤

codereflection commented 7 years ago

Awesome! I meant to do this yesterday, but work happened.

mwrock commented 7 years ago

Just released v1.7.1 with the fix. Thanks everyone for reporting and helping with a fix!

coderjoe commented 7 years ago

Just installed it at work and couldn't be happier. Thanks for the quick turn around everybody!