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 all 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
84 changes: 80 additions & 4 deletions internal/pkg/artifact/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -64,6 +65,42 @@ 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 config")
}

if err := r.reloadSourceURI(rawConfig); err != nil {
return errors.New(err, "failed to reload source URI")
}

return nil
}

func (r *Reloader) reloadConfig(rawConfig *config.Config) error {
type reloadConfig struct {
C *Config `json:"agent.download" config:"agent.download"`
}
tmp := &reloadConfig{
C: DefaultConfig(),
}
if err := rawConfig.Unpack(&tmp); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

is it possible tmp.C is nil after this? do we need the nil check for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no we init with default and upack replaces values inside. nil C should not happen

return err
}

*(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
}

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 @@ -78,11 +115,11 @@ func (r *Reloader) Reload(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
}

if newSourceURI != "" {
Expand Down Expand Up @@ -148,3 +185,42 @@ func (c *Config) Arch() string {
c.Architecture = arch
return c.Architecture
}

// Unpack reads a config object into the settings.
func (c *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: c.OperatingSystem,
Architecture: c.Architecture,
SourceURI: c.SourceURI,
TargetDirectory: c.TargetDirectory,
InstallPath: c.InstallPath,
DropPath: c.DropPath,
}

if err := cfg.Unpack(&tmp); err != nil {
return err
}

transport := DefaultConfig().HTTPTransportSettings
if err := cfg.Unpack(&transport); err != nil {
return err
}

*c = Config{
OperatingSystem: tmp.OperatingSystem,
Architecture: tmp.Architecture,
SourceURI: tmp.SourceURI,
TargetDirectory: tmp.TargetDirectory,
InstallPath: tmp.InstallPath,
DropPath: tmp.DropPath,
HTTPTransportSettings: transport,
}
return nil
}
247 changes: 247 additions & 0 deletions internal/pkg/artifact/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
// 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"
"time"

"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) {
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{
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{
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{
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{
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{
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,
},
{
input: `agent.download:
source_uri: "testing.uri"
sourceURI: "another.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,
},
}

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))

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.Equal(t, tc.expectedTimeout, cfg.Timeout)

require.Equal(t, tc.expectedDisableProxy, 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)
}
}
}
Loading