-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MSYS-754] : Added validate/download actions to windows_certificate in the Windows cookbook #558
[MSYS-754] : Added validate/download actions to windows_certificate in the Windows cookbook #558
Conversation
Please before merging this PR, first merge PR: #552 due to |
a3a8fa2
to
b0e6dcf
Compare
libraries/windows_helper.rb
Outdated
store.add(cert_obj) | ||
end | ||
|
||
def delete_cert |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should just move these new helper methods right into the action class of the resource. That's the way we'll use it in chef-client itself if/when we move this resource into chef and it prevents doing requires on every chef node when we don't actually use these libraries. This will save
when '.der' | ||
out_file.puts(cert_obj.to_der) | ||
when '.cer' | ||
cert_out = powershell_out("openssl x509 -text -inform DER -in #{cert_obj.to_pem} -outform CER").stdout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this something we could do in ruby vs. shelling out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes @tas50 we can achieve that in ruby as well but this is super easy way with less amount of code also if openssl
command failed due to any reason powershell_out track error easily. Please let me know your thought about it.
f4e85b0
to
d871673
Compare
@@ -9,3 +9,4 @@ | |||
source_url 'https://github.com/chef-cookbooks/windows' | |||
issues_url 'https://github.com/chef-cookbooks/windows/issues' | |||
chef_version '>= 12.7' if respond_to?(:chef_version) | |||
gem 'win32-certstore', '= 0.1.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't need a version constraint here, win32-certstore
is new enough there aren't any 'new' features we're dependent on.
@lamont-granquist said that 'we pass all the options to bundler' here, so it would be interesting to try something like this to see if we can limit this gem install on non-windows platforms.
gem 'win32-certstore', :platforms => [ :mingw, :x64_mingw, :mswin ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @btm for bundler doc, I have updated metadata.rb in also keep it in mind.
79c9f50
to
53faa88
Compare
…r create/delete Signed-off-by: piyushawasthi <[email protected]>
Signed-off-by: piyushawasthi <[email protected]>
Signed-off-by: piyushawasthi <[email protected]>
53faa88
to
a608714
Compare
Merging. I'll fix the gem loading stuff. I need this in so I can convert the pester tests |
Description
Added verify certificate and fetch certificate actions to windows_certificate in the Windows cookbook
Verify Certificate Action
It verify certificate by thumbprint using
win32-certstore
->verify?
method and return certificate valid or not, In case no certificate found by given thumbprint it returnCertificate not found
message also it handle the exception if raise.Fetch Certificate Action
The fetch certificate action provide 2 options.
PEM
formate.cert_path