From ce8729ec588081eda5e08d88afd6da056400dca6 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 21 Sep 2022 09:56:25 +0200 Subject: [PATCH 1/7] Fixed: source uri reload for download/verify components --- internal/pkg/agent/operation/operator.go | 39 ++++++++++++++++++++++++ internal/pkg/artifact/config.go | 8 ++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/internal/pkg/agent/operation/operator.go b/internal/pkg/agent/operation/operator.go index ed28b7cb633..0edf9972cd2 100644 --- a/internal/pkg/agent/operation/operator.go +++ b/internal/pkg/agent/operation/operator.go @@ -141,6 +141,12 @@ func (o *Operator) Reload(rawConfig *config.Config) error { return errors.New(err, "failed to unpack artifact config") } + sourceURI, err := o.reloadSourceURI(rawConfig) + if err != nil { + return errors.New(err, "failed to parse source URI") + } + tmp.C.SourceURI = sourceURI + if err := o.reloadComponent(o.downloader, "downloader", tmp.C); err != nil { return err } @@ -148,6 +154,39 @@ func (o *Operator) Reload(rawConfig *config.Config) error { return o.reloadComponent(o.verifier, "verifier", tmp.C) } +func (o *Operator) reloadSourceURI(rawConfig *config.Config) (string, 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"` + + // 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 "", errors.New(err, "failed to unpack config during reload") + } + + var newSourceURI string + if fleetURI := strings.TrimSpace(cfg.FleetSourceURI); fleetURI != "" { + // fleet configuration takes precedence + newSourceURI = fleetURI + } else if sourceURI := strings.TrimSpace(cfg.SourceURI); sourceURI != "" { + newSourceURI = sourceURI + } + + if newSourceURI != "" { + o.logger.Infof("Source URI in operator changed to %q", newSourceURI) + return newSourceURI, nil + } + + // source uri unset, reset to default + o.logger.Infof("Source URI in reset %q", artifact.DefaultSourceURI) + return artifact.DefaultSourceURI, nil + +} + func (o *Operator) reloadComponent(component interface{}, name string, cfg *artifact.Config) error { r, ok := component.(artifact.ConfigReloader) if !ok { diff --git a/internal/pkg/artifact/config.go b/internal/pkg/artifact/config.go index 76637c28d31..65c021ff9b3 100644 --- a/internal/pkg/artifact/config.go +++ b/internal/pkg/artifact/config.go @@ -22,7 +22,7 @@ const ( linux = "linux" windows = "windows" - defaultSourceURI = "https://artifacts.elastic.co/downloads/" + DefaultSourceURI = "https://artifacts.elastic.co/downloads/" ) type ConfigReloader interface { @@ -139,8 +139,8 @@ func (r *Reloader) reloadSourceURI(rawConfig *config.Config) error { 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 + r.log.Infof("Source URI reset from %q to %q", r.cfg.SourceURI, DefaultSourceURI) + r.cfg.SourceURI = DefaultSourceURI } return nil @@ -156,7 +156,7 @@ func DefaultConfig() *Config { transport.Timeout = 10 * time.Minute return &Config{ - SourceURI: defaultSourceURI, + SourceURI: DefaultSourceURI, TargetDirectory: paths.Downloads(), InstallPath: paths.Install(), HTTPTransportSettings: transport, From f9fb805c3bad745cb4ed48a4c6df12df7bc1b608 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 21 Sep 2022 15:49:00 +0200 Subject: [PATCH 2/7] UT --- internal/pkg/agent/operation/operator.go | 8 ++-- internal/pkg/agent/operation/operator_test.go | 48 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/internal/pkg/agent/operation/operator.go b/internal/pkg/agent/operation/operator.go index 0edf9972cd2..29d401efd86 100644 --- a/internal/pkg/agent/operation/operator.go +++ b/internal/pkg/agent/operation/operator.go @@ -141,7 +141,7 @@ func (o *Operator) Reload(rawConfig *config.Config) error { return errors.New(err, "failed to unpack artifact config") } - sourceURI, err := o.reloadSourceURI(rawConfig) + sourceURI, err := reloadSourceURI(o.logger, rawConfig) if err != nil { return errors.New(err, "failed to parse source URI") } @@ -154,7 +154,7 @@ func (o *Operator) Reload(rawConfig *config.Config) error { return o.reloadComponent(o.verifier, "verifier", tmp.C) } -func (o *Operator) reloadSourceURI(rawConfig *config.Config) (string, error) { +func reloadSourceURI(logger *logger.Logger, rawConfig *config.Config) (string, 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"` @@ -177,12 +177,12 @@ func (o *Operator) reloadSourceURI(rawConfig *config.Config) (string, error) { } if newSourceURI != "" { - o.logger.Infof("Source URI in operator changed to %q", newSourceURI) + logger.Infof("Source URI in operator changed to %q", newSourceURI) return newSourceURI, nil } // source uri unset, reset to default - o.logger.Infof("Source URI in reset %q", artifact.DefaultSourceURI) + logger.Infof("Source URI in reset %q", artifact.DefaultSourceURI) return artifact.DefaultSourceURI, nil } diff --git a/internal/pkg/agent/operation/operator_test.go b/internal/pkg/agent/operation/operator_test.go index 731f04eea8b..17797155618 100644 --- a/internal/pkg/agent/operation/operator_test.go +++ b/internal/pkg/agent/operation/operator_test.go @@ -15,10 +15,13 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent-client/v7/pkg/proto" "github.com/elastic/elastic-agent/internal/pkg/agent/program" + "github.com/elastic/elastic-agent/internal/pkg/artifact" + "github.com/elastic/elastic-agent/internal/pkg/config" "github.com/elastic/elastic-agent/internal/pkg/core/state" ) @@ -462,6 +465,51 @@ func TestConfigurableService(t *testing.T) { } } +func TestReloadSourceURI(t *testing.T) { + testCases := map[string]struct { + IncomingConfig map[string]interface{} + ExpectedSourceURI string + }{ + "no-config": { + IncomingConfig: map[string]interface{}{}, + ExpectedSourceURI: artifact.DefaultSourceURI, + }, + "source-uri-provided": { + IncomingConfig: map[string]interface{}{ + "agent.download.sourceURI": "http://source-uri", + }, + ExpectedSourceURI: "http://source-uri", + }, + "fleet-source-uri-provided": { + IncomingConfig: map[string]interface{}{ + "agent.download.source_uri": "http://fleet-source-uri", + }, + ExpectedSourceURI: "http://fleet-source-uri", + }, + "both-source-uri-provided": { + IncomingConfig: map[string]interface{}{ + "agent.download.sourceURI": "http://source-uri", + "agent.download.source_uri": "http://fleet-source-uri", + }, + ExpectedSourceURI: "http://fleet-source-uri", + }, + } + + l := getLogger() + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + cfg, err := config.NewConfigFrom(tc.IncomingConfig) + require.NoError(t, err) + require.NotNil(t, cfg) + + sourceUri, err := reloadSourceURI(l, cfg) + require.NoError(t, err) + require.Equal(t, tc.ExpectedSourceURI, sourceUri) + + }) + } +} + func isAvailable(name, version string) error { p := getProgram(name, version) spec := p.ProcessSpec() From 2f389528d8ae9f98f102e092e053c89864a63e90 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 21 Sep 2022 15:50:01 +0200 Subject: [PATCH 3/7] changelog --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 6047c71b3d7..e0a416502d2 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -115,6 +115,7 @@ - Use the Elastic Agent configuration directory as the root of the `inputs.d` folder. {issues}663[663] - Fix a panic caused by a race condition when installing the Elastic Agent. {issues}806[806] - Remove fleet event reporter and events from checkin body. {issue}993[993] +- Source uri reload for download/verify components {pull}1252[1252] ==== New features From b292b3c18dc3293c27a9a82693174225a0b80d77 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 21 Sep 2022 15:53:35 +0200 Subject: [PATCH 4/7] lint --- internal/pkg/agent/operation/operator_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/pkg/agent/operation/operator_test.go b/internal/pkg/agent/operation/operator_test.go index 17797155618..b482273d00d 100644 --- a/internal/pkg/agent/operation/operator_test.go +++ b/internal/pkg/agent/operation/operator_test.go @@ -74,7 +74,7 @@ func TestConfigurableRun(t *testing.T) { if err := operator.start(p, nil); err != nil { t.Fatal(err) } - defer operator.stop(p) // failure catch, to ensure no sub-process stays running + defer func() { _ = operator.stop(p) }() // failure catch, to ensure no sub-process stays running waitFor(t, func() error { items := operator.State() @@ -90,6 +90,7 @@ func TestConfigurableRun(t *testing.T) { // try to configure cfg := make(map[string]interface{}) + //nolint:gosec tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath if err := operator.pushConfig(p, cfg); err != nil { @@ -148,7 +149,7 @@ func TestConfigurableFailed(t *testing.T) { if err := operator.start(p, nil); err != nil { t.Fatal(err) } - defer operator.stop(p) // failure catch, to ensure no sub-process stays running + defer func() { _ = operator.stop(p) }() // failure catch, to ensure no sub-process stays running var pid int waitFor(t, func() error { @@ -175,6 +176,7 @@ func TestConfigurableFailed(t *testing.T) { // try to configure (with failed status) cfg := make(map[string]interface{}) + //nolint:gosec tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath cfg["Status"] = proto.StateObserved_FAILED @@ -257,7 +259,7 @@ func TestConfigurableCrash(t *testing.T) { if err := operator.start(p, nil); err != nil { t.Fatal(err) } - defer operator.stop(p) // failure catch, to ensure no sub-process stays running + defer func() { _ = operator.stop(p) }() // failure catch, to ensure no sub-process stays running var pid int waitFor(t, func() error { @@ -275,6 +277,7 @@ func TestConfigurableCrash(t *testing.T) { // try to configure (with failed status) cfg := make(map[string]interface{}) + //nolint:gosec tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath cfg["Crash"] = true @@ -355,7 +358,7 @@ func TestConfigurableStartStop(t *testing.T) { p := getProgram("configurable", "1.0") operator := getTestOperator(t, downloadPath, installPath, p) - defer operator.stop(p) // failure catch, to ensure no sub-process stays running + defer func() { _ = operator.stop(p) }() // failure catch, to ensure no sub-process stays running // start and stop it 3 times for i := 0; i < 3; i++ { @@ -399,7 +402,7 @@ func TestConfigurableService(t *testing.T) { if err := operator.start(p, nil); err != nil { t.Fatal(err) } - defer operator.stop(p) // failure catch, to ensure no sub-process stays running + defer func() { _ = operator.stop(p) }() // failure catch, to ensure no sub-process stays running // emulating a service, so we need to start the binary here in the test spec := p.ProcessSpec() @@ -426,6 +429,7 @@ func TestConfigurableService(t *testing.T) { // try to configure cfg := make(map[string]interface{}) + //nolint:gosec tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath if err := operator.pushConfig(p, cfg); err != nil { From 4e07d50104f08d363c95642caf96d656c2e9cfeb Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 21 Sep 2022 15:59:38 +0200 Subject: [PATCH 5/7] lint --- internal/pkg/agent/operation/operator_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/agent/operation/operator_test.go b/internal/pkg/agent/operation/operator_test.go index b482273d00d..84a64662629 100644 --- a/internal/pkg/agent/operation/operator_test.go +++ b/internal/pkg/agent/operation/operator_test.go @@ -90,7 +90,7 @@ func TestConfigurableRun(t *testing.T) { // try to configure cfg := make(map[string]interface{}) - //nolint:gosec + //nolint:gosec // rand is ok for test tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath if err := operator.pushConfig(p, cfg); err != nil { @@ -176,7 +176,7 @@ func TestConfigurableFailed(t *testing.T) { // try to configure (with failed status) cfg := make(map[string]interface{}) - //nolint:gosec + //nolint:gosec // rand is ok for test tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath cfg["Status"] = proto.StateObserved_FAILED @@ -277,7 +277,7 @@ func TestConfigurableCrash(t *testing.T) { // try to configure (with failed status) cfg := make(map[string]interface{}) - //nolint:gosec + //nolint:gosec // rand is ok for test tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath cfg["Crash"] = true @@ -429,7 +429,7 @@ func TestConfigurableService(t *testing.T) { // try to configure cfg := make(map[string]interface{}) - //nolint:gosec + //nolint:gosec // rand is ok for test tstFilePath := filepath.Join(os.TempDir(), fmt.Sprintf("tmp%d", rand.Uint32())) cfg["TestFile"] = tstFilePath if err := operator.pushConfig(p, cfg); err != nil { From 600a0629105384479d9c7804d8bc7eb1a495668c Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 21 Sep 2022 16:27:44 +0200 Subject: [PATCH 6/7] lint --- internal/pkg/agent/operation/operator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/operation/operator_test.go b/internal/pkg/agent/operation/operator_test.go index 84a64662629..5c0cf112ed5 100644 --- a/internal/pkg/agent/operation/operator_test.go +++ b/internal/pkg/agent/operation/operator_test.go @@ -406,7 +406,7 @@ func TestConfigurableService(t *testing.T) { // emulating a service, so we need to start the binary here in the test spec := p.ProcessSpec() - cmd := exec.Command(spec.BinaryPath, fmt.Sprintf("%d", p.ServicePort())) + cmd := exec.Command(spec.BinaryPath, fmt.Sprintf("%d", p.ServicePort())) //nolint:gosec,G204 // this is fine cmd.Env = append(cmd.Env, os.Environ()...) cmd.Dir = filepath.Dir(spec.BinaryPath) cmd.Stdout = os.Stdout From d5d5038b998ed172c483760df29d11857037f7f3 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 22 Sep 2022 08:25:17 +0200 Subject: [PATCH 7/7] Update CHANGELOG.next.asciidoc Co-authored-by: Craig MacKenzie --- CHANGELOG.next.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index e0a416502d2..c21529480c8 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -115,7 +115,7 @@ - Use the Elastic Agent configuration directory as the root of the `inputs.d` folder. {issues}663[663] - Fix a panic caused by a race condition when installing the Elastic Agent. {issues}806[806] - Remove fleet event reporter and events from checkin body. {issue}993[993] -- Source uri reload for download/verify components {pull}1252[1252] +- Fix unintended reset of source URI when downloading components {pull}1252[1252] ==== New features