moved implementation to custom patcher

Author: michalpristas

internal/pkg/agent/application/application.go

@@ -209,7 +209,11 @@ func New(
 			log.Info("Parsed configuration and determined agent is in Fleet Server bootstrap mode")
 
 			compModifiers = append(compModifiers, FleetServerComponentModifier(cfg.Fleet.Server))
-			configMgr = coordinator.NewConfigPatchManager(newFleetServerBootstrapManager(log), PatchAPMConfig(log, rawConfig))
+			configMgr = coordinator.NewConfigPatchManager(
+				newFleetServerBootstrapManager(log),
+				PatchAPMConfig(log, rawConfig),
+				PatchFleetConfig(log, rawConfig, caps, isManaged),
+			)
 		} else {
 			log.Info("Parsed configuration and determined agent is managed by Fleet")
 
@@ -259,7 +263,12 @@ func New(
 			if err != nil {
 				return nil, nil, nil, err
 			}
-			configMgr = coordinator.NewConfigPatchManager(managed, injectOutputOverrides(log, rawConfig), PatchAPMConfig(log, rawConfig))
+			configMgr = coordinator.NewConfigPatchManager(
+				managed,
+				injectOutputOverrides(log, rawConfig),
+				PatchAPMConfig(log, rawConfig),
+				PatchFleetConfig(log, rawConfig, caps, isManaged),
+			)
 		}
 	}
 

internal/pkg/agent/application/coordinator/coordinator.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"reflect"
 	"strings"
 	"sync"
@@ -1610,14 +1609,6 @@ func (c *Coordinator) processConfigAgent(ctx context.Context, cfg *config.Config
 		c.logger.Errorf("failed to add secret markers: %v", err)
 	}
 
-	// override retrieved config from Fleet with persisted config from AgentConfig file
-
-	if c.caps != nil {
-		if err := applyPersistedConfig(cfg, paths.ConfigFile(), c.caps.AllowFleetOverride); err != nil {
-			return fmt.Errorf("could not apply persisted configuration: %w", err)
-		}
-	}
-
 	// perform and verify ast translation
 	m, err := cfg.ToMapStr()
 	if err != nil {
@@ -1705,32 +1696,6 @@ func (c *Coordinator) generateAST(cfg *config.Config, m map[string]interface{})
 	return nil
 }
 
-func applyPersistedConfig(cfg *config.Config, configFile string, checkFn func() bool) error {
-	if !checkFn() {
-		// Feature is disabled, nothing to do
-		return nil
-	}
-
-	f, err := os.OpenFile(configFile, os.O_RDONLY, 0)
-	if err != nil && os.IsNotExist(err) {
-		return nil
-	} else if err != nil {
-		return fmt.Errorf("opening config file: %w", err)
-	}
-	defer f.Close()
-
-	persisted, err := config.NewConfigFrom(f)
-	if err != nil {
-		return fmt.Errorf("parsing persisted config: %w", err)
-	}
-
-	err = cfg.Merge(persisted)
-	if err != nil {
-		return fmt.Errorf("merging persisted config: %w", err)
-	}
-	return nil
-}
-
 // observeASTVars identifies the variables that are referenced in the computed AST and passed to
 // the varsMgr so it knows what providers are being referenced. If a providers is not being
 // referenced then the provider does not need to be running.

internal/pkg/agent/application/coordinator/coordinator_test.go

@@ -1011,36 +1011,6 @@ func TestCoordinator_UpgradeDetails(t *testing.T) {
 	require.Equal(t, expectedErr.Error(), coord.state.UpgradeDetails.Metadata.ErrorMsg)
 }
 
