Added a check to the domain commands for extra positional arguments. (#…

…6658)

What changed?
Added a check to error out when positional arguments are specified in domain commands.

Why?
In this pr: #6552 we fixed the behaviour of the --cl c1 c2 argument in the domain commands to be instead --cl c1,c2. This was needed due to the update of the urfave CLI framework to v2 (see the PR for more info).

In much of our documentation we still mention that the clusters in the domain commands should be specified as --cl c1 c2 however as mention this no longer works.

Before this PR, anything after the first positional argument was ignored, so the command cadence --do foo domain register --cl c1 c2 --st securityToken was interpreted as cadence --do foo domain register --cl c1  with c2 --st securityToken as ignored positional arguments. This command will error out with "No permission to do this operation." which is very misleading.

The new error will make the user immediately aware of the problem and direct them to the help documentation.

How did you test it?

Potential risks

Release notes

Documentation Changes
Yes wee should fix documentation, at least in these two places:

https://cadenceworkflow.io/docs/cli
https://cadenceworkflow.io/docs/concepts/cross-dc-replication

tools/cli/domain.go

@@ -22,6 +22,7 @@ package cli
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/urfave/cli/v2"
 
@@ -47,6 +48,13 @@ func checkRequiredDomainDataKVs(domainData map[string]string) error {
 	return nil
 }
 
+func checkNoAdditionalArgsPassed(c *cli.Context) error {
+	if c.NArg() > 0 {
+		return fmt.Errorf("Domain commands cannot have arguments: <%v>\nClusters are now specified as --clusters c1,c2 see help for more info", strings.Join(c.Args().Slice(), " "))
+	}
+	return nil
+}
+
 func newDomainCommands() []*cli.Command {
 	return []*cli.Command{
 		{
@@ -55,6 +63,10 @@ func newDomainCommands() []*cli.Command {
 			Usage:   "Register workflow domain",
 			Flags:   registerDomainFlags,
 			Action: func(c *cli.Context) error {
+				err := checkNoAdditionalArgsPassed(c)
+				if err != nil {
+					return err
+				}
 				return withDomainClient(c, false, func(dc *domainCLIImpl) error {
 					return dc.RegisterDomain(c)
 				})
@@ -66,6 +78,10 @@ func newDomainCommands() []*cli.Command {
 			Usage:   "Update existing workflow domain",
 			Flags:   updateDomainFlags,
 			Action: func(c *cli.Context) error {
+				err := checkNoAdditionalArgsPassed(c)
+				if err != nil {
+					return err
+				}
 				return withDomainClient(c, false, func(dc *domainCLIImpl) error {
 					return dc.UpdateDomain(c)
 				})
@@ -77,6 +93,10 @@ func newDomainCommands() []*cli.Command {
 			Usage:   "Deprecate existing workflow domain",
 			Flags:   deprecateDomainFlags,
 			Action: func(c *cli.Context) error {
+				err := checkNoAdditionalArgsPassed(c)
+				if err != nil {
+					return err
+				}
 				return withDomainClient(c, false, func(dc *domainCLIImpl) error {
 					return dc.DeprecateDomain(c)
 				})
@@ -88,6 +108,10 @@ func newDomainCommands() []*cli.Command {
 			Usage:   "Describe existing workflow domain",
 			Flags:   describeDomainFlags,
 			Action: func(c *cli.Context) error {
+				err := checkNoAdditionalArgsPassed(c)
+				if err != nil {
+					return err
+				}
 				return withDomainClient(c, false, func(dc *domainCLIImpl) error {
 					return dc.DescribeDomain(c)
 				})
@@ -99,9 +123,13 @@ func newDomainCommands() []*cli.Command {
 			Usage:   "Migrate existing domain to new domain. This command only validates the settings. It does not perform actual data migration",
 			Flags:   migrateDomainFlags,
 			Action: func(c *cli.Context) error {
+				err := checkNoAdditionalArgsPassed(c)
+				if err != nil {
+					return err
+				}
 				// exit on error already handled in the command
 				// TODO best practice is to return error if validation fails
-				err := NewDomainMigrationCommand(c).Validation(c)
+				err = NewDomainMigrationCommand(c).Validation(c)
 				if err != nil {
 					return commoncli.Problem("Failed validation: ", err)
 				}

tools/cli/domain_commands_test.go

@@ -129,6 +129,18 @@ func (s *cliAppSuite) TestDomainRegister() {
 			"option domain is required",
 			nil,
 		},
+		{
+			"fail on extra arguments at end",
+			"cadence --do test-domain domain register --global_domain true --retention 5 --desc description --active_cluster c1 --clusters c1,c2 unused_arg",
+			"Domain commands cannot have arguments: <unused_arg>\nClusters are now specified as --clusters c1,c2 see help for more info",
+			nil,
+		},
+		{
+			"fail on extra arguments at in command",
+			"cadence --do test-domain domain register --global_domain true --retention 5 --desc description --active_cluster c1 unused_arg --clusters c1,c2",
+			"Domain commands cannot have arguments: <unused_arg --clusters c1,c2>\nClusters are now specified as --clusters c1,c2 see help for more info",
+			nil,
+		},
 		{
 			"invalid global domain flag",
 			"cadence --do test-domain domain register --global_domain invalid",