theforeman / puppet-pulpcore

Puppet module for setting up Pulp 3 as part of Katello installation
GNU General Public License v3.0
2 stars 28 forks source link

Reuse headers from pulpcore::apache class #354

Closed ekohl closed 1 week ago

ekohl commented 2 weeks ago

We have the exact same code in both pulpcore::plugin::container as in pulpcore::apache. This reuses the variables rather than duplicating the logic.

Fixes: 21aa39e28f19 ("Fixes #37308 - set REMOTE_USER properly for pulpcore registry")

ekohl commented 1 week ago

When adding tests I figured out that at least there it's somehow different.

Redoing it, I first wrote tests prior to this patch (e3d1ddb169713d6e2b68ff99abe0cf4b3fdd720c, based on @evgeni's work elsewhere). Then reapplied this (bd1dbecec0b47316a57b8d18aa17f72c2d666bce) and fixed tests the (66b97fed46a4ce506ec475ad37ee1ef38b2fe7d2).

Why does it result in an additional line when it should use the exact same variables? Still trying to figure that out.

evgeni commented 1 week ago

Why does it result in an additional line when it should use the exact same variables? Still trying to figure that out.

Because the content of the variables are not identical!

The "global" ones: https://github.com/theforeman/puppet-pulpcore/blob/5b6d0885a44d5b5027f15c866de9d7c39e5f71b0/manifests/apache.pp#L48-L55

The ones you removed here: https://github.com/theforeman/puppet-pulpcore/blob/c63cb654ab2ac78c3cc4828b1fe504aa2d3bb577/manifests/plugin/container.pp#L12-L18

the global ones, that we now use have an additional

"set ${remote_user_environ_header} \"%{SSL_CLIENT_S_DN_CN}s\" env=SSL_CLIENT_S_DN_CN", 
ekohl commented 1 week ago

I was so blindly staring at api_additional_request_headers that I didn't look too carefully at api_default_request_headers.

ekohl commented 1 week ago

https://github.com/theforeman/puppet-pulpcore/pull/356