Move APM-specific template defaults to idxmgmt (#4458)

* Move APM-specific template defaults to idxmgmt

Move APM-specific `setup.template.settings` defaults
away from using config overrides, and add the logic
to the idxmgmt package.

Similarly drop `logging.files.rotateeverbytes`, as
we were setting what is already the default defined
by libbeat.

Now when you run `apm-server export config`, these
defaults do not show up at all. The relevant system
tests have been migrated to Go, and the expectations
updated to match the change in behaviour.

We customise `logging.metrics.enabled`, and there
is no other way to change the default before it is
interpreted by libbeat, so it remains in config
overrides for now.

changelogs/head.asciidoc

@@ -11,6 +11,7 @@ https://github.com/elastic/apm-server/compare/7.10\...master[View commits]
 * JSON schema metricset: nest type and subtype under span {pull}4329[4329]
 * JSON schema metricset: nest type and name under transaction {pull}4329[4329]
 * Carriage returns are now stripped from source-mapped context source lines {pull}4348[4348]
+* Remove config defaults from `apm-server export config` {pull}4458[4458]
 
 [float]
 ==== Intake API Changes

cmd/root.go

@@ -48,31 +48,9 @@ var libbeatConfigOverrides = []cfgfile.ConditionalOverride{{
 			"metrics": map[string]interface{}{
 				"enabled": false,
 			},
-			"files": map[string]interface{}{
-				"rotateeverybytes": 10 * 1024 * 1024,
-			},
-		},
-		"setup": map[string]interface{}{
-			"template": map[string]interface{}{
-				"settings": map[string]interface{}{
-					"index": map[string]interface{}{
-						"codec": "best_compression",
-						"mapping": map[string]interface{}{
-							"total_fields": map[string]int{
-								"limit": 2000,
-							},
-						},
-						"number_of_shards": 1,
-					},
-					"_source": map[string]interface{}{
-						"enabled": true,
-					},
-				},
-			},
 		},
 	}),
-},
-}
+}}
 
 // DefaultSettings return the default settings for APM Server to pass into
 // the GenRootCmdWithSettings.

idxmgmt/supporter.go

