On 26.11.2015 14:36, Tomas Babej wrote:


On 11/26/2015 01:33 PM, Jan Cholasta wrote:
On 25.11.2015 09:01, Jan Cholasta wrote:
On 24.11.2015 15:56, Tomas Babej wrote:


On 11/23/2015 04:43 PM, Jan Cholasta wrote:
3)

+    host_name = Knob(
+        str, None,
+        description="fully qualified name of this host",
+        cli_name='hostname',
+    )

This knob is already defined in BaseServer, there's no need to redefine
it here (just remove the "host_name = None").

If you want to change the description, change it in BaseServer.

Yep, that was the reasoning here. Changed in BaseServer.

You can remove the duplicate knob from Replica, since it is inherited from BaseServer.

5) The replica file argument conflicts with the --realm, --domain,
--server, --admin-password and --principal options. This should be
checked in Replica.__init__().

The --hostname option should either be conflicting as well or be
implemented properly for legacy replica install.


All of them now conflict --replica-file.

Replica file is not an option but rather an argument in
ipa-replica-install.

I think the error message should use the same wording which is used for
--forwarder vs. --no-forwarder and --reverse-zone vs. --no-reverse: "You
cannot specify a --something option together with replica file".

Nitpick: the namedtuple seems like an overkill, given it's only used in a single place. (But I'm fine with keeping it.)




6) I think --admin-password should be renamed to --password and the
description changed, since it now also allows OTP enrollment.


I changed the requirements to mandate --principal when --password is
used, as we discussed.

I was wrong here, sorry.

ipa-replica-install assumes "admin" for principal by default, so we just
need to make sure ipa-client-install is called with --principal when
password is used, whether it's the provided principal or the default
"admin".

ipa-replica-install assumes "admin", but it's not the default value of --principal, so it may be None here:

+        if installer.admin_password:
+            args.extend(["--password", installer.admin_password])
+            args.extend(["--principal", installer.principal])


This should be removed:

+            # Make sure --password is not used without principal,
+            # that would mean OTP join for ipa-client-install
+
+            password_and_principal = [self.admin_password, self.principal]
+ if any(password_and_principal) and not all(password_and_principal):
+                raise RuntimeError(
+                    "The --password and --principal options must be used "
+                    "together."
+                )

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to