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

Wed Aug 12 12:08:59 UTC 2015

On 12/08/15 13:15, David Kupka wrote:
> On 12/08/15 12:45, thierry bordaz wrote:
>> On 08/12/2015 12:35 PM, Martin Basti wrote:
>>>
>>>
>>> On 08/11/2015 10:40 AM, thierry bordaz wrote:
>>>> On 08/11/2015 09:32 AM, Martin Basti wrote:
>>>>> 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
>>>>>
>>>> So my understanding is that you think a new verb should be created to
>>>> allow: 'Active' -> 'Stage'
>>>> I do not recall why this was not discussed or if it as already been
>>>> rejected.
>>>>
>>>> I like the idea and I think it could be useful to Support Engineer.
>>>> Now I am not sure we want to make 'easy' the action to 'unactivate' a
>>>> user (currently it requires two operations).
>>>> In your opinion, does it replace 'stageuser-add --from-delete' or
>>>> 'user-undel --to-stage' ? or is it an additional subcommand.
>>>> Also, activate/unactivate is not a NULL operation. Some values has
>>>> been changed (uid,gid,uniqueuiid...) and some values will be lost
>>>> (membership).
>>>>
>>>> About the verb, this is not because the previous action is 'activate'
>>>> that we should use 'unactivate'. For example, Thomas raised the point
>>>> that after 'user-del', 'user-restore' would have been more user
>>>> friendly than 'user-undel'
>>>>
>>>> Thanks
>>>> thierry
>>>>
>>> We had discussion off-list discussion, and result is following 
>>> proposal:
>>>
>>> * remove 'stageuser-add --from-delete'
>>> * add new command: 'user-stage'
>>>
>>> the user-stage command will move both deleted or active users to
>>> staged area.
>>> $ user-stage <deleted_user>
>>> replaces the stage-user --from-delete, keeps the same behavior
>>>
>>> $ user-stage <active_user>
>>> this is stretch goal, nice to have, but I don't know how easy is to
>>> implement this
>>>
>>> For better visualization, here is link to our board screen
>>> https://pvoborni.fedorapeople.org/images/user-lifecycle.jpg
>>>
>>> Thierry, do you agree with this?
>>> Martin^2
>>
>>
>> Hello,
>>
>> I really like the idea (as well as the drawing) of having the same cli
>> for both active/deleted user.
>> About the exact verb 'user-stage', I am always bad at this exercise and
>> it would be great to have Thomas ack on that.
>>
>> Just a question about the other verbs user-disable/user-enable. I know
>> they are doing something different but do you think there is a risk of
>> confusion for admin when he should do user-stage or user-disable ?
>>
>> thanks
>> thierry
>>
>>
>>
>
> Adding Tomas to the loop.
Hello everyone,

   I probably don't have all the information and perhaps cannot see all 
possible side effects but on the handover session for this feature, I 
was concerned that some commands did not match in GUI and in CLI 
(restore, undel).

Also, from the UX perspective, I thought it would be more friendly to 
have symmetrical commands and not confuse users with two delete modes as 
in "do you want to mock-delete the user or delete delete it?"

Therefore, I proposed to have two simple pairs of commands, also 
reflected in the UI:

"add" and "delete"  - for just adding and completely removing users

"retire" and "restore" - for just putting a user to or taking it out of 
the user "archive"

as for the "--from-delete" option , I think the proposed "user-stage" 
overlaps a little with the existing "stageuser-*" commands. As the 
command would move a user to the initial state of the lifecycle, be it 
an active or retired user, I'd try to emphasize the fact by proposing a 
similar but perhaps more cogent "user-restage".

Do you think it would work?

Tomas

-- 
Tomáš Čapek
tcapek at redhat.com
IRC Nick: tcapek
Team name: Customer Content Services (CCS)