diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 0588d6c843f..3068c165e29 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -4,3 +4,4 @@ /deploy/kubernetes @elastic/obs-cloudnative-monitoring /dev-tools/kubernetes @elastic/obs-cloudnative-monitoring /internal/pkg/composable/providers/kubernetes @elastic/obs-cloudnative-monitoring +/internal/pkg/agent/application/configuration_embed_changed_test.go @elastic/cloudbeat diff --git a/_meta/config/providers.yml.tmpl b/_meta/config/providers.yml.tmpl index 02d81408cb0..c9376e5113f 100644 --- a/_meta/config/providers.yml.tmpl +++ b/_meta/config/providers.yml.tmpl @@ -4,6 +4,11 @@ # and conditionals. Each provider's keys are automatically prefixed with the name # of the provider. +# All registered providers are enabled by default. + +# Disable all providers by default and only enable explicitly configured providers. +# agent.providers.initial_default: false + #providers: # Agent provides information about the running agent. diff --git a/changelog/fragments/1707070032-providers-default-disable.yaml b/changelog/fragments/1707070032-providers-default-disable.yaml new file mode 100644 index 00000000000..ced06bc342f --- /dev/null +++ b/changelog/fragments/1707070032-providers-default-disable.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: enhancement + +# Change summary; a 80ish characters long description of the change. +summary: Add agent.providers.initial_default configuration flag to disable providers by default + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +pr: https://github.com/elastic/elastic-agent/pull/4166 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +issue: https://github.com/elastic/elastic-agent/issues/4145 diff --git a/elastic-agent.docker.yml b/elastic-agent.docker.yml index b9ad5e3f156..87aa2cdd0cd 100644 --- a/elastic-agent.docker.yml +++ b/elastic-agent.docker.yml @@ -218,6 +218,11 @@ agent.logging.to_stderr: true # and conditionals. Each provider's keys are automatically prefixed with the name # of the provider. +# All registered providers are enabled by default. + +# Disable all providers by default and only enable explicitly configured providers. +# agent.providers.initial_default: false + #providers: # Agent provides information about the running agent. diff --git a/elastic-agent.reference.yml b/elastic-agent.reference.yml index db84c2e062e..7ee19406674 100644 --- a/elastic-agent.reference.yml +++ b/elastic-agent.reference.yml @@ -271,6 +271,11 @@ agent.logging.to_stderr: true # and conditionals. Each provider's keys are automatically prefixed with the name # of the provider. +# All registered providers are enabled by default. + +# Disable all providers by default and only enable explicitly configured providers. +# agent.providers.initial_default: false + #providers: # Agent provides information about the running agent. diff --git a/elastic-agent.yml b/elastic-agent.yml index 0434c57ccb8..342fd50f432 100644 --- a/elastic-agent.yml +++ b/elastic-agent.yml @@ -261,6 +261,11 @@ agent.logging.to_stderr: true # and conditionals. Each provider's keys are automatically prefixed with the name # of the provider. +# All registered providers are enabled by default. + +# Disable all providers by default and only enable explicitly configured providers. +# agent.providers.initial_default: false + #providers: # Agent provides information about the running agent. diff --git a/internal/pkg/agent/application/configuration_embed_changed_test.go b/internal/pkg/agent/application/configuration_embed_changed_test.go new file mode 100644 index 00000000000..2197e007575 --- /dev/null +++ b/internal/pkg/agent/application/configuration_embed_changed_test.go @@ -0,0 +1,22 @@ +// 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 application + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v2" +) + +// This test exists to notify the cloudbeat team in case the default agent fleet config is changed. +func TestDefaultAgentFleetConfig(t *testing.T) { + cfg := map[string]interface{}{} + + err := yaml.Unmarshal(DefaultAgentFleetConfig, &cfg) + assert.NoError(t, err) + + assert.Equal(t, map[string]interface{}{"fleet": map[interface{}]interface{}{"enabled": true}}, cfg) +} diff --git a/internal/pkg/agent/storage/replace_store.go b/internal/pkg/agent/storage/replace_store.go index fa495f9dc1e..aa17535236d 100644 --- a/internal/pkg/agent/storage/replace_store.go +++ b/internal/pkg/agent/storage/replace_store.go @@ -59,6 +59,10 @@ func (r *ReplaceOnSuccessStore) Save(in io.Reader) error { return nil } + if shouldSkipReplace(target, r.replaceWith) { + return nil + } + // Windows is tricky with the characters permitted for the path and filename, so we have // to remove any colon from the string. We are using nanosec precision here because of automated // tools. diff --git a/internal/pkg/agent/storage/skip_replace.go b/internal/pkg/agent/storage/skip_replace.go new file mode 100644 index 00000000000..fc40c07a4a5 --- /dev/null +++ b/internal/pkg/agent/storage/skip_replace.go @@ -0,0 +1,38 @@ +// 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 storage + +import ( + "strings" + + "github.com/google/go-cmp/cmp" + "gopkg.in/yaml.v3" +) + +// This is a temporary solution to avoid replacing the target file if the content of the replacement is contained in the target file. +// It only works for YAML files, since the only use case is for the default agent fleet config. +// Returns true only if the replacement configuration is already contained in the original. +func shouldSkipReplace(original []byte, replacement []byte) bool { + replacementYaml := map[string]interface{}{} + originalYaml := map[string]interface{}{} + + err := yaml.Unmarshal(replacement, &replacementYaml) + if err != nil { + return false + } + + err = yaml.Unmarshal(original, &originalYaml) + if err != nil { + return false + } + + diff := cmp.Diff(replacementYaml, originalYaml) + if strings.HasPrefix(diff, "-") || strings.Contains(diff, "\n-") { + // Lines starting with - represent values in the replacement that are not present in the original + return false + } + + return true +} diff --git a/internal/pkg/agent/storage/skip_replace_test.go b/internal/pkg/agent/storage/skip_replace_test.go new file mode 100644 index 00000000000..3f8941bcca2 --- /dev/null +++ b/internal/pkg/agent/storage/skip_replace_test.go @@ -0,0 +1,67 @@ +// 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 storage + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestShouldSkipReplace(t *testing.T) { + tests := []struct { + name string + original []byte + replacement []byte + expected bool + }{ + { + name: "original and replacement are the same", + original: []byte("fleet:\n enabled: true\n"), + replacement: []byte("fleet:\n enabled: true\n"), + expected: true, + }, + { + name: "original and replacement are different", + original: []byte("fleet:\n enabled: true\n"), + replacement: []byte("fleet:\n enabled: false\n"), + expected: false, + }, + { + name: "original is not a valid yaml", + original: []byte("fleet: enabled: true\n"), + expected: false, + }, + { + name: "replacement is not a valid yaml", + replacement: []byte("fleet: enabled: true\n"), + expected: false, + }, + { + name: "original and replacement are different just in comments and spaces", + original: []byte("#bla bla bla\nfleet:\n enabled: true\n"), + replacement: []byte("fleet: \n enabled: true\n#oh right bla bla bla\n"), + expected: true, + }, + { + name: "original contains replacement and more", + original: []byte("#bla bla bla\nfleet:\n enabled: true\nanother: value\nmore:\n stuff: true\n"), + replacement: []byte("fleet:\n enabled: true\n"), + expected: true, + }, + { + name: "original contains replacement and more, but in different order", + original: []byte("fleet:\n a_key_that_ruins: sad\n enabled: true\n"), + replacement: []byte("fleet:\n enabled: true\n"), + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, shouldSkipReplace(tt.original, tt.replacement)) + }) + } +} diff --git a/internal/pkg/agent/storage/storage_test.go b/internal/pkg/agent/storage/storage_test.go index 98f11a772e0..2d9001de3b1 100644 --- a/internal/pkg/agent/storage/storage_test.go +++ b/internal/pkg/agent/storage/storage_test.go @@ -6,6 +6,7 @@ package storage import ( "bytes" + "errors" "fmt" "io" "os" @@ -14,7 +15,6 @@ import ( "testing" "time" - "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -101,6 +101,33 @@ func TestReplaceOrRollbackStore(t *testing.T) { }) + t.Run("when replace is skipped due to target already containing source content", func(t *testing.T) { + yamlTarget := []byte("fleet:\n enabled: true\nother: value\n") + yamlReplaceWith := []byte("#This comment is left out\nfleet:\n enabled: true\n") + target, err := genFile(yamlTarget) + + require.NoError(t, err) + dir := filepath.Dir(target) + defer os.RemoveAll(dir) + + requireFilesCount(t, dir, 1) + + s := NewReplaceOnSuccessStore( + target, + yamlReplaceWith, + failure, + ) + + err = s.Save(in) + require.Error(t, err) + + writtenContent, err := os.ReadFile(target) + require.NoError(t, err) + + require.True(t, bytes.Equal(writtenContent, yamlTarget)) + requireFilesCount(t, dir, 1) + }) + t.Run("when target file do not exist", func(t *testing.T) { s := NewReplaceOnSuccessStore( fmt.Sprintf("%s/%d", os.TempDir(), time.Now().Unix()), @@ -176,7 +203,10 @@ func genFile(b []byte) (string, error) { if err != nil { return "", err } - f.Write(b) + _, err = f.Write(b) + if err != nil { + return "", err + } name := f.Name() if err := f.Close(); err != nil { return "", err diff --git a/internal/pkg/composable/config.go b/internal/pkg/composable/config.go index deae19bc238..b35771e7e12 100644 --- a/internal/pkg/composable/config.go +++ b/internal/pkg/composable/config.go @@ -8,5 +8,6 @@ import "github.com/elastic/elastic-agent/internal/pkg/config" // Config is config for multiple providers. type Config struct { - Providers map[string]*config.Config `config:"providers"` + Providers map[string]*config.Config `config:"providers"` + ProvidersInitialDefault *bool `config:"agent.providers.initial_default"` } diff --git a/internal/pkg/composable/controller.go b/internal/pkg/composable/controller.go index 6ae3220e59c..02ad5186be0 100644 --- a/internal/pkg/composable/controller.go +++ b/internal/pkg/composable/controller.go @@ -61,11 +61,17 @@ func New(log *logger.Logger, c *config.Config, managed bool) (Controller, error) } } + // Unless explicitly configured otherwise, All registered providers are enabled by default + providersInitialDefault := true + if providersCfg.ProvidersInitialDefault != nil { + providersInitialDefault = *providersCfg.ProvidersInitialDefault + } + // build all the context providers contextProviders := map[string]*contextProviderState{} for name, builder := range Providers.contextProviders { pCfg, ok := providersCfg.Providers[name] - if ok && !pCfg.Enabled() { + if (ok && !pCfg.Enabled()) || (!ok && !providersInitialDefault) { // explicitly disabled; skipping continue } @@ -84,7 +90,7 @@ func New(log *logger.Logger, c *config.Config, managed bool) (Controller, error) dynamicProviders := map[string]*dynamicProviderState{} for name, builder := range Providers.dynamicProviders { pCfg, ok := providersCfg.Providers[name] - if ok && !pCfg.Enabled() { + if (ok && !pCfg.Enabled()) || (!ok && !providersInitialDefault) { // explicitly disabled; skipping continue } diff --git a/internal/pkg/composable/controller_test.go b/internal/pkg/composable/controller_test.go index ec418795c19..5c1d52556f4 100644 --- a/internal/pkg/composable/controller_test.go +++ b/internal/pkg/composable/controller_test.go @@ -133,6 +133,127 @@ func TestController(t *testing.T) { assert.Equal(t, "value2", localMap["key1"]) } +func TestProvidersDefaultDisabled(t *testing.T) { + tests := []struct { + name string + cfg map[string]interface{} + want int + }{ + { + name: "default disabled", + cfg: map[string]interface{}{ + "agent.providers.initial_default": "false", + }, + want: 0, + }, + { + name: "default enabled", + cfg: map[string]interface{}{ + "agent.providers.initial_default": "true", + }, + want: 1, + }, + { + name: "default enabled - no config", + cfg: map[string]interface{}{}, + want: 1, + }, + { + name: "default enabled - explicit config", + cfg: map[string]interface{}{ + "providers": map[string]interface{}{ + "env": map[string]interface{}{ + "enabled": "false", + }, + "local": map[string]interface{}{ + "vars": map[string]interface{}{ + "key1": "value1", + }, + }, + "local_dynamic": map[string]interface{}{ + "items": []map[string]interface{}{ + { + "vars": map[string]interface{}{ + "key1": "value1", + }, + "processors": []map[string]interface{}{ + { + "add_fields": map[string]interface{}{ + "fields": map[string]interface{}{ + "add": "value1", + }, + "to": "dynamic", + }, + }, + }, + }, + { + "vars": map[string]interface{}{ + "key1": "value2", + }, + "processors": []map[string]interface{}{ + { + "add_fields": map[string]interface{}{ + "fields": map[string]interface{}{ + "add": "value2", + }, + "to": "dynamic", + }, + }, + }, + }, + }, + }, + }, + }, + want: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg, err := config.NewConfigFrom(tt.cfg) + require.NoError(t, err) + + log, err := logger.New("", false) + require.NoError(t, err) + c, err := composable.New(log, cfg, false) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + timeoutCtx, timeoutCancel := context.WithTimeout(ctx, 1*time.Second) + defer timeoutCancel() + + var setVars []*transpiler.Vars + go func() { + defer cancel() + for { + select { + case <-timeoutCtx.Done(): + return + case vars := <-c.Watch(): + setVars = vars + } + } + }() + + errCh := make(chan error) + go func() { + errCh <- c.Run(ctx) + }() + err = <-errCh + if errors.Is(err, context.Canceled) { + err = nil + } + require.NoError(t, err) + + assert.Len(t, setVars, tt.want) + }) + } +} + func TestCancellation(t *testing.T) { cfg, err := config.NewConfigFrom(map[string]interface{}{ "providers": map[string]interface{}{