Cert: Fix role certificate parameter

[colin-pm]

The create_ca_certificate_role currently accepts a certificate parameter that can either be a PEM-formatted certificate or a path to a PEM-fromatted certificate file. The parameter is differentiated by trying to open a file with what's passed to certificate. For some systems, the lengh of a certificate is long enough that it will cause a OSError because of the path length.

The conditional was updated to actually inspect what is passed to contents. To avoid these issues moving forward, certificate will only accept a certificate string. Paths should be passed to the new certificate_file parameter. For now, passing a certificate file to certificate will generate a warning.

Signed-off-by: Colin McAllister colinmca242@gmail.com

Closes #841

[codecov]

Codecov Report

Merging #886 (67b8f8e) into develop (5c91597) will decrease coverage by 1.98%.
The diff coverage is 92.30%.

@@             Coverage Diff             @@
##           develop     #886      +/-   ##
===========================================
- Coverage    82.93%   80.95%   -1.99%     
===========================================
  Files           59       59              
  Lines         2748     2772      +24     
===========================================
- Hits          2279     2244      -35     
- Misses         469      528      +59     

Impacted Files	Coverage Δ
hvac/api/auth_methods/cert.py	91.04% <92.30%> (+1.21%)	⬆️
hvac/api/secrets_engines/transform.py	29.68% <0.00%> (-42.97%)	⬇️
hvac/api/system_backend/namespace.py	50.00% <0.00%> (-33.34%)	⬇️
hvac/api/auth_methods/mfa.py	100.00% <0.00%> (ø)
hvac/api/secrets_engines/kv.py	100.00% <0.00%> (ø)
hvac/api/secrets_engines/identity.py	94.00% <0.00%> (+0.02%)	⬆️
hvac/utils.py	77.27% <0.00%> (+0.20%)	⬆️
hvac/adapters.py	86.86% <0.00%> (+0.27%)	⬆️
hvac/api/vault_api_category.py	95.12% <0.00%> (+0.67%)	⬆️

[briantist]

Just a few questions:

• Have we decided in which version certificate will stop accepting files altogether?
• Can we report that version in the warning?
• Are we tracking that in GitHub anywhere? (we could create an issue for removing it, assign it to a v2.0.0 milestone for example)
• I don't see tests updated, are we missing tests for this method currently?

[colin-pm]

Just a few questions:

• Have we decided in which version certificate will stop accepting files altogether?

Let's do v2.0.0

• Can we report that version in the warning?

I did add documentation in the API, which will propagate into the docs. Should we also add the version support will be removed in the warning message?

• Are we tracking that in GitHub anywhere? (we could create an issue for removing it, assign it to a v2.0.0 milestone for example)

Created #914!

• I don't see tests updated, are we missing tests for this method currently?

Just added tests 😄, forgot about doing that when I initially created this PR.

[briantist]

I did add documentation in the API, which will propagate into the docs. Should we also add the version support will be removed in the warning message?

imo, yes, it's helpful to see in output, we could also add a link to #914 maybe?

• Are we tracking that in GitHub anywhere? (we could create an issue for removing it, assign it to a v2.0.0 milestone for example)

Created #914!

Thanks! I updated the title and pinned the issue to make it more visible (maybe we can come up with a different way of surfacing upcoming breaking changes but should be ok for now).