[Freeipa-devel] [PATCH] 89 ipa-adtrust-install: allow to reset te NetBIOS domain name
Martin Kosek
mkosek at redhat.com
Mon Nov 5 12:18:49 UTC 2012
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?
# 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
More information about the Freeipa-devel
mailing list