Commit e4c2c40a authored by Brad Davidson's avatar Brad Davidson Committed by Brad Davidson

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: 's avatarBrad Davidson <brad.davidson@rancher.com> (cherry picked from commit ff5c633f) Signed-off-by: 's avatarBrad Davidson <brad.davidson@rancher.com>
parent e79ef532
...@@ -8,6 +8,7 @@ import ( ...@@ -8,6 +8,7 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"slices"
"strconv" "strconv"
"strings" "strings"
...@@ -27,6 +28,7 @@ import ( ...@@ -27,6 +28,7 @@ import (
) )
var criDefaultConfigPath = "/etc/crictl.yaml" var criDefaultConfigPath = "/etc/crictl.yaml"
var externalCLIActions = []string{"crictl", "ctr", "kubectl"}
// main entrypoint for the k3s multicall binary // main entrypoint for the k3s multicall binary
func main() { func main() {
...@@ -105,7 +107,7 @@ func findDebug(args []string) bool { ...@@ -105,7 +107,7 @@ func findDebug(args []string) bool {
if debug { if debug {
return debug return debug
} }
debug, _ = strconv.ParseBool(configfilearg.MustFindString(args, "debug")) debug, _ = strconv.ParseBool(configfilearg.MustFindString(args, "debug", externalCLIActions...))
return debug return debug
} }
...@@ -125,7 +127,7 @@ func findDataDir(args []string) string { ...@@ -125,7 +127,7 @@ func findDataDir(args []string) string {
if dataDir != "" { if dataDir != "" {
return dataDir return dataDir
} }
dataDir = configfilearg.MustFindString(args, "data-dir") dataDir = configfilearg.MustFindString(args, "data-dir", externalCLIActions...)
if d, err := datadir.Resolve(dataDir); err == nil { if d, err := datadir.Resolve(dataDir); err == nil {
dataDir = d dataDir = d
} else { } else {
...@@ -143,7 +145,7 @@ func findPreferBundledBin(args []string) bool { ...@@ -143,7 +145,7 @@ func findPreferBundledBin(args []string) bool {
fs.SetOutput(io.Discard) fs.SetOutput(io.Discard)
fs.BoolVar(&preferBundledBin, "prefer-bundled-bin", false, "Prefer bundled binaries") 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 != "" { if preferRes != "" {
preferBundledBin, _ = strconv.ParseBool(preferRes) preferBundledBin, _ = strconv.ParseBool(preferRes)
} }
...@@ -158,8 +160,7 @@ func findPreferBundledBin(args []string) bool { ...@@ -158,8 +160,7 @@ func findPreferBundledBin(args []string) bool {
// it returns false so that standard CLI wrapping can occur. // it returns false so that standard CLI wrapping can occur.
func runCLIs(dataDir string) bool { func runCLIs(dataDir string) bool {
progName := filepath.Base(os.Args[0]) progName := filepath.Base(os.Args[0])
switch progName { if slices.Contains(externalCLIActions, progName) {
case "crictl", "ctr", "kubectl":
if err := externalCLI(progName, dataDir, os.Args[1:]); err != nil && !errors.Is(err, context.Canceled) { if err := externalCLI(progName, dataDir, os.Args[1:]); err != nil && !errors.Is(err, context.Canceled) {
logrus.Fatal(err) logrus.Fatal(err)
} }
......
package configfilearg package configfilearg
import ( import (
"slices"
"github.com/k3s-io/k3s/pkg/cli/cmds" "github.com/k3s-io/k3s/pkg/cli/cmds"
"github.com/k3s-io/k3s/pkg/version" "github.com/k3s-io/k3s/pkg/version"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
...@@ -23,15 +25,31 @@ func MustParse(args []string) []string { ...@@ -23,15 +25,31 @@ func MustParse(args []string) []string {
return result 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{ parser := &Parser{
OverrideFlags: []string{"--help", "-h", "--version", "-v"}, OverrideFlags: overrideFlags,
EnvName: version.ProgramUpper + "_CONFIG_FILE", EnvName: version.ProgramUpper + "_CONFIG_FILE",
DefaultConfig: "/etc/rancher/" + version.Program + "/config.yaml", DefaultConfig: "/etc/rancher/" + version.Program + "/config.yaml",
} }
result, err := parser.FindString(args, target) result, err := parser.FindString(args, target)
if err != nil { if err != nil {
if len(overrideFlags) > 0 {
logrus.Fatal(err) logrus.Fatal(err)
} else {
logrus.Warn(err)
}
} }
return result return result
} }
...@@ -85,7 +85,7 @@ func Test_UnitMustFindString(t *testing.T) { ...@@ -85,7 +85,7 @@ func Test_UnitMustFindString(t *testing.T) {
}{ }{
{ {
name: "Target not found in config file", name: "Target not found in config file",
args: []string{"--foo", "bar"}, args: []string{"k3s", "--foo", "bar"},
target: "token", target: "token",
want: "", want: "",
...@@ -95,7 +95,7 @@ func Test_UnitMustFindString(t *testing.T) { ...@@ -95,7 +95,7 @@ func Test_UnitMustFindString(t *testing.T) {
}, },
{ {
name: "Target found in config file", name: "Target found in config file",
args: []string{"--foo", "bar"}, args: []string{"k3s", "--foo", "bar"},
target: "token", target: "token",
want: "12345", want: "12345",
...@@ -104,11 +104,31 @@ func Test_UnitMustFindString(t *testing.T) { ...@@ -104,11 +104,31 @@ func Test_UnitMustFindString(t *testing.T) {
teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") }, teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") },
}, },
{ {
name: "Override flag found, function is short-circuited", name: "Override flag is returned if found",
args: []string{"--foo", "bar", "-h"}, args: []string{"k3s", "--foo", "bar", "--version"},
target: "token", 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") }, setup: func() error { return os.Setenv("K3S_CONFIG_FILE", "./testdata/defaultdata.yaml") },
teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") }, teardown: func() error { return os.Unsetenv("K3S_CONFIG_FILE") },
...@@ -121,7 +141,9 @@ func Test_UnitMustFindString(t *testing.T) { ...@@ -121,7 +141,9 @@ func Test_UnitMustFindString(t *testing.T) {
t.Errorf("Setup for MustFindString() failed = %v", err) t.Errorf("Setup for MustFindString() failed = %v", err)
return 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) t.Errorf("MustFindString() = %+v\nWant = %+v", got, tt.want)
} }
}) })
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment