URL: https://github.com/freeipa/freeipa/pull/856
Author: stlaz
 Title: #856: adtrustinstance: fix a bug + pep8, py3 fixes
Action: opened

PR body:
"""
There was a bug in adtrustinstance where there was a comparison
of a number to a string which was tailored so that it would always
pass (or at least in the default case). Luckily for us, Python 3 is a bit
more clever so it was throwing exceptions there.

We may want a ticket different from https://pagure.io/freeipa/issue/4985
for that but I am keeping that one for now, will only create a new one if
requested.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/856/head:pr856
git checkout pr856
From f471f653f44c6cc054680ae619bb1496ed79429c Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Wed, 7 Jun 2017 08:10:20 +0200
Subject: [PATCH 1/3] adtrustinstance: fix ID range comparison

The ID range comparison was comparing numbers to a string or possibly
to `None` and was tailored in such a way that the check would always
pass although it went directly against the definition of the absolute
value of a substitution.

https://pagure.io/freeipa/issue/4985
---
 ipaserver/install/adtrustinstance.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 66dd6b57b6..b5d5751276 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -345,9 +345,14 @@ def __add_rid_bases(self):
 
             # Abort if RID bases are too close
             local_range = ranges_with_no_rid_base[0]
-            size = local_range.single_value.get('ipaIDRangeSize')
+            try:
+                size = int(local_range.single_value.get('ipaIDRangeSize'))
+            except ValueError:
+                raise RuntimeError('ipaIDRangeSize is set to a non-integer '
+                                   'value or is not set at all (got {val})'
+                                   .format(val=size))
 
-            if abs(self.rid_base - self.secondary_rid_base) > size:
+            if abs(self.rid_base - self.secondary_rid_base) < size:
                 self.print_msg("Primary and secondary RID base are too close. "
                       "They have to differ at least by %d." % size)
                 raise RuntimeError("RID bases too close.\n")

From 7b9f7a3ce53736dbf4077e3ea8fd04589e6ca131 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Wed, 7 Jun 2017 08:16:17 +0200
Subject: [PATCH 2/3] adtrustinstance: pep8 fix

---
 ipaserver/install/adtrustinstance.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index b5d5751276..c303d032d4 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -354,7 +354,7 @@ def __add_rid_bases(self):
 
             if abs(self.rid_base - self.secondary_rid_base) < size:
                 self.print_msg("Primary and secondary RID base are too close. "
-                      "They have to differ at least by %d." % size)
+                               "They have to differ at least by %d." % size)
                 raise RuntimeError("RID bases too close.\n")
 
             # Modify the range

From 2dfe19fdfab39dd019f10467845222d65ff0f7f9 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Wed, 7 Jun 2017 08:18:32 +0200
Subject: [PATCH 3/3] adtrustinstance: write the conf as a string

Since ipautil.template_file() returns a string, we should not try
to write it as bytes.

https://pagure.io/freeipa/issue/4985
---
 ipaserver/install/adtrustinstance.py | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index c303d032d4..5e5f13f747 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -509,23 +509,16 @@ def __add_s4u2proxy_target(self):
             self.print_msg(UPGRADE_ERROR % dict(dn=targets_dn))
 
     def __write_smb_registry(self):
-        template = os.path.join(paths.USR_SHARE_IPA_DIR, "smb.conf.template")
-        conf = ipautil.template_file(template, self.sub_dict)
-        [tmp_fd, tmp_name] = tempfile.mkstemp()
-        os.write(tmp_fd, conf)
-        os.close(tmp_fd)
-
         # Workaround for: https://fedorahosted.org/freeipa/ticket/5687
         # We make sure that paths.SMB_CONF file exists, hence touch it
         with open(paths.SMB_CONF, 'a'):
             os.utime(paths.SMB_CONF, None)
 
-        args = [paths.NET, "conf", "import", tmp_name]
-
-        try:
-            ipautil.run(args)
-        finally:
-            os.remove(tmp_name)
+        template = os.path.join(paths.USR_SHARE_IPA_DIR, "smb.conf.template")
+        conf = ipautil.template_file(template, self.sub_dict)
+        with tempfile.NamedTemporaryFile(mode='w') as tmp_conf:
+            tmp_conf.write(conf)
+            ipautil.run([paths.NET, "conf", "import", tmp_conf.name])
 
     def __setup_group_membership(self):
         # Add the CIFS and host principals to the 'adtrust agents' group
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to