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

add cert_file type #124

Merged
merged 11 commits into from
Apr 30, 2021
Merged

add cert_file type #124

merged 11 commits into from
Apr 30, 2021

Conversation

rtib
Copy link
Contributor

@rtib rtib commented Apr 23, 2021

A cert_file resource is intended to behave some similar to file, but respecting the content as X.509 certificate. In this initial version it is able to download a certificate from a given URL and to store it in a local file in the chosen format PEM or DER.

@puppet-community-rangefinder
Copy link

cert_file is a type

that may have no external impact to Forge modules.

This module is declared in 2 of 576 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.

@rtib
Copy link
Contributor Author

rtib commented Apr 23, 2021

It needs unit tests, therefore still in draft, but open for discussion and comments.

@raphink raphink added the enhancement New feature or request label Apr 23, 2021
@rtib
Copy link
Contributor Author

rtib commented Apr 27, 2021

It actually fails with Puppet 5. Will the module support Puppet 5 for a long time?

@rtib rtib marked this pull request as ready for review April 27, 2021 15:28
@rtib
Copy link
Contributor Author

rtib commented Apr 28, 2021

Rebased onto #125

@raphink
Copy link
Member

raphink commented Apr 28, 2021

I just merged #125. Could you rebase to reduce the diff please?

@rtib
Copy link
Contributor Author

rtib commented Apr 28, 2021

Rebased onto master and squashed. Unfortunately, tests downloading files are unstable on travis-ci, and now breaking the build again.

@rtib
Copy link
Contributor Author

rtib commented Apr 29, 2021

I think it's done now. I could run the spec test locally multiple times with stable results. It may be a good idea to restart the travis-ci build a couple of times before merging to assure it's stable.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/puppet/provider/cert_file/posix.rb Show resolved Hide resolved
lib/puppet/provider/cert_file/posix.rb Show resolved Hide resolved
lib/puppet/provider/cert_file/posix.rb Outdated Show resolved Hide resolved
lib/puppet/provider/cert_file/posix.rb Show resolved Hide resolved
lib/puppet/type/cert_file.rb Outdated Show resolved Hide resolved
lib/puppet/type/cert_file.rb Show resolved Hide resolved
lib/puppet/type/cert_file.rb Outdated Show resolved Hide resolved
spec/spec_helper_local.rb Outdated Show resolved Hide resolved
rtib and others added 7 commits April 29, 2021 14:24
Co-authored-by: Raphaël Pinson <github+aem1eeshi1@raphink.net>
Co-authored-by: Raphaël Pinson <github+aem1eeshi1@raphink.net>
Co-authored-by: Raphaël Pinson <github+aem1eeshi1@raphink.net>
@rtib rtib requested a review from raphink April 29, 2021 13:43
return false unless Pathname.new(resource[:path]).exist?

debug "Checking file format #{resource[:path]}"
localcert_file = File.read(resource[:path])
Copy link
Member

Choose a reason for hiding this comment

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

This is where it will fail if the path is a directory.

Errno::EISDIR: Is a directory @ io_fread - /etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but the agent continues.

It would log something like:

Info: Applying configuration version 'environment rev: 2021-04-30T08:44:13+02:00 c77a23b2 Tibor Répási: test'
Error: /Stage[main]/Roles_test::Cert/Cert_file[/tmp]: Could not evaluate: Is a directory @ io_fread - /tmp
Notice: /Stage[main]/Roles_test::Cert/File[/tmp/test.txt]/ensure: defined content as '{md5}65a8e27d8879283831b664bd8b7f0ad4' (corrective)
Info: Class[Roles_test::Cert]: Unscheduling all events on Class[Roles_test::Cert]
Notice: Applied catalog in 4.35 seconds

Could catch and re-throw to wrap the error, like e.g.:

Info: Applying configuration version 'environment rev: 2021-04-30T08:44:13+02:00 c77a23b2 Tibor Répási: test'
Error: /Stage[main]/Roles_test::Cert/Cert_file[/tmp]: Could not evaluate: failed to evaluate contents of the file expected at /tmp. Caused by: Is a directory @ io_fread - /tmp
Notice: /Stage[main]/Roles_test::Cert/File[/tmp/test.txt]/ensure: defined content as '{md5}65a8e27d8879283831b664bd8b7f0ad4' (corrective)
Info: Class[Roles_test::Cert]: Unscheduling all events on Class[Roles_test::Cert]
Notice: Applied catalog in 4.32 seconds

This could be a bit more expressive, but the overall behaviour wouldn't change.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, fine if Puppet continues then

@raphink raphink merged commit 493fa36 into voxpupuli:master Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants