Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest using the forge approved module for gpg #86

Closed
edestecd opened this issue Mar 26, 2015 · 12 comments
Closed

Suggest using the forge approved module for gpg #86

edestecd opened this issue Mar 26, 2015 · 12 comments

Comments

@edestecd
Copy link

I propose we swap the exec being used for gpg key management with a module.
This module would probably do the trick. It can handle the key import as well as installing gpg, if needed.
https://forge.puppetlabs.com/golja/gnupg

@edestecd
Copy link
Author

We may need to get a fact added to that module that indicates if gpg is installed.
Other than that, It looks like it can provide the same functionality.

@carlossg
Copy link
Member

It's a bit tricky because I don't think gpg should be installed in this module.
So maybe it is just a matter of documentation to add to the readme "install gpg and this key before rvm" or conditionals to check if the gpg module is present

@edestecd
Copy link
Author

I agree about installing gpg. It should not be installed by this module, if not present already.
I suggest removing the gpg.pp class and adding to the readme to use the above mentioned module, if you want to install gpg.

However, I do think it is appropriate to add the key, if gpg is detected, to prevent rvm from failing.
I suggest depending on the above mentioned module to manage this as opposed to the current exec...
If you agree, I can submit a patch to https://forge.puppetlabs.com/golja/gnupg that includes a fact for detecting if gpg is installed.

@edestecd
Copy link
Author

I also think it might be nice if managing the key was optional, and had a param to turn it off...

@edestecd
Copy link
Author

I have requested facts for gnupg detection.
https://github.com/n1tr0g/golja-gnupg/pull/8/files

@dgolja
Copy link

dgolja commented Mar 27, 2015

Hey guys I am joining the party 👍

So yeah if you want to make your module dependent on mine and you think it will make it work better, I am happy to add the new facters.

@TJM
Copy link

TJM commented Mar 27, 2015

GPG Should, in fact, be installed by the "gnupg" module, and the keys should also be managed by the "gnupg" module. A module should do one thing and do it well. "This" module should not be trying to determine if gpg is installed and if so, managing the key. I would recommend just requiring the "gnupg" module and installing gpg on any platform that is supported by it. It is a GOOD THING :)

EDIT: You may want to only require it if the OS is Linux, incase someone is using the RVM module on OSX or some other obscure OS... or enhance the gnupg module with knowledge on how to install gpg on any desired target platforms.

@carlossg
Copy link
Member

I don't think this module should install gpg nor call the gnupg module to install it. It is not required by RVM and it should be defined at a host level, not here.
Now about adding the gpg key, I'm not sure whether this module should install it, or just document how to do it with the gnupg module.

@TJM
Copy link

TJM commented Mar 27, 2015

On second thought, I agree with @carlossg. This module should document the use of the "gnupg" module to install the necessary keys. It should not be doing any GPG management itself.

@edestecd
Copy link
Author

I agree that the RVM module should not install gpg and that the README should document the use of the gnupg module. I am torn on the key management, however. I could be OK with the RVM module managing it or not.

I could see a separate class that manages the key optionally and the default is to not include the class or wrap it in a param. A good example of this is the elasticsearch module which offers to install java for you with the puppet labs module. It is off by default and you must install the java module yourself as it is not a dependency in the metadata.json.

elasticsearch example:
https://github.com/elastic/puppet-elasticsearch/blob/master/README.md#java-installation
https://github.com/elastic/puppet-elasticsearch/blob/master/manifests/init.pp#L326

@carlossg
Copy link
Member

I have added an initial implementation in the gnupg branch

carlossg added a commit that referenced this issue May 16, 2015
carlossg added a commit that referenced this issue May 16, 2015
Use the gnupg_installed fact in newer golja-gnupg modules

Fix tests
@carlossg
Copy link
Member

merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants