Merge default values with custom API Key Elasticsearch settings (#3342) (#3354)

When configuring an Elasticsearch instance for API Key usage,
Elasticsearch default values should be merged with the custom config,
to avoid having to configure all settings.

fixes #3341

Author: simitt

_meta/beat.yml

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

apm-server.docker.yml

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

apm-server.yml

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

beater/config/api_key.go

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

beater/config/api_key_test.go

@@ -21,6 +21,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/elastic/beats/libbeat/logp"
 
 	"github.com/stretchr/testify/assert"
@@ -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,
@@ -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)
 
 		})
 	}

beater/config/config_test.go

@@ -187,7 +187,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,
 				},
 			},
 		},

elasticsearch/config.go

@@ -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"`