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.

Sign up for GitHub

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

Jump to bottom

Hardening and Improving RateLimitPolicy #85

Closed
2 tasks done
maleck13 opened this issue May 30, 2022 · 2 comments
Closed
2 tasks done

Hardening and Improving RateLimitPolicy #85

maleck13 opened this issue May 30, 2022 · 2 comments
Assignees
@eguzki
Labels
kind/epic Master issue tracking broken down work

Comments

@maleck13
Copy link
Collaborator

maleck13 commented May 30, 2022 •
edited by eguzki
Loading

What

We have focused on fast iteration around the rate limit policy resource. It is solidifying now and so we want to turn our attention towards hardening and quality.
@maleck13 maleck13 added the kind/epic Master issue tracking broken down work label May 30, 2022
@maleck13
Copy link
Collaborator Author

@rahulanand16nov @eguzki could you outline any improvements / tech debt you would like to address as issues here?

@eguzki
Copy link
Contributor

eguzki commented Jul 18, 2022 •
edited by maleck13
Loading

The RLP refactor is being done and merged in Kuadrant/kuadrant-controller#175

Regarding unittests and e2e tests:

e2e tests

Cover the following scenarios:

  • RLP object changes target to another HTTPRoute
  • RLP object changes target from HTTPRoute to Gateway
  • HTTPRoute object changes parentRefs (references to parent Gateways)
  • RLP object targeting HTTPRoute is deleted
  • RLP object targeting Gateway is deleted
  • HTTPRoute object changes hostnames list values
  • RLP object changes spec:
    • add new limit(s) / rule(s) / configuration(s)
    • remove limit(s) / rule(s) / configuration(s)
    • update limit(s) / rule(s) / configuration(s) values
  • Supports multiple RLP's with exactly the same hostnames

That should provide code coverage of ~90% of statements

The scope of these e2e tests is the control plane. The integration with Limitador and Envoy are out of scope, i.e. the e2e tests will assert that the Limitador's CR have the expected data, the e2e tests will not assert rate limiting is happening.

Unittests

Currently "common" (not controllers package) code packages

Package Coverage
github.com/kuadrant/kuadrant-controller/pkg/common 9.0% of statements
github.com/kuadrant/kuadrant-controller/apis/apim/v1alpha1 5.6% of statements
github.com/kuadrant/kuadrant-controller/pkg/istio [no test files]
github.com/kuadrant/kuadrant-controller/pkg/log 37.5% of statements
github.com/kuadrant/kuadrant-controller/pkg/reconcilers 45.1% of statements
github.com/kuadrant/kuadrant-controller/pkg/rlptools [no test files]
github.com/kuadrant/kuadrant-controller/version [no test files]

Aim to reach code cov 80% - 90% of statements

  • Basic e2e test that sets up base code needed to run e2e test
  • Basic set of unit tests

@eguzki eguzki self-assigned this Jul 18, 2022
@didierofrivia didierofrivia transferred this issue from Kuadrant/kuadrant-controller Nov 8, 2022
@eguzki eguzki closed this as completed Feb 10, 2023
mikenairn pushed a commit to mikenairn/kuadrant-operator that referenced this issue Mar 23, 2023
@eguzki
ratelimitpolicy virtualhosts ratelimits (Kuadrant#85)
2464ad5 
* ratelimitpolicy virtualhosts ratelimits

* apim/ratelimitpolicy_controller: remove unnecessary route match for virtualhost

* fix lint issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eguzki eguzki

Labels
kind/epic Master issue tracking broken down work
Projects
No open projects
Kuadrant Service Protection
Status: Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@maleck13 @eguzki