[Freeipa-devel] [PATCH] 0211-0212 Make sure --raw option works for trust-add

[Jan Cholasta]

On 19.7.2016 11:32, Alexander Bokovoy wrote:
> On Tue, 19 Jul 2016, Jan Cholasta wrote:
>> On 18.7.2016 12:06, Martin Babinsky wrote:
>>> 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 think it was just because with --raw, the raw LDAP entry should be
>> returned, letter case and all. I don't really care if the names are
>> lowercased or not, but you should never assume anything about raw
>> results in your code anyway, so I think the revert would be pointless.
>> There were a few bugs with --raw unrelated to letter case in the past
>> and most of the offending code was fixed, the same should be done here
>> IMO.
> This is not about LDAP entry, this is about Python dictionary from
> entry_to_dict(). LDAP attribute name is case-insensitive.
>
> We call entry_to_dict() in multiple places and we expect that
> dictionary keys are lowcased in Python code. I don't think it is fair to
> have this assumption broken when --raw is used -- after all, we are
> dealing with Python dictionary and LDAP attributes are case insensitive.

We don't expect anything about raw results anywhere in our code base, 
except for buggy code, which needs be fixed anyway.

Martin's patch fixes the issue without any intrusive changes, so ACK. We 
can talk bigger changes later for 4.5.

Pushed to master: 2234a774416309a44aecb84f27e6cf4c6a1a990f

-- 
Jan Cholasta