Repository: knative-sandbox/kn-plugin-func
Pull request #702 (2021-12-06 16:57:16)

chore: refactor CLI flags for consistency across commands
# Changes

:broom: refactor path and namespace CLI flags for consistency across commands

These flags are used by multiple commands, and explicitly setting them for
each command risks errors that may not be easily detected (see the deleted
flag text for `namespace` in deploy.go). The downside to this is that wording
is not sligtly tweaked for the command - see again some of the removed text
for `namespace` that was slightly different for each command, but usefully so.

I'm not 100% convinced this change is needed, since it does reduce the flexibility
of the user facing text. But if it had existed before we wouldn't have had
incorrect text for `deploy`.

Fixes: https://github.com/knative-sandbox/kn-plugin-func/issues/221

Signed-off-by: Lance Ball <lball@redhat.com>

/kind chore


knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/702#" title="Author self-approved">lance</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/main/OWNERS)~~ [lance]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/cmd/build.go b/cmd/build.go
index c20d206d..8ba3c46b 100644
--- a/cmd/build.go
+++ b/cmd/build.go
@@ -91,9 +91,9 @@ kn func build --builder cnbs/sample-builder:bionic
 	cmd.Flags().StringP("builder", "b", "", "Buildpack builder, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml for subsequent builds.")
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)")
 	cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE)")
-	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	cmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'.  The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)")
 	cmd.Flags().BoolP("push", "u", false, "Attempt to push the function image after being successfully built")
+	setPathFlag(cmd)
 
 	if err := cmd.RegisterFlagCompletionFunc("builder", CompleteBuilderList); err != nil {
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
diff --git a/cmd/config.go b/cmd/config.go
index a604789a..b27429fb 100644
--- a/cmd/config.go
+++ b/cmd/config.go
@@ -45,7 +45,7 @@ var defaultLoaderSaver standardLoaderSaver
 
 func init() {
 	root.AddCommand(configCmd)
-	configCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
+	setPathFlag(configCmd)
 	configCmd.AddCommand(NewConfigLabelsCmd(defaultLoaderSaver))
 }
 
diff --git a/cmd/config_envs.go b/cmd/config_envs.go
index 1c5523ae..8d4684d9 100644
--- a/cmd/config_envs.go
+++ b/cmd/config_envs.go
@@ -15,12 +15,12 @@ import (
 )
 
 func init() {
+	setPathFlag(configEnvsCmd)
+	setPathFlag(configEnvsAddCmd)
+	setPathFlag(configEnvsRemoveCmd)
 	configCmd.AddCommand(configEnvsCmd)
-	configEnvsCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	configEnvsCmd.AddCommand(configEnvsAddCmd)
-	configEnvsAddCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	configEnvsCmd.AddCommand(configEnvsRemoveCmd)
-	configEnvsRemoveCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 }
 
 var configEnvsCmd = &cobra.Command{
diff --git a/cmd/config_labels.go b/cmd/config_labels.go
index 86de13f9..e10b92c9 100644
--- a/cmd/config_labels.go
+++ b/cmd/config_labels.go
@@ -79,11 +79,11 @@ directory or from the directory specified with --path.
 		},
 	}
 
-	configLabelsCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
+	setPathFlag(configLabelsCmd)
+	setPathFlag(configLabelsAddCmd)
+	setPathFlag(configLabelsRemoveCmd)
 	configLabelsCmd.AddCommand(configLabelsAddCmd)
-	configLabelsAddCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	configLabelsCmd.AddCommand(configLabelsRemoveCmd)
-	configLabelsRemoveCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 
 	return configLabelsCmd
 }
diff --git a/cmd/config_volumes.go b/cmd/config_volumes.go
index 9b26cb7b..4d2f8c60 100644
--- a/cmd/config_volumes.go
+++ b/cmd/config_volumes.go
@@ -14,12 +14,12 @@ import (
 )
 
 func init() {
+	setPathFlag(configVolumesCmd)
+	setPathFlag(configVolumesAddCmd)
+	setPathFlag(configVolumesRemoveCmd)
 	configCmd.AddCommand(configVolumesCmd)
-	configVolumesCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	configVolumesCmd.AddCommand(configVolumesAddCmd)
-	configVolumesAddCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	configVolumesCmd.AddCommand(configVolumesRemoveCmd)
-	configVolumesRemoveCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 }
 
 var configVolumesCmd = &cobra.Command{
diff --git a/cmd/delete.go b/cmd/delete.go
index df0bd818..7de33a55 100644
--- a/cmd/delete.go
+++ b/cmd/delete.go
@@ -63,8 +63,8 @@ kn func delete -n apps myfunc
 	}
 
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)")
-	cmd.Flags().StringP("path", "p", cwd(), "Path to the function project that should be undeployed (Env: $FUNC_PATH)")
-	cmd.Flags().StringP("namespace", "n", "", "Namespace of the function to undeploy. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)")
+	setNamespaceFlag(cmd)
+	setPathFlag(cmd)
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		return runDelete(cmd, args, clientFn)
diff --git a/cmd/deploy.go b/cmd/deploy.go
index eb95a8e5..8ff33687 100644
--- a/cmd/deploy.go
+++ b/cmd/deploy.go
@@ -93,10 +93,10 @@ kn func deploy --image quay.io/myuser/myfunc -n myns
 		"You may provide this flag multiple times for setting multiple environment variables. "+
 		"To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).")
 	cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE)")
-	cmd.Flags().StringP("namespace", "n", "", "Namespace of the function to undeploy. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)")
-	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 	cmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'.  The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)")
 	cmd.Flags().BoolP("build", "b", true, "Build the image before deploying (Env: $FUNC_BUILD)")
