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

RLP conditions and variables order does not matter #147

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

eguzki
Copy link
Contributor

@eguzki eguzki commented Feb 10, 2023

On the reconciliation processing, these two limits were considered different and that triggered a update in the limitador object

      limits:
        - conditions: 
            -  a == '1'
            - b == '1'
          maxValue: 2
          seconds: 30
          variables: []
      limits:
        - conditions: 
            - b == '1'
            -  a == '1'
          maxValue: 2
          seconds: 30
          variables: []

However, actually, two limits with reordered conditions/variables are effectively the same.

This fix prevents reconciliation update when the only difference is the order of the conditions/variables.

This PR will be in Draft mode until #144 is merged as it depends on it

@eguzki eguzki force-pushed the rlp-conditions-variables-order-does-not-matter branch from bac5ede to 0aea137 Compare February 13, 2023 15:46
@eguzki eguzki marked this pull request as ready for review February 13, 2023 15:51
@eguzki eguzki requested a review from a team as a code owner February 13, 2023 15:51
@eguzki eguzki merged commit 4fa5d06 into main Feb 13, 2023
@eguzki eguzki deleted the rlp-conditions-variables-order-does-not-matter branch February 13, 2023 16:13
mikenairn pushed a commit to mikenairn/kuadrant-operator that referenced this pull request Mar 23, 2023
* fix httproute authorization annotation feature

* new demo: updating the ratelimitpolicy targetref

* small doc fixes

* demo: authenticated rate limit

* fix doc rate limit authenticated
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.

2 participants