-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/20/2010 03:33 PM, Jakub Hrozek wrote:
> On 12/20/2010 02:49 PM, Jakub Hrozek wrote:
>> Attached is a patch that changes the uniqueness constraint of automount
>> keys from (key) to (key,info) pairs. The patch is not really standard
>> baseldap style. The reason is that during development, I found that
>> baseldap is really dependent on having a single primary key and also
>> during many operations accessing it as keys[-1].
> 
>> Please note that the ipa automountkey-* commands used to have three
>> args, now its two args and two required options (that compose the tuple
>> that is primary key). I know next to nothing about UI, but I assume this
>> has consequences as the JSON marshalled call needs to be different now.
>> Can someone point me to the place in code that I need to fix now?
> 
>> Fixes:
>> https://fedorahosted.org/freeipa/ticket/293
> 
> Sorry, I left some debugging statements in. Attached is a new patch.

Attached is a patch that applies cleanly on top of origin/master.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk0rMKEACgkQHsardTLnvCUwyACfV2eIX/+c0hjKsLyRkGLnBH3r
sesAn2xpp8dXdZMuRQ4SICupUDP9EbwR
=JhYQ
-----END PGP SIGNATURE-----
From 7fbffdf0dcc4ae8da26b1b3a24bcdce5d632d5ae Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Sun, 19 Dec 2010 21:23:16 +0100
Subject: [PATCH] Enforce uniqueness on (key,info) pairs in automount keys

https://fedorahosted.org/freeipa/ticket/293
---
 install/share/bootstrap-template.ldif      |    3 +-
 ipalib/plugins/automount.py                |  177 ++++++++++++++++++++++++++--
 tests/test_xmlrpc/test_automount_plugin.py |   83 ++++++++++---
 3 files changed, 235 insertions(+), 28 deletions(-)

diff --git a/install/share/bootstrap-template.ldif b/install/share/bootstrap-template.ldif
index 52f0c97..fb943f1 100644
--- a/install/share/bootstrap-template.ldif
+++ b/install/share/bootstrap-template.ldif
@@ -64,11 +64,12 @@ changetype: add
 objectClass: automountMap
 automountMapName: auto.direct
 
-dn: automountkey=/-,automountmapname=auto.master,cn=default,cn=automount,$SUFFIX
+dn: description=/- auto.direct,automountmapname=auto.master,cn=default,cn=automount,$SUFFIX
 changetype: add
 objectClass: automount
 automountKey: /-
 automountInformation: auto.direct
+description: /- auto.direct
 
 dn: cn=hbacservices,cn=accounts,$SUFFIX
 changetype: add
diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index 39605d4..a568908 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -88,16 +88,19 @@ Keys:
 
   Create a new key for the auto.share map in location baltimore. This ties
   the map we previously created to auto.master:
-  ipa automountkey-add baltimore auto.master /share --info=auto.share
+    ipa automountkey-add baltimore auto.master --key=/share --info=auto.share
 
   Create a new key for our auto.share map, an NFS mount for man pages:
-    ipa automountkey-add baltimore auto.share man --info="-ro,soft,rsize=8192,wsize=8192 ipa.example.com:/shared/man"
+    ipa automountkey-add baltimore auto.share --key=man --info="-ro,soft,rsize=8192,wsize=8192 ipa.example.com:/shared/man"
 
   Find all keys for the auto.share map:
-    ipa automountkey-find baltimore auto.share
+    ipa automountkey-find baltimore --info=auto.share
+
+  Find all direct automount keys:
+    ipa automountkey-find baltimore --key=/-
 
   Remove the man key from the auto.share map:
-    ipa automountkey-del baltimore auto.share man
+    ipa automountkey-del baltimore auto.share --key=man
 """
 
 """
@@ -362,7 +365,11 @@ class automountlocation_import(LDAPQuery):
 
             # Add a new key to the auto.master map for the new map file
             try:
