From f1d98a0f5bc8ffa0699547f0d7a01c9d7af4818e Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 10:08:51 +0200 Subject: [PATCH 01/21] added source uri reloading --- .../pkg/agent/application/managed_mode.go | 10 +++ .../agent/application/managed_mode_test.go | 1 + .../handlers/handler_action_policy_change.go | 77 +++++++++++++++++++ .../handler_action_policy_change_test.go | 76 ++++++++++++++++++ 4 files changed, 164 insertions(+) diff --git a/internal/pkg/agent/application/managed_mode.go b/internal/pkg/agent/application/managed_mode.go index d334ae0198c..154e0d3acda 100644 --- a/internal/pkg/agent/application/managed_mode.go +++ b/internal/pkg/agent/application/managed_mode.go @@ -207,6 +207,16 @@ func newManaged( agentInfo, cfg, storeSaver, + map[string]handlers.ReloadFunc{ + "agent.download.sourceURI": func(value interface{}) error { + if strVal, ok := value.(string); !ok { + return errors.New("provided soruce_uri is not a string") + } else if strVal != "" { + cfg.Settings.DownloadConfig.SourceURI = strVal + } + return nil + }, + }, ) actionDispatcher.MustRegister( diff --git a/internal/pkg/agent/application/managed_mode_test.go b/internal/pkg/agent/application/managed_mode_test.go index 7f111eae322..3d93c4953b9 100644 --- a/internal/pkg/agent/application/managed_mode_test.go +++ b/internal/pkg/agent/application/managed_mode_test.go @@ -61,6 +61,7 @@ func TestManagedModeRouting(t *testing.T) { agentInfo, cfg, nullStore, + nil, ), ) diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go index 3775d12b352..06523ece1ac 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go @@ -11,6 +11,7 @@ import ( "io" "io/ioutil" "sort" + "strings" "time" "gopkg.in/yaml.v2" @@ -33,6 +34,8 @@ const ( apiStatusTimeout = 15 * time.Second ) +type ReloadFunc func(value interface{}) error + // PolicyChange is a handler for POLICY_CHANGE action. type PolicyChange struct { log *logger.Logger @@ -41,6 +44,7 @@ type PolicyChange struct { config *configuration.Configuration store storage.Store setters []actions.ClientSetter + reloaders map[string]ReloadFunc } // NewPolicyChange creates a new PolicyChange handler. @@ -50,6 +54,7 @@ func NewPolicyChange( agentInfo *info.AgentInfo, config *configuration.Configuration, store storage.Store, + reloaders map[string]ReloadFunc, setters ...actions.ClientSetter, ) *PolicyChange { return &PolicyChange{ @@ -59,6 +64,7 @@ func NewPolicyChange( config: config, store: store, setters: setters, + reloaders: reloaders, } } @@ -84,6 +90,11 @@ func (h *PolicyChange) Handle(ctx context.Context, a fleetapi.Action, acker stor return errors.New(err, "could not parse the configuration from the policy", errors.TypeConfig) } + err = h.handleReloads(ctx, c) + if err != nil { + return err + } + h.log.Debugf("handlerPolicyChange: emit configuration for action %+v", a) err = h.handleFleetServerHosts(ctx, c) if err != nil { @@ -96,6 +107,72 @@ func (h *PolicyChange) Handle(ctx context.Context, a fleetapi.Action, acker stor return acker.Ack(ctx, action) } +func (h *PolicyChange) handleReloads(ctx context.Context, c *config.Config) error { + if len(h.reloaders) == 0 { + return nil + } + + data, err := c.ToMapStr() + if err != nil { + return errors.New(err, "could not convert the configuration from the policy", errors.TypeConfig) + } + + for key, reloader := range h.reloaders { + if err := h.handleReload(ctx, data, key, reloader); err != nil { + return err + } + } + return nil +} + +func (h *PolicyChange) handleReload(ctx context.Context, configMap map[string]interface{}, key string, reloader ReloadFunc) error { + if configMap == nil { + return nil + } + + findFn := func(key string, data map[string]interface{}) (interface{}, bool) { + for { + // break loop on cancellation + if ctx.Err() != nil { + return nil, false + } + if key == "" { + return nil, false + } + + if val, found := data[key]; found { + return val, true + } + + sepIdx := strings.IndexRune(key, '.') + if sepIdx < 0 { + // simple key, no config counterpart found + return nil, false + } + + k := key[:sepIdx] + val, found := data[k] + if !found { + return nil, false + } + + expectedMap, ok := val.(map[string]interface{}) + if !ok { + return nil, false + } + + data = expectedMap + key = key[sepIdx+1:] + } + } + + if val, found := findFn(key, configMap); found { + return reloader(val) + } + + return nil +} + func (h *PolicyChange) handleFleetServerHosts(ctx context.Context, c *config.Config) (err error) { // do not update fleet-server host from policy; no setters provided with local Fleet Server if len(h.setters) == 0 { diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index d887e755154..7047bd5f3c9 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -86,6 +86,82 @@ func TestPolicyChange(t *testing.T) { err := handler.Handle(context.Background(), action, ack) require.Error(t, err) }) + + t.Run("Receive and handle source_uri correctly", func(t *testing.T) { + emitter := &mockEmitter{} + + conf := map[string]interface{}{ + "agent.download.sourceURI": "test", + } + action := &fleetapi.ActionPolicyChange{ + ActionID: "abc123", + ActionType: "POLICY_CHANGE", + Policy: conf, + } + + cfg := configuration.DefaultConfiguration() + handler := &PolicyChange{ + log: log, + emitter: emitter.Emitter, + agentInfo: agentInfo, + config: cfg, + store: nullStore, + reloaders: map[string]ReloadFunc{ + "agent.download.sourceURI": func(value interface{}) error { + if strVal, ok := value.(string); !ok { + return errors.New("provided soruce_uri is not a string") + } else if strVal != "" { + cfg.Settings.DownloadConfig.SourceURI = strVal + } + return nil + }, + }, + } + + err := handler.Handle(context.Background(), action, ack) + require.NoError(t, err) + require.Equal(t, "test", cfg.Settings.DownloadConfig.SourceURI) + }) + + t.Run("Receive and handle source_uri correctly, broken keys", func(t *testing.T) { + emitter := &mockEmitter{} + + conf := map[string]interface{}{ + "agent": map[string]interface{}{ + "download": map[string]interface{}{ + "sourceURI": "test", + }, + }, + } + action := &fleetapi.ActionPolicyChange{ + ActionID: "abc123", + ActionType: "POLICY_CHANGE", + Policy: conf, + } + + cfg := configuration.DefaultConfiguration() + handler := &PolicyChange{ + log: log, + emitter: emitter.Emitter, + agentInfo: agentInfo, + config: cfg, + store: nullStore, + reloaders: map[string]ReloadFunc{ + "agent.download.sourceURI": func(value interface{}) error { + if strVal, ok := value.(string); !ok { + return errors.New("provided soruce_uri is not a string") + } else if strVal != "" { + cfg.Settings.DownloadConfig.SourceURI = strVal + } + return nil + }, + }, + } + + err := handler.Handle(context.Background(), action, ack) + require.NoError(t, err) + require.Equal(t, "test", cfg.Settings.DownloadConfig.SourceURI) + }) } func TestPolicyAcked(t *testing.T) { From 319bd630c3ce74e98c2ddabf77e442d8a5b1764f Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 10:31:47 +0200 Subject: [PATCH 02/21] typo --- internal/pkg/agent/application/managed_mode.go | 2 +- .../actions/handlers/handler_action_policy_change_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/pkg/agent/application/managed_mode.go b/internal/pkg/agent/application/managed_mode.go index 154e0d3acda..c64cdece3f8 100644 --- a/internal/pkg/agent/application/managed_mode.go +++ b/internal/pkg/agent/application/managed_mode.go @@ -210,7 +210,7 @@ func newManaged( map[string]handlers.ReloadFunc{ "agent.download.sourceURI": func(value interface{}) error { if strVal, ok := value.(string); !ok { - return errors.New("provided soruce_uri is not a string") + return errors.New("provided source_uri is not a string") } else if strVal != "" { cfg.Settings.DownloadConfig.SourceURI = strVal } diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index 7047bd5f3c9..d41b65e494a 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -109,7 +109,7 @@ func TestPolicyChange(t *testing.T) { reloaders: map[string]ReloadFunc{ "agent.download.sourceURI": func(value interface{}) error { if strVal, ok := value.(string); !ok { - return errors.New("provided soruce_uri is not a string") + return errors.New("provided source_uri is not a string") } else if strVal != "" { cfg.Settings.DownloadConfig.SourceURI = strVal } @@ -149,7 +149,7 @@ func TestPolicyChange(t *testing.T) { reloaders: map[string]ReloadFunc{ "agent.download.sourceURI": func(value interface{}) error { if strVal, ok := value.(string); !ok { - return errors.New("provided soruce_uri is not a string") + return errors.New("provided source_uri is not a string") } else if strVal != "" { cfg.Settings.DownloadConfig.SourceURI = strVal } From 78cd614d350cc8961eec5518dfdbe69aa78d0435 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 10:57:01 +0200 Subject: [PATCH 03/21] typo --- .../actions/handlers/handler_action_policy_change.go | 2 +- .../actions/handlers/handler_action_policy_change_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go index 06523ece1ac..9465fe83382 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go @@ -228,7 +228,7 @@ func (h *PolicyChange) handleFleetServerHosts(ctx context.Context, c *config.Con errors.TypeNetwork, errors.M("hosts", h.config.Fleet.Client.Hosts)) } // discard body for proper cancellation and connection reuse - io.Copy(ioutil.Discard, resp.Body) + _, _ = io.Copy(ioutil.Discard, resp.Body) resp.Body.Close() reader, err := fleetToReader(h.agentInfo, h.config) diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index d41b65e494a..c019a633600 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -44,7 +44,7 @@ func TestPolicyChange(t *testing.T) { conf := map[string]interface{}{"hello": "world"} action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123", + ActionID: "abc123-test1", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -69,7 +69,7 @@ func TestPolicyChange(t *testing.T) { conf := map[string]interface{}{"hello": "world"} action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123", + ActionID: "abc123-test2", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -94,7 +94,7 @@ func TestPolicyChange(t *testing.T) { "agent.download.sourceURI": "test", } action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123", + ActionID: "abc123-test3", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -134,7 +134,7 @@ func TestPolicyChange(t *testing.T) { }, } action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123", + ActionID: "abc123-test4", ActionType: "POLICY_CHANGE", Policy: conf, } From 19082fcf38ddc7e914ba876e95ce1e80ef752536 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 11:17:14 +0200 Subject: [PATCH 04/21] typo --- .../actions/handlers/handler_action_policy_change_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index c019a633600..d894c92bd51 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -44,7 +44,7 @@ func TestPolicyChange(t *testing.T) { conf := map[string]interface{}{"hello": "world"} action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123-test1", + ActionID: "abc1", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -69,7 +69,7 @@ func TestPolicyChange(t *testing.T) { conf := map[string]interface{}{"hello": "world"} action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123-test2", + ActionID: "abc2", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -94,7 +94,7 @@ func TestPolicyChange(t *testing.T) { "agent.download.sourceURI": "test", } action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123-test3", + ActionID: "abc3", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -134,7 +134,7 @@ func TestPolicyChange(t *testing.T) { }, } action := &fleetapi.ActionPolicyChange{ - ActionID: "abc123-test4", + ActionID: "abc4", ActionType: "POLICY_CHANGE", Policy: conf, } From 635f75bfc1c02f130ed0c99651f405e557c5e341 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 11:17:59 +0200 Subject: [PATCH 05/21] typo --- .../handlers/handler_action_policy_change_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index d894c92bd51..30cb149c870 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -44,7 +44,7 @@ func TestPolicyChange(t *testing.T) { conf := map[string]interface{}{"hello": "world"} action := &fleetapi.ActionPolicyChange{ - ActionID: "abc1", + ActionID: "TestPolicyChange-abc1", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -69,7 +69,7 @@ func TestPolicyChange(t *testing.T) { conf := map[string]interface{}{"hello": "world"} action := &fleetapi.ActionPolicyChange{ - ActionID: "abc2", + ActionID: "TestPolicyChange-abc2", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -94,7 +94,7 @@ func TestPolicyChange(t *testing.T) { "agent.download.sourceURI": "test", } action := &fleetapi.ActionPolicyChange{ - ActionID: "abc3", + ActionID: "TestPolicyChange-abc3", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -134,7 +134,7 @@ func TestPolicyChange(t *testing.T) { }, } action := &fleetapi.ActionPolicyChange{ - ActionID: "abc4", + ActionID: "TestPolicyChange-abc4", ActionType: "POLICY_CHANGE", Policy: conf, } @@ -176,7 +176,7 @@ func TestPolicyAcked(t *testing.T) { emitter := &mockEmitter{err: mockErr} config := map[string]interface{}{"hello": "world"} - actionID := "abc123" + actionID := "TestPolicyAcked-abc123" action := &fleetapi.ActionPolicyChange{ ActionID: actionID, ActionType: "POLICY_CHANGE", @@ -205,7 +205,7 @@ func TestPolicyAcked(t *testing.T) { emitter := &mockEmitter{} config := map[string]interface{}{"hello": "world"} - actionID := "abc123" + actionID := "TestPolicyAcked-abc123" action := &fleetapi.ActionPolicyChange{ ActionID: actionID, ActionType: "POLICY_CHANGE", From 3b89b2b46c1e94402f36eb1fb01874088e171e84 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 15:16:01 +0200 Subject: [PATCH 06/21] use reloadable --- .../pkg/agent/application/managed_mode.go | 12 +-- .../handlers/handler_action_policy_change.go | 77 ------------------- .../handler_action_policy_change_test.go | 76 ------------------ internal/pkg/artifact/config.go | 31 ++++++++ 4 files changed, 33 insertions(+), 163 deletions(-) diff --git a/internal/pkg/agent/application/managed_mode.go b/internal/pkg/agent/application/managed_mode.go index c64cdece3f8..8d811de155c 100644 --- a/internal/pkg/agent/application/managed_mode.go +++ b/internal/pkg/agent/application/managed_mode.go @@ -31,6 +31,7 @@ import ( "github.com/elastic/elastic-agent/internal/pkg/agent/operation" "github.com/elastic/elastic-agent/internal/pkg/agent/storage" "github.com/elastic/elastic-agent/internal/pkg/agent/storage/store" + "github.com/elastic/elastic-agent/internal/pkg/artifact" "github.com/elastic/elastic-agent/internal/pkg/capabilities" "github.com/elastic/elastic-agent/internal/pkg/composable" "github.com/elastic/elastic-agent/internal/pkg/config" @@ -157,6 +158,7 @@ func newManaged( }, caps, monitor, + artifact.NewReloader(cfg.Settings.DownloadConfig), ) if err != nil { return nil, err @@ -207,16 +209,6 @@ func newManaged( agentInfo, cfg, storeSaver, - map[string]handlers.ReloadFunc{ - "agent.download.sourceURI": func(value interface{}) error { - if strVal, ok := value.(string); !ok { - return errors.New("provided source_uri is not a string") - } else if strVal != "" { - cfg.Settings.DownloadConfig.SourceURI = strVal - } - return nil - }, - }, ) actionDispatcher.MustRegister( diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go index 9465fe83382..ad75299e420 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change.go @@ -11,7 +11,6 @@ import ( "io" "io/ioutil" "sort" - "strings" "time" "gopkg.in/yaml.v2" @@ -34,8 +33,6 @@ const ( apiStatusTimeout = 15 * time.Second ) -type ReloadFunc func(value interface{}) error - // PolicyChange is a handler for POLICY_CHANGE action. type PolicyChange struct { log *logger.Logger @@ -44,7 +41,6 @@ type PolicyChange struct { config *configuration.Configuration store storage.Store setters []actions.ClientSetter - reloaders map[string]ReloadFunc } // NewPolicyChange creates a new PolicyChange handler. @@ -54,7 +50,6 @@ func NewPolicyChange( agentInfo *info.AgentInfo, config *configuration.Configuration, store storage.Store, - reloaders map[string]ReloadFunc, setters ...actions.ClientSetter, ) *PolicyChange { return &PolicyChange{ @@ -64,7 +59,6 @@ func NewPolicyChange( config: config, store: store, setters: setters, - reloaders: reloaders, } } @@ -90,11 +84,6 @@ func (h *PolicyChange) Handle(ctx context.Context, a fleetapi.Action, acker stor return errors.New(err, "could not parse the configuration from the policy", errors.TypeConfig) } - err = h.handleReloads(ctx, c) - if err != nil { - return err - } - h.log.Debugf("handlerPolicyChange: emit configuration for action %+v", a) err = h.handleFleetServerHosts(ctx, c) if err != nil { @@ -107,72 +96,6 @@ func (h *PolicyChange) Handle(ctx context.Context, a fleetapi.Action, acker stor return acker.Ack(ctx, action) } -func (h *PolicyChange) handleReloads(ctx context.Context, c *config.Config) error { - if len(h.reloaders) == 0 { - return nil - } - - data, err := c.ToMapStr() - if err != nil { - return errors.New(err, "could not convert the configuration from the policy", errors.TypeConfig) - } - - for key, reloader := range h.reloaders { - if err := h.handleReload(ctx, data, key, reloader); err != nil { - return err - } - } - return nil -} - -func (h *PolicyChange) handleReload(ctx context.Context, configMap map[string]interface{}, key string, reloader ReloadFunc) error { - if configMap == nil { - return nil - } - - findFn := func(key string, data map[string]interface{}) (interface{}, bool) { - for { - // break loop on cancellation - if ctx.Err() != nil { - return nil, false - } - if key == "" { - return nil, false - } - - if val, found := data[key]; found { - return val, true - } - - sepIdx := strings.IndexRune(key, '.') - if sepIdx < 0 { - // simple key, no config counterpart found - return nil, false - } - - k := key[:sepIdx] - val, found := data[k] - if !found { - return nil, false - } - - expectedMap, ok := val.(map[string]interface{}) - if !ok { - return nil, false - } - - data = expectedMap - key = key[sepIdx+1:] - } - } - - if val, found := findFn(key, configMap); found { - return reloader(val) - } - - return nil -} - func (h *PolicyChange) handleFleetServerHosts(ctx context.Context, c *config.Config) (err error) { // do not update fleet-server host from policy; no setters provided with local Fleet Server if len(h.setters) == 0 { diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index 30cb149c870..866c2c89b63 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -86,82 +86,6 @@ func TestPolicyChange(t *testing.T) { err := handler.Handle(context.Background(), action, ack) require.Error(t, err) }) - - t.Run("Receive and handle source_uri correctly", func(t *testing.T) { - emitter := &mockEmitter{} - - conf := map[string]interface{}{ - "agent.download.sourceURI": "test", - } - action := &fleetapi.ActionPolicyChange{ - ActionID: "TestPolicyChange-abc3", - ActionType: "POLICY_CHANGE", - Policy: conf, - } - - cfg := configuration.DefaultConfiguration() - handler := &PolicyChange{ - log: log, - emitter: emitter.Emitter, - agentInfo: agentInfo, - config: cfg, - store: nullStore, - reloaders: map[string]ReloadFunc{ - "agent.download.sourceURI": func(value interface{}) error { - if strVal, ok := value.(string); !ok { - return errors.New("provided source_uri is not a string") - } else if strVal != "" { - cfg.Settings.DownloadConfig.SourceURI = strVal - } - return nil - }, - }, - } - - err := handler.Handle(context.Background(), action, ack) - require.NoError(t, err) - require.Equal(t, "test", cfg.Settings.DownloadConfig.SourceURI) - }) - - t.Run("Receive and handle source_uri correctly, broken keys", func(t *testing.T) { - emitter := &mockEmitter{} - - conf := map[string]interface{}{ - "agent": map[string]interface{}{ - "download": map[string]interface{}{ - "sourceURI": "test", - }, - }, - } - action := &fleetapi.ActionPolicyChange{ - ActionID: "TestPolicyChange-abc4", - ActionType: "POLICY_CHANGE", - Policy: conf, - } - - cfg := configuration.DefaultConfiguration() - handler := &PolicyChange{ - log: log, - emitter: emitter.Emitter, - agentInfo: agentInfo, - config: cfg, - store: nullStore, - reloaders: map[string]ReloadFunc{ - "agent.download.sourceURI": func(value interface{}) error { - if strVal, ok := value.(string); !ok { - return errors.New("provided source_uri is not a string") - } else if strVal != "" { - cfg.Settings.DownloadConfig.SourceURI = strVal - } - return nil - }, - }, - } - - err := handler.Handle(context.Background(), action, ack) - require.NoError(t, err) - require.Equal(t, "test", cfg.Settings.DownloadConfig.SourceURI) - }) } func TestPolicyAcked(t *testing.T) { diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index c190c02d239..268b416a914 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -11,6 +11,8 @@ import ( "github.com/elastic/elastic-agent-libs/transport/httpcommon" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" + "github.com/elastic/elastic-agent/internal/pkg/config" + "github.com/elastic/elastic-agent/pkg/core/logger" ) const ( @@ -46,6 +48,35 @@ type Config struct { httpcommon.HTTPTransportSettings `config:",inline" yaml:",inline"` // Note: use anonymous struct for json inline } +type Reloader struct { + log *logger.Logger + cfg *Config +} + +func NewReloader(cfg *Config) *Reloader { + return &Reloader{cfg: cfg} +} + +func (r *Reloader) Reload(rawConfig *config.Config) error { + type c struct { + Config *Config `config:"agent.download" yaml:"agent.download" json:"agent.download"` + } + + cfg := &c{ + Config: DefaultConfig(), + } + if err := rawConfig.Unpack(&cfg); err != nil { + return err + } + + r.log.Debugf("Source URI changed from %q to %q", r.cfg.SourceURI, cfg.Config.SourceURI) + if cfg.Config.SourceURI != "" { + r.cfg.SourceURI = cfg.Config.SourceURI + } + + return nil +} + // DefaultConfig creates a config with pre-set default values. func DefaultConfig() *Config { transport := httpcommon.DefaultHTTPTransportSettings() From 48aa3875abdf2222bbafe725e64cd9d272879c49 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 15:19:30 +0200 Subject: [PATCH 07/21] use reloadable --- internal/pkg/agent/application/managed_mode_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/pkg/agent/application/managed_mode_test.go b/internal/pkg/agent/application/managed_mode_test.go index 3d93c4953b9..7f111eae322 100644 --- a/internal/pkg/agent/application/managed_mode_test.go +++ b/internal/pkg/agent/application/managed_mode_test.go @@ -61,7 +61,6 @@ func TestManagedModeRouting(t *testing.T) { agentInfo, cfg, nullStore, - nil, ), ) From 0649adf960385e811b2001ac8f591b5ddf76636b Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 7 Jul 2022 15:21:42 +0200 Subject: [PATCH 08/21] use reloadable --- .../actions/handlers/handler_action_policy_change_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go index 866c2c89b63..e2d480ee6fe 100644 --- a/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go +++ b/internal/pkg/agent/application/pipeline/actions/handlers/handler_action_policy_change_test.go @@ -100,7 +100,7 @@ func TestPolicyAcked(t *testing.T) { emitter := &mockEmitter{err: mockErr} config := map[string]interface{}{"hello": "world"} - actionID := "TestPolicyAcked-abc123" + actionID := "TestPolicyAcked-abc1" action := &fleetapi.ActionPolicyChange{ ActionID: actionID, ActionType: "POLICY_CHANGE", @@ -129,7 +129,7 @@ func TestPolicyAcked(t *testing.T) { emitter := &mockEmitter{} config := map[string]interface{}{"hello": "world"} - actionID := "TestPolicyAcked-abc123" + actionID := "TestPolicyAcked-abc2" action := &fleetapi.ActionPolicyChange{ ActionID: actionID, ActionType: "POLICY_CHANGE", From 5d08d6738a35113cf55337ed47404ddfb3ac161b Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 25 Jul 2022 15:50:27 +0200 Subject: [PATCH 09/21] fleet compatibility --- .../pkg/agent/application/managed_mode.go | 2 +- internal/pkg/artifact/config.go | 41 ++++++++++++++----- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/internal/pkg/agent/application/managed_mode.go b/internal/pkg/agent/application/managed_mode.go index 8d811de155c..3f98e78fd62 100644 --- a/internal/pkg/agent/application/managed_mode.go +++ b/internal/pkg/agent/application/managed_mode.go @@ -158,7 +158,7 @@ func newManaged( }, caps, monitor, - artifact.NewReloader(cfg.Settings.DownloadConfig), + artifact.NewReloader(cfg.Settings.DownloadConfig, log), ) if err != nil { return nil, err diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 268b416a914..0a95788c637 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -19,6 +19,8 @@ const ( darwin = "darwin" linux = "linux" windows = "windows" + + defaultSourceURI = "https://artifacts.elastic.co/downloads/" ) // Config is a configuration used for verifier and downloader @@ -53,25 +55,42 @@ type Reloader struct { cfg *Config } -func NewReloader(cfg *Config) *Reloader { - return &Reloader{cfg: cfg} +func NewReloader(cfg *Config, log *logger.Logger) *Reloader { + return &Reloader{ + cfg: cfg, + log: log, + } } func (r *Reloader) Reload(rawConfig *config.Config) error { - type c struct { - Config *Config `config:"agent.download" yaml:"agent.download" json:"agent.download"` - } + type reloadConfig struct { + // SourceURI: source of the artifacts, e.g https://artifacts.elastic.co/downloads/ + SourceURI string `json:"agent.download.sourceURI" config:"agent.download.sourceURI"` - cfg := &c{ - Config: DefaultConfig(), + // FleetSourceURI: source of the artifacts, e.g https://artifacts.elastic.co/downloads/ coming from fleet which uses + // different naming. + FleetSourceURI string `json:"agent.download.source_uri" config:"agent.download.source_uri"` } + cfg := &reloadConfig{} if err := rawConfig.Unpack(&cfg); err != nil { return err } - r.log.Debugf("Source URI changed from %q to %q", r.cfg.SourceURI, cfg.Config.SourceURI) - if cfg.Config.SourceURI != "" { - r.cfg.SourceURI = cfg.Config.SourceURI + var newSourceURI string + if cfg.FleetSourceURI != "" { + // fleet configuration takes precedense + newSourceURI = cfg.FleetSourceURI + } else if cfg.SourceURI != "" { + newSourceURI = cfg.SourceURI + } + + if newSourceURI != "" { + r.log.Infof("Source URI changed from %q to %q", r.cfg.SourceURI, newSourceURI) + r.cfg.SourceURI = newSourceURI + } else { + // source uri unset, reset to default + r.log.Infof("Source URI reset from %q to %q", r.cfg.SourceURI, defaultSourceURI) + r.cfg.SourceURI = defaultSourceURI } return nil @@ -87,7 +106,7 @@ func DefaultConfig() *Config { transport.Timeout = 10 * time.Minute return &Config{ - SourceURI: "https://artifacts.elastic.co/downloads/", + SourceURI: defaultSourceURI, TargetDirectory: paths.Downloads(), InstallPath: paths.Install(), HTTPTransportSettings: transport, From 10fa7b2a66dd5759732ed7d316bad856e4d9c157 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 25 Jul 2022 16:00:28 +0200 Subject: [PATCH 10/21] typo --- internal/pkg/artifact/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 0a95788c637..7b0db4af48c 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -78,7 +78,7 @@ func (r *Reloader) Reload(rawConfig *config.Config) error { var newSourceURI string if cfg.FleetSourceURI != "" { - // fleet configuration takes precedense + // fleet configuration takes precedence newSourceURI = cfg.FleetSourceURI } else if cfg.SourceURI != "" { newSourceURI = cfg.SourceURI From c61abb378400e77fa65a8729120feec6c7e3617f Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 26 Jul 2022 13:57:34 +0200 Subject: [PATCH 11/21] divide and conquer --- internal/pkg/agent/application/local_mode.go | 2 + internal/pkg/artifact/config.go | 77 ++++++++++++++++++-- internal/pkg/artifact/config_test.go | 40 ++++++++++ 3 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 internal/pkg/artifact/config_test.go diff --git a/internal/pkg/agent/application/local_mode.go b/internal/pkg/agent/application/local_mode.go index 29f311fe582..ffcf40a6354 100644 --- a/internal/pkg/agent/application/local_mode.go +++ b/internal/pkg/agent/application/local_mode.go @@ -22,6 +22,7 @@ import ( "github.com/elastic/elastic-agent/internal/pkg/agent/configuration" "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/agent/operation" + "github.com/elastic/elastic-agent/internal/pkg/artifact" "github.com/elastic/elastic-agent/internal/pkg/capabilities" "github.com/elastic/elastic-agent/internal/pkg/composable" "github.com/elastic/elastic-agent/internal/pkg/config" @@ -131,6 +132,7 @@ func newLocal( }, caps, monitor, + artifact.NewReloader(cfg.Settings.DownloadConfig, log), ) if err != nil { return nil, err diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 7b0db4af48c..03099d85c02 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -25,6 +25,12 @@ const ( // Config is a configuration used for verifier and downloader type Config struct { + agentArtifactSettings `config:",inline" yaml:",inline"` + + httpcommon.HTTPTransportSettings `config:",inline" yaml:",inline"` // Note: use anonymous struct for json inline +} + +type agentArtifactSettings struct { // OperatingSystem: operating system [linux, windows, darwin] OperatingSystem string `json:"-" config:",ignore"` @@ -46,8 +52,6 @@ type Config struct { // local or network disk. // If not provided FileSystem Downloader will fallback to /beats subfolder of elastic-agent directory. DropPath string `yaml:"dropPath" config:"drop_path"` - - httpcommon.HTTPTransportSettings `config:",inline" yaml:",inline"` // Note: use anonymous struct for json inline } type Reloader struct { @@ -63,6 +67,66 @@ func NewReloader(cfg *Config, log *logger.Logger) *Reloader { } func (r *Reloader) Reload(rawConfig *config.Config) error { + if err := r.reloadArtifactSettings(rawConfig); err != nil { + return err + } + + if err := r.reloadTransport(rawConfig); err != nil { + return err + } + + if err := r.reloadSourceURI(rawConfig); err != nil { + return err + } + + return nil +} + +func (r *Reloader) reloadArtifactSettings(rawConfig *config.Config) error { + type artifactSettings struct { + Config agentArtifactSettings `json:"agent.download" config:"agent.download"` + } + + cfg := &artifactSettings{} + cfg.Config = DefaultConfig().agentArtifactSettings + if err := rawConfig.Unpack(&cfg); err != nil { + return err + } + + if cfg.Config.DropPath != "" { + r.cfg.DropPath = cfg.Config.DropPath + } + if cfg.Config.TargetDirectory != "" { + r.cfg.TargetDirectory = cfg.Config.TargetDirectory + } + if cfg.Config.InstallPath != "" { + r.cfg.InstallPath = cfg.Config.InstallPath + } + return nil +} + +func (r *Reloader) reloadTransport(rawConfig *config.Config) error { + type transportSettings struct { + Config httpcommon.HTTPTransportSettings `json:"agent.download" config:"agent.download"` + } + + cfg := &transportSettings{} + cfg.Config = DefaultConfig().HTTPTransportSettings + if err := rawConfig.Unpack(&cfg); err != nil { + return err + } + + if cfg.Config.TLS != nil { + r.cfg.TLS = cfg.Config.TLS + } + + r.cfg.Proxy = cfg.Config.Proxy + r.cfg.Timeout = cfg.Config.Timeout + + return nil +} + +func (r *Reloader) reloadSourceURI(rawConfig *config.Config) error { type reloadConfig struct { // SourceURI: source of the artifacts, e.g https://artifacts.elastic.co/downloads/ SourceURI string `json:"agent.download.sourceURI" config:"agent.download.sourceURI"` @@ -105,10 +169,13 @@ func DefaultConfig() *Config { // The HTTP download will log progress in the case that it is taking a while to download. transport.Timeout = 10 * time.Minute + artifactSettings := &agentArtifactSettings{ + SourceURI: defaultSourceURI, + TargetDirectory: paths.Downloads(), + InstallPath: paths.Install(), + } return &Config{ - SourceURI: defaultSourceURI, - TargetDirectory: paths.Downloads(), - InstallPath: paths.Install(), + agentArtifactSettings: *artifactSettings, HTTPTransportSettings: transport, } } diff --git a/internal/pkg/artifact/config_test.go b/internal/pkg/artifact/config_test.go new file mode 100644 index 00000000000..d3f84abd8cc --- /dev/null +++ b/internal/pkg/artifact/config_test.go @@ -0,0 +1,40 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package artifact + +import ( + "testing" + + "github.com/elastic/elastic-agent/internal/pkg/config" + "github.com/elastic/elastic-agent/pkg/core/logger" + "github.com/stretchr/testify/require" +) + +func TestReload(t *testing.T) { + cfg := DefaultConfig() + l, _ := logger.NewTesting("t") + reloader := NewReloader(cfg, l) + + input := `agent.download: + sourceURI: "testing.uri" + target_directory: "a/b/c" + install_path: "i/p" + drop_path: "d/p" + ssl.enabled: true + proxy_disable: true +` + + c, err := config.NewConfigFrom(input) + require.NoError(t, err) + + require.NoError(t, reloader.Reload(c)) + + require.Equal(t, "testing.uri", cfg.SourceURI) + require.Equal(t, "a/b/c", cfg.TargetDirectory) + require.NotNil(t, cfg.TLS) + require.Equal(t, true, *cfg.TLS.Enabled) + require.NotNil(t, cfg.Proxy) + require.Equal(t, true, cfg.Proxy.Disable) +} From 6d3c94f00cfb5f4ac4b46f30e45e4cb283ddff1f Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 26 Jul 2022 14:12:40 +0200 Subject: [PATCH 12/21] wrap --- internal/pkg/artifact/config.go | 12 +++++------- internal/pkg/artifact/config_test.go | 4 +++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 03099d85c02..b71cab0e9bc 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -11,6 +11,7 @@ import ( "github.com/elastic/elastic-agent-libs/transport/httpcommon" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" + "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/config" "github.com/elastic/elastic-agent/pkg/core/logger" ) @@ -68,15 +69,15 @@ func NewReloader(cfg *Config, log *logger.Logger) *Reloader { func (r *Reloader) Reload(rawConfig *config.Config) error { if err := r.reloadArtifactSettings(rawConfig); err != nil { - return err + return errors.New(err, "failed to reload artifact settings") } if err := r.reloadTransport(rawConfig); err != nil { - return err + return errors.New(err, "failed to reload transport settings") } if err := r.reloadSourceURI(rawConfig); err != nil { - return err + return errors.New(err, "failed to reload source URI") } return nil @@ -116,10 +117,7 @@ func (r *Reloader) reloadTransport(rawConfig *config.Config) error { return err } - if cfg.Config.TLS != nil { - r.cfg.TLS = cfg.Config.TLS - } - + r.cfg.TLS = cfg.Config.TLS r.cfg.Proxy = cfg.Config.Proxy r.cfg.Timeout = cfg.Config.Timeout diff --git a/internal/pkg/artifact/config_test.go b/internal/pkg/artifact/config_test.go index d3f84abd8cc..68edbed611c 100644 --- a/internal/pkg/artifact/config_test.go +++ b/internal/pkg/artifact/config_test.go @@ -22,8 +22,9 @@ func TestReload(t *testing.T) { target_directory: "a/b/c" install_path: "i/p" drop_path: "d/p" - ssl.enabled: true proxy_disable: true + ssl.enabled: true + ssl.ca_trusted_fingerprint: "my_finger_print" ` c, err := config.NewConfigFrom(input) @@ -35,6 +36,7 @@ func TestReload(t *testing.T) { require.Equal(t, "a/b/c", cfg.TargetDirectory) require.NotNil(t, cfg.TLS) require.Equal(t, true, *cfg.TLS.Enabled) + require.Equal(t, "my_finger_print", cfg.TLS.CATrustedFingerprint) require.NotNil(t, cfg.Proxy) require.Equal(t, true, cfg.Proxy.Disable) } From 6952e6f104e5340e08ca235895978641b123b073 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 26 Jul 2022 14:34:17 +0200 Subject: [PATCH 13/21] vet --- internal/pkg/agent/operation/common_test.go | 14 +++++---- .../pkg/agent/operation/monitoring_test.go | 6 ++-- internal/pkg/artifact/config.go | 12 ++++---- .../pkg/artifact/download/fs/verifier_test.go | 22 ++++++++------ .../artifact/download/http/downloader_test.go | 30 +++++++++++-------- .../artifact/download/http/elastic_test.go | 12 +++++--- .../artifact/download/snapshot/downloader.go | 14 +++++---- 7 files changed, 66 insertions(+), 44 deletions(-) diff --git a/internal/pkg/agent/operation/common_test.go b/internal/pkg/agent/operation/common_test.go index 60193c4c4e2..bada2c767f6 100644 --- a/internal/pkg/agent/operation/common_test.go +++ b/internal/pkg/agent/operation/common_test.go @@ -47,8 +47,10 @@ func getTestOperator(t *testing.T, downloadPath string, installPath string, p *a FailureTimeout: 1, // restart instantly }, DownloadConfig: &artifact.Config{ - TargetDirectory: downloadPath, - InstallPath: installPath, + AgentArtifactSettings: artifact.AgentArtifactSettings{ + TargetDirectory: downloadPath, + InstallPath: installPath, + }, }, LoggingConfig: logger.DefaultLoggingConfig(), } @@ -103,9 +105,11 @@ func getLogger() *logger.Logger { func getProgram(binary, version string) *app.Descriptor { spec := program.SupportedMap[binary] downloadCfg := &artifact.Config{ - InstallPath: installPath, - OperatingSystem: "darwin", - Architecture: "64", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + InstallPath: installPath, + OperatingSystem: "darwin", + Architecture: "64", + }, } return app.NewDescriptor(spec, version, downloadCfg, nil) } diff --git a/internal/pkg/agent/operation/monitoring_test.go b/internal/pkg/agent/operation/monitoring_test.go index cc365cae540..cd83e63cae2 100644 --- a/internal/pkg/agent/operation/monitoring_test.go +++ b/internal/pkg/agent/operation/monitoring_test.go @@ -150,8 +150,10 @@ func getMonitorableTestOperator(t *testing.T, installPath string, m monitoring.M }, ProcessConfig: &process.Config{}, DownloadConfig: &artifact.Config{ - InstallPath: installPath, - OperatingSystem: "darwin", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + InstallPath: installPath, + OperatingSystem: "darwin", + }, }, MonitoringConfig: mcfg, } diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index b71cab0e9bc..2fa74800d40 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -26,12 +26,12 @@ const ( // Config is a configuration used for verifier and downloader type Config struct { - agentArtifactSettings `config:",inline" yaml:",inline"` + AgentArtifactSettings `config:",inline" yaml:",inline"` httpcommon.HTTPTransportSettings `config:",inline" yaml:",inline"` // Note: use anonymous struct for json inline } -type agentArtifactSettings struct { +type AgentArtifactSettings struct { // OperatingSystem: operating system [linux, windows, darwin] OperatingSystem string `json:"-" config:",ignore"` @@ -85,11 +85,11 @@ func (r *Reloader) Reload(rawConfig *config.Config) error { func (r *Reloader) reloadArtifactSettings(rawConfig *config.Config) error { type artifactSettings struct { - Config agentArtifactSettings `json:"agent.download" config:"agent.download"` + Config AgentArtifactSettings `json:"agent.download" config:"agent.download"` } cfg := &artifactSettings{} - cfg.Config = DefaultConfig().agentArtifactSettings + cfg.Config = DefaultConfig().AgentArtifactSettings if err := rawConfig.Unpack(&cfg); err != nil { return err } @@ -167,13 +167,13 @@ func DefaultConfig() *Config { // The HTTP download will log progress in the case that it is taking a while to download. transport.Timeout = 10 * time.Minute - artifactSettings := &agentArtifactSettings{ + artifactSettings := &AgentArtifactSettings{ SourceURI: defaultSourceURI, TargetDirectory: paths.Downloads(), InstallPath: paths.Install(), } return &Config{ - agentArtifactSettings: *artifactSettings, + AgentArtifactSettings: *artifactSettings, HTTPTransportSettings: transport, } } diff --git a/internal/pkg/artifact/download/fs/verifier_test.go b/internal/pkg/artifact/download/fs/verifier_test.go index a758f90f300..226b29c6b50 100644 --- a/internal/pkg/artifact/download/fs/verifier_test.go +++ b/internal/pkg/artifact/download/fs/verifier_test.go @@ -48,11 +48,13 @@ func TestFetchVerify(t *testing.T) { defer os.RemoveAll(targetPath) config := &artifact.Config{ - TargetDirectory: targetPath, - DropPath: dropPath, - InstallPath: installPath, - OperatingSystem: "darwin", - Architecture: "32", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + TargetDirectory: targetPath, + DropPath: dropPath, + InstallPath: installPath, + OperatingSystem: "darwin", + Architecture: "32", + }, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, @@ -179,10 +181,12 @@ func TestVerify(t *testing.T) { timeout := 30 * time.Second config := &artifact.Config{ - TargetDirectory: targetDir, - DropPath: filepath.Join(targetDir, "drop"), - OperatingSystem: "linux", - Architecture: "32", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + TargetDirectory: targetDir, + DropPath: filepath.Join(targetDir, "drop"), + OperatingSystem: "linux", + Architecture: "32", + }, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, diff --git a/internal/pkg/artifact/download/http/downloader_test.go b/internal/pkg/artifact/download/http/downloader_test.go index aac16a60f5d..666ce1f6c02 100644 --- a/internal/pkg/artifact/download/http/downloader_test.go +++ b/internal/pkg/artifact/download/http/downloader_test.go @@ -49,10 +49,12 @@ func TestDownloadBodyError(t *testing.T) { } config := &artifact.Config{ - SourceURI: srv.URL, - TargetDirectory: targetDir, - OperatingSystem: "linux", - Architecture: "64", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + SourceURI: srv.URL, + TargetDirectory: targetDir, + OperatingSystem: "linux", + Architecture: "64", + }, } log := newRecordLogger() @@ -97,10 +99,12 @@ func TestDownloadLogProgressWithLength(t *testing.T) { } config := &artifact.Config{ - SourceURI: srv.URL, - TargetDirectory: targetDir, - OperatingSystem: "linux", - Architecture: "64", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + SourceURI: srv.URL, + TargetDirectory: targetDir, + OperatingSystem: "linux", + Architecture: "64", + }, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: totalTime, }, @@ -151,10 +155,12 @@ func TestDownloadLogProgressWithoutLength(t *testing.T) { } config := &artifact.Config{ - SourceURI: srv.URL, - TargetDirectory: targetDir, - OperatingSystem: "linux", - Architecture: "64", + AgentArtifactSettings: artifact.AgentArtifactSettings{ + SourceURI: srv.URL, + TargetDirectory: targetDir, + OperatingSystem: "linux", + Architecture: "64", + }, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: totalTime, }, diff --git a/internal/pkg/artifact/download/http/elastic_test.go b/internal/pkg/artifact/download/http/elastic_test.go index c29b8115089..b7c7e2a5e10 100644 --- a/internal/pkg/artifact/download/http/elastic_test.go +++ b/internal/pkg/artifact/download/http/elastic_test.go @@ -57,8 +57,10 @@ func TestDownload(t *testing.T) { elasticClient := getElasticCoClient() config := &artifact.Config{ - SourceURI: source, - TargetDirectory: targetDir, + AgentArtifactSettings: artifact.AgentArtifactSettings{ + SourceURI: source, + TargetDirectory: targetDir, + }, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, @@ -98,8 +100,10 @@ func TestVerify(t *testing.T) { elasticClient := getElasticCoClient() config := &artifact.Config{ - SourceURI: source, - TargetDirectory: targetDir, + AgentArtifactSettings: artifact.AgentArtifactSettings{ + SourceURI: source, + TargetDirectory: targetDir, + }, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, diff --git a/internal/pkg/artifact/download/snapshot/downloader.go b/internal/pkg/artifact/download/snapshot/downloader.go index 2fbe027ae4b..eaa9a633ded 100644 --- a/internal/pkg/artifact/download/snapshot/downloader.go +++ b/internal/pkg/artifact/download/snapshot/downloader.go @@ -34,12 +34,14 @@ func snapshotConfig(config *artifact.Config, versionOverride string) (*artifact. } return &artifact.Config{ - OperatingSystem: config.OperatingSystem, - Architecture: config.Architecture, - SourceURI: snapshotURI, - TargetDirectory: config.TargetDirectory, - InstallPath: config.InstallPath, - DropPath: config.DropPath, + AgentArtifactSettings: artifact.AgentArtifactSettings{ + OperatingSystem: config.OperatingSystem, + Architecture: config.Architecture, + SourceURI: snapshotURI, + TargetDirectory: config.TargetDirectory, + InstallPath: config.InstallPath, + DropPath: config.DropPath, + }, HTTPTransportSettings: config.HTTPTransportSettings, }, nil } From 4aaff6892c5804c376515c0a305b16cf6e044a4f Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 26 Jul 2022 14:39:49 +0200 Subject: [PATCH 14/21] lint --- internal/pkg/agent/application/local_mode.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/application/local_mode.go b/internal/pkg/agent/application/local_mode.go index ffcf40a6354..f06949bcba1 100644 --- a/internal/pkg/agent/application/local_mode.go +++ b/internal/pkg/agent/application/local_mode.go @@ -205,7 +205,7 @@ func (l *Local) AgentInfo() *info.AgentInfo { } func discoverer(patterns ...string) discoverFunc { - var p []string + p := make([]string, 0, len(patterns)) for _, newP := range patterns { if len(newP) == 0 { continue From 9e0d2d51553c9cfb22389bb768572a1e0846c522 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 27 Jul 2022 09:14:39 +0200 Subject: [PATCH 15/21] few other test cases --- internal/pkg/artifact/config.go | 10 +- internal/pkg/artifact/config_test.go | 220 +++++++++++++++++++++++++-- 2 files changed, 210 insertions(+), 20 deletions(-) diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 2fa74800d40..31e4ba106fc 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -139,13 +139,15 @@ func (r *Reloader) reloadSourceURI(rawConfig *config.Config) error { } var newSourceURI string - if cfg.FleetSourceURI != "" { + if fleetURI := strings.TrimSpace(cfg.FleetSourceURI); fleetURI != "" { // fleet configuration takes precedence - newSourceURI = cfg.FleetSourceURI - } else if cfg.SourceURI != "" { - newSourceURI = cfg.SourceURI + newSourceURI = fleetURI + } else if sourceURI := strings.TrimSpace(cfg.SourceURI); sourceURI != "" { + newSourceURI = sourceURI } + newSourceURI = strings.TrimSpace(newSourceURI) + if newSourceURI != "" { r.log.Infof("Source URI changed from %q to %q", r.cfg.SourceURI, newSourceURI) r.cfg.SourceURI = newSourceURI diff --git a/internal/pkg/artifact/config_test.go b/internal/pkg/artifact/config_test.go index 68edbed611c..8a1f0d9ff46 100644 --- a/internal/pkg/artifact/config_test.go +++ b/internal/pkg/artifact/config_test.go @@ -6,6 +6,7 @@ package artifact import ( "testing" + "time" "github.com/elastic/elastic-agent/internal/pkg/config" "github.com/elastic/elastic-agent/pkg/core/logger" @@ -13,30 +14,217 @@ import ( ) func TestReload(t *testing.T) { - cfg := DefaultConfig() - l, _ := logger.NewTesting("t") - reloader := NewReloader(cfg, l) - - input := `agent.download: + type testCase struct { + input string + initialConfig *Config + expectedSourceURI string + expectedTargetDirectory string + expectedInstallDirectory string + expectedDropDirectory string + expectedFingerprint string + expectedTLS bool + expectedTLSEnabled bool + expectedDisableProxy bool + expectedTimeout time.Duration + } + defaultValues := DefaultConfig() + testCases := []testCase{ + { + input: `agent.download: sourceURI: "testing.uri" target_directory: "a/b/c" install_path: "i/p" drop_path: "d/p" proxy_disable: true + timeout: 33s ssl.enabled: true ssl.ca_trusted_fingerprint: "my_finger_print" -` +`, + initialConfig: DefaultConfig(), + expectedSourceURI: "testing.uri", + expectedTargetDirectory: "a/b/c", + expectedInstallDirectory: "i/p", + expectedDropDirectory: "d/p", + expectedFingerprint: "my_finger_print", + expectedTLS: true, + expectedTLSEnabled: true, + expectedDisableProxy: true, + expectedTimeout: 33 * time.Second, + }, + { + input: `agent.download: + sourceURI: "testing.uri" +`, + initialConfig: DefaultConfig(), + expectedSourceURI: "testing.uri", + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + sourceURI: "" +`, + initialConfig: &Config{ + AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + HTTPTransportSettings: defaultValues.HTTPTransportSettings, + }, + expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to empty + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: ``, + initialConfig: &Config{ + AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + HTTPTransportSettings: defaultValues.HTTPTransportSettings, + }, + expectedSourceURI: defaultValues.SourceURI, // fallback to default when not set + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + sourceURI: " " +`, + initialConfig: &Config{ + AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + HTTPTransportSettings: defaultValues.HTTPTransportSettings, + }, + expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to whitespace + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + source_uri: " " +`, + initialConfig: &Config{ + AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + HTTPTransportSettings: defaultValues.HTTPTransportSettings, + }, + expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to whitespace + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + source_uri: " " + sourceURI: " " +`, + initialConfig: DefaultConfig(), + expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to whitespace + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: ``, + initialConfig: &Config{ + AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + HTTPTransportSettings: defaultValues.HTTPTransportSettings, + }, + expectedSourceURI: defaultValues.SourceURI, + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + source_uri: " " + sourceURI: "testing.uri" +`, + initialConfig: DefaultConfig(), + expectedSourceURI: "testing.uri", + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + source_uri: "testing.uri" + sourceURI: " " +`, + initialConfig: DefaultConfig(), + expectedSourceURI: "testing.uri", + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + } + + l, _ := logger.NewTesting("t") + for _, tc := range testCases { + cfg := tc.initialConfig + reloader := NewReloader(cfg, l) + + c, err := config.NewConfigFrom(tc.input) + require.NoError(t, err) + + require.NoError(t, reloader.Reload(c)) - c, err := config.NewConfigFrom(input) - require.NoError(t, err) + require.Equal(t, tc.expectedSourceURI, cfg.SourceURI) + require.Equal(t, tc.expectedTargetDirectory, cfg.TargetDirectory) + require.Equal(t, tc.expectedInstallDirectory, cfg.InstallPath) + require.Equal(t, tc.expectedDropDirectory, cfg.DropPath) - require.NoError(t, reloader.Reload(c)) + require.Equal(t, tc.expectedDisableProxy, cfg.Proxy.Disable) - require.Equal(t, "testing.uri", cfg.SourceURI) - require.Equal(t, "a/b/c", cfg.TargetDirectory) - require.NotNil(t, cfg.TLS) - require.Equal(t, true, *cfg.TLS.Enabled) - require.Equal(t, "my_finger_print", cfg.TLS.CATrustedFingerprint) - require.NotNil(t, cfg.Proxy) - require.Equal(t, true, cfg.Proxy.Disable) + if tc.expectedTLS { + require.NotNil(t, cfg.TLS) + require.Equal(t, tc.expectedTLSEnabled, *cfg.TLS.Enabled) + require.Equal(t, tc.expectedFingerprint, cfg.TLS.CATrustedFingerprint) + } else { + require.Nil(t, cfg.TLS) + } + } } From 0522a3e4091e8ef19e5c13dc09f1190b0c6a048a Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 27 Jul 2022 09:15:21 +0200 Subject: [PATCH 16/21] few other test cases --- internal/pkg/artifact/config_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/pkg/artifact/config_test.go b/internal/pkg/artifact/config_test.go index 8a1f0d9ff46..435bb901507 100644 --- a/internal/pkg/artifact/config_test.go +++ b/internal/pkg/artifact/config_test.go @@ -188,6 +188,22 @@ func TestReload(t *testing.T) { input: `agent.download: source_uri: "testing.uri" sourceURI: " " +`, + initialConfig: DefaultConfig(), + expectedSourceURI: "testing.uri", + expectedTargetDirectory: defaultValues.TargetDirectory, + expectedInstallDirectory: defaultValues.InstallPath, + expectedDropDirectory: defaultValues.DropPath, + expectedFingerprint: "", + expectedTLS: defaultValues.TLS != nil, + expectedTLSEnabled: false, + expectedDisableProxy: defaultValues.Proxy.Disable, + expectedTimeout: defaultValues.Timeout, + }, + { + input: `agent.download: + source_uri: "testing.uri" + sourceURI: "another.uri" `, initialConfig: DefaultConfig(), expectedSourceURI: "testing.uri", From 790888dc39e847015ad843d3d05192814621d007 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 27 Jul 2022 09:17:10 +0200 Subject: [PATCH 17/21] few other test cases --- internal/pkg/artifact/config_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/pkg/artifact/config_test.go b/internal/pkg/artifact/config_test.go index 435bb901507..8ca6269c7a8 100644 --- a/internal/pkg/artifact/config_test.go +++ b/internal/pkg/artifact/config_test.go @@ -232,6 +232,7 @@ func TestReload(t *testing.T) { require.Equal(t, tc.expectedTargetDirectory, cfg.TargetDirectory) require.Equal(t, tc.expectedInstallDirectory, cfg.InstallPath) require.Equal(t, tc.expectedDropDirectory, cfg.DropPath) + require.Equal(t, tc.expectedTimeout, cfg.Timeout) require.Equal(t, tc.expectedDisableProxy, cfg.Proxy.Disable) From 6037332ca1d51eba760a135b1c37037157224e37 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 28 Jul 2022 16:11:35 +0200 Subject: [PATCH 18/21] fixed unpack --- internal/pkg/agent/operation/common_test.go | 14 +-- .../pkg/agent/operation/monitoring_test.go | 6 +- internal/pkg/artifact/config.go | 112 ++++++++++-------- internal/pkg/artifact/config_test.go | 10 +- .../pkg/artifact/download/fs/verifier_test.go | 22 ++-- .../artifact/download/http/downloader_test.go | 30 ++--- .../artifact/download/http/elastic_test.go | 12 +- .../artifact/download/snapshot/downloader.go | 15 ++- 8 files changed, 106 insertions(+), 115 deletions(-) diff --git a/internal/pkg/agent/operation/common_test.go b/internal/pkg/agent/operation/common_test.go index bada2c767f6..60193c4c4e2 100644 --- a/internal/pkg/agent/operation/common_test.go +++ b/internal/pkg/agent/operation/common_test.go @@ -47,10 +47,8 @@ func getTestOperator(t *testing.T, downloadPath string, installPath string, p *a FailureTimeout: 1, // restart instantly }, DownloadConfig: &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - TargetDirectory: downloadPath, - InstallPath: installPath, - }, + TargetDirectory: downloadPath, + InstallPath: installPath, }, LoggingConfig: logger.DefaultLoggingConfig(), } @@ -105,11 +103,9 @@ func getLogger() *logger.Logger { func getProgram(binary, version string) *app.Descriptor { spec := program.SupportedMap[binary] downloadCfg := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - InstallPath: installPath, - OperatingSystem: "darwin", - Architecture: "64", - }, + InstallPath: installPath, + OperatingSystem: "darwin", + Architecture: "64", } return app.NewDescriptor(spec, version, downloadCfg, nil) } diff --git a/internal/pkg/agent/operation/monitoring_test.go b/internal/pkg/agent/operation/monitoring_test.go index cd83e63cae2..cc365cae540 100644 --- a/internal/pkg/agent/operation/monitoring_test.go +++ b/internal/pkg/agent/operation/monitoring_test.go @@ -150,10 +150,8 @@ func getMonitorableTestOperator(t *testing.T, installPath string, m monitoring.M }, ProcessConfig: &process.Config{}, DownloadConfig: &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - InstallPath: installPath, - OperatingSystem: "darwin", - }, + InstallPath: installPath, + OperatingSystem: "darwin", }, MonitoringConfig: mcfg, } diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 31e4ba106fc..0fdb8d07d83 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -9,6 +9,7 @@ import ( "strings" "time" + c "github.com/elastic/elastic-agent-libs/config" "github.com/elastic/elastic-agent-libs/transport/httpcommon" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" "github.com/elastic/elastic-agent/internal/pkg/agent/errors" @@ -26,12 +27,6 @@ const ( // Config is a configuration used for verifier and downloader type Config struct { - AgentArtifactSettings `config:",inline" yaml:",inline"` - - httpcommon.HTTPTransportSettings `config:",inline" yaml:",inline"` // Note: use anonymous struct for json inline -} - -type AgentArtifactSettings struct { // OperatingSystem: operating system [linux, windows, darwin] OperatingSystem string `json:"-" config:",ignore"` @@ -53,6 +48,8 @@ type AgentArtifactSettings struct { // local or network disk. // If not provided FileSystem Downloader will fallback to /beats subfolder of elastic-agent directory. DropPath string `yaml:"dropPath" config:"drop_path"` + + httpcommon.HTTPTransportSettings `config:",inline" yaml:",inline"` // Note: use anonymous struct for json inline } type Reloader struct { @@ -68,12 +65,8 @@ func NewReloader(cfg *Config, log *logger.Logger) *Reloader { } func (r *Reloader) Reload(rawConfig *config.Config) error { - if err := r.reloadArtifactSettings(rawConfig); err != nil { - return errors.New(err, "failed to reload artifact settings") - } - - if err := r.reloadTransport(rawConfig); err != nil { - return errors.New(err, "failed to reload transport settings") + if err := r.reloadConfig(rawConfig); err != nil { + return errors.New(err, "failed to reload source URI") } if err := r.reloadSourceURI(rawConfig); err != nil { @@ -83,43 +76,26 @@ func (r *Reloader) Reload(rawConfig *config.Config) error { return nil } -func (r *Reloader) reloadArtifactSettings(rawConfig *config.Config) error { - type artifactSettings struct { - Config AgentArtifactSettings `json:"agent.download" config:"agent.download"` - } - - cfg := &artifactSettings{} - cfg.Config = DefaultConfig().AgentArtifactSettings - if err := rawConfig.Unpack(&cfg); err != nil { - return err - } - - if cfg.Config.DropPath != "" { - r.cfg.DropPath = cfg.Config.DropPath - } - if cfg.Config.TargetDirectory != "" { - r.cfg.TargetDirectory = cfg.Config.TargetDirectory - } - if cfg.Config.InstallPath != "" { - r.cfg.InstallPath = cfg.Config.InstallPath +func (r *Reloader) reloadConfig(rawConfig *config.Config) error { + type reloadConfig struct { + C *Config `json:"agent.download" config:"agent.download"` } - return nil -} - -func (r *Reloader) reloadTransport(rawConfig *config.Config) error { - type transportSettings struct { - Config httpcommon.HTTPTransportSettings `json:"agent.download" config:"agent.download"` + tmp := &reloadConfig{ + C: DefaultConfig(), } - - cfg := &transportSettings{} - cfg.Config = DefaultConfig().HTTPTransportSettings - if err := rawConfig.Unpack(&cfg); err != nil { + if err := rawConfig.Unpack(&tmp); err != nil { return err } - r.cfg.TLS = cfg.Config.TLS - r.cfg.Proxy = cfg.Config.Proxy - r.cfg.Timeout = cfg.Config.Timeout + *(r.cfg) = Config{ + OperatingSystem: tmp.C.OperatingSystem, + Architecture: tmp.C.Architecture, + SourceURI: tmp.C.SourceURI, + TargetDirectory: tmp.C.TargetDirectory, + InstallPath: tmp.C.InstallPath, + DropPath: tmp.C.DropPath, + HTTPTransportSettings: tmp.C.HTTPTransportSettings, + } return nil } @@ -169,13 +145,10 @@ func DefaultConfig() *Config { // The HTTP download will log progress in the case that it is taking a while to download. transport.Timeout = 10 * time.Minute - artifactSettings := &AgentArtifactSettings{ - SourceURI: defaultSourceURI, - TargetDirectory: paths.Downloads(), - InstallPath: paths.Install(), - } return &Config{ - AgentArtifactSettings: *artifactSettings, + SourceURI: defaultSourceURI, + TargetDirectory: paths.Downloads(), + InstallPath: paths.Install(), HTTPTransportSettings: transport, } } @@ -214,3 +187,42 @@ func (c *Config) Arch() string { c.Architecture = arch return c.Architecture } + +// Unpack reads a config object into the settings. +func (settings *Config) Unpack(cfg *c.C) error { + tmp := struct { + OperatingSystem string `json:"-" config:",ignore"` + Architecture string `json:"-" config:",ignore"` + SourceURI string `json:"sourceURI" config:"sourceURI"` + TargetDirectory string `json:"targetDirectory" config:"target_directory"` + InstallPath string `yaml:"installPath" config:"install_path"` + DropPath string `yaml:"dropPath" config:"drop_path"` + }{ + OperatingSystem: settings.OperatingSystem, + Architecture: settings.Architecture, + SourceURI: settings.SourceURI, + TargetDirectory: settings.TargetDirectory, + InstallPath: settings.InstallPath, + DropPath: settings.DropPath, + } + + if err := cfg.Unpack(&tmp); err != nil { + return err + } + + transport := DefaultConfig().HTTPTransportSettings + if err := cfg.Unpack(&transport); err != nil { + return err + } + + *settings = Config{ + OperatingSystem: tmp.OperatingSystem, + Architecture: tmp.Architecture, + SourceURI: tmp.SourceURI, + TargetDirectory: tmp.TargetDirectory, + InstallPath: tmp.InstallPath, + DropPath: tmp.DropPath, + HTTPTransportSettings: transport, + } + return nil +} diff --git a/internal/pkg/artifact/config_test.go b/internal/pkg/artifact/config_test.go index 8ca6269c7a8..3a9a694b757 100644 --- a/internal/pkg/artifact/config_test.go +++ b/internal/pkg/artifact/config_test.go @@ -71,7 +71,7 @@ func TestReload(t *testing.T) { sourceURI: "" `, initialConfig: &Config{ - AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + SourceURI: "testing.uri", HTTPTransportSettings: defaultValues.HTTPTransportSettings, }, expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to empty @@ -87,7 +87,7 @@ func TestReload(t *testing.T) { { input: ``, initialConfig: &Config{ - AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + SourceURI: "testing.uri", HTTPTransportSettings: defaultValues.HTTPTransportSettings, }, expectedSourceURI: defaultValues.SourceURI, // fallback to default when not set @@ -105,7 +105,7 @@ func TestReload(t *testing.T) { sourceURI: " " `, initialConfig: &Config{ - AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + SourceURI: "testing.uri", HTTPTransportSettings: defaultValues.HTTPTransportSettings, }, expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to whitespace @@ -123,7 +123,7 @@ func TestReload(t *testing.T) { source_uri: " " `, initialConfig: &Config{ - AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + SourceURI: "testing.uri", HTTPTransportSettings: defaultValues.HTTPTransportSettings, }, expectedSourceURI: defaultValues.SourceURI, // fallback to default when set to whitespace @@ -155,7 +155,7 @@ func TestReload(t *testing.T) { { input: ``, initialConfig: &Config{ - AgentArtifactSettings: AgentArtifactSettings{SourceURI: "testing.uri"}, + SourceURI: "testing.uri", HTTPTransportSettings: defaultValues.HTTPTransportSettings, }, expectedSourceURI: defaultValues.SourceURI, diff --git a/internal/pkg/artifact/download/fs/verifier_test.go b/internal/pkg/artifact/download/fs/verifier_test.go index 226b29c6b50..a758f90f300 100644 --- a/internal/pkg/artifact/download/fs/verifier_test.go +++ b/internal/pkg/artifact/download/fs/verifier_test.go @@ -48,13 +48,11 @@ func TestFetchVerify(t *testing.T) { defer os.RemoveAll(targetPath) config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - TargetDirectory: targetPath, - DropPath: dropPath, - InstallPath: installPath, - OperatingSystem: "darwin", - Architecture: "32", - }, + TargetDirectory: targetPath, + DropPath: dropPath, + InstallPath: installPath, + OperatingSystem: "darwin", + Architecture: "32", HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, @@ -181,12 +179,10 @@ func TestVerify(t *testing.T) { timeout := 30 * time.Second config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - TargetDirectory: targetDir, - DropPath: filepath.Join(targetDir, "drop"), - OperatingSystem: "linux", - Architecture: "32", - }, + TargetDirectory: targetDir, + DropPath: filepath.Join(targetDir, "drop"), + OperatingSystem: "linux", + Architecture: "32", HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, diff --git a/internal/pkg/artifact/download/http/downloader_test.go b/internal/pkg/artifact/download/http/downloader_test.go index 666ce1f6c02..aac16a60f5d 100644 --- a/internal/pkg/artifact/download/http/downloader_test.go +++ b/internal/pkg/artifact/download/http/downloader_test.go @@ -49,12 +49,10 @@ func TestDownloadBodyError(t *testing.T) { } config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - SourceURI: srv.URL, - TargetDirectory: targetDir, - OperatingSystem: "linux", - Architecture: "64", - }, + SourceURI: srv.URL, + TargetDirectory: targetDir, + OperatingSystem: "linux", + Architecture: "64", } log := newRecordLogger() @@ -99,12 +97,10 @@ func TestDownloadLogProgressWithLength(t *testing.T) { } config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - SourceURI: srv.URL, - TargetDirectory: targetDir, - OperatingSystem: "linux", - Architecture: "64", - }, + SourceURI: srv.URL, + TargetDirectory: targetDir, + OperatingSystem: "linux", + Architecture: "64", HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: totalTime, }, @@ -155,12 +151,10 @@ func TestDownloadLogProgressWithoutLength(t *testing.T) { } config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - SourceURI: srv.URL, - TargetDirectory: targetDir, - OperatingSystem: "linux", - Architecture: "64", - }, + SourceURI: srv.URL, + TargetDirectory: targetDir, + OperatingSystem: "linux", + Architecture: "64", HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: totalTime, }, diff --git a/internal/pkg/artifact/download/http/elastic_test.go b/internal/pkg/artifact/download/http/elastic_test.go index b7c7e2a5e10..c29b8115089 100644 --- a/internal/pkg/artifact/download/http/elastic_test.go +++ b/internal/pkg/artifact/download/http/elastic_test.go @@ -57,10 +57,8 @@ func TestDownload(t *testing.T) { elasticClient := getElasticCoClient() config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - SourceURI: source, - TargetDirectory: targetDir, - }, + SourceURI: source, + TargetDirectory: targetDir, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, @@ -100,10 +98,8 @@ func TestVerify(t *testing.T) { elasticClient := getElasticCoClient() config := &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - SourceURI: source, - TargetDirectory: targetDir, - }, + SourceURI: source, + TargetDirectory: targetDir, HTTPTransportSettings: httpcommon.HTTPTransportSettings{ Timeout: timeout, }, diff --git a/internal/pkg/artifact/download/snapshot/downloader.go b/internal/pkg/artifact/download/snapshot/downloader.go index eaa9a633ded..c3680147927 100644 --- a/internal/pkg/artifact/download/snapshot/downloader.go +++ b/internal/pkg/artifact/download/snapshot/downloader.go @@ -34,14 +34,13 @@ func snapshotConfig(config *artifact.Config, versionOverride string) (*artifact. } return &artifact.Config{ - AgentArtifactSettings: artifact.AgentArtifactSettings{ - OperatingSystem: config.OperatingSystem, - Architecture: config.Architecture, - SourceURI: snapshotURI, - TargetDirectory: config.TargetDirectory, - InstallPath: config.InstallPath, - DropPath: config.DropPath, - }, + OperatingSystem: config.OperatingSystem, + Architecture: config.Architecture, + SourceURI: snapshotURI, + TargetDirectory: config.TargetDirectory, + InstallPath: config.InstallPath, + DropPath: config.DropPath, + HTTPTransportSettings: config.HTTPTransportSettings, }, nil } From 0923823f50c231aa5ce3c80bbf54d1f9e98e6764 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 28 Jul 2022 16:20:07 +0200 Subject: [PATCH 19/21] fixed lint --- internal/pkg/artifact/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 0fdb8d07d83..54d11ba25be 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -189,7 +189,7 @@ func (c *Config) Arch() string { } // Unpack reads a config object into the settings. -func (settings *Config) Unpack(cfg *c.C) error { +func (c *Config) Unpack(cfg *c.C) error { tmp := struct { OperatingSystem string `json:"-" config:",ignore"` Architecture string `json:"-" config:",ignore"` From 099ee222a0228b71de105e8feebbaf78368eb9e2 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 28 Jul 2022 16:28:36 +0200 Subject: [PATCH 20/21] fixed lint --- internal/pkg/artifact/config.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 54d11ba25be..c22f2a51f74 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -198,12 +198,12 @@ func (c *Config) Unpack(cfg *c.C) error { InstallPath string `yaml:"installPath" config:"install_path"` DropPath string `yaml:"dropPath" config:"drop_path"` }{ - OperatingSystem: settings.OperatingSystem, - Architecture: settings.Architecture, - SourceURI: settings.SourceURI, - TargetDirectory: settings.TargetDirectory, - InstallPath: settings.InstallPath, - DropPath: settings.DropPath, + OperatingSystem: c.OperatingSystem, + Architecture: c.Architecture, + SourceURI: c.SourceURI, + TargetDirectory: c.TargetDirectory, + InstallPath: c.InstallPath, + DropPath: c.DropPath, } if err := cfg.Unpack(&tmp); err != nil { @@ -215,7 +215,7 @@ func (c *Config) Unpack(cfg *c.C) error { return err } - *settings = Config{ + *c = Config{ OperatingSystem: tmp.OperatingSystem, Architecture: tmp.Architecture, SourceURI: tmp.SourceURI, From 1fdaeb1d68bbecfda91fd473e3934ce7c2e5b5e4 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 1 Aug 2022 08:09:44 +0200 Subject: [PATCH 21/21] comments resolved, trim optimised, error context back --- internal/pkg/artifact/config.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index c22f2a51f74..d88031e5de5 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -66,7 +66,7 @@ func NewReloader(cfg *Config, log *logger.Logger) *Reloader { func (r *Reloader) Reload(rawConfig *config.Config) error { if err := r.reloadConfig(rawConfig); err != nil { - return errors.New(err, "failed to reload source URI") + return errors.New(err, "failed to reload config") } if err := r.reloadSourceURI(rawConfig); err != nil { @@ -111,7 +111,7 @@ func (r *Reloader) reloadSourceURI(rawConfig *config.Config) error { } cfg := &reloadConfig{} if err := rawConfig.Unpack(&cfg); err != nil { - return err + return errors.New(err, "failed to unpack config during reload") } var newSourceURI string @@ -122,8 +122,6 @@ func (r *Reloader) reloadSourceURI(rawConfig *config.Config) error { newSourceURI = sourceURI } - newSourceURI = strings.TrimSpace(newSourceURI) - if newSourceURI != "" { r.log.Infof("Source URI changed from %q to %q", r.cfg.SourceURI, newSourceURI) r.cfg.SourceURI = newSourceURI