On 07/16/2016 12:50 PM, Alexander Bokovoy wrote:
Hi,


I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)

Note that this fix (patch 0211) has potential for a break but also
introduces a correct behavior in my view as we should not really have
non-lower cased keys in LDAP dictionaries in entry_to_dict() for both
normal and --raw modes.

----- 0211

Since commit a8dd7aa337f25abd938a582d0fcba51d3b356410 if IPA command
is called with --raw option, a retrieved LDAP entry's attribute
names aren't normalized to lower case when converting the entry
to a dictionary. This breaks overall assumption that dictionary
keys are in lower case.

Partially fixes 'ipa trust-add --raw' issues.

https://fedorahosted.org/freeipa/ticket/6059

----- 0212

Make sure we display raw values for 'trust-add --raw' case.

https://fedorahosted.org/freeipa/ticket/6059





Hi Alexander,

I am CC'ing Jan since I hope he knows why a8dd7aa337f25abd938a582d0fcba51d3b356410 was implemented in this way. I think there was a reason behind this decision and we should not revert it without further discussion.

I had the fix for trust-add ready on Friday but was unable to test it properly because of some issues with my VMs. I am attaching it for reference, since it is similar to your patch 212 but relies on attrs_list passed in to LDAP search in order to fetch lowercased attributes.

--
Martin^3 Babinsky
From c3c197822364c01d62626acdc3d69274a14ca291 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 15 Jul 2016 12:38:00 +0200
Subject: [PATCH] trust-add: handle `--all/--raw` options properly

`trust-add` command did not handle these options correctly often resulting in
internal errors or mangled output. This patch implements a behavior which is
more in-line with the rest of the API commands.

https://fedorahosted.org/freeipa/ticket/6059
---
 ipaserver/plugins/trust.py | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py
index 8536202b9b785507bd27b3c7b1896b721f8c5927..18a7cfd3447143393fb087f840edb406e98285a4 100644
--- a/ipaserver/plugins/trust.py
+++ b/ipaserver/plugins/trust.py
@@ -710,6 +710,25 @@ sides.
     msg_summary = _('Added Active Directory trust for realm "%(value)s"')
     msg_summary_existing = _('Re-established trust to domain "%(value)s"')
 
+    def _format_trust_attrs(self, result, **options):
+
+        # Format the output into human-readable values
+        attributes = int(result['result'].get('ipanttrustattributes', [0])[0])
+
+        if not options.get('raw', False):
+            result['result']['trusttype'] = [trust_type_string(
+                result['result']['ipanttrusttype'][0], attributes)]
+            result['result']['trustdirection'] = [trust_direction_string(
+                result['result']['ipanttrustdirection'][0])]
+            result['result']['truststatus'] = [trust_status_string(
+                result['verified'])]
+
+        if attributes:
+            result['result'].pop('ipanttrustattributes', None)
+
+        result['result'].pop('ipanttrustauthoutgoing', None)
+        result['result'].pop('ipanttrustauthincoming', None)
+
     def execute(self, *keys, **options):
         ldap = self.obj.backend
 
@@ -729,10 +748,15 @@ sides.
         else:
             created_range_type = old_range['result']['iparangetype'][0]
 
+        attrs_list = self.obj.default_attributes
+        if options.get('all', False):
+            attrs_list.append('*')
+
         trust_filter = "cn=%s" % result['value']
         (trusts, truncated) = ldap.find_entries(
                          base_dn=DN(self.api.env.container_trusts, self.api.env.basedn),
-                         filter=trust_filter)
+                         filter=trust_filter,
+                         attrs_list=attrs_list)
 
         result['result'] = entry_to_dict(trusts[0], **options)
 
@@ -761,20 +785,9 @@ sides.
                 # add_new_domains_from_trust() on its own.
                 fetch_trusted_domains_over_dbus(self.api, self.log, result['value'])
 
-        # Format the output into human-readable values
-        attributes = int(result['result'].get('ipanttrustattributes', [0])[0])
-        result['result']['trusttype'] = [trust_type_string(
-            result['result']['ipanttrusttype'][0], attributes)]
-        result['result']['trustdirection'] = [trust_direction_string(
-            result['result']['ipanttrustdirection'][0])]
-        result['result']['truststatus'] = [trust_status_string(
-            result['verified'])]
-        if attributes:
-            result['result'].pop('ipanttrustattributes', None)
-
+        # Format the output into human-readable values unless `--raw` is given
+        self._format_trust_attrs(result, **options)
         del result['verified']
-        result['result'].pop('ipanttrustauthoutgoing', None)
-        result['result'].pop('ipanttrustauthincoming', None)
 
         return result
 
-- 
2.7.4

-- 
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