On Thu, 2012-01-26 at 00:07 -0600, Endi Sukma Dewata wrote:
> On 12/13/2011 8:13 AM, Martin Kosek wrote:
> >>>> Host object has a virtual attribute "managing" containing all hosts
> >>>> it manages (governed by managedBy attribute). This patch also adds
> >>>> standard membership filtering options:
> >>>> --man-hosts=HOSTS: Only hosts managing _all_ HOSTS are returned
> >>>> --not-man-hosts=HOSTS: Only hosts which do not manage _any_ host
> >>>> in HOSTS are returned
> >>>>
> >>>> https://fedorahosted.org/freeipa/ticket/1675
> >>>
> >>> ACK, it works for me. I'll update the UI to use the new option.
> >>
> >> I take that back. It's NACK.
> >> Patch #178 causes the dnszone-find --forward-only to return the reverse
> >> zone.
> >
> > Good catch, thanks. Fixed.
> 
> Sorry for the delay. I applied 178-2 and rebased 179-2 into 179-3 (patch 
> attached). It works except for one issue, when the host has no 
> managedby, calling host-find with either of these options will generate 
> an internal error.
> 
> # ipa host-add test.example.com --force
> -----------------------------
> Added host "test.example.com"
> -----------------------------
>    Host name: test.example.com
>    Principal name: host/test.example....@example.com
>    Password: False
>    Keytab: False
>    Managed by: test.example.com
> 
> # ipa host-remove-managedby test.example.com --hosts=test.example.com
>    Host name: test.example.com
>    Principal name: host/test.example....@example.com
> ---------------------------
> Number of members removed 1
> ---------------------------
> 
> # ipa host-find --man-hosts=test.example.com
> ipa: ERROR: an internal error has occurred
> 
> The above command should have returned no hosts.
> 
> # ipa host-find --not-man-hosts=test.example.com
> ipa: ERROR: an internal error has occurred
> 
> The above command should have returned all hosts.
> 

Thanks for the review Endi and a good catch. I rebased the patches and
fixed this issue.

Martin
>From 363107d725ec581fa24e518d1786c13125e432b5 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 8 Dec 2011 17:34:26 +0100
Subject: [PATCH 1/2] Fix ldap2 combine_filters for ldap2.MATCH_NONE

"!" is a unary LDAP filter operator and cannot be treated in the
same way as binary operators ("&", "|"). Otherwise, an invalid
LDAP filter is created.

https://fedorahosted.org/freeipa/ticket/1675
---
 ipaserver/plugins/ldap2.py |   24 ++++++++++--------------
 1 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 0698034591328d9fef60cc83f7d09a1c79f675ce..dbe6084f02f5e43d165d9b609f2642f1f8e6ffe1 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -575,6 +575,10 @@ class ldap2(CrudBackend, Encoder):
         """
         assert isinstance(filters, (list, tuple))
         filters = [f for f in filters if f]
+        if filters and rules == self.MATCH_NONE: # unary operator
+            return '(%s%s)' % (self.MATCH_NONE,
+                               self.combine_filters(filters, self.MATCH_ANY))
+
         if len(filters) > 1:
             flt = '(%s' % rules
         else:
@@ -603,19 +607,10 @@ class ldap2(CrudBackend, Encoder):
                                       False - forbid trailing filter wildcard when exact=False
         """
         if isinstance(value, (list, tuple)):
-            flts = []
-            if rules == self.MATCH_NONE:
-                for v in value:
-                    flts.append(
-                        self.make_filter_from_attr(attr, v, exact=exact,
+            make_filter_rules = self.MATCH_ANY if rules == self.MATCH_NONE else rules
+            flts = [ self.make_filter_from_attr(attr, v, exact=exact,
                             leading_wildcard=leading_wildcard,
-                            trailing_wildcard=trailing_wildcard)
-                    )
-                return '(!%s)' % self.combine_filters(flts)
-            for v in value:
-                flts.append(self.make_filter_from_attr(attr, v, rules, exact,
-                            leading_wildcard=leading_wildcard,
-                            trailing_wildcard=trailing_wildcard))
+                            trailing_wildcard=trailing_wildcard) for v in value ]
             return self.combine_filters(flts, rules)
         elif value is not None:
             value = _ldap_filter.escape_filter_chars(value)
