diff --git a/changelogs/head.asciidoc b/changelogs/head.asciidoc index f62c6a71729..444be711910 100644 --- a/changelogs/head.asciidoc +++ b/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 diff --git a/cmd/root.go b/cmd/root.go index eda0dcd68f4..57aa3502a6a 100644 --- a/cmd/root.go +++ b/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. diff --git a/idxmgmt/supporter.go b/idxmgmt/supporter.go index e8a8f676f31..eeffdcee412 100644 --- a/idxmgmt/supporter.go +++ b/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,21 +91,10 @@ 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 @@ -113,7 +102,7 @@ func newSupporter(log *logp.Logger, info beat.Info, cfg *IndexManagementConfig) // 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 -} diff --git a/idxmgmt/supporter_factory.go b/idxmgmt/supporter_factory.go index 9c8c801f6c4..0c826286b3e 100644 --- a/idxmgmt/supporter_factory.go +++ b/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 } diff --git a/idxmgmt/supporter_factory_test.go b/idxmgmt/supporter_factory_test.go index 132007d303f..e8f0a4841ce 100644 --- a/idxmgmt/supporter_factory_test.go +++ b/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,6 +74,7 @@ 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", @@ -80,6 +82,6 @@ func TestMakeDefaultSupporter(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "`setup.template.name` and `setup.template.pattern` have to be set ") assert.Nil(t, s) - }) + } diff --git a/systemtest/export_test.go b/systemtest/export_test.go new file mode 100644 index 00000000000..b11940d7245 --- /dev/null +++ b/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)) +} diff --git a/tests/system/test_export.py b/tests/system/test_export.py index 3a2da401ab0..ee4ef8f61ad 100644 --- a/tests/system/test_export.py +++ b/tests/system/test_export.py @@ -12,90 +12,6 @@ class ExportCommandTest(SubCommandTest): register_pipeline_disabled = True -@integration_test -class ExportConfigDefaultTest(ExportCommandTest): - """ - Test export config subcommand. - """ - - def start_args(self): - return { - "extra_args": ["export", "config"], - "logging_args": None, - } - - def test_export_config(self): - """ - Test export default config - """ - config = yaml.load(self.command_output, Loader=Loader) - # logging settings - self.assertDictEqual( - {"metrics": {"enabled": False}, 'files': {'rotateeverybytes': 10485760}, }, config["logging"] - ) - - # template settings - self.assertDictEqual( - { - "template": { - "settings": { - "_source": {"enabled": True}, - "index": { - "codec": "best_compression", - "mapping": { - "total_fields": {"limit": 2000} - }, - "number_of_shards": 1, - }, - }, - }, - }, config["setup"]) - - -@integration_test -class ExportConfigTest(ExportCommandTest): - """ - Test export config subcommand. - """ - - def start_args(self): - return { - "extra_args": ["export", "config", - "-E", "logging.metrics.enabled=true", - "-E", "setup.template.settings.index.mapping.total_fields.limit=5", - ], - "logging_args": None, - } - - def test_export_config(self): - """ - Test export customized config - """ - config = yaml.load(self.command_output, Loader=Loader) - # logging settings - assert "metrics" in config["logging"] - self.assertDictEqual( - {"enabled": True}, config["logging"]["metrics"] - ) - - # template settings - self.assertDictEqual( - { - "template": { - "settings": { - "_source": {"enabled": True}, - "index": { - "codec": "best_compression", - "mapping": { - "total_fields": {"limit": 5} - }, - "number_of_shards": 1, - }, - }, - }, - }, config["setup"]) - - @integration_test class TestExportTemplate(ExportCommandTest): """