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

(MODULES-3572) Ip6tables service is not managed in the redhat family. #641

Merged
merged 2 commits into from
Dec 23, 2016

Conversation

marcofl
Copy link
Contributor

@marcofl marcofl commented Jun 28, 2016

For the redhat OS family the service for managing ip6tables is called
ip6tables. This service is currently not managed with this module.
This commit fixes this issue by introducing an additional parameter
$service_name_v6 for the ipv6 version of the service.

@marcofl marcofl changed the title Ip6tables service is not managed in the redhat family. (MODULES-3572) Ip6tables service is not managed in the redhat family. Jul 7, 2016
@marcofl marcofl force-pushed the fix/ip6tables_redhat_service branch from 9e7f026 to 0b27f7f Compare July 7, 2016 07:25
@marcofl
Copy link
Contributor Author

marcofl commented Aug 2, 2016

looks like some dependency issue in the test environment with "mime-types-data"...

@@ -5,6 +5,7 @@
case $::operatingsystem {
Copy link
Contributor

@hunner hunner Sep 8, 2016

Choose a reason for hiding this comment

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

You could add the variable assignment above this line, similar to the Debian case below (for simplicity)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@hunner
Copy link
Contributor

hunner commented Sep 8, 2016

Thanks for this contribution! Could you also add the service_name_v6 parameter to the readme, and also an rspec-puppet test similar to the way that gentoo & archlinux do it: https://github.com/puppetlabs/puppetlabs-firewall/blob/1.8.1/spec/unit/classes/firewall_linux_archlinux_spec.rb#L10-L17

For reference, when looking at the way that service_name accepts an array of multiple values on gentoo and archlinux would be complicated to apply to redhat due to the sysconfig file resource and service dependency. It's possible, but would require using join(), prefix() and create_resources() :/ (but you can if you want!)

It would also be nice to squash this into one commit, but we can do that too if it's too much trouble. Thanks!

@marcofl marcofl force-pushed the fix/ip6tables_redhat_service branch 5 times, most recently from cf561ef to e6328ef Compare September 20, 2016 07:54
@marcofl
Copy link
Contributor Author

marcofl commented Sep 20, 2016

@hunner thank you for looking into this. I updated the README, simplified the params for RHEL and copied the correct tests (I hope so) for RHEL. about the array for service_name(_v6): As I cannot think of any reason to have more than one iptables or ip6tables service under RHEL I just put a validate_string to the parameter to ensure no array is supplied. but looks like you don't use stdlib functions, so I removed that again. What do you think about this?

…me for iptables and ip6tables. update README

For the redhat OS family the service for managing ip6tables is called
ip6tables. This service is currently not managed with this module.
This commit fixes this issue by introducing an additional parameter
$service_name_v6 for the ipv6 version of the service.
@tobru
Copy link

tobru commented Oct 10, 2016

@hunner Would you mind having a look into this again?

@tobru
Copy link

tobru commented Oct 25, 2016

Any chance to get this merged soon? It's a rather annyoing issue and it would be great if this issue get's fixed

@tphoney
Copy link
Contributor

tphoney commented Dec 23, 2016

Thanks for the PR, apologies for taking an age to get at it.

@tphoney tphoney merged commit 8b30e7f into puppetlabs:master Dec 23, 2016
wilson208 pushed a commit to wilson208/puppetlabs-firewall that referenced this pull request Jan 3, 2017
Caused through merge of PR's puppetlabs#658 and then subsequently this older PR puppetlabs#641
wilson208 pushed a commit to wilson208/puppetlabs-firewall that referenced this pull request Jan 3, 2017
Caused through merge of PR's puppetlabs#658 and then subsequently this older PR puppetlabs#641
wilson208 pushed a commit to wilson208/puppetlabs-firewall that referenced this pull request Jan 3, 2017
Caused through merge of PR's puppetlabs#658 and then subsequently this older PR puppetlabs#641
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.

5 participants