[Freeipa-devel] [PATCH] 89 ipa-adtrust-install: allow to reset te NetBIOS domain name
Martin Kosek
mkosek at redhat.com
Fri Nov 2 13:54:32 UTC 2012
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...
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" % \
Martin
More information about the Freeipa-devel
mailing list