Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unpacking of artifact config #776

Merged
merged 22 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion internal/pkg/agent/application/local_mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -131,6 +132,7 @@ func newLocal(
},
caps,
monitor,
artifact.NewReloader(cfg.Settings.DownloadConfig, log),
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -203,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
Expand Down
14 changes: 9 additions & 5 deletions internal/pkg/agent/operation/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down Expand Up @@ -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)
}
Expand Down
6 changes: 4 additions & 2 deletions internal/pkg/agent/operation/monitoring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
76 changes: 70 additions & 6 deletions internal/pkg/artifact/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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"`

Expand All @@ -47,8 +53,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 {
Expand All @@ -64,6 +68,63 @@ 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.reloadSourceURI(rawConfig); err != nil {
return errors.New(err, "failed to reload source URI")
}

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
}

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"`
Expand All @@ -74,7 +135,7 @@ func (r *Reloader) Reload(rawConfig *config.Config) error {
}
cfg := &reloadConfig{}
if err := rawConfig.Unpack(&cfg); err != nil {
return errors.New(err, "failed to unpack config during reload")
return err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the benefit of removing the additional context from the the error here? it was wrapping the error before the change

}

var newSourceURI string
Expand Down Expand Up @@ -106,10 +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{
SourceURI: defaultSourceURI,
TargetDirectory: paths.Downloads(),
InstallPath: paths.Install(),
}
return &Config{
SourceURI: defaultSourceURI,
TargetDirectory: paths.Downloads(),
InstallPath: paths.Install(),
AgentArtifactSettings: *artifactSettings,
HTTPTransportSettings: transport,
}
}
Expand Down
42 changes: 42 additions & 0 deletions internal/pkg/artifact/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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"
proxy_disable: true
ssl.enabled: true
ssl.ca_trusted_fingerprint: "my_finger_print"
`

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.Equal(t, "my_finger_print", cfg.TLS.CATrustedFingerprint)
require.NotNil(t, cfg.Proxy)
require.Equal(t, true, cfg.Proxy.Disable)
}
22 changes: 13 additions & 9 deletions internal/pkg/artifact/download/fs/verifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down
30 changes: 18 additions & 12 deletions internal/pkg/artifact/download/http/downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down
12 changes: 8 additions & 4 deletions internal/pkg/artifact/download/http/elastic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down
14 changes: 8 additions & 6 deletions internal/pkg/artifact/download/snapshot/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down