Feature: Make selboolean management optional

[bastelfreak]

No description provided.

[ekohl]

Some bikeshedding: manage_selinux could also imply foreman-selinux is not installed. How about manage_selinux_booleans?

[ekohl]

Thanks!

[ekohl]

It does look like the tests fail because of this. Mid taking a look?

[alexjfisher]

Not on systems that don't support SELinux it won't...

[ekohl]

puppet-foreman/spec/classes/foreman_config_ipa_spec.rb

Lines 58 to 74 in 63bba21

	context 'with SELinux' do
	let(:facts) { override_facts(super(), os: {'selinux' => {'enabled' => selinux}}) }
	context 'enabled' do
	let(:selinux) { true }
	it { should contain_selboolean('allow_httpd_mod_auth_pam') }
	it { should contain_selboolean('httpd_dbus_sssd') }
	end
	context 'disabled' do
	let(:selinux) { false }
	it { should_not contain_selboolean('allow_httpd_mod_auth_pam') }
	it { should_not contain_selboolean('httpd_dbus_sssd') }
	end
	end

has a similar pattern, but that entire class is already only run on the Red Hat OS family.

[alexjfisher]

puppet-foreman/spec/classes/foreman_config_ipa_spec.rb

Lines 58 to 74 in 63bba21

	context 'with SELinux' do
	let(:facts) { override_facts(super(), os: {'selinux' => {'enabled' => selinux}}) }
	context 'enabled' do
	let(:selinux) { true }
	it { should contain_selboolean('allow_httpd_mod_auth_pam') }
	it { should contain_selboolean('httpd_dbus_sssd') }
	end
	context 'disabled' do
	let(:selinux) { false }
	it { should_not contain_selboolean('allow_httpd_mod_auth_pam') }
	it { should_not contain_selboolean('httpd_dbus_sssd') }
	end
	end

has a similar pattern, but that entire class is already only run on the Red Hat OS family.

oh, ok. Seemed a bit redundant to execute the tests on Debian systems though. Don't they take long enough already?

[alexjfisher]

puppet-foreman/spec/classes/foreman_config_ipa_spec.rb

Lines 58 to 74 in 63bba21

	context 'with SELinux' do
	let(:facts) { override_facts(super(), os: {'selinux' => {'enabled' => selinux}}) }
	context 'enabled' do
	let(:selinux) { true }
	it { should contain_selboolean('allow_httpd_mod_auth_pam') }
	it { should contain_selboolean('httpd_dbus_sssd') }
	end
	context 'disabled' do
	let(:selinux) { false }
	it { should_not contain_selboolean('allow_httpd_mod_auth_pam') }
	it { should_not contain_selboolean('httpd_dbus_sssd') }
	end
	end

has a similar pattern, but that entire class is already only run on the Red Hat OS family.

oh, ok. Seemed a bit redundant to execute the tests on Debian systems though. Don't they take long enough already?

EDIT: I see. You still need to set {'selinux' => {'enabled' => selinux}}) } as that wouldn't be the default even in RedHat.

[alexjfisher]

@bastelfreak Maybe just skip the lot when osfamily != RedHat, to not waste time.

[bastelfreak]

@alexjfisher added it

[alexjfisher]

What about

puppet-foreman/manifests/config.pp

Line 157 in 63bba21

selboolean { ['allow_httpd_mod_auth_pam', 'httpd_dbus_sssd']:

?

The name of this parameter would suggest when setting to false all selinux_booleans won't be managed.

[bastelfreak]

How should I change it? manage_apache_selinux_boolean? manage_httpd_can_network_connect_boolean?

[ekohl]

How about the alternative and use ensure_resource?

[ekohl]

This has been stale for a while. What should we do with this?

[kBite]

@ekohl / @alexjfisher Is this just about naming the new parameter? In this case I'd create a new PR based on @bastelfreak's and rebased against master. Btw.: I'd suggest manage_httpd_selinux_boolean as selbooleans also have httpd in their names.

[ekohl]

@kBite I think it's about naming and consistency. As @alexjfisher pointed out: the current name implies all booleans are managed. Either the parameter name should by changed to imply it only manages a specific boolean or the parameter should manage all booleans.