search

theforeman / puppet-foreman

Puppet module for Foreman
GNU General Public License v3.0
104 stars 271 forks source link

Fixes #35870 - Ensure mod_expires is loaded #1101

Closed ekohl closed 1 year ago

ekohl commented 1 year ago

In case only minimal Apache modules are loaded then mod_expires may not be present. The result is that assets are sent without headers that allow browsers to cache it.

evgeni commented 1 year ago 
      # Puppet::Resource::Catalog::DuplicateResourceError:
      #   Duplicate declaration: Apache::Mod[expires] is already declared at (file: /home/runner/work/puppet-foreman/puppet-foreman/spec/fixtures/modules/apache/manifests/default_mods.pp, line: 73); cannot redeclare (file: /home/runner/work/puppet-foreman/puppet-foreman/spec/fixtures/modules/apache/manifests/mod/expires.pp, line: 23)

This somehow implies our tests don't run with apache::default_mods: false?

Edit: no, it's set?! https://github.com/theforeman/puppet-foreman/blob/a57aa4b4e2c1ece7713db81bacceaecb51b49156/spec/acceptance/hieradata/common.yaml#L2

Edit2: oh no, that's just for acceptance!

ekohl commented 1 year ago

I just noticed this yesterday on my own server, but I'll do some more digging next week.

And it does indicate an incompatibility, which is actually good to know about: we have to choose one or the other. Now I'm reminded about https://github.com/puppetlabs/puppetlabs-apache/pull/2288, which I started but didn't have a discussion.

ekohl commented 1 year ago

No code change, but I opened https://projects.theforeman.org/issues/35870 so we don't lose track of this.

evgeni commented 1 year ago

FWIW, the regression is there also before #33956

The original "disable default modules" change triggered that and I see the headers missing on a 3.3 installation that does not yet have the "assets via apache" part.

ekohl commented 1 year ago

I ended up working around apache::mod::expires including additional config so it can be cherry picked safely.