From 33b6054ccde452fa3fe11506197adf539697ead7 Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Wed, 26 Sep 2018 11:13:17 +0800 Subject: [PATCH] Add prompt on change when delete or update action As more and more production system uses openwhisk, Users will need some feature to protect their action to be deleted or updated by mistake. --- commands/action.go | 21 ++++++++ commands/commands.go | 2 +- commands/flags.go | 30 ++++++----- commands/property.go | 53 +++++++++++++----- tests/src/integration/command_test.go | 18 ++++++- tests/src/integration/integration_test.go | 65 ++++++++++++++++++++++- wski18n/resources/en_US.all.json | 24 +++++++++ 7 files changed, 184 insertions(+), 29 deletions(-) diff --git a/commands/action.go b/commands/action.go index 615f56b0d..929ac43e2 100644 --- a/commands/action.go +++ b/commands/action.go @@ -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) } @@ -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) } @@ -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")) @@ -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")) diff --git a/commands/commands.go b/commands/commands.go index 6a7a070c5..f29f2f137 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -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 diff --git a/commands/flags.go b/commands/flags.go index 223ccfe08..f4a0aa001 100644 --- a/commands/flags.go +++ b/commands/flags.go @@ -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 @@ -145,6 +146,7 @@ type ActionFlags struct { url bool save bool saveAs string + force bool } func IsVerbose() bool { diff --git a/commands/property.go b/commands/property.go index 84230bf8f..2dbfbb732 100644 --- a/commands/property.go +++ b/commands/property.go @@ -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 = "" @@ -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{ @@ -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) @@ -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) @@ -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 } @@ -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)) } @@ -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`")) @@ -376,6 +398,7 @@ 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")) @@ -383,6 +406,7 @@ func init() { 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")) } @@ -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 @@ -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 } diff --git a/tests/src/integration/command_test.go b/tests/src/integration/command_test.go index d4808f10b..6cfc038e5 100644 --- a/tests/src/integration/command_test.go +++ b/tests/src/integration/command_test.go @@ -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) @@ -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) } diff --git a/tests/src/integration/integration_test.go b/tests/src/integration/integration_test.go index 583f48cdf..796d7ffb4 100644 --- a/tests/src/integration/integration_test.go +++ b/tests/src/integration/integration_test.go @@ -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) @@ -357,6 +357,69 @@ 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", "update", "hello", helloFile, "--force") + assert.Equal(t, nil, err, "The command action update failed to run.") + + 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) diff --git a/wski18n/resources/en_US.all.json b/wski18n/resources/en_US.all.json index 3e7024ec9..9561d6303 100644 --- a/wski18n/resources/en_US.all.json +++ b/wski18n/resources/en_US.all.json @@ -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}}" @@ -360,6 +380,10 @@ "id": "whisk namespace", "translation": "whisk namespace" }, + { + "id": "whisk promptOnChange", + "translation": "whisk promptOnChange" + }, { "id": "whisk CLI version", "translation": "whisk CLI version"