@@ -651,11 +646,12 @@ class ldap2(CrudBackend, Encoder):
         ldap2.MATCH_ALL - match entries that match all attributes
         ldap2.MATCH_ANY - match entries that match any of attribute
         """
+        make_filter_rules = self.MATCH_ANY if rules == self.MATCH_NONE else rules
         flts = []
         if attrs_list is None:
             for (k, v) in entry_attrs.iteritems():
                 flts.append(
-                    self.make_filter_from_attr(k, v, rules, exact,
+                    self.make_filter_from_attr(k, v, make_filter_rules, exact,
                         leading_wildcard, trailing_wildcard)
                 )
         else:
@@ -663,7 +659,7 @@ class ldap2(CrudBackend, Encoder):
                 value = entry_attrs.get(a, None)
                 if value is not None:
                     flts.append(
-                        self.make_filter_from_attr(a, value, rules, exact,
+                        self.make_filter_from_attr(a, value, make_filter_rules, exact,
                             leading_wildcard, trailing_wildcard)
                     )
         return self.combine_filters(flts, rules)
-- 
1.7.7.5

>From 113b48eae4191f8ce5f64161ac10fd6bbb8eab2e Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 26 Jan 2012 13:41:39 +0100
Subject: [PATCH 2/2] Add missing managing hosts filtering options

Host object has a virtual attribute "managing" containing all hosts
it manages (governed by managedBy attribute). This patch also adds
standard membership filtering options:
  --man-hosts=HOSTS: Only hosts managing _all_ HOSTS are returned
  --not-man-hosts=HOSTS: Only hosts which do not manage _any_ host
    in HOSTS are returned

https://fedorahosted.org/freeipa/ticket/1675
---
 API.txt                               |    4 ++-
 VERSION                               |    2 +-
 ipalib/plugins/baseldap.py            |   50 +++++++++++++++++---------------
 ipalib/plugins/host.py                |   47 +++++++++++++++++++++++++++++++
 tests/test_xmlrpc/test_host_plugin.py |   33 +++++++++++++++++++++
 5 files changed, 111 insertions(+), 25 deletions(-)

diff --git a/API.txt b/API.txt
index 2937c24f4d6aa53b1028f430a05ef6453544ea7c..65e82d3ec719fbee3099e58a996df552e06a5e94 100644
--- a/API.txt
+++ b/API.txt
@@ -1696,7 +1696,7 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('result', <type 'bool'>, None)
 output: Output('value', <type 'unicode'>, None)
 command: host_find
-args: 1,28,4
+args: 1,30,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Str('fqdn', attribute=True, autofill=False, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
@@ -1726,6 +1726,8 @@ option: Str('enroll_by_user*', cli_name='enroll_by_users', csv=True)
 option: Str('not_enroll_by_user*', cli_name='not_enroll_by_users', csv=True)
 option: Str('man_by_host*', cli_name='man_by_hosts', csv=True)
 option: Str('not_man_by_host*', cli_name='not_man_by_hosts', csv=True)
+option: Str('man_host*', cli_name='man_hosts', csv=True)
+option: Str('not_man_host*', cli_name='not_man_hosts', csv=True)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
 output: Output('count', <type 'int'>, None)
diff --git a/VERSION b/VERSION
index ff245b559f67f80f83119d15a154c30185ad0f55..2afbba27df3864db51223e9c7dfa9b185a466853 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=21
+IPA_API_VERSION_MINOR=22
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 3d6480458a852ed43547384072def0d9ecca7e9d..f59a0d4106729573cfdbd8bdf2c407619eb051d5 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1595,6 +1595,31 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
         for arg in super(crud.Search, self).get_args():
             yield arg
 
+    def get_member_options(self, attr):
+        for ldap_obj_name in self.obj.attribute_members[attr]:
+            ldap_obj = self.api.Object[ldap_obj_name]
+            relationship = self.obj.relationships.get(
+                attr, ['member', '', 'no_']
+            )
+            doc = self.member_param_incl_doc % (
+                self.obj.object_name_plural, relationship[0].lower(),
+                ldap_obj.object_name_plural
+            )
+            name = '%s%s' % (relationship[1], to_cli(ldap_obj_name))
+            yield Str(
+                '%s*' % name, cli_name='%ss' % name, doc=doc,
+                label=ldap_obj.object_name, csv=True
+            )
+            doc = self.member_param_excl_doc % (
+                self.obj.object_name_plural, relationship[0].lower(),
+                ldap_obj.object_name_plural
+            )
+            name = '%s%s' % (relationship[2], to_cli(ldap_obj_name))
+            yield Str(
+                '%s*' % name, cli_name='%ss' % name, doc=doc,
+                label=ldap_obj.object_name, csv=True
+            )
+
     def get_options(self):
         for option in super(LDAPSearch, self).get_options():
             yield option
@@ -1602,29 +1627,8 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
                 'no_output' not in self.obj.primary_key.flags:
             yield gen_pkey_only_option(self.obj.primary_key.cli_name)
         for attr in self.member_attributes:
-            for ldap_obj_name in self.obj.attribute_members[attr]:
-                ldap_obj = self.api.Object[ldap_obj_name]
-                relationship = self.obj.relationships.get(
-                    attr, ['member', '', 'no_']
-                )
-                doc = self.member_param_incl_doc % (
-                    self.obj.object_name_plural, relationship[0].lower(),
-                    ldap_obj.object_name_plural
-                )
-                name = '%s%s' % (relationship[1], to_cli(ldap_obj_name))
-                yield Str(
-                    '%s*' % name, cli_name='%ss' % name, doc=doc,
-                    label=ldap_obj.object_name, csv=True
-                )
-                doc = self.member_param_excl_doc % (
-                    self.obj.object_name_plural, relationship[0].lower(),
-                    ldap_obj.object_name_plural
-                )
-                name = '%s%s' % (relationship[2], to_cli(ldap_obj_name))
-                yield Str(
-                    '%s*' % name, cli_name='%ss' % name, doc=doc,
-                    label=ldap_obj.object_name, csv=True
-                )
+            for option in self.get_member_options(attr):
+                yield option
 
     def get_member_filter(self, ldap, **options):
         filter = ''
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index fb5f4ea2416abead430487eb71d3a531c80ad80d..052adbeee00e469d471ea126292d7e26399dd591 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -33,6 +33,7 @@ from ipalib.plugins.dns import dns_container_exists, _record_types
 from ipalib.plugins.dns import add_forward_record
 from ipalib import _, ngettext
 from ipalib import x509
+from ipalib.dn import *
 from ipapython.ipautil import ipa_generate_password, CheckedIPAddress
 from ipalib.request import context
 import base64
@@ -713,10 +714,56 @@ class host_find(LDAPSearch):
     )
     member_attributes = ['memberof', 'enrolledby', 'managedby']
 
+    def get_options(self):
+        for option in super(host_find, self).get_options():
+            yield option
+        # "managing" membership has to be added and processed separately
+        for option in self.get_member_options('managing'):
+            yield option
+
     def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **options):
         if 'locality' in attrs_list:
             attrs_list.remove('locality')
             attrs_list.append('l')
+        if 'man_host' in options or 'not_man_host' in options:
+            hosts = []
+            if options.get('man_host') is not None:
+                for pkey in options.get('man_host', []):
+                    dn = self.obj.get_dn(pkey)
+                    try:
+                        (dn, entry_attrs) = ldap.get_entry(dn, ['managedby'])
+                    except errors.NotFound:
+                        self.obj.handle_not_found(pkey)
+                    hosts.append(set(entry_attrs.get('managedby', '')))
+                hosts = list(reduce(lambda s1, s2: s1 & s2, hosts))
+
+                if not hosts:
+                    # There is no host managing _all_ hosts in --man-hosts
+                    filter = ldap.combine_filters(
+                        (filter, '(objectclass=disabled)'), ldap.MATCH_ALL
+                    )
+
+            not_hosts = []
+            if options.get('not_man_host') is not None:
+                for pkey in options.get('not_man_host', []):
+                    dn = self.obj.get_dn(pkey)
+                    try:
+                        (dn, entry_attrs) = ldap.get_entry(dn, ['managedby'])
+                    except errors.NotFound:
+                        self.obj.handle_not_found(pkey)
+                    not_hosts += entry_attrs.get('managedby', [])
+                not_hosts = list(set(not_hosts))
+
+            for target_hosts, filter_op in ((hosts, ldap.MATCH_ANY),
+                                            (not_hosts, ldap.MATCH_NONE)):
+                hosts_avas = [DN(host)[0][0] for host in target_hosts]
+                hosts_filters = [ldap.make_filter_from_attr(ava.attr, ava.value) for ava in hosts_avas]
+                hosts_filter = ldap.combine_filters(hosts_filters, filter_op)
+
+                filter = ldap.combine_filters(
+                        (filter, hosts_filter), ldap.MATCH_ALL
+                    )
+
         return (filter.replace('locality', 'l'), base_dn, scope)
 
     def post_callback(self, ldap, entries, truncated, *args, **options):
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 5e71370600a31727d5d3907d465c14370489f7d8..c0f9b8959b7edf9b7b0c72c77b28cfad52ad736c 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -391,6 +391,39 @@ class test_host(Declarative):
         ),
 
         dict(
+            desc='Search for hosts with --man-hosts and --not-man-hosts',
+            command=('host_find', [], {'man_host' : fqdn3, 'not_man_host' : fqdn1}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 host matched',
+                result=[
+                    dict(
+                        dn=lambda x: DN(x) == dn3,
+                        fqdn=[fqdn3],
+                        description=[u'Test host 2'],
+                        l=[u'Undisclosed location 2'],
+                        krbprincipalname=[u'host/%s@%s' % (fqdn3, api.env.realm)],
+                        has_keytab=False,
+                        has_password=False,
+                        managedby_host=[u'%s' % fqdn3, u'%s' % fqdn1],
+                    ),
+                ],
+            ),
+        ),
+
+        dict(
+            desc='Try to search for hosts with --man-hosts',
+            command=('host_find', [], {'man_host' : [fqdn3,fqdn4]}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 hosts matched',
+                result=[],
+            ),
+        ),
+
+        dict(
             desc='Remove managedby_host %r from %r' % (fqdn1, fqdn3),
             command=('host_remove_managedby', [fqdn3],
                 dict(
-- 
1.7.7.5

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

Reply via email to