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

Feature: Make selboolean management optional #849

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions manifests/config.pp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@
keycloak => $foreman::keycloak,
keycloak_app_name => $foreman::keycloak_app_name,
keycloak_realm => $foreman::keycloak_realm,
manage_selinux_booleans => $foreman::manage_selinux_booleans,
}

contain foreman::config::apache
Expand Down
6 changes: 5 additions & 1 deletion manifests/config/apache.pp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@
# @param keycloak_realm
# The realm as passed to keycloak-httpd-client-install
#
# @param manage_selinux_booleans
# If true AND selinux is enabled on the node, set httpd_can_network_connect so apache works properly
#
class foreman::config::apache(
Stdlib::Absolutepath $app_root = '/usr/share/foreman',
String $priority = '05',
Expand Down Expand Up @@ -131,6 +134,7 @@
Boolean $keycloak = false,
String[1] $keycloak_app_name = 'foreman-openidc',
String[1] $keycloak_realm = 'ssl-realm',
Boolean $manage_selinux_booleans = true,
) {
$docroot = "${app_root}/public"

Expand Down Expand Up @@ -232,7 +236,7 @@
],
}

if $facts['os']['selinux']['enabled'] {
if $facts['os']['selinux']['enabled'] and $manage_selinux_booleans {
selboolean { 'httpd_can_network_connect':
persistent => true,
value => 'on',
Expand Down
3 changes: 3 additions & 0 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@
#
# $rails_cache_store:: Set rails cache store
#
# $manage_selinux_booleans:: If true AND selinux is enabled on the node, set httpd_can_network_connect so apache works properly
Copy link
Contributor

Choose a reason for hiding this comment

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

What about

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

How should I change it? manage_apache_selinux_boolean? manage_httpd_can_network_connect_boolean?

#
# === Keycloak parameters:
#
# $keycloak:: Enable Keycloak support. Note this is limited
Expand Down Expand Up @@ -308,6 +310,7 @@
Boolean $keycloak = $foreman::params::keycloak,
String[1] $keycloak_app_name = $foreman::params::keycloak_app_name,
String[1] $keycloak_realm = $foreman::params::keycloak_realm,
Boolean $manage_selinux_booleans = true,
) inherits foreman::params {
if $db_sslmode == 'UNSET' and $db_root_cert {
$db_sslmode_real = 'verify-full'
Expand Down
38 changes: 38 additions & 0 deletions spec/classes/foreman_config_apache_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,44 @@
end
end

describe 'without manage_selinux_booleans', if: facts[:os]['family'] == 'RedHat' do
let :facts do
override_facts(super(), os: {'selinux' => {'enabled' => true}})
bastelfreak marked this conversation as resolved.
Show resolved Hide resolved
end

bastelfreak marked this conversation as resolved.
Show resolved Hide resolved
it 'should contain the selinux resource' do
should contain_selboolean('httpd_can_network_connect')
Copy link
Contributor

Choose a reason for hiding this comment

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

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

end
end

describe 'with manage_selinux_booleans to true', if: facts[:os]['family'] == 'RedHat' do
bastelfreak marked this conversation as resolved.
Show resolved Hide resolved
let :params do
super().merge(
manage_selinux_booleans: true
)
end

let :facts do
override_facts(super(), os: {'selinux' => {'enabled' => true}})
end

it 'should contain the selinux resource' do
should contain_selboolean('httpd_can_network_connect')
end
end

describe 'with manage_selinux_booleans to false', if: facts[:os]['family'] == 'RedHat' do
let :params do
super().merge(
manage_selinux_booleans: false
)
end

it 'should not contain the selinux resource' do
should_not contain_selboolean('httpd_can_network_connect')
end
end

describe 'with passenger' do
let(:params) do
super().merge(
Expand Down