Also I made explicit conversion to list for default values upon Petr's
request.

On 09/04/2014 12:32 PM, Tomas Babej wrote:
> On 09/03/2014 04:53 PM, Petr Viktorin wrote:
>> On 09/03/2014 03:53 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> Makes sure that any new sources added are not already present
>>> in the entry.
>>>
>>> https://fedorahosted.org/freeipa/ticket/4508
>> It works fine, ACK.
>>
>> I do have some comments, but 4.0.x is a stabilization release, so
>> they'd probably be better in a 4.1 patch:
>>
>> The way you first join default_value to make configured_services, and
>> then repeatedly split it, looks quite wasteful. Wouldn't
>> configured_services be better as a list?
> Yes, the string handling was kind of unfortunate. I fixed it in this
> iteration of the patch.
>
>> Also I wonder if configure_nsswitch_database needs those unused
>> preserve/append options.
> Although not used yet, I think it's handy to have them there, they do
> not complicate the code much.
>
>> Should I push now?
>>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

>From 5475706d74a07f87fbc3e0f7a530af59aeed9ad8 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 27 Aug 2014 09:10:59 +0200
Subject: [PATCH] ipa-client-install: Do not add already configured sources to
 nsswitch.conf entries

Makes sure that any new sources added are not already present
in the entry.

https://fedorahosted.org/freeipa/ticket/4508
---
 ipa-client/ipa-install/ipa-client-install | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 08fefc86d31392e9abf66ee4f8fff54a88179795..6b5ebf91e16fbead7cb38e1f5107f092b069dc7c 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -430,28 +430,34 @@ def configure_nsswitch_database(fstore, database, services, preserve=True,
             opts = conf.parse(f)
             raw_database_entry = conf.findOpts(opts, 'option', database)[1]
 
-            if not raw_database_entry:
-                # If there is no database entry, database is not present in
-                # the nsswitch.conf. Set the list of services to the
-                # default list, if passed.
-                configured_services = ' '.join(default_value or [])
-            else:
-                configured_services = raw_database_entry['value'].strip()
+        # Detect the list of already configured services
+        if not raw_database_entry:
+            # If there is no database entry, database is not present in
+            # the nsswitch.conf. Set the list of services to the
+            # default list, if passed.
+            configured_services = list(default_value) or []
+        else:
+            configured_services = raw_database_entry['value'].strip().split()
 
+        # Make sure no service is added if already mentioned in the list
+        added_services = [s for s in services
+                          if s not in configured_services]
+
+        # Prepend / append the list of new services
         if append:
-            new_services = ' ' + configured_services + ' ' + ' '.join(services)
+            new_value = ' ' + ' '.join(configured_services + added_services)
         else:
-            new_services = ' ' +  ' '.join(services) + ' ' + configured_services
+            new_value = ' ' + ' '.join(added_services + configured_services)
 
     else:
         # Preserve not set, let's rewrite existing configuration
-        new_services = ' ' + ' '.join(services)
+        new_value = ' ' + ' '.join(services)
 
     # Set new services as sources for database
     opts = [{'name': database,
              'type':'option',
              'action':'set',
-             'value': new_services
+             'value': new_value
             },
             {'name':'empty',
              'type':'empty'
-- 
1.9.3

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

Reply via email to