Skip to content

create_resources() input validation for hash#1956

Closed
big-samantha wants to merge 3 commits intopuppetlabs:masterfrom
big-samantha:master
Closed

create_resources() input validation for hash#1956
big-samantha wants to merge 3 commits intopuppetlabs:masterfrom
big-samantha:master

Conversation

@big-samantha
Copy link

Currently, create resources doesn't check that the 2nd argument is a hash. It should. Tada!

Currently, create resources doesn't check that the 2nd argument is a hash. It should. Tada!
@puppetcla
Copy link

CLA signed by all contributors.

@big-samantha
Copy link
Author

Redmine issue: https://projects.puppetlabs.com/issues/22740

@adrienthebo
Copy link
Contributor

Thank you very much for this contribution!

Pull Request - Missing Tests

This looks good, but before we can merge the change set into the code base we also need to include examples that exercise this new behavior so we're able to validate the expected results. The existing tests at https://github.com/puppetlabs/puppet/blob/master/spec/unit/parser/functions/create_resources_spec.rb#L21 could be a good reference to test this new behavior.

In addition it would be good to have the commit message amended a little. The first line of the commit should have the issue number and a description of the problem or fix, and the body of the commit message should explain what is this existing behavior and how this change fixes it. We have a more complete explanation of this at https://github.com/puppetlabs/puppet/blob/master/CONTRIBUTING.md#making-changes .

Zachary Alex Stern and others added 2 commits October 2, 2013 13:11
The create_resources() function in puppet expects a hash as its second
argument.

Currently, the only validation of input we perform is for number of
arguments. If the second parameter is not a hash, is nil, etc, Puppet
explodes. For example, you might get a message like `undefined method
`each' for "":String at /etc/puppetlabs/puppet/manifests/site.pp:45 on
node pe-301-master.puppetdebug.vlan`. Not very useful.

This patch adds a check that the second parameter is a hash, and raises an
error if it is not.
@big-samantha
Copy link
Author

Will edit and resubmit.

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.

3 participants