Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes #34590 - Enable only minimum apache modules #754

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

wbclark
Copy link
Contributor

@wbclark wbclark commented Mar 10, 2022

The puppetlabs-apache puppet module used by the foreman-installer
installs Apache httpd with a set of default Apache modules, some of
which are not necessary for our use case. This commit reduces that set
of Apache modules down to a minimum, by overriding the
apache::default_mods parameter in the common tuning profile. The user
can enable additional modules if desired by overriding
apache::default_mods to an expanded list in custom-hiera.yaml.

@wbclark
Copy link
Contributor Author

wbclark commented Mar 10, 2022

(My testing thus far of this feature was in combination with #753 )

@wbclark
Copy link
Contributor Author

wbclark commented Mar 10, 2022

My current method of testing them (as well as the tuning changes in the other PR):

  1. Checkout https://github.com/wbclark/forklift/tree/event_mpm

  2. Inspect the changes in roles/foreman_installer/defaults/main.yml:

foreman_installer_custom_hiera: |
  apache::default_mods:
    - 'setenvif'
    - 'alias'
    - 'env'
  apache::mpm_module: 'event'
  apache::mod::event::serverlimit: 64
  apache::mod::event::startservers: 16
  apache::mod::event::maxrequestworkers: 1024
  apache::mod::event::minsparethreads: 128
  apache::mod::event::maxsparethreads: 384
  apache::mod::event::threadsperchild: 16
  apache::mod::event::maxconnectionsperchild: 4096
  1. Run ansible-playbook pipelines/install_pipeline.yml -e forklift_state=up -e pipeline_os=centos8-stream -e pipeline_type=luna -e pipeline_version=nightly

@ehelms
Copy link
Member

ehelms commented Apr 25, 2022

This seems more related to tightening our deployments to only what's necessary thus https://github.com/theforeman/foreman-installer/blob/develop/config/foreman.hiera/common.yaml is likely a better spot than tieing this to tuning.

@wbclark
Copy link
Contributor Author

wbclark commented Apr 25, 2022

Testing the latest version with https://github.com/wbclark/forklift/tree/default_mods (now updated to no longer require the previously unmerged branch of puppet-FPC)

To test, first clean up the previous environment if any:

$ for id in $(vagrant global-status --prune 2>&1 | grep $(pwd) | awk '{print $1}') ; do vagrant destroy -f $id ; done

Then run the test (Katello in this example):

$ ansible-playbook pipelines/install_pipeline.yml -e forklift_state=up -e pipeline_os=centos8-stream -e pipeline_type=katello -e pipeline_version=nightly

This runs foreman-installer nightly with apache::default_mods: false and it results in a successful install with the following httpd modules enabled for the Katello scenario:

# httpd -M
Loaded Modules:
 core_module (static)
 so_module (static)
 http_module (static)
 alias_module (shared)
 authz_core_module (shared)
 authz_host_module (shared)
 env_module (shared)
 filter_module (shared)
 headers_module (shared)
 log_config_module (shared)
 mime_module (shared)
 mpm_prefork_module (shared)
 proxy_module (shared)
 proxy_http_module (shared)
 proxy_wstunnel_module (shared)
 rewrite_module (shared)
 setenvif_module (shared)
 socache_shmcb_module (shared)
 ssl_module (shared)
 systemd_module (shared)
 unixd_module (shared)

And the following modules for the foreman-proxy-content scenario:

# httpd -M
Loaded Modules:
 core_module (static)
 so_module (static)
 http_module (static)
 alias_module (shared)
 authz_core_module (shared)
 authz_host_module (shared)
 filter_module (shared)
 headers_module (shared)
 log_config_module (shared)
 mime_module (shared)
 mpm_prefork_module (shared)
 proxy_module (shared)
 proxy_http_module (shared)
 socache_shmcb_module (shared)
 ssl_module (shared)
 systemd_module (shared)
 unixd_module (shared)

To make review easier, I'm now running a comparison pipeline which doesn't contain this change, so that diffs can be compared.

The puppetlabs-apache module installs Apache httpd with a set of default Apache modules, some of which are not necessary for our use case. This commit reduces that set of Apache modules to a minimum, by setting `apache::default_mods: false` in `config/foreman.hiera/common.yaml`. The user can enable additional modules if desired by overriding `apache::default_mods` in custom-hiera.yaml with a list of Apache modules. For more information, see documentation for puppetlabs-apache.
@wbclark wbclark force-pushed the 34590_default_apache_mods branch from ade32c7 to 77941fd Compare April 25, 2022 15:52
@wbclark
Copy link
Contributor Author

wbclark commented Apr 25, 2022

This seems more related to tightening our deployments to only what's necessary thus https://github.com/theforeman/foreman-installer/blob/develop/config/foreman.hiera/common.yaml is likely a better spot than tieing this to tuning.

That makes sense. I've moved it to config/foreman.hiera/common.yaml. Thanks!

I'll update with the latest diffs when the install pipeline completes without this configuration.

@wbclark
Copy link
Contributor Author

wbclark commented Apr 26, 2022

The comparing the diff of httpd -M in Katello scenario when disabling the apache default modules:

$ diff server_{,no}defaults
6,7d5
<  access_compat_module (shared)
<  actions_module (shared)
9,14d6
<  auth_basic_module (shared)
<  auth_digest_module (shared)
<  authn_anon_module (shared)
<  authn_core_module (shared)
<  authn_dbm_module (shared)
<  authn_file_module (shared)
16,17d7
<  authz_dbm_module (shared)
<  authz_groupfile_module (shared)
19,27d8
<  authz_owner_module (shared)
<  authz_user_module (shared)
<  autoindex_module (shared)
<  cache_module (shared)
<  cgi_module (shared)
<  dav_module (shared)
<  dav_fs_module (shared)
<  deflate_module (shared)
<  dir_module (shared)
29,30d9
<  expires_module (shared)
<  ext_filter_module (shared)
33d11
<  include_module (shared)
35d12
<  logio_module (shared)
37,38d13
<  mime_magic_module (shared)
<  negotiation_module (shared)
46d20
<  speling_module (shared)
48,49d21
<  substitute_module (shared)
<  suexec_module (shared)
52,55d23
<  usertrack_module (shared)
<  version_module (shared)
<  vhost_alias_module (shared)

And in foreman-proxy-content scenario:

$ diff proxy_{,no}defaults
5,6d4
<  access_compat_module (shared)
<  actions_module (shared)
8,13d5
<  auth_basic_module (shared)
<  auth_digest_module (shared)
<  authn_anon_module (shared)
<  authn_core_module (shared)
<  authn_dbm_module (shared)
<  authn_file_module (shared)
15,16d6
<  authz_dbm_module (shared)
<  authz_groupfile_module (shared)
18,29d7
<  authz_owner_module (shared)
<  authz_user_module (shared)
<  autoindex_module (shared)
<  cache_module (shared)
<  cgi_module (shared)
<  dav_module (shared)
<  dav_fs_module (shared)
<  deflate_module (shared)
<  dir_module (shared)
<  env_module (shared)
<  expires_module (shared)
<  ext_filter_module (shared)
32d9
<  include_module (shared)
34d10
<  logio_module (shared)
36,37d11
<  mime_magic_module (shared)
<  negotiation_module (shared)
41,42d14
<  rewrite_module (shared)
<  setenvif_module (shared)
44d15
<  speling_module (shared)
46,47d16
<  substitute_module (shared)
<  suexec_module (shared)
50,53d18
<  usertrack_module (shared)
<  version_module (shared)
<  vhost_alias_module (shared)

Copy link
Member

@ehelms ehelms left a comment

Choose a reason for hiding this comment

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

Looks OK to me, and tests will really give us insight. @evgeni if you wouldn't also mind taking a look to help ensure we didn't miss something

@evgeni
Copy link
Member

evgeni commented Apr 26, 2022

the list looks sane to me (besides using non-unified format 😝)

@ehelms
Copy link
Member

ehelms commented Apr 26, 2022

Thanks @wbclark

@ehelms ehelms merged commit b2b6eea into theforeman:develop Apr 26, 2022
@wbclark
Copy link
Contributor Author

wbclark commented Apr 26, 2022

the list looks sane to me (besides using non-unified format stuck_out_tongue_closed_eyes)

Hey come on now, this way you can see only what is removed :)

@gvde
Copy link

gvde commented Jul 15, 2022

This change will remove mod_autoindex from apache, which means there is no directory index anymore on foreman.example.com/pub/ which some people seem to find confusing.

https://community.theforeman.org/t/upgrade-to-rhel-8-6-and-foreman-3-3-pub-directory-returning-404-and-not-listing-anything/29176/24

@ekohl
Copy link
Member

ekohl commented Jul 15, 2022

theforeman/puppet-foreman_proxy_content#422 should address that.

@jhutar
Copy link

jhutar commented Aug 9, 2022

Wouldn't removing autoindex_module break .../pub/ file listing? Need to try.

@ekohl
Copy link
Member

ekohl commented Aug 9, 2022

@jhutar that's what https://projects.theforeman.org/issues/35227 is about already.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants