On 06/05/2013 10:07 AM, Petr Viktorin wrote:
On 06/05/2013 09:20 AM, Tomas Babej wrote:
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.
Yes, but what if you call a child process after the context manager
exits?
I know that currently there is no code after the context manager
exits, but that's no reason to surprise people who will want to reuse
it later.
Context managers are expected to clean up after themselves. If there's
no way to do this then you should at least document that this one is
special. But in this case it shouldn't be that hard to clean up.
Not at all, I actually had the code there at some point, but removed it.
Updated patch attached.
Tomas
[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 24ea3e0f7b717eff0928bf7bbe783328a12d4107 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 | 22 ++++++++++++++++++++++
5 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 81c11834547c37b01c4749079284affd13bb10d7..3b7e9d206e35e68aef7af64172d34a2ee9f25342 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,
+ private_ccache)
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 private_ccache():
+ 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..47bc31b4786c32caf97f20de3cbf20bc767dfe1d 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 private_ccache():
+ 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..04cad42f6e4c16ee8e4b5076e96dc24bd887828f 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, private_ccache)
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 private_ccache():
+ 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..3e18c8e002275d984fbb81a0a46f81b38e49916e 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 private_ccache():
+ 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..a568eae7c9a16596d9e2d2ead9c4febd1b28a907 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,24 @@ def check_pkcs12(pkcs12_info, ca_file, hostname):
(pkcs12_filename, e))
return server_cert_name
+
+
+@contextmanager
+def private_ccache():
+
+ (desc, path) = tempfile.mkstemp(prefix='krbcc')
+ os.close(desc)
+
+ original_value = os.environ.get('KRB5CCNAME', None)
+
+ os.environ['KRB5CCNAME'] = path
+
+ yield
+
+ if original_value is not None:
+ os.environ['KRB5CCNAME'] = original_value
+ else:
+ os.environ.pop('KRB5CCNAME')
+
+ 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
