On 12/02/2011 04:16 PM, Rob Crittenden wrote:
Ondrej Hamada wrote:
On 11/29/2011 10:33 PM, Rob Crittenden wrote:
Ondrej Hamada wrote:
On 11/11/2011 02:55 PM, Ondrej Hamada wrote:
https://fedorahosted.org/freeipa/ticket/2063

In order to check presence of nss_ldap when installing client with
'--no-sssd' option there was added code into ipa-client-install. Check
is base on existence of nss_ldap configuration files. This
configuration could be in 'etc/ldap.conf', '/etc/nss_ldap.conf' or
'/etc/libnss_ldap.conf'. Presence of any of these files is considered
as success otherwise failure.



_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
I've rewritten it. Additionally it checks for existence of nss-pam-ldapd
and makes the results reusable by configure_{ldap|nslcd}_conf()
functions.

https://fedorahosted.org/freeipa/ticket/2063

In order to check presence of nss_ldap or nss-pam-ldapd when installing
client
with '--no-sssd' option there was added code into ipa-client-install.
Checking is based on existence of nss_ldap configuration files. This
configuration could be in 'etc/ldap.conf', '/etc/nss_ldap.conf' or
'/etc/libnss_ldap.conf'. Optionaly the nss_ldap could cooperate with
pam_ldap
module and hence the presence of it is checked by looking for
'pam_ldap.conf' file.
Existence of nss-pam-ldapd is checked against existence of 'nslcd.conf'
file.
All this checking is done by function nssldap_exists().
Because both main modules are maintained by two different functions, the
function
returns tuple containing return code and dictionary structure - its key
is name
of target function and value is list of existing configuration files.
Files to check are specified inside the nssldap_exists() function.

In order to fit the returned values, the functions
configure_{ldap|nslcd}_conf()
were slightly modified. They accept one more parameter which is list of
existing files.
They are not checking existence of above mentioned files anymore.

The patch looks good, just a couple of issues.

1. In the nslcd configurator you add ''.join(files). Did you mean
','.join(files)?

2. The commit message lines wrap making it difficult to read. Can you
limit the lines to ~70 chars per line?

3. I think the message printed when neither package is available can
be simplified to:

One of these packages must be installed: nss_ldap or nss-pam-ldapd

It needs a rebase too.

rob
corrected, corrected, changed, rebased



In order to check presence of nss_ldap or nss-pam-ldapd when
installing client with '--no-sssd' option there was added
code intoipa-client-install. Checking is based on existence
of one of nss_ldap configuration files. This configuration
could be in 'etc/ldap.conf', '/etc/nss_ldap.conf' or
'/etc/libnss_ldap.conf'. Optionaly the nss_ldap could
cooperate with pam_ldap module and hence the presence of it
is checked by looking for 'pam_ldap.conf' file. Existence
of nss-pam-ldapd is checked against existence of
'nslcd.conf' file. All this checking is done by function
nssldap_exists(). Because both modules are maintained by
two different functions, the function returns tuple
containing return code and dictionary structure - its
key is name of target function and value is list of
existing configuration files. Files to check are specified
inside the nssldap_exists() function.

In order to fit the returned values, the functions
configure_{ldap|nslcd}_conf() were slightly modified. They
accept one more parameter which is list of existing files.
They are not checking existence of above mentioned
files anymore.

https://fedorahosted.org/freeipa/ticket/2063


Can you add a block header to nssldap_exists()? I think in particular you need explain that it returns 1 and 0 because that value can eventually be the return value of the installer itself (normally an exists would return True/False).
I've changed it to return True/False and added comment

Seeing a traceback:

# ipa-client-install --no-sssd

[ snip ]

Enrolled in IPA realm EXAMPLE.COM
Created /etc/ipa/default.conf
Configured /etc/krb5.conf for IPA realm EXAMPLE.COM
LDAP enabled
Kerberos 5 enabled
Traceback (most recent call last):
  File "/usr/sbin/ipa-client-install", line 1294, in <module>
    sys.exit(main())
  File "/usr/sbin/ipa-client-install", line 1281, in main
    rval = install(options, env, fstore, statestore)
  File "/usr/sbin/ipa-client-install", line 1211, in install
(retcode, conf, filename) = configurer(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options)
TypeError: configure_ldap_conf() takes exactly 8 arguments (7 given)

rob
corrected

--
Regards,

Ondrej Hamada
FreeIPA team
jabber: [email protected]
IRC: ohamada

From 988f22f934aa67ee1c5065103f3ea4cefe9fd5d8 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada <[email protected]>
Date: Mon, 5 Dec 2011 10:19:10 +0100
Subject: [PATCH] Client install checks for nss_ldap

In order to check presence of nss_ldap or nss-pam-ldapd when
installing client with '--no-sssd' option there was added
code into ipa-client-install. Checking is based on existence
of one of nss_ldap configuration files. This configuration
could be in 'etc/ldap.conf', '/etc/nss_ldap.conf' or
'/etc/libnss_ldap.conf'. Optionaly the nss_ldap could
cooperate with pam_ldap module and hence the presence of it
is checked by looking for 'pam_ldap.conf' file. Existence
of nss-pam-ldapd is checked against existence of
'nslcd.conf' file. All this checking is done by function
nssldap_exists(). Because both modules are maintained by
two different functions, the function returns tuple
containing return code and dictionary structure - its
key is name of target function and value is list of
existing configuration files. Files to check are specified
inside the nssldap_exists() function. nssldap_exists() also
returns True if any of the mandatory files was found,
otherwise returns False.