-func Test_ApplyPersistedConfig(t *testing.T) {
-	cfgFile := filepath.Join(".", "testdata", "overrides.yml")
-
-	testCases := []struct {
-		name          string
-		featureEnable bool
-		expectedLogs  bool
-	}{
-		{name: "enabled", featureEnable: true, expectedLogs: false},
-		{name: "disabled", featureEnable: false, expectedLogs: true},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			cfg, err := config.LoadFile(filepath.Join(".", "testdata", "config.yaml"))
-			require.NoError(t, err)
-
-			err = applyPersistedConfig(cfg, cfgFile, func() bool { return tc.featureEnable })
-			require.NoError(t, err)
-
-			c := &configuration.Configuration{}
-			require.NoError(t, cfg.Agent.Unpack(&c))
-
-			require.Equal(t, tc.expectedLogs, c.Settings.MonitoringConfig.MonitorLogs)
-			require.True(t, c.Settings.MonitoringConfig.MonitorMetrics)
-			require.True(t, c.Settings.MonitoringConfig.Enabled)
-		})
-	}
-}
-
 func BenchmarkCoordinator_generateComponentModel(b *testing.B) {
 	// load variables
 	varsMaps := []map[string]any{}

internal/pkg/agent/application/fleet_config_patcher.go

@@ -0,0 +1,28 @@
+package application
+
+import (
+	"github.com/elastic/elastic-agent/internal/pkg/agent/application/coordinator"
+	"github.com/elastic/elastic-agent/internal/pkg/capabilities"
+	"github.com/elastic/elastic-agent/internal/pkg/config"
+	"github.com/elastic/elastic-agent/pkg/core/logger"
+)
+
+func PatchFleetConfig(log *logger.Logger,
+	rawConfig *config.Config, // original config from fleet, this one won't reload
+	caps capabilities.Capabilities,
+	isManaged bool,
+) func(change coordinator.ConfigChange) coordinator.ConfigChange {
+	if !isManaged || // no need to override fleet config when not managed
+		caps == nil || !caps.AllowFleetOverride() {
+		return noop
+	}
+
+	return func(change coordinator.ConfigChange) coordinator.ConfigChange {
+		newConfig := change.Config()
+		if err := newConfig.Merge(rawConfig); err != nil {
+			log.Errorf("error merging fleet config into config change: %v", err)
+		}
+
+		return change
+	}
+}

internal/pkg/agent/application/fleet_config_patcher_test.go

@@ -0,0 +1,86 @@
+package application
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/elastic/elastic-agent/internal/pkg/agent/configuration"
+	"github.com/elastic/elastic-agent/internal/pkg/config"
+	"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_FleetPatcher(t *testing.T) {
+	configFile := filepath.Join(".", "coordinator", "testdata", "overrides.yml")
+
+	testCases := []struct {
+		name          string
+		isManaged     bool
+		featureEnable bool
+		expectedLogs  bool
+	}{
+		{name: "managed - enabled", isManaged: true, featureEnable: true, expectedLogs: false},
+		{name: "managed - disabled", isManaged: true, featureEnable: false, expectedLogs: true},
+		{name: "not managed - enabled", isManaged: false, featureEnable: true, expectedLogs: true},
+		{name: "not managed - disabled", isManaged: false, featureEnable: false, expectedLogs: true},
+	}
+
+	overridesFile, err := os.OpenFile(configFile, os.O_RDONLY, 0)
+	require.NoError(t, err)
+	defer overridesFile.Close()
+
+	rawConfig, err := config.NewConfigFrom(overridesFile)
+	require.NoError(t, err)
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			caps := &mockCapabilities{}
+			caps.On("AllowFleetOverride").Return(tc.featureEnable)
+
+			log, _ := loggertest.New(t.Name())
+
+			cfg, err := config.LoadFile(filepath.Join(".", "coordinator", "testdata", "config.yaml"))
+			require.NoError(t, err)
+
+			configChange := &mockConfigChange{
+				c: cfg,
+			}
+
+			patcher := PatchFleetConfig(log, rawConfig, caps, tc.isManaged)
+			patcher(configChange)
+
+			c := &configuration.Configuration{}
+			require.NoError(t, cfg.Agent.Unpack(&c))
+			assert.Equal(t, tc.expectedLogs, c.Settings.MonitoringConfig.MonitorLogs)
+			require.True(t, c.Settings.MonitoringConfig.MonitorMetrics)
+			require.True(t, c.Settings.MonitoringConfig.Enabled)
+		})
+	}
+}
+
+type mockCapabilities struct {
+	mock.Mock
+}
+
+func (m *mockCapabilities) AllowUpgrade(version string, sourceURI string) bool {
+	args := m.Called(version, sourceURI)
+	return args.Bool(0)
+}
+
+func (m *mockCapabilities) AllowInput(name string) bool {
+	args := m.Called()
+	return args.Bool(0)
+}
+
+func (m *mockCapabilities) AllowOutput(name string) bool {
+	args := m.Called()
+	return args.Bool(0)
+}
+
+func (m *mockCapabilities) AllowFleetOverride() bool {
+	args := m.Called()
+	return args.Bool(0)
+}