-                api.Command['automountkey_add'](args[0], u'auto.master', unicode(am[0]), automountinformation=unicode(' '.join(am[1:])))
+                api.Command['automountkey_add'](
+                            args[0],
+                            u'auto.master',
+                            automountkey=unicode(am[0]),
+                            automountinformation=unicode(' '.join(am[1:])))
                 result['keys'].append([am[0], u'auto.master'])
             except errors.DuplicateEntry, e:
                 if options.get('continue', False):
@@ -410,7 +417,11 @@ class automountlocation_import(LDAPQuery):
                 am = x.split(None)
                 key = unicode(am[0].replace('"',''))
                 try:
-                    api.Command['automountkey_add'](args[0], unicode(m), key, automountinformation=unicode(' '.join(am[1:])))
+                    api.Command['automountkey_add'](
+                            args[0],
+                            unicode(m),
+                            automountkey=key,
+                            automountinformation=unicode(' '.join(am[1:])))
                     result['keys'].append([key,m])
                 except errors.DuplicateEntry, e:
                     if options.get('continue', False):
@@ -566,25 +577,86 @@ class automountkey(LDAPObject):
     default_attributes = [
         'automountkey', 'automountinformation', 'description'
     ]
+    rdnattr = 'description'
+    rdn_separator = ' '
 
     takes_params = (
         IA5Str('automountkey',
                cli_name='key',
                label=_('Key'),
                doc=_('Automount key name'),
-               primary_key=True,
         ),
         IA5Str('automountinformation',
                cli_name='info',
                label=_('Mount information'),
         ),
-        Str('description?',
-            cli_name='desc',
+        Str('description',
             label=_('description'),
+            primary_key=True,
+            required=False,
+            flags=['no_create', 'no_update', 'no_search', 'no_output'],
+            exclude='webui',
         ),
     )
 
+    num_parents = 2
     label = _('Automount Keys')
+    already_exists_msg = _('The key,info pair must be unique. A key named %(key)s with info %(info)s already exists')
+    object_not_found_msg = _('The automount key %(key)s with info %(info)s does not exist')
+
+    def get_dn(self, *keys, **kwargs):
+        # all commands except for create send pk in keys, too
+        # create cannot due to validation in frontend.py
+        if len(keys) == self.num_parents:
+            try:
+                pkey = kwargs[self.primary_key.name]
+            except KeyError:
+                raise ValueError('Not enough keys and pkey not in kwargs')
+            parent_keys = keys
+        else:
+            pkey = keys[-1]
+            parent_keys = keys[:-1]
+
+        parent_dn = self.api.Object[self.parent_object].get_dn(*parent_keys)
+        dn = self.backend.make_dn_from_attr(
+            self.primary_key.name,
+            pkey,
+            parent_dn
+        )
+        return dn
+
+    def handle_not_found(self, *keys):
+        pkey = keys[-1]
+        key = pkey.split(self.rdn_separator)[0]
+        info = self.rdn_separator.join(pkey.split(self.rdn_separator)[1:])
+        raise errors.NotFound(
+            reason=self.object_not_found_msg % {
+                'key': key, 'info': info,
+            }
+        )
+
+    def handle_duplicate_entry(self, *keys):
+        pkey = keys[-1]
+        key = pkey.split(self.rdn_separator)[0]
+        info = self.rdn_separator.join(pkey.split(self.rdn_separator)[1:])
+        raise errors.DuplicateEntry(
+            message=self.already_exists_msg % {
+                'key': key, 'info': info,
+            }
+        )
+
+    def get_pk(self, key, info):
+        return self.rdn_separator.join((key,info))
+
+    def check_key_uniqueness(self, location, map, **keykw):
+        key = keykw.get('automountkey')
+        info = keykw.get('automountinformation')
+        if key is None or info is None:
+            return
+
+        entries = self.methods.find(location, map, **keykw)['result']
+        if len(entries) > 0:
+            self.handle_duplicate_entry(location, map, self.get_pk(key, info))
 
 api.register(automountkey)
 
@@ -593,6 +665,19 @@ class automountkey_add(LDAPCreate):
     """
     Create new automount key.
     """
+    def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        self.obj.check_key_uniqueness(keys[-2], keys[-1], **options)
+        return dn
+
+    def get_args(self):
+        for key in self.obj.get_ancestor_primary_keys():
+            yield key
+
+    def execute(self, *keys, **options):
+        options[self.obj.primary_key.name] = self.obj.get_pk(
+                                            options['automountkey'],
+                                            options['automountinformation'])
+        return super(automountkey_add, self).execute(*keys, **options)
 
 api.register(automountkey_add)
 
@@ -619,7 +704,8 @@ class automountmap_add_indirect(LDAPCreate):
         result = self.api.Command['automountmap_add'](*keys, **options)
         options['automountinformation'] = keys[1]
         self.api.Command['automountkey_add'](
-            keys[0], options['parentmap'], options['key'], **options
+            keys[0], options['parentmap'],
+            automountkey=options['key'], **options
         )
         return result
 
@@ -630,6 +716,29 @@ class automountkey_del(LDAPDelete):
     """
     Delete automount key.
     """
+    takes_options = LDAPDelete.takes_options + (
+        IA5Str('automountkey',
+               cli_name='key',
+               label=_('Key'),
+               doc=_('Automount key name'),
+        ),
+        IA5Str('automountinformation',
+               cli_name='info',
+               label=_('Mount information'),
+        ),
+    )
+
+    def get_args(self):
+        for key in self.obj.get_ancestor_primary_keys():
+            yield key
+
+    def execute(self, *keys, **options):
+        keys += (self.obj.get_pk(options['automountkey'],
+                                 options['automountinformation']), )
+        options[self.obj.primary_key.name] = self.obj.get_pk(
+                                            options['automountkey'],
+                                            options['automountinformation'])
+        return super(automountkey_del, self).execute(*keys, **options)
 
 api.register(automountkey_del)
 
@@ -638,6 +747,31 @@ class automountkey_mod(LDAPUpdate):
     """
     Modify automount key.
     """
+    takes_options = LDAPUpdate.takes_options + (
+        IA5Str('newautomountinformation',
+               cli_name='newinfo',
+               label=_('New mount information'),
+        ),
+    )
+
+    def get_args(self):
+        for key in self.obj.get_ancestor_primary_keys():
+            yield key
+
+    def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        entry_attrs['automountinformation'] = options['newautomountinformation']
+        entry_attrs['description'] = self.obj.get_pk(
+                                            options['automountkey'],
+                                            options['newautomountinformation'])
+        return dn
+
+    def execute(self, *keys, **options):
+        keys += (self.obj.get_pk(options['automountkey'],
+                                 options['automountinformation']), )
+        options[self.obj.primary_key.name] = self.obj.get_pk(
+                                            options['automountkey'],
+                                            options['automountinformation'])
+        return super(automountkey_mod, self).execute(*keys, **options)
 
 api.register(automountkey_mod)
 
@@ -654,5 +788,28 @@ class automountkey_show(LDAPRetrieve):
     """
     Display automount key.
     """
+    takes_options = LDAPRetrieve.takes_options + (
+        IA5Str('automountkey',
+               cli_name='key',
+               label=_('Key'),
+               doc=_('Automount key name'),
+        ),
+        IA5Str('automountinformation',
+               cli_name='info',
+               label=_('Mount information'),
+        ),
+    )
+
+    def get_args(self):
+        for key in self.obj.get_ancestor_primary_keys():
+            yield key
+
+    def execute(self, *keys, **options):
+        keys += (self.obj.get_pk(options['automountkey'],
+                                 options['automountinformation']), )
+        options[self.obj.primary_key.name] = self.obj.get_pk(
+                                            options['automountkey'],
+                                            options['automountinformation'])
+        return super(automountkey_show, self).execute(*keys, **options)
 
 api.register(automountkey_show)
diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py
index 92c695d..c2fa15a 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -37,6 +37,7 @@ class test_automount(XMLRPC_test):
     keyname2 = u'testkey2'
     description = u'description of map'
     info = u'ro'
+    newinfo = u'rw'
     map_kw = {'automountmapname': mapname, 'description': description, 'raw': True}
     key_kw = {'automountkey': keyname, 'automountinformation': info, 'raw': True}
     key_kw2 = {'automountkey': keyname2, 'automountinformation': info, 'raw': True}
@@ -105,7 +106,7 @@ class test_automount(XMLRPC_test):
         """
         Test the `xmlrpc.automountkey_show` method.
         """
-        showkey_kw={'automountkey': self.keyname, 'raw': True}
+        showkey_kw={'automountkey': self.keyname, 'automountinformation' : self.info, 'raw': True}
         res = api.Command['automountkey_show'](self.locname, self.mapname, **showkey_kw)['result']
         assert res
         assert_attr_equal(res, 'automountkey', self.keyname)
@@ -125,12 +126,10 @@ class test_automount(XMLRPC_test):
         """
         Test the `xmlrpc.automountkey_mod` method.
         """
-        self.key_kw['automountinformation'] = u'rw'
-        self.key_kw['description'] = u'new description'
+        self.key_kw['newautomountinformation'] = self.newinfo
         res = api.Command['automountkey_mod'](self.locname, self.mapname, **self.key_kw)['result']
         assert res
         assert_attr_equal(res, 'automountinformation', 'rw')
-        assert_attr_equal(res, 'description', 'new description')
 
     def test_a_automountmap_mod(self):
         """
@@ -145,7 +144,7 @@ class test_automount(XMLRPC_test):
         """
         Test the `xmlrpc.automountkey_del` method.
         """
-        delkey_kw={'automountkey': self.keyname, 'raw': True}
+        delkey_kw={'automountkey': self.keyname, 'automountinformation' : self.newinfo, 'raw': True}
         res = api.Command['automountkey_del'](self.locname, self.mapname, **delkey_kw)['result']
         assert res
         assert_attr_equal(res, 'failed', '')
@@ -179,7 +178,7 @@ class test_automount(XMLRPC_test):
         Test that the `xmlrpc.automountlocation_del` method removes all maps and keys
         """
         # Verify that the second key we added is gone
-        key_kw = {'automountkey': self.keyname2, 'raw': True}
+        key_kw = {'automountkey': self.keyname2, 'automountinformation': self.info, 'raw': True}
         try:
             api.Command['automountkey_show'](self.locname, self.mapname, **key_kw)
         except errors.NotFound:
@@ -187,6 +186,57 @@ class test_automount(XMLRPC_test):
         else:
             assert False
 
+class test_automount_direct(XMLRPC_test):
+    """
+    Test the `automount` plugin indirect map functionality.
+    """
+    locname = u'testlocation'
+    mapname = u'auto.direct2'
+    keyname = u'/-'
+    direct_kw = { 'key' : keyname }
+
+    def test_0_automountlocation_add(self):
+        """
+        Test adding a location.
+        """
+        res = api.Command['automountlocation_add'](self.locname, raw=True)['result']
+        assert res
+        assert_attr_equal(res, 'cn', self.locname)
+
+    def test_1_automountmap_add_direct(self):
+        """
+        Test adding a second direct map with a different info
+        """
+        res = api.Command['automountmap_add_indirect'](self.locname, self.mapname, **self.direct_kw)['result']
+        assert res
+        assert_attr_equal(res, 'automountmapname', self.mapname)
+
+    def test_2_automountmap_add_duplicate(self):
+        """
+        Test adding a duplicate direct map.
+        """
+        try:
+            res = api.Command['automountmap_add_indirect'](self.locname, self.mapname, **self.direct_kw)['result']
+        except errors.DuplicateEntry:
+            pass
+        else:
+            assert False
+
+    def test_3_automountlocation_del(self):
+        """
+        Remove the location.
+        """
+        res = api.Command['automountlocation_del'](self.locname)['result']
+        assert res
+        assert_attr_equal(res, 'failed', '')
+
+        # Verity that it is gone
+        try:
+            api.Command['automountlocation_show'](self.locname)
+        except errors.NotFound:
+            pass
+        else:
+            assert False
 
 class test_automount_indirect(XMLRPC_test):
     """
@@ -196,8 +246,9 @@ class test_automount_indirect(XMLRPC_test):
     mapname = u'auto.home'
     keyname = u'/home'
     parentmap = u'auto.master'
-    description = u'Home directories'
-    map_kw = {'key': keyname, 'parentmap': parentmap, 'description': description, 'raw': True}
+    info = u'somehost:/homes'
+    map_kw = {'key': keyname, 'parentmap': parentmap, 'info': info, 'raw': True}
+    key_kw = {'automountkey': keyname, 'automountinformation': mapname}
 
     def test_0_automountlocation_add(self):
         """
@@ -219,22 +270,21 @@ class test_automount_indirect(XMLRPC_test):
         """
         Test the `xmlrpc.automountmap_show` method.
         """
-        res = api.Command['automountkey_show'](self.locname, self.parentmap, self.keyname, raw=True)['result']
+        res = api.Command['automountmap_show'](self.locname, self.mapname, raw=True)['result']
         assert res
-        assert_attr_equal(res, 'automountkey', self.keyname)
+        assert_attr_equal(res, 'automountmapname', self.mapname)
 
     def test_3_automountkey_del(self):
         """
         Remove the indirect key /home.
         """
-        delkey_kw = {'automountkey': self.keyname}
-        res = api.Command['automountkey_del'](self.locname, self.parentmap, **delkey_kw)['result']
+        res = api.Command['automountkey_del'](self.locname, self.parentmap, **self.key_kw)['result']
         assert res
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
         try:
-            api.Command['automountkey_show'](self.locname, self.parentmap, **delkey_kw)
+            api.Command['automountkey_show'](self.locname, self.parentmap, **self.key_kw)
         except errors.NotFound:
             pass
         else:
@@ -281,8 +331,7 @@ class test_automount_indirect_no_parent(XMLRPC_test):
     mapname = u'auto.home'
     keyname = u'/home'
     parentmap = u'auto.master'
-    description = u'Home directories'
-    map_kw = {'key': keyname, 'description': description, 'raw': True}
+    map_kw = {'key': keyname, 'raw': True}
 
     def test_0_automountlocation_add(self):
         """
@@ -304,7 +353,7 @@ class test_automount_indirect_no_parent(XMLRPC_test):
         """
         Test the `xmlrpc.automountkey_show` method with default parent.
         """
-        showkey_kw = {'automountkey': self.keyname, 'raw': True}
+        showkey_kw = {'automountkey': self.keyname, 'automountinformation': self.mapname, 'raw': True}
         res = api.Command['automountkey_show'](self.locname, self.parentmap, **showkey_kw)['result']
         assert res
         assert_attr_equal(res, 'automountkey', self.keyname)
@@ -313,7 +362,7 @@ class test_automount_indirect_no_parent(XMLRPC_test):
         """
         Remove the indirect key /home.
         """
-        delkey_kw={'automountkey': self.keyname}
+        delkey_kw={'automountkey': self.keyname, 'automountinformation': self.mapname}
         res = api.Command['automountkey_del'](self.locname, self.parentmap, **delkey_kw)['result']
         assert res
         assert_attr_equal(res, 'failed', '')
-- 
1.7.3.4

Attachment: jhrozek-freeipa-029-03-automount-keys-uniqueness.patch.sig
Description: PGP signature

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

Reply via email to