(odo init): Add input validation for component name (#6088)

* (odo init): Add input validation for component name Signed-off-by: Parthvi Vala <pvala@redhat.com> * Update docs/website/docs/command-reference/init.md Co-authored-by: Armel Soro <armel@rm3l.org> * Apply suggestions from code review Co-authored-by: Armel Soro <armel@rm3l.org> * Use instead of \n for interactive tests * Ask to re-enter component name until a valid value is passed Signed-off-by: Parthvi Vala <pvala@redhat.com> * feloy's review Signed-off-by: Parthvi Vala <pvala@redhat.com> Signed-off-by: Parthvi Vala <pvala@redhat.com> Co-authored-by: Armel Soro <armel@rm3l.org>
redhat-developer · Sep 6, 2022 · fe0832b · fe0832b
1 parent 3add393
commit fe0832b
Show file tree

Hide file tree

Showing 10 changed files with 140 additions and 6 deletions.
diff --git a/docs/website/docs/command-reference/init.md b/docs/website/docs/command-reference/init.md
@@ -14,7 +14,7 @@ The command can be executed in two flavors, either interactive or non-interactiv
 In interactive mode, you will be guided to choose:
 - a devfile from the list of devfiles present in the registry or registries referenced (using the `odo registry` command),
 - a starter project referenced by the selected devfile,
-- a name for the component present in the devfile.
+- a name for the component present in the devfile; this name must follow the [Kubernetes naming convention](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names) and not be all-numeric.
 
 ## Non-interactive mode
 
@@ -27,7 +27,7 @@ If you prefer to download a devfile from an URL or from the local filesystem, yo
 
 The `--starter` flag indicates the name of the starter project (as referenced in the selected devfile), that you want to use to start your development. To see the available starter projects for devfile stacks in the official devfile registry use its [web interface](https://registry.devfile.io/viewer) to view its content.  
 
-The required `--name` flag indicates how the component initialized by this command should be named.
+The required `--name` flag indicates how the component initialized by this command should be named. The name must follow the [Kubernetes naming convention](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names) and not be all-numeric.
 
 ## Examples
 

diff --git a/pkg/init/asker/asker.go b/pkg/init/asker/asker.go
@@ -5,6 +5,7 @@ import (
 	"sort"
 
 	"github.com/AlecAivazis/survey/v2"
+
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/registry"

diff --git a/pkg/init/backend/flags.go b/pkg/init/backend/flags.go
@@ -111,8 +111,12 @@ func (o *FlagsBackend) SelectStarterProject(devfile parser.DevfileObj, flags map
 	return nil, fmt.Errorf("starter project %q not found in devfile", starter)
 }
 
-func (o *FlagsBackend) PersonalizeName(devfile parser.DevfileObj, flags map[string]string) (string, error) {
+func (o *FlagsBackend) PersonalizeName(_ parser.DevfileObj, flags map[string]string) (string, error) {
+	if validK8sNameErr := dfutil.ValidateK8sResourceName("name", flags[FLAG_NAME]); validK8sNameErr != nil {
+		return "", validK8sNameErr
+	}
 	return flags[FLAG_NAME], nil
+
 }
 
 func (o FlagsBackend) PersonalizeDevfileConfig(devfileobj parser.DevfileObj) (parser.DevfileObj, error) {

diff --git a/pkg/init/backend/flags_test.go b/pkg/init/backend/flags_test.go
@@ -422,6 +422,27 @@ func TestFlagsBackend_PersonalizeName(t *testing.T) {
 				return newName == args.flags["name"]
 			},
 		},
+		{
+			name: "invalid name flag",
+			args: args{
+				devfile: func(fs dffilesystem.Filesystem) parser.DevfileObj {
+					devfileData, _ := data.NewDevfileData(string(data.APISchemaVersion200))
+					obj := parser.DevfileObj{
+						Ctx:  parsercontext.FakeContext(fs, "/tmp/devfile.yaml"),
+						Data: devfileData,
+					}
+					return obj
+				},
+				flags: map[string]string{
+					"devfile": "adevfile",
+					"name":    "1234",
+				},
+			},
+			wantErr: true,
+			checkResult: func(newName string, args args) bool {
+				return newName == ""
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/init/backend/interactive.go b/pkg/init/backend/interactive.go
@@ -9,6 +9,7 @@ import (
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/library/pkg/devfile/parser"
 	parsercommon "github.com/devfile/library/pkg/devfile/parser/data/v2/common"
+	dfutil "github.com/devfile/library/pkg/util"
 	"k8s.io/klog"
 
 	"github.com/redhat-developer/odo/pkg/alizer"
@@ -136,7 +137,20 @@ func (o *InteractiveBackend) PersonalizeName(devfile parser.DevfileObj, flags ma
 		return "", fmt.Errorf("unable to detect the name")
 	}
 
-	return o.askerClient.AskName(name)
+	var userReturnedName string
+	// keep asking the name until the user enters a valid name
+	for {
+		userReturnedName, err = o.askerClient.AskName(name)
+		if err != nil {
+			return "", err
+		}
+		validK8sNameErr := dfutil.ValidateK8sResourceName("name", userReturnedName)
+		if validK8sNameErr == nil {
+			break
+		}
+		log.Error(validK8sNameErr)
+	}
+	return userReturnedName, nil
 }
 
 func (o *InteractiveBackend) PersonalizeDevfileConfig(devfileobj parser.DevfileObj) (parser.DevfileObj, error) {

diff --git a/pkg/init/backend/interactive_test.go b/pkg/init/backend/interactive_test.go
@@ -247,7 +247,35 @@ func TestInteractiveBackend_PersonalizeName(t *testing.T) {
 			checkResult: func(newName string, args args) bool {
 				return newName == "aname"
 			},
-		}}
+		},
+		{
+			name: "invalid name",
+			fields: fields{
+				asker: func(ctrl *gomock.Controller) asker.Asker {
+					client := asker.NewMockAsker(ctrl)
+					client.EXPECT().AskName(gomock.Any()).Return("ls;aname", nil)
+					client.EXPECT().AskName(gomock.Any()).Return("aname", nil)
+					return client
+				},
+			},
+			args: args{
+				devfile: func(fs filesystem.Filesystem) parser.DevfileObj {
+					devfileData, _ := data.NewDevfileData(string(data.APISchemaVersion200))
+
+					obj := parser.DevfileObj{
+						Ctx:  parsercontext.FakeContext(fs, "/tmp/devfile.yaml"),
+						Data: devfileData,
+					}
+					return obj
+				},
+				flags: map[string]string{},
+			},
+			wantErr: false,
+			checkResult: func(newName string, args args) bool {
+				return newName == "aname"
+			},
+		},
+	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctrl := gomock.NewController(t)

diff --git a/pkg/init/init_test.go b/pkg/init/init_test.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/golang/mock/gomock"
+
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/preference"
 	"github.com/redhat-developer/odo/pkg/registry"

diff --git a/pkg/odo/cli/init/init.go b/pkg/odo/cli/init/init.go
@@ -248,7 +248,7 @@ func NewCmdInit(name, fullName string) *cobra.Command {
 	}
 	clientset.Add(initCmd, clientset.PREFERENCE, clientset.FILESYSTEM, clientset.REGISTRY, clientset.INIT)
 
-	initCmd.Flags().String(backend.FLAG_NAME, "", "name of the component to create")
+	initCmd.Flags().String(backend.FLAG_NAME, "", "name of the component to create; it must follow the RFC 1123 Label Names standard and not be all-numeric")
 	initCmd.Flags().String(backend.FLAG_DEVFILE, "", "name of the devfile in devfile registry")
 	initCmd.Flags().String(backend.FLAG_DEVFILE_REGISTRY, "", "name of the devfile registry (as configured in \"odo preference view\"). It can be used in combination with --devfile, but not with --devfile-path")
 	initCmd.Flags().String(backend.FLAG_STARTER, "", "name of the starter project")

diff --git a/tests/integration/cmd_devfile_init_test.go b/tests/integration/cmd_devfile_init_test.go
@@ -35,6 +35,10 @@ var _ = Describe("odo devfile init command tests", func() {
 			helper.Cmd("odo", "init", "--name", "aname").ShouldFail()
 		})
 
+		By("using an invalid component name", func() {
+			helper.Cmd("odo", "init", "--devfile", "go", "--name", "123").ShouldFail()
+		})
+
 		By("running odo init with json and no other flags", func() {
 			res := helper.Cmd("odo", "init", "-o", "json").ShouldFail()
 			stdout, stderr := res.Out(), res.Err()

diff --git a/tests/integration/interactive_init_test.go b/tests/integration/interactive_init_test.go
@@ -61,6 +61,33 @@ var _ = Describe("odo init interactive command tests", func() {
 		Expect(helper.ListFilesInDir(commonVar.Context)).To(ContainElements("devfile.yaml"))
 	})
 
+	It("should ask to re-enter the component name when an invalid value is passed", func() {
+		command := []string{"odo", "init"}
+		_, err := helper.RunInteractive(command, nil, func(ctx helper.InteractiveContext) {
+
+			helper.ExpectString(ctx, "Select language")
+			helper.SendLine(ctx, "go")
+
+			helper.ExpectString(ctx, "Select project type")
+			helper.SendLine(ctx, "")
+
+			helper.ExpectString(ctx, "Which starter project do you want to use")
+			helper.SendLine(ctx, "")
+
+			helper.ExpectString(ctx, "Enter component name")
+			helper.SendLine(ctx, "myapp-<script>alert('Injected!');</script>")
+
+			helper.ExpectString(ctx, "name \"myapp-<script>alert('Injected!');</script>\" is not valid, name should conform the following requirements")
+
+			helper.ExpectString(ctx, "Enter component name")
+			helper.SendLine(ctx, "my-go-app")
+
+			helper.ExpectString(ctx, "Your new component 'my-go-app' is ready in the current directory")
+		})
+		Expect(err).To(BeNil())
+		Expect(helper.ListFilesInDir(commonVar.Context)).To(ContainElements("devfile.yaml"))
+	})
+
 	It("should download correct devfile", func() {
 
 		command := []string{"odo", "init"}
@@ -262,6 +289,40 @@ var _ = Describe("odo init interactive command tests", func() {
 				Expect(lines[len(lines)-1]).To(Equal(fmt.Sprintf("Your new component '%s' is ready in the current directory", projectName)))
 
 			})
+			It("should ask to re-enter the component name if invalid value is passed by the user", func() {
+				language := "javascript"
+				projectType := "nodejs"
+				projectName := "node-echo"
+
+				_, err := helper.RunInteractive([]string{"odo", "init"}, nil, func(ctx helper.InteractiveContext) {
+					helper.ExpectString(ctx, "Based on the files in the current directory odo detected")
+
+					helper.ExpectString(ctx, fmt.Sprintf("Language: %s", language))
+
+					helper.ExpectString(ctx, fmt.Sprintf("Project type: %s", projectType))
+
+					helper.ExpectString(ctx,
+						fmt.Sprintf("The devfile \"%s\" from the registry \"DefaultDevfileRegistry\" will be downloaded.", projectType))
+
+					helper.ExpectString(ctx, "Is this correct")
+					helper.SendLine(ctx, "")
+
+					helper.ExpectString(ctx, "Select container for which you want to change configuration")
+					helper.SendLine(ctx, "")
+
+					helper.ExpectString(ctx, "Enter component name")
+					helper.SendLine(ctx, "myapp-<script>alert('Injected!');</script>")
+
+					helper.ExpectString(ctx, "name \"myapp-<script>alert('Injected!');</script>\" is not valid, name should conform the following requirements")
+
+					helper.ExpectString(ctx, "Enter component name")
+					helper.SendLine(ctx, "")
+
+					helper.ExpectString(ctx, fmt.Sprintf("Your new component '%s' is ready in the current directory", projectName))
+				})
+				Expect(err).To(BeNil())
+				Expect(helper.ListFilesInDir(commonVar.Context)).To(ContainElements("devfile.yaml"))
+			})
 		})
 	})