[Freeipa-devel] [PATCHES] 0016 Fixes for{add, set, del}attr with managed attributes

[Rob Crittenden]

Petr Viktorin wrote:
> On 02/29/2012 04:34 PM, Petr Viktorin wrote:
>> On 02/29/2012 03:50 PM, Rob Crittenden wrote:
>>> Petr Viktorin wrote:
>>>> On 02/27/2012 11:03 PM, Rob Crittenden wrote:
>>>>> Petr Viktorin wrote:
>>>>>> Patch 16 defers validation & conversion until after
>>>>>> {add,del,set}attr is
>>>>>> processed, so that we don't search for an integer in a list of
>>>>>> strings
>>>>>> (this caused ticket #2405), and so that the end result of these
>>>>>> operations is validated (#2407).
>>>>>>
>>>>>>
>>>>>> Patch 17 makes these options honor params marked no_create and
>>>>>> no_update.
>>>>>>
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/2405
>>>>>> https://fedorahosted.org/freeipa/ticket/2407
>>>>>> https://fedorahosted.org/freeipa/ticket/2408
>>>>>
>>>>> NACK on Patch 17 which breaks patch 16.
>>>>
>>>> How is patch 16 broken? It works for me.
>>>
>>> My point is they rely on one another, IMHO, so without 17 the reported
>>> problem still exists.
>>>
>>>>
>>>>> *attr is specifically made to be powerful. We don't want to
>>>>> arbitrarily
>>>>> block updating certain values.
>>>>
>>>> Noted
>>>>
>>>>> Not having patch 17 means that the problem reported in 2408 still
>>>>> occurs. It should probably check both the schema and the param to
>>>>> determine if something can have multiple values and reject that way.
>>>>
>>>> I see the problem now: the certificate subject base is defined as a
>>>> multi-value attribute in the LDAP schema. If it's changed to
>>>> single-value the existing validation should catch it.
>>>>
>>>> Also, most of the config attributes should probably be required in the
>>>> schema. Am I right?
>>>>
>>>> I'm a newbie here; what do I need to do when changing the schema? Is
>>>> there a patch that does something similar I could use as an example?
>>>>
>>>
>>> The framework should be able to impose its own single-value will as
>>> well. If a Param is designated as single-value the *attr should honor
>>> it.
>>
>> Here is the updated patch.
>> Since *attr is powerful enough to modify 'no_update' Params, which
>> CRUDUpdate forgets about, I need to check the params of the LDAPObject
>> itself.
>>
>>> Updating schema is a bit of a nasty business right now. See
>>> 10-selinuxusermap.update for an example.
>>
>> I'll leave schema changes for after the release, then.
>>
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
> Attached patch includes tests. Note that the test depends on my patches
> 12-13, which make ipasearchrecordslimit required.

I gather that this eliminates the need for patch 17? It seems to work as-is.

The patch doesn't apply because of an encoding change Martin made recently.

It does seem to do the right thing though.

rob