[Freeipa-devel] [PATCH 0385] replicainstall: Add possiblity to install client in one
Tomas Babej
tbabej at redhat.com
Mon Nov 30 11:25:42 UTC 2015
On 11/30/2015 09:25 AM, Jan Cholasta wrote:
> 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.
>
Yeah, this is mine brainfart. Removed.
>>>>>> 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.)
>
This is just to improve the readability (by a small bit).
>>>>
>>>>>
>>>>>>
>>>>>> 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."
> + )
>
Same here, fixed.
Updated patch attached.
Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0385-5-replicainstall-Add-possiblity-to-install-client-in-o.patch
Type: text/x-patch
Size: 6681 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151130/32459004/attachment.bin>
More information about the Freeipa-devel
mailing list