Detect module dependencies when modules are not default-loaded

[gcoxmoz]

Additional Context

If you accept default_mods => true you get a -lot- of apache modules automatically. In my experience it's too many, which has led to us doing default_mods => [] to reduce the sprawl, and then adding back in what we need. That leads to a bit of whack-a-mole upon new server buildout, as a lot of this module's code assumes that you have included the default modules and it doesn't need to check. It also leads to "I'm not sure if I can remove this module" later, if you remove directives.

Summary

This covers off the worst offenders I've hit in our environment: mod_dir, mod_autoindex, mod_expires, mod_dav, and mod_cgi. If you use vhost directory parameters that will invoke these module directives, vhost will try to make sure the module is loaded. If you're on default_mods => true, this is a 'wasted' extra load; if you don't have the module loaded (say, default_mods => false) then this brings the module in for you.

Related Issues (if any)

N/A

Checklist

• 🟢 Spec tests.
• 🟢 Acceptance tests.
• Manually verified. (For example puppet apply)

[binford2k]

Should this magically load dependencies, or should it display errors so people know to correct their code? I'm not sure I have a strong opinion there. Perhaps it could load, but also print an INFO message.

[gcoxmoz]

There's plenty of prior art of the puppet-module "acting on your behalf" in grabbing needed apache-modules. One for a quick example:

puppetlabs-apache/manifests/vhost.pp

Lines 2261 to 2263 in 1c72075

	if $virtual_docroot {
	include apache::mod::vhost_alias
	}

is "if your vhost directive asked for virtual_docroot (which defaults to false), jump in and grab the apache-module that will make it work." This parallels what I'm looking for: 'demand an apache module when it is needed'. The difference is that a lot of the puppet module is built on the assumption that some of the apache modules are always there... and they don't have to be. Which is actually a bit odd, since the spec test

puppetlabs-apache/spec/defines/vhost_spec.rb

Line 11 in 1c72075

"class {'apache': default_vhost => false, default_mods => false, vhost_enable_dir => '/etc/#{apache_name}/sites-enabled'}"

assumes default_mods => false while most of the code is built with an unspoken assumption of default_mods => true

That all happens without an INFO or an explicit apache-module declaration, so IMO autoloading the apache-modules (quietly, as needed) is the way to go here.

[ekohl]

I've been pushing in this direction. Like #2255 and #2337. Exactly for the reason you mention: I want to run the server with default_mods => false but not think about which modules to include.

My ideal is that we actually define a proper data type for $directories and enforce it.

[ekohl]

I want to let you know that I've essentially stopped reviewing anything that is Perforce maintained since they are very unclear on outside maintainers. There used to be https://puppet.com/ecosystem/trusted-contributors/ but that's now gone.

[ekohl]

Can't you combine these 2 statements?

Suggested change

	if !('-ExecCGI' in $directory['options']) {
	if 'ExecCGI' in $directory['options'] {
	if !('-ExecCGI' in $directory['options']) and 'ExecCGI' in $directory['options'] {

And I forgot if this is valid or I'm confusing Python

Suggested change

	if !('-ExecCGI' in $directory['options']) {
	if 'ExecCGI' in $directory['options'] {
	if '-ExecCGI' not in $directory['options'] and 'ExecCGI' in $directory['options'] {