[Freeipa-devel] [PATCH] 89 ipa-adtrust-install: allow to reset te NetBIOS domain name

[Steeve Goveas]

On 11/07/2012 06:33 PM, Martin Kosek wrote:
> On 11/07/2012 01:54 PM, Sumit Bose wrote:
>> On Mon, Nov 05, 2012 at 01:18:49PM +0100, Martin Kosek wrote:
>>> On 11/02/2012 09:50 PM, Sumit Bose wrote:
>>>> On Fri, Nov 02, 2012 at 02:54:32PM +0100, Martin Kosek wrote:
>>>>> On 11/02/2012 12:54 PM, Sumit Bose wrote:
>>>>>> On Wed, Oct 31, 2012 at 04:03:14PM +0100, Martin Kosek wrote:
>>>>>>> On 10/30/2012 12:16 PM, Sumit Bose wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> this patch allows ipa-adtrust-install to reset the NetBIOS domain name
>>>>>>>> and fixes https://fedorahosted.org/freeipa/ticket/3192 .
>>>>>>>>
>>>>>>>> bye,
>>>>>>>> Sumit
>>>>>>>>
>>>>>>>
>>>>>>> Hello Sumit,
>>>>>>>
>>>>>>> I found few issues with your patch:
>>>>>> Thank you for the review.
>>>>>>
>>>>>>> 1) It requires admin to be kinited ("conn.do_sasl_gssapi_bind()") I do not
>>>>>>> think this is necessary, ipa-adtrust-install already requires admin password to
>>>>>>> be passed and it already connects to LDAP with these credentials:
>>>>>>>
>>>>>>> api.Backend.ldap2.connect(ccache.name)
>>>>>>>
>>>>>>> You could use ipa.Backend.ldap2 object to do entry retrieval
>>>>>>> (ipa.Backend.ldap2.get_entry) without a need to init IPAdmin at all.
>>>>>> fixed
>>>>>>
>>>>>>> 2) When doing try..except statement, rule of thumb says that it should be as
>>>>>>> short as possible, so that it does not hide other potential errors and makes
>>>>>>> clear what function raises the catched exception.
>>>>>>>
>>>>>>> In your case:
>>>>>>>
>>>>>>> try:
>>>>>>>      entry = api.Backend.ldap2.get_entry(DN(('cn', api.env.domain),
>>>>>>>                                          api.env.container_cifsdomains,
>>>>>>>                                          self.api.env.basedn),
>>>>>>>                                         ['ipantflatname'])
>>>>>>> except errors.NotFound:
>>>>>>>      reset_netbios_name = False
>>>>>>> else:
>>>>>>>      # process entry
>>>>>>>
>>>>>>> Should be a pattern that you want.
>>>>>> fixed
>>>>>>
>>>>>> I also move all the NetBIOS name related code into a separate function.
>>>>>>> 3) I think this line is redundant:
>>>>>>> +                    print "Say 'yes' if the NetBIOS shall be changed and " \
>>>>>>> +                          "'no' if the old one shall be kept."
>>>>>>>
>>>>>>> IMO, the question:
>>>>>>>
>>>>>>> reset_netbios_name = ipautil.user_input( 'Reset NetBIOS domain name?',  default
>>>>>>> = False, allow_empty = False)
>>>>>>>
>>>>>>> and the information printed before is enough.
>>>>>> I would prefer to keep it this way to make clear that
>>>>>> ipa-adtrust-install will continue processing, but the old name if kept
>>>>>> even if a new name was given with --netbios-name on the command line.
>>>>>>
>>>>>> New version attached.
>>>>>>
>>>>>> bye,
>>>>>> Sumit
>>>>>>
>>>>>>> Martin
>>>>>
>>>>> The new approach looks much better. Sending issues I found with the new patch:
>>>>>
>>>>> 1) When I run ipa-adtrust-install on a clean IPA, I can no longer enter NetBIOS
>>>>> name interactively. I can only change it via script option...
>>>>>
>>>> fixed
>>>>
>>>>> 2) I saw few typos:
>>>>>
>>>>> +        print "Current NetBIOS domain name is %s new name is %s.\n" % \
>>>>> should be:
>>>>> +        print "Current NetBIOS domain name is %s, new name is %s.\n" % \
>>>>>
>>>>> +            print "NetBIOS domain name will be changes to %s.\n" % \
>>>>> should be:
>>>>> +            print "NetBIOS domain name will be changed to %s.\n" % \
>>>>>
>>>>>
>>>> fixed
>>>>
>>>> new version attached.
>>>>
>>>> bye,
>>>> Sumit
>>>>> Martin
>>> NetBIOS name is now asked when first installing ipa-adtrust-install.
>>>
>>> But I see that NetBIOS name is still not queried when I run re-install of
>>> ADTRUST, I can only change current name via option. Is this is an intended
>>> behavior so that people cannot mess it with by mistake?
>> Yes. The old code didn't check if the NetBIOS name was already set in
>> LDAP or not, hence it always asked the user if the generated NetBIOS
>> name is the one the user expected.
>>
>> The new version looks up the NetBIOS name in the LDAP server and if set
>> and no new name is given on the command line assumes that the admin
>> does not want to change the NetBIOS name and skips the dialog.
>>
>> I'll add the QE team to hear what they prefer.
>>
>> Jenny, Scott, Steeve, assume ipa-adtrust-install is run after trust was
>> already configured and no --netbios-name option is given on the command
>> line. Shall the following dialog be shown:
>>
>> .....
>> Enter the NetBIOS name for the IPA domain.
>> Only up to 15 uppercase ASCII letters and digits are allowed.
>> Example: EXAMPLE.
>>
>>
>> NetBIOS domain name [IPA17]:
>> ....
>>
>> The admin then has to press enter to confirm the current NetBIOS name or
>> can enter a new one. Or shall the dialog be skipped which means that the
>> NetBIOS can only be resetted if a new one is given at the command line
>> with the --netbios-name option?
> Ok, thanks from explanation. But from my devel perspective, since a change of
> NetBIOS domain name can break existing trusts I would rather not offer a change
> of NetBIOS name during interactive prompt. A mere stating of a current value
> with asking to user to change it should be enough.
>
> QE input is welcome, yes.
>
> Martin
>
If changing the netbios name post creating trust would break existing 
trust, then I dont think that changing the netbios named should be 
allowed. A clear message to drop the trust and re-add it would be better.

-Steeve
>> Thank you for your input.
>>
>> bye,
>> Sumit
>>
>>> # ipa-adtrust-install
>>>
>>> The log file for this installation can be found in /var/log/ipaserver-install.log
>>> ==============================================================================
>>> This program will setup components needed to establish trust to AD domains for
>>> the FreeIPA Server.
>>>
>>> This includes:
>>>    * Configure Samba
>>>    * Add trust related objects to FreeIPA LDAP server
>>>
>>> To accept the default shown in brackets, press the Enter key.
>>>
>>> IPA generated smb.conf detected.
>>> Overwrite smb.conf? [no]: y
>>>
>>> The following operations may take some minutes to complete.
>>> Please wait until the prompt is returned.
>>>
>>> <<< no NetBIOS name asked interactively
>>>
>>> Configuring cross-realm trusts for IPA server requires password for user 'admin'.
>>> This user is a regular system account used for IPA server administration.
>>>
>>> admin password:
>>>
>>> Configuring CIFS
>>>    [1/18]: stopping smbd
>>> ...
>>>
>>>
>>> Otherwise the patch looks OK.
>>>
>>> Martin