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 _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
