Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eskip: use Predicate instead of matcher #2871

Merged
merged 1 commit into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 18 additions & 28 deletions eskip/eskip.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,6 @@ func (df *DefaultFilters) Do(routes []*Route) []*Route {
return nextRoutes
}

// Represents a matcher condition for incoming requests.
type matcher struct {
// The name of the matcher, e.g. Path or Header
name string

// The args of the matcher, e.g. the path to be matched.
args []interface{}
}

// BackendType indicates whether a route is a network backend, a shunt or a loopback.
type BackendType int

Expand All @@ -228,7 +219,7 @@ var errMixedProtocols = errors.New("loadbalancer endpoints cannot have mixed pro
// document.
type parsedRoute struct {
id string
matchers []*matcher
predicates []*Predicate
filters []*Filter
shunt bool
loopback bool
Expand Down Expand Up @@ -483,48 +474,48 @@ func applyPredicates(route *Route, proute *parsedRoute) error {
methodSet bool
)

for _, m := range proute.matchers {
for _, p := range proute.predicates {
if err != nil {
return err
}

switch m.name {
switch p.Name {
case "Path":
if pathSet {
return duplicatePathTreePredicateError
}

if args, err = getStringArgs(1, m.args); err == nil {
if args, err = getStringArgs(1, p.Args); err == nil {
route.Path = args[0]
pathSet = true
}
case "Host":
if args, err = getStringArgs(1, m.args); err == nil {
if args, err = getStringArgs(1, p.Args); err == nil {
route.HostRegexps = append(route.HostRegexps, args[0])
}
case "PathRegexp":
if args, err = getStringArgs(1, m.args); err == nil {
if args, err = getStringArgs(1, p.Args); err == nil {
route.PathRegexps = append(route.PathRegexps, args[0])
}
case "Method":
if methodSet {
return duplicateMethodPredicateError
}

if args, err = getStringArgs(1, m.args); err == nil {
if args, err = getStringArgs(1, p.Args); err == nil {
route.Method = args[0]
methodSet = true
}
case "HeaderRegexp":
if args, err = getStringArgs(2, m.args); err == nil {
if args, err = getStringArgs(2, p.Args); err == nil {
if route.HeaderRegexps == nil {
route.HeaderRegexps = make(map[string][]string)
}

route.HeaderRegexps[args[0]] = append(route.HeaderRegexps[args[0]], args[1])
}
case "Header":
if args, err = getStringArgs(2, m.args); err == nil {
if args, err = getStringArgs(2, p.Args); err == nil {
if route.Headers == nil {
route.Headers = make(map[string]string)
}
Expand All @@ -538,9 +529,7 @@ func applyPredicates(route *Route, proute *parsedRoute) error {
case "*", "Any":
// void
default:
route.Predicates = append(
route.Predicates,
&Predicate{m.name, m.args})
route.Predicates = append(route.Predicates, p)
}
}

Expand Down Expand Up @@ -698,15 +687,16 @@ func ParsePredicates(p string) ([]*Predicate, error) {
return nil, nil
}

var ps []*Predicate
for _, matcher := range rs[0].matchers {
if matcher.name != "*" {
ps = append(ps, &Predicate{
Name: matcher.name,
Args: matcher.args,
})
r := rs[0]
ps := make([]*Predicate, 0, len(r.predicates))
for _, p := range r.predicates {
if p.Name != "*" {
ps = append(ps, p)
}
}
if len(ps) == 0 {
ps = nil
}

return ps, nil
}
Expand Down
22 changes: 11 additions & 11 deletions eskip/parser.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 20 additions & 20 deletions eskip/parser.y
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright 2015 Zalando SE
//
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//
// http://www.apache.org/licenses/LICENSE-2.0
//
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -32,8 +32,8 @@ func convertNumber(s string) float64 {
token string
route *parsedRoute
routes []*parsedRoute
matchers []*matcher
matcher *matcher
predicates []*Predicate
predicate *Predicate
filter *Filter
filters []*Filter
args []interface{}
Expand Down Expand Up @@ -110,9 +110,9 @@ routeid:
}

route:
frontend arrow backend {
predicates arrow backend {
$$.route = &parsedRoute{
matchers: $1.matchers,
predicates: $1.predicates,
backend: $3.backend,
shunt: $3.shunt,
loopback: $3.loopback,
Expand All @@ -121,13 +121,13 @@ route:
lbAlgorithm: $3.lbAlgorithm,
lbEndpoints: $3.lbEndpoints,
}
$1.matchers = nil
$1.predicates = nil
$3.lbEndpoints = nil
}
|
frontend arrow filters arrow backend {
predicates arrow filters arrow backend {
$$.route = &parsedRoute{
matchers: $1.matchers,
predicates: $1.predicates,
filters: $3.filters,
backend: $5.backend,
shunt: $5.shunt,
Expand All @@ -137,28 +137,28 @@ route:
lbAlgorithm: $5.lbAlgorithm,
lbEndpoints: $5.lbEndpoints,
}
$1.matchers = nil
$1.predicates = nil
$3.filters = nil
$5.lbEndpoints = nil
}

frontend:
matcher {
$$.matchers = []*matcher{$1.matcher}
predicates:
Copy link
Member Author

@AlexanderYastrebov AlexanderYastrebov Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This renames rules consistently and opens a way to parse predicates and filters separately, see draft branch https://github.com/zalando/skipper/tree/eskip/separate-parse-predicates-filters based on this

predicate {
$$.predicates = []*Predicate{$1.predicate}
}
|
frontend and matcher {
$$.matchers = $1.matchers
$$.matchers = append($$.matchers, $3.matcher)
predicates and predicate {
$$.predicates = $1.predicates
$$.predicates = append($$.predicates, $3.predicate)
}

matcher:
predicate:
any {
$$.matcher = &matcher{"*", nil}
$$.predicate = &Predicate{"*", nil}
}
|
symbol openparen args closeparen {
$$.matcher = &matcher{$1.token, $3.args}
$$.predicate = &Predicate{$1.token, $3.args}
$3.args = nil
}

Expand Down
12 changes: 6 additions & 6 deletions eskip/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ const (
)

func checkSingleRouteExample(r *parsedRoute, t *testing.T) {
if len(r.matchers) != 2 ||
r.matchers[0].name != "PathRegexp" || len(r.matchers[0].args) != 1 ||
r.matchers[0].args[0] != "\\.html$" ||
r.matchers[1].name != "Header" || len(r.matchers[1].args) != 2 ||
r.matchers[1].args[0] != "Accept" || r.matchers[1].args[1] != "text/html" {
if len(r.predicates) != 2 ||
r.predicates[0].Name != "PathRegexp" || len(r.predicates[0].Args) != 1 ||
r.predicates[0].Args[0] != "\\.html$" ||
r.predicates[1].Name != "Header" || len(r.predicates[1].Args) != 2 ||
r.predicates[1].Args[0] != "Accept" || r.predicates[1].Args[1] != "text/html" {
t.Error("failed to parse match expression")
}

Expand Down Expand Up @@ -227,7 +227,7 @@ func testRegExpOnce(t *testing.T, regexpStr string, expectedRegExp string) {
return
}

if expectedRegExp != routes[0].matchers[0].args[0] {
if expectedRegExp != routes[0].predicates[0].Args[0] {
t.Error("failed to parse PathRegexp:"+regexpStr+", expected regexp to be "+expectedRegExp, err)
}
}
Expand Down