From 44eb382fbf69b81b0f9426697b49cfb1be3f978e Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:43:41 +0200 Subject: [PATCH 1/4] Properly require proxy mods This ensures the proxy module is always loaded at the right time. --- manifests/mod/proxy_ajp.pp | 2 +- manifests/mod/proxy_balancer.pp | 6 ++---- manifests/mod/proxy_connect.pp | 2 +- manifests/mod/proxy_fcgi.pp | 2 +- manifests/mod/proxy_html.pp | 4 ++-- manifests/mod/proxy_http.pp | 2 +- manifests/mod/proxy_wstunnel.pp | 4 ++-- manifests/vhost.pp | 1 - 8 files changed, 10 insertions(+), 13 deletions(-) diff --git a/manifests/mod/proxy_ajp.pp b/manifests/mod/proxy_ajp.pp index ff14f0a0b4..66b520c397 100644 --- a/manifests/mod/proxy_ajp.pp +++ b/manifests/mod/proxy_ajp.pp @@ -4,6 +4,6 @@ # @see https://httpd.apache.org/docs/current/mod/mod_proxy_ajp.html for additional documentation. # class apache::mod::proxy_ajp { - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_ajp'] + require apache::mod::proxy ::apache::mod { 'proxy_ajp': } } diff --git a/manifests/mod/proxy_balancer.pp b/manifests/mod/proxy_balancer.pp index 74f35ba46e..1616f69d84 100644 --- a/manifests/mod/proxy_balancer.pp +++ b/manifests/mod/proxy_balancer.pp @@ -21,14 +21,12 @@ Array $allow_from = ['127.0.0.1','::1'], Optional[String] $apache_version = $apache::apache_version, ) { - include apache::mod::proxy - include apache::mod::proxy_http + require apache::mod::proxy + require apache::mod::proxy_http if versioncmp($apache_version, '2.4') >= 0 { ::apache::mod { 'slotmem_shm': } } - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_balancer'] - Class['apache::mod::proxy_http'] -> Class['apache::mod::proxy_balancer'] ::apache::mod { 'proxy_balancer': } if $manager { include apache::mod::status diff --git a/manifests/mod/proxy_connect.pp b/manifests/mod/proxy_connect.pp index 5ece8d4666..5134639700 100644 --- a/manifests/mod/proxy_connect.pp +++ b/manifests/mod/proxy_connect.pp @@ -4,6 +4,6 @@ # class apache::mod::proxy_connect { include apache - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_connect'] + require apache::mod::proxy apache::mod { 'proxy_connect': } } diff --git a/manifests/mod/proxy_fcgi.pp b/manifests/mod/proxy_fcgi.pp index 3196f5b68d..467bdbef3e 100644 --- a/manifests/mod/proxy_fcgi.pp +++ b/manifests/mod/proxy_fcgi.pp @@ -4,6 +4,6 @@ # @see https://httpd.apache.org/docs/current/mod/mod_proxy_fcgi.html for additional documentation. # class apache::mod::proxy_fcgi { - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_fcgi'] + require apache::mod::proxy ::apache::mod { 'proxy_fcgi': } } diff --git a/manifests/mod/proxy_html.pp b/manifests/mod/proxy_html.pp index 881d7e87b3..da57b5dbcf 100644 --- a/manifests/mod/proxy_html.pp +++ b/manifests/mod/proxy_html.pp @@ -5,8 +5,8 @@ # class apache::mod::proxy_html { include apache - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_html'] - Class['apache::mod::proxy_http'] -> Class['apache::mod::proxy_html'] + require apache::mod::proxy + require apache::mod::proxy_http # Add libxml2 case $facts['os']['family'] { diff --git a/manifests/mod/proxy_http.pp b/manifests/mod/proxy_http.pp index 76b7667ea2..54ad8af43c 100644 --- a/manifests/mod/proxy_http.pp +++ b/manifests/mod/proxy_http.pp @@ -4,6 +4,6 @@ # @see https://httpd.apache.org/docs/current/mod/mod_proxy_http.html for additional documentation. # class apache::mod::proxy_http { - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_http'] + require apache::mod::proxy ::apache::mod { 'proxy_http': } } diff --git a/manifests/mod/proxy_wstunnel.pp b/manifests/mod/proxy_wstunnel.pp index 1d12c6b465..c57db79387 100644 --- a/manifests/mod/proxy_wstunnel.pp +++ b/manifests/mod/proxy_wstunnel.pp @@ -4,7 +4,7 @@ # @see https://httpd.apache.org/docs/current/mod/mod_proxy_wstunnel.html for additional documentation. # class apache::mod::proxy_wstunnel { - include apache, apache::mod::proxy - Class['apache::mod::proxy'] -> Class['apache::mod::proxy_wstunnel'] + include apache + require apache::mod::proxy ::apache::mod { 'proxy_wstunnel': } } diff --git a/manifests/vhost.pp b/manifests/vhost.pp index d5aa692678..5aa736d5bc 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2488,7 +2488,6 @@ # - $proxy_add_headers # - $no_proxy_uris if ($proxy_dest or $proxy_pass or $proxy_pass_match or $proxy_dest_match or $proxy_preserve_host) and $ensure == 'present' { - include apache::mod::proxy include apache::mod::proxy_http concat::fragment { "${name}-proxy": From c1a95383a49801585cdfd37938719040b505794d Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 28 Jun 2022 15:58:02 +0200 Subject: [PATCH 2/4] Factor out mod_authz_groupfile to its own class This makes it easier to run with default mods disabled while still using this mod. --- manifests/default_mods.pp | 3 +-- manifests/mod/authz_groupfile.pp | 9 +++++++++ spec/classes/mod/authz_groupfile_spec.rb | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 manifests/mod/authz_groupfile.pp create mode 100644 spec/classes/mod/authz_groupfile_spec.rb diff --git a/manifests/default_mods.pp b/manifests/default_mods.pp index 083f1bccd3..68bf56ac53 100644 --- a/manifests/default_mods.pp +++ b/manifests/default_mods.pp @@ -157,8 +157,7 @@ } include apache::mod::authz_user - - ::apache::mod { 'authz_groupfile': } + include apache::mod::authz_groupfile include apache::mod::env } elsif $mods { ::apache::default_mods::load { $mods: } diff --git a/manifests/mod/authz_groupfile.pp b/manifests/mod/authz_groupfile.pp new file mode 100644 index 0000000000..7bf0e795fd --- /dev/null +++ b/manifests/mod/authz_groupfile.pp @@ -0,0 +1,9 @@ +# @summary +# Installs `mod_authz_groupfile` +# +# @see https://httpd.apache.org/docs/current/mod/mod_authz_groupfile.html for additional documentation. +# +class apache::mod::authz_groupfile { + include apache + apache::mod { 'authz_groupfile': } +} diff --git a/spec/classes/mod/authz_groupfile_spec.rb b/spec/classes/mod/authz_groupfile_spec.rb new file mode 100644 index 0000000000..dbab5e7392 --- /dev/null +++ b/spec/classes/mod/authz_groupfile_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'apache::mod::authz_groupfile' do + it_behaves_like 'a mod class, without including apache' + + context 'default configuration with parameters' do + context 'on a Debian OS' do + include_examples 'Debian 11' + + it { is_expected.to contain_apache__mod('authz_groupfile') } + end + end +end From de70a475309bd07ff2605a17ad786d06062558e0 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:34:46 +0200 Subject: [PATCH 3/4] Tighten apache::vhost::directories parameter This tightens the data type to a simple array of hashes (as the parameter documentation suggests). This allows the logic to be simpler. Because of this, it is now fairly easy to include the correct modules if needed. Some options are implemented but there are more which could be automatically included. --- manifests/vhost.pp | 56 +++++++++++++++++++------------- spec/acceptance/vhost_spec.rb | 5 ++- templates/vhost/_directories.erb | 4 +-- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 5aa736d5bc..21a531f7f9 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -428,12 +428,14 @@ # krb_method_negotiate => 'on', # krb_auth_realms => ['EXAMPLE.ORG'], # krb_local_user_mapping => 'on', -# directories => { -# path => '/var/www/html', -# auth_name => 'Kerberos Login', -# auth_type => 'Kerberos', -# auth_require => 'valid-user', -# }, +# directories => [ +# { +# path => '/var/www/html', +# auth_name => 'Kerberos Login', +# auth_type => 'Kerberos', +# auth_require => 'valid-user', +# }, +# ], # } # ``` # @@ -1163,9 +1165,12 @@ # suphp_addhandler => 'x-httpd-php', # suphp_engine => 'on', # suphp_configpath => '/etc/php5/apache2', -# directories => { path => '/home/appuser/myphpapp', -# 'suphp' => { user => 'myappuser', group => 'myappgroup' }, -# } +# directories => [ +# { +# 'path' => '/home/appuser/myphpapp', +# 'suphp' => { user => 'myappuser', group => 'myappgroup' }, +# }, +# ], # } # ``` # @@ -1180,9 +1185,12 @@ # suphp_addhandler => 'x-httpd-php', # suphp_engine => 'on', # suphp_configpath => '/etc/php5/apache2', -# directories => { path => '/home/appuser/myphpapp', -# 'suphp' => { user => 'myappuser', group => 'myappgroup' }, -# } +# directories => [ +# { +# 'path' => '/home/appuser/myphpapp', +# 'suphp' => { user => 'myappuser', group => 'myappgroup' }, +# }, +# ], # } # ``` # @@ -1197,9 +1205,12 @@ # suphp_addhandler => 'x-httpd-php', # suphp_engine => 'on', # suphp_configpath => '/etc/php5/apache2', -# directories => { path => '/home/appuser/myphpapp', -# 'suphp' => { user => 'myappuser', group => 'myappgroup' }, -# } +# directories => [ +# { +# 'path' => '/home/appuser/myphpapp', +# 'suphp' => { user => 'myappuser', group => 'myappgroup' }, +# }, +# ], # } # ``` # @@ -1407,10 +1418,12 @@ # ``` puppet # apache::vhost { 'sample.example.net': # docroot => '/path/to/directory', -# directories => { -# path => '/path/to/directory', -# headers => 'Set X-Robots-Tag "noindex, noarchive, nosnippet"', -# }, +# directories => [ +# { +# path => '/path/to/directory', +# headers => 'Set X-Robots-Tag "noindex, noarchive, nosnippet"', +# }, +# ], # } # ``` # @@ -1457,7 +1470,6 @@ # @param gssapi # Specfies mod_auth_gssapi parameters for particular directories in a virtual host directory # ```puppet -# include apache::mod::auth_gssapi # apache::vhost { 'sample.example.net': # docroot => '/path/to/directory', # directories => [ @@ -1804,7 +1816,7 @@ Boolean $use_servername_for_filenames = false, Boolean $use_port_for_filenames = false, Array[Hash[String[1], String[1]]] $aliases = [], - Optional[Variant[Hash, Array[Variant[Array,Hash]]]] $directories = undef, + Optional[Array[Hash]] $directories = undef, Boolean $error_log = true, Optional[String] $error_log_file = undef, Optional[String] $error_log_pipe = undef, @@ -2353,7 +2365,7 @@ # - $apache_version # - $suphp_engine # - $shibboleth_enabled - if $_directories and ! empty($_directories) { + if $_directories and ! empty($_directories) and $ensure == 'present' { concat::fragment { "${name}-directories": target => "${priority_real}${filename}.conf", order => 60, diff --git a/spec/acceptance/vhost_spec.rb b/spec/acceptance/vhost_spec.rb index a5b54f635d..b2dcc11f11 100644 --- a/spec/acceptance/vhost_spec.rb +++ b/spec/acceptance/vhost_spec.rb @@ -327,7 +327,7 @@ class { 'apache': } class { 'apache': } if versioncmp($apache_version, '2.4') >= 0 { - $_files_match_directory = { 'path' => 'private.html$', 'provider' => 'filesmatch', 'require' => 'all denied' } + $_files_match_directory = [{ 'path' => 'private.html$', 'provider' => 'filesmatch', 'require' => 'all denied' }] } else { $_files_match_directory = [ { 'path' => 'private.html$', 'provider' => 'filesmatch', 'deny' => 'from all' }, @@ -338,8 +338,7 @@ class { 'apache': } $_directories = [ { 'path' => '/var/www/files', }, { 'path' => '/foo/', 'provider' => 'location', 'directoryindex' => 'notindex.html', }, - $_files_match_directory, - ] + ] + $_files_match_directory apache::vhost { 'files.example.net': docroot => '/var/www/files', diff --git a/templates/vhost/_directories.erb b/templates/vhost/_directories.erb index 4b5a7523f4..406dd24828 100644 --- a/templates/vhost/_directories.erb +++ b/templates/vhost/_directories.erb @@ -1,8 +1,7 @@ -<% if @_directories and ! @_directories.empty? -%> <%- scope.setvar('_template_scope', {}) -%> ## Directories, there should at least be a declaration for <%= @docroot %> - <%- [@_directories].flatten.compact.each do |directory| -%> + <%- @_directories.each do |directory| -%> <%- if scope.function_versioncmp([@apache_version, '2.4']) >= 0 -%> <%- if directory['allow'] and ! [ false, 'false', '' ].include?(directory['allow']) -%> <%- scope.function_warning(["Apache::Vhost: Using allow is deprecated in your Apache version"]) -%> @@ -520,4 +519,3 @@ > <%- end -%> <%- end -%> -<%- end -%> From 040eab99941978665c4ebe37e0932470d8627918 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:34:46 +0200 Subject: [PATCH 4/4] Automatically include modules used in vhost directories The data passed to the directories parameter of apache::vhost often implies a module should be present. This change inspects the passed data and automatically includes modules if needed. This makes it easier to use via Hiera while disabling all default mods. Because of this change one acceptance test becomes obsolete: it no longer fails and just works. --- manifests/vhost.pp | 34 +++++++++++++++++++++++ spec/acceptance/default_mods_spec.rb | 41 ---------------------------- spec/defines/vhost_spec.rb | 10 ++++++- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 21a531f7f9..09c3100a6e 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2366,6 +2366,40 @@ # - $suphp_engine # - $shibboleth_enabled if $_directories and ! empty($_directories) and $ensure == 'present' { + $_directories.each |Hash $directory| { + if 'auth_basic_authoritative' in $directory or 'auth_basic_fake' in $directory or 'auth_basic_provider' in $directory { + include apache::mod::auth_basic + } + + if 'auth_user_file' in $directory { + include apache::mod::authn_file + } + + if 'auth_group_file' in $directory { + include apache::mod::authz_groupfile + } + + if 'gssapi' in $directory { + include apache::mod::auth_gssapi + } + + if $directory['provider'] and $directory['provider'] =~ 'location' and ('proxy_pass' in $directory or 'proxy_pass_match' in $directory) { + include apache::mod::proxy_http + } + + if 'request_headers' in $directory { + include apache::mod::headers + } + + if 'rewrites' in $directory { + include apache::mod::rewrite + } + + if 'setenv' in $directory { + include apache::mod::env + } + } + concat::fragment { "${name}-directories": target => "${priority_real}${filename}.conf", order => 60, diff --git a/spec/acceptance/default_mods_spec.rb b/spec/acceptance/default_mods_spec.rb index 8e38d98f0f..1694b94c92 100644 --- a/spec/acceptance/default_mods_spec.rb +++ b/spec/acceptance/default_mods_spec.rb @@ -22,47 +22,6 @@ class { 'apache': end end - unless os[:family] == 'sles' && os[:release].to_i >= 12 - describe 'no default mods and failing' do - before :all do - pp = <<-PP - include apache::params - class { 'apache': default_mods => false, service_ensure => stopped, } - PP - apply_manifest(pp) - end - # Using puppet_apply as a helper - pp = <<-MANIFEST - class { 'apache': - default_mods => false, - } - apache::vhost { 'defaults.example.com': - docroot => '#{apache_hash['doc_root']}/defaults', - aliases => [ - { - alias => '/css', - path => '#{apache_hash['doc_root']}/css', - }, - ], - directories => [ - { - 'path' => "#{apache_hash['doc_root']}/admin", - 'auth_basic_fake' => 'demo demopass', - } - ], - setenv => 'TEST1 one', - } - MANIFEST - it 'applies with errors' do - apply_manifest(pp, expect_failures: true) - end - end - - describe service(apache_hash['service_name']) do - it { is_expected.not_to be_running } - end - end - describe 'alternative default mods' do # Using puppet_apply as a helper let(:pp) do diff --git a/spec/defines/vhost_spec.rb b/spec/defines/vhost_spec.rb index eab020c6f4..eaa3af9946 100644 --- a/spec/defines/vhost_spec.rb +++ b/spec/defines/vhost_spec.rb @@ -194,6 +194,10 @@ 'path' => '/', 'provider' => 'location', 'auth_ldap_referrals' => 'off', + 'auth_basic_fake' => 'demo demopass', + 'auth_user_file' => '/path/to/authz_user_file', + 'auth_group_file' => '/path/to/authz_group_file', + 'setenv' => ['SPECIAL_PATH /foo/bin'], }, { 'path' => '/proxy', @@ -568,6 +572,10 @@ 'mode' => '0600') } it { is_expected.to contain_class('apache::mod::alias') } + it { is_expected.to contain_class('apache::mod::auth_basic') } + it { is_expected.to contain_class('apache::mod::authn_file') } + it { is_expected.to contain_class('apache::mod::authz_groupfile') } + it { is_expected.to contain_class('apache::mod::auth_gssapi') } it { is_expected.to contain_class('apache::mod::env') } it { is_expected.to contain_class('apache::mod::filter') } it { is_expected.to contain_class('apache::mod::headers') } @@ -1289,7 +1297,7 @@ it { is_expected.not_to contain_concat__fragment('rspec.example.com-aliases') } it { is_expected.not_to contain_concat__fragment('rspec.example.com-itk') } it { is_expected.not_to contain_concat__fragment('rspec.example.com-fallbackresource') } - it { is_expected.to contain_concat__fragment('rspec.example.com-directories') } + it { is_expected.not_to contain_concat__fragment('rspec.example.com-directories') } it { is_expected.not_to contain_concat__fragment('rspec.example.com-additional_includes') } it { is_expected.to contain_concat__fragment('rspec.example.com-logging') } it { is_expected.to contain_concat__fragment('rspec.example.com-serversignature') }