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

Petr Viktorin pviktori at redhat.com
Fri Mar 16 14:18:04 UTC 2012


On 03/15/2012 09:24 PM, Rob Crittenden wrote:
> 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.

Yes. Patch 17 made *attr honor no_create and no_update, which you said 
is not desired behavior.

> The patch doesn't apply because of an encoding change Martin made recently.
>
> It does seem to do the right thing though.
>
> rob

Attaching rebased patch.
This deletes Martin's change, but unless I tested wrong, his bug 
(https://fedorahosted.org/freeipa/ticket/2418) stays fixed. The tests in 
my patch should apply to that ticket as well.

In another fork of this thread, there's discussion if this approach is 
good at all. Maybe we're overengineering a corner case here.

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0016-04-Defer-conversion-and-validation-until-after-add-del-.patch
Type: text/x-patch
Size: 7603 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120316/ffcbbb4b/attachment.bin>


More information about the Freeipa-devel mailing list