search

dgolja / golja-gnupg

Puppet module for managing GnuPG package and public keys
Apache License 2.0
13 stars 46 forks source link

Add fact that indicates if gpg is installed or not. #7

Closed edestecd closed 9 years ago

edestecd commented 9 years ago

I would like to manage a key only if gpg is installed on a system. RVM only uses gpg if it is installed, so I would like to replicate this behavior in the RVM module here: https://forge.puppetlabs.com/maestrodev/rvm

It currently uses an exec for key management, but I would prefer to switch to this module. Is there anyway currently to tell if gpg is already installed? If not, what do you think of adding a fact? I could write one and submit a PR.

dgolja commented 9 years ago

Hey Chris,

thank you for the commit. To be honest I am not a big fun of adding custom facts, if they are not really necessary.

In a big puppet installation more custom facts you have, more time it takes the puppet agent to start/finish. Hopefully cfacter will fix that, but is still few months away.

Usually if I really need some custom facts I add them per environment in the common module. Also if I install gpg keys on a node, I know that I need them there, and if so I include the gnupg class.

What are your thoughts on that ?

edestecd commented 9 years ago

RVM uses the which command to determine if gpg is installed. If it is, it looks for a gpg key and fails if one is not present. https://github.com/wayneeseguin/rvm/blob/480fbc35211940dc2d5ac09ce630e1a161090f70/binscripts/rvm-installer#L386 We are simply trying to replicate this in the RVM module and only add a key if needed to avoid failure.

The RVM module should not install gnupg if it is not present. That is what this module is for. So we need to determine if in fact gpg is present. We are currently using this exec, which I find sloppy. https://github.com/maestrodev/puppet-rvm/blob/master/manifests/system.pp#L38 I think this is a good use of a fact and I would much rather use this modules gnupg_key type.

I suppose we could add the fact to the RVM module, but I feel it makes more sense for it to go in this module, as it is more related to gpg.

More info here: https://github.com/maestrodev/puppet-rvm/issues/86

dgolja commented 9 years ago

HaHa I like your obsession to make the code less sloppy :+1:

I guess if https://github.com/carlossg want to make his module depending on mine we can add this new feature.

dgolja commented 9 years ago

done ... sorry for the delay and thank you again for the PR.