[Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

[Martin Basti]

On 11/08/15 09:17, Jan Cholasta wrote:
> On 5.8.2015 12:34, thierry bordaz wrote:
>> On 08/05/2015 12:13 PM, Jan Cholasta wrote:
>>> Dne 5.8.2015 v 11:55 thierry bordaz napsal(a):
>>>> On 08/05/2015 11:27 AM, Martin Basti wrote:
>>>>>
>>>>> ----- Original Message -----
>>>>> From: "thierry bordaz" <tbordaz at redhat.com>
>>>>> To: "Jan Cholasta" <jcholast at redhat.com>
>>>>> Cc: freeipa-devel at redhat.com
>>>>> Sent: Monday, August 3, 2015 5:34:02 PM
>>>>> Subject: Re: [Freeipa-devel] Replace stageuser-add --from-delete with
>>>>> user-undel --to-staged
>>>>>
>>>>> On 07/28/2015 12:34 PM, Jan Cholasta wrote:
>>>>>> Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a):
>>>>>>>
>>>>>>> Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):
>>>>>>>> Dne 27.7.2015 v 17:59 Martin Basti napsal(a):
>>>>>>>>> On 23/07/15 14:43, Martin Basti wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> I tried to fix #5145 and I partially succeeded.
>>>>>>>>>>
>>>>>>>>>> However, I cannot fix this part of ticket, where user is
>>>>>>>>>> prompted to
>>>>>>>>>> write name and surname.
>>>>>>>>>>
>>>>>>>>>> $ ipa stageuser-add tuser --from-delete
>>>>>>>>>> First name: this will be ignored
>>>>>>>>>> Last name: this will be also ignored
>>>>>>>>>> ------------------------
>>>>>>>>>> Added stage user "tuser"
>>>>>>>>>> ------------------------
>>>>>>>>>>
>>>>>>>>>> As the first name and last name are mandatory attributes of
>>>>>>>>>> stageuser-add command, but they are not needed by when the
>>>>>>>>>> --from-delete option is used.
>>>>>>>>>> I would like to ask how to fix this issue, IMO this will be huge
>>>>>>>>>> hack
>>>>>>>>>> in internal API. Or should we just document this bug as known
>>>>>>>>>> issue
>>>>>>>>>> (thierry wrote that this is not use case that should be used
>>>>>>>>>> often)?
>>>>>>>>>>
>>>>>>>>>> The best solution would be separate command, but this idea was
>>>>>>>>>> rejected in thread "[Freeipa-devel] User life cycle: question
>>>>>>>>>> regarding the design"
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Martin^2
>>>>>>>>>>
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> as was mentioned before, we have issue with current internal API
>>>>>>>>> and the
>>>>>>>>> stageuser-add --from-delete command.
>>>>>>>>>
>>>>>>>>> We discussed this today, and we did not find a nice way how to 
>>>>>>>>> fix
>>>>>>>>> it,
>>>>>>>>> so we propose this (which is IMO the best solution):
>>>>>>>>>
>>>>>>>>> * stageuser-add --from-delete should be deprecated
>>>>>>>> +1
>>>>>>>>
>>>>>>>>> * create new option for user-undel: used-undel --to-staged  (or
>>>>>>>>> create
>>>>>>>>> new command) that will handle moving deleted users to staged
>>>>>>>>> area as
>>>>>>>>> --from-delete did.
>>>>>>>> Make it new command please.
>>>>>>>>
>>>>>>>>> Instead of stageuser-add and option --from-delete, which work
>>>>>>>>> totally
>>>>>>>>> different, the command user-undel does similar operation than
>>>>>>>>> stage-user
>>>>>>>>> --from-delete, it just uses different container.
>>>>>>>> NACK on stuffing everything into a single command just because it
>>>>>>>> does
>>>>>>>> something similar.
>>>>>>> How about making it a 'stageuser-undel'? The 'user-undel' moves
>>>>>>> preserved user to active, so the 'stageuser-undel' would move
>>>>>>> preserved
>>>>>>> to staged. The action is similar, but has slightly different
>>>>>>> specifics
>>>>>>> (which attributes are preserved etc.), and for me the
>>>>>>> 'stageuser-undel'
>>>>>>> feels more natural than 'user-undel --to-staged' since it's 
>>>>>>> basically
>>>>>>> the same as there is 'stageuser-add' for creating a staged user, 
>>>>>>> not
>>>>>>> 'user-add --to-staged'. It would be in the same style as all the
>>>>>>> other
>>>>>>> commands concerning operations with users in staged container.
>>>>>> Well, user-undel is the opposite of user-del, and stageuser-undel
>>>>>> should be the opposite of stageuser-del. The stageuser-undel you are
>>>>>> suggesting is not.
>>>>>>
>>>>>> Also I'm not sure if we want to (always) remove the deleted user 
>>>>>> once
>>>>>> a staged user is created from it, but -undel behaves like that.
>>>>>>
>>>>>> I don't think the command should be limited to deleted users only.
>>>>>> Active and deleted users share the same namespace, so it is an
>>>>>> arbitrary limitation.
>>>>> preserved users has been valid active user. In that sense
>>>>> active/preserved are managed by a same set of CLI
>>>>> (user-find,user-del,user-show) because a preserved user is a 
>>>>> 'user'. So
>>>>> I would vote for continuing with a 'user-*' commands and use
>>>>> 'user-undel
>>>>> --to-stage'.
>>>>>
>>>>> But then if we will make any incompatible change between "user-undel"
>>>>> and "user-undel --to-stage" we may hit this issue again. I agree with
>>>>> Honza, this should be a separate command.
>>>> What do you mean 'incompatible change' ?
>>>>
>>>> --to-stage option would only select a different container that the
>>>> 'Active' one ?
>>>
>>> That's not sufficient. The command should do the reverse of
>>> stageuser-activate, which is ADD and DELETE, possibly with some
>>> modifications of the entry between them, not MODRDN like user-undel 
>>> does.
>>>
>> That is a good point. Do we need to modify anything from a deleted entry
>> to a add it in the stage container.
>> Already delete entry have been purged of several values (password, krb
>> keys, membership,..) do you think of other attributes to remove ?
>>
>> IIRC the use case is a support engineer who activated too early an
>> entry. So you are right he wants to unactivate it. A question is does
>> the unactivation requires more modifications than the one did by
>> 'user-del --preserve'. Note that we can not retrieve the attribute
>> values when the entry was activated from stage.
>
> I don't know if any modifications are needed ATM (doesn't mean there 
> can't be any in the future), but the point is that if you are creating 
> object A from object B using operation X, you should be creating 
> object B from object A using the reverse of operation X, otherwise 
> there *will* be inconsistencies, and we don't want that.
>
+1
I agree with this

-- 
Martin Basti