openwhisk-cli icon indicating copy to clipboard operation
openwhisk-cli copied to clipboard

Add confirm mechanism when delete or update action

Open ningyougang opened this issue 6 years ago • 15 comments

I writed a issue here: https://github.com/apache/incubator-openwhisk-cli/issues/371

As more and more production system uses openwhisk, Users will need some feature to protect their action to be deleted or updated by mistake.

ningyougang avatar Sep 20 '18 08:09 ningyougang

What about adding a—safe mode which will prevent delete without a confirmation?

rabbah avatar Sep 20 '18 11:09 rabbah

this appears to be a breaking change as it is currently implemented. if so, i'm not in favor of this change as implemented.

one possible implementation is to use an env variable, say WSK_CLI_SAFE_MODE or WSK_CLI_ACTION_MGMT_SAFE_MODE or..., that when set will cause the cli to always prompt for confirmation of action deletes/updates. when env is not present, the current behavior remains. no need for additional command line flag.

mdeuser avatar Sep 20 '18 12:09 mdeuser

I like the suggestions. Could also store it in whisk properties.

rabbah avatar Sep 20 '18 12:09 rabbah

yep I like the using environment variable. But I'm not in love with the wording SAFE. How about something along WSK_CLI_PROMPT_ON_CHANGE or WSK_CLI_PROMPT=true|false (default) to false. Also this PR only touches on actions. I think it should be done for all changes (ie. wsk update | delete) I guess the user wants to be prompted if by mistake it updates or deletes a rule.

csantanapr avatar Sep 20 '18 14:09 csantanapr

@csantanapr , thanks for your reply. if used WSK_CLI_PROMPT_ON_CHANGE or WSK_CLI_PROMPT=true|false Does this env WSK_CLI_PROMPT_ON_CHANGE existed in openwhisk server side, exactly controller side(not in wsk client side)?

ningyougang avatar Sep 21 '18 02:09 ningyougang

WSK_CLI_PROMPT_ON_CHANGE and/or WSK_CLI_PROMPT do not exist in the backend

mdeuser avatar Sep 21 '18 12:09 mdeuser

@mdeuser Regarding WSK_CLI_PROMPT_ON_CHANGE and/or WSK_CLI_PROMPT do not exist in the backend

so you mean WSK_CLI_PROMPT_ON_CHANGE and/or WSK_CLI_PROMPT exists in wsk client side? if exist in wsk client side, for wsk client users, they may not know above environment env. How to notify wsk client users to know above environment env?

ningyougang avatar Sep 25 '18 07:09 ningyougang

A property saved in wskprops would address the environment issue since switching files will then configure the cli as desired. We have no precedent for an env file that changes the cli behavior outside of WSK_CONFIG_FILE so a stand-alone property to me for this behavior seems like the wrong direction.

rabbah avatar Sep 25 '18 10:09 rabbah

@rabbah ,already modified.

The patch implement the protect feature at CLI layer.

How about add the protect mechanism at the backed server layer also? because users may deploy their action using API (not wsk CLI)

But i am afraid that if added in backed server layer, may lead to client side do huge breakpoint. How do you think?

ningyougang avatar Sep 26 '18 05:09 ningyougang

On the backend I think the model is finer grained entitlement than we have today, with unix style permissions.

rabbah avatar Sep 26 '18 11:09 rabbah

I think a prompt would be better than an additional flag.

dubee avatar Sep 26 '18 20:09 dubee

@rabbah , so we have no need to add the protect mechanism for backend layer, right?

ningyougang avatar Sep 27 '18 00:09 ningyougang

If by prompt you mean something interactive which requires input on the console then this will make it very inconvenient to actually update a namespace when it is so intended.

rabbah avatar Sep 27 '18 07:09 rabbah

@rabbah ,already added test cases.

Regarding add protection feature to backend layer, i also think it these days. First, we should consider the original created action, if we add protection feature in backend layer ,we should promise have no influences to the original created action.

So one option is, add promptOnChange fileld to action, default value is false, When create action, we can assign the promptOnChange to true. if that, when call delete action API, should add force request parameter to delete it.

If we add promptOnChange field to backend's action, so have no need to add promptOnChange to WSK's wskprops file. How do you think?

ningyougang avatar Sep 28 '18 01:09 ningyougang

Please don't merge this patch May be we can solve this problem in backend layer: https://github.com/apache/incubator-openwhisk/pull/4058

ningyougang avatar Oct 09 '18 02:10 ningyougang