Skip to content
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

cert_get call throws an error for missing certificate #83

Closed
rishichawda opened this issue Sep 7, 2021 · 0 comments
Closed

cert_get call throws an error for missing certificate #83

rishichawda opened this issue Sep 7, 2021 · 0 comments

Comments

@rishichawda
Copy link
Member

Description

Currently cert_get call is checking if the value returned by get_cert_pem for powershell_exec! output is empty, and raises an error. However, we do have a cert_validate that achieves the same thing by calling verify_certificate which will return "Certificate not found" as string. It doesn't throw any errors, and only returns the string with the error message. This should also be eliminated since the valid? call is expected to return either true or false.

There's also a related issue on chef/chef#11994 which was reported since we use get on a Win32::Certstore instance in the windows_certificate resource. Based on how we use it, get shouldn't throw any errors.

For a use case where we want to throw an error, it should probably be one of the either:

  • Add a different method that would throw error like get! where we can have a check if cert is empty (If we want a usage where error should be thrown if we don't get the certificate)
  • Check for empty? on the return from get at top-level where such a use case is required.

In either, get shouldn't raise an error by default, which will then make the behaviour similar to the powershell command.

Proposed changes:

  • cert_get should not raise error. This raise should be moved to a different parent method like get! that would raise error when there's no certificate. This way we still get the actual output by using get and raise error only when we want to, by using the new counterpart.
  • return false on the empty? check on verify_certificate method since valid? is only supposed to return true or false. Whenever we want a use case where it should throw an error for missing certificate, the new get! method should be sufficient by throwing a "Certificate not found" error.

Gem Version

v0.6.4 (latest)

Windows Version

Windows 10

@rishichawda rishichawda changed the title cert_get call throws an error for empty certificate cert_get call throws an error for missing certificate Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant