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

[7.6] Merge default values with custom API Key Elasticsearch settings (#3342) #3356

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 1 addition & 1 deletion _meta/beat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ apm-server:
#proxy_disable: false

# Configure http request timeout before failing an request to Elasticsearch.
#timeout: 10s
#timeout: 5s

# Enable custom SSL settings. Set to false to ignore custom SSL settings for secure communication.
#ssl.enabled: true
Expand Down
2 changes: 1 addition & 1 deletion apm-server.docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ apm-server:
#proxy_disable: false

# Configure http request timeout before failing an request to Elasticsearch.
#timeout: 10s
#timeout: 5s

# Enable custom SSL settings. Set to false to ignore custom SSL settings for secure communication.
#ssl.enabled: true
Expand Down
2 changes: 1 addition & 1 deletion apm-server.yml
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ apm-server:
#proxy_disable: false

# Configure http request timeout before failing an request to Elasticsearch.
#timeout: 10s
#timeout: 5s

# Enable custom SSL settings. Set to false to ignore custom SSL settings for secure communication.
#ssl.enabled: true
Expand Down
25 changes: 18 additions & 7 deletions beater/config/api_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type APIKeyConfig struct {
Enabled bool `config:"enabled"`
LimitPerMin int `config:"limit"`
ESConfig *elasticsearch.Config `config:"elasticsearch"`

esConfigured bool
}

// IsEnabled returns whether or not API Key authorization is enabled
Expand All @@ -42,22 +44,31 @@ func (c *APIKeyConfig) IsEnabled() bool {
}

func (c *APIKeyConfig) setup(log *logp.Logger, outputESCfg *common.Config) error {
if c == nil || !c.Enabled || c.ESConfig != nil {
return nil
}
c.ESConfig = elasticsearch.DefaultConfig()
if outputESCfg == nil {
if c == nil || !c.Enabled || c.esConfigured || outputESCfg == nil {
return nil
}
log.Info("Falling back to elasticsearch output for API Key usage")
if err := outputESCfg.Unpack(c.ESConfig); err != nil {
return errors.Wrap(err, "unpacking Elasticsearch config into API key config")
}

return nil

}

func defaultAPIKeyConfig() *APIKeyConfig {
return &APIKeyConfig{Enabled: false, LimitPerMin: apiKeyLimit}
return &APIKeyConfig{Enabled: false, LimitPerMin: apiKeyLimit, ESConfig: elasticsearch.DefaultConfig()}
}

func (c *APIKeyConfig) Unpack(inp *common.Config) error {
cfg := tmpAPIKeyConfig(*defaultAPIKeyConfig())
if err := inp.Unpack(&cfg); err != nil {
return errors.Errorf("error unpacking api_key config: %w", err)
}
*c = APIKeyConfig(cfg)
if inp.HasField("elasticsearch") {
c.esConfigured = true
}
return nil
}

type tmpAPIKeyConfig APIKeyConfig
31 changes: 19 additions & 12 deletions beater/config/api_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/elastic/beats/libbeat/logp"

"github.com/stretchr/testify/assert"
Expand All @@ -38,39 +40,42 @@ func TestAPIKeyConfig_IsEnabled(t *testing.T) {

func TestAPIKeyConfig_ESConfig(t *testing.T) {
for name, tc := range map[string]struct {
cfg *APIKeyConfig
cfg *common.Config
esCfg *common.Config

expectedConfig *APIKeyConfig
expectedErr error
}{
"default": {
cfg: defaultAPIKeyConfig(),
cfg: common.NewConfig(),
expectedConfig: defaultAPIKeyConfig(),
},
"ES config missing": {
cfg: &APIKeyConfig{Enabled: true, LimitPerMin: apiKeyLimit},
cfg: common.MustNewConfigFrom(`{"enabled": true}`),
expectedConfig: &APIKeyConfig{
Enabled: true,
LimitPerMin: apiKeyLimit,
ESConfig: elasticsearch.DefaultConfig()},
},
"ES configured": {
cfg: &APIKeyConfig{
Enabled: true,
ESConfig: &elasticsearch.Config{Hosts: elasticsearch.Hosts{"192.0.0.1:9200"}}},
cfg: common.MustNewConfigFrom(`{"enabled": true, "elasticsearch.timeout":"7s"}`),
esCfg: common.MustNewConfigFrom(`{"hosts":["186.0.0.168:9200"]}`),
expectedConfig: &APIKeyConfig{
Enabled: true,
ESConfig: &elasticsearch.Config{Hosts: elasticsearch.Hosts{"192.0.0.1:9200"}}},
Enabled: true,
LimitPerMin: apiKeyLimit,
ESConfig: &elasticsearch.Config{
Hosts: elasticsearch.Hosts{"localhost:9200"},
Protocol: "http",
Timeout: 7 * time.Second},
esConfigured: true},
},
"disabled with ES from output": {
cfg: defaultAPIKeyConfig(),
cfg: common.NewConfig(),
esCfg: common.MustNewConfigFrom(`{"hosts":["192.0.0.168:9200"]}`),
expectedConfig: defaultAPIKeyConfig(),
},
"ES from output": {
cfg: &APIKeyConfig{Enabled: true, LimitPerMin: 20},
cfg: common.MustNewConfigFrom(`{"enabled": true, "limit": 20}`),
esCfg: common.MustNewConfigFrom(`{"hosts":["192.0.0.168:9200"],"username":"foo","password":"bar"}`),
expectedConfig: &APIKeyConfig{
Enabled: true,
Expand All @@ -84,13 +89,15 @@ func TestAPIKeyConfig_ESConfig(t *testing.T) {
},
} {
t.Run(name, func(t *testing.T) {
err := tc.cfg.setup(logp.NewLogger("api_key"), tc.esCfg)
var apiKeyConfig APIKeyConfig
require.NoError(t, tc.cfg.Unpack(&apiKeyConfig))
err := apiKeyConfig.setup(logp.NewLogger("api_key"), tc.esCfg)
if tc.expectedErr == nil {
assert.NoError(t, err)
} else {
assert.Error(t, err)
}
assert.Equal(t, tc.expectedConfig, tc.cfg)
assert.Equal(t, tc.expectedConfig, &apiKeyConfig)

})
}
Expand Down
6 changes: 5 additions & 1 deletion beater/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,11 @@ func Test_UnpackConfig(t *testing.T) {
APIKeyConfig: &APIKeyConfig{
Enabled: true,
LimitPerMin: 200,
ESConfig: &elasticsearch.Config{Hosts: elasticsearch.Hosts{"localhost:9201", "localhost:9202"}},
ESConfig: &elasticsearch.Config{
Hosts: elasticsearch.Hosts{"localhost:9201", "localhost:9202"},
Protocol: "http",
Timeout: 5 * time.Second},
esConfigured: true,
},
},
},
Expand Down
10 changes: 10 additions & 0 deletions changelogs/7.6.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,18 @@

https://github.com/elastic/apm-server/compare/7.5\...7.6[View commits]

* <<release-notes-7.6.1>>
* <<release-notes-7.6.0>>

[[release-notes-7.6.1]]
=== APM Server version 7.6.1

https://github.com/elastic/apm-server/compare/v7.6.0\...v7.6.1[View commits]

[float]
==== Bug fixes
* Merge default values with custom Elasticsearch config for API Keys and add `required` tag for `host` {pull}3342[3342]

[[release-notes-7.6.0]]
=== APM Server version 7.6.0

Expand Down
2 changes: 1 addition & 1 deletion elasticsearch/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var (

// Config holds all configurable fields that are used to create a Client
type Config struct {
Hosts Hosts `config:"hosts"`
Hosts Hosts `config:"hosts" validate:"required"`
Protocol string `config:"protocol"`
Path string `config:"path"`
ProxyURL string `config:"proxy_url"`
Expand Down