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

expose assorted rpm properties in rpmkey type #7

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jhoblitt
Copy link

No description provided.

@jhoblitt
Copy link
Author

This is intended to be a proof of concept. If it's concerned a worth while feature, it should probably be modified to remove the dependency on nokogiri or have a puppet class added to ensure that libraries presence on agents.

The motivation was to make it easier to determine the rpmkey resource name when writing manifests and to provide enough information when using to puppet resource --edit to be able to reasonable remove keys.

$ RUBYLIB=./lib/ be puppet resource rpmkey
rpmkey { '172FF33D':
  ensure       => 'present',
  build_date   => 'Mon Aug 20 12:40:32 MST 2012',
  install_date => 'Wed Jan 15 08:28:09 MST 2014',
  package      => 'gpg-pubkey-172ff33d-503292b0',
  packager     => 'RPM Fusion free repository for Fedora (19) <rpmfusion-buildsys@lists.rpmfusion.org>',
}
rpmkey { '246110C1':
  ensure       => 'present',
  build_date   => 'Thu May 16 14:29:46 MST 2013',
  install_date => 'Fri Feb 14 18:58:33 MST 2014',
  package      => 'gpg-pubkey-246110c1-51954fca',
  packager     => 'Fedora (20) <fedora@fedoraproject.org>',
}
rpmkey { '5FF054BD':
  ensure       => 'present',
  build_date   => 'Fri Jun  7 15:55:31 MST 2013',
  install_date => 'Sun Oct 12 17:21:21 MST 2014',
  package      => 'gpg-pubkey-5ff054bd-51b264e3',
  packager     => 'Blue Jeans Network <netops@bluejeans.com>',
}
...

@stschulte
Copy link
Owner

I understand your usecase, and while I personally don't work with puppet resource a lot and the information that you query can be obtained otherwise, I can see the value of the change. So if I am confident that it works, I'm fine with merging the change.

comments:

  • your change currently breaks the spec tests on ruby 1.8.7 because nokogiri simly does not support ruby 1.8.7 anymore. But beeing compatible with ruby 1.8.7 (that still ships with RHEL6) is important so as you already mentioned: Best to have something with no additional external dependencies
  • does rpm had this --xml queryformat since always or may we break compatibility with older systems?
  • the readonly behaviour is currently realized at provider level. I'd argue that it is not an implementation issue of the specific provider that we cannot overwrite the packager. As a result I think it makes more sense to realize that at a type level in the validate method (e.g. as in lib/puppet/type/file/ctime.rb)

Let me know what you think.

@genebean
Copy link

FWIW, I agree that keeping compatibility with Ruby 1.8.7 is key. Not only does it still ship with RHEL / CentOS 6 but is the version that Puppet pulls in as a dependency on RHEL / CentOS 5.

@genebean
Copy link

Also, in an ideal world, you should not need libraries installed on the client but rather utilize module sync to get any ruby files needed copied to each client.

@jhoblitt
Copy link
Author

@stschulte I've checked el5.11 & el6.6 and both support the --xml flag. By eye, they also appear to have the same output format.

I believe it should be fairly straight forward to replace nokogiri with REXML, which is part of ruby 1.8.7 core. I simply used what I was familiar for experimenting.

That's a very valid point about the package data being immutable regardless of the provider -- that's an easy change.

@jhoblitt
Copy link
Author

I've replaced nokogiri with REXML, added validation on the type, and unit tests for the new properties on the type/provider.

@jhoblitt
Copy link
Author

jhoblitt commented Aug 2, 2015

@stschulte Does this PR need addition modifications before it can be merged?

@stschulte
Copy link
Owner

@jhoblitt sorry for not getting back to you. I still have to review your change. One thing I noticed while running rpm with --xml on a RHEL machine at work was that for me Packager was always empty. But I have to reverify that.

Another thing that I noticed was the way you remove a key by giving it the package name as name-version-release. If I recall correctly I stumbled upon a system with a key that was installed twice and I am not sure if they had two different releases (but the same version / keyID). That's why I remove the package by only giving it the version and why I call erase with --allmatches. Don't know if this is common though.

So as I said I will review your change and merge it then

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

Successfully merging this pull request may close these issues.

None yet

3 participants