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 mod_security apache module #948

Merged
merged 1 commit into from
Dec 18, 2014

Conversation

jlambert121
Copy link
Contributor

MODULES-1561: Add support for the Web Application Firewall mod_security

@jlambert121 jlambert121 force-pushed the mod_security branch 7 times, most recently from 05f3d35 to 716db1f Compare December 3, 2014 12:51

describe file("#{mod_dir}/security.conf") do
it { is_expected.to contain "mod_security2.c" }
end
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't mod_security also depend on mod_uuid?

@underscorgan
Copy link
Contributor

@jlambert121 it looks like this will be changing default behavior (enabling mod_sec in apache::vhost by default), unless I'm missing something. If adding the ability to manage this, I'd prefer to keep the default behavior as is.

@igalic thoughts?

@jlambert121
Copy link
Contributor Author

@mhaskel after I looked at it again it actually didn't change the defaults, but I had to look at it again to understand it wasn't. I think that alone means it needs a rework so it makes more sense.

@jlambert121
Copy link
Contributor Author

@mhaskel I have renamed one of the parameters in vhost, let me know if that is clearer. If you include apache::mod::security modsecurity is enabled globally. The parameters in the vhost.pp only allow overriding that global enablement/configuration.

@igalic
Copy link
Contributor

igalic commented Dec 17, 2014

sounds good to me.

@@ -862,11 +864,13 @@ mod_reqtimeout configuration.

####Class: `apache::mod::reqtimeout`
Copy link
Contributor

Choose a reason for hiding this comment

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

The correct change here would've been changing the class title, not changing the description for the class, since apache::mod::reqtimeout is already documented above (see #960)

@underscorgan
Copy link
Contributor

@jlambert121 ok, so, there was one issue with the README that I commented on, but otherwise 👍

@jlambert121
Copy link
Contributor Author

@mhaskel thanks for the catch there - I'm not sure what I did to annoy git on the README. I just checked out the previous version of the readme and applied the updates for mod_security again.

@underscorgan
Copy link
Contributor

@jlambert121 we're getting really close! Two more comments: what is the templates/mod/security_crs.conf.erb2 file for? I don't see it referenced anywhere else in your code. Also, can you add a quick section to the README for apache::security::rule_link and make sure include the expected format of the $title. Thanks for all your work on this!

@jlambert121
Copy link
Contributor Author

The erb2 template was a development leftover - removed.
I added a description of the rule_link define in the "Private Defined Types" section.

underscorgan pushed a commit that referenced this pull request Dec 18, 2014
add mod_security apache module
@underscorgan underscorgan merged commit 257613a into puppetlabs:master Dec 18, 2014
@underscorgan
Copy link
Contributor

\o/ thanks @jlambert121!

underscorgan pushed a commit to underscorgan/puppetlabs-apache that referenced this pull request Dec 19, 2014
cyberious added a commit that referenced this pull request Dec 19, 2014
underscorgan pushed a commit to underscorgan/puppetlabs-apache that referenced this pull request Dec 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants