From 4af8f21b53d22f10235b4363ec84aaa15afadaab Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 16 Dec 2024 11:13:13 +0100 Subject: [PATCH] Cache conditions when applying variables to config (#6229) (#6281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Cache conditions when applying variables to config # Conflicts: # internal/pkg/agent/transpiler/ast_test.go * Add test for AST insertion (cherry picked from commit cab5754e4b8e6640d92cf9213142e4a5570ec968) # Conflicts: # internal/pkg/agent/transpiler/ast_test.go # Conflicts: # internal/pkg/agent/transpiler/ast_test.go Co-authored-by: Mikołaj Świątek --- ...1733411331-cache-conditions-in-config.yaml | 32 ++++++++++++++++ internal/pkg/agent/transpiler/ast.go | 20 +++++++--- internal/pkg/agent/transpiler/ast_test.go | 37 +++++++++++++++++++ 3 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 changelog/fragments/1733411331-cache-conditions-in-config.yaml diff --git a/changelog/fragments/1733411331-cache-conditions-in-config.yaml b/changelog/fragments/1733411331-cache-conditions-in-config.yaml new file mode 100644 index 00000000000..57c424a38d2 --- /dev/null +++ b/changelog/fragments/1733411331-cache-conditions-in-config.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: Cache conditions when applying variables to config + +# 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/owner/repo/1234 + +# 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/owner/repo/1234 diff --git a/internal/pkg/agent/transpiler/ast.go b/internal/pkg/agent/transpiler/ast.go index b6490586ca1..341c63ebc48 100644 --- a/internal/pkg/agent/transpiler/ast.go +++ b/internal/pkg/agent/transpiler/ast.go @@ -211,13 +211,14 @@ func (d *Dict) sort() { // Key represents a Key / value pair in the dictionary. type Key struct { - name string - value Node + name string + value Node + condition *eql.Expression } // NewKey creates a new key with provided name node pair. func NewKey(name string, val Node) *Key { - return &Key{name, val} + return &Key{name: name, value: val} } func (k *Key) String() string { @@ -288,11 +289,18 @@ func (k *Key) Apply(vars *Vars) (Node, error) { case *BoolVal: return k, nil case *StrVal: - cond, err := eql.Eval(v.value, vars, true) + var err error + if k.condition == nil { + k.condition, err = eql.New(v.value) + if err != nil { + return nil, fmt.Errorf(`invalid condition "%s": %w`, v.value, err) + } + } + cond, err := k.condition.Eval(vars, true) if err != nil { return nil, fmt.Errorf(`condition "%s" evaluation failed: %w`, v.value, err) } - return &Key{k.name, NewBoolVal(cond)}, nil + return &Key{name: k.name, value: NewBoolVal(cond)}, nil } return nil, fmt.Errorf("condition key's value must be a string; received %T", k.value) } @@ -303,7 +311,7 @@ func (k *Key) Apply(vars *Vars) (Node, error) { if v == nil { return nil, nil } - return &Key{k.name, v}, nil + return &Key{name: k.name, value: v}, nil } // Processors returns any attached processors, because of variable substitution. diff --git a/internal/pkg/agent/transpiler/ast_test.go b/internal/pkg/agent/transpiler/ast_test.go index dd4f2f9d448..844fc82a046 100644 --- a/internal/pkg/agent/transpiler/ast_test.go +++ b/internal/pkg/agent/transpiler/ast_test.go @@ -11,6 +11,8 @@ import ( "github.com/elastic/elastic-agent-libs/mapstr" + "github.com/elastic/elastic-agent/internal/pkg/eql" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -957,6 +959,41 @@ func TestShallowClone(t *testing.T) { } } +func TestCondition(t *testing.T) { + vars := mustMakeVars(map[string]interface{}{ + "other": map[string]interface{}{ + "data": "info", + }}) + + input := NewKey("condition", NewStrVal("${other.data} == 'info'")) + expected := NewKey("condition", NewBoolVal(true)) + + // the condition string hasn't been parsed yet + assert.Nil(t, input.condition) + + output, err := input.Apply(vars) + require.NoError(t, err) + assert.Equal(t, expected, output) + + // check if the condition was parsed and cached + assert.NotNil(t, input.condition) + condition, err := eql.New(input.value.Value().(string)) + require.NoError(t, err) + assert.Equal(t, condition, input.condition) + + // create a dict with the key + dict := NewDict([]Node{input}) + ast := &AST{root: NewKey("key", dict)} + // the cached condition remains + assert.Equal(t, condition, input.condition) + + // replace the key with a new one, without a cached condition + input2 := NewKey("condition", NewStrVal("${other.data} == 'info'")) + err = Insert(ast, input2, "") + require.NoError(t, err) + assert.Nil(t, input2.condition) +} + func mustMakeVars(mapping map[string]interface{}) *Vars { v, err := NewVars("", mapping, nil) if err != nil {