From b63643e1d35f6f0fe6b72532252acee93f27d5b1 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/flags.go | 30 +++++++------ commands/property.go | 53 ++++++++++++++++++----- tests/src/integration/command_test.go | 27 +++++++++++- tests/src/integration/integration_test.go | 51 +++++++++++++++++++++- wski18n/resources/en_US.all.json | 24 ++++++++++ 6 files changed, 177 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/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..10dab6c8d 100644 --- a/tests/src/integration/command_test.go +++ b/tests/src/integration/command_test.go @@ -100,7 +100,7 @@ func TestValidateDefaultProperties(t *testing.T) { 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.") - stdout, err := wsk.RunCommand("property", "unset", "--auth", "--apihost", "--apiversion", "--namespace") + stdout, err := wsk.RunCommand("property", "unset", "--auth", "--apihost", "--apiversion", "--namespace", "--promptOnChange") assert.Equal(t, nil, err, "The command property unset failed to run.") outputString := string(stdout) assert.Contains(t, outputString, "ok: whisk auth unset", @@ -111,6 +111,8 @@ func TestValidateDefaultProperties(t *testing.T) { "The output of the command does not contain \"ok: whisk API version unset\".") assert.Contains(t, outputString, "ok: whisk namespace unset", "The output of the command does not contain \"ok: whisk namespace unset\".") + assert.Contains(t, outputString, "ok: whisk promptOnChange unset", + "The output of the command does not contain \"ok: whisk promptOnChange unset\".") stdout, err = wsk.RunCommand("property", "get", "--auth") assert.Equal(t, nil, err, "The command property get --auth failed to run.") @@ -127,6 +129,11 @@ func TestValidateDefaultProperties(t *testing.T) { assert.Equal(t, "whisk namespace _", common.RemoveRedundentSpaces(string(stdout)), "The output of the command does not equal to \"whisk namespace _\".") + stdout, err = wsk.RunCommand("property", "get", "--promptOnChange") + assert.Equal(t, nil, err, "The command property get --promptOnChange failed to run.") + assert.Equal(t, "whisk promptOnChange", common.RemoveRedundentSpaces(string(stdout)), + "The output of the command does not equal to \"whisk promptOnChange\".") + common.DeleteFile(tmpProp) } @@ -145,6 +152,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 +175,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..2361d88f8 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,55 @@ 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]) + fmt.Println(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, "--promptOnChange") + assert.Equal(t, nil, err, "The command property set --apihost --auth --namespace --promptOnChange failed to run.") + stdout, err := wsk.RunCommand("action", "delete", "actionName") + assert.Equal(t, nil, err, "The command action delete actionName failed to run.") + 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\".") + } + 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]) + fmt.Println(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, "--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", "update", "hello", helloFile) + assert.Equal(t, nil, err, "The command action update hello failed to run.") + 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\".") + } + 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"