From 11d37f4fcec19ac605dd2fcb152c1535d59d5709 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:37:46 +0200 Subject: [PATCH 01/19] Use a stricter data type on apache::vhost::aliases This also implements a real test on the rendered template. It appears it was passing in something invalid ever since it was introduced in f1d64a0a0b71af0102d10da69e333aa9ac5a15f5. --- manifests/vhost.pp | 7 ++++--- spec/acceptance/default_mods_spec.rb | 20 ++++++++++++-------- spec/defines/vhost_spec.rb | 13 +++++++++++-- templates/vhost/_aliases.erb | 4 ++-- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index a92331f68d..6a80e351c2 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -1803,7 +1803,7 @@ Optional[Array[Hash]] $access_logs = undef, Boolean $use_servername_for_filenames = false, Boolean $use_port_for_filenames = false, - Optional[Variant[Array[Hash],Hash,String]] $aliases = undef, + Array[Hash] $aliases = [], Optional[Variant[Hash, Array[Variant[Array,Hash]]]] $directories = undef, Boolean $error_log = true, Optional[String] $error_log_file = undef, @@ -2222,7 +2222,6 @@ # Load mod_alias if needed and not yet loaded if ($scriptalias or $scriptaliases != []) - or ($aliases and $aliases != []) or ($redirect_source and $redirect_dest) or ($redirectmatch_regexp or $redirectmatch_status or $redirectmatch_dest) { if ! defined(Class['apache::mod::alias']) and ($ensure == 'present') { @@ -2376,7 +2375,9 @@ # Template uses: # - $aliases - if $aliases and ! empty($aliases) { + if ! empty($aliases) and $ensure == 'present' { + include apache::mod::alias + concat::fragment { "${name}-aliases": target => "${priority_real}${filename}.conf", order => 20, diff --git a/spec/acceptance/default_mods_spec.rb b/spec/acceptance/default_mods_spec.rb index fa5a6beabe..8e38d98f0f 100644 --- a/spec/acceptance/default_mods_spec.rb +++ b/spec/acceptance/default_mods_spec.rb @@ -38,10 +38,12 @@ class { 'apache': } apache::vhost { 'defaults.example.com': docroot => '#{apache_hash['doc_root']}/defaults', - aliases => { - alias => '/css', - path => '#{apache_hash['doc_root']}/css', - }, + aliases => [ + { + alias => '/css', + path => '#{apache_hash['doc_root']}/css', + }, + ], directories => [ { 'path' => "#{apache_hash['doc_root']}/admin", @@ -76,10 +78,12 @@ class { 'apache': } apache::vhost { 'defaults.example.com': docroot => '#{apache_hash['doc_root']}/defaults', - aliases => { - alias => '/css', - path => '#{apache_hash['doc_root']}/css', - }, + aliases => [ + { + alias => '/css', + path => '#{apache_hash['doc_root']}/css', + }, + ], setenv => 'TEST1 one', } MANIFEST diff --git a/spec/defines/vhost_spec.rb b/spec/defines/vhost_spec.rb index 3a408977cc..67973fa080 100644 --- a/spec/defines/vhost_spec.rb +++ b/spec/defines/vhost_spec.rb @@ -109,7 +109,12 @@ 'access_log_syslog' => true, 'access_log_format' => '%h %l %u %t \"%r\" %>s %b', 'access_log_env_var' => '', - 'aliases' => '/image', + 'aliases' => [ + { + 'alias' => '/image', + 'path' => '/rspec/image', + }, + ], 'directories' => [ { 'path' => '/var/www/files', @@ -619,7 +624,11 @@ ) } it { is_expected.to contain_concat__fragment('rspec.example.com-docroot') } - it { is_expected.to contain_concat__fragment('rspec.example.com-aliases') } + it { + is_expected.to contain_concat__fragment('rspec.example.com-aliases').with( + content: %r{^\s+Alias /image "/rspec/image"$}, + ) + } it { is_expected.to contain_concat__fragment('rspec.example.com-itk') } it { is_expected.to contain_concat__fragment('rspec.example.com-fallbackresource') } it { is_expected.to contain_concat__fragment('rspec.example.com-directories') } diff --git a/templates/vhost/_aliases.erb b/templates/vhost/_aliases.erb index f9771bc728..024f05723f 100644 --- a/templates/vhost/_aliases.erb +++ b/templates/vhost/_aliases.erb @@ -1,6 +1,6 @@ -<% if @aliases and ! @aliases.empty? -%> +<% unless @aliases.empty? -%> ## Alias declarations for resources outside the DocumentRoot - <%- [@aliases].flatten.compact.each do |alias_statement| -%> + <%- @aliases.each do |alias_statement| -%> <%- if alias_statement["path"] != '' -%> <%- if alias_statement["alias"] and alias_statement["alias"] != '' -%> Alias <%= alias_statement["alias"] %> "<%= alias_statement["path"] %>" From 64465808f8d2c157b4107a3b5c3f4c41b26c3659 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:43:12 +0200 Subject: [PATCH 02/19] Correct parameter alignment --- manifests/vhost.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 6a80e351c2..cb7283d6e4 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -1847,7 +1847,7 @@ Boolean $proxy_preserve_host = false, Optional[Variant[String,Boolean]] $proxy_add_headers = undef, Boolean $proxy_error_override = false, - Variant[String,Array[String]] $redirect_source = '/', + Variant[String,Array[String]] $redirect_source = '/', Optional[Variant[Array[String],String]] $redirect_dest = undef, Optional[Variant[Array[String],String]] $redirect_status = undef, Optional[Variant[Array[String],String]] $redirectmatch_status = undef, From 7bf82633c07ab6be5a26e06bdb9e85c1d6b47ee2 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:46:20 +0200 Subject: [PATCH 03/19] Include apache::mod::alias locally --- manifests/vhost.pp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index cb7283d6e4..ee552ba975 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2220,15 +2220,6 @@ } } - # Load mod_alias if needed and not yet loaded - if ($scriptalias or $scriptaliases != []) - or ($redirect_source and $redirect_dest) - or ($redirectmatch_regexp or $redirectmatch_status or $redirectmatch_dest) { - if ! defined(Class['apache::mod::alias']) and ($ensure == 'present') { - include apache::mod::alias - } - } - # Load mod_proxy if needed and not yet loaded if ($proxy_dest or $proxy_pass or $proxy_pass_match or $proxy_dest_match) { if ! defined(Class['apache::mod::proxy']) { @@ -2577,7 +2568,9 @@ # - $redirectmatch_status_a # - $redirectmatch_regexp_a # - $redirectmatch_dest - if ($redirect_source and $redirect_dest) or ($redirectmatch_regexp and $redirectmatch_dest) { + if (($redirect_source and $redirect_dest) or ($redirectmatch_regexp and $redirectmatch_dest)) and $ensure == 'present' { + include apache::mod::alias + concat::fragment { "${name}-redirect": target => "${priority_real}${filename}.conf", order => 180, @@ -2604,7 +2597,9 @@ # Template uses: # - $scriptaliases # - $scriptalias - if ( $scriptalias or $scriptaliases != []) { + if ($scriptalias or !empty($scriptaliases)) and $ensure == 'present' { + include apache::mod::alias + concat::fragment { "${name}-scriptalias": target => "${priority_real}${filename}.conf", order => 200, From a69c33a9620455626f4e61f2e80cdcdb5a233951 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:47:47 +0200 Subject: [PATCH 04/19] Include apache::mod::proxy locally --- manifests/vhost.pp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index ee552ba975..5e19fa425c 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2220,16 +2220,6 @@ } } - # Load mod_proxy if needed and not yet loaded - if ($proxy_dest or $proxy_pass or $proxy_pass_match or $proxy_dest_match) { - if ! defined(Class['apache::mod::proxy']) { - include apache::mod::proxy - } - if ! defined(Class['apache::mod::proxy_http']) { - include apache::mod::proxy_http - } - } - # Load mod_fastcgi if needed and not yet loaded if $fastcgi_server and $fastcgi_socket { if ! defined(Class['apache::mod::fastcgi']) { @@ -2547,7 +2537,10 @@ # - $proxy_preserve_host # - $proxy_add_headers # - $no_proxy_uris - if $proxy_dest or $proxy_pass or $proxy_pass_match or $proxy_dest_match or $proxy_preserve_host { + 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": target => "${priority_real}${filename}.conf", order => 170, From 3d5015796ca38f2b078a0af9ad7a784fb2d39a7f Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:48:56 +0200 Subject: [PATCH 05/19] Inlcude apache::mod::fastcgi locally --- manifests/vhost.pp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 5e19fa425c..965162a018 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2220,13 +2220,6 @@ } } - # Load mod_fastcgi if needed and not yet loaded - if $fastcgi_server and $fastcgi_socket { - if ! defined(Class['apache::mod::fastcgi']) { - include apache::mod::fastcgi - } - } - # Check if mod_env is required and not yet loaded. # create an expression to simplify the conditional check $use_env_mod = $setenv and ! empty($setenv) @@ -2752,7 +2745,9 @@ # - $fastcgi_dir # - $fastcgi_idle_timeout # - $apache_version - if $fastcgi_server or $fastcgi_dir { + if ($fastcgi_server or $fastcgi_dir) and $ensure == 'present' { + include apache::mod::fastcgi + concat::fragment { "${name}-fastcgi": target => "${priority_real}${filename}.conf", order => 280, From de7f60a7e36fb30290cba60aa120d259446d6860 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:51:52 +0200 Subject: [PATCH 06/19] Clean up inclusion of mod::env and mod::setenvif There is no need to check if the variables are true because the data type doesn't allow anything that evaluates to false. The logic to include it is also moved to the template which makes it easier to follow the logic. The same is applied to the template. --- manifests/vhost.pp | 29 ++++++++++------------------- templates/vhost/_setenv.erb | 6 +++--- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 965162a018..22f24ebd80 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2220,24 +2220,6 @@ } } - # Check if mod_env is required and not yet loaded. - # create an expression to simplify the conditional check - $use_env_mod = $setenv and ! empty($setenv) - if ($use_env_mod) { - if ! defined(Class['apache::mod::env']) { - include apache::mod::env - } - } - # Check if mod_setenvif is required and not yet loaded. - # create an expression to simplify the conditional check - $use_setenvif_mod = ($setenvif and ! empty($setenvif)) or ($setenvifnocase and ! empty($setenvifnocase)) - - if ($use_setenvif_mod) { - if ! defined(Class['apache::mod::setenvif']) { - include apache::mod::setenvif - } - } - ## Create a default directory list if none defined if $directories { $_directories = $directories @@ -2606,7 +2588,16 @@ # Template uses: # - $setenv # - $setenvif - if ($use_env_mod or $use_setenvif_mod) { + $use_env_mod = !empty($setenv) + $use_setenvif_mod = !empty($setenvif) or !empty($setenvifnocase) + if ($use_env_mod or $use_setenvif_mod) and $ensure == 'present' { + if $use_env_mod { + include apache::mod::env + } + if $use_setenvif_mod { + include apache::mod::setenvif + } + concat::fragment { "${name}-setenv": target => "${priority_real}${filename}.conf", order => 220, diff --git a/templates/vhost/_setenv.erb b/templates/vhost/_setenv.erb index 476a6b19ca..0bcbc1bd57 100644 --- a/templates/vhost/_setenv.erb +++ b/templates/vhost/_setenv.erb @@ -1,16 +1,16 @@ -<% if @setenv and ! @setenv.empty? -%> +<% unless @setenv.empty? -%> ## SetEnv/SetEnvIf for environment variables <%- Array(@setenv).each do |envvar| -%> SetEnv <%= envvar %> <%- end -%> <% end -%> -<% if @setenvif and ! @setenvif.empty? -%> +<% unless @setenvif.empty? -%> <%- Array(@setenvif).each do |envifvar| -%> SetEnvIf <%= envifvar %> <%- end -%> <% end -%> -<% if @setenvifnocase and ! @setenvifnocase.empty? -%> +<% unless @setenvifnocase.empty? -%> <%- Array(@setenvifnocase).each do |envifncvar| -%> SetEnvIfNoCase <%= envifncvar %> <%- end -%> From 0df7f8f7314b15d6c107ceedc6ac98e101e2442c Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:54:58 +0200 Subject: [PATCH 07/19] Simplify scriptalias template The data type only allows an array for scriptaliases so there is no need to check for the type. It is also checked at in vhost.pp that the value is set so there's no need to repeat that logic in the template. --- templates/vhost/_scriptalias.erb | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/templates/vhost/_scriptalias.erb b/templates/vhost/_scriptalias.erb index bb4f6b316e..ffa69f932e 100644 --- a/templates/vhost/_scriptalias.erb +++ b/templates/vhost/_scriptalias.erb @@ -1,17 +1,8 @@ -<%- if @scriptaliases.is_a?(Array) -%> -<%- aliases = @scriptaliases -%> -<%- elsif @scriptaliases.is_a?(Hash) -%> -<%- aliases = [@scriptaliases] -%> -<%- else -%> -<%- # Nothing to do with any other data type -%> -<%- aliases = [] -%> -<%- end -%> -<%- if @scriptalias or !aliases.empty? -%> +<%- aliases = @scriptaliases -%> ## Script alias directives -<%# Combine scriptalais and scriptaliases into a single data structure -%> +<%# Combine scriptalias and scriptaliases into a single data structure -%> <%# for backward compatibility and ease of implementation -%> <%- aliases << { 'alias' => '/cgi-bin', 'path' => @scriptalias } if @scriptalias -%> -<%- aliases.flatten.compact! -%> <%- aliases.each do |salias| -%> <%- if salias["path"] != '' -%> <%- if salias["alias"] and salias["alias"] != '' -%> @@ -21,4 +12,3 @@ <%- end -%> <%- end -%> <%- end -%> -<%- end -%> From e92c1952c94ca084adbbd26c473d9eff4209cb01 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:00:09 +0200 Subject: [PATCH 08/19] Localize apache::mod::vhost_alias inclusion This is now only included if docroot evaluates to true. The template does suggest that it should be `($docroot or $virtual_docroot)` but this remains compatible with the previous code. --- manifests/vhost.pp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 22f24ebd80..149759de1a 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2039,10 +2039,6 @@ include apache::mod::auth_openidc } - if $virtual_docroot { - include apache::mod::vhost_alias - } - if $wsgi_application_group or $wsgi_daemon_process or ($wsgi_import_script and $wsgi_import_script_options) or $wsgi_process_group or ($wsgi_script_aliases and ! empty($wsgi_script_aliases)) or $wsgi_pass_authorization { include apache::mod::wsgi } @@ -2321,7 +2317,11 @@ # - $virtual_docroot # - $virtual_use_default_docroot # - $docroot - if $docroot { + if $docroot and $ensure == 'present' { + if $virtual_docroot { + include apache::mod::vhost_alias + } + concat::fragment { "${name}-docroot": target => "${priority_real}${filename}.conf", order => 10, From 2f376bcddfca6a64488418fca58591790da067ca Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:04:34 +0200 Subject: [PATCH 09/19] Localize apache::mod::wsgi inclusion --- manifests/vhost.pp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 149759de1a..33a7428048 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2039,10 +2039,6 @@ include apache::mod::auth_openidc } - if $wsgi_application_group or $wsgi_daemon_process or ($wsgi_import_script and $wsgi_import_script_options) or $wsgi_process_group or ($wsgi_script_aliases and ! empty($wsgi_script_aliases)) or $wsgi_pass_authorization { - include apache::mod::wsgi - } - if $suexec_user_group { include apache::mod::suexec } @@ -2712,7 +2708,9 @@ if $wsgi_daemon_process_options { deprecation('apache::vhost::wsgi_daemon_process_options', 'This parameter is deprecated. Please add values inside Hash `wsgi_daemon_process`.') } - if $wsgi_application_group or $wsgi_daemon_process or ($wsgi_import_script and $wsgi_import_script_options) or $wsgi_process_group or ($wsgi_script_aliases and ! empty($wsgi_script_aliases)) or $wsgi_pass_authorization { + if ($wsgi_application_group or $wsgi_daemon_process or ($wsgi_import_script and $wsgi_import_script_options) or $wsgi_process_group or ($wsgi_script_aliases and ! empty($wsgi_script_aliases)) or $wsgi_pass_authorization) and $ensure == 'present' { + include apache::mod::wsgi + concat::fragment { "${name}-wsgi": target => "${priority_real}${filename}.conf", order => 260, From 8ba9830936e0eaaca914f66ffd335c832e6ce2d2 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:05:27 +0200 Subject: [PATCH 10/19] Localize apache::mod::suexec inclusion --- manifests/vhost.pp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 33a7428048..add236f05e 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2039,10 +2039,6 @@ include apache::mod::auth_openidc } - if $suexec_user_group { - include apache::mod::suexec - } - if $passenger_enabled != undef or $passenger_start_timeout != undef or $passenger_ruby != undef or $passenger_python != undef or $passenger_nodejs != undef or $passenger_meteor_app_settings != undef or $passenger_app_env != undef or $passenger_app_root != undef or $passenger_app_group_name != undef or $passenger_app_start_command != undef or $passenger_app_type != undef or $passenger_startup_file != undef or $passenger_restart_dir != undef or $passenger_spawn_method != undef or $passenger_load_shell_envvars != undef or $passenger_preload_bundler != undef or $passenger_rolling_restarts != undef or $passenger_resist_deployment_errors != undef or $passenger_min_instances != undef or $passenger_max_instances != undef or $passenger_max_preloader_idle_time != undef or $passenger_force_max_concurrent_requests_per_process != undef or $passenger_concurrency_model != undef or $passenger_thread_count != undef or $passenger_high_performance != undef or $passenger_max_request_queue_size != undef or $passenger_max_request_queue_time != undef or $passenger_user != undef or $passenger_group != undef or $passenger_friendly_error_pages != undef or $passenger_buffer_upload != undef or $passenger_buffer_response != undef or $passenger_allow_encoded_slashes != undef or $passenger_lve_min_uid != undef or $passenger_base_uri != undef or $passenger_error_override != undef or $passenger_sticky_sessions != undef or $passenger_sticky_sessions_cookie_name != undef or $passenger_sticky_sessions_cookie_attributes != undef or $passenger_app_log_file != undef or $passenger_debugger != undef or $passenger_max_requests != undef or $passenger_max_request_time != undef or $passenger_memory_limit != undef { include apache::mod::passenger } @@ -2746,7 +2742,9 @@ # Template uses: # - $suexec_user_group - if $suexec_user_group { + if $suexec_user_group and $ensure == 'present' { + include apache::mod::suexec + concat::fragment { "${name}-suexec": target => "${priority_real}${filename}.conf", order => 290, From dfd14f9e6533f9a548b7a6bf29e6544ee2125501 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:06:37 +0200 Subject: [PATCH 11/19] Localize apache::mod::passenger inclusion --- manifests/vhost.pp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index add236f05e..426a4a5784 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2039,10 +2039,6 @@ include apache::mod::auth_openidc } - if $passenger_enabled != undef or $passenger_start_timeout != undef or $passenger_ruby != undef or $passenger_python != undef or $passenger_nodejs != undef or $passenger_meteor_app_settings != undef or $passenger_app_env != undef or $passenger_app_root != undef or $passenger_app_group_name != undef or $passenger_app_start_command != undef or $passenger_app_type != undef or $passenger_startup_file != undef or $passenger_restart_dir != undef or $passenger_spawn_method != undef or $passenger_load_shell_envvars != undef or $passenger_preload_bundler != undef or $passenger_rolling_restarts != undef or $passenger_resist_deployment_errors != undef or $passenger_min_instances != undef or $passenger_max_instances != undef or $passenger_max_preloader_idle_time != undef or $passenger_force_max_concurrent_requests_per_process != undef or $passenger_concurrency_model != undef or $passenger_thread_count != undef or $passenger_high_performance != undef or $passenger_max_request_queue_size != undef or $passenger_max_request_queue_time != undef or $passenger_user != undef or $passenger_group != undef or $passenger_friendly_error_pages != undef or $passenger_buffer_upload != undef or $passenger_buffer_response != undef or $passenger_allow_encoded_slashes != undef or $passenger_lve_min_uid != undef or $passenger_base_uri != undef or $passenger_error_override != undef or $passenger_sticky_sessions != undef or $passenger_sticky_sessions_cookie_name != undef or $passenger_sticky_sessions_cookie_attributes != undef or $passenger_app_log_file != undef or $passenger_debugger != undef or $passenger_max_requests != undef or $passenger_max_request_time != undef or $passenger_memory_limit != undef { - include apache::mod::passenger - } - # Configure the defaultness of a vhost if $priority { $priority_real = "${priority}-" @@ -2823,7 +2819,9 @@ # - $passenger_max_requests # - $passenger_max_request_time # - $passenger_memory_limit - if $passenger_enabled != undef or $passenger_start_timeout != undef or $passenger_ruby != undef or $passenger_python != undef or $passenger_nodejs != undef or $passenger_meteor_app_settings != undef or $passenger_app_env != undef or $passenger_app_root != undef or $passenger_app_group_name != undef or $passenger_app_start_command != undef or $passenger_app_type != undef or $passenger_startup_file != undef or $passenger_restart_dir != undef or $passenger_spawn_method != undef or $passenger_load_shell_envvars != undef or $passenger_preload_bundler != undef or $passenger_rolling_restarts != undef or $passenger_resist_deployment_errors != undef or $passenger_min_instances != undef or $passenger_max_instances != undef or $passenger_max_preloader_idle_time != undef or $passenger_force_max_concurrent_requests_per_process != undef or $passenger_concurrency_model != undef or $passenger_thread_count != undef or $passenger_high_performance != undef or $passenger_max_request_queue_size != undef or $passenger_max_request_queue_time != undef or $passenger_user != undef or $passenger_group != undef or $passenger_friendly_error_pages != undef or $passenger_buffer_upload != undef or $passenger_buffer_response != undef or $passenger_allow_encoded_slashes != undef or $passenger_lve_min_uid != undef or $passenger_base_uri != undef or $passenger_error_override != undef or $passenger_sticky_sessions != undef or $passenger_sticky_sessions_cookie_name != undef or $passenger_sticky_sessions_cookie_attributes != undef or $passenger_app_log_file != undef or $passenger_debugger != undef or $passenger_max_requests != undef or $passenger_max_request_time != undef or $passenger_memory_limit != undef { + if ($passenger_enabled != undef or $passenger_start_timeout != undef or $passenger_ruby != undef or $passenger_python != undef or $passenger_nodejs != undef or $passenger_meteor_app_settings != undef or $passenger_app_env != undef or $passenger_app_root != undef or $passenger_app_group_name != undef or $passenger_app_start_command != undef or $passenger_app_type != undef or $passenger_startup_file != undef or $passenger_restart_dir != undef or $passenger_spawn_method != undef or $passenger_load_shell_envvars != undef or $passenger_preload_bundler != undef or $passenger_rolling_restarts != undef or $passenger_resist_deployment_errors != undef or $passenger_min_instances != undef or $passenger_max_instances != undef or $passenger_max_preloader_idle_time != undef or $passenger_force_max_concurrent_requests_per_process != undef or $passenger_concurrency_model != undef or $passenger_thread_count != undef or $passenger_high_performance != undef or $passenger_max_request_queue_size != undef or $passenger_max_request_queue_time != undef or $passenger_user != undef or $passenger_group != undef or $passenger_friendly_error_pages != undef or $passenger_buffer_upload != undef or $passenger_buffer_response != undef or $passenger_allow_encoded_slashes != undef or $passenger_lve_min_uid != undef or $passenger_base_uri != undef or $passenger_error_override != undef or $passenger_sticky_sessions != undef or $passenger_sticky_sessions_cookie_name != undef or $passenger_sticky_sessions_cookie_attributes != undef or $passenger_app_log_file != undef or $passenger_debugger != undef or $passenger_max_requests != undef or $passenger_max_request_time != undef or $passenger_memory_limit != undef) and $ensure == 'present' { + include apache::mod::passenger + concat::fragment { "${name}-passenger": target => "${priority_real}${filename}.conf", order => 300, From e05f97f04c37a23528bca68ba8380400366b4c23 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:08:16 +0200 Subject: [PATCH 12/19] Localize apache::mod::auth_kerb inclusion --- manifests/vhost.pp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 426a4a5784..d3814f3d9f 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2031,10 +2031,6 @@ } } - if $auth_kerb and $ensure == 'present' { - include apache::mod::auth_kerb - } - if $auth_oidc and $ensure == 'present' { include apache::mod::auth_openidc } @@ -2646,7 +2642,9 @@ # - $krb_auth_realms # - $krb_5keytab # - $krb_local_user_mapping - if $auth_kerb { + if $auth_kerb and $ensure == 'present' { + include apache::mod::auth_kerb + concat::fragment { "${name}-auth_kerb": target => "${priority_real}${filename}.conf", order => 230, From b5d74a9a62629728af113f7e7c6e0f8c291e1ccb Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:08:50 +0200 Subject: [PATCH 13/19] Localize apache::mod::auth_openidc inclusion --- manifests/vhost.pp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index d3814f3d9f..9a49c61c00 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2031,10 +2031,6 @@ } } - if $auth_oidc and $ensure == 'present' { - include apache::mod::auth_openidc - } - # Configure the defaultness of a vhost if $priority { $priority_real = "${priority}-" @@ -2912,7 +2908,9 @@ # Template uses: # - $auth_oidc # - $oidc_settings - if $auth_oidc { + if $auth_oidc and $ensure == 'present' { + include apache::mod::auth_openidc + concat::fragment { "${name}-auth_oidc": target => "${priority_real}${filename}.conf", order => 360, From 342b1605f2a167505abcfc5582649e7aabfcd120 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:10:12 +0200 Subject: [PATCH 14/19] Localize apache::mod::ssl inclusion The apache::mod::mime inclusion is dropped since apache::mod::ssl already includes it (since fc8fee7ef3d91e8c4ec2fcc2e19e6ad4cb46028e) and apache/vhost/_ssl.erb does not use AddType. --- manifests/vhost.pp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 9a49c61c00..a559015804 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2013,12 +2013,6 @@ # Input validation ends - if $ssl and $ensure == 'present' { - include apache::mod::ssl - # Required for the AddType lines. - include apache::mod::mime - } - if $ssl_honorcipherorder =~ Boolean or $ssl_honorcipherorder == undef { $_ssl_honorcipherorder = $ssl_honorcipherorder } else { @@ -2605,6 +2599,8 @@ # - $ssl_stapling # - $apache_version if $ssl and $ensure == 'present' { + include apache::mod::ssl + concat::fragment { "${name}-ssl": target => "${priority_real}${filename}.conf", order => 230, From d4320a721a7965d11562dd1b4acdb3606537914e Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:15:50 +0200 Subject: [PATCH 15/19] Simplify apache::vhost::serveraliases logic The data type does not allow a value that evaluates to false so no need ot check for it. --- manifests/vhost.pp | 2 +- templates/vhost/_serveralias.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index a559015804..97b1a64bda 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2551,7 +2551,7 @@ # Template uses: # - $serveraliases - if $serveraliases and ! empty($serveraliases) { + if ! empty($serveraliases) and $ensure == 'present' { concat::fragment { "${name}-serveralias": target => "${priority_real}${filename}.conf", order => 210, diff --git a/templates/vhost/_serveralias.erb b/templates/vhost/_serveralias.erb index e08a55e328..ef8bec8554 100644 --- a/templates/vhost/_serveralias.erb +++ b/templates/vhost/_serveralias.erb @@ -1,4 +1,4 @@ -<% if @serveraliases and ! @serveraliases.empty? -%> +<% unless @serveraliases.empty? -%> ## Server aliases <%- Array(@serveraliases).each do |serveralias| -%> From 0c5c847f27f9cf41a91ff5298b24d7cad007df7d Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:19:00 +0200 Subject: [PATCH 16/19] Document uses of $mdomain Fixes: 2349ba7f35442493d29a08676382044aebaa6eae --- manifests/vhost.pp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 97b1a64bda..65c0c94674 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2281,6 +2281,7 @@ # - $protocols # - $protocols_honor_order # - $apache_version + # - $mdomain concat::fragment { "${name}-apache-header": target => "${priority_real}${filename}.conf", order => 0, @@ -2598,6 +2599,7 @@ # - $ssl_openssl_conf_cmd # - $ssl_stapling # - $apache_version + # - $mdomain if $ssl and $ensure == 'present' { include apache::mod::ssl From 9951becc12afeb1a6b369010787776949265301b Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:20:06 +0200 Subject: [PATCH 17/19] Only include modules if ensure is present --- manifests/vhost.pp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 65c0c94674..59dd711dcb 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -2740,7 +2740,7 @@ } } - if $h2_copy_files != undef or $h2_direct != undef or $h2_early_hints != undef or $h2_max_session_streams != undef or $h2_modern_tls_only != undef or $h2_push != undef or $h2_push_diary_size != undef or $h2_push_priority != [] or $h2_push_resource != [] or $h2_serialize_headers != undef or $h2_stream_max_mem_size != undef or $h2_tls_cool_down_secs != undef or $h2_tls_warm_up_size != undef or $h2_upgrade != undef or $h2_window_size != undef { + if ($h2_copy_files != undef or $h2_direct != undef or $h2_early_hints != undef or $h2_max_session_streams != undef or $h2_modern_tls_only != undef or $h2_push != undef or $h2_push_diary_size != undef or $h2_push_priority != [] or $h2_push_resource != [] or $h2_serialize_headers != undef or $h2_stream_max_mem_size != undef or $h2_tls_cool_down_secs != undef or $h2_tls_warm_up_size != undef or $h2_upgrade != undef or $h2_window_size != undef) and $ensure == 'present' { include apache::mod::http2 concat::fragment { "${name}-http2": @@ -2750,13 +2750,13 @@ } } - if $mdomain { + if $mdomain and $ensure == 'present' { include apache::mod::md } # Template uses: # - $userdir - if $userdir { + if $userdir and $ensure == 'present' { include apache::mod::userdir concat::fragment { "${name}-userdir": From 619e9c9bcc028745a7642a36efb19ffd4389ac31 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:43:41 +0200 Subject: [PATCH 18/19] 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 59dd711dcb..585f748726 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 18eed8ef03370c64b9c019b6ed04256fbed92343 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 12:34:46 +0200 Subject: [PATCH 19/19] 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 | 90 ++++++++++++++++++++++++-------- spec/acceptance/vhost_spec.rb | 5 +- templates/vhost/_directories.erb | 4 +- 3 files changed, 71 insertions(+), 28 deletions(-) diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 585f748726..3d8a0824a1 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] $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,41 @@ # - $apache_version # - $suphp_engine # - $shibboleth_enabled - if $_directories and ! empty($_directories) { + 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::setenv + } + } + 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 -%>