Skip to content

Commit

Permalink
Add prompt on change when delete or update action
Browse files Browse the repository at this point in the history
As more and more production system uses openwhisk,
Users will need some feature to protect their action to be
deleted or updated by mistake.
  • Loading branch information
ningyougang committed Sep 27, 2018
1 parent e01606d commit d84129f
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 29 deletions.
21 changes: 21 additions & 0 deletions commands/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ var actionUpdateCmd = &cobra.Command{
return actionParseError(cmd, args, err)
}

if Properties.PromptOnChange {
if !Flags.action.force {
errMsg := wski18n.T("please update action using --force if you really want to update it")
whiskErr := whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL,
whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
return whiskErr
}
}

if _, _, err = Client.Actions.Insert(action, true); err != nil {
return actionInsertError(action, err)
}
Expand Down Expand Up @@ -335,6 +344,15 @@ var actionDeleteCmd = &cobra.Command{

Client.Namespace = qualifiedName.GetNamespace()

if Properties.PromptOnChange {
if !Flags.action.force {
errMsg := wski18n.T("please delete action using --force if you really want to delete it")
whiskErr := whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL,
whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
return whiskErr
}
}

if _, err = Client.Actions.Delete(qualifiedName.GetEntityName()); err != nil {
return actionDeleteError(qualifiedName.GetEntityName(), err)
}
Expand Down Expand Up @@ -1269,6 +1287,8 @@ func init() {
actionCreateCmd.Flags().StringVar(&Flags.action.web, WEB_FLAG, "", wski18n.T("treat ACTION as a web action, a raw HTTP web action, or as a standard action; yes | true = web action, raw = raw HTTP web action, no | false = standard action"))
actionCreateCmd.Flags().StringVar(&Flags.action.websecure, WEB_SECURE_FLAG, "", wski18n.T("secure the web action. where `SECRET` is true, false, or any string. Only valid when the ACTION is a web action"))

actionDeleteCmd.Flags().BoolVar(&Flags.action.force, "force", false, wski18n.T("force to do this operation when property promptOnChange is true"))

actionUpdateCmd.Flags().BoolVar(&Flags.action.native, "native", false, wski18n.T("treat ACTION as native action (zip file provides a compatible executable to run)"))
actionUpdateCmd.Flags().StringVar(&Flags.action.docker, "docker", "", wski18n.T("use provided docker image (a path on DockerHub) to run the action"))
actionUpdateCmd.Flags().BoolVar(&Flags.action.copy, "copy", false, wski18n.T("treat ACTION as the name of an existing action"))
Expand All @@ -1284,6 +1304,7 @@ func init() {
actionUpdateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
actionUpdateCmd.Flags().StringVar(&Flags.action.web, WEB_FLAG, "", wski18n.T("treat ACTION as a web action, a raw HTTP web action, or as a standard action; yes | true = web action, raw = raw HTTP web action, no | false = standard action"))
actionUpdateCmd.Flags().StringVar(&Flags.action.websecure, WEB_SECURE_FLAG, "", wski18n.T("secure the web action. where `SECRET` is true, false, or any string. Only valid when the ACTION is a web action"))
actionUpdateCmd.Flags().BoolVar(&Flags.action.force, "force", false, wski18n.T("force to do this operation when property promptOnChange is true"))

actionInvokeCmd.Flags().StringSliceVarP(&Flags.common.param, "param", "p", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
actionInvokeCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
Expand Down
2 changes: 1 addition & 1 deletion commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func SetupClientConfig(cmd *cobra.Command, args []string) error {
Flags.property.cert || Flags.property.key || Flags.property.apihost || Flags.property.namespace ||
Flags.property.apiversion || Flags.property.cliversion)) ||
(cmd.Parent().Name() == "property" && cmd.Name() == "set" && (len(Flags.property.apihostSet) > 0 ||
len(Flags.property.apiversionSet) > 0 || len(Flags.Global.Auth) > 0)) ||
len(Flags.property.apiversionSet) > 0 || len(Flags.Global.Auth) > 0 || Flags.property.promptOnChange)) ||
(cmd.Parent().Name() == "sdk" && cmd.Name() == "install" && len(args) > 0 && args[0] == "bashauto")

// Display an error if the parent command requires an API host to be set, and the current API host is not valid
Expand Down
30 changes: 16 additions & 14 deletions commands/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,21 @@ type FlagsStruct struct {
}

property struct {
cert bool
key bool
auth bool
apihost bool
apiversion bool
namespace bool
cliversion bool
apibuild bool
apibuildno bool
insecure bool
all bool
apihostSet string
apiversionSet string
namespaceSet string
cert bool
key bool
auth bool
apihost bool
apiversion bool
namespace bool
promptOnChange bool
cliversion bool
apibuild bool
apibuildno bool
insecure bool
all bool
apihostSet string
apiversionSet string
namespaceSet string
}

action ActionFlags
Expand Down Expand Up @@ -145,6 +146,7 @@ type ActionFlags struct {
url bool
save bool
saveAs string
force bool
}

func IsVerbose() bool {
Expand Down
53 changes: 41 additions & 12 deletions commands/property.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,17 @@ import (
)

var Properties struct {
Cert string
Key string
Auth string
APIHost string
APIVersion string
APIBuild string
APIBuildNo string
CLIVersion string
Namespace string
PropsFile string
Cert string
Key string
Auth string
APIHost string
APIVersion string
APIBuild string
APIBuildNo string
CLIVersion string
Namespace string
PromptOnChange bool
PropsFile string
}

const DefaultCert string = ""
Expand All @@ -51,6 +52,7 @@ const DefaultAPIVersion string = "v1"
const DefaultAPIBuild string = ""
const DefaultAPIBuildNo string = ""
const DefaultNamespace string = "_"
const DefaultPromptOnChange bool = false
const DefaultPropsFile string = "~/.wskprops"

var propertyCmd = &cobra.Command{
Expand Down Expand Up @@ -165,6 +167,13 @@ var propertySetCmd = &cobra.Command{
}
}

if promptOnChange := Flags.property.promptOnChange; promptOnChange {
props["PROMPTONCHANGE"] = "true"
okMsg += fmt.Sprintf(
wski18n.T("{{.ok}} whisk promptOnChange set to {{.name}}\n",
map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(promptOnChange)}))
}

err = WriteProps(Properties.PropsFile, props)
if err != nil {
whisk.Debug(whisk.DbgError, "writeProps(%s, %#v) failed: %s\n", Properties.PropsFile, props, err)
Expand Down Expand Up @@ -261,6 +270,13 @@ var propertyUnsetCmd = &cobra.Command{
}
}

if Flags.property.promptOnChange {
delete(props, "PROMPTONCHANGE")
okMsg += fmt.Sprintf(
wski18n.T("{{.ok}} whisk promptOnChange unset.\n",
map[string]interface{}{"ok": color.GreenString("ok:")}))
}

err = WriteProps(Properties.PropsFile, props)
if err != nil {
whisk.Debug(whisk.DbgError, "writeProps(%s, %#v) failed: %s\n", Properties.PropsFile, props, err)
Expand Down Expand Up @@ -291,8 +307,9 @@ var propertyGetCmd = &cobra.Command{
if !(Flags.property.all || Flags.property.cert ||
Flags.property.key || Flags.property.auth ||
Flags.property.apiversion || Flags.property.cliversion ||
Flags.property.namespace || Flags.property.apibuild ||
Flags.property.apihost || Flags.property.apibuildno) {
Flags.property.namespace || Flags.property.promptOnChange ||
Flags.property.apibuild || Flags.property.apihost ||
Flags.property.apibuildno) {
Flags.property.all = true
}

Expand Down Expand Up @@ -320,6 +337,10 @@ var propertyGetCmd = &cobra.Command{
fmt.Fprintf(color.Output, "%s\t\t%s\n", wski18n.T("whisk namespace"), boldString(Properties.Namespace))
}

if Flags.property.all || Flags.property.promptOnChange {
fmt.Fprintf(color.Output, "%s\t\t%s\n", wski18n.T("whisk promptOnChange"), boldString(Properties.PromptOnChange))
}

if Flags.property.all || Flags.property.cliversion {
fmt.Fprintf(color.Output, "%s\t%s\n", wski18n.T("whisk CLI version"), boldString(Properties.CLIVersion))
}
Expand Down Expand Up @@ -368,6 +389,7 @@ func init() {
propertyGetCmd.Flags().BoolVar(&Flags.property.apibuildno, "apibuildno", false, wski18n.T("whisk API build number"))
propertyGetCmd.Flags().BoolVar(&Flags.property.cliversion, "cliversion", false, wski18n.T("whisk CLI version"))
propertyGetCmd.Flags().BoolVar(&Flags.property.namespace, "namespace", false, wski18n.T("whisk namespace"))
propertyGetCmd.Flags().BoolVar(&Flags.property.promptOnChange, "promptOnChange", false, wski18n.T("whisk promptOnChange"))
propertyGetCmd.Flags().BoolVar(&Flags.property.all, "all", false, wski18n.T("all properties"))

propertySetCmd.Flags().StringVarP(&Flags.Global.Auth, "auth", "u", "", wski18n.T("authorization `KEY`"))
Expand All @@ -376,13 +398,15 @@ func init() {
propertySetCmd.Flags().StringVar(&Flags.property.apihostSet, "apihost", "", wski18n.T("whisk API `HOST`"))
propertySetCmd.Flags().StringVar(&Flags.property.apiversionSet, "apiversion", "", wski18n.T("whisk API `VERSION`"))
propertySetCmd.Flags().StringVar(&Flags.property.namespaceSet, "namespace", "", wski18n.T("whisk `NAMESPACE`"))
propertySetCmd.Flags().BoolVar(&Flags.property.promptOnChange, "promptOnChange", false, wski18n.T("whisk promptOnChange"))

propertyUnsetCmd.Flags().BoolVar(&Flags.property.cert, "cert", false, wski18n.T("client cert"))
propertyUnsetCmd.Flags().BoolVar(&Flags.property.key, "key", false, wski18n.T("client key"))
propertyUnsetCmd.Flags().BoolVar(&Flags.property.auth, "auth", false, wski18n.T("authorization key"))
propertyUnsetCmd.Flags().BoolVar(&Flags.property.apihost, "apihost", false, wski18n.T("whisk API host"))
propertyUnsetCmd.Flags().BoolVar(&Flags.property.apiversion, "apiversion", false, wski18n.T("whisk API version"))
propertyUnsetCmd.Flags().BoolVar(&Flags.property.namespace, "namespace", false, wski18n.T("whisk namespace"))
propertyUnsetCmd.Flags().BoolVar(&Flags.property.promptOnChange, "promptOnChange", false, wski18n.T("whisk promptOnChange"))

}

Expand All @@ -391,6 +415,7 @@ func SetDefaultProperties() {
Properties.Cert = DefaultKey
Properties.Auth = DefaultAuth
Properties.Namespace = DefaultNamespace
Properties.PromptOnChange = DefaultPromptOnChange
Properties.APIHost = DefaultAPIHost
Properties.APIBuild = DefaultAPIBuild
Properties.APIBuildNo = DefaultAPIBuildNo
Expand Down Expand Up @@ -492,6 +517,10 @@ func loadProperties() error {
Properties.Namespace = namespace
}

if promptOnChange, hasProp := props["PROMPTONCHANGE"]; hasProp && len(promptOnChange) > 0 && promptOnChange == "true" {
Properties.PromptOnChange = true
}

return nil
}

Expand Down
18 changes: 17 additions & 1 deletion tests/src/integration/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,21 @@ func TestSetAuth(t *testing.T) {
common.DeleteFile(tmpProp)
}

// Test case to set promptOnChange in property file.
func TestSetPromptOnChange(t *testing.T) {
common.CreateFile(tmpProp)

os.Setenv("WSK_CONFIG_FILE", tmpProp)
assert.Equal(t, os.Getenv("WSK_CONFIG_FILE"), tmpProp, "The environment variable WSK_CONFIG_FILE has not been set.")

_, err := wsk.RunCommand("property", "set", "--promptOnChange")
assert.Equal(t, nil, err, "The command property set --promptOnChange failed to run.")
output := common.ReadFile(tmpProp)
assert.Contains(t, output, "PROMPTONCHANGE=true",
"The wsk property file does not contain \"PROMPTONCHANGE=true\".")
common.DeleteFile(tmpProp)
}

// Test case to set multiple property values with single command.
func TestSetMultipleValues(t *testing.T) {
common.CreateFile(tmpProp)
Expand All @@ -153,13 +168,14 @@ func TestSetMultipleValues(t *testing.T) {
assert.Equal(t, os.Getenv("WSK_CONFIG_FILE"), tmpProp, "The environment variable WSK_CONFIG_FILE has not been set.")

_, err := wsk.RunCommand("property", "set", "--auth", "testKey", "--apihost", "openwhisk.ng.bluemix.net",
"--apiversion", "v1")
"--apiversion", "v1", "--promptOnChange")
assert.Equal(t, nil, err, "The command property set --auth --apihost --apiversion failed to run.")
output := common.ReadFile(tmpProp)
assert.Contains(t, output, "AUTH=testKey", "The wsk property file does not contain \"AUTH=testKey\".")
assert.Contains(t, output, "APIHOST=openwhisk.ng.bluemix.net",
"The wsk property file does not contain \"APIHOST=openwhisk.ng.bluemix.net\".")
assert.Contains(t, output, "APIVERSION=v1", "The wsk property file does not contain \"APIVERSION=v1\".")
assert.Contains(t, output, "PROMPTONCHANGE=true", "The wsk property file does not contain \"PROMPTONCHANGE=true\".")
common.DeleteFile(tmpProp)
}

Expand Down
62 changes: 61 additions & 1 deletion tests/src/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ func TestSetAPIHostAuthNamespace(t *testing.T) {
namespaces := strings.Split(strings.TrimSpace(string(namespace)), "\n")
expectedNamespace := string(namespaces[len(namespaces)-1])
fmt.Println(wsk.Wskprops.APIHost)
if wsk.Wskprops.APIHost != "" && wsk.Wskprops.APIHost != "" {
if wsk.Wskprops.APIHost != "" && wsk.Wskprops.AuthKey != "" {
stdout, err := wsk.RunCommand("property", "set", "--apihost", wsk.Wskprops.APIHost,
"--auth", wsk.Wskprops.AuthKey, "--namespace", expectedNamespace)
ouputString := string(stdout)
Expand All @@ -357,6 +357,66 @@ func TestSetAPIHostAuthNamespace(t *testing.T) {
common.DeleteFile(tmpProp)
}

// Test delete action when property promptOnChange is true
func TestDeleteActionWhenPromptOnChangeIsTrue(t *testing.T) {
common.CreateFile(tmpProp)
common.WriteFile(tmpProp, []string{})

os.Setenv("WSK_CONFIG_FILE", tmpProp)
assert.Equal(t, os.Getenv("WSK_CONFIG_FILE"), tmpProp, "The environment variable WSK_CONFIG_FILE has not been set.")

namespace, _ := wsk.ListNamespaces()
namespaces := strings.Split(strings.TrimSpace(string(namespace)), "\n")
expectedNamespace := string(namespaces[len(namespaces)-1])
if wsk.Wskprops.APIHost != "" && wsk.Wskprops.AuthKey != "" {
stdout, err := wsk.RunCommand("property", "set", "--apihost", wsk.Wskprops.APIHost,
"--auth", wsk.Wskprops.AuthKey, "--namespace", expectedNamespace, "--promptOnChange")
assert.Equal(t, nil, err, "The command property set --apihost --auth --namespace --promptOnChange failed to run.")

helloFile := common.GetTestActionFilename("hello.js")
stdout, err = wsk.RunCommand("action", "create", "hello", helloFile)
assert.Equal(t, nil, err, "The command action create failed to run.")

stdout, err = wsk.RunCommand("action", "delete", "hello")
assert.Contains(t, common.RemoveRedundentSpaces(string(stdout)), "please delete action using --force if you really want to delete it",
"The output of the command does not contain \"please delete action using --force if you really want to delete it\".")

stdout, err = wsk.RunCommand("action", "delete", "hello", "--force")
assert.Equal(t, nil, err, "The command action delete failed to run.")
}
common.DeleteFile(tmpProp)
}

// Test update action when property promptOnChange is true
func TestUpdateActionWhenPromptOnChangeIsTrue(t *testing.T) {
common.CreateFile(tmpProp)
common.WriteFile(tmpProp, []string{})

os.Setenv("WSK_CONFIG_FILE", tmpProp)
assert.Equal(t, os.Getenv("WSK_CONFIG_FILE"), tmpProp, "The environment variable WSK_CONFIG_FILE has not been set.")

namespace, _ := wsk.ListNamespaces()
namespaces := strings.Split(strings.TrimSpace(string(namespace)), "\n")
expectedNamespace := string(namespaces[len(namespaces)-1])
if wsk.Wskprops.APIHost != "" && wsk.Wskprops.AuthKey != "" {
stdout, err := wsk.RunCommand("property", "set", "--apihost", wsk.Wskprops.APIHost,
"--auth", wsk.Wskprops.AuthKey, "--namespace", expectedNamespace, "--promptOnChange")
assert.Equal(t, nil, err, "The command property set --apihost --auth --namespace --promptOnChange failed to run.")

helloFile := common.GetTestActionFilename("hello.js")
stdout, err = wsk.RunCommand("action", "create", "hello", helloFile)
assert.Equal(t, nil, err, "The command action create failed to run.")

stdout, err = wsk.RunCommand("action", "update", "hello", helloFile)
assert.Contains(t, common.RemoveRedundentSpaces(string(stdout)), "please update action using --force if you really want to update it",
"The output of the command does not contain \"please update action using --force if you really want to update it\".")

stdout, err = wsk.RunCommand("action", "delete", "hello", "--force")
assert.Equal(t, nil, err, "The command action delete failed to run.")
}
common.DeleteFile(tmpProp)
}

// Test case to show api build version using property file.
func TestShowAPIBuildVersion(t *testing.T) {
common.CreateFile(tmpProp)
Expand Down
24 changes: 24 additions & 0 deletions wski18n/resources/en_US.all.json
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,26 @@
"id": "{{.ok}} whisk namespace set to {{.name}}\n",
"translation": "{{.ok}} whisk namespace set to {{.name}}\n"
},
{
"id": "{{.ok}} whisk promptOnChange set to {{.name}}\n",
"translation": "{{.ok}} whisk promptOnChange set to {{.name}}\n"
},
{
"id": "{{.ok}} whisk promptOnChange unset.\n",
"translation": "{{.ok}} whisk promptOnChange unset.\n"
},
{
"id": "force to do this operation when property promptOnChange is true",
"translation": "force to do this operation when property promptOnChange is true"
},
{
"id": "please update action using --force if you really want to update it",
"translation": "please update action using --force if you really want to update it"
},
{
"id": "please delete action using --force if you really want to delete it",
"translation": "please delete action using --force if you really want to delete it"
},
{
"id": "Unable to set the property value(s): {{.err}}",
"translation": "Unable to set the property value(s): {{.err}}"
Expand Down Expand Up @@ -360,6 +380,10 @@
"id": "whisk namespace",
"translation": "whisk namespace"
},
{
"id": "whisk promptOnChange",
"translation": "whisk promptOnChange"
},
{
"id": "whisk CLI version",
"translation": "whisk CLI version"
Expand Down

0 comments on commit d84129f

Please sign in to comment.