Skip to content

MODULES-11309 : convert a string to a resource #1233

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

Merged
merged 2 commits into from
May 16, 2022
Merged

MODULES-11309 : convert a string to a resource #1233

merged 2 commits into from
May 16, 2022

Conversation

jcpunk
Copy link
Contributor

@jcpunk jcpunk commented Apr 14, 2022

I'll need a bit of help loading the resources into the catalog for the tests, but in theory that should be all that is required.

@jcpunk jcpunk requested a review from a team as a code owner April 14, 2022 17:31
@puppet-community-rangefinder
Copy link

str2resource is a function

that may have no external impact to Forge modules.

This module is declared in 318 of 579 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@jcpunk jcpunk requested a review from ekohl April 15, 2022 16:06
@jcpunk
Copy link
Contributor Author

jcpunk commented Apr 18, 2022

I'm not sure what to do about the windows test failure....

@jcpunk jcpunk requested a review from alexjfisher April 18, 2022 15:45
@jcpunk
Copy link
Contributor Author

jcpunk commented Apr 19, 2022

I was trying to get the test timeout to go away and added an example of how I'd use this. There are dozens of modules on the forge that have various versions of this pattern. In theory having this in stdlib would help unify the community and reduce the maintenance burden on various folks.

Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm questioning the usefulness of this. Do you have examples where it makes sense?

Also for ordering you can use a collector. I think this should work, whether the file exists or not.

Package['mypackage'] -> File <| title == '/foo' |>

@jcpunk
Copy link
Contributor Author

jcpunk commented Apr 19, 2022

I'm questioning the usefulness of this. Do you have examples where it makes sense?

This was inspired in part by https://forge.puppet.com/modules/jake/resource_tree and https://forge.puppet.com/modules/jaydub/resource_factory, however, those aren't up to date. I've also been using a few elements from https://github.com/voxpupuli/puppet-system (some one off sysctl and package resources).

Most of the modules I work with have a hash argument to create their defined resources, but puppet itself doesn't have a way pass in "make these trivial things" from the builtin resources.

My primary usage would be something like this - but not this many on one host (using the class I've demo'd here which I can drop if requested):

stdlib::manage::create_resources:
  file:
    /etc/krb5.conf:
      source: https://authentication.fnal.gov/krb5conf/Linux/krb5.conf
  package:
    avahi:
      ensure: absent
  archive:
    FermiCerts:
      path: /tmp/._certs.zip
      source: https://authentication.fnal.gov/certs/FERMI_CA_Certs-PEM.zip
      extract: true
      extract_path: /etc/pki/ca-trust/source/anchors
      creates: /etc/pki/ca-trust/source/anchors/Fermilab_Root_CA.pem
      require: Package[unzip]

I see the biggest gains in this mostly around ensure => 'absent' for packages. Building up a well structured and tested class for removing a package is a bit more effort than just slapping a few lines of yaml down.

I'd like to do something like:

stdlib::manage::create_resources:
  file:
    /etc/motd.d/patch_group:
      content: "\nThis system is part of patch group: %{::patching_as_code::patch_group}"

My existing motd module doesn't have a great way of using the data I can extract from patching_as_code for me to expose out to the users. Rather than reworking the module or building up a whole other class to link the two together, I could do something like that.

My profile::common module has tacitly grown a whole separate puppet tree underneath it as more and more "common" things appear and one offs need to be stashed somewhere. With hiera I can do something a bit more reasonable via:

hierarchy:
 <snip>
  - name: 'Common Defaults'
    glob: 'common/*.yaml'

Or put the one-off declaration actually in hostname/%{trusted.certname}.yaml

@jcpunk
Copy link
Contributor Author

jcpunk commented Apr 19, 2022

Package['mypackage'] -> File <| title == '/foo' |>

I'm not sure how I'd deference the variables to get the Package/File resource for use with the collectors....

Copy link
Member

@david22swan david22swan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcpunk This look's good to me so I'm gonna go ahead and merge.
Thank you for putting the work in to get it up and over the line :)

@david22swan david22swan merged commit 8f8353d into puppetlabs:main May 16, 2022
@jcpunk jcpunk deleted the str2resource branch May 16, 2022 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants