keystore needs to be checked on each puppet run #15
Conversation
|
Hi @pseiler looks like you need to rebase for this to merge. Also the tests should be updated for this resource. If no tests exist, could you please add one. |
|
@ghoneycutt I haven't wrote tests yet (i only fixed the one for my other pr). But I'll do my best. |
|
@ghoneycutt Please have a look now. Excuse my force pushes ;) |
|
@ghoneycutt ping |
|
@ghoneycutt friendly reminder. And could you add a New release after merging the pull request? |
There was a problem hiding this comment.
Other than the alignment issues this makes sense. There's already an unless which should guarantee idempotency.
However, this whole class is a very bad pattern and needs a fix. Using predictable file names in /tmp is a recipe for security issues and with CAs that's definitely a real danger. Not saying you need to do that in this PR though.
manifests/ca.pp
Outdated
| @@ -40,8 +40,8 @@ | |||
| define trusted_ca::ca ( | |||
| Optional[String] $source = undef, | |||
| Optional[Pattern['^[A-Za-z0-9+/\n=-]+$']] $content = undef, | |||
| Stdlib::Absolutepath $install_path = $trusted_ca::install_path, | |||
| String $certfile_suffix = $trusted_ca::certfile_suffix, | |||
| Stdlib::Absolutepath $install_path = $trusted_ca::install_path, | |||
There was a problem hiding this comment.
Please don't align class parameters. It's unrelated to the actual fix and just introduces noise.
|
I will revert it and ping you. Sorry I thought the alignment is some sort of guidelines. After that I try to improve security and the behavior of the class in another pr. We use it in our company so I have a high interest in better code quality |
|
@ekohl I reverted the alignment changes. Thank you for the infos. I try to rework the class in another pr. |
otherwise it looses the certs after a java update
|
hopefully this is my final force push. Could be merged now |
|
@ekohl ping |
|
Sorry, missed this one in the flood of Github notifications. |
|
No problem. Thank you very much |
Pull Request (PR) description
keystore needs to be checked on each puppet run
otherwise it looses the certs after a java update
This Pull Request (PR) fixes the following issues