On 06/04/2013 06:09 PM, Petr Viktorin wrote:
On 06/04/2013 01:29 PM, Tomas Babej wrote:
On 06/03/2013 02:58 PM, Martin Kosek wrote:
On 06/03/2013 02:43 PM, Tomas Babej wrote:
Hi,

this patch fixes the installation problems on master on F19 with krb5
packages
= 1.11.2-6
https://fedorahosted.org/freeipa/ticket/3666

Tomas
1) Leaving cache_desc open:

+        (cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
+        os.environ['KRB5CCNAME'] = cache_path

Why do we keep the descriptor open and close it at the and of the
installation?
Can we close it right after tempfile.mkstemp? I think we do it this
way in
other places in installation.

2) What about other installers where we handle Kerberos auth, like
ipa-{replica,dns,ca}-install?

A common function, other shared means, of handling KRB5CCNAME may be
appropriate to avoid duplicating code too much.

Martin
I moved the code responsible to PrivateCCache class, both for
readability and conciseness.

Private ccache now used in replica,dns and ca the installers. I managed
to reproduce the error only with
dns-install though(fails on adding the service principal), but having a
private ccache for the installer should not hurt.

Ipa-adtrust-install requires the admin ticket, so there shouldn't be an
issue.

Tomas

Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?

There's no need to, since the value of the environment variable is inherited only into child processes (pkispawn, etc.). Hence the KRB5CCNAME
environment variable is not available outside the install script.

[root@vm-002 labtool]# ipa-server-install
[snip]
Be sure to back up the CA certificate stored in /root/cacert.p12
This file is required to create replicas. The password for this
file is the Directory Manager password

[root@vm-002 labtool]# echo $KRB5CCNAME

[root@vm-002 labtool]#


Two nitpicks:

ipaserver/install/installutils.py: new blank line at EOF

For most context managers I prefer @contextlib.contextmanager rather than writing out the class, it makes them shorter and easier to read

Addressed in the updated patch.

Tomas
From 1e8fac58c0af6626129ba8934d5d4ed6e29698f2 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 3 Jun 2013 12:06:06 +0200
Subject: [PATCH] Use private ccache in ipa install tools

All installers that handle Kerberos auth, have been altered to use
private ccache, that is ipa-server-install, ipa-dns-install,
ipa-replica-install, ipa-ca-install.

https://fedorahosted.org/freeipa/ticket/3666
---
 install/tools/ipa-ca-install      | 13 +++++++------
 install/tools/ipa-dns-install     |  5 +++--
 install/tools/ipa-replica-install | 13 +++++++------
 install/tools/ipa-server-install  |  7 +++++--
 ipaserver/install/installutils.py | 14 ++++++++++++++
 5 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 81c11834547c37b01c4749079284affd13bb10d7..fcc8240583402eabb80a6bc701ae05d46adf0f60 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -28,9 +28,9 @@ from ipapython import services as ipaservices
 
 from ipaserver.install import installutils, service
 from ipaserver.install import certs
-from ipaserver.install.installutils import HostnameLocalhost
-from ipaserver.install.installutils import ReplicaConfig, expand_replica_info, read_replica_info
-from ipaserver.install.installutils import get_host_name, BadHostError
+from ipaserver.install.installutils import (HostnameLocalhost, ReplicaConfig,
+        expand_replica_info, read_replica_info, get_host_name, BadHostError,
+        privateCCache)
 from ipaserver.install import dsinstance, cainstance, bindinstance
 from ipaserver.install.replication import replica_conn_check
 from ipapython import version
@@ -212,9 +212,10 @@ Run /usr/sbin/ipa-server-install --uninstall to clean up.
 
 if __name__ == '__main__':
     try:
