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

Automatically include mod_expires if required via vhost directories #2559

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ekohl
Copy link
Collaborator

@ekohl ekohl commented Aug 16, 2024

Summary

Similar to other options passed in the vhost's directory this now inspects the directories entry and if expires_* is passed, mod_expires` is included.

Additional Context

This is a pattern that I've started to apply more and more. It allows passing complex directories (also via Hiera). On our servers we disable the default mods and this ensures we have the correct mods enabled.

Checklist

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

Similar to other options passed in the vhost's directory this now
inspects the directories entry and if expires_* is passed, mod_expires
is included.
@ekohl ekohl force-pushed the auto-include-expires branch from 6e42fb2 to d51909d Compare August 22, 2024 09:57
@@ -2362,6 +2362,10 @@
if 'setenv' in $directory {
include apache::mod::env
}

if 'expires_active' in $directory or 'expires_default' in $directory or 'expires_by_type' in $directory {
include apache::mod::expires
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One thing to note: apache::mod::expires by default writes down a configuration file (unlike other modules). That could be a behavior difference and break setups. Is that a problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have other modules that do that and thus we include them differently?

Copy link
Contributor

Choose a reason for hiding this comment

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

Like

::apache::mod { 'expires': }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is exactly the point I tried to make and I'm surprised that CI didn't fail on this:

::apache::mod { 'expires': }

::apache::mod { 'expires': }

It feels ugly, but perhaps we should have some helper class that is only the module definition without the configuration file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants