On 31.05.2016 13:04, Martin Babinsky wrote:
On 05/26/2016 04:52 PM, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/5915

Patches attached



ACK.

Even better patches attached.


From 1918df3017504354834c9175faf8d09108feb07a Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Fri, 13 May 2016 18:39:47 +0200
Subject: [PATCH 1/2] DNS Locations: prevent to remove used locations

User should be notified that location is used by IPA server(s) and
deletion should be aborted without --force option.

Referint plugin is configured to remove references of deleted locations.

https://fedorahosted.org/freeipa/ticket/2008
---
 API.txt                            |  3 ++-
 VERSION                            |  4 ++--
 install/updates/25-referint.update |  1 +
 ipalib/plugins/location.py         | 27 ++++++++++++++++++++++++++-
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/API.txt b/API.txt
index 608f1083faf2b03214432c134f403c40ccdb6700..10a2d9f076391f1a9133cf0899dd07d81043b0a3 100644
--- a/API.txt
+++ b/API.txt
@@ -2772,9 +2772,10 @@ output: Entry('result')
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
 output: PrimaryKey('value')
 command: location_del
-args: 1,2,3
+args: 1,3,3
 arg: DNSNameParam('idnsname+', cli_name='name')
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Flag('force', autofill=True, default=False)
 option: Str('version?')
 output: Output('result', type=[<type 'dict'>])
 output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>])
diff --git a/VERSION b/VERSION
index 93ea86438d56f6206b28decbf95d2e7ccd57f8ed..90b240383a95bdc1783de99f416df1551641bfba 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=172
-# Last change: mbasti - location-show: list servers in the location
+IPA_API_VERSION_MINOR=173
+# Last change: mbasti - server-del: prevent to remove used locations
diff --git a/install/updates/25-referint.update b/install/updates/25-referint.update
index 3f78ee9755823fb3d5838d3069f4506c57a69d05..b887ede9c98f100709d24aae26b75d501f581016 100644
--- a/install/updates/25-referint.update
+++ b/install/updates/25-referint.update
@@ -19,3 +19,4 @@ add: referint-membership-attr: ipaassignedidview
 add: referint-membership-attr: ipaallowedtarget
 add: referint-membership-attr: ipamemberca
 add: referint-membership-attr: ipamembercertprofile
+add: referint-membership-attr: ipalocation
diff --git a/ipalib/plugins/location.py b/ipalib/plugins/location.py
index 6d876d51204f9957a091c12d89414fead0fc95c6..c38ff580964f314675dc0743a52dca27638a1f06 100644
--- a/ipalib/plugins/location.py
+++ b/ipalib/plugins/location.py
@@ -9,8 +9,10 @@ from ipalib import (
     ngettext,
     api,
     Str,
-    DNSNameParam
+    DNSNameParam,
+    Flag,
 )
+from ipalib.errors import DependentEntry
 from ipalib.plugable import Registry
 from ipalib.plugins.baseldap import (
     LDAPCreate,
@@ -134,6 +136,29 @@ class location_del(LDAPDelete):
 
     msg_summary = _('Deleted IPA location "%(value)s"')
 
+    takes_options = LDAPDelete.takes_options + (
+        Flag(
+            'force',
+            label=_('Force'),
+            doc=_('force location removal'),
+        ),
+    )
+
+    def pre_callback(self, ldap, dn, *keys, **options):
+        assert isinstance(dn, DN)
+        if not options.get('force'):
+            servers = self.api.Command.server_find(
+                in_location=keys[-1])['result']
+            location_members = u', '.join(
+                server['cn'][0] for server in servers)
+            if location_members:
+                raise DependentEntry(
+                    label=_('IPA Server(s)'),
+                    key=keys[-1],
+                    dependent=location_members
+                )
+        return dn
+
 
 @register()
 class location_mod(LDAPUpdate):
-- 
2.5.5

From bdd9280996de8d28dfb8b74fe07dd597eeb13bc5 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Tue, 17 May 2016 13:08:59 +0200
Subject: [PATCH 2/2] DNS Locations: extend tests with server-* commands

https://fedorahosted.org/freeipa/ticket/2008
---
 ipatests/test_xmlrpc/test_location_plugin.py    | 101 +++++++++++++++++++++-
 ipatests/test_xmlrpc/tracker/location_plugin.py |  29 +++++--
 ipatests/test_xmlrpc/tracker/server_plugin.py   | 107 ++++++++++++++++++++++++
 3 files changed, 230 insertions(+), 7 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/server_plugin.py

diff --git a/ipatests/test_xmlrpc/test_location_plugin.py b/ipatests/test_xmlrpc/test_location_plugin.py
index 1ca3eac7c72e0662034cb67039e1d0925bd1acca..a5d0797958b62e72714ac0f3e1e8c50dd5836d6b 100644
--- a/ipatests/test_xmlrpc/test_location_plugin.py
+++ b/ipatests/test_xmlrpc/test_location_plugin.py
@@ -5,12 +5,14 @@ from __future__ import absolute_import
 
 import pytest
 
-from ipalib import errors
+from ipalib import errors, api
 from ipatests.test_xmlrpc.tracker.location_plugin import LocationTracker
+from ipatests.test_xmlrpc.tracker.server_plugin import ServerTracker
 from ipatests.test_xmlrpc.xmlrpc_test import (
     XMLRPC_test,
     raises_exact,
 )
+from ipapython.dnsutil import DNSName
 
 
 @pytest.fixture(scope='class', params=[u'location1', u'sk\xfa\u0161ka.idna'])
@@ -31,6 +33,12 @@ def location_absolute(request):
     return tracker.make_fixture(request)
 
 
+@pytest.fixture(scope='class')
+def server(request):
+    tracker = ServerTracker(api.env.host)
+    return tracker
+
+
 @pytest.mark.tier1
 class TestNonexistentIPALocation(XMLRPC_test):
     def test_retrieve_nonexistent(self, location):
@@ -111,3 +119,94 @@ class TestCRUD(XMLRPC_test):
 
     def test_delete_location(self, location):
         location.delete()
+
+
+@pytest.mark.tier1
+class TestLocationsServer(XMLRPC_test):
+
+    def test_add_nonexistent_location_to_server(self, server):
+        nonexistent_loc = DNSName(u'nonexistent-location')
+        command = server.make_update_command(
+            updates=dict(
+                ipalocation_location=nonexistent_loc,
+            )
+        )
+        with raises_exact(errors.NotFound(
+                reason=u"{location}: location not found".format(
+                    location=nonexistent_loc
+                ))):
+            command()
+
+    def test_add_location_to_server(self, location, server):
+        location.ensure_exists()
+        server.update(
+            dict(ipalocation_location=location.idnsname_obj),
+            expected_updates=dict(
+                ipalocation_location=[location.idnsname_obj],
+            )
+        )
+        location.add_server_to_location(server.server_name)
+        location.retrieve()
+
+    def test_retrieve(self, server):
+        server.retrieve()
+
+    def test_retrieve_all(self, server):
+        server.retrieve(all=True)
+
+    def test_search_server_with_location(self, location, server):
+        command = server.make_find_command(
+            server.server_name, in_location=location.idnsname_obj)
+        result = command()
+        server.check_find(result)
+
+    def test_search_server_without_location(self, location, server):
+        command = server.make_find_command(
+            server.server_name, not_in_location=location.idnsname_obj)
+        result = command()
+        server.check_find_nomatch(result)
+
+    def test_add_location_to_server_custom_weight(self, location, server):
+        location.ensure_exists()
+        server.update(
+            dict(
+                ipalocation_location=location.idnsname_obj,
+                ipalocationweight=200,
+            ),
+            expected_updates=dict(
+                ipalocation_location=[location.idnsname_obj],
+                ipalocationweight=[u'200'],
+            )
+        )
+        # remove invalid data from the previous test
+        location.remove_server_from_location(server.server_name)
+
+        location.add_server_to_location(server.server_name, weight=200)
+        location.retrieve()
+
+    def test_remove_location_from_server(self, location, server):
+        server.update(dict(ipalocation_location=None))
+        location.remove_server_from_location(server.server_name, weight=200)
+        location.retrieve()
+
+    def test_remove_location_weight_from_server(self, location, server):
+        server.update(dict(ipalocationweight=None))
+        location.retrieve()
+
+
+@pytest.mark.tier1
+class TestReferintLocation(XMLRPC_test):
+    def test_location_referint(self, location, server):
+        location.ensure_exists()
+        server.update(
+            dict(ipalocation_location=location.idnsname_obj),
+            expected_updates=dict(
+                ipalocation_location=[location.idnsname_obj],
+            )
+        )
+        command = location.make_delete_command(force=True)
+        command()
+        location.track_delete()
+        # here referint plugins should remove location from servers
+        del server.attrs['ipalocation_location']
+        server.retrieve()
diff --git a/ipatests/test_xmlrpc/tracker/location_plugin.py b/ipatests/test_xmlrpc/tracker/location_plugin.py
index c7af09768d0b4e7258ec78e4be533c72bf32e35f..7a5e369d8b47e3cdbb12da2a111dc721ec0f4ac2 100644
--- a/ipatests/test_xmlrpc/tracker/location_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/location_plugin.py
@@ -11,11 +11,11 @@ from ipatests.test_xmlrpc.tracker.base import Tracker
 
 class LocationTracker(Tracker):
     """Tracker for IPA Location tests"""
-    retrieve_keys = {'idnsname', 'description', 'dn'}
+    retrieve_keys = {'idnsname', 'description', 'dn', 'servers'}
     retrieve_all_keys = retrieve_keys | {'objectclass'}
-    create_keys = retrieve_keys | {'objectclass'}
-    find_keys = retrieve_keys
-    find_all_keys = retrieve_all_keys
+    create_keys = {'idnsname', 'description', 'dn', 'objectclass'}
+    find_keys = {'idnsname', 'description', 'dn',}
+    find_all_keys = find_keys | {'objectclass'}
     update_keys = {'idnsname', 'description'}
 
     def __init__(self, name, description=u"Location description"):
@@ -40,9 +40,9 @@ class LocationTracker(Tracker):
             'location_add', self.idnsname, description=self.description,
         )
 
-    def make_delete_command(self):
+    def make_delete_command(self, force=False):
         """Make function that removes this location using location-del"""
-        return self.make_command('location_del', self.idnsname)
+        return self.make_command('location_del', self.idnsname, force=force)
 
     def make_retrieve_command(self, all=False, raw=False):
         """Make function that retrieves this location using location-show"""
@@ -117,3 +117,20 @@ class LocationTracker(Tracker):
             summary=u'Modified IPA location "{loc}"'.format(loc=self.idnsname),
             result=self.filter_attrs(self.update_keys | set(extra_keys))
         ), result)
+
+    def add_server_to_location(self, server_name, weight=100):
+        servers = self.attrs.setdefault('servers', [])
+        servers.append(u"{server} (weight: {weight})".format(
+            server=server_name, weight=weight))
+
+    def remove_server_from_location(self, server_name, weight=100):
+        if 'servers' not in self.attrs:
+            return
+        servers = self.attrs['servers']
+        try:
+            servers.remove(u"{server} (weight: {weight})".format(
+                server=server_name, weight=weight))
+        except ValueError:
+            pass
+        if not servers:
+            del self.attrs['servers']
diff --git a/ipatests/test_xmlrpc/tracker/server_plugin.py b/ipatests/test_xmlrpc/tracker/server_plugin.py
new file mode 100644
index 0000000000000000000000000000000000000000..90e9ef3a279c68b011d6b353985bf49ce7f85dd7
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/server_plugin.py
@@ -0,0 +1,107 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+from __future__ import absolute_import
+
+from ipapython.dn import DN
+from ipatests.util import assert_deepequal
+from ipatests.test_xmlrpc.tracker.base import Tracker
+
+
+class ServerTracker(Tracker):
+    """Tracker for IPA Location tests"""
+    retrieve_keys = {
+        'cn', 'dn', 'ipamaxdomainlevel', 'ipamindomainlevel',
+        'iparepltopomanagedsuffix_topologysuffix', 'ipalocation_location',
+        'ipalocationweight',
+    }
+    retrieve_all_keys = retrieve_keys | {'objectclass'}
+    create_keys = retrieve_keys | {'objectclass'}
+    find_keys = retrieve_keys
+    find_all_keys = retrieve_all_keys
+    update_keys = {
+        'cn', 'ipamaxdomainlevel', 'ipamindomainlevel',
+        'ipalocation_location', 'ipalocationweight',
+    }
+
+    def __init__(self, name):
+        super(ServerTracker, self).__init__(default_version=None)
+        self.server_name = name
+        self.dn = DN(
+            ('cn', self.server_name),
+            'cn=masters,cn=ipa,cn=etc',
+            self.api.env.basedn
+        )
+        self.exists = True  # we cannot add server manually using server-add
+        self.attrs = dict(
+            dn=self.dn,
+            cn=[self.server_name],
+            iparepltopomanagedsuffix_topologysuffix=[u'domain', u'ca'],
+            objectclass=[
+                u"ipalocationmember",
+                u"ipaReplTopoManagedServer",
+                u"top",
+                u"ipaConfigObject",
+                u"nsContainer",
+                u"ipaSupportedDomainLevelConfig"
+            ],
+            ipamaxdomainlevel=[u"1"],
+            ipamindomainlevel=[u"0"],
+        )
+        self.exists = True
+
+    def make_retrieve_command(self, all=False, raw=False):
+        """Make function that retrieves this server using server-show"""
+        return self.make_command(
+            'server_show', self.name, all=all, raw=raw
+        )
+
+    def make_find_command(self, *args, **kwargs):
+        """Make function that finds servers using server-find"""
+        return self.make_command('server_find', *args, **kwargs)
+
+    def make_update_command(self, updates):
+        """Make function that modifies the server using server-mod"""
+        return self.make_command('server_mod', self.name, **updates)
+
+    def check_retrieve(self, result, all=False, raw=False):
+        """Check `server-show` command result"""
+        if all:
+            expected = self.filter_attrs(self.retrieve_all_keys)
+        else:
+            expected = self.filter_attrs(self.retrieve_keys)
+        assert_deepequal(dict(
+            value=self.server_name,
+            summary=None,
+            result=expected,
+        ), result)
+
+    def check_find(self, result, all=False, raw=False):
+        """Check `server-find` command result"""
+        if all:
+            expected = self.filter_attrs(self.find_all_keys)
+        else:
+            expected = self.filter_attrs(self.find_keys)
+        assert_deepequal(dict(
+            count=1,
+            truncated=False,
+            summary=u'1 IPA server matched',
+            result=[expected],
+        ), result)
+
+    def check_find_nomatch(self, result):
+        """ Check 'server-find' command result when no match is expected """
+        assert_deepequal(dict(
+            count=0,
+            truncated=False,
+            summary=u'0 IPA servers matched',
+            result=[],
+        ), result)
+
+    def check_update(self, result, extra_keys=()):
+        """Check `server-update` command result"""
+        assert_deepequal(dict(
+            value=self.server_name,
+            summary=u'Modified IPA server "{server}"'.format(server=self.name),
+            result=self.filter_attrs(self.update_keys | set(extra_keys))
+        ), result)
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to