Manage DNF module for mod_auth_openidc #2283
Conversation
ekohl
force-pushed
the
add-mod_auth_openidc-acceptance-test
branch
2 times, most recently
from
aa66074
to
b4ac0d7
Compare
|
Odd thing: why is it running acceptance tests on Scientific 6 (which is EOL since 2020) but not on Red Hat 6? |
ekohl
force-pushed
the
add-mod_auth_openidc-acceptance-test
branch
from
b4ac0d7
to
07292d2
Compare
|
I think this is now ready. It's starting to look greener and greener in the checks. |
ekohl
changed the title
| # | ||
| class apache::mod::auth_openidc inherits apache::params { | ||
| class apache::mod::auth_openidc ( | ||
| Boolean $manage_dnf_module = $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8', |
There was a problem hiding this comment.
Note that EL9 doesn't have this in a module, so it's limited to EL8.
| # | ||
| class apache::mod::auth_openidc inherits apache::params { | ||
| class apache::mod::auth_openidc ( | ||
| Boolean $manage_dnf_module = $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8', |
There was a problem hiding this comment.
This is correct, because while EL9 has module support, mod_auth_oidc is not modular there.
(:exploding_head:)
ekohl
force-pushed
the
add-mod_auth_openidc-acceptance-test
branch
from
07292d2
to
fcf1ea1
Compare
|
I just pushed an update that added a unit test to ensure the DNF module isn't present on non-EL8. |
|
The RHEL 9 failure looks like an intermittent failure on provisioning the machine. It passed previously and Puppet 7 is green. Shouldn't be blocking. |
|
Something @evgeni and I were discussing, does this module also need an include on @asieraguado you added the class initially, perhaps you can weigh in? |
Sorry, I didn't check if the module works without |
|
Yeah, I think it's needed for all of them, as without |
Are we talkin about |
The authn_core module is needed for AuthType, which is needed to select a specific auth. While modules may load without them, it's unlikely to work in practice. This is only relevant when default mods are disabled since authn_core is a default module.
On EL 8 mod_auth_openidc is in a DNF module that must be enabled. Otherwise the package is uninstallable. This is verified by adding an acceptance test for the class. The inheritance on apache::params is removed since it was redundant. That is only needed if a class parameter uses apache::params. $oidc_settings on apache::vhost is changed to have a default. The template expects one and With that it's impossible to miscompile. The alternative is to implement a fail() inside the code if it is empty, but this provides some safety.
ekohl
force-pushed
the
add-mod_auth_openidc-acceptance-test
branch
from
fcf1ea1
to
8417c0b
Compare
Technically |
|
@david22swan thoughts? Ideally we would also get a release out soon with this. Also, you don't do stable branches, right? Ideally we'd get this into a 7.x release as well. |
I'd prefer to keep that for a future PR. Can we merge this then? Ideally also get a release out (with the other fixes for which PRs are open now). |
Sure, that's mostly unrelated to the changes in this PR, just something that I started thinking of because of this PR ;) |
|
Ok, everyone sounds happy so gonna go ahead and merge |
On EL 8 mod_auth_openidc is in a DNF module that must be enabled. Otherwise the package is uninstallable. This is verified by adding an acceptance test for the class.
The inheritance on apache::params is removed since it was redundant. That is only needed if a class parameter uses apache::params.
$oidc_settings on apache::vhost is changed to have a default. The template expects one and With that it's impossible to miscompile. The alternative is to implement a fail() inside the code if it is empty, but this provides some safety.