Fixes #29649 - Drop default_server argument in IPA #935
Conversation
|
@ekohl Is it ok to merge this? |
|
I've got most of my lab set up again on my new desktop, I'll see about verifying this with a real IPA sever. |
ekohl
force-pushed
the
29649-remove-default-server
branch
from
c079d9f
to
0046758
Compare
ekohl
force-pushed
the
29649-remove-default-server
branch
from
0046758
to
d92bc90
Compare
ipa-getkeytab can figure out the default server on its own[1]. There is no need to specify it and can even break things. For example, DNS can be used to detect servers. Then the fact is empty and it fails while the command would actually pass. The foreman_ipa fact is removed since it's a major version bump anyway and nothing else should use our foreman_ipa fact. [1] theforeman#880 (comment)
ekohl
force-pushed
the
29649-remove-default-server
branch
from
d92bc90
to
0d06d2e
Compare
|
Rebased |
There was a problem hiding this comment.
Overall I am fine with the change.
The only thing that left me wondering is the "not enrolled system" guard that we're removing, but I also don't really see any harm in doing so?
It has been there since 378d602 introduced IPA support in the installer
| @@ -126,10 +126,6 @@ | |||
| $foreman_socket_override = template('foreman/foreman.socket-overrides.erb') | |||
|
|
|||
| if $foreman::ipa_authentication { | |||
| unless fact('foreman_ipa.default_server') { | |||
| fail("${facts['networking']['hostname']}: The system does not seem to be IPA-enrolled") | |||
There was a problem hiding this comment.
I wonder what this guard originally was about?
Worst that can happen without it is that ipa-getkeytab below fails?
There was a problem hiding this comment.
Previously it would guaranteed generate an incorrect command if that fact was missing. However, we can't really reliably detect it (DNS records can be used for example). So you're right that ipa-getkeytab could fail during runtime and exec resources don't log well in our installer (happens at the info level and to make the installer quieter, info is only sent to /var/log). However, I do think that fully supporting all possible registrations is more important than perfect error handling in this case.
ipa-getkeytab can figure out the default server on its own[1]. There is no need to specify it and can even break things. For example, DNS can be used to detect servers. Then the fact is empty and it fails while the command would actually pass.
The fact is simplified since it's a major version bump anyway and nothing else should use our foreman_ipa fact.
[1] #880 (comment)
Replaces #880