Added support for reload if private key defined

[vhartikainen]

The original version didn't support reloads if authentication was required. Not sure how clumsy / right way the proposed patch is, but it should support the reloading over cli when private key has been defined in cli_helper.

[jhoblitt]

@vhartikainen Did you try setting the ssh_keyfile param on the jenkins::cli_helper class?

https://github.com/jenkinsci/puppet-jenkins/blob/master/manifests/cli_helper.pp#L29-L38

[vhartikainen]

Yeah, I did, but the reload is not using the cli_helper at all as it's directly invoking a CLI command instead of utilizing the "groovy wrapper".

If you check this:
https://github.com/jenkinsci/puppet-jenkins/blob/master/manifests/cli/reload.pp#L13

it's directly referring to cli::cmd which doesn't have the key option (-i) defined at all:
https://github.com/jenkinsci/puppet-jenkins/blob/master/manifests/cli.pp#L32

[jhoblitt]

Indeed, you are correct. Hmmm... I'm not really enthusiastic at the prospect of having to set the ssh key in multiple places. What do you think about adding it as a parameter to the jenkins class and then deprecating the param on jenkins::cli_helper? I've had to wrestle with this same issue as part of my on going work to replace all of the exec resources with native types and providers.

This is the config class I've been using to pass global configuration to providers, but its not well integrated at this point. However, it does at least let you pass the key as a string would makes it easy to set from either hiera or read in with template().
https://github.com/jhoblitt/puppet-jenkins/blob/feature/types_and_providers/manifests/cli/config.pp

[matthewbarr]

Wouldn't the jenkins::params class be an option to put this kind of stuff? Instead of a separate config class?

[jhoblitt]

@matthewbarr for my provider work? No, because variables are not present in the catalog. The only data available to introspect against is resource parameters.

[matthewbarr]

Do the parameters that use defaults show up for introspection? Basically, adding them as parameters in jenkins, but then setting the defaults for those from variables in params.

Then again, the whole params inherits thing is going away, i believe :(

I would really like them to actually get the hiera in module stuff actually working, across the board, vs using RI's hack

[jhoblitt]

Any parameter set on a resource in the DSL is visible in the catalog.

[jenkinsadmin]

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

[vhartikainen]

@jhoblitt , I think that parameter passing would work just as fine if it's better way to do this. Do you have some major refactoring now ongoing? I mean, is it feasible for me to just modify the code to just pass the key as parameter to jenkins class or wait for your commits on some major incoming change?

I also realized it's impossible to now provision an instance from scratch, enable security and then update the config with same manifest. I suppose this is because the mentioned Jenkins bug when providing a key file to CLI when security is enabled. So you have to call either without (when security not enabled) or with key (when security is enabled).

[jhoblitt]

@vanderhallenf The chicken and egg problem with security is one of the main reasons I'm working on native types and providers. I have proper handling of auth/no-auth with retrying working for users, the security realm, and authentication strategy but I've discovered I need to re-implement pretty much everything before it'll be ready for merge along with some refactoring. This has turned out to be a larger job then expected. I've got ~3 weeks of full time work into the patch set and its over already over 3K lines. I've had to pause work on this front for a bit due to my underestimate and am about to leave on vacation. I may be ready to start submitting work upstream by the end of June but I wouldn't be surprised if it slipped. Your more than welcome to give my branch a try but the only documentation is currently the strings in the source. https://github.com/jhoblitt/puppet-jenkins/tree/feature/types_and_providers

Contemplating this a bit more, I think merging this PR as-is is the way to go since it is both reasonable and doesn't modify the syntax of the public API. This lets us punt on API refactoring until there is a good solution to the authentication problem.

[jhoblitt]

This should be resolved by #352