From d427d288f0db7879996763b4657d3013b43e21b2 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:46:06 +0200 Subject: [PATCH 01/32] renamed and moved request and pattern matcher as these are going to be used directly by the rule factory implementaiton --- internal/rules/config/matcher_constraints.go | 46 ------- .../rules/config/matcher_constraints_test.go | 88 ------------ .../request_matcher.go => route_matcher.go} | 129 ++++++++++++------ ..._matcher_test.go => route_matcher_test.go} | 2 +- .../pattern_matcher.go => typed_matcher.go} | 14 +- ..._matcher_test.go => typed_matcher_test.go} | 0 6 files changed, 100 insertions(+), 179 deletions(-) delete mode 100644 internal/rules/config/matcher_constraints.go delete mode 100644 internal/rules/config/matcher_constraints_test.go rename internal/rules/{config/request_matcher.go => route_matcher.go} (53%) rename internal/rules/{config/request_matcher_test.go => route_matcher_test.go} (99%) rename internal/rules/{config/pattern_matcher.go => typed_matcher.go} (71%) rename internal/rules/{config/pattern_matcher_test.go => typed_matcher_test.go} (100%) diff --git a/internal/rules/config/matcher_constraints.go b/internal/rules/config/matcher_constraints.go deleted file mode 100644 index c51086b74..000000000 --- a/internal/rules/config/matcher_constraints.go +++ /dev/null @@ -1,46 +0,0 @@ -package config - -import "slices" - -type MatcherConstraints struct { - Scheme string `json:"scheme" yaml:"scheme" validate:"omitempty,oneof=http https"` //nolint:tagalign - Methods []string `json:"methods" yaml:"methods" validate:"omitempty,dive,required"` //nolint:tagalign - HostGlob string `json:"host_glob" yaml:"host_glob" validate:"excluded_with=HostRegex"` //nolint:tagalign - HostRegex string `json:"host_regex" yaml:"host_regex" validate:"excluded_with=HostGlob"` //nolint:tagalign - PathGlob string `json:"path_glob" yaml:"path_glob" validate:"excluded_with=PathRegex"` //nolint:tagalign - PathRegex string `json:"path_regex" yaml:"path_regex" validate:"excluded_with=PathGlob"` //nolint:tagalign -} - -func (mc *MatcherConstraints) ToRequestMatcher(slashHandling EncodedSlashesHandling) (RequestMatcher, error) { - if mc == nil { - return compositeMatcher{}, nil - } - - hostMatcher, err := createHostMatcher(mc.HostGlob, mc.HostRegex) - if err != nil { - return nil, err - } - - pathMatcher, err := createPathMatcher(mc.PathGlob, mc.PathRegex, slashHandling) - if err != nil { - return nil, err - } - - methodMatcher, err := createMethodMatcher(mc.Methods) - if err != nil { - return nil, err - } - - return compositeMatcher{ - schemeMatcher(mc.Scheme), - methodMatcher, - hostMatcher, - pathMatcher, - }, nil -} - -func (mc *MatcherConstraints) DeepCopyInto(out *MatcherConstraints) { - *out = *mc - - out.Methods = slices.Clone(mc.Methods) -} diff --git a/internal/rules/config/matcher_constraints_test.go b/internal/rules/config/matcher_constraints_test.go deleted file mode 100644 index d5d840c7e..000000000 --- a/internal/rules/config/matcher_constraints_test.go +++ /dev/null @@ -1,88 +0,0 @@ -package config - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/dadrus/heimdall/internal/heimdall" -) - -func TestMatcherConstraintsToRequestMatcher(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - uc string - constraints *MatcherConstraints - slashHandling EncodedSlashesHandling - assert func(t *testing.T, matcher RequestMatcher, err error) - }{ - { - uc: "no constraints", - assert: func(t *testing.T, matcher RequestMatcher, err error) { - t.Helper() - - require.NoError(t, err) - assert.Empty(t, matcher) - }, - }, - { - uc: "host matcher creation fails", - constraints: &MatcherConstraints{HostRegex: "?>?<*??"}, - assert: func(t *testing.T, _ RequestMatcher, err error) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.ErrorContains(t, err, "filed to compile host expression") - }, - }, - { - uc: "path matcher creation fails", - constraints: &MatcherConstraints{PathRegex: "?>?<*??"}, - assert: func(t *testing.T, _ RequestMatcher, err error) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.ErrorContains(t, err, "filed to compile path expression") - }, - }, - { - uc: "method matcher creation fails", - constraints: &MatcherConstraints{Methods: []string{""}}, - assert: func(t *testing.T, _ RequestMatcher, err error) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.ErrorContains(t, err, "methods list contains empty values") - }, - }, - { - uc: "with all matchers", - constraints: &MatcherConstraints{ - Methods: []string{"GET"}, - Scheme: "https", - HostRegex: "^example.com", - PathGlob: "/foo/bar/*", - }, - assert: func(t *testing.T, matcher RequestMatcher, err error) { - t.Helper() - - require.NoError(t, err) - assert.Len(t, matcher, 4) - - assert.Contains(t, matcher, schemeMatcher("https")) - assert.Contains(t, matcher, methodMatcher{"GET"}) - }, - }, - } { - t.Run(tc.uc, func(t *testing.T) { - matcher, err := tc.constraints.ToRequestMatcher(tc.slashHandling) - - tc.assert(t, matcher, err) - }) - } -} diff --git a/internal/rules/config/request_matcher.go b/internal/rules/route_matcher.go similarity index 53% rename from internal/rules/config/request_matcher.go rename to internal/rules/route_matcher.go index 63d2c3cd6..558a68644 100644 --- a/internal/rules/config/request_matcher.go +++ b/internal/rules/route_matcher.go @@ -1,7 +1,8 @@ -package config +package rules import ( "errors" + "github.com/dadrus/heimdall/internal/rules/config" "net/http" "net/url" "slices" @@ -22,17 +23,17 @@ var ( ErrRequestPathMismatch = errors.New("request path mismatch") ) -//go:generate mockery --name RequestMatcher --structname RequestMatcherMock +//go:generate mockery --name RouteMatcher --structname RouteMatcherMock -type RequestMatcher interface { - Matches(request *heimdall.Request) error +type RouteMatcher interface { + Matches(request *heimdall.Request, keys, values []string) error } -type compositeMatcher []RequestMatcher +type compositeMatcher []RouteMatcher -func (c compositeMatcher) Matches(request *heimdall.Request) error { +func (c compositeMatcher) Matches(request *heimdall.Request, keys, values []string) error { for _, matcher := range c { - if err := matcher.Matches(request); err != nil { + if err := matcher.Matches(request, keys, values); err != nil { return err } } @@ -46,7 +47,7 @@ func (alwaysMatcher) match(_ string) bool { return true } type schemeMatcher string -func (s schemeMatcher) Matches(request *heimdall.Request) error { +func (s schemeMatcher) Matches(request *heimdall.Request, _, _ []string) error { if len(s) != 0 && string(s) != request.URL.Scheme { return errorchain.NewWithMessagef(ErrRequestSchemeMismatch, "expected %s, got %s", s, request.URL.Scheme) } @@ -56,7 +57,7 @@ func (s schemeMatcher) Matches(request *heimdall.Request) error { type methodMatcher []string -func (m methodMatcher) Matches(request *heimdall.Request) error { +func (m methodMatcher) Matches(request *heimdall.Request, _, _ []string) error { if len(m) == 0 { return nil } @@ -69,10 +70,10 @@ func (m methodMatcher) Matches(request *heimdall.Request) error { } type hostMatcher struct { - patternMatcher + typedMatcher } -func (m *hostMatcher) Matches(request *heimdall.Request) error { +func (m *hostMatcher) Matches(request *heimdall.Request, _, _ []string) error { if !m.match(request.URL.Host) { return errorchain.NewWithMessagef(ErrRequestHostMismatch, "%s is not expected", request.URL.Host) } @@ -80,15 +81,35 @@ func (m *hostMatcher) Matches(request *heimdall.Request) error { return nil } +type paramMatcher struct { + typedMatcher + + name string +} + +func (m *paramMatcher) Matches(_ *heimdall.Request, keys, values []string) error { + idx := slices.Index(keys, m.name) + if idx == -1 { + return errorchain.NewWithMessagef(ErrRequestPathMismatch, "%s is not expected", m.name) + } + + value := values[idx] + if !m.match(value) { + return errorchain.NewWithMessagef(ErrRequestPathMismatch, "%s is not expected", value) + } + + return nil +} + type pathMatcher struct { - patternMatcher + typedMatcher - slashHandling EncodedSlashesHandling + slashHandling config.EncodedSlashesHandling } func (m *pathMatcher) Matches(request *heimdall.Request) error { var path string - if len(request.URL.RawPath) == 0 || m.slashHandling == EncodedSlashesOn { + if len(request.URL.RawPath) == 0 || m.slashHandling == config.EncodedSlashesOn { path = request.URL.Path } else { unescaped, _ := url.PathUnescape(strings.ReplaceAll(request.URL.RawPath, "%2F", "$$$escaped-slash$$$")) @@ -130,38 +151,64 @@ func createMethodMatcher(methods []string) (methodMatcher, error) { return slicex.Subtract(methods, tbr), nil } -func createPathMatcher( - globExpression string, regexExpression string, slashHandling EncodedSlashesHandling, -) (*pathMatcher, error) { - matcher, err := createPatternMatcher(globExpression, '/', regexExpression) - if err != nil { - return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "filed to compile path expression").CausedBy(err) - } +func createHostMatcher(hosts []config.HostMatcher) (RouteMatcher, error) { + matchers := make(compositeMatcher, len(hosts)) + + for idx, host := range hosts { + var ( + tm typedMatcher + err error + ) + + switch host.Type { + case "glob": + tm, err = newGlobMatcher(host.Value, '.') + case "regex": + tm, err = newRegexMatcher(host.Value) + case "exact": + matchers[idx] = &hostMatcher{&valueMatcher{host.Value}} + default: + return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, + "unsupported expression type for host") + } - return &pathMatcher{matcher, slashHandling}, nil -} + if err != nil { + return nil, err + } -func createHostMatcher(globExpression string, regexExpression string) (*hostMatcher, error) { - matcher, err := createPatternMatcher(globExpression, '.', regexExpression) - if err != nil { - return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "filed to compile host expression").CausedBy(err) + matchers[idx] = &hostMatcher{tm} } - return &hostMatcher{matcher}, nil + return matchers, nil } -func createPatternMatcher(globExpression string, globSeparator rune, regexExpression string) (patternMatcher, error) { - glob := spaceReplacer.Replace(globExpression) - regex := spaceReplacer.Replace(regexExpression) - - switch { - case len(glob) != 0: - return newGlobMatcher(glob, globSeparator) - case len(regex) != 0: - return newRegexMatcher(regex) - default: - return alwaysMatcher{}, nil +func createParamsMatcher(params []config.ParameterMatcher) (RouteMatcher, error) { + matchers := make(compositeMatcher, len(params)) + + for idx, param := range params { + var ( + tm typedMatcher + err error + ) + + switch param.Type { + case "glob": + tm, err = newGlobMatcher(param.Value, '/') + case "regex": + tm, err = newRegexMatcher(param.Value) + case "exact": + matchers[idx] = ¶mMatcher{&valueMatcher{param.Value}, param.Name} + default: + return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, + "unsupported expression type for parameter") + } + + if err != nil { + return nil, err + } + + matchers[idx] = ¶mMatcher{tm, param.Name} } + + return matchers, nil } diff --git a/internal/rules/config/request_matcher_test.go b/internal/rules/route_matcher_test.go similarity index 99% rename from internal/rules/config/request_matcher_test.go rename to internal/rules/route_matcher_test.go index d98c23069..c7b2c9620 100644 --- a/internal/rules/config/request_matcher_test.go +++ b/internal/rules/route_matcher_test.go @@ -1,4 +1,4 @@ -package config +package rules import ( "net/http" diff --git a/internal/rules/config/pattern_matcher.go b/internal/rules/typed_matcher.go similarity index 71% rename from internal/rules/config/pattern_matcher.go rename to internal/rules/typed_matcher.go index 7206d66bb..cfbaf1523 100644 --- a/internal/rules/config/pattern_matcher.go +++ b/internal/rules/typed_matcher.go @@ -13,7 +13,7 @@ var ( ) type ( - patternMatcher interface { + typedMatcher interface { match(pattern string) bool } @@ -24,6 +24,10 @@ type ( regexpMatcher struct { compiled *regexp.Regexp } + + valueMatcher struct { + value string + } ) func (m *globMatcher) match(value string) bool { @@ -34,7 +38,9 @@ func (m *regexpMatcher) match(matchAgainst string) bool { return m.compiled.MatchString(matchAgainst) } -func newGlobMatcher(pattern string, separator rune) (patternMatcher, error) { +func (m *valueMatcher) match(value string) bool { return m.value == value } + +func newGlobMatcher(pattern string, separator rune) (typedMatcher, error) { if len(pattern) == 0 { return nil, ErrNoGlobPatternDefined } @@ -47,7 +53,7 @@ func newGlobMatcher(pattern string, separator rune) (patternMatcher, error) { return &globMatcher{compiled: compiled}, nil } -func newRegexMatcher(pattern string) (patternMatcher, error) { +func newRegexMatcher(pattern string) (typedMatcher, error) { if len(pattern) == 0 { return nil, ErrNoRegexPatternDefined } @@ -59,3 +65,5 @@ func newRegexMatcher(pattern string) (patternMatcher, error) { return ®expMatcher{compiled: compiled}, nil } + +func newValueMatcher(value string) (typedMatcher, error) { return &valueMatcher{value: value}, nil } diff --git a/internal/rules/config/pattern_matcher_test.go b/internal/rules/typed_matcher_test.go similarity index 100% rename from internal/rules/config/pattern_matcher_test.go rename to internal/rules/typed_matcher_test.go From 7a0ddcf61c1fbd50dbc120777eb1487b468af039 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:47:05 +0200 Subject: [PATCH 02/32] radic tree implementation updated to allow matching of captured values --- internal/x/radixtree/matcher.go | 31 +++++++--- internal/x/radixtree/tree.go | 55 +++++++----------- internal/x/radixtree/tree_benchmark_test.go | 28 ++++----- internal/x/radixtree/tree_test.go | 64 +++++++++++++-------- 4 files changed, 97 insertions(+), 81 deletions(-) diff --git a/internal/x/radixtree/matcher.go b/internal/x/radixtree/matcher.go index 7a6f84249..622e838b6 100644 --- a/internal/x/radixtree/matcher.go +++ b/internal/x/radixtree/matcher.go @@ -1,18 +1,33 @@ package radixtree -// Matcher is used for additional checks while performing the lookup in the spanned tree. -type Matcher[V any] interface { - // Match should return true if the value should be returned by the lookup. If it returns false, it - // instructs the lookup to continue with backtracking from the current tree position. +// LookupMatcher is used for additional checks while performing the lookup of values in the spanned tree. +type LookupMatcher[V any] interface { + // Match should return true if the value should be returned by the lookup. + Match(value V, keys, values []string) bool +} + +// The LookupMatcherFunc type is an adapter to allow the use of ordinary functions as match functions. +// If f is a function with the appropriate signature, LookupMatcherFunc(f) is a [LookupMatcher] +// that calls f. +type LookupMatcherFunc[V any] func(value V, keys, values []string) bool + +// Match calls f(value). +func (f LookupMatcherFunc[V]) Match(value V, keys, values []string) bool { + return f(value, keys, values) +} + +// ValueMatcher is used for additional checks while deleting of values in the spanned tree. +type ValueMatcher[V any] interface { + // Match should return true if the value should be deleted from the tree. Match(value V) bool } -// The MatcherFunc type is an adapter to allow the use of ordinary functions as match functions. -// If f is a function with the appropriate signature, MatcherFunc(f) is a [Matcher] +// The ValueMatcherFunc type is an adapter to allow the use of ordinary functions as match functions. +// If f is a function with the appropriate signature, ValueMatcherFunc(f) is a [ValueMatcher] // that calls f. -type MatcherFunc[V any] func(value V) bool +type ValueMatcherFunc[V any] func(value V) bool // Match calls f(value). -func (f MatcherFunc[V]) Match(value V) bool { +func (f ValueMatcherFunc[V]) Match(value V) bool { return f(value) } diff --git a/internal/x/radixtree/tree.go b/internal/x/radixtree/tree.go index bbb07087e..2324b13f1 100644 --- a/internal/x/radixtree/tree.go +++ b/internal/x/radixtree/tree.go @@ -206,7 +206,7 @@ func (n *Tree[V]) addNode(path string, wildcardKeys []string, inStaticToken bool } //nolint:cyclop,funlen -func (n *Tree[V]) delNode(path string, matcher Matcher[V]) bool { +func (n *Tree[V]) delNode(path string, matcher ValueMatcher[V]) bool { pathLen := len(path) if pathLen == 0 { if len(n.values) == 0 { @@ -330,12 +330,11 @@ func (n *Tree[V]) delEdge(token byte) { } //nolint:funlen,gocognit,cyclop -func (n *Tree[V]) findNode(path string, matcher Matcher[V]) (*Tree[V], int, []string, bool) { +func (n *Tree[V]) findNode(path string, captures []string, matcher LookupMatcher[V]) (*Tree[V], int, []string, bool) { var ( - found *Tree[V] - params []string - idx int - value V + found *Tree[V] + idx int + value V ) backtrack := true @@ -347,8 +346,8 @@ func (n *Tree[V]) findNode(path string, matcher Matcher[V]) (*Tree[V], int, []st } for idx, value = range n.values { - if match := matcher.Match(value); match { - return n, idx, nil, false + if match := matcher.Match(value, n.wildcardKeys, captures); match { + return n, idx, captures, false } } @@ -364,7 +363,7 @@ func (n *Tree[V]) findNode(path string, matcher Matcher[V]) (*Tree[V], int, []st if pathLen >= childPathLen && child.path == path[:childPathLen] { nextPath := path[childPathLen:] - found, idx, params, backtrack = child.findNode(nextPath, matcher) + found, idx, captures, backtrack = child.findNode(nextPath, captures, matcher) } break @@ -372,7 +371,7 @@ func (n *Tree[V]) findNode(path string, matcher Matcher[V]) (*Tree[V], int, []st } if found != nil || !backtrack { - return found, idx, params, backtrack + return found, idx, captures, backtrack } if n.wildcardChild != nil { //nolint:nestif @@ -382,15 +381,10 @@ func (n *Tree[V]) findNode(path string, matcher Matcher[V]) (*Tree[V], int, []st nextToken := path[nextSeparator:] if len(thisToken) > 0 { // Don't match on empty tokens. - found, idx, params, backtrack = n.wildcardChild.findNode(nextToken, matcher) + tmp := append(captures, thisToken) + found, idx, tmp, backtrack = n.wildcardChild.findNode(nextToken, tmp, matcher) if found != nil { - if params == nil { - // we don't expect more than 3 parameters to be defined for a path - // even 3 is already too much - params = make([]string, 0, 3) //nolint:mnd - } - - return found, idx, append(params, thisToken), backtrack + return found, idx, tmp, backtrack } else if !backtrack { return nil, 0, nil, false } @@ -400,20 +394,15 @@ func (n *Tree[V]) findNode(path string, matcher Matcher[V]) (*Tree[V], int, []st if n.catchAllChild != nil { // Hit the catchall, so just assign the whole remaining path. for idx, value = range n.catchAllChild.values { - if match := matcher.Match(value); match { - // we don't expect more than 3 parameters to be defined for a path - // even 3 is already too much - params = make([]string, 1, 3) //nolint:mnd - params[0] = path - - return n.catchAllChild, idx, params, false + if match := matcher.Match(value, n.wildcardKeys, captures); match { + return n.catchAllChild, idx, append(captures, path), false } } - return nil, 0, nil, n.backtrackingEnabled + return nil, 0, captures, n.backtrackingEnabled } - return nil, 0, nil, true + return nil, 0, captures, true } func (n *Tree[V]) splitCommonPrefix(existingNodeIndex int, path string) (*Tree[V], int) { @@ -448,8 +437,8 @@ func (n *Tree[V]) splitCommonPrefix(existingNodeIndex int, path string) (*Tree[V return newNode, i } -func (n *Tree[V]) Find(path string, matcher Matcher[V]) (*Entry[V], error) { - found, idx, params, _ := n.findNode(path, matcher) +func (n *Tree[V]) Find(path string, matcher LookupMatcher[V]) (*Entry[V], error) { + found, idx, params, _ := n.findNode(path, make([]string, 0, 3), matcher) if found == nil { return nil, fmt.Errorf("%w: %s", ErrNotFound, path) } @@ -458,14 +447,10 @@ func (n *Tree[V]) Find(path string, matcher Matcher[V]) (*Entry[V], error) { Value: found.values[idx], } - if len(params) == 0 { - return entry, nil - } - entry.Parameters = make(map[string]string, len(params)) for i, param := range params { - key := found.wildcardKeys[len(params)-1-i] + key := found.wildcardKeys[i] if key != "*" { entry.Parameters[key] = param } @@ -493,7 +478,7 @@ func (n *Tree[V]) Add(path string, value V, opts ...AddOption[V]) error { return nil } -func (n *Tree[V]) Delete(path string, matcher Matcher[V]) error { +func (n *Tree[V]) Delete(path string, matcher ValueMatcher[V]) error { if !n.delNode(path, matcher) { return fmt.Errorf("%w: %s", ErrFailedToDelete, path) } diff --git a/internal/x/radixtree/tree_benchmark_test.go b/internal/x/radixtree/tree_benchmark_test.go index 93556eb3e..4f90aa6e0 100644 --- a/internal/x/radixtree/tree_benchmark_test.go +++ b/internal/x/radixtree/tree_benchmark_test.go @@ -7,7 +7,7 @@ import ( ) func BenchmarkNodeSearchNoPaths(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -17,12 +17,12 @@ func BenchmarkNodeSearchNoPaths(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("", tm) + tree.findNode("", nil, tm) } } func BenchmarkNodeSearchRoot(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -32,12 +32,12 @@ func BenchmarkNodeSearchRoot(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("/", tm) + tree.findNode("/", nil, tm) } } func BenchmarkNodeSearchOneStaticPath(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -49,12 +49,12 @@ func BenchmarkNodeSearchOneStaticPath(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("abc", tm) + tree.findNode("abc", nil, tm) } } func BenchmarkNodeSearchOneLongStaticPath(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -66,12 +66,12 @@ func BenchmarkNodeSearchOneLongStaticPath(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("foo/bar/baz", tm) + tree.findNode("foo/bar/baz", nil, tm) } } func BenchmarkNodeSearchOneWildcardPath(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -83,12 +83,12 @@ func BenchmarkNodeSearchOneWildcardPath(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("abc", tm) + tree.findNode("abc", nil, tm) } } func BenchmarkNodeSearchOneLongWildcards(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -100,12 +100,12 @@ func BenchmarkNodeSearchOneLongWildcards(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("abcdefghijklmnop/aaaabbbbccccddddeeeeffffgggg/hijkl", tm) + tree.findNode("abcdefghijklmnop/aaaabbbbccccddddeeeeffffgggg/hijkl", nil, tm) } } func BenchmarkNodeSearchOneFreeWildcard(b *testing.B) { - tm := testMatcher[string](true) + tm := lookupMatcher[string](true) tree := &Tree[string]{ path: "/", canAdd: func(_ []string, _ string) bool { return true }, @@ -117,6 +117,6 @@ func BenchmarkNodeSearchOneFreeWildcard(b *testing.B) { b.ResetTimer() for range b.N { - tree.findNode("foo", tm) + tree.findNode("foo", nil, tm) } } diff --git a/internal/x/radixtree/tree_test.go b/internal/x/radixtree/tree_test.go index 283d207e0..8cc4f6198 100644 --- a/internal/x/radixtree/tree_test.go +++ b/internal/x/radixtree/tree_test.go @@ -9,7 +9,13 @@ import ( "golang.org/x/exp/maps" ) -func testMatcher[V any](matches bool) MatcherFunc[V] { return func(_ V) bool { return matches } } +func lookupMatcher[V any](matches bool) LookupMatcherFunc[V] { + return func(_ V, _, _ []string) bool { return matches } +} + +func deleteMatcher[V any](matches bool) ValueMatcherFunc[V] { + return func(_ V) bool { return matches } +} func TestTreeSearch(t *testing.T) { t.Parallel() @@ -62,15 +68,15 @@ func TestTreeSearch(t *testing.T) { require.NoError(t, err) } - trueMatcher := testMatcher[string](true) - falseMatcher := testMatcher[string](false) + trueMatcher := lookupMatcher[string](true) + falseMatcher := lookupMatcher[string](false) for _, tc := range []struct { path string expPath string expErr error expParams map[string]string - matcher Matcher[string] + matcher LookupMatcher[string] }{ {path: "/users/abc/updatePassword", expPath: "/users/:id/updatePassword", expParams: map[string]string{"id": "abc"}}, {path: "/users/all/something", expPath: "/users/:pk/:related", expParams: map[string]string{"pk": "all", "related": "something"}}, @@ -121,7 +127,7 @@ func TestTreeSearch(t *testing.T) { {path: "/カ", expPath: "/カ"}, } { t.Run(tc.path, func(t *testing.T) { - var matcher Matcher[string] + var matcher LookupMatcher[string] if tc.matcher == nil { matcher = trueMatcher } else { @@ -138,7 +144,12 @@ func TestTreeSearch(t *testing.T) { require.NoError(t, err) assert.Equalf(t, tc.expPath, entry.Value, "Path %s matched %s, expected %s", tc.path, entry.Value, tc.expPath) - assert.Equal(t, tc.expParams, entry.Parameters, "Path %s expected parameters are %v, saw %v", tc.path, tc.expParams, entry.Parameters) + + expParams := tc.expParams + if expParams == nil { + expParams = map[string]string{} + } + assert.Equal(t, expParams, entry.Parameters, "Path %s expected parameters are %v, saw %v", tc.path, tc.expParams, entry.Parameters) }) } } @@ -156,7 +167,8 @@ func TestTreeSearchWithBacktracking(t *testing.T) { require.NoError(t, err) // WHEN - entry, err := tree.Find("/date/2024/abc", MatcherFunc[string](func(value string) bool { return value != "first" })) + entry, err := tree.Find("/date/2024/abc", + LookupMatcherFunc[string](func(value string, _, _ []string) bool { return value != "first" })) // THEN require.NoError(t, err) @@ -176,9 +188,10 @@ func TestTreeSearchWithoutBacktracking(t *testing.T) { require.NoError(t, err) // WHEN - entry, err := tree.Find("/date/2024/abc", MatcherFunc[string](func(value string) bool { - return value != "first" - })) + entry, err := tree.Find("/date/2024/abc", + LookupMatcherFunc[string](func(value string, _, _ []string) bool { + return value != "first" + })) // THEN require.Error(t, err) @@ -198,16 +211,18 @@ func TestTreeAddPathDuplicates(t *testing.T) { err = tree.Add(path, "second") require.NoError(t, err) - entry, err := tree.Find("/date/2024/04/abc", MatcherFunc[string](func(value string) bool { - return value == "first" - })) + entry, err := tree.Find("/date/2024/04/abc", + LookupMatcherFunc[string](func(value string, _, _ []string) bool { + return value == "first" + })) require.NoError(t, err) assert.Equal(t, "first", entry.Value) assert.Equal(t, map[string]string{"year": "2024", "month": "04"}, entry.Parameters) - entry, err = tree.Find("/date/2024/04/abc", MatcherFunc[string](func(value string) bool { - return value == "second" - })) + entry, err = tree.Find("/date/2024/04/abc", + LookupMatcherFunc[string](func(value string, _, _ []string) bool { + return value == "second" + })) require.NoError(t, err) assert.Equal(t, "second", entry.Value) assert.Equal(t, map[string]string{"year": "2024", "month": "04"}, entry.Parameters) @@ -281,10 +296,10 @@ func TestTreeDeleteStaticPaths(t *testing.T) { } for i := len(paths) - 1; i >= 0; i-- { - err := tree.Delete(paths[i], testMatcher[int](true)) + err := tree.Delete(paths[i], deleteMatcher[int](true)) require.NoError(t, err) - err = tree.Delete(paths[i], testMatcher[int](true)) + err = tree.Delete(paths[i], deleteMatcher[int](true)) require.Error(t, err) } } @@ -316,16 +331,16 @@ func TestTreeDeleteStaticAndWildcardPaths(t *testing.T) { for i := len(paths) - 1; i >= 0; i-- { tbdPath := paths[i] - err := tree.Delete(tbdPath, testMatcher[int](true)) + err := tree.Delete(tbdPath, deleteMatcher[int](true)) require.NoErrorf(t, err, "Should be able to delete %s", paths[i]) - err = tree.Delete(tbdPath, testMatcher[int](true)) + err = tree.Delete(tbdPath, deleteMatcher[int](true)) require.Errorf(t, err, "Should not be able to delete %s", paths[i]) deletedPaths = append(deletedPaths, tbdPath) for idx, path := range paths { - entry, err := tree.Find(path, testMatcher[int](true)) + entry, err := tree.Find(path, lookupMatcher[int](true)) if slices.Contains(deletedPaths, path) { require.Errorf(t, err, "Should not be able to find %s after deleting %s", path, tbdPath) @@ -370,10 +385,10 @@ func TestTreeDeleteMixedPaths(t *testing.T) { for i := len(paths) - 1; i >= 0; i-- { tbdPath := paths[i] - err := tree.Delete(tbdPath, testMatcher[int](true)) + err := tree.Delete(tbdPath, deleteMatcher[int](true)) require.NoErrorf(t, err, "Should be able to delete %s", paths[i]) - err = tree.Delete(tbdPath, testMatcher[int](true)) + err = tree.Delete(tbdPath, deleteMatcher[int](true)) require.Errorf(t, err, "Should not be able to delete %s", paths[i]) } @@ -401,7 +416,8 @@ func TestTreeClone(t *testing.T) { clone := tree.Clone() for _, path := range maps.Values(paths) { - entry, err := clone.Find(path, MatcherFunc[string](func(_ string) bool { return true })) + entry, err := clone.Find(path, + LookupMatcherFunc[string](func(_ string, _, _ []string) bool { return true })) require.NoError(t, err) assert.Equal(t, path, entry.Value) From b0860e885001e0c7a359efba3ffc4417718e48e9 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:48:22 +0200 Subject: [PATCH 03/32] obsolete request matcher mock removed --- .../rules/config/mocks/request_matcher.go | 81 ------------------- 1 file changed, 81 deletions(-) delete mode 100644 internal/rules/config/mocks/request_matcher.go diff --git a/internal/rules/config/mocks/request_matcher.go b/internal/rules/config/mocks/request_matcher.go deleted file mode 100644 index c8360085f..000000000 --- a/internal/rules/config/mocks/request_matcher.go +++ /dev/null @@ -1,81 +0,0 @@ -// Code generated by mockery v2.42.1. DO NOT EDIT. - -package mocks - -import ( - heimdall "github.com/dadrus/heimdall/internal/heimdall" - mock "github.com/stretchr/testify/mock" -) - -// RequestMatcherMock is an autogenerated mock type for the RequestMatcher type -type RequestMatcherMock struct { - mock.Mock -} - -type RequestMatcherMock_Expecter struct { - mock *mock.Mock -} - -func (_m *RequestMatcherMock) EXPECT() *RequestMatcherMock_Expecter { - return &RequestMatcherMock_Expecter{mock: &_m.Mock} -} - -// Matches provides a mock function with given fields: request -func (_m *RequestMatcherMock) Matches(request *heimdall.Request) error { - ret := _m.Called(request) - - if len(ret) == 0 { - panic("no return value specified for Matches") - } - - var r0 error - if rf, ok := ret.Get(0).(func(*heimdall.Request) error); ok { - r0 = rf(request) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// RequestMatcherMock_Matches_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Matches' -type RequestMatcherMock_Matches_Call struct { - *mock.Call -} - -// Matches is a helper method to define mock.On call -// - request *heimdall.Request -func (_e *RequestMatcherMock_Expecter) Matches(request interface{}) *RequestMatcherMock_Matches_Call { - return &RequestMatcherMock_Matches_Call{Call: _e.mock.On("Matches", request)} -} - -func (_c *RequestMatcherMock_Matches_Call) Run(run func(request *heimdall.Request)) *RequestMatcherMock_Matches_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(*heimdall.Request)) - }) - return _c -} - -func (_c *RequestMatcherMock_Matches_Call) Return(_a0 error) *RequestMatcherMock_Matches_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *RequestMatcherMock_Matches_Call) RunAndReturn(run func(*heimdall.Request) error) *RequestMatcherMock_Matches_Call { - _c.Call.Return(run) - return _c -} - -// NewRequestMatcherMock creates a new instance of RequestMatcherMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewRequestMatcherMock(t interface { - mock.TestingT - Cleanup(func()) -}) *RequestMatcherMock { - mock := &RequestMatcherMock{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} From 142ca1735a2cc9733474d47e4c284f8ae9779e8f Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:49:04 +0200 Subject: [PATCH 04/32] license header added to encoded_slash_handling file --- internal/rules/config/encoded_slash_handling.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/rules/config/encoded_slash_handling.go b/internal/rules/config/encoded_slash_handling.go index d7d158c12..ed7f138e6 100644 --- a/internal/rules/config/encoded_slash_handling.go +++ b/internal/rules/config/encoded_slash_handling.go @@ -1,3 +1,19 @@ +// Copyright 2023 Dimitrij Drus +// +// 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. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + package config type EncodedSlashesHandling string From 9ac87f29ae9d6aa600e2c20bf5bdff4f4245b674 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:50:13 +0200 Subject: [PATCH 05/32] rule configuration updated to implement the new API --- internal/rules/config/matcher.go | 48 +++++++-- internal/rules/config/matcher_test.go | 87 ++++++++++------ internal/rules/config/parser_test.go | 142 +++++++++++--------------- internal/rules/config/rule_test.go | 83 ++++++++------- 4 files changed, 200 insertions(+), 160 deletions(-) diff --git a/internal/rules/config/matcher.go b/internal/rules/config/matcher.go index 2121525e6..4c042511d 100644 --- a/internal/rules/config/matcher.go +++ b/internal/rules/config/matcher.go @@ -16,18 +16,52 @@ package config +import "slices" + type Matcher struct { - Path string `json:"path" yaml:"path" validate:"required"` //nolint:lll,tagalign - BacktrackingEnabled *bool `json:"backtracking_enabled" yaml:"backtracking_enabled" validate:"excluded_without=With"` //nolint:lll,tagalign - With *MatcherConstraints `json:"with" yaml:"with" validate:"omitnil,required"` //nolint:lll,tagalign + Routes []Route `json:"routes" yaml:"routes" validate:"required,dive"` //nolint:lll + BacktrackingEnabled *bool `json:"backtracking_enabled" yaml:"backtracking_enabled"` //nolint:lll + Scheme string `json:"scheme" yaml:"scheme" validate:"omitempty,oneof=http https"` //nolint:lll + Methods []string `json:"methods" yaml:"methods" validate:"omitempty,dive,required"` //nolint:lll + Hosts []HostMatcher `json:"hosts" yaml:"hosts" validate:"omitempty,dive,required"` //nolint:lll +} + +type Route struct { + Path string `json:"path" yaml:"path" validate:"required"` + PathParams []ParameterMatcher `json:"path_params" yaml:"path_params" validate:"omitempty,dive,required"` +} + +func (r *Route) DeepCopyInto(out *Route) { + *out = *r + + out.PathParams = slices.Clone(r.PathParams) +} + +type ParameterMatcher struct { + Name string `json:"name" yaml:"name" validate:"required"` + Value string `json:"value" yaml:"value" validate:"required"` + Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` +} + +type HostMatcher struct { + Value string `json:"value" yaml:"value" validate:"required"` + Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` } func (m *Matcher) DeepCopyInto(out *Matcher) { - *out = *m + var withBacktracking *bool + if m.BacktrackingEnabled != nil { + value := *m.BacktrackingEnabled + withBacktracking = &value + } - if m.With != nil { - in, out := m.With, out.With + out.Scheme = m.Scheme + out.BacktrackingEnabled = withBacktracking + out.Methods = slices.Clone(m.Methods) + out.Hosts = slices.Clone(m.Hosts) - in.DeepCopyInto(out) + out.Routes = make([]Route, len(m.Routes)) + for i, route := range m.Routes { + route.DeepCopyInto(&out.Routes[i]) } } diff --git a/internal/rules/config/matcher_test.go b/internal/rules/config/matcher_test.go index d8851a261..a2b0c7f2c 100644 --- a/internal/rules/config/matcher_test.go +++ b/internal/rules/config/matcher_test.go @@ -1,51 +1,80 @@ +// Copyright 2022 Dimitrij Drus +// +// 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. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + package config import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestMatcherDeepCopyInto(t *testing.T) { t.Parallel() + trueValue := true + for _, tc := range []struct { - uc string - in *Matcher - assert func(t *testing.T, out *Matcher) + uc string + in *Matcher }{ { - uc: "with path only", - in: &Matcher{Path: "/foo/bar"}, - assert: func(t *testing.T, out *Matcher) { - t.Helper() - - assert.Equal(t, "/foo/bar", out.Path) - assert.Nil(t, out.With) + uc: "single route defining only a path", + in: &Matcher{ + Routes: []Route{{Path: "/foo/bar"}}, }, }, { - uc: "with path and simple constraints", - in: &Matcher{Path: "/foo/bar", With: &MatcherConstraints{Scheme: "http"}}, - assert: func(t *testing.T, out *Matcher) { - t.Helper() - - assert.Equal(t, "/foo/bar", out.Path) - require.NotNil(t, out.With) - assert.Equal(t, "http", out.With.Scheme) + uc: "single route defining path and some path parameters", + in: &Matcher{ + Routes: []Route{ + { + Path: "/:foo/:bar", + PathParams: []ParameterMatcher{ + {Name: "foo", Value: "bar", Type: "glob"}, + {Name: "bar", Value: "baz", Type: "regex"}, + }, + }, + }, }, }, { - uc: "with path and complex constraints", - in: &Matcher{Path: "/foo/bar", With: &MatcherConstraints{Methods: []string{"GET"}, Scheme: "http"}}, - assert: func(t *testing.T, out *Matcher) { - t.Helper() - - assert.Equal(t, "/foo/bar", out.Path) - require.NotNil(t, out.With) - assert.Equal(t, "http", out.With.Scheme) - assert.ElementsMatch(t, out.With.Methods, []string{"GET"}) + uc: "multiple routes and additional constraints", + in: &Matcher{ + Routes: []Route{ + { + Path: "/:foo/:bar", + PathParams: []ParameterMatcher{ + {Name: "foo", Value: "bar", Type: "glob"}, + {Name: "bar", Value: "baz", Type: "regex"}, + }, + }, + { + Path: "/some/static/path", + }, + }, + BacktrackingEnabled: &trueValue, + Scheme: "https", + Hosts: []HostMatcher{ + { + Value: "*example.com", + Type: "glob", + }, + }, + Methods: []string{"GET", "POST"}, }, }, } { @@ -54,7 +83,7 @@ func TestMatcherDeepCopyInto(t *testing.T) { tc.in.DeepCopyInto(out) - tc.assert(t, out) + assert.Equal(t, tc.in, out) }) } } diff --git a/internal/rules/config/parser_test.go b/internal/rules/config/parser_test.go index 4e111f820..d428405db 100644 --- a/internal/rules/config/parser_test.go +++ b/internal/rules/config/parser_test.go @@ -110,60 +110,20 @@ func TestParseRules(t *testing.T) { content: []byte(`{ "version": "1", "name": "foo", -"rules": [{"id": "foo", "match":{"with": {"host_glob":"**"}}, "execute": [{"authenticator":"test"}]}] -}`), - assert: func(t *testing.T, err error, ruleSet *RuleSet) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.Contains(t, err.Error(), "'rules'[0].'match'.'path' is a required field") - require.Nil(t, ruleSet) - }, - }, - { - uc: "JSON rule set with a rule which match definition contains conflicting fields for host matching", - contentType: "application/json", - content: []byte(`{ -"version": "1", -"name": "foo", "rules": [ { "id": "foo", - "match":{"path":"/foo/bar", "with": {"host_glob":"**", "host_regex":"**"}}, - "execute": [{"authenticator":"test"}] - }] + "match": { + "hosts":[{ "value": "*", "type": "glob" }] + }, + "execute": [{"authenticator":"test"}]}] }`), assert: func(t *testing.T, err error, ruleSet *RuleSet) { t.Helper() require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.Contains(t, err.Error(), "'rules'[0].'match'.'with'.'host_glob' is an excluded field") - require.Contains(t, err.Error(), "'rules'[0].'match'.'with'.'host_regex' is an excluded field") - require.Nil(t, ruleSet) - }, - }, - { - uc: "JSON rule set with a rule which match definition contains conflicting fields for path matching", - contentType: "application/json", - content: []byte(`{ -"version": "1", -"name": "foo", -"rules": [ - { - "id": "foo", - "match":{"path":"/foo/bar", "with": {"path_glob":"**", "path_regex":"**"}}, - "execute": [{"authenticator":"test"}] - }] -}`), - assert: func(t *testing.T, err error, ruleSet *RuleSet) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.Contains(t, err.Error(), "'rules'[0].'match'.'with'.'path_glob' is an excluded field") - require.Contains(t, err.Error(), "'rules'[0].'match'.'with'.'path_regex' is an excluded field") + require.Contains(t, err.Error(), "'rules'[0].'match'.'routes' is a required field") require.Nil(t, ruleSet) }, }, @@ -176,7 +136,11 @@ func TestParseRules(t *testing.T) { "rules": [ { "id": "foo", - "match":{"path":"/foo/bar", "with": {"scheme":"foo", "methods":["ALL"]}}, + "match":{ + "routes": [{ "path":"/foo/bar" }], + "scheme":"foo", + "methods":["ALL"] + }, "execute": [{"authenticator":"test"}] }] }`), @@ -185,7 +149,7 @@ func TestParseRules(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.Contains(t, err.Error(), "'rules'[0].'match'.'with'.'scheme' must be one of [http https]") + require.Contains(t, err.Error(), "'rules'[0].'match'.'scheme' must be one of [http https]") require.Nil(t, ruleSet) }, }, @@ -198,7 +162,9 @@ func TestParseRules(t *testing.T) { "rules": [ { "id": "foo", - "match":{"path":"/foo/bar"}, + "match":{ + "routes": [{ "path":"/foo/bar" }] + }, "execute": [{"authenticator":"test"}], "forward_to": { "rewrite": {"scheme": "http"}} }] @@ -221,7 +187,9 @@ func TestParseRules(t *testing.T) { "rules": [ { "id": "foo", - "match":{"path":"/foo/bar"}, + "match":{ + "routes": [{ "path":"/foo/bar" }] + }, "allow_encoded_slashes": "foo", "execute": [{"authenticator":"test"}] }] @@ -234,27 +202,6 @@ func TestParseRules(t *testing.T) { require.Nil(t, ruleSet) }, }, - { - uc: "JSON rule set with invalid backtracking_enabled settings", - contentType: "application/json", - content: []byte(`{ -"version": "1", -"name": "foo", -"rules": [ - { - "id": "foo", - "match":{"path":"/foo/bar", "backtracking_enabled": true }, - "execute": [{"authenticator":"test"}] - }] -}`), - assert: func(t *testing.T, err error, ruleSet *RuleSet) { - t.Helper() - - require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.Contains(t, err.Error(), "'backtracking_enabled' is an excluded field") - require.Nil(t, ruleSet) - }, - }, { uc: "Valid JSON rule set", contentType: "application/json", @@ -264,7 +211,11 @@ func TestParseRules(t *testing.T) { "rules": [ { "id": "foo", - "match":{"path":"/foo/bar", "with": { "methods": ["ALL"] }, "backtracking_enabled": true }, + "match":{ + "routes": [{ "path":"/foo/bar" }], + "methods": ["ALL"], + "backtracking_enabled": true + }, "execute": [{"authenticator":"test"}] }] }`), @@ -280,8 +231,10 @@ func TestParseRules(t *testing.T) { rul := ruleSet.Rules[0] require.NotNil(t, rul) assert.Equal(t, "foo", rul.ID) - assert.Equal(t, "/foo/bar", rul.Matcher.Path) - assert.ElementsMatch(t, []string{"ALL"}, rul.Matcher.With.Methods) + assert.Len(t, rul.Matcher.Routes, 1) + assert.Equal(t, "/foo/bar", rul.Matcher.Routes[0].Path) + assert.ElementsMatch(t, []string{"ALL"}, rul.Matcher.Methods) + assert.True(t, *rul.Matcher.BacktrackingEnabled) assert.Len(t, rul.Execute, 1) assert.Equal(t, "test", rul.Execute[0]["authenticator"]) }, @@ -295,10 +248,10 @@ name: foo rules: - id: bar match: - path: /foo/bar - with: - methods: - - GET + routes: + - path: /foo/bar + methods: + - GET forward_to: host: test allow_encoded_slashes: no_decode @@ -316,8 +269,10 @@ rules: rul := ruleSet.Rules[0] require.NotNil(t, rul) assert.Equal(t, "bar", rul.ID) - assert.Equal(t, "/foo/bar", rul.Matcher.Path) - assert.ElementsMatch(t, []string{"GET"}, rul.Matcher.With.Methods) + assert.Len(t, rul.Matcher.Routes, 1) + assert.Equal(t, "/foo/bar", rul.Matcher.Routes[0].Path) + assert.ElementsMatch(t, []string{"GET"}, rul.Matcher.Methods) + assert.Equal(t, "test", rul.Backend.Host) assert.Equal(t, EncodedSlashesOnNoDecode, rul.EncodedSlashesHandling) assert.Len(t, rul.Execute, 1) assert.Equal(t, "test", rul.Execute[0]["authenticator"]) @@ -395,7 +350,8 @@ name: foo rules: - id: bar match: - path: foo + routes: + - path: foo execute: - authenticator: test `), @@ -411,7 +367,8 @@ rules: rul := ruleSet.Rules[0] require.NotNil(t, rul) assert.Equal(t, "bar", rul.ID) - assert.Equal(t, "foo", rul.Matcher.Path) + assert.Len(t, rul.Matcher.Routes, 1) + assert.Equal(t, "foo", rul.Matcher.Routes[0].Path) }, }, { @@ -441,7 +398,14 @@ name: ${FOO} rules: - id: bar match: - path: foo + routes: + - path: /foo/:bar + path_params: + - name: bar + type: glob + value: "[a-z]" + methods: + - GET execute: - authenticator: test `), @@ -457,7 +421,13 @@ rules: rul := ruleSet.Rules[0] require.NotNil(t, rul) assert.Equal(t, "bar", rul.ID) - assert.Equal(t, "foo", rul.Matcher.Path) + assert.Len(t, rul.Matcher.Routes, 1) + assert.Equal(t, "/foo/:bar", rul.Matcher.Routes[0].Path) + assert.Len(t, rul.Matcher.Routes[0].PathParams, 1) + assert.Equal(t, "bar", rul.Matcher.Routes[0].PathParams[0].Name) + assert.Equal(t, "glob", rul.Matcher.Routes[0].PathParams[0].Type) + assert.Equal(t, "[a-z]", rul.Matcher.Routes[0].PathParams[0].Value) + assert.Equal(t, "GET", rul.Matcher.Methods[0]) }, }, { @@ -468,7 +438,8 @@ name: ${FOO} rules: - id: bar match: - path: foo + routes: + - path: foo execute: - authenticator: test `), @@ -484,7 +455,8 @@ rules: rul := ruleSet.Rules[0] require.NotNil(t, rul) assert.Equal(t, "bar", rul.ID) - assert.Equal(t, "foo", rul.Matcher.Path) + assert.Len(t, rul.Matcher.Routes, 1) + assert.Equal(t, "foo", rul.Matcher.Routes[0].Path) }, }, } { diff --git a/internal/rules/config/rule_test.go b/internal/rules/config/rule_test.go index e97f955a2..c51a03ea8 100644 --- a/internal/rules/config/rule_test.go +++ b/internal/rules/config/rule_test.go @@ -28,21 +28,35 @@ import ( func TestRuleConfigDeepCopyInto(t *testing.T) { t.Parallel() + trueValue := true + // GIVEN var out Rule in := Rule{ ID: "foo", Matcher: Matcher{ - Path: "bar", - With: &MatcherConstraints{ - Methods: []string{"GET", "PATCH"}, - Scheme: "https", - HostGlob: "**.example.com", - HostRegex: ".*\\.example.com", - PathGlob: "**.css", - PathRegex: ".*\\.css", + Routes: []Route{ + { + Path: "/:foo/*something", + PathParams: []ParameterMatcher{ + {Name: "foo", Value: "bar", Type: "glob"}, + {Name: "something", Value: ".*\\.css", Type: "regex"}, + }, + }, + { + Path: "/some/static/path", + }, + }, + BacktrackingEnabled: &trueValue, + Scheme: "https", + Hosts: []HostMatcher{ + { + Value: "**.example.com", + Type: "glob", + }, }, + Methods: []string{"GET", "PATCH"}, }, Backend: &Backend{ Host: "baz", @@ -61,35 +75,36 @@ func TestRuleConfigDeepCopyInto(t *testing.T) { in.DeepCopyInto(&out) // THEN - assert.Equal(t, in.ID, out.ID) - assert.Equal(t, in.Matcher.Path, out.Matcher.Path) - assert.Equal(t, in.Matcher.With.Methods, out.Matcher.With.Methods) - assert.Equal(t, in.Matcher.With.Scheme, out.Matcher.With.Scheme) - assert.Equal(t, in.Matcher.With.HostGlob, out.Matcher.With.HostGlob) - assert.Equal(t, in.Matcher.With.HostRegex, out.Matcher.With.HostRegex) - assert.Equal(t, in.Matcher.With.PathGlob, out.Matcher.With.PathGlob) - assert.Equal(t, in.Matcher.With.PathRegex, out.Matcher.With.PathRegex) - assert.Equal(t, in.Backend, out.Backend) - assert.Equal(t, in.Execute, out.Execute) - assert.Equal(t, in.ErrorHandler, out.ErrorHandler) + assert.Equal(t, in, out) } func TestRuleConfigDeepCopy(t *testing.T) { t.Parallel() // GIVEN - in := Rule{ + in := &Rule{ ID: "foo", Matcher: Matcher{ - Path: "bar", - With: &MatcherConstraints{ - Methods: []string{"GET", "PATCH"}, - Scheme: "https", - HostGlob: "**.example.com", - HostRegex: ".*\\.example.com", - PathGlob: "**.css", - PathRegex: ".*\\.css", + Routes: []Route{ + { + Path: "/:foo/*something", + PathParams: []ParameterMatcher{ + {Name: "foo", Value: "bar", Type: "glob"}, + {Name: "something", Value: ".*\\.css", Type: "regex"}, + }, + }, + { + Path: "/some/static/path", + }, + }, + Scheme: "https", + Hosts: []HostMatcher{ + { + Value: "**.example.com", + Type: "glob", + }, }, + Methods: []string{"GET", "PATCH"}, }, Backend: &Backend{ Host: "baz", @@ -112,15 +127,5 @@ func TestRuleConfigDeepCopy(t *testing.T) { require.NotSame(t, &in, out) // but same contents - assert.Equal(t, in.ID, out.ID) - assert.Equal(t, in.Matcher.Path, out.Matcher.Path) - assert.Equal(t, in.Matcher.With.Methods, out.Matcher.With.Methods) - assert.Equal(t, in.Matcher.With.Scheme, out.Matcher.With.Scheme) - assert.Equal(t, in.Matcher.With.HostGlob, out.Matcher.With.HostGlob) - assert.Equal(t, in.Matcher.With.HostRegex, out.Matcher.With.HostRegex) - assert.Equal(t, in.Matcher.With.PathGlob, out.Matcher.With.PathGlob) - assert.Equal(t, in.Matcher.With.PathRegex, out.Matcher.With.PathRegex) - assert.Equal(t, in.Backend, out.Backend) - assert.Equal(t, in.Execute, out.Execute) - assert.Equal(t, in.ErrorHandler, out.ErrorHandler) + assert.Equal(t, in, out) } From bd65ea6af1287cf5c843d9fdd08f61bf46fea5ee Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:52:49 +0200 Subject: [PATCH 06/32] rule interface updated, new route interface added --- internal/rules/rule/mocks/rule.go | 144 +++++++++++++++--------------- internal/rules/rule/route.go | 9 ++ internal/rules/rule/rule.go | 6 +- 3 files changed, 85 insertions(+), 74 deletions(-) create mode 100644 internal/rules/rule/route.go diff --git a/internal/rules/rule/mocks/rule.go b/internal/rules/rule/mocks/rule.go index 354cbfd71..153234324 100644 --- a/internal/rules/rule/mocks/rule.go +++ b/internal/rules/rule/mocks/rule.go @@ -22,12 +22,12 @@ func (_m *RuleMock) EXPECT() *RuleMock_Expecter { return &RuleMock_Expecter{mock: &_m.Mock} } -// BacktrackingEnabled provides a mock function with given fields: -func (_m *RuleMock) BacktrackingEnabled() bool { +// AllowsBacktracking provides a mock function with given fields: +func (_m *RuleMock) AllowsBacktracking() bool { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for BacktrackingEnabled") + panic("no return value specified for AllowsBacktracking") } var r0 bool @@ -40,29 +40,75 @@ func (_m *RuleMock) BacktrackingEnabled() bool { return r0 } -// RuleMock_BacktrackingEnabled_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'BacktrackingEnabled' -type RuleMock_BacktrackingEnabled_Call struct { +// RuleMock_AllowsBacktracking_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AllowsBacktracking' +type RuleMock_AllowsBacktracking_Call struct { *mock.Call } -// BacktrackingEnabled is a helper method to define mock.On call -func (_e *RuleMock_Expecter) BacktrackingEnabled() *RuleMock_BacktrackingEnabled_Call { - return &RuleMock_BacktrackingEnabled_Call{Call: _e.mock.On("BacktrackingEnabled")} +// AllowsBacktracking is a helper method to define mock.On call +func (_e *RuleMock_Expecter) AllowsBacktracking() *RuleMock_AllowsBacktracking_Call { + return &RuleMock_AllowsBacktracking_Call{Call: _e.mock.On("AllowsBacktracking")} } -func (_c *RuleMock_BacktrackingEnabled_Call) Run(run func()) *RuleMock_BacktrackingEnabled_Call { +func (_c *RuleMock_AllowsBacktracking_Call) Run(run func()) *RuleMock_AllowsBacktracking_Call { _c.Call.Run(func(args mock.Arguments) { run() }) return _c } -func (_c *RuleMock_BacktrackingEnabled_Call) Return(_a0 bool) *RuleMock_BacktrackingEnabled_Call { +func (_c *RuleMock_AllowsBacktracking_Call) Return(_a0 bool) *RuleMock_AllowsBacktracking_Call { _c.Call.Return(_a0) return _c } -func (_c *RuleMock_BacktrackingEnabled_Call) RunAndReturn(run func() bool) *RuleMock_BacktrackingEnabled_Call { +func (_c *RuleMock_AllowsBacktracking_Call) RunAndReturn(run func() bool) *RuleMock_AllowsBacktracking_Call { + _c.Call.Return(run) + return _c +} + +// EqualTo provides a mock function with given fields: other +func (_m *RuleMock) EqualTo(other rule.Rule) bool { + ret := _m.Called(other) + + if len(ret) == 0 { + panic("no return value specified for EqualTo") + } + + var r0 bool + if rf, ok := ret.Get(0).(func(rule.Rule) bool); ok { + r0 = rf(other) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// RuleMock_EqualTo_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'EqualTo' +type RuleMock_EqualTo_Call struct { + *mock.Call +} + +// EqualTo is a helper method to define mock.On call +// - other rule.Rule +func (_e *RuleMock_Expecter) EqualTo(other interface{}) *RuleMock_EqualTo_Call { + return &RuleMock_EqualTo_Call{Call: _e.mock.On("EqualTo", other)} +} + +func (_c *RuleMock_EqualTo_Call) Run(run func(other rule.Rule)) *RuleMock_EqualTo_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(rule.Rule)) + }) + return _c +} + +func (_c *RuleMock_EqualTo_Call) Return(_a0 bool) *RuleMock_EqualTo_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *RuleMock_EqualTo_Call) RunAndReturn(run func(rule.Rule) bool) *RuleMock_EqualTo_Call { _c.Call.Return(run) return _c } @@ -170,93 +216,49 @@ func (_c *RuleMock_ID_Call) RunAndReturn(run func() string) *RuleMock_ID_Call { return _c } -// Matches provides a mock function with given fields: ctx -func (_m *RuleMock) Matches(ctx heimdall.Context) bool { - ret := _m.Called(ctx) - - if len(ret) == 0 { - panic("no return value specified for Matches") - } - - var r0 bool - if rf, ok := ret.Get(0).(func(heimdall.Context) bool); ok { - r0 = rf(ctx) - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// RuleMock_Matches_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Matches' -type RuleMock_Matches_Call struct { - *mock.Call -} - -// Matches is a helper method to define mock.On call -// - ctx heimdall.Context -func (_e *RuleMock_Expecter) Matches(ctx interface{}) *RuleMock_Matches_Call { - return &RuleMock_Matches_Call{Call: _e.mock.On("Matches", ctx)} -} - -func (_c *RuleMock_Matches_Call) Run(run func(ctx heimdall.Context)) *RuleMock_Matches_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(heimdall.Context)) - }) - return _c -} - -func (_c *RuleMock_Matches_Call) Return(_a0 bool) *RuleMock_Matches_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *RuleMock_Matches_Call) RunAndReturn(run func(heimdall.Context) bool) *RuleMock_Matches_Call { - _c.Call.Return(run) - return _c -} - -// PathExpression provides a mock function with given fields: -func (_m *RuleMock) PathExpression() string { +// Routes provides a mock function with given fields: +func (_m *RuleMock) Routes() []rule.Route { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for PathExpression") + panic("no return value specified for Routes") } - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { + var r0 []rule.Route + if rf, ok := ret.Get(0).(func() []rule.Route); ok { r0 = rf() } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).([]rule.Route) + } } return r0 } -// RuleMock_PathExpression_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'PathExpression' -type RuleMock_PathExpression_Call struct { +// RuleMock_Routes_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Routes' +type RuleMock_Routes_Call struct { *mock.Call } -// PathExpression is a helper method to define mock.On call -func (_e *RuleMock_Expecter) PathExpression() *RuleMock_PathExpression_Call { - return &RuleMock_PathExpression_Call{Call: _e.mock.On("PathExpression")} +// Routes is a helper method to define mock.On call +func (_e *RuleMock_Expecter) Routes() *RuleMock_Routes_Call { + return &RuleMock_Routes_Call{Call: _e.mock.On("Routes")} } -func (_c *RuleMock_PathExpression_Call) Run(run func()) *RuleMock_PathExpression_Call { +func (_c *RuleMock_Routes_Call) Run(run func()) *RuleMock_Routes_Call { _c.Call.Run(func(args mock.Arguments) { run() }) return _c } -func (_c *RuleMock_PathExpression_Call) Return(_a0 string) *RuleMock_PathExpression_Call { +func (_c *RuleMock_Routes_Call) Return(_a0 []rule.Route) *RuleMock_Routes_Call { _c.Call.Return(_a0) return _c } -func (_c *RuleMock_PathExpression_Call) RunAndReturn(run func() string) *RuleMock_PathExpression_Call { +func (_c *RuleMock_Routes_Call) RunAndReturn(run func() []rule.Route) *RuleMock_Routes_Call { _c.Call.Return(run) return _c } diff --git a/internal/rules/rule/route.go b/internal/rules/rule/route.go new file mode 100644 index 000000000..5512bf463 --- /dev/null +++ b/internal/rules/rule/route.go @@ -0,0 +1,9 @@ +package rule + +import "github.com/dadrus/heimdall/internal/heimdall" + +type Route interface { + Path() string + Matches(ctx heimdall.Context, keys, values []string) bool + Rule() Rule +} diff --git a/internal/rules/rule/rule.go b/internal/rules/rule/rule.go index e7539c431..fe59916de 100644 --- a/internal/rules/rule/rule.go +++ b/internal/rules/rule/rule.go @@ -26,8 +26,8 @@ type Rule interface { ID() string SrcID() string Execute(ctx heimdall.Context) (Backend, error) - Matches(ctx heimdall.Context) bool - PathExpression() string - BacktrackingEnabled() bool + Routes() []Route SameAs(other Rule) bool + EqualTo(other Rule) bool + AllowsBacktracking() bool } From 93f15d7eb039b43f892a101549b98ef79e237506 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:54:37 +0200 Subject: [PATCH 07/32] basis matchers for the new route based matching functionality --- internal/rules/route_matcher.go | 93 ++++---- internal/rules/route_matcher_test.go | 316 +++++++++++++++++++-------- internal/rules/typed_matcher.go | 24 +- internal/rules/typed_matcher_test.go | 40 +++- 4 files changed, 337 insertions(+), 136 deletions(-) diff --git a/internal/rules/route_matcher.go b/internal/rules/route_matcher.go index 558a68644..6e721a47a 100644 --- a/internal/rules/route_matcher.go +++ b/internal/rules/route_matcher.go @@ -1,14 +1,29 @@ +// Copyright 2024 Dimitrij Drus +// +// 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. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + package rules import ( "errors" - "github.com/dadrus/heimdall/internal/rules/config" "net/http" - "net/url" "slices" "strings" "github.com/dadrus/heimdall/internal/heimdall" + "github.com/dadrus/heimdall/internal/rules/config" "github.com/dadrus/heimdall/internal/x/errorchain" "github.com/dadrus/heimdall/internal/x/slicex" ) @@ -23,8 +38,6 @@ var ( ErrRequestPathMismatch = errors.New("request path mismatch") ) -//go:generate mockery --name RouteMatcher --structname RouteMatcherMock - type RouteMatcher interface { Matches(request *heimdall.Request, keys, values []string) error } @@ -41,10 +54,6 @@ func (c compositeMatcher) Matches(request *heimdall.Request, keys, values []stri return nil } -type alwaysMatcher struct{} - -func (alwaysMatcher) match(_ string) bool { return true } - type schemeMatcher string func (s schemeMatcher) Matches(request *heimdall.Request, _, _ []string) error { @@ -81,43 +90,31 @@ func (m *hostMatcher) Matches(request *heimdall.Request, _, _ []string) error { return nil } -type paramMatcher struct { +type pathParamMatcher struct { typedMatcher - name string + name string + slashHandling config.EncodedSlashesHandling } -func (m *paramMatcher) Matches(_ *heimdall.Request, keys, values []string) error { +func (m *pathParamMatcher) Matches(request *heimdall.Request, keys, values []string) error { idx := slices.Index(keys, m.name) if idx == -1 { - return errorchain.NewWithMessagef(ErrRequestPathMismatch, "%s is not expected", m.name) + return errorchain.NewWithMessagef(ErrRequestPathMismatch, "path parameter %s is not expected", m.name) } value := values[idx] - if !m.match(value) { - return errorchain.NewWithMessagef(ErrRequestPathMismatch, "%s is not expected", value) + // URL.RawPath is set only if the original url contains url encoded parts + if len(request.URL.RawPath) != 0 && + m.slashHandling == config.EncodedSlashesOff && + strings.Contains(request.URL.RawPath, "%2F") { + return errorchain.NewWithMessagef(ErrRequestPathMismatch, + "value for path parameter %s contains encoded slashes which are not allowed", keys[idx]) } - return nil -} - -type pathMatcher struct { - typedMatcher - - slashHandling config.EncodedSlashesHandling -} - -func (m *pathMatcher) Matches(request *heimdall.Request) error { - var path string - if len(request.URL.RawPath) == 0 || m.slashHandling == config.EncodedSlashesOn { - path = request.URL.Path - } else { - unescaped, _ := url.PathUnescape(strings.ReplaceAll(request.URL.RawPath, "%2F", "$$$escaped-slash$$$")) - path = strings.ReplaceAll(unescaped, "$$$escaped-slash$$$", "%2F") - } - - if !m.match(path) { - return errorchain.NewWithMessagef(ErrRequestPathMismatch, "%s is not expected", path) + if !m.match(value) { + return errorchain.NewWithMessagef(ErrRequestPathMismatch, + "captured values for path parameter %s is not expected", value) } return nil @@ -141,7 +138,8 @@ func createMethodMatcher(methods []string) (methodMatcher, error) { methods = slices.Compact(methods) if res := slicex.Filter(methods, func(s string) bool { return len(s) == 0 }); len(res) != 0 { return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "methods list contains empty values. have you forgotten to put the corresponding value into braces?") + "methods list contains empty values. "+ + "have you forgotten to put the corresponding value into braces?") } tbr := slicex.Filter(methods, func(s string) bool { return strings.HasPrefix(s, "!") }) @@ -166,14 +164,15 @@ func createHostMatcher(hosts []config.HostMatcher) (RouteMatcher, error) { case "regex": tm, err = newRegexMatcher(host.Value) case "exact": - matchers[idx] = &hostMatcher{&valueMatcher{host.Value}} + tm, err = newExactMatcher(host.Value) default: - return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "unsupported expression type for host") + return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, + "unsupported host matching expression type '%s' at index %d", host.Type, idx) } if err != nil { - return nil, err + return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, + "failed to compile host matching expression at index %d", idx).CausedBy(err) } matchers[idx] = &hostMatcher{tm} @@ -182,7 +181,7 @@ func createHostMatcher(hosts []config.HostMatcher) (RouteMatcher, error) { return matchers, nil } -func createParamsMatcher(params []config.ParameterMatcher) (RouteMatcher, error) { +func createPathParamsMatcher(params []config.ParameterMatcher, esh config.EncodedSlashesHandling) (RouteMatcher, error) { matchers := make(compositeMatcher, len(params)) for idx, param := range params { @@ -197,17 +196,21 @@ func createParamsMatcher(params []config.ParameterMatcher) (RouteMatcher, error) case "regex": tm, err = newRegexMatcher(param.Value) case "exact": - matchers[idx] = ¶mMatcher{&valueMatcher{param.Value}, param.Name} + tm, err = newExactMatcher(param.Value) default: - return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "unsupported expression type for parameter") + return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, + "unsupported path parameter expression type '%s' for parameter '%s' at index %d", + param.Type, param.Name, idx) } if err != nil { - return nil, err + return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, + "failed to compile path params matching expression for parameter '%s' at index %d", + param.Name, idx). + CausedBy(err) } - matchers[idx] = ¶mMatcher{tm, param.Name} + matchers[idx] = &pathParamMatcher{tm, param.Name, esh} } return matchers, nil diff --git a/internal/rules/route_matcher_test.go b/internal/rules/route_matcher_test.go index c7b2c9620..42b201521 100644 --- a/internal/rules/route_matcher_test.go +++ b/internal/rules/route_matcher_test.go @@ -1,3 +1,19 @@ +// Copyright 2024 Dimitrij Drus +// +// 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. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + package rules import ( @@ -9,6 +25,7 @@ import ( "github.com/stretchr/testify/require" "github.com/dadrus/heimdall/internal/heimdall" + "github.com/dadrus/heimdall/internal/rules/config" ) func TestCreateMethodMatcher(t *testing.T) { @@ -73,132 +90,212 @@ func TestCreateMethodMatcher(t *testing.T) { } } -func TestCreatePathMatcher(t *testing.T) { +func TestCreateHostMatcher(t *testing.T) { t.Parallel() for _, tc := range []struct { uc string - glob string - regex string - assert func(t *testing.T, mather *pathMatcher, err error) + conf []config.HostMatcher + assert func(t *testing.T, matcher RouteMatcher, err error) }{ { uc: "empty configuration", - assert: func(t *testing.T, mather *pathMatcher, err error) { + assert: func(t *testing.T, matcher RouteMatcher, err error) { t.Helper() require.NoError(t, err) - assert.IsType(t, alwaysMatcher{}, mather.patternMatcher) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Empty(t, matcher) }, }, { uc: "valid glob expression", - glob: "/**", - assert: func(t *testing.T, mather *pathMatcher, err error) { + conf: []config.HostMatcher{{Value: "/**", Type: "glob"}}, + assert: func(t *testing.T, matcher RouteMatcher, err error) { t.Helper() require.NoError(t, err) - assert.IsType(t, &globMatcher{}, mather.patternMatcher) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Len(t, matcher, 1) + + hms := matcher.(compositeMatcher) + assert.IsType(t, &hostMatcher{}, hms[0]) + assert.IsType(t, &globMatcher{}, hms[0].(*hostMatcher).typedMatcher) }, }, { uc: "invalid glob expression", - glob: "!*][)(*", - assert: func(t *testing.T, _ *pathMatcher, err error) { + conf: []config.HostMatcher{{Value: "!*][)(*", Type: "glob"}}, + assert: func(t *testing.T, _ RouteMatcher, err error) { t.Helper() require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "failed to compile host matching expression at index 0") }, }, { - uc: "valid regex expression", - regex: ".*", - assert: func(t *testing.T, mather *pathMatcher, err error) { + uc: "valid regex expression", + conf: []config.HostMatcher{{Value: ".*", Type: "regex"}}, + assert: func(t *testing.T, matcher RouteMatcher, err error) { t.Helper() require.NoError(t, err) - assert.IsType(t, ®expMatcher{}, mather.patternMatcher) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Len(t, matcher, 1) + + hms := matcher.(compositeMatcher) + assert.IsType(t, &hostMatcher{}, hms[0]) + assert.IsType(t, ®expMatcher{}, hms[0].(*hostMatcher).typedMatcher) }, }, { - uc: "invalid regex expression", - regex: "?>?<*??", - assert: func(t *testing.T, _ *pathMatcher, err error) { + uc: "invalid regex expression", + conf: []config.HostMatcher{{Value: "?>?<*??", Type: "regex"}}, + assert: func(t *testing.T, _ RouteMatcher, err error) { t.Helper() require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "failed to compile host matching expression at index 0") + }, + }, + { + uc: "exact expression", + conf: []config.HostMatcher{{Value: "?>?<*??", Type: "exact"}}, + assert: func(t *testing.T, matcher RouteMatcher, err error) { + t.Helper() + + require.NoError(t, err) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Len(t, matcher, 1) + + hms := matcher.(compositeMatcher) + assert.IsType(t, &hostMatcher{}, hms[0]) + assert.IsType(t, &exactMatcher{}, hms[0].(*hostMatcher).typedMatcher) + }, + }, + { + uc: "unsupported type", + conf: []config.HostMatcher{{Value: "foo", Type: "bar"}}, + assert: func(t *testing.T, _ RouteMatcher, err error) { + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "unsupported host matching expression type 'bar' at index 0") }, }, } { t.Run(tc.uc, func(t *testing.T) { - hm, err := createPathMatcher(tc.glob, tc.regex, EncodedSlashesOnNoDecode) + hm, err := createHostMatcher(tc.conf) tc.assert(t, hm, err) }) } } -func TestCreateHostMatcher(t *testing.T) { +func TestCreatePathParamsMatcher(t *testing.T) { t.Parallel() for _, tc := range []struct { uc string - glob string - regex string - assert func(t *testing.T, mather *hostMatcher, err error) + conf []config.ParameterMatcher + assert func(t *testing.T, matcher RouteMatcher, err error) }{ { uc: "empty configuration", - assert: func(t *testing.T, mather *hostMatcher, err error) { + assert: func(t *testing.T, matcher RouteMatcher, err error) { t.Helper() require.NoError(t, err) - assert.IsType(t, alwaysMatcher{}, mather.patternMatcher) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Empty(t, matcher) }, }, { uc: "valid glob expression", - glob: "/**", - assert: func(t *testing.T, mather *hostMatcher, err error) { + conf: []config.ParameterMatcher{{Name: "foo", Value: "/**", Type: "glob"}}, + assert: func(t *testing.T, matcher RouteMatcher, err error) { t.Helper() require.NoError(t, err) - assert.IsType(t, &globMatcher{}, mather.patternMatcher) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Len(t, matcher, 1) + + hms := matcher.(compositeMatcher) + assert.IsType(t, &pathParamMatcher{}, hms[0]) + assert.IsType(t, &globMatcher{}, hms[0].(*pathParamMatcher).typedMatcher) }, }, { uc: "invalid glob expression", - glob: "!*][)(*", - assert: func(t *testing.T, _ *hostMatcher, err error) { + conf: []config.ParameterMatcher{{Name: "foo", Value: "!*][)(*", Type: "glob"}}, + assert: func(t *testing.T, _ RouteMatcher, err error) { + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "failed to compile path params matching expression for parameter 'foo' at index 0") + }, + }, + { + uc: "valid regex expression", + conf: []config.ParameterMatcher{{Name: "foo", Value: ".*", Type: "regex"}}, + assert: func(t *testing.T, matcher RouteMatcher, err error) { + t.Helper() + + require.NoError(t, err) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Len(t, matcher, 1) + + hms := matcher.(compositeMatcher) + assert.IsType(t, &pathParamMatcher{}, hms[0]) + assert.IsType(t, ®expMatcher{}, hms[0].(*pathParamMatcher).typedMatcher) + }, + }, + { + uc: "invalid regex expression", + conf: []config.ParameterMatcher{{Name: "foo", Value: "?>?<*??", Type: "regex"}}, + assert: func(t *testing.T, _ RouteMatcher, err error) { t.Helper() require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "failed to compile path params matching expression for parameter 'foo' at index 0") }, }, { - uc: "valid regex expression", - regex: ".*", - assert: func(t *testing.T, mather *hostMatcher, err error) { + uc: "exact expression", + conf: []config.ParameterMatcher{{Name: "foo", Value: "?>?<*??", Type: "exact"}}, + assert: func(t *testing.T, matcher RouteMatcher, err error) { t.Helper() require.NoError(t, err) - assert.IsType(t, ®expMatcher{}, mather.patternMatcher) + assert.IsType(t, compositeMatcher{}, matcher) + assert.Len(t, matcher, 1) + + hms := matcher.(compositeMatcher) + assert.IsType(t, &pathParamMatcher{}, hms[0]) + assert.IsType(t, &exactMatcher{}, hms[0].(*pathParamMatcher).typedMatcher) }, }, { - uc: "invalid regex expression", - regex: "?>?<*??", - assert: func(t *testing.T, _ *hostMatcher, err error) { + uc: "unsupported type", + conf: []config.ParameterMatcher{{Name: "foo", Value: "foo", Type: "bar"}}, + assert: func(t *testing.T, _ RouteMatcher, err error) { t.Helper() require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "unsupported path parameter expression type 'bar' for parameter 'foo' at index 0") }, }, } { t.Run(tc.uc, func(t *testing.T) { - hm, err := createHostMatcher(tc.glob, tc.regex) + pm, err := createPathParamsMatcher(tc.conf, config.EncodedSlashesOff) - tc.assert(t, hm, err) + tc.assert(t, pm, err) }) } } @@ -214,15 +311,20 @@ func TestSchemeMatcherMatches(t *testing.T) { }{ {uc: "matches any schemes", matcher: schemeMatcher(""), toMatch: "foo", matches: true}, {uc: "matches", matcher: schemeMatcher("http"), toMatch: "http", matches: true}, - {uc: "does not match", matcher: schemeMatcher("http"), toMatch: "https", matches: false}, + {uc: "does not match", matcher: schemeMatcher("http"), toMatch: "https"}, } { t.Run(tc.uc, func(t *testing.T) { - err := tc.matcher.Matches(&heimdall.Request{URL: &heimdall.URL{URL: url.URL{Scheme: tc.toMatch}}}) + err := tc.matcher.Matches( + &heimdall.Request{URL: &heimdall.URL{URL: url.URL{Scheme: tc.toMatch}}}, + nil, + nil, + ) if tc.matches { require.NoError(t, err) } else { require.Error(t, err) + require.ErrorIs(t, err, ErrRequestSchemeMismatch) } }) } @@ -239,15 +341,16 @@ func TestMethodMatcherMatches(t *testing.T) { }{ {uc: "matches any methods", matcher: methodMatcher{}, toMatch: "GET", matches: true}, {uc: "matches", matcher: methodMatcher{"GET"}, toMatch: "GET", matches: true}, - {uc: "does not match", matcher: methodMatcher{"GET"}, toMatch: "POST", matches: false}, + {uc: "does not match", matcher: methodMatcher{"GET"}, toMatch: "POST"}, } { t.Run(tc.uc, func(t *testing.T) { - err := tc.matcher.Matches(&heimdall.Request{Method: tc.toMatch}) + err := tc.matcher.Matches(&heimdall.Request{Method: tc.toMatch}, nil, nil) if tc.matches { require.NoError(t, err) } else { require.Error(t, err) + require.ErrorIs(t, err, ErrRequestMethodMismatch) } }) } @@ -257,88 +360,125 @@ func TestHostMatcherMatches(t *testing.T) { t.Parallel() for _, tc := range []struct { - uc string - expression string - toMatch string - matches bool + uc string + conf []config.HostMatcher + toMatch string + matches bool }{ - {uc: "matches any host", expression: "**", toMatch: "foo.example.com", matches: true}, - {uc: "matches", expression: "example.com", toMatch: "example.com", matches: true}, - {uc: "does not match", expression: "example.com", toMatch: "foo.example.com", matches: false}, + {uc: "matches any host", conf: []config.HostMatcher{{Value: "**", Type: "glob"}}, toMatch: "foo.example.com", matches: true}, + {uc: "matches", conf: []config.HostMatcher{{Value: "example.com", Type: "exact"}}, toMatch: "example.com", matches: true}, + {uc: "does not match", conf: []config.HostMatcher{{Value: "^example.com", Type: "regex"}}, toMatch: "foo.example.com"}, } { t.Run(tc.uc, func(t *testing.T) { - hm, err := createHostMatcher(tc.expression, "") + hm, err := createHostMatcher(tc.conf) require.NoError(t, err) - err = hm.Matches(&heimdall.Request{URL: &heimdall.URL{URL: url.URL{Host: tc.toMatch}}}) + err = hm.Matches(&heimdall.Request{URL: &heimdall.URL{URL: url.URL{Host: tc.toMatch}}}, nil, nil) if tc.matches { require.NoError(t, err) } else { require.Error(t, err) + require.ErrorIs(t, err, ErrRequestHostMismatch) } }) } } -func TestPathMatcherMatches(t *testing.T) { +func TestPathParamsMatcherMatches(t *testing.T) { t.Parallel() for _, tc := range []struct { uc string - expression string - slashEncoding EncodedSlashesHandling - toMatch string + conf []config.ParameterMatcher + slashHandling config.EncodedSlashesHandling + toMatch url.URL + keys []string + values []string matches bool }{ { - uc: "matches any path", - slashEncoding: EncodedSlashesOn, - toMatch: "foo.example.com", - matches: true, + uc: "parameter not present in keys", + conf: []config.ParameterMatcher{ + {Name: "foo", Type: "exact", Value: "bar"}, + }, + keys: []string{"bar"}, + values: []string{"baz"}, }, { - uc: "matches path containing encoded slash with slash encoding on", - expression: "/foo/bar/*", - slashEncoding: EncodedSlashesOn, - toMatch: "foo%2Fbar/baz", - matches: true, + uc: "encoded slashes are not allowed", + conf: []config.ParameterMatcher{ + {Name: "foo", Type: "exact", Value: "bar%2Fbaz"}, + }, + slashHandling: config.EncodedSlashesOff, + keys: []string{"foo"}, + values: []string{"bar%2Fbaz"}, + toMatch: func() url.URL { + uri, err := url.Parse("http://example.com/bar%2Fbaz") + require.NoError(t, err) + + return *uri + }(), }, { - uc: "matches path containing encoded slash without slash decoding", - expression: "/foo%2Fbar/*", - slashEncoding: EncodedSlashesOnNoDecode, - toMatch: "foo%2Fbar/baz", - matches: true, + uc: "matches with path having allowed but not decoded encoded slashes", + conf: []config.ParameterMatcher{ + {Name: "foo", Type: "exact", Value: "bar%2Fbaz"}, + }, + slashHandling: config.EncodedSlashesOnNoDecode, + keys: []string{"foo"}, + values: []string{"bar%2Fbaz"}, + toMatch: func() url.URL { + uri, err := url.Parse("http://example.com/bar%2Fbaz") + require.NoError(t, err) + + return *uri + }(), + matches: true, }, { - uc: "does not match path containing encoded slash with slash encoding on", - expression: "foo/bar", - slashEncoding: EncodedSlashesOn, - toMatch: "foo%2Fbar/baz", - matches: false, + uc: "matches with path having allowed encoded slashes", + conf: []config.ParameterMatcher{ + {Name: "foo", Type: "exact", Value: "bar%2Fbaz"}, + }, + slashHandling: config.EncodedSlashesOn, + keys: []string{"foo"}, + values: []string{"bar%2Fbaz"}, + toMatch: func() url.URL { + uri, err := url.Parse("http://example.com/bar%2Fbaz") + require.NoError(t, err) + + return *uri + }(), + matches: true, }, { - uc: "does not match path containing encoded slash without slash encoding", - expression: "foo%2Fbar", - slashEncoding: EncodedSlashesOnNoDecode, - toMatch: "foo%2Fbar/baz", - matches: false, + uc: "doesn't match", + conf: []config.ParameterMatcher{ + {Name: "foo", Type: "exact", Value: "bar"}, + }, + slashHandling: config.EncodedSlashesOn, + keys: []string{"foo"}, + values: []string{"baz"}, + toMatch: func() url.URL { + uri, err := url.Parse("http://example.com/bar") + require.NoError(t, err) + + return *uri + }(), }, } { t.Run(tc.uc, func(t *testing.T) { - hm, err := createPathMatcher(tc.expression, "", tc.slashEncoding) - require.NoError(t, err) - - uri, err := url.Parse("https://example.com/" + tc.toMatch) + hm, err := createPathParamsMatcher(tc.conf, tc.slashHandling) require.NoError(t, err) - err = hm.Matches(&heimdall.Request{URL: &heimdall.URL{URL: *uri}}) + err = hm.Matches(&heimdall.Request{URL: &heimdall.URL{URL: tc.toMatch}}, tc.keys, tc.values) if tc.matches { require.NoError(t, err) } else { require.Error(t, err) + require.ErrorIs(t, err, ErrRequestPathMismatch) } }) } @@ -377,7 +517,11 @@ func TestCompositeMatcherMatches(t *testing.T) { }, } { t.Run(tc.uc, func(t *testing.T) { - err := tc.matcher.Matches(&heimdall.Request{Method: tc.method, URL: &heimdall.URL{URL: url.URL{Scheme: tc.scheme}}}) + err := tc.matcher.Matches( + &heimdall.Request{Method: tc.method, URL: &heimdall.URL{URL: url.URL{Scheme: tc.scheme}}}, + nil, + nil, + ) if tc.matches { require.NoError(t, err) diff --git a/internal/rules/typed_matcher.go b/internal/rules/typed_matcher.go index cfbaf1523..b8b696db7 100644 --- a/internal/rules/typed_matcher.go +++ b/internal/rules/typed_matcher.go @@ -1,4 +1,20 @@ -package config +// Copyright 2024 Dimitrij Drus +// +// 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. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package rules import ( "errors" @@ -25,7 +41,7 @@ type ( compiled *regexp.Regexp } - valueMatcher struct { + exactMatcher struct { value string } ) @@ -38,7 +54,7 @@ func (m *regexpMatcher) match(matchAgainst string) bool { return m.compiled.MatchString(matchAgainst) } -func (m *valueMatcher) match(value string) bool { return m.value == value } +func (m *exactMatcher) match(value string) bool { return m.value == value } func newGlobMatcher(pattern string, separator rune) (typedMatcher, error) { if len(pattern) == 0 { @@ -66,4 +82,4 @@ func newRegexMatcher(pattern string) (typedMatcher, error) { return ®expMatcher{compiled: compiled}, nil } -func newValueMatcher(value string) (typedMatcher, error) { return &valueMatcher{value: value}, nil } +func newExactMatcher(value string) (typedMatcher, error) { return &exactMatcher{value: value}, nil } diff --git a/internal/rules/typed_matcher_test.go b/internal/rules/typed_matcher_test.go index c03570275..4cc91b18e 100644 --- a/internal/rules/typed_matcher_test.go +++ b/internal/rules/typed_matcher_test.go @@ -1,4 +1,20 @@ -package config +// Copyright 2024 Dimitrij Drus +// +// 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. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package rules import ( "testing" @@ -134,3 +150,25 @@ func TestGlobPatternMatcher(t *testing.T) { }) } } + +func TestExactMatcher(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + uc string + expression string + toMatch string + matches bool + }{ + {uc: "matches", expression: "foo", toMatch: "foo", matches: true}, + {uc: "doesn't match", expression: "foo", toMatch: "bar"}, + } { + t.Run(tc.uc, func(t *testing.T) { + matcher, err := newExactMatcher(tc.expression) + require.NoError(t, err) + + matches := matcher.match(tc.toMatch) + assert.Equal(t, tc.matches, matches) + }) + } +} From 9dca4c5937fe2b0cb17f57cd373e725494682fd7 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:55:29 +0200 Subject: [PATCH 08/32] rule implementation updated to adhere to the updated intrerface, route implementation --- internal/rules/rule_impl.go | 53 +++++++++++++++++++++---------- internal/rules/rule_impl_test.go | 54 -------------------------------- 2 files changed, 36 insertions(+), 71 deletions(-) diff --git a/internal/rules/rule_impl.go b/internal/rules/rule_impl.go index 146d6bb65..a0f2a0f2e 100644 --- a/internal/rules/rule_impl.go +++ b/internal/rules/rule_impl.go @@ -17,6 +17,7 @@ package rules import ( + "bytes" "net/url" "strings" @@ -32,10 +33,9 @@ type ruleImpl struct { id string srcID string isDefault bool - hash []byte - pathExpression string - matcher config.RequestMatcher allowsBacktracking bool + hash []byte + routes []rule.Route slashesHandling config.EncodedSlashesHandling backend *config.Backend sc compositeSubjectCreator @@ -99,13 +99,40 @@ func (r *ruleImpl) Execute(ctx heimdall.Context) (rule.Backend, error) { return upstream, nil } -func (r *ruleImpl) Matches(ctx heimdall.Context) bool { - request := ctx.Request() - logger := zerolog.Ctx(ctx.AppContext()).With().Str("_source", r.srcID).Str("_id", r.id).Logger() +func (r *ruleImpl) ID() string { return r.id } + +func (r *ruleImpl) SrcID() string { return r.srcID } + +func (r *ruleImpl) SameAs(other rule.Rule) bool { + return r.ID() == other.ID() && r.SrcID() == other.SrcID() +} + +func (r *ruleImpl) Routes() []rule.Route { return r.routes } + +func (r *ruleImpl) EqualTo(other rule.Rule) bool { + return r.ID() == other.ID() && + r.SrcID() == other.SrcID() && + bytes.Equal(r.hash, other.(*ruleImpl).hash) +} + +func (r *ruleImpl) AllowsBacktracking() bool { return r.allowsBacktracking } + +type routeImpl struct { + rule *ruleImpl + path string + matcher RouteMatcher +} + +func (r *routeImpl) Matches(ctx heimdall.Context, keys, values []string) bool { + logger := zerolog.Ctx(ctx.AppContext()).With(). + Str("_source", r.rule.srcID). + Str("_id", r.rule.id). + Str("route", r.path). + Logger() logger.Debug().Msg("Matching rule") - if err := r.matcher.Matches(request); err != nil { + if err := r.matcher.Matches(ctx.Request(), keys, values); err != nil { logger.Debug().Err(err).Msg("Request does not satisfy matching conditions") return false @@ -116,17 +143,9 @@ func (r *ruleImpl) Matches(ctx heimdall.Context) bool { return true } -func (r *ruleImpl) ID() string { return r.id } - -func (r *ruleImpl) SrcID() string { return r.srcID } - -func (r *ruleImpl) PathExpression() string { return r.pathExpression } - -func (r *ruleImpl) BacktrackingEnabled() bool { return r.allowsBacktracking } +func (r *routeImpl) Path() string { return r.path } -func (r *ruleImpl) SameAs(other rule.Rule) bool { - return r.ID() == other.ID() && r.SrcID() == other.SrcID() -} +func (r *routeImpl) Rule() rule.Rule { return r.rule } type backend struct { targetURL *url.URL diff --git a/internal/rules/rule_impl_test.go b/internal/rules/rule_impl_test.go index 1202c9faa..6a256d8d1 100644 --- a/internal/rules/rule_impl_test.go +++ b/internal/rules/rule_impl_test.go @@ -18,19 +18,15 @@ package rules import ( "context" - "errors" - "net/http" "net/url" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/dadrus/heimdall/internal/heimdall" heimdallmocks "github.com/dadrus/heimdall/internal/heimdall/mocks" "github.com/dadrus/heimdall/internal/rules/config" - mocks2 "github.com/dadrus/heimdall/internal/rules/config/mocks" "github.com/dadrus/heimdall/internal/rules/mechanisms/subject" "github.com/dadrus/heimdall/internal/rules/mocks" "github.com/dadrus/heimdall/internal/rules/rule" @@ -38,56 +34,6 @@ import ( "github.com/dadrus/heimdall/internal/x/testsupport" ) -func TestRuleMatches(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - uc string - rule *ruleImpl - toMatch *heimdall.Request - matches bool - }{ - { - uc: "doesn't match", - rule: &ruleImpl{ - matcher: func() config.RequestMatcher { - rm := mocks2.NewRequestMatcherMock(t) - rm.EXPECT().Matches(mock.Anything).Return(errors.New("test error")) - - return rm - }(), - }, - toMatch: &heimdall.Request{Method: http.MethodGet, URL: &heimdall.URL{}}, - matches: false, - }, - { - uc: "matches", - rule: &ruleImpl{ - matcher: func() config.RequestMatcher { - rm := mocks2.NewRequestMatcherMock(t) - rm.EXPECT().Matches(mock.Anything).Return(nil) - - return rm - }(), - }, - toMatch: &heimdall.Request{Method: http.MethodPost, URL: &heimdall.URL{}}, - matches: true, - }, - } { - t.Run("case="+tc.uc, func(t *testing.T) { - ctx := heimdallmocks.NewContextMock(t) - ctx.EXPECT().AppContext().Return(context.TODO()) - ctx.EXPECT().Request().Return(tc.toMatch) - - // WHEN - matched := tc.rule.Matches(ctx) - - // THEN - assert.Equal(t, tc.matches, matched) - }) - } -} - func TestRuleExecute(t *testing.T) { t.Parallel() From 3d604db8c25d6eec438e9c28a944dfd02b6351af Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:55:57 +0200 Subject: [PATCH 09/32] rule factory implementattion updated --- internal/rules/rule_factory_impl.go | 89 +++++++----- internal/rules/rule_factory_impl_test.go | 174 +++++++++++++++++------ 2 files changed, 186 insertions(+), 77 deletions(-) diff --git a/internal/rules/rule_factory_impl.go b/internal/rules/rule_factory_impl.go index 352f86944..cdeb61efd 100644 --- a/internal/rules/rule_factory_impl.go +++ b/internal/rules/rule_factory_impl.go @@ -51,11 +51,12 @@ func NewRuleFactory( } type ruleFactory struct { - hf mechanisms.MechanismFactory - logger zerolog.Logger - defaultRule *ruleImpl - hasDefaultRule bool - mode config.OperationMode + hf mechanisms.MechanismFactory + logger zerolog.Logger + defaultRule *ruleImpl + hasDefaultRule bool + mode config.OperationMode + defaultBacktracking bool } //nolint:funlen,gocognit,cyclop @@ -158,11 +159,6 @@ func (f *ruleFactory) CreateRule(version, srcID string, ruleConfig config2.Rule) config2.EncodedSlashesOff, ) - matcher, err := ruleConfig.Matcher.With.ToRequestMatcher(slashesHandling) - if err != nil { - return nil, err - } - authenticators, subHandlers, finalizers, err := f.createExecutePipeline(version, ruleConfig.Execute) if err != nil { return nil, err @@ -173,14 +169,16 @@ func (f *ruleFactory) CreateRule(version, srcID string, ruleConfig config2.Rule) return nil, err } - var defaultBacktracking bool + var allowsBacktracking bool if f.defaultRule != nil { authenticators = x.IfThenElse(len(authenticators) != 0, authenticators, f.defaultRule.sc) subHandlers = x.IfThenElse(len(subHandlers) != 0, subHandlers, f.defaultRule.sh) finalizers = x.IfThenElse(len(finalizers) != 0, finalizers, f.defaultRule.fi) errorHandlers = x.IfThenElse(len(errorHandlers) != 0, errorHandlers, f.defaultRule.eh) - defaultBacktracking = f.defaultRule.allowsBacktracking + allowsBacktracking = x.IfThenElseExec(ruleConfig.Matcher.BacktrackingEnabled != nil, + func() bool { return *ruleConfig.Matcher.BacktrackingEnabled }, + func() bool { return f.defaultBacktracking }) } if len(authenticators) == 0 { @@ -192,25 +190,47 @@ func (f *ruleFactory) CreateRule(version, srcID string, ruleConfig config2.Rule) return nil, err } - allowsBacktracking := x.IfThenElseExec(ruleConfig.Matcher.BacktrackingEnabled != nil, - func() bool { return *ruleConfig.Matcher.BacktrackingEnabled }, - func() bool { return defaultBacktracking }) - - return &ruleImpl{ + rul := &ruleImpl{ id: ruleConfig.ID, srcID: srcID, - isDefault: false, - allowsBacktracking: allowsBacktracking, slashesHandling: slashesHandling, - matcher: matcher, - pathExpression: ruleConfig.Matcher.Path, + allowsBacktracking: allowsBacktracking, backend: ruleConfig.Backend, hash: hash, sc: authenticators, sh: subHandlers, fi: finalizers, eh: errorHandlers, - }, nil + } + + mm, err := createMethodMatcher(ruleConfig.Matcher.Methods) + if err != nil { + return nil, err + } + + hm, err := createHostMatcher(ruleConfig.Matcher.Hosts) + if err != nil { + return nil, err + } + + sm := schemeMatcher(ruleConfig.Matcher.Scheme) + + for _, rc := range ruleConfig.Matcher.Routes { + ppm, err := createPathParamsMatcher(rc.PathParams, slashesHandling) + if err != nil { + return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, + "failed creating route '%s'", rc.Path). + CausedBy(err) + } + + rul.routes = append(rul.routes, &routeImpl{ + rule: rul, + path: rc.Path, + matcher: compositeMatcher{sm, mm, hm, ppm}, + }) + } + + return rul, nil } func (f *ruleFactory) createOnErrorPipeline( @@ -276,18 +296,18 @@ func (f *ruleFactory) initWithDefaultRule(ruleConfig *config.DefaultRule, logger } f.defaultRule = &ruleImpl{ - id: "default", - slashesHandling: config2.EncodedSlashesOff, - srcID: "config", - isDefault: true, - allowsBacktracking: ruleConfig.BacktrackingEnabled, - sc: authenticators, - sh: subHandlers, - fi: finalizers, - eh: errorHandlers, + id: "default", + slashesHandling: config2.EncodedSlashesOff, + srcID: "config", + isDefault: true, + sc: authenticators, + sh: subHandlers, + fi: finalizers, + eh: errorHandlers, } f.hasDefaultRule = true + f.defaultBacktracking = ruleConfig.BacktrackingEnabled return nil } @@ -330,11 +350,12 @@ func getConfig(conf any) config.MechanismConfig { return nil } - if m, ok := conf.(map[string]any); ok { - return m + m, ok := conf.(map[string]any) + if !ok { + panic(fmt.Sprintf("unexpected type for config %T", conf)) } - panic(fmt.Sprintf("unexpected type for config %T", conf)) + return m } func getExecutionCondition(conf any) (executionCondition, error) { diff --git a/internal/rules/rule_factory_impl_test.go b/internal/rules/rule_factory_impl_test.go index ad2934db5..c5119f8bc 100644 --- a/internal/rules/rule_factory_impl_test.go +++ b/internal/rules/rule_factory_impl_test.go @@ -409,6 +409,8 @@ func TestRuleFactoryNew(t *testing.T) { func TestRuleFactoryCreateRule(t *testing.T) { t.Parallel() + trueValue := true + for _, tc := range []struct { uc string opMode config.OperationMode @@ -422,7 +424,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { opMode: config.ProxyMode, config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, }, assert: func(t *testing.T, err error, _ *ruleImpl) { t.Helper() @@ -433,27 +435,89 @@ func TestRuleFactoryCreateRule(t *testing.T) { }, }, { - uc: "with error while creating request matcher", + uc: "with error while creating method matcher", + config: config2.Rule{ + ID: "foobar", + Matcher: config2.Matcher{ + Routes: []config2.Route{{Path: "/foo/bar"}}, + Methods: []string{""}, + }, + Execute: []config.MechanismConfig{ + {"authenticator": "foo"}, + }, + }, + configureMocks: func(t *testing.T, mhf *mocks3.MechanismFactoryMock) { + t.Helper() + + mhf.EXPECT().CreateAuthenticator("test", "foo", mock.Anything).Return(&mocks2.AuthenticatorMock{}, nil) + }, + assert: func(t *testing.T, err error, _ *ruleImpl) { + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "methods list contains empty values") + }, + }, + { + uc: "with error while creating route path params matcher", + config: config2.Rule{ + ID: "foobar", + Matcher: config2.Matcher{ + Routes: []config2.Route{ + { + Path: "/foo/:bar", + PathParams: []config2.ParameterMatcher{{Name: "bar", Type: "foo", Value: "baz"}}, + }, + }, + }, + Execute: []config.MechanismConfig{ + {"authenticator": "foo"}, + }, + }, + configureMocks: func(t *testing.T, mhf *mocks3.MechanismFactoryMock) { + t.Helper() + + mhf.EXPECT().CreateAuthenticator("test", "foo", mock.Anything).Return(&mocks2.AuthenticatorMock{}, nil) + }, + assert: func(t *testing.T, err error, _ *ruleImpl) { + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "failed creating route '/foo/:bar'") + }, + }, + { + uc: "with error while creating host matcher", config: config2.Rule{ ID: "foobar", Matcher: config2.Matcher{ - Path: "/foo/bar", - With: &config2.MatcherConstraints{HostRegex: "?>?<*??"}, + Routes: []config2.Route{{Path: "/foo/bar"}}, + Hosts: []config2.HostMatcher{{Type: "regex", Value: "?>?<*??"}}, }, + Execute: []config.MechanismConfig{ + {"authenticator": "foo"}, + }, + }, + configureMocks: func(t *testing.T, mhf *mocks3.MechanismFactoryMock) { + t.Helper() + + mhf.EXPECT().CreateAuthenticator("test", "foo", mock.Anything).Return(&mocks2.AuthenticatorMock{}, nil) }, assert: func(t *testing.T, err error, _ *ruleImpl) { t.Helper() require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "filed to compile host expression") + require.ErrorContains(t, err, "failed to compile host matching expression") }, }, { uc: "with error while creating execute pipeline", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{{"authenticator": "foo"}}, }, configureMocks: func(t *testing.T, mhf *mocks3.MechanismFactoryMock) { @@ -472,7 +536,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "with error while creating on_error pipeline", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, ErrorHandler: []config.MechanismConfig{{"error_handler": "foo"}}, }, configureMocks: func(t *testing.T, mhf *mocks3.MechanismFactoryMock) { @@ -491,7 +555,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "without default rule and without any execute configuration", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, }, assert: func(t *testing.T, err error, _ *ruleImpl) { t.Helper() @@ -505,7 +569,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "without default rule and minimum required configuration in decision mode", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{ {"authenticator": "foo"}, }, @@ -525,8 +589,9 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) assert.Equal(t, config2.EncodedSlashesOff, rul.slashesHandling) - assert.NotNil(t, rul.matcher) - assert.Equal(t, "/foo/bar", rul.PathExpression()) + assert.Len(t, rul.Routes(), 1) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/bar", rul.Routes()[0].Path()) assert.Len(t, rul.sc, 1) assert.Empty(t, rul.sh) assert.Empty(t, rul.fi) @@ -539,7 +604,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { config: config2.Rule{ ID: "foobar", Backend: &config2.Backend{Host: "foo.bar"}, - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{ {"authenticator": "foo"}, }, @@ -559,8 +624,9 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) assert.Equal(t, config2.EncodedSlashesOff, rul.slashesHandling) - assert.NotNil(t, rul.matcher) - assert.Equal(t, "/foo/bar", rul.PathExpression()) + assert.Len(t, rul.Routes(), 1) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/bar", rul.Routes()[0].Path()) assert.Len(t, rul.sc, 1) assert.Empty(t, rul.sh) assert.Empty(t, rul.fi) @@ -569,10 +635,10 @@ func TestRuleFactoryCreateRule(t *testing.T) { }, }, { - uc: "with default rule and with id and path expression only", + uc: "with default rule and regular rule with id and a single route only", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, }, defaultRule: &ruleImpl{ sc: compositeSubjectCreator{&mocks.SubjectCreatorMock{}}, @@ -589,8 +655,9 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.Equal(t, "test", rul.srcID) assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) - assert.NotNil(t, rul.matcher) - assert.Equal(t, "/foo/bar", rul.PathExpression()) + assert.Len(t, rul.Routes(), 1) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/bar", rul.Routes()[0].Path()) assert.Len(t, rul.sc, 1) assert.Len(t, rul.sh, 1) assert.Len(t, rul.fi, 1) @@ -598,17 +665,24 @@ func TestRuleFactoryCreateRule(t *testing.T) { }, }, { - uc: "with default rule and with all attributes defined by the rule itself in decision mode", + uc: "with default rule and with all attributes defined by the regular rule itself in decision mode", config: config2.Rule{ ID: "foobar", Matcher: config2.Matcher{ - Path: "/foo/:resource", - With: &config2.MatcherConstraints{ - Scheme: "https", - HostGlob: "**.example.com", - PathRegex: "^/foo/(bar|baz)", - Methods: []string{"BAR", "BAZ"}, + Routes: []config2.Route{ + { + Path: "/foo/:resource", + PathParams: []config2.ParameterMatcher{{Name: "resource", Type: "regex", Value: "(bar|baz)"}}, + }, + { + Path: "/bar/:resource", + PathParams: []config2.ParameterMatcher{{Name: "resource", Type: "glob", Value: "{a,b}"}}, + }, }, + BacktrackingEnabled: &trueValue, + Scheme: "https", + Methods: []string{"BAR", "BAZ"}, + Hosts: []config2.HostMatcher{{Type: "glob", Value: "**.example.com"}}, }, EncodedSlashesHandling: config2.EncodedSlashesOnNoDecode, Execute: []config.MechanismConfig{ @@ -651,8 +725,11 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) assert.Equal(t, config2.EncodedSlashesOnNoDecode, rul.slashesHandling) - assert.Equal(t, "/foo/:resource", rul.PathExpression()) - assert.NotNil(t, rul.matcher) + assert.Len(t, rul.Routes(), 2) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/:resource", rul.Routes()[0].Path()) + assert.Equal(t, rul, rul.Routes()[1].Rule()) + assert.Equal(t, "/bar/:resource", rul.Routes()[1].Path()) // nil checks above mean the responses from the mockHandlerFactory are used // and not the values from the default rule @@ -673,13 +750,19 @@ func TestRuleFactoryCreateRule(t *testing.T) { config: config2.Rule{ ID: "foobar", Matcher: config2.Matcher{ - Path: "/foo/:resource", - With: &config2.MatcherConstraints{ - Scheme: "https", - HostGlob: "**.example.com", - PathRegex: "^/foo/(bar|baz)", - Methods: []string{"BAR", "BAZ"}, + Routes: []config2.Route{ + { + Path: "/foo/:resource", + PathParams: []config2.ParameterMatcher{{Name: "resource", Type: "regex", Value: "(bar|baz)"}}, + }, + { + Path: "/bar/:resource", + PathParams: []config2.ParameterMatcher{{Name: "resource", Type: "glob", Value: "{a,b}"}}, + }, }, + Scheme: "https", + Methods: []string{"BAR", "BAZ"}, + Hosts: []config2.HostMatcher{{Type: "glob", Value: "**.example.com"}}, }, EncodedSlashesHandling: config2.EncodedSlashesOn, Backend: &config2.Backend{ @@ -731,8 +814,11 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) assert.Equal(t, config2.EncodedSlashesOn, rul.slashesHandling) - assert.Equal(t, "/foo/:resource", rul.PathExpression()) - assert.NotNil(t, rul.matcher) + assert.Len(t, rul.Routes(), 2) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/:resource", rul.Routes()[0].Path()) + assert.Equal(t, rul, rul.Routes()[1].Rule()) + assert.Equal(t, "/bar/:resource", rul.Routes()[1].Path()) assert.Equal(t, "https://bar.foo/baz/bar?foo=bar", rul.backend.CreateURL(&url.URL{ Scheme: "http", Host: "foo.bar:8888", @@ -758,7 +844,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "with conditional execution configuration type error", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{ {"authenticator": "foo"}, {"finalizer": "bar", "if": 1}, @@ -781,7 +867,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "with empty conditional execution configuration", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{ {"authenticator": "foo"}, {"finalizer": "bar", "if": ""}, @@ -804,7 +890,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "with conditional execution for some mechanisms", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{ {"authenticator": "foo"}, {"authorizer": "bar", "if": "false"}, @@ -834,8 +920,9 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.Equal(t, "test", rul.srcID) assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) - assert.Equal(t, "/foo/bar", rul.PathExpression()) - assert.NotNil(t, rul.matcher) + assert.Len(t, rul.Routes(), 1) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/bar", rul.Routes()[0].Path()) require.Len(t, rul.sc, 1) assert.NotNil(t, rul.sc[0]) @@ -869,7 +956,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { uc: "with conditional execution for error handler", config: config2.Rule{ ID: "foobar", - Matcher: config2.Matcher{Path: "/foo/bar"}, + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, Execute: []config.MechanismConfig{ {"authenticator": "foo"}, {"authorizer": "bar"}, @@ -905,8 +992,9 @@ func TestRuleFactoryCreateRule(t *testing.T) { assert.Equal(t, "test", rul.srcID) assert.False(t, rul.isDefault) assert.Equal(t, "foobar", rul.id) - assert.Equal(t, "/foo/bar", rul.PathExpression()) - assert.NotNil(t, rul.matcher) + assert.Len(t, rul.Routes(), 1) + assert.Equal(t, rul, rul.Routes()[0].Rule()) + assert.Equal(t, "/foo/bar", rul.Routes()[0].Path()) require.Len(t, rul.sc, 1) assert.NotNil(t, rul.sc[0]) From 65da350d41db0c10f069f8a7f2cb564c51054a28 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:56:17 +0200 Subject: [PATCH 10/32] rule repository implementation updated --- internal/rules/repository_impl.go | 66 ++++----- internal/rules/repository_impl_test.go | 183 +++++++++++++++---------- 2 files changed, 145 insertions(+), 104 deletions(-) diff --git a/internal/rules/repository_impl.go b/internal/rules/repository_impl.go index 5bfafcb78..064002dc7 100644 --- a/internal/rules/repository_impl.go +++ b/internal/rules/repository_impl.go @@ -17,7 +17,6 @@ package rules import ( - "bytes" "slices" "sync" @@ -35,7 +34,7 @@ type repository struct { knownRules []rule.Rule knownRulesMutex sync.Mutex - index *radixtree.Tree[rule.Rule] + index *radixtree.Tree[rule.Route] rulesTreeMutex sync.RWMutex } @@ -44,10 +43,10 @@ func newRepository(ruleFactory rule.Factory) rule.Repository { dr: x.IfThenElseExec(ruleFactory.HasDefaultRule(), func() rule.Rule { return ruleFactory.DefaultRule() }, func() rule.Rule { return nil }), - index: radixtree.New[rule.Rule]( - radixtree.WithValuesConstraints(func(oldValues []rule.Rule, newValue rule.Rule) bool { + index: radixtree.New[rule.Route]( + radixtree.WithValuesConstraints(func(oldValues []rule.Route, newValue rule.Route) bool { // only rules from the same rule set can be placed in one node - return len(oldValues) == 0 || oldValues[0].SrcID() == newValue.SrcID() + return len(oldValues) == 0 || oldValues[0].Rule().SrcID() == newValue.Rule().SrcID() }), ), } @@ -61,7 +60,9 @@ func (r *repository) FindRule(ctx heimdall.Context) (rule.Rule, error) { entry, err := r.index.Find( x.IfThenElse(len(request.URL.RawPath) != 0, request.URL.RawPath, request.URL.Path), - radixtree.MatcherFunc[rule.Rule](func(candidate rule.Rule) bool { return candidate.Matches(ctx) }), + radixtree.LookupMatcherFunc[rule.Route](func(route rule.Route, keys, values []string) bool { + return route.Matches(ctx, keys, values) + }), ) if err != nil { if r.dr != nil { @@ -74,7 +75,7 @@ func (r *repository) FindRule(ctx heimdall.Context) (rule.Rule, error) { request.URL.Captures = entry.Parameters - return entry.Value, nil + return entry.Value.Rule(), nil } func (r *repository) AddRuleSet(_ string, rules []rule.Rule) error { @@ -106,16 +107,12 @@ func (r *repository) UpdateRuleSet(srcID string, rules []rule.Rule) error { // find new rules, as well as those, which have been changed. toBeAdded := slicex.Filter(rules, func(newRule rule.Rule) bool { - candidate := newRule.(*ruleImpl) //nolint: forcetypeassert - ruleIsNew := !slices.ContainsFunc(applicable, func(existingRule rule.Rule) bool { - return existingRule.ID() == newRule.ID() + return existingRule.SameAs(newRule) }) ruleChanged := slices.ContainsFunc(applicable, func(existingRule rule.Rule) bool { - existing := existingRule.(*ruleImpl) //nolint: forcetypeassert - - return existing.ID() == candidate.ID() && !bytes.Equal(existing.hash, candidate.hash) + return existingRule.SameAs(newRule) && !existingRule.EqualTo(newRule) }) return ruleIsNew || ruleChanged @@ -123,16 +120,12 @@ func (r *repository) UpdateRuleSet(srcID string, rules []rule.Rule) error { // find deleted rules, as well as those, which have been changed. toBeDeleted := slicex.Filter(applicable, func(existingRule rule.Rule) bool { - existing := existingRule.(*ruleImpl) //nolint: forcetypeassert - ruleGone := !slices.ContainsFunc(rules, func(newRule rule.Rule) bool { - return newRule.ID() == existingRule.ID() + return newRule.SameAs(existingRule) }) ruleChanged := slices.ContainsFunc(rules, func(newRule rule.Rule) bool { - candidate := newRule.(*ruleImpl) //nolint: forcetypeassert - - return existing.ID() == candidate.ID() && !bytes.Equal(existing.hash, candidate.hash) + return newRule.SameAs(existingRule) && !newRule.EqualTo(existingRule) }) return ruleGone || ruleChanged @@ -187,28 +180,35 @@ func (r *repository) DeleteRuleSet(srcID string) error { return nil } -func (r *repository) addRulesTo(tree *radixtree.Tree[rule.Rule], rules []rule.Rule) error { +func (r *repository) addRulesTo(tree *radixtree.Tree[rule.Route], rules []rule.Rule) error { for _, rul := range rules { - if err := tree.Add( - rul.PathExpression(), - rul, - radixtree.WithBacktracking[rule.Rule](rul.BacktrackingEnabled())); err != nil { - return errorchain.NewWithMessagef(heimdall.ErrInternal, "failed adding rule ID='%s'", rul.ID()). - CausedBy(err) + for _, route := range rul.Routes() { + if err := tree.Add( + route.Path(), + route, + radixtree.WithBacktracking[rule.Route](rul.AllowsBacktracking()), + ); err != nil { + return errorchain.NewWithMessagef(heimdall.ErrInternal, "failed adding rule ID='%s'", rul.ID()). + CausedBy(err) + } } } return nil } -func (r *repository) removeRulesFrom(tree *radixtree.Tree[rule.Rule], tbdRules []rule.Rule) error { +func (r *repository) removeRulesFrom(tree *radixtree.Tree[rule.Route], tbdRules []rule.Rule) error { for _, rul := range tbdRules { - if err := tree.Delete( - rul.PathExpression(), - radixtree.MatcherFunc[rule.Rule](func(existing rule.Rule) bool { return existing.SameAs(rul) }), - ); err != nil { - return errorchain.NewWithMessagef(heimdall.ErrInternal, "failed deleting rule ID='%s'", rul.ID()). - CausedBy(err) + for _, route := range rul.Routes() { + if err := tree.Delete( + route.Path(), + radixtree.ValueMatcherFunc[rule.Route](func(route rule.Route) bool { + return route.Rule().SameAs(rul) + }), + ); err != nil { + return errorchain.NewWithMessagef(heimdall.ErrInternal, "failed deleting rule ID='%s'", rul.ID()). + CausedBy(err) + } } } diff --git a/internal/rules/repository_impl_test.go b/internal/rules/repository_impl_test.go index af7e8f957..6db22388d 100644 --- a/internal/rules/repository_impl_test.go +++ b/internal/rules/repository_impl_test.go @@ -23,13 +23,10 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/dadrus/heimdall/internal/heimdall" mocks2 "github.com/dadrus/heimdall/internal/heimdall/mocks" - "github.com/dadrus/heimdall/internal/rules/config" - mocks3 "github.com/dadrus/heimdall/internal/rules/config/mocks" "github.com/dadrus/heimdall/internal/rules/rule" "github.com/dadrus/heimdall/internal/rules/rule/mocks" "github.com/dadrus/heimdall/internal/x" @@ -41,9 +38,13 @@ func TestRepositoryAddRuleSetWithoutViolation(t *testing.T) { // GIVEN repo := newRepository(&ruleFactory{}).(*repository) //nolint: forcetypeassert - rules := []rule.Rule{ - &ruleImpl{id: "1", srcID: "1", pathExpression: "/foo/1"}, - } + + rule1 := &ruleImpl{id: "1", srcID: "1"} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/foo/1"}) + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/foo/2"}) + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/foo/3"}) + + rules := []rule.Rule{rule1} // WHEN err := repo.AddRuleSet("1", rules) @@ -53,7 +54,12 @@ func TestRepositoryAddRuleSetWithoutViolation(t *testing.T) { assert.Len(t, repo.knownRules, 1) assert.False(t, repo.index.Empty()) assert.ElementsMatch(t, repo.knownRules, rules) - _, err = repo.index.Find("/foo/1", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + + _, err = repo.index.Find("/foo/1", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) + _, err = repo.index.Find("/foo/2", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) + _, err = repo.index.Find("/foo/3", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) require.NoError(t, err) } @@ -62,8 +68,16 @@ func TestRepositoryAddRuleSetWithViolation(t *testing.T) { // GIVEN repo := newRepository(&ruleFactory{}).(*repository) //nolint: forcetypeassert - rules1 := []rule.Rule{&ruleImpl{id: "1", srcID: "1", pathExpression: "/foo/1"}} - rules2 := []rule.Rule{&ruleImpl{id: "2", srcID: "2", pathExpression: "/foo/1"}} + + rule1 := &ruleImpl{id: "1", srcID: "1"} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/foo/1"}) + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/foo/2"}) + + rule2 := &ruleImpl{id: "2", srcID: "2"} + rule2.routes = append(rule2.routes, &routeImpl{rule: rule2, path: "/foo/1"}) + + rules1 := []rule.Rule{rule1} + rules2 := []rule.Rule{rule2} require.NoError(t, repo.AddRuleSet("1", rules1)) @@ -77,7 +91,9 @@ func TestRepositoryAddRuleSetWithViolation(t *testing.T) { assert.Len(t, repo.knownRules, 1) assert.False(t, repo.index.Empty()) assert.ElementsMatch(t, repo.knownRules, rules1) - _, err = repo.index.Find("/foo/1", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/foo/1", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) + _, err = repo.index.Find("/foo/1", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) require.NoError(t, err) } @@ -86,14 +102,22 @@ func TestRepositoryRemoveRuleSet(t *testing.T) { // GIVEN repo := newRepository(&ruleFactory{}).(*repository) //nolint: forcetypeassert - rules1 := []rule.Rule{ - &ruleImpl{id: "1", srcID: "1", pathExpression: "/foo/1"}, - &ruleImpl{id: "2", srcID: "1", pathExpression: "/foo/2"}, - &ruleImpl{id: "3", srcID: "1", pathExpression: "/foo/3"}, - &ruleImpl{id: "4", srcID: "1", pathExpression: "/foo/4"}, - } - require.NoError(t, repo.AddRuleSet("1", rules1)) + rule1 := &ruleImpl{id: "1", srcID: "1"} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/foo/1"}) + + rule2 := &ruleImpl{id: "2", srcID: "1"} + rule2.routes = append(rule2.routes, &routeImpl{rule: rule2, path: "/foo/2"}) + + rule3 := &ruleImpl{id: "3", srcID: "1"} + rule3.routes = append(rule3.routes, &routeImpl{rule: rule3, path: "/foo/4"}) + + rule4 := &ruleImpl{id: "4", srcID: "1"} + rule4.routes = append(rule4.routes, &routeImpl{rule: rule4, path: "/foo/4"}) + + rules := []rule.Rule{rule1, rule2, rule3, rule4} + + require.NoError(t, repo.AddRuleSet("1", rules)) assert.Len(t, repo.knownRules, 4) assert.False(t, repo.index.Empty()) @@ -112,17 +136,24 @@ func TestRepositoryRemoveRulesFromDifferentRuleSets(t *testing.T) { // GIVEN repo := newRepository(&ruleFactory{}).(*repository) //nolint: forcetypeassert - rules1 := []rule.Rule{ - &ruleImpl{id: "1", srcID: "bar", pathExpression: "/bar/1"}, - &ruleImpl{id: "3", srcID: "bar", pathExpression: "/bar/3"}, - &ruleImpl{id: "4", srcID: "bar", pathExpression: "/bar/4"}, - } - rules2 := []rule.Rule{ - &ruleImpl{id: "2", srcID: "baz", pathExpression: "/baz/2"}, - } - rules3 := []rule.Rule{ - &ruleImpl{id: "4", srcID: "foo", pathExpression: "/foo/4"}, - } + rule1 := &ruleImpl{id: "1", srcID: "bar"} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/bar/1"}) + + rule2 := &ruleImpl{id: "3", srcID: "bar"} + rule2.routes = append(rule2.routes, &routeImpl{rule: rule2, path: "/bar/3"}) + + rule3 := &ruleImpl{id: "4", srcID: "bar"} + rule3.routes = append(rule3.routes, &routeImpl{rule: rule3, path: "/bar/4"}) + + rule4 := &ruleImpl{id: "2", srcID: "baz"} + rule4.routes = append(rule4.routes, &routeImpl{rule: rule4, path: "/baz/2"}) + + rule5 := &ruleImpl{id: "4", srcID: "foo"} + rule5.routes = append(rule5.routes, &routeImpl{rule: rule5, path: "/foo/4"}) + + rules1 := []rule.Rule{rule1, rule2, rule3} + rules2 := []rule.Rule{rule4} + rules3 := []rule.Rule{rule5} // WHEN require.NoError(t, repo.AddRuleSet("bar", rules1)) @@ -141,19 +172,19 @@ func TestRepositoryRemoveRulesFromDifferentRuleSets(t *testing.T) { assert.Len(t, repo.knownRules, 2) assert.ElementsMatch(t, repo.knownRules, []rule.Rule{rules2[0], rules3[0]}) - _, err = repo.index.Find("/bar/1", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/bar/1", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.Error(t, err) //nolint:testifylint - _, err = repo.index.Find("/bar/3", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/bar/3", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.Error(t, err) //nolint:testifylint - _, err = repo.index.Find("/bar/4", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/bar/4", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.Error(t, err) //nolint:testifylint - _, err = repo.index.Find("/baz/2", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/baz/2", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.NoError(t, err) //nolint:testifylint - _, err = repo.index.Find("/foo/4", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/foo/4", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.NoError(t, err) //nolint:testifylint // WHEN @@ -164,10 +195,10 @@ func TestRepositoryRemoveRulesFromDifferentRuleSets(t *testing.T) { assert.Len(t, repo.knownRules, 1) assert.ElementsMatch(t, repo.knownRules, []rule.Rule{rules2[0]}) - _, err = repo.index.Find("/foo/4", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/foo/4", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.Error(t, err) //nolint:testifylint - _, err = repo.index.Find("/baz/2", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) + _, err = repo.index.Find("/baz/2", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) assert.NoError(t, err) //nolint:testifylint // WHEN @@ -185,21 +216,35 @@ func TestRepositoryUpdateRuleSet(t *testing.T) { // GIVEN repo := newRepository(&ruleFactory{}).(*repository) //nolint: forcetypeassert - initialRules := []rule.Rule{ - &ruleImpl{id: "1", srcID: "1", pathExpression: "/bar/1", hash: []byte{1}}, - &ruleImpl{id: "2", srcID: "1", pathExpression: "/bar/2", hash: []byte{1}}, - &ruleImpl{id: "3", srcID: "1", pathExpression: "/bar/3", hash: []byte{1}}, - &ruleImpl{id: "4", srcID: "1", pathExpression: "/bar/4", hash: []byte{1}}, - } + rule1 := &ruleImpl{id: "1", srcID: "1", hash: []byte{1}} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/bar/1"}) + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/bar/1a"}) + + rule2 := &ruleImpl{id: "2", srcID: "1", hash: []byte{1}} + rule2.routes = append(rule2.routes, &routeImpl{rule: rule2, path: "/bar/2"}) + + rule3 := &ruleImpl{id: "3", srcID: "1", hash: []byte{1}} + rule3.routes = append(rule3.routes, &routeImpl{rule: rule3, path: "/bar/3"}) + + rule4 := &ruleImpl{id: "4", srcID: "1", hash: []byte{1}} + rule4.routes = append(rule4.routes, &routeImpl{rule: rule4, path: "/bar/4"}) + + initialRules := []rule.Rule{rule1, rule2, rule3, rule4} require.NoError(t, repo.AddRuleSet("1", initialRules)) - updatedRules := []rule.Rule{ - &ruleImpl{id: "1", srcID: "1", pathExpression: "/bar/1", hash: []byte{2}}, // changed - // rule with id 2 is deleted - &ruleImpl{id: "3", srcID: "1", pathExpression: "/foo/3", hash: []byte{2}}, // changed and path expression changed - &ruleImpl{id: "4", srcID: "1", pathExpression: "/bar/4", hash: []byte{1}}, // same as before - } + // rule 1 changed: /bar/1a gone, /bar/1b added + rule1 = &ruleImpl{id: "1", srcID: "1", hash: []byte{2}} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/bar/1"}) + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/bar/1b"}) + // rule with id 2 is deleted + // rule 3 changed: /bar/2 gone, /foo/3 and /foo/4 added + rule3 = &ruleImpl{id: "3", srcID: "1", hash: []byte{2}} + rule3.routes = append(rule3.routes, &routeImpl{rule: rule3, path: "/foo/3"}) + rule3.routes = append(rule3.routes, &routeImpl{rule: rule3, path: "/foo/4"}) + // rule 4 same as before + + updatedRules := []rule.Rule{rule1, rule3, rule4} // WHEN err := repo.UpdateRuleSet("1", updatedRules) @@ -210,20 +255,25 @@ func TestRepositoryUpdateRuleSet(t *testing.T) { assert.Len(t, repo.knownRules, 3) assert.False(t, repo.index.Empty()) - _, err = repo.index.Find("/bar/1", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) - assert.NoError(t, err) //nolint:testifylint - - _, err = repo.index.Find("/bar/2", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) - assert.Error(t, err) //nolint:testifylint + _, err = repo.index.Find("/bar/1", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) + _, err = repo.index.Find("/bar/1a", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.Error(t, err) + _, err = repo.index.Find("/bar/1b", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) - _, err = repo.index.Find("/bar/3", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) - assert.Error(t, err) //nolint:testifylint + _, err = repo.index.Find("/bar/2", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.Error(t, err) - _, err = repo.index.Find("/foo/3", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) - assert.NoError(t, err) //nolint:testifylint + _, err = repo.index.Find("/bar/3", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.Error(t, err) + _, err = repo.index.Find("/foo/3", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) + _, err = repo.index.Find("/foo/4", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) - _, err = repo.index.Find("/bar/4", radixtree.MatcherFunc[rule.Rule](func(_ rule.Rule) bool { return true })) - assert.NoError(t, err) //nolint:testifylint + _, err = repo.index.Find("/bar/4", radixtree.LookupMatcherFunc[rule.Route](func(_ rule.Route, _, _ []string) bool { return true })) + require.NoError(t, err) } func TestRepositoryFindRule(t *testing.T) { @@ -278,19 +328,10 @@ func TestRepositoryFindRule(t *testing.T) { addRules: func(t *testing.T, repo *repository) { t.Helper() - err := repo.AddRuleSet("baz", []rule.Rule{ - &ruleImpl{ - id: "test2", - srcID: "baz", - pathExpression: "/baz/bar", - matcher: func() config.RequestMatcher { - rm := mocks3.NewRequestMatcherMock(t) - rm.EXPECT().Matches(mock.Anything).Return(nil) - - return rm - }(), - }, - }) + rule1 := &ruleImpl{id: "test2", srcID: "baz", hash: []byte{1}} + rule1.routes = append(rule1.routes, &routeImpl{rule: rule1, path: "/baz/bar", matcher: compositeMatcher{}}) + + err := repo.AddRuleSet("baz", []rule.Rule{rule1}) require.NoError(t, err) }, assert: func(t *testing.T, err error, rul rule.Rule) { From 060f4c05bf8789d5b3413dd427928d484be96d36 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:56:48 +0200 Subject: [PATCH 11/32] filesystem provider test updated to use the new configuration --- .../provider/filesystem/provider_test.go | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/internal/rules/provider/filesystem/provider_test.go b/internal/rules/provider/filesystem/provider_test.go index f06c23f43..7450c0d77 100644 --- a/internal/rules/provider/filesystem/provider_test.go +++ b/internal/rules/provider/filesystem/provider_test.go @@ -200,12 +200,19 @@ func TestProviderLifecycle(t *testing.T) { _, err := file.WriteString(` version: "1" +name: test rules: - id: foo match: - path: /foo/bar + routes: + - path: /foo/:bar + path_params: + - name: bar + type: glob + value: "*baz" + methods: [ GET ] execute: - - authenticator: test + - authenticator: test `) require.NoError(t, err) @@ -225,9 +232,19 @@ rules: ruleSet := mock2.ArgumentCaptorFrom[*config2.RuleSet](&processor.Mock, "captor1").Value() assert.Contains(t, ruleSet.Source, "file_system:") + require.NotNil(t, ruleSet) + assert.Equal(t, "test", ruleSet.Name) assert.Equal(t, "1", ruleSet.Version) assert.Len(t, ruleSet.Rules, 1) assert.Equal(t, "foo", ruleSet.Rules[0].ID) + require.Len(t, ruleSet.Rules[0].Matcher.Routes, 1) + assert.Equal(t, "/foo/:bar", ruleSet.Rules[0].Matcher.Routes[0].Path) + require.Len(t, ruleSet.Rules[0].Matcher.Routes[0].PathParams, 1) + assert.Equal(t, "bar", ruleSet.Rules[0].Matcher.Routes[0].PathParams[0].Name) + assert.Equal(t, "glob", ruleSet.Rules[0].Matcher.Routes[0].PathParams[0].Type) + assert.Equal(t, "*baz", ruleSet.Rules[0].Matcher.Routes[0].PathParams[0].Value) + assert.Equal(t, []string{"GET"}, ruleSet.Rules[0].Matcher.Methods) + assert.NotEmpty(t, ruleSet.Hash) }, }, { @@ -256,7 +273,8 @@ version: "2" rules: - id: foo match: - path: /foo/bar + routes: + - path: /foo/bar execute: - authenticator: test `) @@ -299,7 +317,8 @@ version: "1" rules: - id: foo match: - path: /foo/bar + routes: + - path: /foo/bar execute: - authenticator: test `) @@ -335,7 +354,8 @@ version: "1" rules: - id: foo match: - path: /foo/bar + routes: + - path: /foo/bar execute: - authenticator: test `) @@ -386,7 +406,8 @@ version: "1" rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test `) @@ -402,7 +423,8 @@ version: "1" rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test `) @@ -418,7 +440,8 @@ version: "2" rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test `) From fb2ad8e7dececefc0c72c32431c0c6ccbd03c4a1 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:57:15 +0200 Subject: [PATCH 12/32] httpendpoint provider tests updated to use the new configuration --- .../provider/httpendpoint/provider_test.go | 54 +++++++++++-------- .../httpendpoint/ruleset_endpoint_test.go | 43 ++++++++++----- 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/internal/rules/provider/httpendpoint/provider_test.go b/internal/rules/provider/httpendpoint/provider_test.go index e936b8d69..39f2d368a 100644 --- a/internal/rules/provider/httpendpoint/provider_test.go +++ b/internal/rules/provider/httpendpoint/provider_test.go @@ -263,9 +263,9 @@ name: test rules: - id: foo match: - path: /foo - with: - methods: [ "GET" ] + routes: + - path: /foo + methods: [ "GET" ] execute: - authenticator: test `)) @@ -311,9 +311,9 @@ name: test rules: - id: bar match: - path: /bar - with: - methods: [ "GET" ] + routes: + - path: /bar + methods: [ "GET" ] execute: - authenticator: test `)) @@ -364,9 +364,9 @@ name: test rules: - id: foo match: - path: /foo - with: - methods: [ GET ] + routes: + - path: /foo + methods: [ GET ] execute: - authenticator: test `)) @@ -381,9 +381,9 @@ name: test rules: - id: bar match: - path: /bar - with: - methods: [ GET ] + routes: + - path: /bar + methods: [ GET ] execute: - authenticator: test `)) @@ -452,7 +452,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test `)) @@ -465,7 +466,8 @@ name: test rules: - id: baz match: - path: /baz + routes: + - path: /baz execute: - authenticator: test `)) @@ -478,7 +480,8 @@ name: test rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test `)) @@ -491,7 +494,8 @@ name: test rules: - id: foz match: - path: /foz + routes: + - path: /foz execute: - authenticator: test `)) @@ -565,7 +569,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test `)) @@ -614,7 +619,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test `)) @@ -661,7 +667,8 @@ name: test rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test `)) @@ -702,7 +709,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test `)) @@ -715,7 +723,8 @@ name: test rules: - id: baz match: - path: /baz + routes: + - path: /baz execute: - authenticator: test `)) @@ -761,7 +770,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test `)) diff --git a/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go b/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go index 3f5e83d64..320be3886 100644 --- a/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go +++ b/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go @@ -135,7 +135,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar `)) require.NoError(t, err) }, @@ -185,9 +186,13 @@ name: test rules: - id: foo match: - path: /foo - with: - methods: [ GET ] + routes: + - path: /foo/:bar + path_params: + - name: bar + type: glob + value: "*baz" + methods: [ GET ] execute: - authenticator: test `)) @@ -199,9 +204,18 @@ rules: require.NoError(t, err) require.NotNil(t, ruleSet) + assert.Equal(t, "test", ruleSet.Name) + assert.Equal(t, "1", ruleSet.Version) assert.Len(t, ruleSet.Rules, 1) assert.Equal(t, "foo", ruleSet.Rules[0].ID) - require.NotEmpty(t, ruleSet.Hash) + require.Len(t, ruleSet.Rules[0].Matcher.Routes, 1) + assert.Equal(t, "/foo/:bar", ruleSet.Rules[0].Matcher.Routes[0].Path) + require.Len(t, ruleSet.Rules[0].Matcher.Routes[0].PathParams, 1) + assert.Equal(t, "bar", ruleSet.Rules[0].Matcher.Routes[0].PathParams[0].Name) + assert.Equal(t, "glob", ruleSet.Rules[0].Matcher.Routes[0].PathParams[0].Type) + assert.Equal(t, "*baz", ruleSet.Rules[0].Matcher.Routes[0].PathParams[0].Value) + assert.Equal(t, []string{"GET"}, ruleSet.Rules[0].Matcher.Methods) + assert.NotEmpty(t, ruleSet.Hash) }, }, { @@ -220,7 +234,13 @@ rules: "version": "1", "name": "test", "rules": [ - { "id": "foo", "match": { "path": "/foo", "with": { "methods" : ["GET"] }}, "execute": [{ "authenticator": "test"}] } + { + "id": "foo", + "match": { + "routes": [{"path": "/foo"}], + "methods" : ["GET"] + }, + "execute": [{ "authenticator": "test"}] } ] }`)) require.NoError(t, err) @@ -255,12 +275,11 @@ rules: { "id": "foo", "match": { - "path": "/foo/bar/:*", - "with": { - "host_glob": "moobar.local:9090", - "path_glob": "/foo/bar/**", - "methods": [ "GET" ] - } + "routes": [ + { "path": "/foo/bar/:*", "path_params": [{ "name": "*", "type":"glob", "value":"{*.ico,*.js}" }] } + ], + "methods": [ "GET" ], + "hosts": [{ "value":"moobar.local:9090", "type": "exact"}], }, "execute": [{ "authenticator": "test"}] } From a34c8d80d65b0ac8ae58b3b87ba9ac16382e862a Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:57:44 +0200 Subject: [PATCH 13/32] cloudblob provider tests updated to use the new configuration --- .../rules/provider/cloudblob/provider_test.go | 21 ++++-- .../cloudblob/ruleset_endpoint_test.go | 64 ++++++++++--------- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/internal/rules/provider/cloudblob/provider_test.go b/internal/rules/provider/cloudblob/provider_test.go index 2e8db962b..63b2ecc34 100644 --- a/internal/rules/provider/cloudblob/provider_test.go +++ b/internal/rules/provider/cloudblob/provider_test.go @@ -243,7 +243,8 @@ name: test rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test ` @@ -291,7 +292,8 @@ name: test rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test ` @@ -344,7 +346,8 @@ name: test rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test ` @@ -362,7 +365,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test ` @@ -438,7 +442,8 @@ name: test rules: - id: foo match: - path: /foo + routes: + - path: /foo execute: - authenticator: test ` @@ -453,7 +458,8 @@ name: test rules: - id: bar match: - path: /bar + routes: + - path: /bar execute: - authenticator: test ` @@ -468,7 +474,8 @@ name: test rules: - id: baz match: - path: /baz + routes: + - path: /baz execute: - authenticator: test ` diff --git a/internal/rules/provider/cloudblob/ruleset_endpoint_test.go b/internal/rules/provider/cloudblob/ruleset_endpoint_test.go index aac46b338..592894ee3 100644 --- a/internal/rules/provider/cloudblob/ruleset_endpoint_test.go +++ b/internal/rules/provider/cloudblob/ruleset_endpoint_test.go @@ -188,12 +188,12 @@ func TestFetchRuleSets(t *testing.T) { "rules": [{ "id": "foobar", "match": { - "path": "/foo/bar/api1", - "with": { - "scheme": "http", - "host_glob": "**", - "methods": ["GET", "POST"] - } + "routes": [ + { "path": "/foo/bar/api1" } + ], + "scheme": "http", + "hosts": [{ "type": "glob", "value": "**"}], + "methods": ["GET", "POST"] }, "execute": [ { "authenticator": "foobar" } @@ -207,13 +207,15 @@ name: test2 rules: - id: barfoo match: - path: /foo/bar/api2 - with: - scheme: http - host_glob: "**" - methods: - - GET - - POST + routes: + - path: /foo/bar/api2 + scheme: http + hosts: + - type: glob + value: "**" + methods: + - GET + - POST execute: - authenticator: barfoo ` @@ -264,12 +266,12 @@ rules: "rules": [{ "id": "foobar", "match": { - "path": "/foo/bar/api1", - "with": { - "scheme": "http", - "host_glob": "**", - "methods": ["GET", "POST"] - } + "routes": [ + { "path": "/foo/bar/api1" } + ], + "scheme": "http", + "hosts": [{ "type": "glob", "value": "**" }], + "methods": ["GET", "POST"] }, "execute": [ { "authenticator": "foobar" } @@ -282,12 +284,12 @@ rules: "rules": [{ "id": "barfoo", "match": { - "path": "/foo/bar/api2", - "with": { - "scheme": "http", - "host_glob": "**", - "methods": ["GET", "POST"] - } + "routes": [ + { "path": "/foo/bar/api2" } + ], + "scheme": "http", + "hosts": [{ "type": "glob", "value": "**"}], + "methods": ["GET", "POST"] }, "execute": [ { "authenticator": "barfoo" } @@ -382,12 +384,12 @@ rules: "rules": [{ "id": "foobar", "match": { - "path": "/foo/bar/api1", - "with": { - "scheme": "http", - "host_glob": "**", - "methods": ["GET", "POST"] - } + "routes": [ + { "path": "/foo/bar/api1" } + ], + "scheme": "http", + "hosts": [{ "type": "glob", "value": "**" }], + "methods": ["GET", "POST"] }, "execute": [ { "authenticator": "foobar" } From 8bf37b29da2f3797dcc18cf5d7f1e5b1d5938835 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:58:07 +0200 Subject: [PATCH 14/32] kubernetes provider tests updated to use the new configuration --- .../admissioncontroller/controller_test.go | 16 +-- .../kubernetes/api/v1alpha4/client_test.go | 41 ++++-- .../provider/kubernetes/provider_test.go | 134 ++++++++++-------- 3 files changed, 111 insertions(+), 80 deletions(-) diff --git a/internal/rules/provider/kubernetes/admissioncontroller/controller_test.go b/internal/rules/provider/kubernetes/admissioncontroller/controller_test.go index ce31272c8..7506d2318 100644 --- a/internal/rules/provider/kubernetes/admissioncontroller/controller_test.go +++ b/internal/rules/provider/kubernetes/admissioncontroller/controller_test.go @@ -272,11 +272,9 @@ func TestControllerLifecycle(t *testing.T) { { ID: "test", Matcher: config2.Matcher{ - Path: "/foo.bar", - With: &config2.MatcherConstraints{ - Scheme: "http", - Methods: []string{http.MethodGet}, - }, + Routes: []config2.Route{{Path: "/foo.bar"}}, + Scheme: "http", + Methods: []string{http.MethodGet}, }, Backend: &config2.Backend{ Host: "baz", @@ -367,11 +365,9 @@ func TestControllerLifecycle(t *testing.T) { { ID: "test", Matcher: config2.Matcher{ - Path: "/foo.bar", - With: &config2.MatcherConstraints{ - Scheme: "http", - Methods: []string{http.MethodGet}, - }, + Routes: []config2.Route{{Path: "/foo.bar"}}, + Scheme: "http", + Methods: []string{http.MethodGet}, }, Backend: &config2.Backend{ Host: "baz", diff --git a/internal/rules/provider/kubernetes/api/v1alpha4/client_test.go b/internal/rules/provider/kubernetes/api/v1alpha4/client_test.go index 948b8ab6b..be1b9059b 100644 --- a/internal/rules/provider/kubernetes/api/v1alpha4/client_test.go +++ b/internal/rules/provider/kubernetes/api/v1alpha4/client_test.go @@ -57,13 +57,21 @@ const response = `{ ], "id": "test:rule", "match": { - "path": "/foobar/:*", - "with": { - "scheme": "http", - "host_glob": "127.0.0.1:*", - "path_glob": "/foobar/foos*", - "methods": ["GET", "POST"] - } + "routes": [ + { + "path": "/foobar/*foo", + "path_params": [{ "name": "foo", "type": "glob", "value": "foos*" }] + }, + { + "path": "/foobar/baz" + } + ], + "scheme": "http", + "hosts": [ + {"type": "exact","value": "127.0.0.1"}, + {"type": "glob","value": "172.*.*.1"} + ], + "methods": ["GET", "POST"] }, "forward_to": { "host": "foo.bar", @@ -141,11 +149,20 @@ func verifyRuleSetList(t *testing.T, rls *RuleSetList) { rule := ruleSet.Spec.Rules[0] assert.Equal(t, "test:rule", rule.ID) - assert.Equal(t, "/foobar/:*", rule.Matcher.Path) - assert.Equal(t, "http", rule.Matcher.With.Scheme) - assert.Equal(t, "127.0.0.1:*", rule.Matcher.With.HostGlob) - assert.Equal(t, "/foobar/foos*", rule.Matcher.With.PathGlob) - assert.ElementsMatch(t, rule.Matcher.With.Methods, []string{"GET", "POST"}) + assert.Len(t, rule.Matcher.Routes, 2) + assert.Equal(t, "/foobar/*foo", rule.Matcher.Routes[0].Path) + assert.Len(t, rule.Matcher.Routes[0].PathParams, 1) + assert.Equal(t, "foo", rule.Matcher.Routes[0].PathParams[0].Name) + assert.Equal(t, "glob", rule.Matcher.Routes[0].PathParams[0].Type) + assert.Equal(t, "foos*", rule.Matcher.Routes[0].PathParams[0].Value) + assert.Equal(t, "/foobar/baz", rule.Matcher.Routes[1].Path) + assert.Equal(t, "http", rule.Matcher.Scheme) + assert.Len(t, rule.Matcher.Hosts, 2) + assert.Equal(t, "127.0.0.1", rule.Matcher.Hosts[0].Value) + assert.Equal(t, "exact", rule.Matcher.Hosts[0].Type) + assert.Equal(t, "172.*.*.1", rule.Matcher.Hosts[1].Value) + assert.Equal(t, "glob", rule.Matcher.Hosts[1].Type) + assert.ElementsMatch(t, rule.Matcher.Methods, []string{"GET", "POST"}) assert.Empty(t, rule.ErrorHandler) assert.Equal(t, "https://foo.bar/baz/bar?foo=bar", rule.Backend.CreateURL(&url.URL{ Scheme: "http", diff --git a/internal/rules/provider/kubernetes/provider_test.go b/internal/rules/provider/kubernetes/provider_test.go index 00d6b40d6..1765c2459 100644 --- a/internal/rules/provider/kubernetes/provider_test.go +++ b/internal/rules/provider/kubernetes/provider_test.go @@ -211,12 +211,10 @@ func (h *RuleSetResourceHandler) writeListResponse(t *testing.T, w http.Response { ID: "test", Matcher: config2.Matcher{ - Path: "/", - With: &config2.MatcherConstraints{ - Scheme: "http", - HostGlob: "foo.bar", - Methods: []string{http.MethodGet}, - }, + Routes: []config2.Route{{Path: "/"}}, + Scheme: "http", + Methods: []string{http.MethodGet}, + Hosts: []config2.HostMatcher{{Value: "foo.bar", Type: "glob"}}, }, Backend: &config2.Backend{ Host: "baz", @@ -370,11 +368,14 @@ func TestProviderLifecycle(t *testing.T) { rule := ruleSet.Rules[0] assert.Equal(t, "test", rule.ID) - assert.Equal(t, "http", rule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", rule.Matcher.With.HostGlob) - assert.Equal(t, "/", rule.Matcher.Path) - assert.Len(t, rule.Matcher.With.Methods, 1) - assert.Contains(t, rule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", rule.Matcher.Scheme) + assert.Len(t, rule.Matcher.Hosts, 1) + assert.Equal(t, "foo.bar", rule.Matcher.Hosts[0].Value) + assert.Equal(t, "glob", rule.Matcher.Hosts[0].Type) + assert.Len(t, rule.Matcher.Routes, 1) + assert.Equal(t, "/", rule.Matcher.Routes[0].Path) + assert.Len(t, rule.Matcher.Methods, 1) + assert.Contains(t, rule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", rule.Backend.Host) assert.Empty(t, rule.ErrorHandler) assert.Len(t, rule.Execute, 2) @@ -470,11 +471,14 @@ func TestProviderLifecycle(t *testing.T) { createdRule := ruleSet.Rules[0] assert.Equal(t, "test", createdRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", createdRule.Backend.Host) assert.Empty(t, createdRule.ErrorHandler) assert.Len(t, createdRule.Execute, 2) @@ -534,11 +538,14 @@ func TestProviderLifecycle(t *testing.T) { createdRule := ruleSet.Rules[0] assert.Equal(t, "test", createdRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", createdRule.Backend.Host) assert.Empty(t, createdRule.ErrorHandler) assert.Len(t, createdRule.Execute, 2) @@ -602,11 +609,14 @@ func TestProviderLifecycle(t *testing.T) { createdRule := ruleSet.Rules[0] assert.Equal(t, "test", createdRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", createdRule.Backend.Host) assert.Empty(t, createdRule.ErrorHandler) assert.Len(t, createdRule.Execute, 2) @@ -680,12 +690,10 @@ func TestProviderLifecycle(t *testing.T) { { ID: "test", Matcher: config2.Matcher{ - Path: "/", - With: &config2.MatcherConstraints{ - Scheme: "http", - HostGlob: "foo.bar", - Methods: []string{http.MethodGet}, - }, + Routes: []config2.Route{{Path: "/"}}, + Scheme: "http", + Methods: []string{http.MethodGet}, + Hosts: []config2.HostMatcher{{Value: "foo.bar", Type: "glob"}}, }, Backend: &config2.Backend{ Host: "bar", @@ -734,11 +742,14 @@ func TestProviderLifecycle(t *testing.T) { createdRule := ruleSet.Rules[0] assert.Equal(t, "test", createdRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", createdRule.Backend.Host) assert.Empty(t, createdRule.ErrorHandler) assert.Len(t, createdRule.Execute, 2) @@ -753,11 +764,14 @@ func TestProviderLifecycle(t *testing.T) { updatedRule := ruleSet.Rules[0] assert.Equal(t, "test", updatedRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "bar", updatedRule.Backend.Host) assert.Empty(t, updatedRule.ErrorHandler) assert.Len(t, updatedRule.Execute, 2) @@ -825,11 +839,14 @@ func TestProviderLifecycle(t *testing.T) { createdRule := ruleSet.Rules[0] assert.Equal(t, "test", createdRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", createdRule.Backend.Host) assert.Empty(t, createdRule.ErrorHandler) assert.Len(t, createdRule.Execute, 2) @@ -844,11 +861,14 @@ func TestProviderLifecycle(t *testing.T) { deleteRule := ruleSet.Rules[0] assert.Equal(t, "test", deleteRule.ID) - assert.Equal(t, "http", createdRule.Matcher.With.Scheme) - assert.Equal(t, "foo.bar", createdRule.Matcher.With.HostGlob) - assert.Equal(t, "/", createdRule.Matcher.Path) - assert.Len(t, createdRule.Matcher.With.Methods, 1) - assert.Contains(t, createdRule.Matcher.With.Methods, http.MethodGet) + assert.Equal(t, "http", createdRule.Matcher.Scheme) + assert.Len(t, createdRule.Matcher.Hosts, 1) + assert.Equal(t, "glob", createdRule.Matcher.Hosts[0].Type) + assert.Equal(t, "foo.bar", createdRule.Matcher.Hosts[0].Value) + assert.Len(t, createdRule.Matcher.Routes, 1) + assert.Equal(t, "/", createdRule.Matcher.Routes[0].Path) + assert.Len(t, createdRule.Matcher.Methods, 1) + assert.Contains(t, createdRule.Matcher.Methods, http.MethodGet) assert.Equal(t, "baz", deleteRule.Backend.Host) assert.Empty(t, deleteRule.ErrorHandler) assert.Len(t, deleteRule.Execute, 2) @@ -882,12 +902,10 @@ func TestProviderLifecycle(t *testing.T) { { ID: "test", Matcher: config2.Matcher{ - Path: "/", - With: &config2.MatcherConstraints{ - Scheme: "http", - HostGlob: "foo.bar", - Methods: []string{http.MethodGet}, - }, + Routes: []config2.Route{{Path: "/"}}, + Scheme: "http", + Methods: []string{http.MethodGet}, + Hosts: []config2.HostMatcher{{Value: "foo.bar", Type: "glob"}}, }, Backend: &config2.Backend{ Host: "bar", From 98901a0075df2bdec4dff2ca52027cd06ce81e7d Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:58:47 +0200 Subject: [PATCH 15/32] configuration of rules used for validation tests updated --- .../invalid-ruleset-for-proxy-usage.yaml | 12 +++++++----- cmd/validate/test_data/valid-ruleset.yaml | 16 +++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/validate/test_data/invalid-ruleset-for-proxy-usage.yaml b/cmd/validate/test_data/invalid-ruleset-for-proxy-usage.yaml index ca8616f88..c5d22614e 100644 --- a/cmd/validate/test_data/invalid-ruleset-for-proxy-usage.yaml +++ b/cmd/validate/test_data/invalid-ruleset-for-proxy-usage.yaml @@ -3,11 +3,13 @@ name: test-rule-set rules: - id: rule:foo match: - path: /** - with: - scheme: http - host_glob: foo.bar - methods: [ GET, POST ] + routes: + - path: /** + scheme: http + hosts: + - type: glob + value: foo.bar + methods: [ GET, POST ] execute: - authenticator: unauthorized_authenticator - authenticator: jwt_authenticator1 diff --git a/cmd/validate/test_data/valid-ruleset.yaml b/cmd/validate/test_data/valid-ruleset.yaml index c13b97839..1f5a0f1d5 100644 --- a/cmd/validate/test_data/valid-ruleset.yaml +++ b/cmd/validate/test_data/valid-ruleset.yaml @@ -3,14 +3,16 @@ name: test-rule-set rules: - id: rule:foo match: - path: /** + routes: + - path: /** backtracking_enabled: true - with: - scheme: http - host_glob: foo.bar - methods: - - POST - - PUT + scheme: http + hosts: + - type: glob + value: foo.bar + methods: + - POST + - PUT forward_to: host: bar.foo rewrite: From 5a569ad09d5f20e0e9fd67da6693f87a153f2287 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 00:59:02 +0200 Subject: [PATCH 16/32] example rules updated --- example_rules.yaml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/example_rules.yaml b/example_rules.yaml index 712a9851f..159853bd4 100644 --- a/example_rules.yaml +++ b/example_rules.yaml @@ -3,14 +3,20 @@ name: test-rule-set rules: - id: rule:foo match: - path: /** + routes: + - path: /foo/:bar/** + path_params: + - name: bar + type: glob + value: "*baz" backtracking_enabled: false - with: - methods: + methods: - GET - POST - host_glob: foo.bar - scheme: http + hosts: + - type: exact + value: foo.bar + scheme: http forward_to: host: bar.foo execute: From 4387b604121bfa720fdd2dfe7c656ef77c866f8c Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 01:19:59 +0200 Subject: [PATCH 17/32] ruleset crd updated --- charts/heimdall/crds/ruleset.yaml | 149 +++++++++++++++++++----------- 1 file changed, 94 insertions(+), 55 deletions(-) diff --git a/charts/heimdall/crds/ruleset.yaml b/charts/heimdall/crds/ruleset.yaml index 87b3552d9..f7fd7bfa7 100644 --- a/charts/heimdall/crds/ruleset.yaml +++ b/charts/heimdall/crds/ruleset.yaml @@ -51,7 +51,7 @@ spec: type: array minItems: 1 items: - description: A himedall rule defining the pipeline mechanisms + description: A heimdall rule defining the pipeline mechanisms type: object required: - id @@ -75,66 +75,105 @@ spec: description: How to match the rule type: object required: - - path + - routes properties: - path: - description: The path to match - type: string - maxLength: 256 + routes: + description: Routes to match + type: array + minItems: 1 + items: + description: Definition of a single route + type: object + required: + - path + properties: + path: + description: The path to match + type: string + maxLength: 512 + path_params: + description: Optional matching definitions for the captured wildcard + type: array + items: + description: Matching definition for a single wildcard + type: object + required: + - name + - type + - value + properties: + name: + description: The name of a wildcard + type: string + maxLength: 64 + type: + description: The type of the matching expression + type: string + maxLength: 5 + enum: + - "exact" + - "glob" + - "regex" + value: + description: The actual matching expression + type: string + maxLength: 256 backtracking_enabled: description: Wither this rule allows backtracking. Defaults to the value inherited from the default rule type: boolean - with: - description: Additional constraints during request matching - type: object - properties: - methods: - description: The HTTP methods to match - type: array - minItems: 1 - items: + methods: + description: The HTTP methods to match + type: array + minItems: 1 + items: + type: string + maxLength: 16 + enum: + - "CONNECT" + - "!CONNECT" + - "DELETE" + - "!DELETE" + - "GET" + - "!GET" + - "HEAD" + - "!HEAD" + - "OPTIONS" + - "!OPTIONS" + - "PATCH" + - "!PATCH" + - "POST" + - "!POST" + - "PUT" + - "!PUT" + - "TRACE" + - "!TRACE" + - "ALL" + scheme: + description: The HTTP scheme, which should be matched. If not set, http and https are matched + type: string + maxLength: 5 + hosts: + description: Optional expressions to match the host if required. If not set, all hosts are matched. + type: array + items: + description: Expression to match a host + type: object + required: + - type + - value + properties: + type: + description: The type of the host matching expression type: string - maxLength: 16 + maxLength: 5 enum: - - "CONNECT" - - "!CONNECT" - - "DELETE" - - "!DELETE" - - "GET" - - "!GET" - - "HEAD" - - "!HEAD" - - "OPTIONS" - - "!OPTIONS" - - "PATCH" - - "!PATCH" - - "POST" - - "!POST" - - "PUT" - - "!PUT" - - "TRACE" - - "!TRACE" - - "ALL" - scheme: - description: The HTTP scheme, which should be matched. If not set, http and https are matched - type: string - maxLength: 5 - host_glob: - description: Glob expression to match the host if required. If not set, all hosts are matched. Mutually exclusive with 'host_regex'. - type: string - maxLength: 512 - host_regex: - description: Regular expression to match the host if required. If not set, all hosts are matched. Mutually exclusive with 'host_glob'. - type: string - maxLength: 512 - path_glob: - description: Additional glob expression the matched path should be matched against. Mutual exclusive with 'regex'. - type: string - maxLength: 256 - path_regex: - description: Additional regular expression the matched path should be matched against. Mutual exclusive with 'glob' - type: string - maxLength: 256 + - "exact" + - "glob" + - "regex" + value: + description: The actual host matching expression + type: string + maxLength: 256 forward_to: description: Where to forward the request to. Required only if heimdall is used in proxy operation mode. type: object From 8a08054a1b0a2e08d90379cf277cc34b9080b3b7 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 01:22:56 +0200 Subject: [PATCH 18/32] rules file for docker-compose quickstart example updated --- .../quickstarts/upstream-rules.yaml | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/examples/docker-compose/quickstarts/upstream-rules.yaml b/examples/docker-compose/quickstarts/upstream-rules.yaml index 1c3380372..739fe8b75 100644 --- a/examples/docker-compose/quickstarts/upstream-rules.yaml +++ b/examples/docker-compose/quickstarts/upstream-rules.yaml @@ -2,9 +2,9 @@ version: "1alpha4" rules: - id: demo:public match: - path: /public - with: - methods: [ GET, POST ] + routes: + - path: /public + methods: [ GET, POST ] forward_to: host: upstream:8081 execute: @@ -13,10 +13,13 @@ rules: - id: demo:protected match: - path: /:user - with: - path_regex: ^/(user|admin) - methods: [ GET, POST ] + routes: + - path: /:user + path_params: + - type: regex + name: user + value: (user|admin) + methods: [ GET, POST ] forward_to: host: upstream:8081 execute: From af01968942c5b6b3fe85f896e4c1709c666d9903 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 01:25:10 +0200 Subject: [PATCH 19/32] rules in kubernetes quickstart examples updated --- examples/kubernetes/quickstarts/demo-app/base/rules.yaml | 9 ++++++--- .../quickstarts/proxy-demo/heimdall-rules.yaml | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/examples/kubernetes/quickstarts/demo-app/base/rules.yaml b/examples/kubernetes/quickstarts/demo-app/base/rules.yaml index ad08590ed..a27fa1848 100644 --- a/examples/kubernetes/quickstarts/demo-app/base/rules.yaml +++ b/examples/kubernetes/quickstarts/demo-app/base/rules.yaml @@ -9,14 +9,16 @@ spec: rules: - id: public-access match: - path: /pub/** + routes: + - path: /pub/** forward_to: # only required for proxy operation mode host: echo-app.quickstarts.svc.cluster.local:8080 execute: - authorizer: allow_all_requests - id: anonymous-access match: - path: /anon/** + routes: + - path: /anon/** forward_to: # only required for proxy operation mode host: echo-app.quickstarts.svc.cluster.local:8080 execute: @@ -24,7 +26,8 @@ spec: - finalizer: create_jwt - id: redirect match: - path: /redir/** + routes: + - path: /redir/** forward_to: # only required for proxy operation mode host: echo-app.quickstarts.svc.cluster.local:8080 execute: diff --git a/examples/kubernetes/quickstarts/proxy-demo/heimdall-rules.yaml b/examples/kubernetes/quickstarts/proxy-demo/heimdall-rules.yaml index 6c0eae814..44fa72fd2 100644 --- a/examples/kubernetes/quickstarts/proxy-demo/heimdall-rules.yaml +++ b/examples/kubernetes/quickstarts/proxy-demo/heimdall-rules.yaml @@ -12,7 +12,8 @@ data: rules: - id: public-access match: - path: /pub/** + routes: + - path: /pub/** forward_to: host: localhost:8080 rewrite: @@ -22,7 +23,8 @@ data: - id: anonymous-access match: - path: /anon/** + routes: + - path: /anon/** forward_to: host: localhost:8080 rewrite: @@ -33,7 +35,8 @@ data: - id: redirect match: - path: /redir/** + routes: + - path: /redir/** forward_to: host: localhost:8080 rewrite: From 04297e313193e0c028cf5f90d6ed366733c82a46 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 01:27:24 +0200 Subject: [PATCH 20/32] forcetypeassert disabled for tests --- .golangci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yaml b/.golangci.yaml index fba8c5ad7..05d9f06ef 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -34,6 +34,7 @@ issues: - canonicalheader - mnd - err113 + - forcetypeassert linters-settings: exhaustive: From f75baec98f9f7d93da95727acfad40ff0e7edc56 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 01:41:52 +0200 Subject: [PATCH 21/32] linter warnings resolved --- internal/rules/config/matcher.go | 25 +++++++++++++------------ internal/rules/route_matcher.go | 12 ++++++------ internal/rules/rule_factory_impl.go | 15 ++++++++------- internal/rules/rule_impl.go | 2 +- internal/rules/typed_matcher.go | 2 +- internal/rules/typed_matcher_test.go | 3 +-- internal/x/radixtree/tree.go | 5 +++-- internal/x/radixtree/tree_test.go | 1 + 8 files changed, 34 insertions(+), 31 deletions(-) diff --git a/internal/rules/config/matcher.go b/internal/rules/config/matcher.go index 4c042511d..bdb5cda68 100644 --- a/internal/rules/config/matcher.go +++ b/internal/rules/config/matcher.go @@ -19,16 +19,16 @@ package config import "slices" type Matcher struct { - Routes []Route `json:"routes" yaml:"routes" validate:"required,dive"` //nolint:lll - BacktrackingEnabled *bool `json:"backtracking_enabled" yaml:"backtracking_enabled"` //nolint:lll - Scheme string `json:"scheme" yaml:"scheme" validate:"omitempty,oneof=http https"` //nolint:lll - Methods []string `json:"methods" yaml:"methods" validate:"omitempty,dive,required"` //nolint:lll - Hosts []HostMatcher `json:"hosts" yaml:"hosts" validate:"omitempty,dive,required"` //nolint:lll + Routes []Route `json:"routes" yaml:"routes" validate:"required,dive"` //nolint:lll,tagalign + BacktrackingEnabled *bool `json:"backtracking_enabled" yaml:"backtracking_enabled"` //nolint:lll,tagalign + Scheme string `json:"scheme" yaml:"scheme" validate:"omitempty,oneof=http https"` //nolint:lll,tagalign + Methods []string `json:"methods" yaml:"methods" validate:"omitempty,dive,required"` //nolint:lll,tagalign + Hosts []HostMatcher `json:"hosts" yaml:"hosts" validate:"omitempty,dive,required"` //nolint:lll,tagalign } type Route struct { - Path string `json:"path" yaml:"path" validate:"required"` - PathParams []ParameterMatcher `json:"path_params" yaml:"path_params" validate:"omitempty,dive,required"` + Path string `json:"path" yaml:"path" validate:"required"` //nolint:lll,tagalign + PathParams []ParameterMatcher `json:"path_params" yaml:"path_params" validate:"omitempty,dive,required"` //nolint:lll,tagalign } func (r *Route) DeepCopyInto(out *Route) { @@ -38,18 +38,19 @@ func (r *Route) DeepCopyInto(out *Route) { } type ParameterMatcher struct { - Name string `json:"name" yaml:"name" validate:"required"` - Value string `json:"value" yaml:"value" validate:"required"` - Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` + Name string `json:"name" yaml:"name" validate:"required"` //nolint:tagalign + Value string `json:"value" yaml:"value" validate:"required"` //nolint:tagalign + Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` //nolint:tagalign } type HostMatcher struct { - Value string `json:"value" yaml:"value" validate:"required"` - Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` + Value string `json:"value" yaml:"value" validate:"required"` //nolint:tagalign + Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` //nolint:tagalign } func (m *Matcher) DeepCopyInto(out *Matcher) { var withBacktracking *bool + if m.BacktrackingEnabled != nil { value := *m.BacktrackingEnabled withBacktracking = &value diff --git a/internal/rules/route_matcher.go b/internal/rules/route_matcher.go index 6e721a47a..156367d95 100644 --- a/internal/rules/route_matcher.go +++ b/internal/rules/route_matcher.go @@ -28,9 +28,6 @@ import ( "github.com/dadrus/heimdall/internal/x/slicex" ) -// nolint: gochecknoglobals -var spaceReplacer = strings.NewReplacer("\t", "", "\n", "", "\v", "", "\f", "", "\r", "", " ", "") - var ( ErrRequestSchemeMismatch = errors.New("request scheme mismatch") ErrRequestMethodMismatch = errors.New("request method mismatch") @@ -164,7 +161,7 @@ func createHostMatcher(hosts []config.HostMatcher) (RouteMatcher, error) { case "regex": tm, err = newRegexMatcher(host.Value) case "exact": - tm, err = newExactMatcher(host.Value) + tm = newExactMatcher(host.Value) default: return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, "unsupported host matching expression type '%s' at index %d", host.Type, idx) @@ -181,7 +178,10 @@ func createHostMatcher(hosts []config.HostMatcher) (RouteMatcher, error) { return matchers, nil } -func createPathParamsMatcher(params []config.ParameterMatcher, esh config.EncodedSlashesHandling) (RouteMatcher, error) { +func createPathParamsMatcher( + params []config.ParameterMatcher, + esh config.EncodedSlashesHandling, +) (RouteMatcher, error) { matchers := make(compositeMatcher, len(params)) for idx, param := range params { @@ -196,7 +196,7 @@ func createPathParamsMatcher(params []config.ParameterMatcher, esh config.Encode case "regex": tm, err = newRegexMatcher(param.Value) case "exact": - tm, err = newExactMatcher(param.Value) + tm = newExactMatcher(param.Value) default: return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, "unsupported path parameter expression type '%s' for parameter '%s' at index %d", diff --git a/internal/rules/rule_factory_impl.go b/internal/rules/rule_factory_impl.go index cdeb61efd..d01adba67 100644 --- a/internal/rules/rule_factory_impl.go +++ b/internal/rules/rule_factory_impl.go @@ -148,13 +148,13 @@ func (f *ruleFactory) createExecutePipeline( func (f *ruleFactory) DefaultRule() rule.Rule { return f.defaultRule } func (f *ruleFactory) HasDefaultRule() bool { return f.hasDefaultRule } +// nolint:cyclop,funlen func (f *ruleFactory) CreateRule(version, srcID string, ruleConfig config2.Rule) (rule.Rule, error) { if f.mode == config.ProxyMode && ruleConfig.Backend == nil { return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, "proxy mode requires forward_to definition") } - slashesHandling := x.IfThenElse( - len(ruleConfig.EncodedSlashesHandling) != 0, + slashesHandling := x.IfThenElse(len(ruleConfig.EncodedSlashesHandling) != 0, ruleConfig.EncodedSlashesHandling, config2.EncodedSlashesOff, ) @@ -223,11 +223,12 @@ func (f *ruleFactory) CreateRule(version, srcID string, ruleConfig config2.Rule) CausedBy(err) } - rul.routes = append(rul.routes, &routeImpl{ - rule: rul, - path: rc.Path, - matcher: compositeMatcher{sm, mm, hm, ppm}, - }) + rul.routes = append(rul.routes, + &routeImpl{ + rule: rul, + path: rc.Path, + matcher: compositeMatcher{sm, mm, hm, ppm}, + }) } return rul, nil diff --git a/internal/rules/rule_impl.go b/internal/rules/rule_impl.go index a0f2a0f2e..6bbe34ec5 100644 --- a/internal/rules/rule_impl.go +++ b/internal/rules/rule_impl.go @@ -112,7 +112,7 @@ func (r *ruleImpl) Routes() []rule.Route { return r.routes } func (r *ruleImpl) EqualTo(other rule.Rule) bool { return r.ID() == other.ID() && r.SrcID() == other.SrcID() && - bytes.Equal(r.hash, other.(*ruleImpl).hash) + bytes.Equal(r.hash, other.(*ruleImpl).hash) // nolint: forcetypeassert } func (r *ruleImpl) AllowsBacktracking() bool { return r.allowsBacktracking } diff --git a/internal/rules/typed_matcher.go b/internal/rules/typed_matcher.go index b8b696db7..11cda713e 100644 --- a/internal/rules/typed_matcher.go +++ b/internal/rules/typed_matcher.go @@ -82,4 +82,4 @@ func newRegexMatcher(pattern string) (typedMatcher, error) { return ®expMatcher{compiled: compiled}, nil } -func newExactMatcher(value string) (typedMatcher, error) { return &exactMatcher{value: value}, nil } +func newExactMatcher(value string) typedMatcher { return &exactMatcher{value: value} } diff --git a/internal/rules/typed_matcher_test.go b/internal/rules/typed_matcher_test.go index 4cc91b18e..a9d05a826 100644 --- a/internal/rules/typed_matcher_test.go +++ b/internal/rules/typed_matcher_test.go @@ -164,8 +164,7 @@ func TestExactMatcher(t *testing.T) { {uc: "doesn't match", expression: "foo", toMatch: "bar"}, } { t.Run(tc.uc, func(t *testing.T) { - matcher, err := newExactMatcher(tc.expression) - require.NoError(t, err) + matcher := newExactMatcher(tc.expression) matches := matcher.match(tc.toMatch) assert.Equal(t, tc.matches, matches) diff --git a/internal/x/radixtree/tree.go b/internal/x/radixtree/tree.go index 2324b13f1..bea7e7d37 100644 --- a/internal/x/radixtree/tree.go +++ b/internal/x/radixtree/tree.go @@ -381,8 +381,9 @@ func (n *Tree[V]) findNode(path string, captures []string, matcher LookupMatcher nextToken := path[nextSeparator:] if len(thisToken) > 0 { // Don't match on empty tokens. - tmp := append(captures, thisToken) - found, idx, tmp, backtrack = n.wildcardChild.findNode(nextToken, tmp, matcher) + var tmp []string + + found, idx, tmp, backtrack = n.wildcardChild.findNode(nextToken, append(captures, thisToken), matcher) if found != nil { return found, idx, tmp, backtrack } else if !backtrack { diff --git a/internal/x/radixtree/tree_test.go b/internal/x/radixtree/tree_test.go index 8cc4f6198..2b34b95b1 100644 --- a/internal/x/radixtree/tree_test.go +++ b/internal/x/radixtree/tree_test.go @@ -149,6 +149,7 @@ func TestTreeSearch(t *testing.T) { if expParams == nil { expParams = map[string]string{} } + assert.Equal(t, expParams, entry.Parameters, "Path %s expected parameters are %v, saw %v", tc.path, tc.expParams, entry.Parameters) }) } From 7ceaf79d74d0f4bb1d8e7686302881ca282fff08 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 02:10:12 +0200 Subject: [PATCH 22/32] small updates to the readme for docker-compose base quickstarts --- examples/docker-compose/quickstarts/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/docker-compose/quickstarts/README.md b/examples/docker-compose/quickstarts/README.md index 9967ede62..f460b42f4 100644 --- a/examples/docker-compose/quickstarts/README.md +++ b/examples/docker-compose/quickstarts/README.md @@ -21,8 +21,8 @@ In that setup heimdall is not integrated with any other reverse proxy. curl -v http://127.0.0.1:9090/public curl -v http://127.0.0.1:9090/private curl -v http://127.0.0.1:9090/user - curl -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiIzZmFmNDkxOS0wZjUwLTQ3NGItOGExMy0yOTYzMjEzNThlOTMiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJ1c2VyIiwic3ViIjoiMiJ9.W5xCpwsFShS0RpOtrm9vrV2dN6K8pRr5gQnt0kluzLE6oNWFzf7Oot-0YLCPa64Z3XPd7cfGcBiSjrzKZSAj4g" 127.0.0.1:9090/user - curl -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiI0NjExZDM5Yy00MzI1LTRhMWYtYjdkOC1iMmYxMTE3NDEyYzAiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJhZG1pbiIsInN1YiI6IjEifQ.mZZ_UqC8RVzEKBPZbPs4eP-MkXLK22Q27ZJ34UwJiioFdaYXqYJ4ZsatP0TbpKeNyF83mkrrCGL_pWLFTho7Gg" 127.0.0.1:9090/admin + curl -v -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiIzZmFmNDkxOS0wZjUwLTQ3NGItOGExMy0yOTYzMjEzNThlOTMiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJ1c2VyIiwic3ViIjoiMiJ9.W5xCpwsFShS0RpOtrm9vrV2dN6K8pRr5gQnt0kluzLE6oNWFzf7Oot-0YLCPa64Z3XPd7cfGcBiSjrzKZSAj4g" 127.0.0.1:9090/user + curl -v -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiI0NjExZDM5Yy00MzI1LTRhMWYtYjdkOC1iMmYxMTE3NDEyYzAiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJhZG1pbiIsInN1YiI6IjEifQ.mZZ_UqC8RVzEKBPZbPs4eP-MkXLK22Q27ZJ34UwJiioFdaYXqYJ4ZsatP0TbpKeNyF83mkrrCGL_pWLFTho7Gg" 127.0.0.1:9090/admin ``` Check the responses @@ -43,8 +43,8 @@ In that setup heimdall is integrated with Traefik. All requests are sent to trae curl -v http://127.0.0.1:9090/public curl -v http://127.0.0.1:9090/private curl -v http://127.0.0.1:9090/user - curl -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiIzZmFmNDkxOS0wZjUwLTQ3NGItOGExMy0yOTYzMjEzNThlOTMiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJ1c2VyIiwic3ViIjoiMiJ9.W5xCpwsFShS0RpOtrm9vrV2dN6K8pRr5gQnt0kluzLE6oNWFzf7Oot-0YLCPa64Z3XPd7cfGcBiSjrzKZSAj4g" 127.0.0.1:9090/user - curl -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiI0NjExZDM5Yy00MzI1LTRhMWYtYjdkOC1iMmYxMTE3NDEyYzAiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJhZG1pbiIsInN1YiI6IjEifQ.mZZ_UqC8RVzEKBPZbPs4eP-MkXLK22Q27ZJ34UwJiioFdaYXqYJ4ZsatP0TbpKeNyF83mkrrCGL_pWLFTho7Gg" 127.0.0.1:9090/admin + curl -v -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiIzZmFmNDkxOS0wZjUwLTQ3NGItOGExMy0yOTYzMjEzNThlOTMiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJ1c2VyIiwic3ViIjoiMiJ9.W5xCpwsFShS0RpOtrm9vrV2dN6K8pRr5gQnt0kluzLE6oNWFzf7Oot-0YLCPa64Z3XPd7cfGcBiSjrzKZSAj4g" 127.0.0.1:9090/user + curl -v -H "Authorization: Bearer eyJhbGciOiJFUzI1NiIsImtpZCI6ImtleS0xIiwidHlwIjoiSldUIn0.eyJleHAiOjIwMjUxMDA3NTEsImlhdCI6MTcwOTc0MDc1MSwiaXNzIjoiZGVtb19pc3N1ZXIiLCJqdGkiOiI0NjExZDM5Yy00MzI1LTRhMWYtYjdkOC1iMmYxMTE3NDEyYzAiLCJuYmYiOjE3MDk3NDA3NTEsInJvbGUiOiJhZG1pbiIsInN1YiI6IjEifQ.mZZ_UqC8RVzEKBPZbPs4eP-MkXLK22Q27ZJ34UwJiioFdaYXqYJ4ZsatP0TbpKeNyF83mkrrCGL_pWLFTho7Gg" 127.0.0.1:9090/admin ``` Check the responses From 5cf2b7966d4c8d677836736965f10faabb3a8e2a Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 02:11:06 +0200 Subject: [PATCH 23/32] made debug log related to the new matching functionality more usable --- internal/rules/route_matcher.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/rules/route_matcher.go b/internal/rules/route_matcher.go index 156367d95..196da7b9a 100644 --- a/internal/rules/route_matcher.go +++ b/internal/rules/route_matcher.go @@ -55,7 +55,7 @@ type schemeMatcher string func (s schemeMatcher) Matches(request *heimdall.Request, _, _ []string) error { if len(s) != 0 && string(s) != request.URL.Scheme { - return errorchain.NewWithMessagef(ErrRequestSchemeMismatch, "expected %s, got %s", s, request.URL.Scheme) + return errorchain.NewWithMessagef(ErrRequestSchemeMismatch, "expected '%s', got '%s'", s, request.URL.Scheme) } return nil @@ -69,7 +69,7 @@ func (m methodMatcher) Matches(request *heimdall.Request, _, _ []string) error { } if !slices.Contains(m, request.Method) { - return errorchain.NewWithMessagef(ErrRequestMethodMismatch, "%s is not expected", request.Method) + return errorchain.NewWithMessagef(ErrRequestMethodMismatch, "'%s' is not expected", request.Method) } return nil @@ -81,7 +81,7 @@ type hostMatcher struct { func (m *hostMatcher) Matches(request *heimdall.Request, _, _ []string) error { if !m.match(request.URL.Host) { - return errorchain.NewWithMessagef(ErrRequestHostMismatch, "%s is not expected", request.URL.Host) + return errorchain.NewWithMessagef(ErrRequestHostMismatch, "'%s' is not expected", request.URL.Host) } return nil @@ -97,7 +97,7 @@ type pathParamMatcher struct { func (m *pathParamMatcher) Matches(request *heimdall.Request, keys, values []string) error { idx := slices.Index(keys, m.name) if idx == -1 { - return errorchain.NewWithMessagef(ErrRequestPathMismatch, "path parameter %s is not expected", m.name) + return errorchain.NewWithMessagef(ErrRequestPathMismatch, "path parameter '%s' is not expected", m.name) } value := values[idx] @@ -105,13 +105,13 @@ func (m *pathParamMatcher) Matches(request *heimdall.Request, keys, values []str if len(request.URL.RawPath) != 0 && m.slashHandling == config.EncodedSlashesOff && strings.Contains(request.URL.RawPath, "%2F") { - return errorchain.NewWithMessagef(ErrRequestPathMismatch, - "value for path parameter %s contains encoded slashes which are not allowed", keys[idx]) + return errorchain.NewWithMessage(ErrRequestPathMismatch, + "request path contains encoded slashes which are not allowed") } if !m.match(value) { return errorchain.NewWithMessagef(ErrRequestPathMismatch, - "captured values for path parameter %s is not expected", value) + "captured value '%s' for path parameter '%s' is not expected", value, m.name) } return nil From 3763bb789fab6a3e9592786c6e2d030fa76d735b Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 03:21:07 +0200 Subject: [PATCH 24/32] not allowing usage of * as name for the wildcard --- internal/rules/config/matcher.go | 2 +- internal/rules/config/parser_test.go | 50 ++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/internal/rules/config/matcher.go b/internal/rules/config/matcher.go index bdb5cda68..5c2f80938 100644 --- a/internal/rules/config/matcher.go +++ b/internal/rules/config/matcher.go @@ -38,7 +38,7 @@ func (r *Route) DeepCopyInto(out *Route) { } type ParameterMatcher struct { - Name string `json:"name" yaml:"name" validate:"required"` //nolint:tagalign + Name string `json:"name" yaml:"name" validate:"required,ne=*"` //nolint:tagalign Value string `json:"value" yaml:"value" validate:"required"` //nolint:tagalign Type string `json:"type" yaml:"type" validate:"required,oneof=exact glob regex"` //nolint:tagalign } diff --git a/internal/rules/config/parser_test.go b/internal/rules/config/parser_test.go index d428405db..4e2995f26 100644 --- a/internal/rules/config/parser_test.go +++ b/internal/rules/config/parser_test.go @@ -114,7 +114,7 @@ func TestParseRules(t *testing.T) { { "id": "foo", "match": { - "hosts":[{ "value": "*", "type": "glob" }] + "hosts":[{ "value": "*.foo.bar", "type": "glob" }] }, "execute": [{"authenticator":"test"}]}] }`), @@ -123,7 +123,7 @@ func TestParseRules(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - require.Contains(t, err.Error(), "'rules'[0].'match'.'routes' is a required field") + require.ErrorContains(t, err, "'rules'[0].'match'.'routes' is a required field") require.Nil(t, ruleSet) }, }, @@ -214,7 +214,9 @@ func TestParseRules(t *testing.T) { "match":{ "routes": [{ "path":"/foo/bar" }], "methods": ["ALL"], - "backtracking_enabled": true + "backtracking_enabled": true, + "hosts":[{ "value": "*.foo.bar", "type": "glob" }], + "scheme": "https" }, "execute": [{"authenticator":"test"}] }] @@ -249,7 +251,11 @@ rules: - id: bar match: routes: - - path: /foo/bar + - path: /foo/:bar + path_params: + - name: bar + type: glob + value: "*foo" methods: - GET forward_to: @@ -270,7 +276,8 @@ rules: require.NotNil(t, rul) assert.Equal(t, "bar", rul.ID) assert.Len(t, rul.Matcher.Routes, 1) - assert.Equal(t, "/foo/bar", rul.Matcher.Routes[0].Path) + assert.Equal(t, "/foo/:bar", rul.Matcher.Routes[0].Path) + assert.Len(t, rul.Matcher.Routes[0].PathParams, 1) assert.ElementsMatch(t, []string{"GET"}, rul.Matcher.Methods) assert.Equal(t, "test", rul.Backend.Host) assert.Equal(t, EncodedSlashesOnNoDecode, rul.EncodedSlashesHandling) @@ -279,7 +286,34 @@ rules: }, }, { - uc: "YAML content type and validation error", + uc: "YAML content type and validation error due to missing properties", + contentType: "application/yaml", + content: []byte(` +version: "1" +name: foo +rules: +- id: bar + match: + routes: + - path: /foo/:* + path_params: + - name: "*" + type: glob + value: "*foo" + execute: + - authenticator: test +`), + assert: func(t *testing.T, err error, ruleSet *RuleSet) { + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "'rules'[0].'match'.'routes'[0].'path_params'[0].'name' should not be equal to *") + require.Nil(t, ruleSet) + }, + }, + { + uc: "YAML content type and validation error due bad path params name", contentType: "application/yaml", content: []byte(` version: "1" @@ -291,7 +325,11 @@ rules: assert: func(t *testing.T, err error, ruleSet *RuleSet) { t.Helper() + require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "'rules'[0].'allow_encoded_slashes' must be one of [off on no_decode]") + require.ErrorContains(t, err, "'rules'[0].'match' is a required field") + require.ErrorContains(t, err, "'rules'[0].'execute' must contain more than 0 items") require.Nil(t, ruleSet) }, }, From 407b8d94ae3a02a65653e06fb08952aff0039143 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 03:34:31 +0200 Subject: [PATCH 25/32] forgotten test updated --- internal/rules/provider/httpendpoint/ruleset_endpoint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go b/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go index 320be3886..e13908533 100644 --- a/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go +++ b/internal/rules/provider/httpendpoint/ruleset_endpoint_test.go @@ -276,7 +276,7 @@ rules: "id": "foo", "match": { "routes": [ - { "path": "/foo/bar/:*", "path_params": [{ "name": "*", "type":"glob", "value":"{*.ico,*.js}" }] } + { "path": "/foo/bar/:baz", "path_params": [{ "name": "baz", "type":"glob", "value":"{*.ico,*.js}" }] } ], "methods": [ "GET" ], "hosts": [{ "value":"moobar.local:9090", "type": "exact"}], From 17c536e8cef004f191565297a9e55f336f4cb394 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 03:40:42 +0200 Subject: [PATCH 26/32] typos in readmes fixed --- examples/README.md | 2 +- examples/docker-compose/quickstarts/README.md | 2 +- examples/kubernetes/README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/README.md b/examples/README.md index a4d24adc0..014474768 100644 --- a/examples/README.md +++ b/examples/README.md @@ -8,4 +8,4 @@ To be able to run the docker compose examples, you'll need Docker and docker-com To be able to run the Kubernetes based examples, you'll need just, kubectl, kustomize, helm and a k8s cluster. Latter can also be created locally using kind. The examples are indeed using it. -**Note:** The main branch may have breaking changes (see pending release PRs for details under https://github.com/dadrus/heimdall/pulls) which would make the usage of the referenced heimdall images impossible (even though the configuration files and rules reflect the latest changes). In such situations you'll have to use the `dev` image, build a heimdall image by yourself and update the setups to use it, ot switch to a tagged (released) version. \ No newline at end of file +**Note:** The main branch may have breaking changes (see pending release PRs for details under https://github.com/dadrus/heimdall/pulls) which would make the usage of the referenced heimdall images impossible (even though the configuration files and rules reflect the latest changes). In such situations you'll have to use the `dev` image, build a heimdall image by yourself and update the setups to use it, or switch to a tagged (released) version. \ No newline at end of file diff --git a/examples/docker-compose/quickstarts/README.md b/examples/docker-compose/quickstarts/README.md index f460b42f4..a8668531f 100644 --- a/examples/docker-compose/quickstarts/README.md +++ b/examples/docker-compose/quickstarts/README.md @@ -2,7 +2,7 @@ This directory contains examples described in the getting started section of the documentation. The demonstration of the decision operation mode is done via integration with some reverse proxies. -**Note:** The main branch may have breaking changes (see pending release PRs for details under https://github.com/dadrus/heimdall/pulls) which would make the usage of the referenced heimdall images impossible (even though the configuration files and rules reflect the latest changes). In such situations you'll have to use the `dev` image, build a heimdall image by yourself and update the setups to use it, ot switch to a tagged (released) version. +**Note:** The main branch may have breaking changes (see pending release PRs for details under https://github.com/dadrus/heimdall/pulls) which would make the usage of the referenced heimdall images impossible (even though the configuration files and rules reflect the latest changes). In such situations you'll have to use the `dev` image, build a heimdall image by yourself and update the setups to use it, or switch to a tagged (released) version. # Proxy Mode Quickstart diff --git a/examples/kubernetes/README.md b/examples/kubernetes/README.md index bb03e985d..9d953f418 100644 --- a/examples/kubernetes/README.md +++ b/examples/kubernetes/README.md @@ -2,7 +2,7 @@ This directory contains working examples described in the getting started, as well as in the integration guides of the documentation. The demonstration of the decision operation mode is done via integration with the corresponding ingress controllers. As of now, these are [Contour](https://projectcontour.io), the [NGINX Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/) and [HAProxy Ingress Controller](https://haproxy-ingress.github.io/). -**Note:** The main branch may have breaking changes (see pending release PRs for details under https://github.com/dadrus/heimdall/pulls) which would make the usage of the referenced heimdall images impossible (even though the configuration files and rules reflect the latest changes). In such situations you'll have to use the `dev` image, build a heimdall image by yourself and update the setups to use it, ot switch to a tagged (released) version. +**Note:** The main branch may have breaking changes (see pending release PRs for details under https://github.com/dadrus/heimdall/pulls) which would make the usage of the referenced heimdall images impossible (even though the configuration files and rules reflect the latest changes). In such situations you'll have to use the `dev` image, build a heimdall image by yourself and update the setups to use it, or switch to a tagged (released) version. # Prerequisites From cd030252120f58b5c4cf68ddb148c20c72b0d7de Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 23:15:39 +0200 Subject: [PATCH 27/32] documentation updated --- docs/content/_index.adoc | 10 +- .../docs/concepts/operating_modes.adoc | 22 +-- .../docs/getting_started/protect_an_app.adoc | 12 +- docs/content/docs/rules/default_rule.adoc | 3 +- docs/content/docs/rules/providers.adoc | 10 +- docs/content/docs/rules/regular_rule.adoc | 163 +++++++++++------- docs/content/docs/rules/rule_sets.adoc | 34 ++-- docs/content/guides/authz/openfga.adoc | 12 +- 8 files changed, 157 insertions(+), 109 deletions(-) diff --git a/docs/content/_index.adoc b/docs/content/_index.adoc index d8f9a5810..14322f76c 100644 --- a/docs/content/_index.adoc +++ b/docs/content/_index.adoc @@ -23,10 +23,12 @@ spec: rules: - id: my_api_rule match: - path: /api/** - with: - scheme: http - host_glob: 127.0.0.1:9090 + routes: + - path: /api/** + scheme: http + hosts: + - type: exact + value: 127.0.0.1:9090 execute: - authenticator: keycloak - authorizer: opa diff --git a/docs/content/docs/concepts/operating_modes.adoc b/docs/content/docs/concepts/operating_modes.adoc index 48dc375a0..1af801798 100644 --- a/docs/content/docs/concepts/operating_modes.adoc +++ b/docs/content/docs/concepts/operating_modes.adoc @@ -76,12 +76,14 @@ And there is a rule, which allows anonymous requests and sets a header with subj ---- id: rule:my-service:anonymous-api-access match: - path: /my-service/api - with: - scheme: http - host_glob: my-backend-service - methods: - - GET + routes: + - path: /my-service/api + scheme: http + hosts: + - type: exact + value: my-backend-service + methods: + - GET execute: - authenticator: anonymous-authn - finalizer: id-header @@ -147,10 +149,10 @@ And there is a rule, which allows anonymous requests and sets a header with subj ---- id: rule:my-service:anonymous-api-access match: - path: /my-service/api - with: - methods: - - GET + routes: + - path: /my-service/api + methods: + - GET forward_to: host: my-backend-service:8888 execute: diff --git a/docs/content/docs/getting_started/protect_an_app.adoc b/docs/content/docs/getting_started/protect_an_app.adoc index a0b539b7d..6d8512692 100644 --- a/docs/content/docs/getting_started/protect_an_app.adoc +++ b/docs/content/docs/getting_started/protect_an_app.adoc @@ -137,7 +137,8 @@ version: "1alpha4" rules: - id: demo:public # <1> match: - path: /public + routes: + - path: /public forward_to: host: upstream:8081 execute: @@ -146,9 +147,12 @@ rules: - id: demo:protected # <2> match: - path: /:user - with: - path_glob: "{/user,/admin}" + routes: + - path: /:user + path_params: + - name: user + type: glob + value: "{user,admin}" forward_to: host: upstream:8081 execute: diff --git a/docs/content/docs/rules/default_rule.adoc b/docs/content/docs/rules/default_rule.adoc index cc151794a..28df9cade 100644 --- a/docs/content/docs/rules/default_rule.adoc +++ b/docs/content/docs/rules/default_rule.adoc @@ -58,7 +58,8 @@ Obviously, the authentication & authorization pipeline (defined in the `execute` ---- id: rule:my-service:protected-api match: - path: /foo + routes: + - path: /foo execute: - authorizer: allow_all_requests_authz ---- diff --git a/docs/content/docs/rules/providers.adoc b/docs/content/docs/rules/providers.adoc index a33b07e07..5f37f534c 100644 --- a/docs/content/docs/rules/providers.adoc +++ b/docs/content/docs/rules/providers.adoc @@ -49,10 +49,12 @@ name: my-rule-set rules: - id: rule:1 match: - path: /** - with: - host_glob: my-service1.local - methods: [ "GET" ] + routes: + - path: /** + hosts: + - type: exact + value: my-service1.local + methods: [ "GET" ] forward_to: host: ${UPSTREAM_HOST:="default-backend:8080"} execute: diff --git a/docs/content/docs/rules/regular_rule.adoc b/docs/content/docs/rules/regular_rule.adoc index c196f1012..6cecea6e1 100644 --- a/docs/content/docs/rules/regular_rule.adoc +++ b/docs/content/docs/rules/regular_rule.adoc @@ -12,7 +12,7 @@ description: Regular rules allow definition and as such execution of arbitrary l :toc: -In simplest case a regular rule will just reuse mechanisms from a previously defined link:{{< relref "/docs/mechanisms/catalogue.adoc" >}}[catalogue] in its pipelines. In more complex cases a rule can reconfigure parts of used mechanisms. Which parts can be reconfigured, respectively overridden are mechanism specific and described in the mechanism specific documentation. Reconfiguration is always limited to the particular rule pipeline and does not affect other rules. +In the simplest case, a regular rule reuses mechanisms from the previously defined link:{{< relref "/docs/mechanisms/catalogue.adoc" >}}[catalogue] in its pipelines. In more complex scenarios, a rule can reconfigure parts of the mechanisms being used. The specific parts that can be reconfigured or overridden depend on the mechanism itself and are described in the mechanism-specific documentation. Reconfiguration is always limited to the particular rule's pipeline and does not affect other rules. == Configuration @@ -20,39 +20,66 @@ A single regular rule consists of the following properties: * *`id`*: _string_ (mandatory) + -The unique identifier of a rule. It must be unique across all rules loaded by the same link:{{< relref "providers.adoc" >}}[Rule Provider]. To ensure this, it is recommended to let the `id` include the name of your upstream service, as well as its purpose. E.g. `rule:my-service:public-api`. +The unique identifier of the rule. It must be unique across all rules loaded by the same link:{{< relref "providers.adoc" >}}[Rule Provider]. To ensure uniqueness, it's recommended to include the upstream service's name and the rule’s purpose in the id. For example, `rule:my-service:public-api`. * *`match`*: _RuleMatcher_ (mandatory) + -Defines how to match a rule and supports the following properties (see also link:{{< relref "#_rule_matching_specificity_backtracking" >}}[Rule Matching Specificity & Backtracking] for more details): +Defines the matching criteria for a rule, with the following properties: -** *`path`*: _link:{{< relref "#_path_expression" >}}[PathExpression]_ (mandatory) +** *`routes`*: _RouteMatcher array_ (mandatory) + -The path expression describing the paths of incoming requests this rule is supposed to match. Supports usage of simple and free (named) wildcards. +Specifies route conditions for matching the rule to incoming HTTP requests with each entry having the following properties: + +*** *`path`*: _string_ (mandatory) ++ +The link:{{< relref "#_path_expression" >}}[Path Expression] describing the request path this rule should match. It supports both simple and free (named) wildcards. + +*** *`path_params`*: _PathParameterConditions_ (optional) ++ +Additional conditions for the values captured by named wildcards in the path expression. Each entry supports the following properties: + +**** *`name`*: _string_ (mandatory) ++ +The name of the wildcard. + +**** *`type`*: _string_ (mandatory) ++ +The type of expression used to match the captured wildcard's value. The supported types are: + +***** `glob`: to use a https://github.com/gobwas/glob[glob expression] to match the captured value (`/` is used as a delimiter, so `*` matches anything until the next `/`). +***** `regex` to use a regular expression to match the captured value. + +**** *`value`*: _string_ (mandatory) ++ +The actual expression based on the given `type`. ** *`backtracking_enabled`*: _boolean_ (optional) + -This property can only be used together with the additional matching conditions (see the `with` property below). Enables or disables backtracking if a request is matched based on the `path` expression, but the additional matching conditions are not satisfied. Inherited from the default rule and defaults to the settings in that rule. If enabled, the lookup will traverse back to a rule with a less specific path expression and potentially (depending on the evaluation of additional conditions defined on that level) match it. +Enables or disables backtracking when a request matches the path expressions but fails to meet additional matching criteria, like `path_params`, `hosts`, etc. This setting is inherited from the default rule and defaults to that rule's setting. When enabled, the system will backtrack to attempt a match with a less specific rule (see link:{{< relref "#_rule_matching_specificity_backtracking" >}}[Rule Matching Specificity & Backtracking] for more details). -** *`with`*: _MatchConditions_ (optional) +** *`hosts`*: _HostMatcher array_ (optional) + -Additional conditions, which all must hold true to have the request matched and the pipeline of this rule executed. This way, you can define different rules for the same path but with different conditions, e.g. to define separate rules for read and write requests to the same resource. +Defines a set of hosts to match against the HTTP Host header. Each entry has the following properties: -*** *`host_glob`*: _string_ (optional) +*** *`type`*: _string_ (mandatory) + -A https://github.com/gobwas/glob[glob expression] which should be satisfied by the host of the incoming request. `.` is used as a delimiter. That means `*` will match anything until the next `.`. Mutually exclusive with `host_regex`. +Specifies the type of expression for matching the host, which can be one of: + +**** `exact` to match the host exactly +**** `glob` to use a https://github.com/gobwas/glob[glob expression] which should be satisfied by the host of the incoming request (`.` is used as a delimiter, which means `*` will match anything until the next `.`). +**** `regex` to use a regular expression which should be satisfied by the host of the incoming request. -*** *`host_regex`*: _string_ (optional) +*** *`value`*: _string_ (mandatory) + -Regular expression to match the host. Mutually exclusive with `host_glob`. +The actual host expression based on the `type`. -*** *`scheme`*: _string_ (optional) +** *`scheme`*: _string_ (optional) + -Expected HTTP scheme. If not specified, both http and https are accepted. +The expected HTTP scheme. If not specified, both http and https are accepted. -*** *`methods`*: _string array_ (optional) +** *`methods`*: _string array_ (optional) + -Which HTTP methods (`GET`, `POST`, `PATCH`, etc) are allowed. If not specified, all methods are allowed. If all, except some specific methods should be allowed, one can specify `ALL` and remove specific methods by adding the `!` sign to the to be removed method. In that case you have to specify the value in braces. See also examples below. +Specifies the allowed HTTP methods (`GET`, `POST`, `PATCH`, etc). If not specified, all methods are allowed. To allow all methods except specific ones, use `ALL` and prefix the methods to exclude with `!`. For example: + [source, yaml] ---- @@ -70,62 +97,54 @@ methods: - "!OPTIONS" ---- -*** *`path_glob`*: _string_ (optional) -+ -A https://github.com/gobwas/glob[glob expression], which should be satisfied by the path of the incoming request. `/` is used as a delimiter. That means `*` will match anything until the next `/`. Mutually exclusive with `path_regex`. - -*** *`path_regex`*: _string_ (optional) -+ -A regular expression, which should be satisfied by the path of the incoming request. Mutually exclusive with `path_glob`. - * *`allow_encoded_slashes`*: _string_ (optional) + -Defines how to handle url-encoded slashes in url paths while matching and forwarding the requests. Can be set to the one of the following values, defaulting to `off`: +Controls how to handle URL-encoded slashes in request paths during matching and forwarding. Options include: -** *`off`* - Reject requests containing encoded slashes. Means, if the request URL contains an url-encoded slash (`%2F`), the rule will not match it. -** *`on`* - Accept requests with encoded slashes. As soon as a rule is matched, encoded slashes present in the path of the request are, decoded and made transparent for the matched rule and the upstream service. That is, the `%2F` becomes a `/` and will be treated as such in all places. -** *`no_decode`* - Accept requests using encoded slashes without touching them. That is, the `%2F` just remains as is. +** *`off`* - Reject requests with encoded slashes (`%2F`). This is the default behavior. +** *`on`* - Accept requests with encoded slashes recoding them to `/`. +** *`no_decode`* - Accept requests with encoded slashes without touching them. + -CAUTION: Since the proxy integrating with heimdall, heimdall by itself, and the upstream service, all may treat the url-encoded slashes differently, accepting requests with url-encoded slashes can, depending on your rules, lead to https://cwe.mitre.org/data/definitions/436.html[Interpretation Conflict] vulnerabilities resulting in privilege escalations. +CAUTION: Handling URL-encoded slashes may differ across the proxies in front of heimdall, heimdall, and the upstream service. Accepting requests with encoded slashes could, depending on your rules, lead to https://cwe.mitre.org/data/definitions/436.html[Interpretation Conflict] vulnerabilities resulting in privilege escalations. * *`forward_to`*: _RequestForwarder_ (mandatory in Proxy operation mode) + -Defines where to forward the proxied request to. Used only when heimdall is operated in the Proxy operation mode and supports the following properties: +Specifies where to forward proxied requests when heimdall is operating in proxy mode. The following properties are supported: ** *`host`*: _string_ (mandatory) + -Host (and port) to be used for request forwarding. If no `rewrite` property (see below) is specified, all other parts, like scheme, path, etc. of the original url are preserved. E.g. if the original request is `\https://mydomain.com/api/v1/something?foo=bar&bar=baz` and the value of this property is set to `my-backend:8080`, the url used to forward the request to the upstream will be `\https://my-backend:8080/api/v1/something?foo=bar&bar=baz` +Host (and port) for forwarding the request. If no `rewrite` property (see below) is defined, the original URL's scheme, path, and other components are preserved. E.g. if the original request is `\https://mydomain.com/api/v1/something?foo=bar&bar=baz` and the value of this property is set to `my-backend:8080`, the url used to forward the request to the upstream will be `\https://my-backend:8080/api/v1/something?foo=bar&bar=baz` + -NOTE: The `Host` header is not preserved while forwarding the request. If you need it to be set to the value from the original request, make use of the link:{{< relref "/docs/mechanisms/finalizers.adoc#_header" >}}[header finalizer] in your `execute` pipeline and set it accordingly. The example below demonstrates that. +NOTE: The `Host` header is not preserved when forwarding the request. To preserve it, use of the link:{{< relref "/docs/mechanisms/finalizers.adoc#_header" >}}[header finalizer] in your `execute` pipeline and set it accordingly. The example below demonstrates that. ** *`rewrite`*: _OriginalURLRewriter_ (optional) + -Can be used to rewrite further parts of the original url before forwarding the request. If specified at least one of the following supported (middleware) properties must be specified: +Can be used to rewrite further parts of the original URL before forwarding the request. If specified, at least one of the following supported (middleware) properties must be specified: *** *`scheme`*: _string_ (optional) + -If defined, heimdall will use the specified value for the url scheme part while forwarding the request to the upstream. +Specifies the URL scheme to use for forwarding the request. *** *`strip_path_prefix`*: _string_ (optional) + -If defined, heimdall will strip the specified prefix from the original url path. E.g. if the path of the original url is `/api/v1/something` and the value of this property is set to `/api/v1`, the request to the upstream will have the url path set to `/something`. +This middleware strips the specified prefix from the original URL path before forwarding. E.g. if the path of the original url is `/api/v1/something` and the value of this property is set to `/api/v1`, the request to the upstream will have the url path set to `/something`. *** *`add_path_prefix`*: _string_ (optional) + -This middleware is applied after the execution of the `strip_path_prefix` middleware described above. If defined, heimdall will add the specified path prefix to the path used to forward the request to the upstream service. E.g. if the path of the original url or the pass resulting after the application of the `strip_path_prefix` middleware is `/something` and the value of this property is set to `/my-backend`, the request to the upstream will have the url path set to `/my-backend/something`. +This middleware is applied after the execution of the `strip_path_prefix` middleware described above. If specified, heimdall will add the specified path prefix to the path used to forward the request to the upstream service. E.g. if the path of the original URL or the path resulting after the application of the `strip_path_prefix` middleware is `/something` and the value of this property is set to `/my-backend`, the request to the upstream will have the URL path set to `/my-backend/something`. *** *`strip_query_parameters`*: _string array_ (optional) + -If defined, heimdall will remove the specified query parameters from the original url before forwarding the request to the upstream service. E.g. if the query parameters part of the original url is `foo=bar&bar=baz` and the value of this property is set to `["foo"]`, the query part of the request to the upstream will be set to `bar=baz` +Removes specified query parameters from the original URL before forwarding. E.g. if the query parameters part of the original URL is `foo=bar&bar=baz` and the value of this property is set to `["foo"]`, the query part of the request to the upstream will be set to `bar=baz` * *`execute`*: _link:{{< relref "#_authentication_authorization_pipeline" >}}[Authentication & Authorization Pipeline]_ (mandatory) + -Which mechanisms to use to authenticate, authorize, contextualize (enrich) and finalize the pipeline. +Specifies the mechanisms used for authentication, authorization, contextualization, and finalization. * *`on_error`*: _link:{{< relref "#_error_pipeline" >}}[Error Pipeline]_ (optional) + -Which error handler mechanisms to use if any of the mechanisms, defined in the `execute` property, fails. This property is optional only, if a link:{{< relref "default_rule.adoc" >}}[default rule] has been configured and contains an `on_error` definition. +Specifies error handling mechanisms if the pipeline defined by the `execute` property fails. Optional, if a link:{{< relref "default_rule.adoc" >}}[default rule] is configured with `on_error` definition. .An example rule ==== @@ -133,11 +152,17 @@ Which error handler mechanisms to use if any of the mechanisms, defined in the ` ---- id: rule:foo:bar match: - path: /** - with: - scheme: http - host_glob: my-service.local - methods: + routes: + - path: /some/:identifier/followed/by/** + path_params: + - name: identifier + type: glob + value: "[a-z]" + scheme: http + hosts: + - type: exact + value: my-service.local + methods: - GET - POST forward_to: @@ -170,8 +195,8 @@ on_error: Path expressions are used to match the incoming requests. When specifying these, you can make use of two types of wildcards: -- free wildcard, which can be defined using `*` and -- single wildcard, which can be defined using `:` +* free wildcard, which can be defined using `*` and +* single wildcard, which can be defined using `:` Both can be named and unnamed, with named wildcards allowing accessing of the matched segments in the pipeline of the rule using the defined name as a key on the link:{{< relref "/docs/mechanisms/evaluation_objects.adoc#_url_captures" >}}[`Request.URL.Captures`] object. Unnamed free wildcard is defined as `\**` and unnamed single wildcard is defined as `:*`. A named wildcard uses some identifier instead of the `*`, so like `*name` for free wildcard and `:name` for single wildcard. @@ -202,9 +227,11 @@ Here is an example demonstrating the usage of a single named wildcard: ---- id: rule:1 match: - path: /files/:uuid/delete - with: - host_glob: hosty.mchostface + routes: + - path: /files/:uuid/delete + hosts: + - type: exact + value: hosty.mchostface execute: - authorizer: openfga_check config: @@ -218,27 +245,27 @@ match: == Rule Matching Specificity & Backtracking -The implementation ensures, that more specific path expressions are matched first regardless of the placement of rules in a rule set. -Indeed, the more specific rules are matched first even the corresponding rules are defined in different rule sets. +The implementation ensures that rules with more specific path expressions are matched first, regardless of their placement within a rule set. In fact, more specific rules are prioritized even when they are defined across different rule sets. -When the path expression is matched to a request, additional conditions, if present in the rule's matching definition, are evaluated. Only if these succeeded, the pipeline of the rule is executed. +When a path expression matches a request, any additional conditions specified in the rule's matching criteria are evaluated. Only if these conditions are met will the rule's pipeline be executed. -CAUTION: If there are multiple rules for the same path expression with matching additional conditions, the first matching rule is taken. The matching order depends on the rule sequence in the rule set. +CAUTION: If multiple rules share the same path expression and all their additional conditions match, the first matching rule will be applied. The matching order is determined by the sequence of rules in the rule set. -If there is no matching rule, backtracking, if enabled, will take place and the next less specific rule may be matched. Backtracking stops if either +If no rule is matched, and backtracking is enabled, the process will backtrack to attempt a match with the next less specific rule. Backtracking will stop when: -* a less specific rule is successfully matched (incl. the evaluation of additional expressions), or -* a less specific rule is not matched and does not allow backtracking. +* a less specific rule successfully matches (including evaluation of any additional conditions), or +* a less specific rule fails to match and does not permit backtracking. -The following examples demonstrates the aspects described above. +The following examples illustrate these principles: -Imagine, there are the following rules +Imagine the following set of rules [source, yaml] ---- id: rule1 match: - path: /files/** + routes: + - path: /files/** execute: - ---- @@ -247,10 +274,13 @@ execute: ---- id: rule2 match: - path: /files/:team/:name + routes: + - path: /files/:team/:name + path_params: + - name: team + type: regex + value: "(team1|team2)" backtracking_enabled: true - with: - path_regex: ^/files/(team1|team2)/.* execute: - ---- @@ -259,16 +289,17 @@ execute: ---- id: rule3 match: - path: /files/team3/:name + routes: + - path: /files/team3/:name execute: - ---- -The request to `/files/team1/document.pdf` will be matched by the rule with id `rule2` as it is more specific to `rule1`. So the pipeline of `rule2` will be executed. +The request to `/files/team1/document.pdf` will be matched by `rule2`, as it is more specific than `rule1`. Consequently, the pipeline for `rule2` will be executed. -The request to `/files/team3/document.pdf` will be matched by the `rule3` as it is more specific than `rule1` and `rule2`. Again the corresponding pipeline will be executed. +The request to `/files/team3/document.pdf` will be matched by `rule3`, which is more specific than both `rule1` and `rule2`. As a result, the corresponding pipeline will be executed. -However, even the request to `/files/team4/document.pdf` will be matched by `rule2`, the regular expression `^/files/(team1|team2)/.*` will fail. Here, since `backtracking_enabled` is set to `true` backtracking will start and the request will be matched by the `rule1` and its pipeline will be then executed. +However, even though the request to `/files/team4/document.pdf` matches the path defined in `rule2`, the regular expression `(team1|team2)` used in the `path_params` for the `team` parameter will not match. Since `backtracking_enabled` is set to `true`, backtracking will occur, and the request will be matched by `rule1`, with its pipeline then being executed. == Authentication & Authorization Pipeline diff --git a/docs/content/docs/rules/rule_sets.adoc b/docs/content/docs/rules/rule_sets.adoc index e6302962d..791386eee 100644 --- a/docs/content/docs/rules/rule_sets.adoc +++ b/docs/content/docs/rules/rule_sets.adoc @@ -49,20 +49,24 @@ name: my-rule-set rules: - id: rule:1 match: - path: /** - with: - methods: [ "GET" ] - scheme: https - host_glob: my-service1.local + routes: + - path: /** + methods: [ "GET" ] + scheme: https + hosts: + - type: exact + value: my-service1.local execute: - authorizer: foobar - id: rule:2 match: - path: /** - with: - scheme: https - host_glob: my-service2.local - methods: [ "GET" ] + routes: + - path: /** + scheme: https + hosts: + - type: exact + value: my-service2.local + methods: [ "GET" ] execute: - authorizer: barfoo ---- @@ -123,10 +127,12 @@ spec: rules: - id: "" match: - path: /foo/** - with: - scheme: https - host_glob: 127.0.0.1:9090 + routes: + - path: /foo/** + scheme: https + hosts: + - type: exact + value: 127.0.0.1:9090 execute: - authenticator: foo - authorizer: bar diff --git a/docs/content/guides/authz/openfga.adoc b/docs/content/guides/authz/openfga.adoc index 43c07591c..35b3fa4c0 100644 --- a/docs/content/guides/authz/openfga.adoc +++ b/docs/content/guides/authz/openfga.adoc @@ -264,9 +264,9 @@ version: "1alpha4" rules: - id: access_document # <1> match: - path: /document/:id # <2> - with: - methods: [ GET, POST, DELETE ] + routes: + - path: /document/:id # <2> + methods: [ GET, POST, DELETE ] forward_to: # <3> host: upstream:8081 execute: @@ -288,9 +288,9 @@ rules: - id: list_documents # <11> match: - path: /documents # <12> - with: - methods: [ GET ] # <14> + routes: + - path: /documents # <12> + methods: [ GET ] # <14> forward_to: # <13> host: upstream:8081 execute: # <15> From ade15f6ac1f66fafdf23ec649d863c897d3132ec Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 23:16:03 +0200 Subject: [PATCH 28/32] DockerHub readme updated --- DockerHub-README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DockerHub-README.md b/DockerHub-README.md index d7900be26..b6ad356fd 100644 --- a/DockerHub-README.md +++ b/DockerHub-README.md @@ -125,7 +125,8 @@ version: "1alpha4" rules: - id: test-rule match: - path: /** + routes: + - path: /** forward_to: host: upstream execute: From a1f5e555310703f78c16dfdf985db1ac29ba0b99 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Tue, 10 Sep 2024 23:19:12 +0200 Subject: [PATCH 29/32] small fix in example --- docs/content/docs/configuration/types.adoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/content/docs/configuration/types.adoc b/docs/content/docs/configuration/types.adoc index 007cb720a..ebf92fa0b 100644 --- a/docs/content/docs/configuration/types.adoc +++ b/docs/content/docs/configuration/types.adoc @@ -565,7 +565,8 @@ auth: headers: X-My-First-Header: foobar X-My-Second-Header: barfoo -enable_http_cache: true +http_cache: + enabled: true ---- ==== From 238e612bbb434b56cb8ac4f3bf4286c3776fd6b5 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Wed, 11 Sep 2024 19:15:28 +0200 Subject: [PATCH 30/32] small fix related to decoding of encoded slashes in paths and making them available in path params matchers --- internal/rules/route_matcher.go | 19 ++++++++++++++----- internal/rules/route_matcher_test.go | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/rules/route_matcher.go b/internal/rules/route_matcher.go index 196da7b9a..bb1b3a531 100644 --- a/internal/rules/route_matcher.go +++ b/internal/rules/route_matcher.go @@ -19,6 +19,7 @@ package rules import ( "errors" "net/http" + "net/url" "slices" "strings" @@ -102,11 +103,19 @@ func (m *pathParamMatcher) Matches(request *heimdall.Request, keys, values []str value := values[idx] // URL.RawPath is set only if the original url contains url encoded parts - if len(request.URL.RawPath) != 0 && - m.slashHandling == config.EncodedSlashesOff && - strings.Contains(request.URL.RawPath, "%2F") { - return errorchain.NewWithMessage(ErrRequestPathMismatch, - "request path contains encoded slashes which are not allowed") + if len(request.URL.RawPath) != 0 { + switch m.slashHandling { + case config.EncodedSlashesOff: + if strings.Contains(request.URL.RawPath, "%2F") { + return errorchain.NewWithMessage(ErrRequestPathMismatch, + "request path contains encoded slashes which are not allowed") + } + case config.EncodedSlashesOn: + value, _ = url.PathUnescape(value) + default: + unescaped, _ := url.PathUnescape(strings.ReplaceAll(value, "%2F", "$$$escaped-slash$$$")) + value = strings.ReplaceAll(unescaped, "$$$escaped-slash$$$", "%2F") + } } if !m.match(value) { diff --git a/internal/rules/route_matcher_test.go b/internal/rules/route_matcher_test.go index 42b201521..1d9db6fde 100644 --- a/internal/rules/route_matcher_test.go +++ b/internal/rules/route_matcher_test.go @@ -437,9 +437,9 @@ func TestPathParamsMatcherMatches(t *testing.T) { matches: true, }, { - uc: "matches with path having allowed encoded slashes", + uc: "matches with path having allowed decoded slashes", conf: []config.ParameterMatcher{ - {Name: "foo", Type: "exact", Value: "bar%2Fbaz"}, + {Name: "foo", Type: "exact", Value: "bar/baz"}, }, slashHandling: config.EncodedSlashesOn, keys: []string{"foo"}, From fd45ef6ed24cae93fd82ffb5628505d213a729f7 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Wed, 11 Sep 2024 19:49:17 +0200 Subject: [PATCH 31/32] typo fixed --- docs/content/docs/rules/regular_rule.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/docs/rules/regular_rule.adoc b/docs/content/docs/rules/regular_rule.adoc index 6cecea6e1..8c4498183 100644 --- a/docs/content/docs/rules/regular_rule.adoc +++ b/docs/content/docs/rules/regular_rule.adoc @@ -102,7 +102,7 @@ methods: Controls how to handle URL-encoded slashes in request paths during matching and forwarding. Options include: ** *`off`* - Reject requests with encoded slashes (`%2F`). This is the default behavior. -** *`on`* - Accept requests with encoded slashes recoding them to `/`. +** *`on`* - Accept requests with encoded slashes decoding them to `/`. ** *`no_decode`* - Accept requests with encoded slashes without touching them. + From a7a4a67f9ed2504a5840722f3cb69042f9d52fa0 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Wed, 11 Sep 2024 19:49:34 +0200 Subject: [PATCH 32/32] better tests --- internal/rules/route_matcher_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/rules/route_matcher_test.go b/internal/rules/route_matcher_test.go index 1d9db6fde..9ad4b047e 100644 --- a/internal/rules/route_matcher_test.go +++ b/internal/rules/route_matcher_test.go @@ -423,13 +423,13 @@ func TestPathParamsMatcherMatches(t *testing.T) { { uc: "matches with path having allowed but not decoded encoded slashes", conf: []config.ParameterMatcher{ - {Name: "foo", Type: "exact", Value: "bar%2Fbaz"}, + {Name: "foo", Type: "exact", Value: "bar%2Fbaz[id]"}, }, slashHandling: config.EncodedSlashesOnNoDecode, keys: []string{"foo"}, - values: []string{"bar%2Fbaz"}, + values: []string{"bar%2Fbaz%5Bid%5D"}, toMatch: func() url.URL { - uri, err := url.Parse("http://example.com/bar%2Fbaz") + uri, err := url.Parse("http://example.com/bar%2Fbaz%5Bid%5D") require.NoError(t, err) return *uri @@ -439,13 +439,13 @@ func TestPathParamsMatcherMatches(t *testing.T) { { uc: "matches with path having allowed decoded slashes", conf: []config.ParameterMatcher{ - {Name: "foo", Type: "exact", Value: "bar/baz"}, + {Name: "foo", Type: "exact", Value: "bar/baz[id]"}, }, slashHandling: config.EncodedSlashesOn, keys: []string{"foo"}, - values: []string{"bar%2Fbaz"}, + values: []string{"bar%2Fbaz%5Bid%5D"}, toMatch: func() url.URL { - uri, err := url.Parse("http://example.com/bar%2Fbaz") + uri, err := url.Parse("http://example.com/foo%2Fbaz%5Bid%5D") require.NoError(t, err) return *uri