Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Route based matching of rules #1766

Merged
merged 33 commits into from
Sep 11, 2024
Merged

feat: Route based matching of rules #1766

merged 33 commits into from
Sep 11, 2024

Conversation

dadrus
Copy link
Owner

@dadrus dadrus commented Sep 10, 2024

Related issue(s)

closes #1718

Checklist

  • I agree to follow this project's Code of Conduct.
  • I have read, and I am following this repository's Contributing Guidelines.
  • I have read the Security Policy.
  • I have referenced an issue describing the bug/feature request.
  • I have added tests that prove the correctness of my implementation.
  • I have updated the documentation.

Description

This PR builds upon the work initiated in #1358 by extending the matching capabilities to support rule matching based on individual routes. This not only simplifies and streamlines the rule matching configuration API, making it more intuitive and user-friendly, but also enhances the efficiency of the implementation, particularly in terms of path specificity—where wildcard path values need to conform to specific patterns. Additionally, the new rule matching API helps reduce the need for redundant rules.

Here how the rule configuration with the new matching API looks like:

id: some rule
match:
  routes: # mandatory. must contain at least one `path` entry
    - path: /some/:user_id/and/:document_id # mandatory
      path_params: # optional. if defined must contain at least one entry
        - name: user_id # all properties are mandatory
          type: glob # or regex, or exact
          value: "[0-9a-f]"
        - { name: document_id, type: glob, value: "[0-9a-f]" }
    - path: /some/:user_id # mandatory
      path_params: # optional
        - { name: user_id, type: glob, value: "[0-9a-f]" }
  methods: [ GET ] # optional, defaults to ALL
  hosts: # optional, default to *. if defined, must contain at least one entry
    - type: glob # all properties are mandatory. type can be glob, regex, or exact
      value: "*.example.com"
    - type: regex
      value: ...
  scheme: https # optional, defaults to http and https
  backtracking_enabled: true # optional, defaults to false
allow_encoded_slashes: off
forward_to:
  host: foo.bar
  rewrite:
    scheme: http
    strip_path_prefix: /some
    add_path_prefix: /users
    # other settings
execute:
  - # actual pipeline
on_error:
  - # error pipeline

And in a minimal representation, a rule definition looks now as follows:

id: some rule
match:
  routes:
    - path: /some/thing
execute:
  - # actual pipeline
on_error:
  - # error pipeline

Please Note: Values captured by unnamed wildcards - :* or ** cannot be referenced in path_params as * is an invalid name.

With that configuration options in place, the negative examples provided in #1718 can be rewritten as follows (only the matching part is shown):

match:
  routes:
    - path: /identity/.well-known/home
    - path: /identity/recovery
    - path: /identity/login
    - path: /identity/logout
    - path: /identity/register
    - path: /identity/activation_pending
    - path: /identity/activate_account
    - path: /identity/account_activated
    - path: /identity/:resource
      path_params:
        - name: resource
          type: glob
          value: "{*.css,*.js,*.ico}"
  methods: [ GET, POST ]
match:
  routes:
    - path: /api/.ory/admin/
    - path: /api/.ory/admin/courier/messages
    - path: /api/.ory/admin/courier/messages/:message_id
      path_params:
        - { name: message_id, type: regex, value: "[-0-9a-fA-F]+" }
    - path: /api/.ory/admin/identities
    - path: /api/.ory/admin/identities/:identity_id
      path_params:
        - { name: identity_id, type: regex, value: "[-0-9a-fA-F]+" }
    - path: /api/.ory/admin/identities/:identity_id/sessions
      path_params:
        - { name: identity_id, type: regex, value: "[-0-9a-fA-F]+" }
    - path: /api/.ory/admin/identities/:identity_id/sessions/:session_id
      path_params:
        - { name: identity_id, type: regex, value: "[-0-9a-fA-F]+" }
        - { name: session_id, type: regex, value: "[-0-9a-fA-F]+" }

Additional Information

This design of this new matching API has been suggested in discussed in discord (https://discord.com/channels/1100447190796742698/1280410897675718697).

…e used directly by the rule factory implementaiton
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 97.60000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.75%. Comparing base (4c059b3) to head (a7a4a67).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/rules/repository_impl.go 89.65% 3 Missing ⚠️
internal/rules/rule_factory_impl.go 95.00% 1 Missing and 1 partial ⚠️
internal/rules/rule_impl.go 94.73% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1766   +/-   ##
=======================================
  Coverage   89.74%   89.75%           
=======================================
  Files         271      270    -1     
  Lines        8993     9039   +46     
=======================================
+ Hits         8071     8113   +42     
- Misses        684      686    +2     
- Partials      238      240    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dadrus dadrus changed the title feat: Route based matching of rules wip: Route based matching of rules Sep 10, 2024
@dadrus dadrus marked this pull request as draft September 10, 2024 00:46
@dadrus dadrus marked this pull request as ready for review September 10, 2024 01:45
@dadrus dadrus changed the title wip: Route based matching of rules feat: Route based matching of rules Sep 11, 2024
@dadrus dadrus merged commit 8ef379d into main Sep 11, 2024
28 checks passed
@dadrus dadrus deleted the feat/match_with_path_lists branch September 11, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Matching Multiple Paths Using Lists
1 participant