Allow - in eql variable names (#710)

* fix to allow dashes in variable names in EQL expressions extend eql to allow the '-' char to appear in variable names, i.e., ${data.some-var} and additional test cases to eql, the transpiler, and the k8s provider to verify this works. Note that the bug was caused by the EQL limitation, the otehr test cases were added when attempting to find it. * Regenerate grammer with antlr 4.7.1, add CHANGELOG * Fix linter issue * Fix typo (cherry picked from commit 28721eb)
elastic · Jul 13, 2022 · 8aa3aea · 8aa3aea
1 parent a09594c
commit 8aa3aea
Show file tree

Hide file tree

Showing 9 changed files with 145 additions and 109 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -102,6 +102,9 @@
 - Update API calls that the agent makes to Kibana when running the container command. {pull}253[253]
 - diagnostics collect log names are fixed on Windows machines, command will ignore failures. AgentID is included in diagnostics(and diagnostics collect) output. {issue}81[81] {issue}92[92] {issue}190[190] {pull}262[262]
 - Collects stdout and stderr of applications run as a process and logs them. {issue}[88]
+- Remove VerificationMode option to empty string. Default value is `full`. {issue}[184]
+- diagnostics collect file mod times are set. {pull}570[570]
+- Allow the - char to appear as part of variable names in eql expressions. {issue}709[709] {pull}710[710]
 
 ==== New features
 

diff --git a/internal/pkg/agent/transpiler/vars_test.go b/internal/pkg/agent/transpiler/vars_test.go
@@ -17,8 +17,9 @@ import (
 func TestVars_Replace(t *testing.T) {
 	vars := mustMakeVars(map[string]interface{}{
 		"un-der_score": map[string]interface{}{
-			"key1": "data1",
-			"key2": "data2",
+			"key1":      "data1",
+			"key2":      "data2",
+			"with-dash": "dash-value",
 			"list": []string{
 				"array1",
 				"array2",
@@ -44,6 +45,12 @@ func TestVars_Replace(t *testing.T) {
 			false,
 			false,
 		},
+		{
+			"${un-der_score.with-dash}",
+			NewStrVal("dash-value"),
+			false,
+			false,
+		},
 		{
 			"${un-der_score.missing}",
 			NewStrVal(""),

diff --git a/internal/pkg/composable/providers/kubernetes/node_test.go b/internal/pkg/composable/providers/kubernetes/node_test.go
@@ -26,7 +26,8 @@ func TestGenerateNodeData(t *testing.T) {
 			Name: "testnode",
 			UID:  types.UID(uid),
 			Labels: map[string]string{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			Annotations: map[string]string{
 				"baz": "ban",
@@ -54,7 +55,8 @@ func TestGenerateNodeData(t *testing.T) {
 			"baz": "ban",
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 	}
 
@@ -64,7 +66,10 @@ func TestGenerateNodeData(t *testing.T) {
 				"name": "devcluster",
 				"url":  "8.8.8.8:9090"},
 		}, "kubernetes": mapstr.M{
-			"labels":      mapstr.M{"foo": "bar"},
+			"labels": mapstr.M{
+				"foo":       "bar",
+				"with-dash": "dash-value",
+			},
 			"annotations": mapstr.M{"baz": "ban"},
 			"node": mapstr.M{
 				"ip":   "node1",
@@ -123,7 +128,8 @@ func (n *nodeMeta) GenerateK8s(obj kubernetes.Resource, opts ...metadata.FieldOp
 			"ip":   "node1",
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 		"annotations": mapstr.M{
 			"baz": "ban",

diff --git a/internal/pkg/composable/providers/kubernetes/pod_test.go b/internal/pkg/composable/providers/kubernetes/pod_test.go
@@ -27,7 +27,8 @@ func TestGeneratePodData(t *testing.T) {
 			UID:       types.UID(uid),
 			Namespace: "testns",
 			Labels: map[string]string{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			Annotations: map[string]string{
 				"app": "production",
@@ -59,7 +60,8 @@ func TestGeneratePodData(t *testing.T) {
 			"nsa": "nsb",
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 		"annotations": mapstr.M{
 			"app": "production",
@@ -74,7 +76,8 @@ func TestGeneratePodData(t *testing.T) {
 		}, "kubernetes": mapstr.M{
 			"namespace": "testns",
 			"labels": mapstr.M{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			"annotations": mapstr.M{"app": "production"},
 			"pod": mapstr.M{
@@ -119,7 +122,8 @@ func TestGenerateContainerPodData(t *testing.T) {
 			UID:       types.UID(uid),
 			Namespace: "testns",
 			Labels: map[string]string{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			Annotations: map[string]string{
 				"app": "production",
@@ -175,7 +179,8 @@ func TestGenerateContainerPodData(t *testing.T) {
 			"app": "production",
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 	}
 
@@ -191,7 +196,9 @@ func TestGenerateContainerPodData(t *testing.T) {
 		}, "kubernetes": mapstr.M{
 			"namespace":   "testns",
 			"annotations": mapstr.M{"app": "production"},
-			"labels":      mapstr.M{"foo": "bar"},
+			"labels": mapstr.M{"foo": "bar",
+				"with-dash": "dash-value",
+			},
 			"pod": mapstr.M{
 				"ip":   "127.0.0.5",
 				"name": "testpod",
@@ -232,7 +239,8 @@ func TestEphemeralContainers(t *testing.T) {
 			UID:       types.UID(uid),
 			Namespace: "testns",
 			Labels: map[string]string{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			Annotations: map[string]string{
 				"app": "production",
@@ -274,7 +282,8 @@ func TestEphemeralContainers(t *testing.T) {
 			"ip":   pod.Status.PodIP,
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 		"container": mapstr.M{
 			"id":      "asdfghdeadbeef",
@@ -300,8 +309,10 @@ func TestEphemeralContainers(t *testing.T) {
 				"name": "devcluster",
 				"url":  "8.8.8.8:9090"},
 		}, "kubernetes": mapstr.M{
-			"namespace":   "testns",
-			"labels":      mapstr.M{"foo": "bar"},
+			"namespace": "testns",
+			"labels": mapstr.M{"foo": "bar",
+				"with-dash": "dash-value",
+			},
 			"annotations": mapstr.M{"app": "production"},
 			"pod": mapstr.M{
 				"ip":   "127.0.0.5",
@@ -383,7 +394,8 @@ func (p *podMeta) GenerateK8s(obj kubernetes.Resource, opts ...metadata.FieldOpt
 			"ip":   k8sPod.Status.PodIP,
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 		"annotations": mapstr.M{
 			"app": "production",

diff --git a/internal/pkg/composable/providers/kubernetes/service_test.go b/internal/pkg/composable/providers/kubernetes/service_test.go
@@ -25,7 +25,8 @@ func TestGenerateServiceData(t *testing.T) {
 			UID:       types.UID(uid),
 			Namespace: "testns",
 			Labels: map[string]string{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			Annotations: map[string]string{
 				"baz": "ban",
@@ -64,7 +65,8 @@ func TestGenerateServiceData(t *testing.T) {
 			"baz": "ban",
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 	}
 
@@ -80,7 +82,8 @@ func TestGenerateServiceData(t *testing.T) {
 				"ip":   "1.2.3.4",
 			},
 			"labels": mapstr.M{
-				"foo": "bar",
+				"foo":       "bar",
+				"with-dash": "dash-value",
 			},
 			"annotations": mapstr.M{
 				"baz": "ban",
@@ -139,7 +142,8 @@ func (s *svcMeta) GenerateK8s(obj kubernetes.Resource, opts ...metadata.FieldOpt
 			"ip":   "1.2.3.4",
 		},
 		"labels": mapstr.M{
-			"foo": "bar",
+			"foo":       "bar",
+			"with-dash": "dash-value",
 		},
 		"annotations": mapstr.M{
 			"baz": "ban",

diff --git a/internal/pkg/eql/Eql.g4 b/internal/pkg/eql/Eql.g4
@@ -22,7 +22,7 @@ NUMBER: [\-]? [0-9]+;
 WHITESPACE: [ \r\n\t]+ -> skip;
 NOT: 'NOT' | 'not';
 NAME: [a-zA-Z_] [a-zA-Z0-9_]*;
-VNAME: [a-zA-Z0-9_.]+('.'[a-zA-Z0-9_]+)*;
+VNAME: [a-zA-Z0-9_.-]+('.'[a-zA-Z0-9_-]+)*;
 STEXT: '\'' ~[\r\n']* '\'';
 DTEXT: '"' ~[\r\n"]* '"';
 LPAR: '(';

diff --git a/internal/pkg/eql/eql_test.go b/internal/pkg/eql/eql_test.go
@@ -42,6 +42,8 @@ func TestEql(t *testing.T) {
 		{expression: "${env.MISSING|host.MISSING|true} == true", result: true},
 		{expression: "${env.MISSING|host.MISSING|false} == false", result: true},
 		{expression: "${'constant'} == 'constant'", result: true},
+		{expression: "${data.with-dash} == 'dash-value'", result: true},
+		{expression: "${'dash-value'} == 'dash-value'", result: true},
 
 		// boolean
 		{expression: "true", result: true},
@@ -306,9 +308,10 @@ func TestEql(t *testing.T) {
 
 	store := &testVarStore{
 		vars: map[string]interface{}{
-			"env.HOSTNAME": "my-hostname",
-			"host.name":    "host-name",
-			"data.array":   []interface{}{"array1", "array2", "array3"},
+			"env.HOSTNAME":   "my-hostname",
+			"host.name":      "host-name",
+			"data.array":     []interface{}{"array1", "array2", "array3"},
+			"data.with-dash": "dash-value",
 			"data.dict": map[string]interface{}{
 				"key1": "dict1",
 				"key2": "dict2",
@@ -327,7 +330,7 @@ func TestEql(t *testing.T) {
 		}
 		t.Run(title, func(t *testing.T) {
 			if showDebug == "1" {
-				debug(test.expression)
+				debug(t, test.expression)
 			}
 
 			r, err := Eval(test.expression, store)
@@ -343,17 +346,17 @@ func TestEql(t *testing.T) {
 	}
 }
 
-func debug(expression string) {
+func debug(t *testing.T, expression string) {
 	raw := antlr.NewInputStream(expression)
 
 	lexer := parser.NewEqlLexer(raw)
 	for {
-		t := lexer.NextToken()
-		if t.GetTokenType() == antlr.TokenEOF {
+		token := lexer.NextToken()
+		if token.GetTokenType() == antlr.TokenEOF {
 			break
 		}
-		fmt.Printf("%s (%q)\n",
-			lexer.SymbolicNames[t.GetTokenType()], t.GetText())
+		t.Logf("%s (%q)\n",
+			lexer.SymbolicNames[token.GetTokenType()], token.GetText())
 	}
 }