-        installutils.run_script(main, log_file_name=log_file_name,
-                operation_name='ipa-ca-install',
-                fail_message=fail_message)
+        with privateCCache():
+            installutils.run_script(main, log_file_name=log_file_name,
+                    operation_name='ipa-ca-install',
+                    fail_message=fail_message)
     finally:
         # always try to remove decrypted replica file
         try:
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index e12a0465ca2d09a6a8d25157a737f620f3ff4b1a..8321ca1161229bdb1462b4dff380bf7f0d4af3bf 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -258,5 +258,6 @@ def main():
     return 0
 
 if __name__ == '__main__':
-    installutils.run_script(main, log_file_name=log_file_name,
-        operation_name='ipa-dns-install')
+    with privateCCache():
+        installutils.run_script(main, log_file_name=log_file_name,
+            operation_name='ipa-dns-install')
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index b194b85a201c2d842938d3251fa9179c57d0bd68..c515d800d24721d6304dd6726cf28f82f16aa130 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -36,9 +36,9 @@ from ipaserver.install import dsinstance, installutils, krbinstance, service
 from ipaserver.install import bindinstance, httpinstance, ntpinstance, certs
 from ipaserver.install import memcacheinstance
 from ipaserver.install.replication import replica_conn_check, ReplicationManager
-from ipaserver.install.installutils import HostnameLocalhost, resolve_host
-from ipaserver.install.installutils import ReplicaConfig, expand_replica_info, read_replica_info
-from ipaserver.install.installutils import get_host_name, BadHostError
+from ipaserver.install.installutils import (HostnameLocalhost, resolve_host,
+        ReplicaConfig, expand_replica_info, read_replica_info ,get_host_name,
+        BadHostError, privateCCache)
 from ipaserver.plugins.ldap2 import ldap2
 from ipaserver.install import cainstance
 from ipalib import api, errors, util
@@ -726,9 +726,10 @@ Run /usr/sbin/ipa-server-install --uninstall to clean up.
 
 if __name__ == '__main__':
     try:
-        installutils.run_script(main, log_file_name=log_file_name,
-                operation_name='ipa-replica-install',
-                fail_message=fail_message)
+        with privateCCache():
+            installutils.run_script(main, log_file_name=log_file_name,
+                    operation_name='ipa-replica-install',
+                    fail_message=fail_message)
     finally:
         # always try to remove decrypted replica file
         try:
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 62adbd5bc5183793f3371e46e276b9ad20077b84..327044bbc0a8950e30472fe3ed14ccc5408e4dcd 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -1210,6 +1210,7 @@ def main():
 
 if __name__ == '__main__':
     success = False
+
     try:
         # FIXME: Common option parsing, logging setup, etc should be factored
         # out from all install scripts
@@ -1219,8 +1220,10 @@ if __name__ == '__main__':
         else:
             log_file_name = "/var/log/ipaserver-install.log"
 
-        installutils.run_script(main, log_file_name=log_file_name,
-            operation_name='ipa-server-install')
+        # Use private ccache
+        with privateCCache():
+            installutils.run_script(main, log_file_name=log_file_name,
+                                    operation_name='ipa-server-install')
         success = True
 
     finally:
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 5ed2689d75ab5b372a40031f03bab5000302752c..769e788622d86ae4d77e7acb19b66497fa0a095a 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -28,6 +28,7 @@ import shutil
 from ConfigParser import SafeConfigParser, NoOptionError
 import traceback
 import textwrap
+from contextlib import contextmanager
 
 from dns import resolver, rdatatype
 from dns.exception import DNSException
@@ -753,3 +754,16 @@ def check_pkcs12(pkcs12_info, ca_file, hostname):
                 (pkcs12_filename, e))
 
         return server_cert_name
+
+
+@contextmanager
+def privateCCache():
+
+    (desc, path) = tempfile.mkstemp(prefix='krbcc')
+    os.close(desc)
+    os.environ['KRB5CCNAME'] = path
+
+    yield
+
+    if os.path.exists(path):
+        os.remove(path)
-- 
1.8.1.4

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

Reply via email to