etcdctl/ctlv3: fix watch with exec commands, interactive mode flag parsing

.gitignore

@@ -9,6 +9,7 @@
 *.log
 /etcd
 /hack/insta-discovery/.env
+*.coverprofile
 *.test
 hack/tls-setup/certs
 .idea

Makefile

@@ -20,9 +20,12 @@ clean:
 	rm -f ./codecov
 	rm -rf ./agent-*
 	rm -rf ./covdir
+	rm -f ./*.coverprofile
 	rm -f ./*.log
 	rm -f ./bin/Dockerfile-release
 	rm -rf ./bin/*.etcd
+	rm -rf ./default.etcd
+	rm -rf ./tests/e2e/default.etcd
 	rm -rf ./gopath
 	rm -rf ./gopath.proto
 	rm -rf ./release

etcdctl/ctlv3/command/watch_command.go

@@ -179,118 +179,159 @@ func printWatchCh(c *clientv3.Client, ch clientv3.WatchChan, execArgs []string)
 // "--" characters are invalid arguments for "spf13/cobra" library,
 // so no need to handle such cases.
 func parseWatchArgs(osArgs, commandArgs []string, envKey, envRange string, interactive bool) (watchArgs []string, execArgs []string, err error) {
-	watchArgs = commandArgs
+	rawArgs := make([]string, len(osArgs))
+	copy(rawArgs, osArgs)
+	watchArgs = make([]string, len(commandArgs))
+	copy(watchArgs, commandArgs)
 
-	// remove preceding commands (e.g. "watch foo bar" in interactive mode)
-	idx := 0
-	for idx = range watchArgs {
-		if watchArgs[idx] == "watch" {
+	// remove preceding commands (e.g. ./bin/etcdctl watch)
+	// handle "./bin/etcdctl watch foo -- echo watch event"
+	for idx := range rawArgs {
+		if rawArgs[idx] == "watch" {
+			rawArgs = rawArgs[idx+1:]
 			break
 		}
 	}
-	if idx < len(watchArgs)-1 || envKey != "" {
-		if idx < len(watchArgs)-1 {
-			watchArgs = watchArgs[idx+1:]
+
+	// remove preceding commands (e.g. "watch foo bar" in interactive mode)
+	// handle "./bin/etcdctl watch foo -- echo watch event"
+	if interactive {
+		if watchArgs[0] != "watch" {
+			// "watch" not found
+			watchPrefix, watchRev, watchPrevKey = false, 0, false
+			return nil, nil, errBadArgsInteractiveWatch
 		}
+		watchArgs = watchArgs[1:]
+	}
 
-		execIdx, execExist := 0, false
-		for execIdx = range osArgs {
-			v := osArgs[execIdx]
-			if v == "--" && execIdx != len(osArgs)-1 {
+	execIdx, execExist := 0, false
+	if !interactive {
+		for execIdx = range rawArgs {
+			if rawArgs[execIdx] == "--" {
 				execExist = true
 				break
 			}
 		}
-
-		if idx == len(watchArgs)-1 && envKey != "" {
-			if len(watchArgs) > 0 && !interactive {
-				// "watch --rev 1 -- echo Hello World" has no conflict
-				if !execExist {
-					// "watch foo" with ETCDCTL_WATCH_KEY=foo
-					// (watchArgs==["foo"])
-					return nil, nil, errBadArgsNumConflictEnv
-				}
-			}
-			// otherwise, watch with no argument and environment key is set
-			// if interactive, first "watch" command string should be removed
-			if interactive {
-				watchArgs = []string{}
-			}
+		if execExist && execIdx == len(rawArgs)-1 {
+			// "watch foo bar --" should error
+			return nil, nil, errBadArgsNumSeparator
 		}
-
-		// "watch foo -- echo hello" with ETCDCTL_WATCH_KEY=foo
-		// (watchArgs==["foo","echo","hello"])
-		if envKey != "" && execExist {
-			widx, oidx := 0, len(osArgs)-1
-			for widx = len(watchArgs) - 1; widx >= 0; widx-- {
-				if watchArgs[widx] == osArgs[oidx] {
-					oidx--
+		// "watch" with no argument should error
+		if !execExist && len(rawArgs) < 1 && envKey == "" {
+			return nil, nil, errBadArgsNum
+		}
+		if execExist && envKey != "" {
+			// "ETCDCTL_WATCH_KEY=foo watch foo -- echo 1" should error
+			// (watchArgs==["foo","echo","1"])
+			widx, ridx := len(watchArgs)-1, len(rawArgs)-1
+			for ; widx >= 0; widx-- {
+				if watchArgs[widx] == rawArgs[ridx] {
+					ridx--
 					continue
 				}
-				if oidx == execIdx { // watchArgs has extra
+				// watchArgs has extra:
+				// ETCDCTL_WATCH_KEY=foo watch foo  --  echo 1
+				// watchArgs:                       foo echo 1
+				if ridx == execIdx {
 					return nil, nil, errBadArgsNumConflictEnv
 				}
 			}
 		}
-	} else if interactive { // "watch" not found
-		return nil, nil, errBadArgsInteractiveWatch
-	}
-	if len(watchArgs) < 1 && envKey == "" {
-		return nil, nil, errBadArgsNum
-	}
+		// check conflicting arguments
+		// e.g. "watch --rev 1 -- echo Hello World" has no conflict
+		if !execExist && len(watchArgs) > 0 && envKey != "" {
+			// "ETCDCTL_WATCH_KEY=foo watch foo" should error
+			// (watchArgs==["foo"])
+			return nil, nil, errBadArgsNumConflictEnv
+		}
+	} else {
+		for execIdx = range watchArgs {
+			if watchArgs[execIdx] == "--" {
+				execExist = true
+				break
+			}
+		}
+		if execExist && execIdx == len(watchArgs)-1 {
+			// "watch foo bar --" should error
+			watchPrefix, watchRev, watchPrevKey = false, 0, false
+			return nil, nil, errBadArgsNumSeparator
+		}
 
-	// remove preceding commands (e.g. ./bin/etcdctl watch)
-	for idx = range osArgs {
-		if osArgs[idx] == "watch" {
-			break
+		flagset := NewWatchCommand().Flags()
+		if err := flagset.Parse(watchArgs); err != nil {
+			watchPrefix, watchRev, watchPrevKey = false, 0, false
+			return nil, nil, err
+		}
+		pArgs := flagset.Args()
+
+		// "watch" with no argument should error
+		if !execExist && envKey == "" && len(pArgs) < 1 {
+			watchPrefix, watchRev, watchPrevKey = false, 0, false
+			return nil, nil, errBadArgsNum
+		}
+		// check conflicting arguments
+		// e.g. "watch --rev 1 -- echo Hello World" has no conflict
+		if !execExist && len(pArgs) > 0 && envKey != "" {
+			// "ETCDCTL_WATCH_KEY=foo watch foo" should error
+			// (watchArgs==["foo"])
+			watchPrefix, watchRev, watchPrevKey = false, 0, false
+			return nil, nil, errBadArgsNumConflictEnv
 		}
-	}
-	if idx < len(osArgs)-1 {
-		osArgs = osArgs[idx+1:]
-	} else if envKey == "" {
-		return nil, nil, errBadArgsNum
 	}
 
-	argsWithSep := osArgs
-	if interactive { // interactive mode pass "--" to the command args
+	argsWithSep := rawArgs
+	if interactive {
+		// interactive mode directly passes "--" to the command args
 		argsWithSep = watchArgs
 	}
-	foundSep := false
+
+	idx, foundSep := 0, false
 	for idx = range argsWithSep {
 		if argsWithSep[idx] == "--" {
 			foundSep = true
 			break
 		}
 	}
+	if foundSep {
+		execArgs = argsWithSep[idx+1:]
+	}
+
 	if interactive {
 		flagset := NewWatchCommand().Flags()
 		if err := flagset.Parse(argsWithSep); err != nil {
 			return nil, nil, err
 		}
 		watchArgs = flagset.Args()
+
+		watchPrefix, err = flagset.GetBool("prefix")
+		if err != nil {
+			return nil, nil, err
+		}
+		watchRev, err = flagset.GetInt64("rev")
+		if err != nil {
+			return nil, nil, err
+		}
+		watchPrevKey, err = flagset.GetBool("prev-kv")
+		if err != nil {
+			return nil, nil, err
+		}
 	}
 
-	// "watch -- echo hello" with ETCDCTL_WATCH_KEY=foo
-	// should be translated to "watch foo -- echo hello"
+	// "ETCDCTL_WATCH_KEY=foo watch -- echo hello"
+	// should translate "watch foo -- echo hello"
 	// (watchArgs=["echo","hello"] should be ["foo","echo","hello"])
 	if envKey != "" {
-		tmp := []string{envKey}
+		ranges := []string{envKey}
 		if envRange != "" {
-			tmp = append(tmp, envRange)
+			ranges = append(ranges, envRange)
 		}
-		watchArgs = append(tmp, watchArgs...)
+		watchArgs = append(ranges, watchArgs...)
 	}
 
 	if !foundSep {
 		return watchArgs, nil, nil
 	}
 
-	if idx == len(argsWithSep)-1 {
-		// "watch foo bar --" should error
-		return nil, nil, errBadArgsNumSeparator
-	}
-	execArgs = argsWithSep[idx+1:]
-
 	// "watch foo bar --rev 1 -- echo hello" or "watch foo --rev 1 bar -- echo hello",
 	// then "watchArgs" is "foo bar echo hello"
 	// so need ignore args after "argsWithSep[idx]", which is "--"