On Wed, 2015-08-05 at 11:24 -0400, Simo Sorce wrote:
> On Wed, 2015-08-05 at 08:20 +0200, Jan Cholasta wrote:
> > Hi,
> > 
> > Dne 31.7.2015 v 12:46 Simo Sorce napsal(a):
> > > I've been carrying these patches in my tree for a while, I think it is
> > > time to put them in master as they stand on their own.
> > >
> > > Simo.
> > 
> > Patch 530: ACK
> > 
> > Patch 531: ACK
> > 
> > Patch 532:
> > 
> > The methods should be static methods:
> > 
> >      @staticmethod
> >      def setOption(name, value):
> >      ...
> 
> Care to explain why ?
> @staticmethod is not used anywhere else in that file.

Rebased patches on master, made requested change +1 more patch.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
>From a124cd5a1361b7d90d918128cffddedc4a75c40c Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Wed, 1 Jul 2015 09:40:09 -0400
Subject: [PATCH 1/6] Remove custom utility function from krbinstance

Remove the custom update_key_val_in_file() and instead use the common
function config_replace_variables() available from ipautil.

Signed-off-by: Simo Sorce <s...@redhat.com>
---
 ipaserver/install/krbinstance.py | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 87491482683e01a10cf30eae28fbe89ae5b027c0..9f5ddcd2cc5c3a86da88cef1da37a10ae1096dc2 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -49,26 +49,6 @@ from distutils import version
 from ipaplatform.tasks import tasks
 from ipaplatform.paths import paths
 
-def update_key_val_in_file(filename, key, val):
-    if os.path.exists(filename):
-        pattern = "^[\s#]*%s\s*=\s*%s\s*" % (re.escape(key), re.escape(val))
-        p = re.compile(pattern)
-        for line in fileinput.input(filename):
-            if p.search(line):
-                fileinput.close()
-                return
-        fileinput.close()
-
-        pattern = "^[\s#]*%s\s*=" % re.escape(key)
-        p = re.compile(pattern)
-        for line in fileinput.input(filename, inplace=1):
-            if not p.search(line):
-                sys.stdout.write(line)
-        fileinput.close()
-    f = open(filename, "a")
-    f.write("%s=%s\n" % (key, val))
-    f.close()
-
 class KpasswdInstance(service.SimpleServiceInstance):
     def __init__(self):
         service.SimpleServiceInstance.__init__(self, "kadmin")
@@ -386,7 +366,9 @@ class KrbInstance(service.Service):
         self.fstore.backup_file(paths.DS_KEYTAB)
         installutils.create_keytab(paths.DS_KEYTAB, ldap_principal)
 
-        update_key_val_in_file(paths.SYSCONFIG_DIRSRV, "KRB5_KTNAME", paths.DS_KEYTAB)
+        vardict = {"KRB5_KTNAME": paths.DS_KEYTAB}
+        ipautil.config_replace_variables(paths.SYSCONFIG_DIRSRV,
+                                         replacevars=vardict)
         pent = pwd.getpwnam(dsinstance.DS_USER)
         os.chown(paths.DS_KEYTAB, pent.pw_uid, pent.pw_gid)
 
-- 
2.4.3

>From f3dcea7fc6cfece067400f3fff7bdddf8060c4ba Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Sun, 5 Jul 2015 07:18:25 -0400
Subject: [PATCH 2/6] Move sasl mappings creation to dsinstance

Sasl mappings can be created directly by the DS Instance, there is
no reason to create them in the krbinstance as they do not depend on
the kdc to be configured just to be created.

Signed-off-by: Simo Sorce <s...@redhat.com>
---
 ipaserver/install/dsinstance.py  | 51 ++++++++++++++++++++++++++++++++++++++++
 ipaserver/install/krbinstance.py | 48 -------------------------------------
 2 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 6089dd85a0d5f53a3a9afda1b25ec4a621366894..075c70f12a232f10f599e2cbd5424da0113cc0ae 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -354,6 +354,7 @@ class DsInstance(service.Service):
         self.__common_setup(True)
 
         self.step("setting up initial replication", self.__setup_replica)
