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

validate checksum_url attribute #472

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pillarsdotnet
Copy link
Contributor

Pull Request (PR) description

Adds missing docs and validation for the checksum_url attribute.

This Pull Request (PR) fixes the following issues

Fixes #471

@bastelfreak bastelfreak changed the title Add missing docs validate checksum_url for http|https|ftp|file URIs Apr 26, 2022
desc 'archive file checksum source (instead of specifying checksum). Supports http|https|ftp|file'
validate do |value|
unless value =~ URI.regexp(%w[http https ftp file])
raise ArgumentError, "invalid source url: #{value}"
Copy link
Member

Choose a reason for hiding this comment

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

thanks for adding the validation, could you add a test here? https://github.com/voxpupuli/puppet-archive/blob/master/spec/unit/puppet/type/archive_spec.rb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out that it doesn't work for file:/// urls. Not sure if it will work for ftp:// urls and can't test locally as we don't have an ftp server. It doesn't seem to take absolute-paths either. It would be nice if the checksum_url attribute would accept the same range of inputs that the source attribute does, but implementing that change is out-of-scope for my current project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may just close this PR and leave the issue open; I've already spent way more billable hours on this than I should have.

@pillarsdotnet pillarsdotnet changed the title validate checksum_url for http|https|ftp|file URIs validate checksum_url param Apr 26, 2022
@pillarsdotnet pillarsdotnet changed the title validate checksum_url param validate checksum_url attribute Apr 26, 2022
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.

uninitialized constant PuppetX::Bodeco::PUPPET
2 participants