Skip to content

Commit

Permalink
Modify flags when needed to accommodate pflag (#1534)
Browse files Browse the repository at this point in the history
  • Loading branch information
RebeccaMahany authored Jan 8, 2024
1 parent ceeac1e commit 27e6aca
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 8 deletions.
20 changes: 18 additions & 2 deletions ee/tuf/library_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}

Expand All @@ -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 != "" {
Expand Down
48 changes: 42 additions & 6 deletions ee/tuf/library_lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}

0 comments on commit 27e6aca

Please sign in to comment.