eskip: do not add empty last route id to parse error (#2875)

Follow up on #1885

Signed-off-by: Alexander Yastrebov <[email protected]>

cmd/webhook/admission/admission_test.go

@@ -107,7 +107,7 @@ func TestRouteGroupAdmitter(t *testing.T) {
 		{
 			name:      "invalid eskip filters",
 			inputFile: "rg-with-invalid-eskip-filters.json",
-			message:   "parse failed after token status, last route id: , position 11: syntax error",
+			message:   "parse failed after token status, position 11: syntax error",
 		},
 		{
 			name:      "valid eskip predicates",
@@ -116,12 +116,12 @@ func TestRouteGroupAdmitter(t *testing.T) {
 		{
 			name:      "invalid eskip predicates",
 			inputFile: "rg-with-invalid-eskip-predicates.json",
-			message:   "parse failed after token Method, last route id: Method, position 6: syntax error",
+			message:   "parse failed after token Method, position 6: syntax error",
 		},
 		{
 			name:      "invalid eskip filters and predicates",
 			inputFile: "rg-with-invalid-eskip-filters-and-predicates.json",
-			message:   "parse failed after token status, last route id: , position 11: syntax error\\nparse failed after token Method, last route id: Method, position 6: syntax error",
+			message:   "parse failed after token status, position 11: syntax error\\nparse failed after token Method, position 6: syntax error",
 		},
 		{
 			name:      "invalid routgroup multiple filters per json/yaml array item",
@@ -185,12 +185,12 @@ func TestIngressAdmitter(t *testing.T) {
 		{
 			name:      "invalid eskip filters",
 			inputFile: "invalid-filters.json",
-			message:   `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, last route id: , position 9: syntax error`,
+			message:   `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, position 9: syntax error`,
 		},
 		{
 			name:      "invalid eskip predicates",
 			inputFile: "invalid-predicates.json",
-			message:   `invalid \"zalando.org/skipper-predicate\" annotation: parse failed after token ), last route id: , position 15: syntax error`,
+			message:   `invalid \"zalando.org/skipper-predicate\" annotation: parse failed after token ), position 15: syntax error`,
 		},
 		{
 			name:      "invalid eskip routes",
@@ -200,7 +200,7 @@ func TestIngressAdmitter(t *testing.T) {
 		{
 			name:      "invalid eskip filters and predicates",
 			inputFile: "invalid-filters-and-predicates.json",
-			message:   `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, last route id: , position 9: syntax error\ninvalid \"zalando.org/skipper-predicate\" annotation: parse failed after token ), last route id: , position 15: syntax error`,
+			message:   `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, position 9: syntax error\ninvalid \"zalando.org/skipper-predicate\" annotation: parse failed after token ), position 15: syntax error`,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {

config/config_test.go

@@ -276,7 +276,7 @@ func Test_Validate(t *testing.T) {
 				c.KubernetesEastWestRangePredicatesString = "WrongEastWestMode"
 			},
 			wantErr: true,
-			want:    errors.New("invalid east-west-range-predicates: parse failed after token ->, last route id: WrongEastWestMode, position 20: syntax error"),
+			want:    errors.New("invalid east-west-range-predicates: parse failed after token ->, position 20: syntax error"),
 		},
 		{
 			name: "test wrong HistoGramBuckets",

dataclients/kubernetes/testdata/routegroups/convert/failing-filter.log

@@ -1 +1 @@
-\[routegroup\] parse failed after token foo, last route id: , position 8: syntax error
+\[routegroup\] parse failed after token foo, position 8: syntax error

dataclients/kubernetes/testdata/routegroups/convert/failing-predicate.log

@@ -1 +1 @@
-\[routegroup\] parse failed after token Header, last route id: Header, position 6: syntax error
+\[routegroup\] parse failed after token Header, position 6: syntax error

dataclients/kubernetes/testdata/routegroups/convert/no-catchall-for-failed-route-group.log

@@ -1 +1 @@
-\[routegroup\] parse failed after token foo, last route id: , position 8: syntax error
+\[routegroup\] parse failed after token foo, position 8: syntax error

dataclients/kubernetes/testdata/routegroups/convert/single-failing-route-makes-the-group-fail.log

@@ -1 +1 @@
-\[routegroup\] parse failed after token foo, last route id: foo, position 3: syntax error
+\[routegroup\] parse failed after token foo, position 3: syntax error

eskip/eskip_test.go

@@ -183,8 +183,7 @@ func TestParse(t *testing.T) {
 		"syntax without id",
 		`* -> #`,
 		nil,
-		// TODO: remove empty last route id
-		"parse failed after token ->, last route id: , position 5: syntax error",
+		"parse failed after token ->, position 5: syntax error",
 	}} {
 		t.Run(ti.msg, func(t *testing.T) {
 			routes, err := Parse(ti.expression)
@@ -240,14 +239,14 @@ func TestParseFilters(t *testing.T) {
 		"error",
 		"trallala",
 		nil,
-		// TODO: remove empty last route id and fix position
-		"parse failed after token ->, last route id: , position 16: syntax error",
+		// TODO: fix position
+		"parse failed after token ->, position 16: syntax error",
 	}, {
 		"error 2",
 		"foo-bar",
 		nil,
-		// TODO: remove empty last route id and fix position
-		"parse failed after token foo, last route id: , position 8: syntax error",
+		// TODO: fix position
+		"parse failed after token foo, position 8: syntax error",
 	}, {
 		"success",
 		`filter1(3.14) -> filter2("key", 42)`,
@@ -279,13 +278,11 @@ func TestParsePredicates(t *testing.T) {
 	}, {
 		title: "invalid",
 		input: `not predicates`,
-		// TODO: fix wrong last route id
-		err: "parse failed after token predicates, last route id: not, position 14: syntax error",
+		err:   "parse failed after token predicates, position 14: syntax error",
 	}, {
 		title: "invalid",
 		input: `Header#`,
-		// TODO: fix wrong last route id
-		err: "parse failed after token Header, last route id: Header, position 6: syntax error",
+		err:   "parse failed after token Header, position 6: syntax error",
 	}, {
 		title:    "single predicate",
 		input:    `Foo("bar")`,

eskip/lexer.go

@@ -367,7 +367,11 @@ func (l *eskipLex) Lex(lval *eskipSymType) int {
 }
 
 func (l *eskipLex) Error(err string) {
+	lastRouteID := ""
+	if l.lastRouteID != "" {
+		lastRouteID = ", last route id: " + l.lastRouteID
+	}
 	l.err = fmt.Errorf(
-		"parse failed after token %s, last route id: %v, position %d: %s",
-		l.lastToken, l.lastRouteID, l.initialLength-len(l.code), err)
+		"parse failed after token %s%s, position %d: %s",
+		l.lastToken, lastRouteID, l.initialLength-len(l.code), err)
 }

eskip/parser.y

@@ -98,13 +98,14 @@ routes:
 	}
 
 routedef:
-	routeid colon route {
-		$$.route = $3.route
+	routeid route {
+		$$.route = $2.route
 		$$.route.id = $1.token
 	}
 
 routeid:
-	symbol {
+	symbol colon {
+		// match symbol and colon to get route id early even if route parsing fails later
 		$$.token = $1.token
 		eskiplex.(*eskipLex).lastRouteID = $1.token
 	}