Skip to content

Commit

Permalink
Cache conditions when applying variables to config (#6229)
Browse files Browse the repository at this point in the history
* Cache conditions when applying variables to config

# Conflicts:
#	internal/pkg/agent/transpiler/ast_test.go

* Add test for AST insertion

(cherry picked from commit cab5754)

# Conflicts:
#	internal/pkg/agent/transpiler/ast_test.go
  • Loading branch information
swiatekm authored and mergify[bot] committed Dec 11, 2024
1 parent e04131b commit 5ecfc6c
Show file tree
Hide file tree
Showing 3 changed files with 237 additions and 6 deletions.
32 changes: 32 additions & 0 deletions changelog/fragments/1733411331-cache-conditions-in-config.yaml
Original file line number Diff line number Diff line change
@@ -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
20 changes: 14 additions & 6 deletions internal/pkg/agent/transpiler/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,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 {
Expand Down Expand Up @@ -313,11 +314,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)
}
Expand All @@ -328,7 +336,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.
Expand Down
191 changes: 191 additions & 0 deletions internal/pkg/agent/transpiler/ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -957,6 +959,195 @@ func TestShallowClone(t *testing.T) {
}
}

<<<<<<< HEAD

Check failure on line 962 in internal/pkg/agent/transpiler/ast_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

syntax error: non-declaration statement outside function body

Check failure on line 962 in internal/pkg/agent/transpiler/ast_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

expected declaration, found '<<' (typecheck)
=======
func TestVars(t *testing.T) {
tests := map[string]struct {
input map[string]interface{}
result []string
}{
"empty": {
input: map[string]interface{}{},
result: nil,
},
"badbracket": {
input: map[string]interface{}{
"badbracket": "${missing.end",
},
result: nil,
},
"allconstant": {
input: map[string]interface{}{
"constant": "${'constant'}",
},
result: nil,
},
"escaped": {
input: map[string]interface{}{
"constant": "$${var1}",
},
result: nil,
},
"nested": {
input: map[string]interface{}{
"novars": map[string]interface{}{
"list1": []interface{}{
map[string]interface{}{
"int": 1,
"float": 1.1234,
"bool": true,
"str": "value1",
},
},
"list2": []interface{}{
map[string]interface{}{
"int": 2,
"float": 2.3456,
"bool": false,
"str": "value2",
},
},
},
"vars1": map[string]interface{}{
"list1": []interface{}{
map[string]interface{}{
"int": 1,
"float": 1.1234,
"bool": true,
"str": "${var1|var2|'constant'}",
},
},
"list2": []interface{}{
map[string]interface{}{
"int": 2,
"float": 2.3456,
"bool": false,
"str": "${var3|var1|'constant'}",
},
},
},
"vars2": map[string]interface{}{
"list1": []interface{}{
map[string]interface{}{
"int": 1,
"float": 1.1234,
"bool": true,
"str": "${var5|var6|'constant'}",
},
},
"list2": []interface{}{
map[string]interface{}{
"int": 2,
"float": 2.3456,
"bool": false,
"str": "${var1}",
},
},
},
},
result: []string{"var1", "var2", "var3", "var1", "var5", "var6", "var1"},
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
ast, err := NewAST(test.input)
require.NoError(t, err)
var vars []string
vars = ast.root.Vars(vars)

Check failure on line 1057 in internal/pkg/agent/transpiler/ast_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

expected declaration, found vars (typecheck)
assert.Equal(t, test.result, vars)
})
}
}

func TestLookup(t *testing.T) {
tests := map[string]struct {
ast *AST
selector Selector
node Node
ok bool
}{
"nil": {
ast: nil,
selector: "",
node: nil,
ok: false,
},
"noroot": {
ast: &AST{},
selector: "",
node: nil,
ok: false,
},
"notfound": {
ast: &AST{
root: NewDict([]Node{NewKey("entry", NewDict([]Node{
NewKey("var1", NewStrVal("value1")),
NewKey("var2", NewStrVal("value2")),
}))}),
},
selector: "entry.var3",
node: nil,
ok: false,
},
"found": {
ast: &AST{
root: NewDict([]Node{NewKey("entry", NewDict([]Node{
NewKey("var1", NewStrVal("value1")),
NewKey("var2", NewStrVal("value2")),
}))}),
},
selector: "entry.var2",
node: NewKey("var2", NewStrVal("value2")),
ok: true,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
node, ok := Lookup(test.ast, test.selector)
if assert.Equal(t, test.ok, ok) {
assert.Equal(t, test.node, node)
}
})
}
}

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)
}

>>>>>>> cab5754e4b (Cache conditions when applying variables to config (#6229))

Check failure on line 1150 in internal/pkg/agent/transpiler/ast_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

syntax error: non-declaration statement outside function body

Check failure on line 1150 in internal/pkg/agent/transpiler/ast_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

invalid character U+0023 '#' (typecheck)

Check failure on line 1150 in internal/pkg/agent/transpiler/ast_test.go

View workflow job for this annotation

GitHub Actions / lint (macos-latest)

illegal character U+0023 '#' (typecheck)
func mustMakeVars(mapping map[string]interface{}) *Vars {
v, err := NewVars("", mapping, nil)
if err != nil {
Expand Down

0 comments on commit 5ecfc6c

Please sign in to comment.