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
From 4925b1871fef526ab8c1932a7541951f9980c055 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 29 Oct 2012 21:43:56 +0100
Subject: [PATCH] ipa-adtrust-install: allow to reset te NetBIOS domain name

Fixes https://fedorahosted.org/freeipa/ticket/3192
---
 install/tools/ipa-adtrust-install       | 109 ++++++++++++++++++++++++++------
 install/tools/man/ipa-adtrust-install.1 |   6 +-
 ipaserver/install/adtrustinstance.py    |  24 ++++++-
 3 Dateien geändert, 117 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)

diff --git a/install/tools/ipa-adtrust-install 
b/install/tools/ipa-adtrust-install
index 
52179038e84a08ea6abb3ee26d8e668efe0a2b13..83c6b8f4f843e6e389a28b9b4527f89a5e7a118d
 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -71,7 +71,7 @@ def parse_options():
     return safe_options, options
 
 def netbios_name_error(name):
-    print "Illegal NetBIOS name [%s].\n" % name
+    print "\nIllegal NetBIOS name [%s].\n" % name
     print "Up to 15 characters and only uppercase ASCII letter and digits are 
allowed."
 
 def read_netbios_name(netbios_default):
@@ -101,6 +101,90 @@ def read_admin_password(admin_name):
     admin_password = read_password(admin_name, confirm=False, validate=None)
     return admin_password
 
+def set_and_check_netbios_name(netbios_name, unattended):
+    """
+    Depending if trust in already configured or not a given NetBIOS domain
+    name must be handled differently.
+
+    If trust is not configured the given NetBIOS is used or the NetBIOS is
+    generated if none was given on the command line.
+
+    If trust is  already configured the given NetBIOS name is used to reset
+    the stored NetBIOS name it it differs from the current one.
+    """
+
+    flat_name_attr = 'ipantflatname'
+    cur_netbios_name = None
+    gen_netbios_name = None
+    reset_netbios_name = False
+    dom_dn = None
+
+    try:
+        (dom_dn, entry) = api.Backend.ldap2.get_entry(DN(('cn', 
api.env.domain),
+                                    api.env.container_cifsdomains,
+                                    ipautil.realm_to_suffix(api.env.realm)),
+                                    [flat_name_attr])
+    except errors.NotFound:
+        # trust not configured
+        pass
+    else:
+        cur_netbios_name = entry.get(flat_name_attr)[0]
+
+    if cur_netbios_name and not netbios_name:
+        # keep the current NetBIOS name
+        netbios_name = cur_netbios_name
+        reset_netbios_name = False
+    elif cur_netbios_name and cur_netbios_name != netbios_name:
+        # change the NetBIOS name
+        print "Current NetBIOS domain name is %s, new name is %s.\n" % \
+              (cur_netbios_name, netbios_name)
+        print "Please note that changing the NetBIOS name might " \
+              "break existing trust relationships."
+        if unattended:
+            reset_netbios_name = True
+            print "NetBIOS domain name will be changed to %s.\n" % \
+                  netbios_name
+        else:
+            print "Say 'yes' if the NetBIOS shall be changed and " \
+                  "'no' if the old one shall be kept."
+            reset_netbios_name = ipautil.user_input(
+                            'Do you want to reset the NetBIOS domain name?',
+                            default = False, allow_empty = False)
+        if not reset_netbios_name:
+            netbios_name = cur_netbios_name
+    elif cur_netbios_name and cur_netbios_name == netbios_name:
+        # keep the current NetBIOS name
+        reset_netbios_name = False
+    elif not cur_netbios_name:
+        if not netbios_name:
+            gen_netbios_name = 
adtrustinstance.make_netbios_name(api.env.domain)
+
+        if dom_dn:
+            # Fix existing trust configuration
+            print "Trust is configured but no NetBIOS domain name found, " \
+                  "setting it now."
+            reset_netbios_name = True
+        else:
+            # initial trust configuration
+            reset_netbios_name = False
+    else:
+        # all possible cases should be covered above
+        raise Exception('Unexpected state while checking NetBIOS domain name')
+
+    if not adtrustinstance.check_netbios_name(netbios_name):
+        if unattended:
+            netbios_name_error(netbios_name)
+            sys.exit("Aborting installation.")
+        else:
+            if netbios_name:
+                netbios_name_error(netbios_name)
+                netbios_name = None
+
+    if not unattended and not netbios_name:
+        netbios_name = read_netbios_name(gen_netbios_name)
+
+    return (netbios_name, reset_netbios_name)
+
 def ensure_admin_kinit(admin_name, admin_password):
     try:
         ipautil.run(['kinit', admin_name], stdin=admin_password+'\n')
@@ -197,22 +281,6 @@ def main():
         print "Please wait until the prompt is returned."
         print ""
 
