Re: [Freeipa-devel] [PATCH] 178+179 Add missing managing hosts filtering options

2012-01-26 Thread Martin Kosek
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 @@ 

Re: [Freeipa-devel] [PATCH] 178+179 Add missing managing hosts filtering options

2012-01-26 Thread Endi Sukma Dewata

On 1/26/2012 6:48 AM, Martin Kosek wrote:

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.


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


I had to rebase 179 again (patch attached) due to a recent API.txt and 
VERSION change. ACK and pushed to master and ipa-2-2.


--
Endi S. Dewata
From f7ad9baf1390ea4f6178288616828a7b0bfae29c 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 ebf23181ad8f5b8754394a351a4bff29affb8bd7..d87dfc3b2f62a21c3dc2092194b3f4463607d510 100644
--- a/API.txt
+++ b/API.txt
@@ -1697,7 +1697,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,29,4
+args: 1,31,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)
@@ -1728,6 +1728,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 2afbba27df3864db51223e9c7dfa9b185a466853..12316e411e8c97619608a07d217d9adf5e509dbf 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=22
+IPA_API_VERSION_MINOR=23
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 

Re: [Freeipa-devel] [PATCH] 178+179 Add missing managing hosts filtering options

2012-01-25 Thread Endi Sukma Dewata

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.

--
Endi S. Dewata
From 94ab8e3446c88f3d28d6f93c73d22665daa2124d Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 8 Dec 2011 17:39:45 +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|   41 +++
 tests/test_xmlrpc/test_host_plugin.py |   33 +
 5 files changed, 105 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=2010061412
 #  #
 
 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 

Re: [Freeipa-devel] [PATCH] 178+179 Add missing managing hosts filtering options

2011-12-13 Thread Martin Kosek
On Fri, 2011-12-09 at 16:04 -0600, Endi Sukma Dewata wrote:
 On 12/9/2011 3:21 PM, Endi Sukma Dewata wrote:
  On 12/8/2011 10:45 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.

Martin
From 950ee966769f5f3f6906dff01cc94fecf0d50616 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 4bfc849d82cd4384afe9bad7b244184265315f7d..4366396fbb1d0842a010f06467cf0244a28d1466 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.3

From aa8b849f00c0717b6417939691ab7ec070758ef5 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 8 Dec 2011 17:39:45 +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