-
Notifications
You must be signed in to change notification settings - Fork 37
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-v2] do not use gateway name nor host in the RL domain #218
Conversation
0bbadd2
to
98cbe4a
Compare
7e7c874
to
9520a03
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the small question about sorting the conditions and variables. Other than this works as describe in PR.
Fixes the reconciliation of the Limitador CR, pairing it with the reconciled wasm config (WasmPlugin CR), so - rate limit definitions won't be duplicated in the Limitador CR; - limits can be defined crossing gateways and hostnames and yet be treated as the same limit in Limitador (case of simple RLPs that target HTTPRoutes with multiple Gateway parent refs)
9520a03
to
5fe61cb
Compare
Verification steps working as expected. After a quick review, LGTM. Probably it would be good to have another reviewer's LGTM |
var uniquePolicyRefs map[string]struct{} | ||
var policyRefs []client.ObjectKey | ||
|
||
gwList := &gatewayapiv1beta1.GatewayList{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only thing I would like to rise.
The previous implementation was designed with a premise: try to avoid "Get all gateways" or "Get all routes" from the cluster. The approach was to reconcile deltas coming from the RLP instance causing the event.
Something that deserves further consideration, maybe for another PR
483021d
to
d9f13e2
Compare
… in the format: limit.<limit-name>__<hash>, where <limit-name> is sanitised to include only characters allowed by Limitador for the identifiers and <hash> is generated out of the original limit name to avoid breaking uniqueness of the name after sanitisation.
d9f13e2
to
73b0572
Compare
Fixes the reconciliation of the Limitador CR, pairing it with the reconciled wasm config (WasmPlugin CR), so
Closes #201
Verification steps
Verification steps for a scenario with 2 Gateways, 3 HTTPRoutes and 4 RateLimitPolicies.
Intro
Disclaimer
Please keep in mind that we are implementing Direct Policy Attachment and not yet Inherited Policy Attachment. I.e.,
Topology
Description of the network resources
gw-1
: N/S (internet) traffic gateway; declares 2 listeners:*.website
(for public websites) and*.io
(for APIs exposed to the internet)gw-2
: E/W (local) traffic gateway; declares 1 listener –*.local
(for APIs consumed within the network)route-1
: attached to the N/S (internet) gateway only; catches traffic directed to*.toystore.(website|io)
; declares 2 route rules:GET /v1/*
orPOST /v1/*
, and/assets/*
route-2
: attached to both gateways; catches traffic directed tostatus.(io|local)
; declares 1 route rule:/*
("catch-all")route-3
: attached to the E/W local gateway only; catches the complement of the traffic directed to*.local
(i.e. all exceptstatus.local
); declares 2 route rules:/v1/*
and/v2/*
State transitions
After setting the topology as described above, each RLP (from 1 to 4) will be created in sequence. This will trigger (4x) the reconciliation of the
WasmPlugin
CRs namedkuadrant-gw-1
andkuadrant-gw-2
, respectivelly for each gateway.A WasmPlugin contains the configs for the RLPs and corresponding affected HTTPRoutes, according to the
targetRefs
of each RLP. For example, whenrlp-1
is created, because it affects two HTTPRoutes (i.e.route-1
androute-2
, by declaringgw-1
as parent), a WasmPlugin CR will be created for this gateway, with namekuadrant-gw-1
, and containing instructions to activate the limits declared inrlp-1
for the route rules stated byroute-1
androute-2
. Later, whenrlp-3
is created, it replacesrlp-1
as the desired state for protectingroute-1
; nevertheless,rlp-1
remains valid for any traffic that flows throughgw-1
toroute-2
, untilrlp-4
is created.Similarly, and for all the other RLPs and affected HTTPRoutes, the table below describes the expected states of the WasmPluging CRs after the creation of each policy.
steps
reconciled
limits from RLPs
matches from
Expected final state
After the creation of all RLPs, any additional event that does not change the topology reconciles the following final state:
At the final state, the following back reference annotations are expected to be found in each of the targeted network resources:
Steps ① → ⑦
① Setup
② Create the Gateways
③ Create the HTTPRoutes
Check no WasmPlugins created at this stage:
kubectl get wasmplugins -A # No resources found
④ Create
rlp-1
targettinggw-1
Check the WasmPlugin for gateway
gw-1
:Expected output
No WasmPlugin for gateway
gw-2
should exist at this point.Check the Limitador CR:
Expected output
⑤ Create
rlp-2
targettinggw-2
Check the WasmPlugin for gateway
gw-2
:Expected output
The WasmPlugin for gateway
gw-1
should be unchanged.Check the Limitador CR:
Expected output
⑥ Create
rlp-3
targettingroute-1
Check the WasmPlugin for gateway
gw-1
:Expected output
The WasmPlugin for gateway
gw-2
should be unchanged.Check the Limitador CR:
Expected output
⑦ Create
rlp-4
targettingroute-2
Check the WasmPlugins for both gateways:
Expected output
Expected output
Check the Limitador CR:
Expected output