From 27e6acacb0a1fc5008ca83009e2675d4c895197e Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Mon, 8 Jan 2024 16:08:51 -0500 Subject: [PATCH] Modify flags when needed to accommodate pflag (#1534) --- ee/tuf/library_lookup.go | 20 +++++++++++++-- ee/tuf/library_lookup_test.go | 48 ++++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/ee/tuf/library_lookup.go b/ee/tuf/library_lookup.go index 1da2543b4..180ad4d2d 100644 --- a/ee/tuf/library_lookup.go +++ b/ee/tuf/library_lookup.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" @@ -64,6 +65,21 @@ func ShouldUseNewAutoupdater(ctx context.Context) bool { // getAutoupdateConfig pulls the configuration values necessary to work with the autoupdate library // from either the given args or from the config file. func getAutoupdateConfig(args []string) (*autoupdateConfig, error) { + // pflag, while mostly great for our usecase here, expects getopt-style flags, which means + // it doesn't support the Golang standard of using single and double dashes interchangeably + // for flags. (e.g., pflag cannot parse `-config`, but Golang treats `-config` the same as + // `--config`.) This transforms all single-dash args to double-dashes so that pflag can parse + // them as expected. + argsToParse := make([]string, len(args)) + for i := 0; i < len(args); i += 1 { + if strings.HasPrefix(args[i], "-") && !strings.HasPrefix(args[i], "--") { + argsToParse[i] = "-" + args[i] + continue + } + + argsToParse[i] = args[i] + } + // Create a flagset with options that are relevant to autoupdate only. // Ensure that we won't fail out when we see other command-line options. pflagSet := pflag.NewFlagSet("autoupdate options", pflag.ContinueOnError) @@ -77,7 +93,7 @@ func getAutoupdateConfig(args []string) (*autoupdateConfig, error) { pflagSet.StringVar(&flUpdateChannel, "update_channel", "", "") pflagSet.StringVar(&flLocalDevelopmentPath, "localdev_path", "", "") - if err := pflagSet.Parse(args); err != nil { + if err := pflagSet.Parse(argsToParse); err != nil { return nil, fmt.Errorf("parsing command-line flags: %w", err) } @@ -88,7 +104,7 @@ func getAutoupdateConfig(args []string) (*autoupdateConfig, error) { // is set) or via command line (flRootDirectory and flUpdateChannel are set), but do not // support a mix of both for this usage. if flConfigFilePath == "" && flRootDirectory == "" && flUpdateChannel == "" { - return getAutoupdateConfigFromFile(launcher.ConfigFilePath(args)) + return getAutoupdateConfigFromFile(launcher.ConfigFilePath(argsToParse)) } if flConfigFilePath != "" { diff --git a/ee/tuf/library_lookup_test.go b/ee/tuf/library_lookup_test.go index dab4c4e8f..c8e393d19 100644 --- a/ee/tuf/library_lookup_test.go +++ b/ee/tuf/library_lookup_test.go @@ -252,14 +252,24 @@ transport jsonrpc require.NoError(t, os.WriteFile(configFilepath, []byte(fileContents), 0755), "expected to set up test config file") - cfg, err := getAutoupdateConfig([]string{"--config", configFilepath}) + cfg1, err := getAutoupdateConfig([]string{"--config", configFilepath}) require.NoError(t, err, "expected no error getting autoupdate config") - require.NotNil(t, cfg, "expected valid autoupdate config") - require.Equal(t, testRootDir, cfg.rootDirectory, "root directory is incorrect") - require.Equal(t, "", cfg.updateDirectory, "update directory should not have been set") - require.Equal(t, testChannel, cfg.channel, "channel is incorrect") - require.Equal(t, "", cfg.localDevelopmentPath, "local development path should not have been set") + require.NotNil(t, cfg1, "expected valid autoupdate config") + require.Equal(t, testRootDir, cfg1.rootDirectory, "root directory is incorrect") + require.Equal(t, "", cfg1.updateDirectory, "update directory should not have been set") + require.Equal(t, testChannel, cfg1.channel, "channel is incorrect") + require.Equal(t, "", cfg1.localDevelopmentPath, "local development path should not have been set") + + // Same thing, just one - instead of 2 + cfg2, err := getAutoupdateConfig([]string{"-config", configFilepath}) + require.NoError(t, err, "expected no error getting autoupdate config") + + require.NotNil(t, cfg2, "expected valid autoupdate config") + require.Equal(t, testRootDir, cfg2.rootDirectory, "root directory is incorrect") + require.Equal(t, "", cfg2.updateDirectory, "update directory should not have been set") + require.Equal(t, testChannel, cfg2.channel, "channel is incorrect") + require.Equal(t, "", cfg2.localDevelopmentPath, "local development path should not have been set") } func Test_getAutoupdateConfig_ConfigFlagNotSet(t *testing.T) { @@ -287,3 +297,29 @@ func Test_getAutoupdateConfig_ConfigFlagNotSet(t *testing.T) { require.Equal(t, testChannel, cfg.channel, "channel is incorrect") require.Equal(t, testLocaldevPath, cfg.localDevelopmentPath, "local development path is incorrect") } + +func Test_getAutoupdateConfig_ConfigFlagNotSet_SingleHyphen(t *testing.T) { + t.Parallel() + + testRootDir := t.TempDir() + testUpdateDir := t.TempDir() + testChannel := "nightly" + testLocaldevPath := filepath.Join("some", "path", "to", "a", "local", "build") + + cfg, err := getAutoupdateConfig([]string{ + "-root_directory", testRootDir, + "-osquery_flag", "enable_watchdog_debug=true", + "-update_directory", testUpdateDir, + "-autoupdate", + "-update_channel", testChannel, + "-localdev_path", testLocaldevPath, + "-transport", "jsonrpc", + }) + require.NoError(t, err, "expected no error getting autoupdate config") + + require.NotNil(t, cfg, "expected valid autoupdate config") + require.Equal(t, testRootDir, cfg.rootDirectory, "root directory is incorrect") + require.Equal(t, testUpdateDir, cfg.updateDirectory, "update directory is incorrect") + require.Equal(t, testChannel, cfg.channel, "channel is incorrect") + require.Equal(t, testLocaldevPath, cfg.localDevelopmentPath, "local development path is incorrect") +}