Fixes #11755: Validate absolute path for custom certificates #58
Conversation
ehelms
force-pushed
the
remove-validate-present
branch
from
5be33fe to
6e8b4cb
Compare
|
@iNecas would you mind reviewing this change? |
| @@ -112,7 +112,7 @@ | |||
| ) inherits certs::params { | |||
|
|
|||
| if $server_cert { | |||
| validate_file_exists($server_cert, $server_cert_req, $server_key, $server_ca_cert) | |||
| validate_absolute_path([$server_cert, $server_cert_req, $server_key, $server_ca_cert]) | |||
There was a problem hiding this comment.
This is not the same: we need to check that the file really exists. On the other hand, even more checks happen now in https://github.com/Katello/katello-installer/blob/master/hooks/pre/20-certs_update.rb#L62, so I don't have that much issues then
ehelms
force-pushed
the
remove-validate-present
branch
from
6e8b4cb to
6bdb653
Compare
ehelms
changed the title
|
@iNecas I re-purposed this PR to instead add absolute path validation for custom certificates since this is a common pitfall. |
|
Can there be a case, where the |
|
I don't follow the question. If the absolute path check fails, then it
|
|
I mean if it would not pass the |
|
One situation is where you have specified these parameters without an absolute path, and you re-run it from a different location. This is probably less a problem with normal puppet workflows, but when used with something like kafo, if you re-run an installer it will complain about not finding these certs since they are not absolute. |
|
Ok, if this is the case, I agree this will fix it, given it will properly report the error message so that the user knows to use absolute path instead |
|
@iNecas is that an ACK or are you asking for something else in addition? |
|
Sry for not being explicit, ACK :) |
|
Thanks @iNecas |
Fixes #11755: Validate absolute path for custom certificates
No description provided.