wip - perhaps fixed double configuring features

libbeat/cfgfile/list.go

@@ -18,7 +18,6 @@
 package cfgfile
 
 import (
-	"fmt"
 	"sync"
 
 	"github.com/joeshaw/multierror"
@@ -28,7 +27,6 @@ import (
 	"github.com/elastic/beats/v7/libbeat/beat"
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/elastic/beats/v7/libbeat/common/reload"
-	"github.com/elastic/beats/v7/libbeat/features"
 	"github.com/elastic/beats/v7/libbeat/publisher/pipetool"
 	"github.com/elastic/elastic-agent-libs/config"
 	"github.com/elastic/elastic-agent-libs/logp"
@@ -100,12 +98,14 @@ func (r *RunnerList) Reload(configs []*reload.ConfigWithMeta) error {
 	// Wait for all runners to stop before starting new ones
 	wg.Wait()
 
-	// Feature flags are per unit. Right now it's actually component/process,
-	// thus it needs to be reloaded only once.
-	if err := features.Parse(configs[0].Config); err != nil {
-		return fmt.Errorf("could not parse features config during reload: %w", err)
-	}
-	r.logger.Infow("reload feature flag fqdn: %t", features.FQDN())
+	// // Feature flags are per unit. Right now it's actually component/process,
+	// // thus it needs to be reloaded only once.
+	// if len(configs) > 0 {
+	// 	if err := features.ParseFromConfig(configs[0].Config); err != nil {
+	// 		return fmt.Errorf("could not parse features config during reload: %w", err)
+	// 	}
+	// 	r.logger.Infof("reload feature flag fqdn: %t", features.FQDN())
+	// }
 
 	// Start new runners
 	for hash, config := range startList {

libbeat/cmd/instance/beat.go

@@ -762,7 +762,7 @@ func (b *Beat) configure(settings Settings) error {
 		return fmt.Errorf("error unpacking config data: %w", err)
 	}
 
-	if err := features.Parse(b.RawConfig); err != nil {
+	if err := features.ParseFromConfig(b.RawConfig); err != nil {
 		logp.L().Errorw("could not parse features, see config",
 			"features_config", b.RawConfig)
 		return fmt.Errorf("could not parse features: %w", err)

libbeat/features/features.go

@@ -10,7 +10,6 @@ import (
 )
 
 var (
-	// flags configs
 	mu sync.Mutex
 
 	flags fflags
@@ -20,42 +19,52 @@ type fflags struct {
 	fqdnEnabled bool
 }
 
-// type configs struct {
-// 	FQDN struct {
-// 		Enabled bool `json:"enabled" yaml:"enabled" config:"enabled"`
-// 	} `json:"fqdn" yaml:"fqdn" config:"fqdn"`
-// }
+func UpdateFromProto(f *proto.Features) {
+	logp.L().Info("features.UpdateFromProto invoked")
 
-func Update(f *proto.Features) {
 	if f == nil {
+		logp.L().Info("feature flag proto is nil!")
 		return
 	}
 
 	mu.Lock()
 	defer mu.Unlock()
 	flags = fflags{fqdnEnabled: f.Fqdn.Enabled}
+
+	logp.L().Infof("features.UpdateFromProto: fqdn: %t", flags.fqdnEnabled)
 }
 
-func Parse(c *conf.C) error {
-	logp.L().Info("features.Parse invoked")
+func ParseFromConfig(c *conf.C) error {
+	logp.L().Info("features.ParseFromConfig invoked")
 	if c == nil {
 		logp.L().Info("feature flag config is nil!")
 		return nil
 	}
 
-	enabled, err := c.Bool("features.fqdn.enabled", -1)
-	if err != nil {
-		return fmt.Errorf("could not FQDN feature config: %w", err)
+	type cfg struct {
+		Features struct {
+			FQDN *conf.C `json:"fqdn" yaml:"fqdn" config:"fqdn"`
+		} `json:"features" yaml:"features" config:"features"`
+	}
+
+	parsedFlags := cfg{}
+	if err := c.Unpack(&parsedFlags); err != nil {
+		logp.L().Errorf("could not Unpack features config: %v", err)
+
+		return fmt.Errorf("could not Unpack features config: %w", err)
 	}
 
 	mu.Lock()
 	defer mu.Unlock()
-	flags.fqdnEnabled = enabled
+	flags.fqdnEnabled = parsedFlags.Features.FQDN.Enabled()
+
+	logp.L().Infof("features.ParseFromConfig: fqdn: %t", flags.fqdnEnabled)
 
 	return nil
 }
 
 // FQDN reports if FQDN should be used instead of hostname for host.name.
+// If it hasn't been set by ParseFromConfig or UpdateFromProto, it returns false.
 func FQDN() bool {
 	return flags.fqdnEnabled
 }

libbeat/features/features_test.go

@@ -1,19 +1,72 @@
 package features
 
-// const configYAML = `
-//   features:
-//     fqdn:
-//       enabled: true
-// `
-//
-// func TestParse(t *testing.T) {
-// 	c, err := config.NewConfigFrom(configYAML)
-// 	if err != nil {
-// 		t.Fatalf("could not parse config YAML: %v", err)
-// 	}
-//
-// 	err = Parse(c)
-// 	if err != nil {
-// 		t.Fatalf("Parse failed: %v", err)
-// 	}
-// }
+import (
+	"testing"
+
+	"github.com/elastic/elastic-agent-libs/config"
+)
+
+func TestFQDN(t *testing.T) {
+	tcs := []struct {
+		name string
+		yaml string
+		want bool
+	}{
+		{
+			name: "FQDN enabled",
+			yaml: `
+  features:
+    fqdn:
+      enabled: true`,
+			want: true,
+		},
+		{
+			name: "FQDN disabled",
+			yaml: `
+  features:
+    fqdn:
+      enabled: false`,
+			want: false,
+		},
+		{
+			name: "FQDN only {}",
+			yaml: `
+  features:
+    fqdn: {}`,
+			want: true,
+		},
+		{
+			name: "FQDN empty",
+			yaml: `
+  features:
+    fqdn:`,
+			want: false,
+		},
+		{
+			name: "FQDN absent",
+			yaml: `
+  features:`,
+			want: false,
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+
+			c, err := config.NewConfigFrom(tc.yaml)
+			if err != nil {
+				t.Fatalf("could not parse config YAML: %v", err)
+			}
+
+			err = ParseFromConfig(c)
+			if err != nil {
+				t.Fatalf("ParseFromConfig failed: %v", err)
+			}
+
+			got := FQDN()
+			if got != tc.want {
+				t.Errorf("want: %t, got %t", tc.want, got)
+			}
+		})
+	}
+}

libbeat/processors/add_host_metadata/add_host_metadata.go

@@ -142,7 +142,7 @@ func (p *addHostMetadata) loadData() error {
 		}
 	}
 
-	p.logger.Infof("addHostMetadata feature FQDN: %t", features.FQDN())
+	p.logger.Infof("addHostMetadata feature FQDN: %t. before switch", features.FQDN())
 	switch {
 	case p.config.Name != "":
 		p.logger.Infof("addHostMetadata config.Name set, taking precedence: %s", p.config.Name)
@@ -151,6 +151,7 @@ func (p *addHostMetadata) loadData() error {
 		p.logger.Infof("addHostMetadata using fqdn: %s", h.Info().FQDN)
 		_, _ = data.Put("host.name", h.Info().FQDN)
 	}
+	p.logger.Infof("addHostMetadata feature FQDN: %t. after switch", features.FQDN())
 
 	p.data.Set(data)
 	return nil

x-pack/libbeat/management/managerV2.go

@@ -197,7 +197,13 @@ func (cm *BeatV2Manager) Start() error {
 	ctx, canceller := context.WithCancel(ctx)
 	cm.errCanceller = canceller
 	go cm.watchErrChan(ctx)
-	cm.client.RegisterDiagnosticHook("beat-rendered-config", "the rendered config used by the beat", "beat-rendered-config.yml", "application/yaml", cm.handleDebugYaml)
+	cm.client.RegisterDiagnosticHook(
+		"beat-rendered-config",
+		"the rendered config used by the beat",
+		"beat-rendered-config.yml",
+		"application/yaml",
+		cm.handleDebugYaml)
+
 	go cm.unitListen()
 	cm.isRunning = true
 	return nil
@@ -573,7 +579,7 @@ func (cm *BeatV2Manager) reloadOutput(unit *client.Unit) error {
 		return fmt.Errorf("failed to generate config for output: %w", err)
 	}
 
-	features.Update(featureFlags)
+	features.UpdateFromProto(featureFlags)
 	err = output.Reload(reloadConfig)
 	if err != nil {
 		return fmt.Errorf("failed to reload output: %w", err)
@@ -610,7 +616,7 @@ func (cm *BeatV2Manager) reloadInputs(inputUnits []*client.Unit) error {
 		inputBeatCfgs = append(inputBeatCfgs, inputCfg...)
 	}
 
-	features.Update(featureFlags) // TODO(Anderson): add fearure flags change to didChange
+	features.UpdateFromProto(featureFlags) // TODO(Anderson): add fearure flags change to didChange
 	if !didChange(cm.lastInputCfgs, inputCfgs) {
 		cm.logger.Debug("Skipped reloading input units; configuration didn't change")
 		return nil