From 3c67cd5463f6c308dc7a9a42a14f4e7c3e6f8128 Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Wed, 25 Sep 2024 14:51:17 -0400 Subject: [PATCH 1/4] Don't register flags on global command line in init. --- libbeat/cfgfile/cfgfile.go | 20 +++++++++++++++++--- libbeat/cmd/root.go | 3 +++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/libbeat/cfgfile/cfgfile.go b/libbeat/cfgfile/cfgfile.go index 21d052d64da2..61ea69f05b4d 100644 --- a/libbeat/cfgfile/cfgfile.go +++ b/libbeat/cfgfile/cfgfile.go @@ -34,8 +34,9 @@ var ( // The default config cannot include the beat name as it is not initialized // when this variable is created. See ChangeDefaultCfgfileFlag which should // be called prior to flags.Parse(). - configfiles = config.StringArrFlag(nil, "c", "beat.yml", "Configuration file, relative to path.config") - overwrites = config.SettingFlag(nil, "E", "Configuration overwrite") + commandLine flag.FlagSet + configfiles = config.StringArrFlag(&commandLine, "c", "beat.yml", "Configuration file, relative to path.config") + overwrites = config.SettingFlag(&commandLine, "E", "Configuration overwrite") // Additional default settings, that must be available for variable expansion defaults = config.MustNewConfigFrom(map[string]interface{}{ @@ -55,7 +56,7 @@ var ( func init() { // add '-path.x' options overwriting paths in 'overwrites' config makePathFlag := func(name, usage string) *string { - return config.ConfigOverwriteFlag(nil, overwrites, name, name, "", usage) + return config.ConfigOverwriteFlag(&commandLine, overwrites, name, name, "", usage) } homePath = makePathFlag("path.home", "Home path") @@ -64,6 +65,19 @@ func init() { makePathFlag("path.logs", "Logs path") } +// InitFlags is for explicitly initializing the flags. +// It may get called repeatedly for different flagsets, but not +// twice for the same one. +func InitFlags(flagset *flag.FlagSet) { + if flagset == nil { + flagset = flag.CommandLine + } + + commandLine.VisitAll(func(f *flag.Flag) { + flagset.Var(f.Value, f.Name, f.Usage) + }) +} + // OverrideChecker checks if a config should be overwritten. type OverrideChecker func(*config.C) bool diff --git a/libbeat/cmd/root.go b/libbeat/cmd/root.go index be2cb9d441ae..60c5c77dce0b 100644 --- a/libbeat/cmd/root.go +++ b/libbeat/cmd/root.go @@ -81,6 +81,9 @@ func GenRootCmdWithSettings(beatCreator beat.Creator, settings instance.Settings panic(fmt.Errorf("failed to set default config file path: %v", err)) } + // Initialize the configuration flags. + cfgfile.InitFlags(nil) + // must be updated prior to CLI flag handling. rootCmd.RunCmd = genRunCmd(settings, beatCreator) From fd1b3491fce50838f97786c1f353d034ce1fc6fb Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Wed, 25 Sep 2024 20:00:51 -0400 Subject: [PATCH 2/4] Fix lint and only register once. --- libbeat/cfgfile/cfgfile.go | 20 ++++++++++---------- libbeat/cmd/root.go | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/libbeat/cfgfile/cfgfile.go b/libbeat/cfgfile/cfgfile.go index 61ea69f05b4d..f77325109779 100644 --- a/libbeat/cfgfile/cfgfile.go +++ b/libbeat/cfgfile/cfgfile.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "path/filepath" + "sync" "github.com/elastic/beats/v7/libbeat/common" "github.com/elastic/beats/v7/libbeat/common/fleetmode" @@ -34,9 +35,10 @@ var ( // The default config cannot include the beat name as it is not initialized // when this variable is created. See ChangeDefaultCfgfileFlag which should // be called prior to flags.Parse(). - commandLine flag.FlagSet - configfiles = config.StringArrFlag(&commandLine, "c", "beat.yml", "Configuration file, relative to path.config") - overwrites = config.SettingFlag(&commandLine, "E", "Configuration overwrite") + commandLine flag.FlagSet + commandLineOnce sync.Once + configfiles = config.StringArrFlag(&commandLine, "c", "beat.yml", "Configuration file, relative to path.config") + overwrites = config.SettingFlag(&commandLine, "E", "Configuration overwrite") // Additional default settings, that must be available for variable expansion defaults = config.MustNewConfigFrom(map[string]interface{}{ @@ -68,13 +70,11 @@ func init() { // InitFlags is for explicitly initializing the flags. // It may get called repeatedly for different flagsets, but not // twice for the same one. -func InitFlags(flagset *flag.FlagSet) { - if flagset == nil { - flagset = flag.CommandLine - } - - commandLine.VisitAll(func(f *flag.Flag) { - flagset.Var(f.Value, f.Name, f.Usage) +func InitFlags() { + commandLineOnce.Do(func() { + commandLine.VisitAll(func(f *flag.Flag) { + flag.CommandLine.Var(f.Value, f.Name, f.Usage) + }) }) } diff --git a/libbeat/cmd/root.go b/libbeat/cmd/root.go index 60c5c77dce0b..671b72fde1ec 100644 --- a/libbeat/cmd/root.go +++ b/libbeat/cmd/root.go @@ -61,7 +61,7 @@ type BeatsRootCmd struct { func GenRootCmdWithSettings(beatCreator beat.Creator, settings instance.Settings) *BeatsRootCmd { // Add global Elasticsearch license endpoint check. // Check we are actually talking with Elasticsearch, to ensure that used features actually exist. - elasticsearch.RegisterGlobalCallback(licenser.FetchAndVerify) + _, _ = elasticsearch.RegisterGlobalCallback(licenser.FetchAndVerify) if err := platformcheck.CheckNativePlatformCompat(); err != nil { fmt.Fprintf(os.Stderr, "Failed to initialize: %v\n", err) @@ -78,11 +78,11 @@ func GenRootCmdWithSettings(beatCreator beat.Creator, settings instance.Settings // Due to a dependence upon the beat name, the default config file path err := cfgfile.ChangeDefaultCfgfileFlag(settings.Name) if err != nil { - panic(fmt.Errorf("failed to set default config file path: %v", err)) + panic(fmt.Errorf("failed to set default config file path: %w", err)) } // Initialize the configuration flags. - cfgfile.InitFlags(nil) + cfgfile.InitFlags() // must be updated prior to CLI flag handling. From 1060127e71e79cf2ad52ff5d86c10f141a7f540d Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Wed, 25 Sep 2024 21:36:55 -0400 Subject: [PATCH 3/4] Fix integration test. --- libbeat/cmd/instance/beat_integration_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libbeat/cmd/instance/beat_integration_test.go b/libbeat/cmd/instance/beat_integration_test.go index a3388399e76a..c923f6ec7e2c 100644 --- a/libbeat/cmd/instance/beat_integration_test.go +++ b/libbeat/cmd/instance/beat_integration_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/elastic/beats/v7/libbeat/beat" + "github.com/elastic/beats/v7/libbeat/cfgfile" "github.com/elastic/beats/v7/libbeat/cmd/instance" "github.com/elastic/beats/v7/libbeat/mock" "github.com/elastic/elastic-agent-libs/config" @@ -77,6 +78,7 @@ func (mb mockbeat) Stop() { } func TestMonitoringNameFromConfig(t *testing.T) { + mockBeat := mockbeat{ done: make(chan struct{}), initDone: make(chan struct{}), @@ -90,6 +92,8 @@ func TestMonitoringNameFromConfig(t *testing.T) { go func() { defer wg.Done() + // Initialize cfgfile flags + cfgfile.InitFlags() // Set the configuration file path flag so the beat can read it flag.Set("c", "testdata/mockbeat.yml") instance.Run(mock.Settings, func(_ *beat.Beat, _ *config.C) (beat.Beater, error) { From 2418b1bf88388752c147b8e686d27f670a014723 Mon Sep 17 00:00:00 2001 From: Blake Rouse Date: Wed, 25 Sep 2024 21:58:30 -0400 Subject: [PATCH 4/4] Fix lint. --- libbeat/cmd/instance/beat_integration_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libbeat/cmd/instance/beat_integration_test.go b/libbeat/cmd/instance/beat_integration_test.go index c923f6ec7e2c..4c2aa94bd1d4 100644 --- a/libbeat/cmd/instance/beat_integration_test.go +++ b/libbeat/cmd/instance/beat_integration_test.go @@ -18,6 +18,7 @@ package instance_test import ( + "context" "encoding/json" "flag" "net/http" @@ -95,8 +96,8 @@ func TestMonitoringNameFromConfig(t *testing.T) { // Initialize cfgfile flags cfgfile.InitFlags() // Set the configuration file path flag so the beat can read it - flag.Set("c", "testdata/mockbeat.yml") - instance.Run(mock.Settings, func(_ *beat.Beat, _ *config.C) (beat.Beater, error) { + _ = flag.Set("c", "testdata/mockbeat.yml") + _ = instance.Run(mock.Settings, func(_ *beat.Beat, _ *config.C) (beat.Beater, error) { return &mockBeat, nil }) }() @@ -113,9 +114,13 @@ func TestMonitoringNameFromConfig(t *testing.T) { // the HTTP server goroutine time.Sleep(10 * time.Millisecond) - resp, err := http.Get("http://localhost:5066/state") + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://localhost:5066/state", nil) if err != nil { - t.Fatal("calling state endpoint: ", err.Error()) + t.Fatalf("error creating request: %v", err) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("calling state endpoint: %v", err) } defer resp.Body.Close()