chef / knife-windows

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

Fix trusted_cert copy script generation on windows #409

Closed coderjoe closed 7 years ago

coderjoe commented 7 years ago

Trusted certs are not being copied because Dir.glob can't handle the windows path separators being returned by PathHelper.escape_glob. This solves the problem by replacing those separators with unix style path separators.

Fixes #404

Signed-off-by: Joseph Bauser coderjoe@coderjoe.net

coderjoe commented 7 years ago

I'll address the failing travis build shortly.

mwrock commented 7 years ago

Cool! So I'm taking a closer look at this and I think the right fix is to use escape_glob_dir which we should "steal" from https://github.com/chef/chef/blob/master/chef-config/lib/chef-config/path_helper.rb#L166. Note the comments there about escape_glob.

Its unfortunate that we have to "copy/paste" here but we do that so that knife-windows can support chef 11. We try to support Major Version - 1.

I really like that you added tests here. You could either add that method in or I can add a commit to this PR either works for me.

coderjoe commented 7 years ago

I'm on the road right now @mwrock I can make changes when I get home in an hour or so or you can go ahead now if you're already spelunking. Either way works for me! :+1:

mwrock commented 7 years ago

Sounds good I'll just commit on top of yours. Thanks!!

coderjoe commented 7 years ago

If you pick it up, be aware that the travis tests may be a bit too naive. The ordering will likely be wrong on at least one of the platforms. It should probably be testing to make sure the script contains the certs, not necessarily rely on the fact that the script is line for line identical anyway.

mwrock commented 7 years ago

Yeah it looks like both windows and linux hosts will order the certs in the opposite order. Thats just greaaat. Will fix it up in a bit.

coderjoe commented 7 years ago

I'm back! @mwrock if you'd like I can merge your work back into my branch and fix up those tests. For the sake of simplicity I'll presume you've already got it covered unless you say otherwise. Cheers!

mwrock commented 7 years ago

Perfect if you can/want! Otherwise I will do it when I am back.

coderjoe commented 7 years ago

Ok, I'll give it a go! Thanks!

coderjoe commented 7 years ago

@mwrock tests are green. Do me a favor and give it a look and, presuming it passes muster, let me know if you'd like me to squash the commits down prior to merging. Thanks for your help!