In order to fit the returned values, the functions
configure_{ldap|nslcd}_conf() were slightly modified. They
accept one more parameter which is list of existing files.
They are not checking existence of above mentioned
files anymore.

https://fedorahosted.org/freeipa/ticket/2063
---
 ipa-client/ipa-install/ipa-client-install |   62 ++++++++++++++++++++--------
 1 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 0a040b8fa3a183b4ac868362ed1ca4c3a54c0343..e763d07a738b6c2d78782ff24bba77b7eb31413b 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -145,6 +145,27 @@ def nickname_exists(nickname):
         else:
             return False
 
+# Checks whether nss_ldap or nss-pam-ldapd is installed. If anyone of mandatory files was found returns True and list of all files found.
+def nssldap_exists():
+    files_to_check = [{'function':'configure_ldap_conf', 'mandatory':['/etc/ldap.conf','/etc/nss_ldap.conf','/etc/libnss-ldap.conf'], 'optional':['/etc/pam_ldap.conf']},
+                      {'function':'configure_nslcd_conf', 'mandatory':['/etc/nslcd.conf']}]
+    files_found = {}
+    retval = False
+
+    for function in files_to_check:
+        files_found[function['function']]=[]
+        for file_type in ['mandatory','optional']:
+            try:
+                for filename in function[file_type]:
+                    if file_exists(filename):
+                        files_found[function['function']].append(filename)
+                        if file_type == 'mandatory':
+                            retval = True
+            except KeyError:
+                pass
+
+    return (retval, files_found)
+
 def emit_quiet(quiet, message):
     if not quiet:
         print message
@@ -409,7 +430,7 @@ def configure_ipa_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server):
 
     return 0
 
-def configure_ldap_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options):
+def configure_ldap_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options, files):
     ldapconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Installer")
     ldapconf.setOptionAssignment(" ")
 
@@ -440,24 +461,22 @@ def configure_ldap_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, d
     opts.append({'name':'empty', 'type':'empty'})
 
     ret = (0, None, None)
-    files = []
+
     # Depending on the release and distribution this may exist in any
     # number of different file names, update what we find
-    for filename in ['/etc/ldap.conf', '/etc/nss_ldap.conf', '/etc/libnss-ldap.conf', '/etc/pam_ldap.conf']:
-        if file_exists(filename):
-            try:
-                fstore.backup_file(filename)
-                ldapconf.newConf(filename, opts)
-                files.append(filename)
-            except Exception, e:
-                print "Creation of %s: %s" % (filename, str(e))
-                return (1, 'LDAP', filename)
+    for filename in files:
+        try:
+            fstore.backup_file(filename)
+            ldapconf.newConf(filename, opts)
+        except Exception, e:
+            print "Creation of %s: %s" % (filename, str(e))
+            return (1, 'LDAP', filename)
 
     if files:
         return (0, 'LDAP', ', '.join(files))
     return ret
 
-def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options):
+def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options, files):
     nslcdconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Installer")
     nslcdconf.setOptionAssignment(" ")
 
@@ -481,12 +500,12 @@ def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server,
 
     opts.append({'name':'empty', 'type':'empty'})
 
-    if file_exists('/etc/nslcd.conf'):
+    for filename in files:
         try:
-            fstore.backup_file('/etc/nslcd.conf')
-            nslcdconf.newConf('/etc/nslcd.conf', opts)
+            fstore.backup_file(filename)
+            nslcdconf.newConf(filename, opts)
         except Exception, e:
-            print "Creation of %s: %s" % ('/etc/nslcd.conf', str(e))
+            print "Creation of %s: %s" % (filename, str(e))
             return (1, None, None)
 
     nslcd = ipaservices.knownservices.nslcd
@@ -505,7 +524,7 @@ def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server,
         root_logger.debug("%s daemon is not installed, skip configuration" % (nslcd.service_name))
         return (0, None, None)
 
-    return (0, 'NSLCD', '/etc/nslcd.conf')
+    return (0, 'NSLCD', ', '.join(files))
 
 def hardcode_ldap_server(cli_server):
     """
@@ -851,6 +870,13 @@ def install(options, env, fstore, statestore):
         print 'Invalid hostname \'%s\', must be lower-case.' % hostname
         return CLIENT_INSTALL_ERROR
 
+    # when installing with '--no-sssd' option, check whether nss-ldap is installed
+    if not options.sssd:
+        (nssldap_installed, nosssd_files) = nssldap_exists()
+        if not nssldap_installed:
+            print "One of these packages must be installed: nss_ldap or nss-pam-ldapd"
+            return CLIENT_INSTALL_ERROR
+
     # Create the discovery instance
     ds = ipadiscovery.IPADiscovery()
 
@@ -1183,7 +1209,7 @@ def install(options, env, fstore, statestore):
     # change its configuration otherways
     if not options.sssd:
         for configurer in [configure_ldap_conf, configure_nslcd_conf]:
-            (retcode, conf, filename) = configurer(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options)
+            (retcode, conf, filename) = configurer(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options, nosssd_files[configurer.__name__])
             if retcode:
                 return CLIENT_INSTALL_ERROR
             if conf:
-- 
1.7.6.4

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to