From 317c79cf967ddcf18866e33b2bce6261dd793924 Mon Sep 17 00:00:00 2001 From: Krasimir Kargov Date: Wed, 13 Aug 2025 23:26:21 +0300 Subject: [PATCH 1/4] Adding displaying of flags when using unknown flags for a command LMCROSSITXSADEPLOY-1100 --- commands/deploy_command_test.go | 39 ++++++++++- commands/download_mta_op_logs_command_test.go | 13 +++- commands/flags_parser.go | 65 ++++++++++++++++++- commands/mta_command_test.go | 11 ++++ commands/mta_operations_command_test.go | 16 ++++- commands/mtas_command_test.go | 15 ++++- commands/purge_config_command_test.go | 12 +++- commands/undeploy_command_test.go | 2 +- 8 files changed, 162 insertions(+), 11 deletions(-) diff --git a/commands/deploy_command_test.go b/commands/deploy_command_test.go index 75a8b47..c7ed992 100644 --- a/commands/deploy_command_test.go +++ b/commands/deploy_command_test.go @@ -213,7 +213,7 @@ var _ = Describe("DeployCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"x", "-l"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -l") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) @@ -223,7 +223,7 @@ var _ = Describe("DeployCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{mtaArchivePath, "-l"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -l") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) @@ -276,7 +276,40 @@ var _ = Describe("DeployCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{mtaArchivePath, "--strategy"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Parsing of arguments has failed") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // more than one unknown flag and one valid flag, which is a boolean, but its value is missing + Context("with argument that is a directory or MTA and with more than one unknown flag and one valid flag, which is a boolean, but its value is missing", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "--nonValidFlag", "--apply-namespace-app-names", "-notAValidOne", "-not-a-valid-one"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag, -notAValidOne, -not-a-valid-one") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // more than one unknown flag and one valid flag, which is a boolean and its value is set + Context("with argument that is a directory or MTA and with more than one unknown flag and one valid flag, which is a boolean and its value is set", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "--nonValidFlag", "--apply-namespace-app-names", "true", "-notAValidOne", "-not-a-valid-one"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag, -notAValidOne, -not-a-valid-one") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // more than one unknown flag and one valid flag that is not boolean and has a value set + Context("with argument that is a directory or MTA and with more than one unknown flag and one valid flag that is not boolean and has a value set", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "--nonValidFlag", "--apps-stage-timeout", "8000", "-notAValidOne", "-inavalid-flag"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag, -notAValidOne, -inavalid-flag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/download_mta_op_logs_command_test.go b/commands/download_mta_op_logs_command_test.go index 54f58e5..e246b6c 100644 --- a/commands/download_mta_op_logs_command_test.go +++ b/commands/download_mta_op_logs_command_test.go @@ -88,7 +88,18 @@ var _ = Describe("DownloadMtaOperationLogsCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // with an unknown flag and one valid flag + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-a", "--mta"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/flags_parser.go b/commands/flags_parser.go index 84bb25f..5f817dd 100644 --- a/commands/flags_parser.go +++ b/commands/flags_parser.go @@ -24,6 +24,10 @@ func NewCommandFlagsParser(flag *flag.FlagSet, parser FlagsParser, validator Fla // Parse parsing the args func (p *CommandFlagsParser) Parse(args []string) error { + if unknownFlags := collectUnknownFlags(p.flag, args); len(unknownFlags) > 0 { + return fmt.Errorf("Unknown or wrong flags: %s", strings.Join(unknownFlags, ", ")) + } + err := p.parser.ParseFlags(p.flag, args) if err != nil { return err @@ -88,7 +92,7 @@ func (p DefaultCommandFlagsParser) ParseFlags(flags *flag.FlagSet, args []string // Parse the arguments err := flags.Parse(args[positionalArgsCount:]) if err != nil { - return errors.New("Unknown or wrong flag") + return errors.New("Parsing of arguments has failed") } // Check for wrong arguments @@ -124,3 +128,62 @@ func (v DefaultCommandFlagsValidator) ValidateParsedFlags(flags *flag.FlagSet) e return nil } + +func collectUnknownFlags(flags *flag.FlagSet, args []string) []string { + var unknownFlags []string + + for i := 0; i < len(args); i++ { + currentArgument := args[i] + + if !strings.HasPrefix(currentArgument, "-") { + continue + } + + currentFlag := currentArgument + flagName := strings.TrimLeft(currentFlag, "-") + + if flagName == "" { + continue + } + + isFlagKnown := flags.Lookup(flagName) + if isFlagKnown != nil { + nextIndex := i + 1 + if nextIndex < len(args) { + isBoolean := isBoolFlag(isFlagKnown) + if !isBoolean { + nextArgument := args[nextIndex] + nextHasPrefixDash := strings.HasPrefix(nextArgument, "-") + if !nextHasPrefixDash { + i = nextIndex + } + } + } + continue + } + + unknownFlags = append(unknownFlags, currentFlag) + + nextIndex := i + 1 + if nextIndex < len(args) { + nextArgument := args[nextIndex] + nextHasPrefixDash := strings.HasPrefix(nextArgument, "-") + if !nextHasPrefixDash { + i = nextIndex + } + } + } + + return unknownFlags +} + +func isBoolFlag(flag *flag.Flag) bool { + type boolFlagInterface interface{ IsBoolFlag() bool } + + boolFlag, isInterfaceImplemented := flag.Value.(boolFlagInterface) + if !isInterfaceImplemented { + return false + } + + return boolFlag.IsBoolFlag() +} diff --git a/commands/mta_command_test.go b/commands/mta_command_test.go index d6fa0d3..cc56d3e 100644 --- a/commands/mta_command_test.go +++ b/commands/mta_command_test.go @@ -90,6 +90,17 @@ var _ = Describe("MtaCommand", func() { }) }) + // with unknown flags and one valid flag - error + Context("with unknown flags and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"--nonValidFlag", "--namespace", "-u"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + // can't connect to backend - error Context("when can't connect to backend", func() { const host = "x" diff --git a/commands/mta_operations_command_test.go b/commands/mta_operations_command_test.go index 008f257..9721d8e 100644 --- a/commands/mta_operations_command_test.go +++ b/commands/mta_operations_command_test.go @@ -56,17 +56,29 @@ var _ = Describe("MtaOperationsCommand", func() { command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200), clientFactory, testTokenFactory, deployServiceURLCalculator) }) + // with an unknown flag - error Context("with an unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) - // // wrong arguments + // with an unknown flag and one valid flag + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-a", "--last"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // wrong arguments Context("with wrong arguments", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { diff --git a/commands/mtas_command_test.go b/commands/mtas_command_test.go index 6397f75..4c216b2 100644 --- a/commands/mtas_command_test.go +++ b/commands/mtas_command_test.go @@ -60,13 +60,24 @@ var _ = Describe("MtasCommand", func() { command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200), clientFactory, testTokenFactory, deployServiceURLCalculator) }) - // unknown flag - error + // with an unknown flag - error Context("with an unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // with an unknown flag and one valid flag + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-nonValidFlag", "-u"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -nonValidFlag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/purge_config_command_test.go b/commands/purge_config_command_test.go index 90d367c..30f0b57 100644 --- a/commands/purge_config_command_test.go +++ b/commands/purge_config_command_test.go @@ -54,7 +54,17 @@ var _ = Describe("PurgeConfigCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-a", "-u"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/undeploy_command_test.go b/commands/undeploy_command_test.go index 8c5f16b..0e64ae1 100644 --- a/commands/undeploy_command_test.go +++ b/commands/undeploy_command_test.go @@ -101,7 +101,7 @@ var _ = Describe("UndeployCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"test-mta-id", "-unknown-flag"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -unknown-flag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) From 3682bcf9feea330c8fb70c77e52fa6a9a14f15ba Mon Sep 17 00:00:00 2001 From: Krasimir Kargov Date: Thu, 14 Aug 2025 13:53:15 +0300 Subject: [PATCH 2/4] Adding formatting fixes and changing logic so no duplicates are present at the end result LMCROSSITXSADEPLOY-1100 --- commands/flags_parser.go | 45 ++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/commands/flags_parser.go b/commands/flags_parser.go index 5f817dd..7d24cb9 100644 --- a/commands/flags_parser.go +++ b/commands/flags_parser.go @@ -131,6 +131,7 @@ func (v DefaultCommandFlagsValidator) ValidateParsedFlags(flags *flag.FlagSet) e func collectUnknownFlags(flags *flag.FlagSet, args []string) []string { var unknownFlags []string + alreadySeenUnknownFLags := make(map[string]int) for i := 0; i < len(args); i++ { currentArgument := args[i] @@ -148,30 +149,15 @@ func collectUnknownFlags(flags *flag.FlagSet, args []string) []string { isFlagKnown := flags.Lookup(flagName) if isFlagKnown != nil { - nextIndex := i + 1 - if nextIndex < len(args) { - isBoolean := isBoolFlag(isFlagKnown) - if !isBoolean { - nextArgument := args[nextIndex] - nextHasPrefixDash := strings.HasPrefix(nextArgument, "-") - if !nextHasPrefixDash { - i = nextIndex - } - } + isBoolean := isBoolFlag(isFlagKnown) + if !isBoolean { + i = tryToGetNext(args, i) } continue } - unknownFlags = append(unknownFlags, currentFlag) - - nextIndex := i + 1 - if nextIndex < len(args) { - nextArgument := args[nextIndex] - nextHasPrefixDash := strings.HasPrefix(nextArgument, "-") - if !nextHasPrefixDash { - i = nextIndex - } - } + appendOnlyWhenCountIsOne(alreadySeenUnknownFLags, currentFlag, &unknownFlags) + i = tryToGetNext(args, i) } return unknownFlags @@ -187,3 +173,22 @@ func isBoolFlag(flag *flag.Flag) bool { return boolFlag.IsBoolFlag() } + +func tryToGetNext(args []string, currentIndex int) int { + nextIndex := currentIndex + 1 + if nextIndex < len(args) { + nextArgument := args[nextIndex] + nextHasPrefixDash := strings.HasPrefix(nextArgument, "-") + if !nextHasPrefixDash { + return nextIndex + } + } + return currentIndex +} + +func appendOnlyWhenCountIsOne(alreadySeenUnknownFLags map[string]int, currentFlag string, unknownFlags *[]string) { + alreadySeenUnknownFLags[currentFlag]++ + if alreadySeenUnknownFLags[currentFlag] == 1 { + *unknownFlags = append(*unknownFlags, currentFlag) + } +} From 0ba0f86ea1cf67bbe09c56c5202b5f913c461345 Mon Sep 17 00:00:00 2001 From: Krasimir Kargov Date: Thu, 14 Aug 2025 14:06:00 +0300 Subject: [PATCH 3/4] Adding a test with duplicated unknown flags LMCROSSITXSADEPLOY-1100 --- commands/deploy_command_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/commands/deploy_command_test.go b/commands/deploy_command_test.go index c7ed992..b35a425 100644 --- a/commands/deploy_command_test.go +++ b/commands/deploy_command_test.go @@ -207,8 +207,8 @@ var _ = Describe("DeployCommand", func() { command.FileUrlReadTimeout = time.Second }) - // unknown flag - error - Context("with argument that is not a directory or MTA", func() { + // with argument that is not a directory or MTA and a valid and unknown flag (unknown flag - error) + Context("with argument that is not a directory or MTA and a valid and unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"x", "-l"}).ToInt() @@ -218,6 +218,7 @@ var _ = Describe("DeployCommand", func() { }) }) + // with argument that is a directory or MTA and with unknown flag (unknown flag - error) Context("with argument that is a directory or MTA and with unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { @@ -228,6 +229,17 @@ var _ = Describe("DeployCommand", func() { }) }) + // with argument that is a directory or MTA and with unknown flags - some even duplicated (unknown flag - error) + Context("with argument that is a directory or MTA and with unknown flags - some even duplicated", func() { + It("should print incorrect usage (and print the duplicating flags only once), call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "-nonValidFlag", "-u", "-nonValidFlag", "-nonValidFlag", "--flagInvalid"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -nonValidFlag, --flagInvalid") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + Context("with a correct URL argument", func() { It("should upload the MTAR from the correct URL and initiate a deploy", func() { command.FileUrlReader = newMockFileReader(correctMtaUrl) From 4f5cd67b61d3f6c09fd5164ece9fd7d9cf4ecc63 Mon Sep 17 00:00:00 2001 From: Krasimir Kargov Date: Thu, 14 Aug 2025 17:22:37 +0300 Subject: [PATCH 4/4] Changing place of execution and fixing failing test LMCROSSITXSADEPLOY-1100 --- commands/flags_parser.go | 10 +++++----- commands/mta_command_test.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/commands/flags_parser.go b/commands/flags_parser.go index 7d24cb9..57d55bd 100644 --- a/commands/flags_parser.go +++ b/commands/flags_parser.go @@ -24,10 +24,6 @@ func NewCommandFlagsParser(flag *flag.FlagSet, parser FlagsParser, validator Fla // Parse parsing the args func (p *CommandFlagsParser) Parse(args []string) error { - if unknownFlags := collectUnknownFlags(p.flag, args); len(unknownFlags) > 0 { - return fmt.Errorf("Unknown or wrong flags: %s", strings.Join(unknownFlags, ", ")) - } - err := p.parser.ParseFlags(p.flag, args) if err != nil { return err @@ -92,7 +88,11 @@ func (p DefaultCommandFlagsParser) ParseFlags(flags *flag.FlagSet, args []string // Parse the arguments err := flags.Parse(args[positionalArgsCount:]) if err != nil { - return errors.New("Parsing of arguments has failed") + if unknownFlags := collectUnknownFlags(flags, args); len(unknownFlags) > 0 { + return fmt.Errorf("Unknown or wrong flags: %s", strings.Join(unknownFlags, ", ")) + } else { + return errors.New("Parsing of arguments has failed") + } } // Check for wrong arguments diff --git a/commands/mta_command_test.go b/commands/mta_command_test.go index cc56d3e..d6bbd59 100644 --- a/commands/mta_command_test.go +++ b/commands/mta_command_test.go @@ -94,7 +94,7 @@ var _ = Describe("MtaCommand", func() { Context("with unknown flags and one valid flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { - return command.Execute([]string{"--nonValidFlag", "--namespace", "-u"}).ToInt() + return command.Execute([]string{"fakeMta", "--nonValidFlag", "--namespace", "-u"}).ToInt() }) ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name}))