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

Support for cacert_file param on archive resource #422

Merged

Conversation

benningm
Copy link
Contributor

@benningm benningm commented Oct 1, 2020

Pull Request (PR) description

This adds a cacert_file parameter to the archive type and
support for this option in curl, wget and ruby provider.

The option could be used to specify a custom CA bundle for
certificate verification in the TLS handshake.

This Pull Request (PR) fixes the following issues

related #400
implements #188

Additional notes

I'm unsure about the part in lib/puppet_x/bodeco/util.rb with ENV['SSL_CERT_FILE'].
Where is it used? Is it still required?

This adds a cacert_file parameter to the archive type and
support for this option in curl, wget and ruby provider.

The option could be used to specify a custom CA bundle for
certificate verification in the TLS handshake.
Remove empty line.
Copy link
Member

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

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

looks good to me, but I'm far away from being an expert for archive. Somebody else should review this as well.

Copy link
Member

@alexjfisher alexjfisher left a comment

Choose a reason for hiding this comment

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

LGTM

lib/puppet/type/archive.rb Outdated Show resolved Hide resolved
lib/puppet_x/bodeco/util.rb Outdated Show resolved Hide resolved
benningm and others added 2 commits January 29, 2021 10:47
Fix validation logic for cacert_file.

Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
Simplified path to windows ca file.

Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
Copy link
Contributor Author

@benningm benningm left a comment

Choose a reason for hiding this comment

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

Regarding the type validation i copied that over from the other path options. Maybe you change all the options to use puppet type validations.

lib/puppet_x/bodeco/util.rb Outdated Show resolved Hide resolved
@ekohl
Copy link
Member

ekohl commented Jan 29, 2021

@alexjfisher any last comments?

@bastelfreak bastelfreak added the enhancement New feature or request label Apr 16, 2021
@bastelfreak bastelfreak merged commit 2430673 into voxpupuli:master Apr 16, 2021
@bastelfreak bastelfreak added skip-changelog and removed enhancement New feature or request labels Apr 16, 2021
@bastelfreak
Copy link
Member

Hi @benningm . I merged this and another PR that introduced tests. code from #422 failed with the new tests so I had to revert it for now. Please check #440 for details. can you submit the PR again and take a look at the failing tests?

@benningm
Copy link
Contributor Author

If you can confirm that it will not take a half year to review it and then will be reverted again.

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.

4 participants