Fixes #21128 - expand certs_tar in pre_validations #542
Conversation
This allows passing the certs_tar as relative path. Also, fix the check on exitence of the file, as the certs_tar is now in different module.
|
Issues: #21128 |
|
This should work both with |
| if param('capsule', 'certs_tar') && (certs_tar = param('capsule', 'certs_tar').value) | ||
| unless File.file?(certs_tar) | ||
| error "The certs tar file generated by the server is not present at #{certs_tar}, exiting." | ||
| certs_tar = param('foreman_proxy_certs', 'certs_tar') || |
There was a problem hiding this comment.
The module foreman_proxy_certs doesn't exist. Did you mean foreman_proxy_content? This also looks like a bonus fix of previously dead code due to the module rename so 👍
There was a problem hiding this comment.
In capsule certs generated, there is mapping defined https://github.com/Katello/katello-installer/blob/master/bin/foreman-proxy-certs-generate#L63 therefor it got here as well. Perhaps @ehelms knows why the mapping is there?
There was a problem hiding this comment.
Looking at it, I can't figure a real reason other than just typoing it during conversion away from capsule.
There was a problem hiding this comment.
I can imagine that in a pure proxy context you use that instead of certs to set the options. Including certs implies that you also create a CA which you don't want on your proxy.
|
You can already pass certs_tar as a relative path for the record [1]. The puppet type is simply 'string' which means no validation is done on pathing. This check adds some robustness to try to ensure that the file does exist at the absolute path and thus has some validity but wanted to clarify. [1] https://github.com/Katello/puppet-certs/blob/master/manifests/foreman_proxy_content.pp#L18 |
| param('foreman_proxy_content', 'certs_tar') | ||
|
|
||
| if certs_tar.value | ||
| certs_tar.value = File.expand_path(certs_tar.value) |
There was a problem hiding this comment.
Do you know if expand_path handles ~ and thus also solves that issue we've avoided through docs?
There was a problem hiding this comment.
Yes, it handles ~ as well
|
The test failures don't seem related |
voxpupuli/librarian#5 should fix that - it's an incompatibility with git 2.14.0 which Travis has installed. |
|
Anything left on me? |
|
Nothing left, thanks! |
This allows passing the certs_tar as relative path. Also, fix the check on existence of the file, as the certs_tar is now in different module. (cherry picked from commit d3dd419)
* Fixes #22517 - Resolvables compatible with option sources * Resolvables refactoring * Whitespace fixes * Removing unused method
This allows passing the certs_tar as relative path.
Also, fix the check on exitence of the file, as the certs_tar is now
in different module.