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

Remove firewall resource #229

Merged
merged 1 commit into from
Jun 19, 2013
Merged

Conversation

hunner
Copy link
Contributor

@hunner hunner commented Jun 18, 2013

The firewall declarations should not be handled by the apache module

The firewall declarations should not be handled by the apache module
@pronix
Copy link
Contributor

pronix commented Jun 18, 2013

hello
where should be firewall declaration ?

@bmurtagh
Copy link

I think the firewall should be declared in another module specific for firewall rules or a wrapper. The Apache module should focus on all things related to Apache setup, configuration, and management. A separate firewall module/wrapper would include all rules needed to grant access for that node or infrastructure. It should be referenced/included/chained by your module containing Apache if needed.

@hunner
Copy link
Contributor Author

hunner commented Jun 18, 2013

@bmurt is correct. The apache module and all of its classes and defines are not given the responsibility of understanding the firewall requirements of every company, and so shouldn't make assumptions about requirements.

Usually pairing the firewall resources that work for you with the apache::vhost resources in a single define that understands your business logic and data binding is the current practice.

@bmurtagh
Copy link

@hunner Are you going to be attempting the build again to get this merged?

@hunner
Copy link
Contributor Author

hunner commented Jun 19, 2013

@hunner Are you going to be attempting the build again to get this merged?

The builds ran successfully. I was just hoping for a little feedback, since I'm currently a one-man team and can't peer-review my own code :P. I'll take this as a "+1" and merge it.

hunner added a commit that referenced this pull request Jun 19, 2013
@hunner hunner merged commit 0e38068 into puppetlabs:master Jun 19, 2013
@hunner hunner deleted the remove_firewall branch June 19, 2013 17:42
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