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

Thu Nov 8 07:20:04 UTC 2012

On 11/07/2012 05:48 PM, Sumit Bose wrote:
> On Wed, Nov 07, 2012 at 07:23:52PM +0530, Steeve Goveas wrote:
>> 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.
> 
> With the current patch you already get a warning when your are about to
> change the NetBIOS name:
> 
> ....
> Current NetBIOS domain name is IPA17, new name is ABC.
> 
> Please note that changing the NetBIOS name might break existing trust
> relationships.
> Say 'yes' if the NetBIOS shall be changed and 'no' if the old one shall
> be kept.
> Do you want to reset the NetBIOS domain name? [no]: 
> ....
> 
> So the admin can still change his mind. I think a description of what
> you should considered when you really want to change the NetBIOS domain
> name should go into the docs and not printed by ipa-adtrust-install. As
> mentioned earlier changing the NetBIOS domain name should be seen as the
> last resort and should be done with great care.
> 
> bye,
> Sumit

I think this is fine.

ACK. Pushed to master, ipa-3-0.

Martin