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

Conglomerate of PRs with tests #61

Closed
wants to merge 6 commits into from

Conversation

jrwesolo
Copy link
Contributor

I was looking at this module and saw a few open pull requests that seem to have been abandoned by their submitters. I went ahead and implemented some of the changes, complete with tests, for the following PRs: #29, #43, #49.

Highlights

  • Simplify the snmpd.conf template with some ruby magic
  • Allow the community and network variables to be strings or arrays
  • Change default groups to be empty, prevent DDoS
  • Allow service_config_perms to be a class parameter
  • Validate additional arrays in init class
  • Add test coverage for new code as well as some old code

* Fixed unit tests
* Defaults to empty array of groups to provide safer defaults
* `service_config_perms` can now be passed as a class parameter
* Added spec tests
…string

* various community variable can now be either arrays or strings
* various network variables can now be either arrays or strings
* simplified template for snmpd.conf using ruby magic
rwcommunity6 <%= @rw_community6 %> <%= @rw_network6 %>
<% end -%>
<% end -%>
<%- [*@ro_community].compact.each do |c| -%>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The [*@ro_community] syntax will take a variable and convert it into an array if it is not already. This allows us to really trim down this file.

@razorsedge razorsedge self-assigned this Nov 23, 2015
@razorsedge
Copy link
Contributor

Awesome work @jrwesolo . If I could ask one favour: could you resubmit against the develop branch?

@jrwesolo
Copy link
Contributor Author

Sure thing! #62.

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