From ff5c633fe7a2422ed2aa18ce8cdf52f758433873 Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Tue, 5 Nov 2024 21:03:50 +0000 Subject: [PATCH] Fix MustFindString returning override flags on external CLI commands External CLI actions cannot short-circuit on --help or --version, so we cannot skip loading the config file if these flags are present when running these wrapped commands. The behavior of just returning the override flag name instead of the requested flag value was breaking data-dir lookup when running wrapped commands. Signed-off-by: Brad Davidson --- cmd/k3s/main.go | 11 ++++---- pkg/configfilearg/defaultparser.go | 24 ++++++++++++++--- pkg/configfilearg/defaultparser_test.go | 34 ++++++++++++++++++++----- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/cmd/k3s/main.go b/cmd/k3s/main.go index 52d5cefbf6..90eae7268e 100644 --- a/cmd/k3s/main.go +++ b/cmd/k3s/main.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strconv" "strings" @@ -27,6 +28,7 @@ import ( ) var criDefaultConfigPath = "/etc/crictl.yaml" +var externalCLIActions = []string{"crictl", "ctr", "kubectl"} // main entrypoint for the k3s multicall binary func main() { @@ -105,7 +107,7 @@ func findDebug(args []string) bool { if debug { return debug } - debug, _ = strconv.ParseBool(configfilearg.MustFindString(args, "debug")) + debug, _ = strconv.ParseBool(configfilearg.MustFindString(args, "debug", externalCLIActions...)) return debug } @@ -125,7 +127,7 @@ func findDataDir(args []string) string { if dataDir != "" { return dataDir } - dataDir = configfilearg.MustFindString(args, "data-dir") + dataDir = configfilearg.MustFindString(args, "data-dir", externalCLIActions...) if d, err := datadir.Resolve(dataDir); err == nil { dataDir = d } else { @@ -143,7 +145,7 @@ func findPreferBundledBin(args []string) bool { fs.SetOutput(io.Discard) fs.BoolVar(&preferBundledBin, "prefer-bundled-bin", false, "Prefer bundled binaries") - preferRes := configfilearg.MustFindString(args, "prefer-bundled-bin") + preferRes := configfilearg.MustFindString(args, "prefer-bundled-bin", externalCLIActions...) if preferRes != "" { preferBundledBin, _ = strconv.ParseBool(preferRes) } @@ -158,8 +160,7 @@ func findPreferBundledBin(args []string) bool { // it returns false so that standard CLI wrapping can occur. func runCLIs(dataDir string) bool { progName := filepath.Base(os.Args[0]) - switch progName { - case "crictl", "ctr", "kubectl": + if slices.Contains(externalCLIActions, progName) { if err := externalCLI(progName, dataDir, os.Args[1:]); err != nil && !errors.Is(err, context.Canceled) { logrus.Fatal(err) } diff --git a/pkg/configfilearg/defaultparser.go b/pkg/configfilearg/defaultparser.go index b9b6d04fca..85d553bc08 100644 --- a/pkg/configfilearg/defaultparser.go +++ b/pkg/configfilearg/defaultparser.go @@ -1,6 +1,8 @@ package configfilearg import ( + "slices" + "github.com/k3s-io/k3s/pkg/cli/cmds" "github.com/k3s-io/k3s/pkg/version" "github.com/sirupsen/logrus" @@ -23,15 +25,31 @@ func MustParse(args []string) []string { return result } -func MustFindString(args []string, target string) string { +func MustFindString(args []string, target string, commandsWithoutOverride ...string) string { + overrideFlags := []string{"--help", "-h", "--version", "-v"} + // Check to see if the command or subcommand being executed supports override flags. + // Some subcommands such as `k3s ctr` or just `ctr` need to be extracted out even to + // provide version or help text, and we cannot short-circuit loading the config file. For + // these commands, treat failure to load the config file as a warning instead of a fatal. + if len(args) > 0 && args[0] == version.Program { + args = args[1:] + } + if len(args) > 0 && slices.Contains(commandsWithoutOverride, args[0]) { + overrideFlags = nil + } + parser := &Parser{ - OverrideFlags: []string{"--help", "-h", "--version", "-v"}, + OverrideFlags: overrideFlags, EnvName: version.ProgramUpper + "_CONFIG_FILE", DefaultConfig: "/etc/rancher/" + version.Program + "/config.yaml", } result, err := parser.FindString(args, target) if err != nil { - logrus.Fatal(err) + if len(overrideFlags) > 0 { + logrus.Fatal(err) + } else { + logrus.Warn(err) + } } return result } diff --git a/pkg/configfilearg/defaultparser_test.go b/pkg/configfilearg/defaultparser_test.go index 8ae8decc26..92002a8cb0 100644 --- a/pkg/configfilearg/defaultparser_test.go +++ b/pkg/configfilearg/defaultparser_test.go @@ -85,7 +85,7 @@ func Test_UnitMustFindString(t *testing.T) { }{ { name: "Target not found in config file", - args: []string{"--foo", "bar"}, + args: []string{"k3s", "--foo", "bar"}, target: "token", want: "", @@ -95,7 +95,7 @@ func Test_UnitMustFindString(t *testing.T) { }, { name: "Target found in config file", - args: []string{"--foo", "bar"}, + args: []string{"k3s", "--foo", "bar"}, target: "token", want: "12345", @@ -104,11 +104,31 @@ func Test_UnitMustFindString(t *testing.T) { teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") }, }, { - name: "Override flag found, function is short-circuited", - args: []string{"--foo", "bar", "-h"}, + name: "Override flag is returned if found", + args: []string{"k3s", "--foo", "bar", "--version"}, target: "token", - want: "-h", + want: "--version", + + setup: func() error { return os.Setenv("K3S_CONFIG_FILE", "./testdata/defaultdata.yaml") }, + teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") }, + }, + { + name: "Override flag is not returned for specific subcommands", + args: []string{"k3s", "ctr", "--foo", "bar", "--version"}, + target: "token", + + want: "12345", + + setup: func() error { return os.Setenv("K3S_CONFIG_FILE", "./testdata/defaultdata.yaml") }, + teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") }, + }, + { + name: "Override flag is not returned for specific subcommands", + args: []string{"kubectl", "--foo", "bar", "--help"}, + target: "token", + + want: "12345", setup: func() error { return os.Setenv("K3S_CONFIG_FILE", "./testdata/defaultdata.yaml") }, teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") }, @@ -121,7 +141,9 @@ func Test_UnitMustFindString(t *testing.T) { t.Errorf("Setup for MustFindString() failed = %v", err) return } - if got := MustFindString(tt.args, tt.target); got != tt.want { + got := MustFindString(tt.args, tt.target, "crictl", "ctr", "kubectl") + t.Logf("MustFindString(%+v, %+v) = %s", tt.args, tt.target, got) + if got != tt.want { t.Errorf("MustFindString() = %+v\nWant = %+v", got, tt.want) } })