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-7681) Add support for bytecode property #771

Merged

Conversation

baurmatt
Copy link
Contributor

This commit adds support for Berkeley Paket Filter iptables rules.

@baurmatt baurmatt force-pushed the feature/add-bytecode-property branch 2 times, most recently from 93b4868 to 6e8291d Compare August 20, 2018 13:17
@david22swan
Copy link
Member

@baurmatt Unfortunately your change failed on Redhat 6 when I ran it through an Adhoc pipeline. Here's the output:

06:23:17 bytecode property
06:23:17   bycode
06:23:17     4,48 0 0 9,21 0 1 6,6 0 0 1,6 0 0 0
06:23:17 
06:23:17 s3x8pdasc1usxr8.delivery.puppetlabs.net (redhat6-64-1) 14:23:17$ mktemp -t apply_manifest.pp.XXXXXX
06:23:17   /tmp/apply_manifest.pp.Jnv6LB
06:23:17 
06:23:17 s3x8pdasc1usxr8.delivery.puppetlabs.net (redhat6-64-1) executed in 0.05 seconds
06:23:17 localhost $ scp /tmp/beaker20181105-2884-eeq746 redhat6-64-1:/tmp/apply_manifest.pp.Jnv6LB {:ignore => }
06:23:17 
06:23:17 s3x8pdasc1usxr8.delivery.puppetlabs.net (redhat6-64-1) 14:23:17$ puppet apply --verbose --detailed-exitcodes /tmp/apply_manifest.pp.Jnv6LB
06:23:17   Info: Loading facts
06:23:19   Info: Loading facts
06:23:19   Notice: Compiled catalog for s3x8pdasc1usxr8.delivery.puppetlabs.net in environment production in 0.11 seconds
06:23:19   Info: Applying configuration version '1541427799'
06:23:19   Notice: /Stage[main]/Firewall::Linux::Redhat/Service[iptables]/ensure: ensure changed 'stopped' to 'running'
06:23:20   Info: /Stage[main]/Firewall::Linux::Redhat/Service[iptables]: Unscheduling refresh on Service[iptables]
06:23:20   Notice: /Stage[main]/Firewall::Linux::Redhat/Service[ip6tables]/ensure: ensure changed 'stopped' to 'running'
06:23:20   Info: /Stage[main]/Firewall::Linux::Redhat/Service[ip6tables]: Unscheduling refresh on Service[ip6tables]
06:23:20   Error: Execution of '/sbin/iptables -I OUTPUT 1 -t filter -p all -j ACCEPT -m bpf --bytecode 4,48 0 0 9,21 0 1 6,6 0 0 1,6 0 0 0 -m comment --comment 102 - test' returned 2: iptables v1.4.7: Couldn't load match `bpf':/lib64/xtables/libipt_bpf.so: cannot open shared object file: No such file or directory
06:23:20   
06:23:20   Try `iptables -h' or 'iptables --help' for more information.
06:23:20   Error: /Stage[main]/Main/Firewall[102 - test]/ensure: change from 'absent' to 'present' failed: Execution of '/sbin/iptables -I OUTPUT 1 -t filter -p all -j ACCEPT -m bpf --bytecode 4,48 0 0 9,21 0 1 6,6 0 0 1,6 0 0 0 -m comment --comment 102 - test' returned 2: iptables v1.4.7: Couldn't load match `bpf':/lib64/xtables/libipt_bpf.so: cannot open shared object file: No such file or directory
06:23:20   
06:23:20   Try `iptables -h' or 'iptables --help' for more information.
06:23:20   Notice: /Stage[main]/Firewall::Linux::Redhat/File[/etc/sysconfig/iptables]: Dependency Firewall[102 - test] has failures: true
06:23:20   Warning: /Stage[main]/Firewall::Linux::Redhat/File[/etc/sysconfig/iptables]: Skipping because of failed dependencies
06:23:20   Notice: /Stage[main]/Firewall::Linux::Redhat/File[/etc/sysconfig/ip6tables]: Dependency Firewall[102 - test] has failures: true
06:23:20   Warning: /Stage[main]/Firewall::Linux::Redhat/File[/etc/sysconfig/ip6tables]: Skipping because of failed dependencies
06:23:20   Info: Class[Firewall::Linux::Redhat]: Unscheduling all events on Class[Firewall::Linux::Redhat]
06:23:20   Info: Creating state file /opt/puppetlabs/puppet/cache/state/state.yaml
06:23:20   Notice: Applied catalog in 0.56 seconds
06:23:20 
06:23:20 s3x8pdasc1usxr8.delivery.puppetlabs.net (redhat6-64-1) executed in 2.94 seconds
06:23:20 Exited: 6
06:23:20       applies (FAILED - 1)
06:23:20 
06:23:20 s3x8pdasc1usxr8.delivery.puppetlabs.net (redhat6-64-1) 14:23:20$ iptables-save
06:23:20   # Generated by iptables-save v1.4.7 on Mon Nov  5 14:23:20 2018
06:23:20   *filter
06:23:20   :INPUT ACCEPT [0:0]
06:23:20   :FORWARD ACCEPT [0:0]
06:23:20   :OUTPUT ACCEPT [19:3960]
06:23:20   -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT 
06:23:20   -A INPUT -p icmp -j ACCEPT 
06:23:20   -A INPUT -i lo -j ACCEPT 
06:23:20   -A INPUT -p tcp -m state --state NEW -m tcp --dport 22 -j ACCEPT 
06:23:20   -A INPUT -j REJECT --reject-with icmp-host-prohibited 
06:23:20   -A FORWARD -j REJECT --reject-with icmp-host-prohibited 
06:23:20   COMMIT
06:23:20   # Completed on Mon Nov  5 14:23:20 2018
06:23:20 
06:23:20 s3x8pdasc1usxr8.delivery.puppetlabs.net (redhat6-64-1) executed in 0.04 seconds
06:23:20       contains the rule (FAILED - 2)

@baurmatt
Copy link
Contributor Author

baurmatt commented Nov 6, 2018

Hey @david22swan, thanks for testing! :) Which Linux kernel version does your RedHat 6 run? I think support for BPF was added with 3.10.x

@david22swan
Copy link
Member

@baurmatt

# uname -r --kernel-name --kernel-release
Linux 2.6.32-642.el6.x86_64

Kernal version is 2.6.32

@baurmatt
Copy link
Contributor Author

baurmatt commented Nov 6, 2018

Yeah, so this "works as expected". How can we probably handle this in CI? Can we somehow ensure that bytecode_spec.rb runs only appropriate OSs?

@david22swan
Copy link
Member

@baurmatt In this sort of situation we usually note the limitation within the documentation, put an exception around the test's so they aren't run against OS's that would cause them to fail and at times, add an exception to the code itself so that if the new feature is used on an OS that does not support it an error message would be given. Though the last part is not required.

@baurmatt
Copy link
Contributor Author

baurmatt commented Nov 6, 2018

Ok, so further investigation into this topic showed that the bpf/bytecode requires nftables which was released with Linux 3.13. RedHat backported this into their RHEL 7.6 kernel for kernel-3.10.0-957.el7 and later.

We use Virtuozzo Linux release 7.5 which uses CentOS/RedHat as basis and currently runs 3.10.0-862.9.1.vz7.63.3. But it does support the --bytecode parameter.

For those reasons, I decided against a validation within the type/provider and just updated the README.md.

Could you please help me with disabling the tests for RedHat/CentOS < 7? I'm not sure how to exclude the bytecode_spec.rb acceptance test.

@david22swan
Copy link
Member

@baurmatt you should be able to use code similar to this:

describe 'bytecode property', unless: fact('operatingsystem') == 'Redhat' && fact('operatingsystemmajrelease') <= '7' do

To exclude specific OS and OS versions.
You can also use the fact:

(fact('osfamily') == 'RedHat'

To exclude entire OS Families.

@baurmatt baurmatt force-pushed the feature/add-bytecode-property branch from 6e8291d to 6805b48 Compare November 6, 2018 14:24
@baurmatt
Copy link
Contributor Author

baurmatt commented Nov 6, 2018

Ha, that unless parameter is awesome! Thanks for the hin! :)

@david22swan
Copy link
Member

@baurmatt Sorry problem with the code I gave you.
Try swapping it to:

describe 'bytecode property', if: !(fact('operatingsystem') == 'Redhat' && fact('operatingsystemmajrelease') <= '7') do

@baurmatt baurmatt force-pushed the feature/add-bytecode-property branch from 6805b48 to 3378513 Compare November 6, 2018 15:03
@baurmatt
Copy link
Contributor Author

baurmatt commented Nov 6, 2018

Right, the unless would exclude e.g. Debian as well 🙈 Fixed it! :)

@david22swan
Copy link
Member

@baurmatt I really hate to say this again, but there's one more correction needed to the code I gave you. It should be RedHat not Redhat.
Sorry this has been going on so long. To make up for it I ran your PR against all the other OS and found a bunch of others it should skip. Here is, what should hopefully be, the finished exclusion code:

describe 'bytecode property', unless: (fact('osfamily') == 'RedHat' && fact('operatingsystemmajrelease') < '7') ||
                                      (fact('operatingsystem') == 'OracleLinux' && fact('operatingsystemmajrelease') <= '7') ||
                                      (fact('operatingsystem') == 'SLES' && fact('operatingsystemmajrelease') <= '11') do

This commit adds support for Berkeley Paket Filter iptables rules.
@baurmatt baurmatt force-pushed the feature/add-bytecode-property branch from 3378513 to b4328bc Compare November 6, 2018 17:15
@baurmatt
Copy link
Contributor Author

baurmatt commented Nov 6, 2018

No worries, I'm very grateful for the help you provide!

I've updated the PR with your code, added a hopefully helpful comment and fixed a typo within the README.md.

Thanks a lot! :)

@david22swan
Copy link
Member

screen shot 2018-11-07 at 9 48 08 am

@david22swan david22swan merged commit e8da6df into puppetlabs:master Nov 7, 2018
@david22swan
Copy link
Member

@baurmatt Everything looks good, thanks for the PR.

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.

2 participants