+        self.step("adding sasl mappings to the directory", self.__configure_sasl_mappings)
         self.step("updating schema", self.__update_schema)
         # See LDIFs for automember configuration during replica install
         self.step("setting Auto Member configuration", self.__add_replica_automember_config)
@@ -378,6 +379,56 @@ class DsInstance(service.Service):
                                r_bindpw=self.dm_password)
         self.run_init_memberof = repl.needs_memberof_fixup()
 
+
+    def __configure_sasl_mappings(self):
+        # we need to remove any existing SASL mappings in the directory as otherwise they
+        # they may conflict.
+
+        if not self.admin_conn:
+            self.ldap_connect()
+
+        try:
+            res = self.admin_conn.get_entries(
+                DN(('cn', 'mapping'), ('cn', 'sasl'), ('cn', 'config')),
+                self.admin_conn.SCOPE_ONELEVEL,
+                "(objectclass=nsSaslMapping)")
+            for r in res:
+                try:
+                    self.admin_conn.delete_entry(r)
+                except Exception, e:
+                    root_logger.critical(
+                        "Error during SASL mapping removal: %s", e)
+                    raise
+        except Exception, e:
+            root_logger.critical("Error while enumerating SASL mappings %s", e)
+            raise
+
+        entry = self.admin_conn.make_entry(
+            DN(
+                ('cn', 'Full Principal'), ('cn', 'mapping'), ('cn', 'sasl'),
+                ('cn', 'config')),
+            objectclass=["top", "nsSaslMapping"],
+            cn=["Full Principal"],
+            nsSaslMapRegexString=['\(.*\)@\(.*\)'],
+            nsSaslMapBaseDNTemplate=[self.suffix],
+            nsSaslMapFilterTemplate=['(krbPrincipalName=\\1@\\2)'],
+            nsSaslMapPriority=['10'],
+        )
+        self.admin_conn.add_entry(entry)
+
+        entry = self.admin_conn.make_entry(
+            DN(
+                ('cn', 'Name Only'), ('cn', 'mapping'), ('cn', 'sasl'),
+                ('cn', 'config')),
+            objectclass=["top", "nsSaslMapping"],
+            cn=["Name Only"],
+            nsSaslMapRegexString=['^[^:@]+$'],
+            nsSaslMapBaseDNTemplate=[self.suffix],
+            nsSaslMapFilterTemplate=['(krbPrincipalName=&@%s)' % self.realm],
+            nsSaslMapPriority=['10'],
+        )
+        self.admin_conn.add_entry(entry)
+
     def __update_schema(self):
         # FIXME: https://fedorahosted.org/389/ticket/47490
         self._ldap_mod("schema-update.ldif")
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 9f5ddcd2cc5c3a86da88cef1da37a10ae1096dc2..5670cc264a4358d60edd3c22d9a8c86c11d66bce 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -150,7 +150,6 @@ class KrbInstance(service.Service):
 
         self.__common_setup(realm_name, host_name, domain_name, admin_password)
 
-        self.step("adding sasl mappings to the directory", self.__configure_sasl_mappings)
         self.step("adding kerberos container to the directory", self.__add_krb_container)
         self.step("configuring KDC", self.__configure_instance)
         self.step("initialize kerberos container", self.__init_ipa_kdb)
@@ -180,7 +179,6 @@ class KrbInstance(service.Service):
 
         self.__common_setup(realm_name, host_name, domain_name, admin_password)
 
-        self.step("adding sasl mappings to the directory", self.__configure_sasl_mappings)
         self.step("configuring KDC", self.__configure_instance)
         self.step("creating a keytab for the directory", self.__create_ds_keytab)
         self.step("creating a keytab for the machine", self.__create_host_keytab)
@@ -245,52 +243,6 @@ class KrbInstance(service.Service):
             root_logger.debug("Persistent keyring CCACHE is not enabled")
             self.sub_dict['OTHER_LIBDEFAULTS'] = ''
 
