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

Add identical hostnames test for RateLimitPolicy #462

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

trepel
Copy link
Contributor

@trepel trepel commented Jun 25, 2024

Overview

Partially addresses #406
Follows up for #439 which added similar tests for AuthPolicy, this PR does the same for RateLimitPolicy (RLP).

It covers two scenarios, both testing current behavior

  1. Two RLPs being attached to GW and HTTPRoute with identical hostname respectively
  2. Two RLPs being attached to two HTTPRoutes with identical hostname each

For details see https://github.com/Kuadrant/kuadrant-operator/blob/main/doc/rate-limiting.md#limitation-multiple-network-resources-with-identical-hostnames

Note that the 1. scenario has been fixed in the meantime, doc is not up-to-date. The GW RLP is now partially enforced meaning that it is actually applied on the route-b. Partially enforced policy has Enforced=True status as well, see Kuadrant/kuadrant-operator#679

What was done

  • added two tests - one for each scenario
  • updated wait_for_ready so that it only returns True only if policy is fully enforced (not only partially)
  • added wait_for_partial_enforced method
  • using /anything/route1 and /anything/route2 paths instead of just / and /anything which better aligns with doc (/foo and /bar). Using just / is problematic because if is "superset" to /anything/ so it was not ideal for 2. scenario
  • renamed 2nd route to route2

Verification Steps

Eye review, execute both tests and make sure they pass. Also execute the two tests for AuthPolicy

@trepel trepel requested review from pehala and azgabur June 25, 2024 12:23
@trepel trepel requested a review from pehala June 26, 2024 13:39
@jsmolar jsmolar merged commit 5011576 into Kuadrant:main Jul 2, 2024
2 checks passed
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.

3 participants