+	setPathFlag(cmd)
+	setNamespaceFlag(cmd)
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		return runDeploy(cmd, args, clientFn)
diff --git a/cmd/emit.go b/cmd/emit.go
index aa88cf29..d695ec36 100644
--- a/cmd/emit.go
+++ b/cmd/emit.go
@@ -80,7 +80,7 @@ kn func emit --sink "http://my.event.broker.com"
 	cmd.Flags().StringP("data", "d", "", "Any arbitrary string to be sent as the CloudEvent data. Ignored if --file is provided  (Env: $FUNC_DATA)")
 	cmd.Flags().StringP("file", "f", "", "Path to a local file containing CloudEvent data to be sent  (Env: $FUNC_FILE)")
 	cmd.Flags().StringP("content-type", "c", "application/json", "The MIME Content-Type for the CloudEvent data  (Env: $FUNC_CONTENT_TYPE)")
-	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory. Ignored when --sink is provided (Env: $FUNC_PATH)")
+	setPathFlag(cmd)
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		return runEmit(cmd, args, clientFn)
diff --git a/cmd/info.go b/cmd/info.go
index 9a0e633b..781cc364 100644
--- a/cmd/info.go
+++ b/cmd/info.go
@@ -56,9 +56,9 @@ kn func info --output yaml --path myotherfunc
 		PreRunE:           bindEnv("namespace", "output", "path"),
 	}
 
-	cmd.Flags().StringP("namespace", "n", "", "Namespace of the function. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)")
+	setNamespaceFlag(cmd)
 	cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml|url) (Env: $FUNC_OUTPUT)")
-	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
+	setPathFlag(cmd)
 
 	if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil {
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
diff --git a/cmd/list.go b/cmd/list.go
index 57ecf99b..e0169213 100644
--- a/cmd/list.go
+++ b/cmd/list.go
@@ -62,8 +62,8 @@ kn func list --all-namespaces --output json
 	}
 
 	cmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.")
-	cmd.Flags().StringP("namespace", "n", "", "Namespace to search for functions. By default, the functions of the actual active namespace are listed. (Env: $FUNC_NAMESPACE)")
 	cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) (Env: $FUNC_OUTPUT)")
+	setNamespaceFlag(cmd)
 
 	if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil {
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
diff --git a/cmd/root.go b/cmd/root.go
index 74ddec34..0fe0f969 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -328,3 +328,13 @@ func mergeEnvs(envs []fn.Env, envToUpdate *util.OrderedMap, envToRemove []string
 
 	return envs, nil
 }
+
+// setPathFlag ensures common text/wording when the --path flag is used
+func setPathFlag(cmd *cobra.Command) {
+	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
+}
+
+// setNamespaceFlag ensures common text/wording when the --namespace flag is used
+func setNamespaceFlag(cmd *cobra.Command) {
+	cmd.Flags().StringP("namespace", "n", "", "The namespace on the cluster. By default, the namespace in func.yaml is used or the currently active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)")
+}
diff --git a/cmd/run.go b/cmd/run.go
index 508774d8..5455c85b 100644
--- a/cmd/run.go
+++ b/cmd/run.go
@@ -50,7 +50,7 @@ kn func run
 		"Environment variable to set in the form NAME=VALUE. "+
 			"You may provide this flag multiple times for setting multiple environment variables. "+
 			"To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).")
-	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
+	setPathFlag(cmd)
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		return runRun(cmd, args, clientFn)
diff --git a/templates.go b/templates.go
index 97c9043a..59d3cf90 100644
--- a/templates.go
+++ b/templates.go
@@ -7,6 +7,7 @@ package function
 
 import (
 	"errors"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -161,7 +162,11 @@ func (t *Templates) Write(f Function) (Function, error) {
 	}
 
 	// Copy the template files from the repo filesystem to the new Function's root
-	return f, copy(templatePath, f.Root, repo.FS)
+	// removing the manifest (if it exists; errors ignored)
+	err = copy(templatePath, f.Root, repo.FS)              // copy everything
+	_ = os.Remove(filepath.Join(f.Root, templateManifest)) // except the manifest
+
+	return f, err
 }
 
 // Embedding Directives


Test patch:
diff --git a/templates_test.go b/templates_test.go
index 2e97cf8e..bd0ee3f5 100644
--- a/templates_test.go
+++ b/templates_test.go
@@ -516,3 +516,38 @@ func TestRepositoryManifestBuildEnvs(t *testing.T) {
 		t.Fatalf("Unexpected difference between repository's manifest.yaml buildEnvs and Function BuildEnvs (-want, +got): %v", diff)
 	}
 }
+
+// TestTemplateManifestRemoved ensures that the manifest is not left in
+// the resultant Function after write.
+func TestTemplateManifestRemoved(t *testing.T) {
+	// create test directory
+	root := "testdata/testTemplateManifestRemoved"
+	defer Using(t, root)()
+
+	// Client whose internal templates will be used.
+	client := fn.New(
+		fn.WithRegistry(TestRegistry),
+		fn.WithRepositories("testdata/repositories"))
+
+	// write out a template
+	err := client.Create(fn.Function{
+		Root:     root,
+		Runtime:  "manifestedRuntime",
+		Template: "customLanguagePackRepo/manifestedTemplate",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert func.yaml exists as expected
+	_, err = os.Stat(filepath.Join(root, "func.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Assert manifest.yaml does not
+	_, err = os.Stat(filepath.Join(root, "manifest.yaml"))
+	if err == nil {
+		t.Fatal("manifest.yaml should not exist after write")
+	}
+
+}