-    def __configure_sasl_mappings(self):
-        # we need to remove any existing SASL mappings in the directory as otherwise they
-        # they may conflict.
-
-        try:
-            res = self.admin_conn.get_entries(
-                DN(('cn', 'mapping'), ('cn', 'sasl'), ('cn', 'config')),
-                self.admin_conn.SCOPE_ONELEVEL,
-                "(objectclass=nsSaslMapping)")
-            for r in res:
-                try:
-                    self.admin_conn.delete_entry(r)
-                except Exception as e:
-                    root_logger.critical(
-                        "Error during SASL mapping removal: %s", e)
-                    raise
-        except Exception as e:
-            root_logger.critical("Error while enumerating SASL mappings %s", e)
-            raise
-
-        entry = self.admin_conn.make_entry(
-            DN(
-                ('cn', 'Full Principal'), ('cn', 'mapping'), ('cn', 'sasl'),
-                ('cn', 'config')),
-            objectclass=["top", "nsSaslMapping"],
-            cn=["Full Principal"],
-            nsSaslMapRegexString=['\(.*\)@\(.*\)'],
-            nsSaslMapBaseDNTemplate=[self.suffix],
-            nsSaslMapFilterTemplate=['(krbPrincipalName=\\1@\\2)'],
-            nsSaslMapPriority=['10'],
-        )
-        self.admin_conn.add_entry(entry)
-
-        entry = self.admin_conn.make_entry(
-            DN(
-                ('cn', 'Name Only'), ('cn', 'mapping'), ('cn', 'sasl'),
-                ('cn', 'config')),
-            objectclass=["top", "nsSaslMapping"],
-            cn=["Name Only"],
-            nsSaslMapRegexString=['^[^:@]+$'],
-            nsSaslMapBaseDNTemplate=[self.suffix],
-            nsSaslMapFilterTemplate=['(krbPrincipalName=&@%s)' % self.realm],
-            nsSaslMapPriority=['10'],
-        )
-        self.admin_conn.add_entry(entry)
-
     def __add_krb_container(self):
         self._ldap_mod("kerberos.ldif", self.sub_dict)
 
-- 
2.4.3

>From 8b5344a45089f8f8a8cfdc6cf4049ad955f5ba89 Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Fri, 26 Jun 2015 20:47:57 -0400
Subject: [PATCH 3/6] Simplify adding options in ipachangeconf

Signed-off-by: Simo Sorce <s...@redhat.com>
---
 ipa-client/ipaclient/ipachangeconf.py | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/ipa-client/ipaclient/ipachangeconf.py b/ipa-client/ipaclient/ipachangeconf.py
index ed53c9dea598993540451269e6608d547d34a2af..58679f3de7c2bb99d6ddceeca60280d687d471a6 100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -536,3 +536,24 @@ class IPAChangeConf:
             except IOError:
                 pass
         return True
+
+    @staticmethod
+    def setOption(name, value):
+        return {'name': name,
+                'type': 'option',
+                'action': 'set',
+                'value': value}
+
+    @staticmethod
+    def rmOption(name):
+        return {'name': name,
+                'type': 'option',
+                'action': 'remove',
+                'value': None}
+
+    @staticmethod
+    def setSection(name, options):
+        return {'name': name,
+                'type': 'section',
+                'action': 'set',
+                'value': options}
-- 
2.4.3

>From 6b4d193992525afb5e8d4ec0373959e6e4df3260 Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Tue, 4 Aug 2015 10:15:36 -0400
Subject: [PATCH 4/6] Insure the admin_conn is disconnected on stop

If we stop or restart the server insure admin_conn gets reset or other
parts may fail to properly connect/authenticate

Signed-off-by: Simo Sorce <s...@redhat.com>
---
 ipaserver/install/dsinstance.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 075c70f12a232f10f599e2cbd5424da0113cc0ae..8320569601a16cd2745796a955181bddde307404 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -530,7 +530,14 @@ class DsInstance(service.Service):
             # Does not apply with newer DS releases
             pass
 
+    def stop(self, *args, **kwargs):
+        if self.admin_conn:
+            self.ldap_disconnect()
+        super(DsInstance, self).stop(*args, **kwargs)
+
     def restart(self, instance=''):
+        if self.admin_conn:
+            self.ldap_disconnect()
         try:
             super(DsInstance, self).restart(instance)
             if not is_ds_running(instance):
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to