Make ipa-replica-install run in interactive mode #117
Conversation
| "with the --principal option.") | ||
| if installer.unattended: | ||
| # Don't add the password to the options in unattended mode | ||
| # ==> it would also appear in the client install logs |
There was a problem hiding this comment.
It actually appears in replica install logs.
| # ==> it would also appear in the client install logs | ||
| stdin = installer.admin_password | ||
| else: | ||
| args.extend(["--password", installer.admin_password]) |
There was a problem hiding this comment.
This way a password is logged in replica install log, which is undesirable. There might be some logging option to clear the password.
There was a problem hiding this comment.
Right, I did not realize there was a nolog option in ipautil run, I will use that instead for both attended/unattended cases.
| @@ -918,47 +918,55 @@ def install(installer): | |||
|
|
|||
|
|
|||
| def ensure_enrolled(installer): | |||
| config = installer._config | |||
| # Prepare options for the installer script | |||
| args = [paths.IPA_CLIENT_INSTALL, "--no-ntp"] | |||
There was a problem hiding this comment.
This changes the default behaviour. Previously, when a user provided all the mandatory arguments, the installation finished successfully in unattended mode. Now the user gets prompted for confirmations even if he provides all the mandatory attributes.
I think replica should only run in interactive mode if some mandatory attribute is missing to keep backwards compatibility.
There was a problem hiding this comment.
However, interactive mode will also allow the user to set domain/server before the confirmation if the IPADiscovery fails. Also, if IPADiscovery succeeds but the server/domain is not the one the user wants, they should be able to make their disapproval count.
This could be solved by having domain + server as mandatory options in ipa-replica-install but then we're losing the interactivity again.
|
NACK, please see inline comments. |
|
ACK Running the command in interactive mode by default is desirable behaviour. Since the |
|
|
||
| ipautil.run(args, stdin=stdin, redirect_output=True) | ||
| # Call client install script | ||
| ipautil.run(args, nolog=nolog, redirect_output=True) |
There was a problem hiding this comment.
The stdin=stdin has to stay, otherwise echo password | ipa-replica-install can't work.
There was a problem hiding this comment.
echo password | ipa-replica-install does not work irregardless of this patch.
There was a problem hiding this comment.
Ah, my bad. However, I still think it's better to pass the admin password to ipa-client-install via stdin so that it's not visible in ps.
There was a problem hiding this comment.
passing admin password in a command line argument is pretty much a NACK without recourse.
There was a problem hiding this comment.
Huh, that's right. Up until this point, admin password would have to be passed in command line option anyway if you wanted to pass it to the installer. In this interactive mode, it may seem to a user that the password is actually hidden to anyone else but it would be revealed like this.
I think we have no other way but to make client install be run from a module, then. I will look into it once FIPS-related work is done.
| if installer.admin_password: | ||
| if installer.principal is None: | ||
| raise ScriptError("The --admin-password option must be used " | ||
| "with the --principal option.") |
There was a problem hiding this comment.
I don't think this is correct. The name of the option implies the principal is admin (which is also how it behaves in domain level 0), so we should not require the user to explicitly specify the principal.
There was a problem hiding this comment.
The name indeed implies that the principal is admin, but that does not necessarily mean "admin", I think. Anyway, as this is the behavior in domain level 0, I've kept it and documented it in the respective man page + help.
|
NACK, see inline comments. |
stlaz
force-pushed
the
trac-6068
branch
2 times, most recently
from
fee8b78
to
314bc73
Compare
|
@stlaz I do not understand the rationale. Ideally the ipa-replica-install command gathers all necessary info and ipa-client-install is always run in unattended mode. |
|
@simo5: There is a LOT of checking of various combinations of options in ipa-client-install, not even mentioning IPADiscovery in interactive mode. It does not make sense to copy-paste all/most of it. |
|
@stlaz, sure, what I meant is that the checking code should be made common and run in ipa-repliuca-install, certainly I was not suggesting to just duplicate all that code. Perhaps refactoring will just do that. |
|
This PR needs to be rebased to reflect installer refactoring. |
Tweaks to replica installation to support interactive mode: - modified man to better document what actually happens - added principal/password prompt for unattended mode of ipa-replica-install if no credentials are set - made ipa-client-install run in interactive mode during replica promotion if it is itself not run in unattended mode https://fedorahosted.org/freeipa/ticket/6068
|
Rebase done. I wanted to wait until some more changes to api bootstrapping to be able to call client installation from module using the latest installer system from the installers refactoring but we agreed with @jcholast that it'd be better to do that later. |
| # get the principal interactively, can't be empty | ||
| installer.principal = ipautil.user_input( | ||
| "User authorized to enroll computers", | ||
| allow_empty=False) |
There was a problem hiding this comment.
I think we should ask for "User authorized to install replicas".
We should not ask for the username and password if the client is already installed and the host is already a member of ipaservers.
We should not ask for the username and password if there already is a valid ccache.
| if not installer.admin_password: | ||
| # higher-level error so script usage is not printed | ||
| raise ScriptError("Password must be provided for %s." % | ||
| installer.principal) |
There was a problem hiding this comment.
This whole block should be moved to promote_check(), interactive prompts elsewhere are done in *_check() as well.
|
I have a WIP patch but since sometimes it's not clear which credentials are used, I am marking this as postponed so that we can wait until the client module can be called properly. |
slaykovsky
added
the
needs rebase
|
Closing due to inactivity. |
ipa-replica-install would not run in interactive mode which confused some users. Make it run ipa-client-install in attended mode so that the required arguments are asked for instead of the installation just failing.