From 1e2df9e440a26a4eab6aee95b909f3419e4f87fb Mon Sep 17 00:00:00 2001 From: dereknola Date: Tue, 2 Nov 2021 11:22:06 -0700 Subject: [PATCH 1/4] Match to last After keyword for parser Signed-off-by: dereknola Signed-off-by: Derek Nola --- pkg/configfilearg/defaultparser.go | 2 +- pkg/configfilearg/parser.go | 7 ++++++- pkg/configfilearg/parser_test.go | 9 ++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/configfilearg/defaultparser.go b/pkg/configfilearg/defaultparser.go index 3b17b95115ea..b0395862e9d9 100644 --- a/pkg/configfilearg/defaultparser.go +++ b/pkg/configfilearg/defaultparser.go @@ -7,7 +7,7 @@ import ( func MustParse(args []string) []string { parser := &Parser{ - After: []string{"server", "agent", "etcd-snapshot"}, + After: []string{"server", "agent", "etcd-snapshot", "save", "delete", "list", "prune"}, FlagNames: []string{"--config", "-c"}, EnvName: version.ProgramUpper + "_CONFIG_FILE", DefaultConfig: "/etc/rancher/" + version.Program + "/config.yaml", diff --git a/pkg/configfilearg/parser.go b/pkg/configfilearg/parser.go index 7994ca37ac5b..8b1458c1a1d7 100644 --- a/pkg/configfilearg/parser.go +++ b/pkg/configfilearg/parser.go @@ -101,13 +101,18 @@ func (p *Parser) findStart(args []string) ([]string, []string, bool) { return []string{}, args, true } + // The last After keyword found will be the split point + lastIndex := -1 for i, val := range args { for _, test := range p.After { if val == test { - return args[0 : i+1], args[i+1:], true + lastIndex = i } } } + if lastIndex != -1 { + return args[0 : lastIndex+1], args[lastIndex+1:], true + } return args, nil, false } diff --git a/pkg/configfilearg/parser_test.go b/pkg/configfilearg/parser_test.go index 18128be350cd..1ff6a5f1f7e2 100644 --- a/pkg/configfilearg/parser_test.go +++ b/pkg/configfilearg/parser_test.go @@ -48,11 +48,18 @@ func Test_UnitParser_findStart(t *testing.T) { prefix: []string{"not-server", "foo", "bar"}, found: false, }, + { + name: "comamnds and subcommands that both match", + args: []string{"etcd-snapshot", "list", "foo", "bar"}, + prefix: []string{"etcd-snapshot", "list"}, + suffix: []string{"foo", "bar"}, + found: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := Parser{ - After: []string{"server", "agent"}, + After: []string{"server", "agent", "etcd-snapshot", "save", "delete", "list", "prune"}, } prefix, suffix, found := p.findStart(tt.args) if !reflect.DeepEqual(prefix, tt.prefix) { From fbc0683aa9c6b539c73da32104dd51524f0bbfe6 Mon Sep 17 00:00:00 2001 From: Derek Nola Date: Wed, 3 Nov 2021 10:56:05 -0700 Subject: [PATCH 2/4] Made parser able to skip over subcommands Signed-off-by: Derek Nola --- pkg/configfilearg/defaultparser.go | 2 +- pkg/configfilearg/parser.go | 29 ++++++++++++++++++++++------- pkg/configfilearg/parser_test.go | 11 +++++++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/pkg/configfilearg/defaultparser.go b/pkg/configfilearg/defaultparser.go index b0395862e9d9..d81b9ea52d73 100644 --- a/pkg/configfilearg/defaultparser.go +++ b/pkg/configfilearg/defaultparser.go @@ -7,7 +7,7 @@ import ( func MustParse(args []string) []string { parser := &Parser{ - After: []string{"server", "agent", "etcd-snapshot", "save", "delete", "list", "prune"}, + After: []string{"server", "agent", "etcd-snapshot:1"}, FlagNames: []string{"--config", "-c"}, EnvName: version.ProgramUpper + "_CONFIG_FILE", DefaultConfig: "/etc/rancher/" + version.Program + "/config.yaml", diff --git a/pkg/configfilearg/parser.go b/pkg/configfilearg/parser.go index 8b1458c1a1d7..d60912a62806 100644 --- a/pkg/configfilearg/parser.go +++ b/pkg/configfilearg/parser.go @@ -7,6 +7,8 @@ import ( "net/url" "os" "path/filepath" + "regexp" + "strconv" "strings" "github.com/rancher/k3s/pkg/agent/util" @@ -101,19 +103,32 @@ func (p *Parser) findStart(args []string) ([]string, []string, bool) { return []string{}, args, true } - // The last After keyword found will be the split point - lastIndex := -1 + afterIndex := make(map[string]int) + re, err := regexp.Compile(`:\d`) + if err != nil { + return args, nil, false + } + // After keywords ending with ":" will set + NUM of arguments as the split point. + // used for matching on subcommmands + for i, arg := range p.After { + if re.MatchString(arg) { + split := strings.Split(arg, ":") + p.After[i] = split[0] + afterIndex[split[0]], err = strconv.Atoi(split[1]) + if err != nil { + return args, nil, false + } + } + } + for i, val := range args { for _, test := range p.After { if val == test { - lastIndex = i + skip := afterIndex[test] + 1 + return args[0 : i+skip], args[i+skip:], true } } } - if lastIndex != -1 { - return args[0 : lastIndex+1], args[lastIndex+1:], true - } - return args, nil, false } diff --git a/pkg/configfilearg/parser_test.go b/pkg/configfilearg/parser_test.go index 1ff6a5f1f7e2..27390d043b06 100644 --- a/pkg/configfilearg/parser_test.go +++ b/pkg/configfilearg/parser_test.go @@ -49,17 +49,24 @@ func Test_UnitParser_findStart(t *testing.T) { found: false, }, { - name: "comamnds and subcommands that both match", + name: "commands and subcommands that both match", args: []string{"etcd-snapshot", "list", "foo", "bar"}, prefix: []string{"etcd-snapshot", "list"}, suffix: []string{"foo", "bar"}, found: true, }, + { + name: "commands and too many subcommands", + args: []string{"etcd-snapshot", "list", "delete", "foo", "bar"}, + prefix: []string{"etcd-snapshot", "list"}, + suffix: []string{"delete", "foo", "bar"}, + found: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := Parser{ - After: []string{"server", "agent", "etcd-snapshot", "save", "delete", "list", "prune"}, + After: []string{"server", "agent", "etcd-snapshot:1"}, } prefix, suffix, found := p.findStart(tt.args) if !reflect.DeepEqual(prefix, tt.prefix) { From acc1c71777fd84c4d4637870273a59ba69775896 Mon Sep 17 00:00:00 2001 From: Derek Nola Date: Wed, 3 Nov 2021 14:19:39 -0700 Subject: [PATCH 3/4] Edge case coverage, reworked regex with groups Signed-off-by: Derek Nola --- pkg/configfilearg/parser.go | 20 ++++++++++++-------- pkg/configfilearg/parser_test.go | 29 +++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/pkg/configfilearg/parser.go b/pkg/configfilearg/parser.go index d60912a62806..7af2bb1d82d2 100644 --- a/pkg/configfilearg/parser.go +++ b/pkg/configfilearg/parser.go @@ -104,17 +104,16 @@ func (p *Parser) findStart(args []string) ([]string, []string, bool) { } afterIndex := make(map[string]int) - re, err := regexp.Compile(`:\d`) + re, err := regexp.Compile(`(.+):(\d)+`) if err != nil { return args, nil, false } - // After keywords ending with ":" will set + NUM of arguments as the split point. + // After keywords ending with ":" can set + NUM of arguments as the split point. // used for matching on subcommmands for i, arg := range p.After { - if re.MatchString(arg) { - split := strings.Split(arg, ":") - p.After[i] = split[0] - afterIndex[split[0]], err = strconv.Atoi(split[1]) + if match := re.FindAllStringSubmatch(arg, -1); match != nil { + p.After[i] = match[0][1] + afterIndex[match[0][1]], err = strconv.Atoi(match[0][2]) if err != nil { return args, nil, false } @@ -124,8 +123,13 @@ func (p *Parser) findStart(args []string) ([]string, []string, bool) { for i, val := range args { for _, test := range p.After { if val == test { - skip := afterIndex[test] + 1 - return args[0 : i+skip], args[i+skip:], true + if skip := afterIndex[test]; skip != 0 { + if len(args) <= i+skip || strings.HasPrefix(args[i+skip], "-") { + return args[0 : i+1], args[i+1:], true + } + return args[0 : i+skip+1], args[i+skip+1:], true + } + return args[0 : i+1], args[i+1:], true } } } diff --git a/pkg/configfilearg/parser_test.go b/pkg/configfilearg/parser_test.go index 27390d043b06..7582a9db11f2 100644 --- a/pkg/configfilearg/parser_test.go +++ b/pkg/configfilearg/parser_test.go @@ -49,14 +49,35 @@ func Test_UnitParser_findStart(t *testing.T) { found: false, }, { - name: "commands and subcommands that both match", - args: []string{"etcd-snapshot", "list", "foo", "bar"}, + name: "command (with optional subcommands) but no flags", + args: []string{"etcd-snapshot"}, + prefix: []string{"etcd-snapshot"}, + suffix: []string{}, + found: true, + }, + { + name: "command (with optional subcommands) and flags", + args: []string{"etcd-snapshot", "-f"}, + prefix: []string{"etcd-snapshot"}, + suffix: []string{"-f"}, + found: true, + }, + { + name: "command and subcommand with no flags", + args: []string{"etcd-snapshot", "list"}, prefix: []string{"etcd-snapshot", "list"}, - suffix: []string{"foo", "bar"}, + suffix: []string{}, + found: true, + }, + { + name: "command and subcommand with flags", + args: []string{"etcd-snapshot", "list", "-f"}, + prefix: []string{"etcd-snapshot", "list"}, + suffix: []string{"-f"}, found: true, }, { - name: "commands and too many subcommands", + name: "command and too many subcommands", args: []string{"etcd-snapshot", "list", "delete", "foo", "bar"}, prefix: []string{"etcd-snapshot", "list"}, suffix: []string{"delete", "foo", "bar"}, From 29ccb63c1e7f3ea335d7e8ec3e05829e339d115b Mon Sep 17 00:00:00 2001 From: Derek Nola Date: Thu, 4 Nov 2021 09:09:34 -0700 Subject: [PATCH 4/4] Regex fix Signed-off-by: Derek Nola --- pkg/configfilearg/parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/configfilearg/parser.go b/pkg/configfilearg/parser.go index 7af2bb1d82d2..2440e8276b43 100644 --- a/pkg/configfilearg/parser.go +++ b/pkg/configfilearg/parser.go @@ -104,7 +104,7 @@ func (p *Parser) findStart(args []string) ([]string, []string, bool) { } afterIndex := make(map[string]int) - re, err := regexp.Compile(`(.+):(\d)+`) + re, err := regexp.Compile(`(.+):(\d+)`) if err != nil { return args, nil, false }