@@ -54,7 +54,7 @@ type supporter struct {
 	dataStreams        bool
 	templateConfig     template.TemplateConfig
 	ilmConfig          ilm.Config
-	unmanagedIdxConfig *unmanaged.Config
+	unmanagedIdxConfig unmanaged.Config
 	migration          bool
 	ilmSupporters      []libilm.Supporter
 
@@ -91,29 +91,18 @@ type ilmIndexSelector struct {
 func newSupporter(log *logp.Logger, info beat.Info, cfg *IndexManagementConfig) (*supporter, error) {
 
 	var (
-		unmanagedIdxCfg unmanaged.Config
-		mode            = cfg.ILM.Mode
-		st              = indexState{}
+		mode = cfg.ILM.Mode
+		st   = indexState{}
 	)
 
-	if cfg.Output.Name() == esKey {
-		if err := cfg.Output.Config().Unpack(&unmanagedIdxCfg); err != nil {
-			return nil, fmt.Errorf("unpacking output elasticsearch index config fails: %+v", err)
-		}
-
-		if err := checkTemplateESSettings(cfg.Template, &unmanagedIdxCfg); err != nil {
-			return nil, err
-		}
-	}
-
 	var disableILM bool
 	if cfg.Output.Name() != esKey || cfg.ILM.Mode == libilm.ModeDisabled {
 		disableILM = true
 	} else if cfg.ILM.Mode == libilm.ModeAuto {
 		// ILM is set to "auto": disable if we're using data streams,
 		// or if we're not using data streams but we're using customised,
 		// unmanaged indices.
-		if cfg.DataStreams || unmanagedIdxCfg.Customized() {
+		if cfg.DataStreams || cfg.unmanagedIdxCfg.Customized() {
 			disableILM = true
 		}
 	}
@@ -133,7 +122,7 @@ func newSupporter(log *logp.Logger, info beat.Info, cfg *IndexManagementConfig)
 		dataStreams:        cfg.DataStreams,
 		templateConfig:     cfg.Template,
 		ilmConfig:          cfg.ILM,
-		unmanagedIdxConfig: &unmanagedIdxCfg,
+		unmanagedIdxConfig: cfg.unmanagedIdxCfg,
 		migration:          false,
 		st:                 st,
 		ilmSupporters:      ilmSupporters,
@@ -277,14 +266,3 @@ func getEventCustomIndex(evt *beat.Event) string {
 
 	return ""
 }
-
-func checkTemplateESSettings(tmplCfg template.TemplateConfig, indexCfg *unmanaged.Config) error {
-	if !tmplCfg.Enabled || indexCfg == nil {
-		return nil
-	}
-
-	if indexCfg.Index != "" && (tmplCfg.Name == "" || tmplCfg.Pattern == "") {
-		return errors.New("`setup.template.name` and `setup.template.pattern` have to be set if `output.elasticsearch` index name is modified")
-	}
-	return nil
-}

idxmgmt/supporter_factory.go

@@ -18,7 +18,7 @@
 package idxmgmt
 
 import (
-	"fmt"
+	"github.com/pkg/errors"
 
 	"github.com/elastic/beats/v7/libbeat/beat"
 	"github.com/elastic/beats/v7/libbeat/common"
@@ -27,16 +27,17 @@ import (
 	"github.com/elastic/beats/v7/libbeat/template"
 
 	"github.com/elastic/apm-server/idxmgmt/ilm"
+	"github.com/elastic/apm-server/idxmgmt/unmanaged"
 	logs "github.com/elastic/apm-server/log"
 )
 
-// functionality largely copied from libbeat
-
 type IndexManagementConfig struct {
 	DataStreams bool
 	Template    template.TemplateConfig
 	ILM         ilm.Config
 	Output      common.ConfigNamespace
+
+	unmanagedIdxCfg unmanaged.Config
 }
 
 // MakeDefaultSupporter creates a new idxmgmt.Supporter, using the given root config.
@@ -56,50 +57,89 @@ func MakeDefaultSupporter(log *logp.Logger, info beat.Info, configRoot *common.C
 	if err != nil {
 		return nil, err
 	}
+	log = namedLogger(log)
+	return newSupporter(log, info, cfg)
+}
+
+func namedLogger(log *logp.Logger) *logp.Logger {
 	if log == nil {
-		log = logp.NewLogger(logs.IndexManagement)
-	} else {
-		log = log.Named(logs.IndexManagement)
+		return logp.NewLogger(logs.IndexManagement)
 	}
-	return newSupporter(log, info, cfg)
+	return log.Named(logs.IndexManagement)
 }
 
+// NewIndexManagementConfig extracts and validates index management config from info and configRoot.
 func NewIndexManagementConfig(info beat.Info, configRoot *common.Config) (*IndexManagementConfig, error) {
-	cfg := struct {
-		DataStreams *common.Config         `config:"apm-server.data_streams"`
-		ILM         *common.Config         `config:"apm-server.ilm"`
-		Template    *common.Config         `config:"setup.template"`
-		Output      common.ConfigNamespace `config:"output"`
-	}{}
+	var cfg struct {
+		DataStreams            *common.Config         `config:"apm-server.data_streams"`
+		RegisterIngestPipeline *common.Config         `config:"apm-server.register.ingest.pipeline"`
+		ILM                    *common.Config         `config:"apm-server.ilm"`
+		Template               *common.Config         `config:"setup.template"`
+		Output                 common.ConfigNamespace `config:"output"`
+	}
 	if configRoot != nil {
 		if err := configRoot.Unpack(&cfg); err != nil {
 			return nil, err
 		}
 	}
 
-	tmplConfig, err := unpackTemplateConfig(cfg.Template)
+	templateConfig, err := unpackTemplateConfig(cfg.Template)
 	if err != nil {
-		return nil, fmt.Errorf("unpacking template config fails: %+v", err)
+		return nil, errors.Wrap(err, "unpacking template config failed")
 	}
 
 	ilmConfig, err := ilm.NewConfig(info, cfg.ILM)
 	if err != nil {
-		return nil, fmt.Errorf("creating ILM config fails: %v", err)
+		return nil, errors.Wrap(err, "creating ILM config fails")
+	}
+
+	var unmanagedIdxCfg unmanaged.Config
+	if cfg.Output.Name() == esKey {
+		if err := cfg.Output.Config().Unpack(&unmanagedIdxCfg); err != nil {
+			return nil, errors.Wrap(err, "failed to unpack output.elasticsearch config")
+		}
+		if err := checkTemplateESSettings(templateConfig, &unmanagedIdxCfg); err != nil {
+			return nil, err
+		}
 	}
 
 	return &IndexManagementConfig{
-		DataStreams: cfg.DataStreams.Enabled(),
-		Template:    tmplConfig,
-		ILM:         ilmConfig,
-		Output:      cfg.Output,
+		Output:   cfg.Output,
+		Template: templateConfig,
+		ILM:      ilmConfig,
+
+		unmanagedIdxCfg: unmanagedIdxCfg,
 	}, nil
 }
 
-func unpackTemplateConfig(cfg *common.Config) (template.TemplateConfig, error) {
-	config := template.DefaultConfig()
-	if cfg == nil {
-		return config, nil
+func checkTemplateESSettings(tmplCfg template.TemplateConfig, indexCfg *unmanaged.Config) error {
+	if !tmplCfg.Enabled || indexCfg == nil {
+		return nil
+	}
+	if indexCfg.Index != "" && (tmplCfg.Name == "" || tmplCfg.Pattern == "") {
+		return errors.New("`setup.template.name` and `setup.template.pattern` have to be set if `output.elasticsearch` index name is modified")
+	}
+	return nil
+}
+
+// unpackTemplateConfig merges APM-specific template settings with (possibly nil)
+// user-defined config, unpacks it over template.DefaultConfig(), returning the result.
+func unpackTemplateConfig(userTemplateConfig *common.Config) (template.TemplateConfig, error) {
+	templateConfig := common.MustNewConfigFrom(`
+settings:
+  index:
+    codec: best_compression
+    mapping.total_fields.limit: 2000
+    number_of_shards: 1
+  _source.enabled: true`)
+	if userTemplateConfig != nil {
+		if err := templateConfig.Merge(userTemplateConfig); err != nil {
+			return template.TemplateConfig{}, errors.Wrap(err, "merging failed")
+		}
+	}
+	out := template.DefaultConfig()
+	if err := templateConfig.Unpack(&out); err != nil {
+		return template.TemplateConfig{}, err
 	}
-	err := cfg.Unpack(&config)
-	return config, err
+	return out, nil
 }

idxmgmt/supporter_factory_test.go

@@ -55,9 +55,10 @@ func TestMakeDefaultSupporter(t *testing.T) {
 		assert.True(t, s.Enabled())
 		assert.NotNil(t, s.log)
 		assert.True(t, s.templateConfig.Enabled)
+		assert.Equal(t, "best_compression", s.templateConfig.Settings.Index["codec"])
 		assert.Equal(t, libilm.ModeAuto, s.ilmConfig.Mode)
 		assert.True(t, s.ilmConfig.Setup.Enabled)
-		assert.Equal(t, &unmanaged.Config{}, s.unmanagedIdxConfig)
+		assert.Equal(t, unmanaged.Config{}, s.unmanagedIdxConfig)
 	})
 
 	t.Run("ILMDisabled", func(t *testing.T) {
@@ -73,13 +74,14 @@ func TestMakeDefaultSupporter(t *testing.T) {
 		assert.Equal(t, libilm.ModeDisabled, s.ilmConfig.Mode)
 		assert.True(t, s.ilmConfig.Setup.Enabled)
 	})
+
 	t.Run("SetupTemplateConfigConflicting", func(t *testing.T) {
 		s, err := buildSupporter(map[string]interface{}{
 			"output.elasticsearch.index": "custom-index",
 		})
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "`setup.template.name` and `setup.template.pattern` have to be set ")
 		assert.Nil(t, s)
-
 	})
+
 }

systemtest/export_test.go

@@ -0,0 +1,81 @@
+// Licensed to Elasticsearch B.V. under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Elasticsearch B.V. licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package systemtest_test
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/elastic/apm-server/systemtest/apmservertest"
+)
+
+func exportConfigCommand(t *testing.T, args ...string) (_ *apmservertest.ServerCmd, homedir string) {
+	tempdir, err := ioutil.TempDir("", "systemtest")
+	require.NoError(t, err)
+	t.Cleanup(func() { os.RemoveAll(tempdir) })
+	err = ioutil.WriteFile(filepath.Join(tempdir, "apm-server.yml"), nil, 0644)
+	require.NoError(t, err)
+
+	allArgs := []string{"config", "--path.home", tempdir}
+	allArgs = append(allArgs, args...)
+	return apmservertest.ServerCommand("export", allArgs...), tempdir
+}
+
+func TestExportConfigDefaults(t *testing.T) {
+	cmd, tempdir := exportConfigCommand(t)
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err)
+
+	expectedConfig := strings.ReplaceAll(`
+logging:
+  metrics:
+    enabled: false
+path:
+  config: /home/apm-server
+  data: /home/apm-server/data
+  home: /home/apm-server
+  logs: /home/apm-server/logs
+`[1:], "/home/apm-server", tempdir)
+	assert.Equal(t, expectedConfig, string(out))
+}
+
+func TestExportConfigOverrideDefaults(t *testing.T) {
+	cmd, tempdir := exportConfigCommand(t,
+		"-E", "logging.metrics.enabled=true",
+	)
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err)
+
+	expectedConfig := strings.ReplaceAll(`
+logging:
+  metrics:
+    enabled: true
+path:
+  config: /home/apm-server
+  data: /home/apm-server/data
+  home: /home/apm-server
+  logs: /home/apm-server/logs
+`[1:], "/home/apm-server", tempdir)
+	assert.Equal(t, expectedConfig, string(out))
+}