-    netbios_name = options.netbios_name
-    if not netbios_name:
-        netbios_name = adtrustinstance.make_netbios_name(api.env.domain)
-
-    if not adtrustinstance.check_netbios_name(netbios_name):
-        if options.unattended:
-            netbios_name_error(netbios_name)
-            sys.exit("Aborting installation.")
-        else:
-            netbios_name = None
-            if options.netbios_name:
-                netbios_name_error(options.netbios_name)
-
-    if not options.unattended and ( not netbios_name or not 
options.netbios_name):
-        netbios_name = read_netbios_name(netbios_name)
-
     admin_password = options.admin_password
     if not (options.unattended or admin_password):
         admin_password = read_admin_password(options.admin_name)
@@ -248,11 +316,16 @@ def main():
     except Exception, e:
         sys.exit("Unrecognized error during check of admin rights: %s" % 
(str(e)))
 
+    (netbios_name, reset_netbios_name) = \
+                                
set_and_check_netbios_name(options.netbios_name,
+                                options.unattended)
+
     smb = adtrustinstance.ADTRUSTInstance(fstore)
     smb.realm = api.env.realm
     smb.autobind = service.ENABLED
     smb.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
-              netbios_name, options.rid_base, options.secondary_rid_base,
+              netbios_name, reset_netbios_name,
+              options.rid_base, options.secondary_rid_base,
               options.no_msdcs, options.add_sids)
     smb.find_local_id_range()
     smb.create_instance()
diff --git a/install/tools/man/ipa-adtrust-install.1 
b/install/tools/man/ipa-adtrust-install.1
index 
9204b7d5fde7493a4c268eb71693e86a63a1b4b7..38957f3a486ec4d3108e7ccdc955880dc65a3873
 100644
--- a/install/tools/man/ipa-adtrust-install.1
+++ b/install/tools/man/ipa-adtrust-install.1
@@ -42,7 +42,11 @@ Enable debug logging when more verbose output is needed
 The IP address of the IPA server. If not provided then this is determined 
based on the hostname of the server.
 .TP
 \fB\-\-netbios\-name\fR=\fINETBIOS_NAME\fR
-The NetBIOS name for the IPA domain. If not provided then this is determined 
based on the leading component of the DNS domain name.
+The NetBIOS name for the IPA domain. If not provided then this is determined
+based on the leading component of the DNS domain name. Running
+ipa\-adtrust\-install for a second time with a different NetBIOS name will
+change the name. Please note that changing the NetBIOS name might break
+existing trust relationships to other domains.
 .TP
 \fB\-\-no\-msdcs\fR
 Do not create DNS service records for Windows in managed DNS server. Since 
those
diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
c27fac99cf624ca6460ce84e76be52db38f11a5b..16f2136a6485e6915fd5de2000e6a378d03b44aa
 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -115,6 +115,7 @@ class ADTRUSTInstance(service.Service):
         self.realm = None
         self.domain_name = None
         self.netbios_name = None
+        self.reset_netbios_name = None
         self.no_msdcs = None
         self.add_sids = None
         self.smbd_user = None
@@ -295,11 +296,27 @@ class ADTRUSTInstance(service.Service):
                                  "define it and run again.")
             raise e
 
+    def __reset_netbios_name(self):
+        """
+        Set the NetBIOS domain name to a new value.
+        """
+        self.print_msg("Reset NetBIOS domain name")
+
+        try:
+            self.admin_conn.modify_s(self.smb_dom_dn,
+                                     [(ldap.MOD_REPLACE, self.ATTR_FLAT_NAME,
+                                       self.netbios_name)])
+        except ldap.LDAPError:
+            self.print_msg("Failed to reset the NetBIOS domain name")
+
     def __create_samba_domain_object(self):
 
         try:
             self.admin_conn.getEntry(self.smb_dom_dn, ldap.SCOPE_BASE)
-            root_logger.info("Samba domain object already exists")
+            if self.reset_netbios_name:
+                self.__reset_netbios_name()
+            else :
+                self.print_msg("Samba domain object already exists")
             return
         except errors.NotFound:
             pass
@@ -653,13 +670,14 @@ class ADTRUSTInstance(service.Service):
                              FQDN = self.fqdn)
 
     def setup(self, fqdn, ip_address, realm_name, domain_name, netbios_name,
-              rid_base, secondary_rid_base, no_msdcs=False, add_sids=False,
-              smbd_user="samba"):
+              reset_netbios_name, rid_base, secondary_rid_base,
+              no_msdcs=False, add_sids=False, smbd_user="samba"):
         self.fqdn = fqdn
         self.ip_address = ip_address
         self.realm = realm_name
         self.domain_name = domain_name
         self.netbios_name = netbios_name
+        self.reset_netbios_name = reset_netbios_name
         self.rid_base = rid_base
         self.secondary_rid_base = secondary_rid_base
         self.no_msdcs = no_msdcs
-- 
1.7.11.4

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

Reply via email to