[Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong

[Tomas Babej]

On 07/01/2014 02:41 PM, Rob Crittenden wrote:
> Tomas Babej wrote:
>> On 07/01/2014 12:19 PM, Martin Kosek wrote:
>>> On 06/26/2014 10:44 AM, Jan Cholasta wrote:
>>>> On 26.6.2014 10:39, Petr Viktorin wrote:
>>>>> On 06/26/2014 10:33 AM, Jan Cholasta wrote:
>>>>>> On 26.6.2014 09:40, Petr Viktorin wrote:
>>>>>>> On 06/26/2014 09:33 AM, Jan Cholasta wrote:
>>>>>>>> On 26.6.2014 09:21, Petr Viktorin wrote:
>>>>>>>>> On 06/26/2014 08:30 AM, Jan Cholasta wrote:
>>>>>>>>>> On 25.6.2014 18:25, Petr Viktorin wrote:
>>>>>>>>>>> On 06/25/2014 05:29 PM, Jan Cholasta wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> On 25.6.2014 17:17, Tomas Babej wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Our datetime conversion does not support full LDAP Generalized
>>>>>>>>>>>>> time syntax. In the unsupported cases, we should fall back
>>>>>>>>>>>>> to string representation of the attribute.
>>>>>>>>>>>>>
>>>>>>>>>>>>> In particular, '0' is used to denote no value of LDAP generalized
>>>>>>>>>>>>> time attribute.
>>>>>>>>>>>>>
>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/4350
>>>>>>>>>>>> NACK, this beats the purpose of decoding of the values, because it
>>>>>>>>>>>> requires you to check the type of the value before using it.
>>>>>>>>>>>>
>>>>>>>>>>>> Instead, you should either fix the code that uses the
>>>>>>>>>>>> nsds5ReplicaLastUpdate{Start,End} attributes to access their raw
>>>>>>>>>>>> value
>>>>>>>>>>>> directly, or exclude the attributes from decoding to datetime by
>>>>>>>>>>>> overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE.
>>>>>>>>>>>>
>>>>>>>>>>>> Honza
>>>>>>>>>>>>
>>>>> [...]
>>>>>>> The problem is that "unset" is a valid value here,
>>>>>> It is not, according to Generalized Time syntax (RFC 4517 section
>>>>>> 3.3.13) "0" is an invalid value and we should treat it the same way as
>>>>>> any other invalid value (hence my original suggestion).
>>>>> OK, in that case ignore what I said here.
>>>>>
>>>>> So am I correct that 389-ds storing a value that doesn't comply with the
>>>>> attribute's syntax?  Should we file a DS bug?
>>>>>
>>>> AFAIK syntax checks are done only on LDAP adds and mods, so unless we are
>>>> setting "0" somewhere and DS does not complain, it is not a bug.
>>> What is the final resolution here? Do we plan to ignore the attribute
>>> conversion for these attributes as, fixing DS or are we fixing the framework?
>>>
>>> Surprisingly, I did not see the filed failure any more in my
>>> ipa-replica-install tests nor in the latest CI run, I wonder if anything
>>> changed in our code or if the issue is intermittent. Anyway, I am willing to
>>> close this ticket if this is no longer a problem.
>>>
>>> Martin
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>> Issue is not 100% reproducible in my testing. The underlying problem
>> here is that sometimes the values of nsds5replicalastupdatestart and
>> nsds5replicalastupdateend attributes are not being set during
>> replication (intermittent issue) and thus 389 falls back and returns '0'
>> if any of these attributes were explicitily requested, which is not a
>> valid value for LDAP Generalized time.
>>
>> The reason you're not hitting the conversion errors is probably that
>> during replication nsds5replicalastupdatestart and
>> nsds5replicalastupdateend were assigned proper values. You can check
>> that in the replication log (search for "Replication Update in progress:
>> %s: status: %s: start: %d: end: %d" in the logs).
>>
>> I am checking with Ludvig, but if this behaviour is specific to the
>> nsds5replicalastupdatestart and nsds5replicalastupdateend I'd go with
>> the SYNTAX_OVERRIDE solution.
>>
>> However, it is worth noting that these conversion errors are harmless
>> themselves.
>>
> IIRC I also saw this with:
>
> ipa-replica-manage list -v `hostname`
>
> That may be easier to reproduce/validate.
>
> rob

Please see my patch 238 on the list, which obsoletes this thread.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org