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?

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

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to