Skip to content

ratelimit(api): support SourceIP#946

Merged
Xunzhuo merged 7 commits intoenvoyproxy:mainfrom
zirain:ratelimit-sourceip
Feb 14, 2023
Merged

ratelimit(api): support SourceIP#946
Xunzhuo merged 7 commits intoenvoyproxy:mainfrom
zirain:ratelimit-sourceip

Conversation

@zirain
Copy link
Copy Markdown
Member

@zirain zirain commented Jan 30, 2023

Signed-off-by: hejianpeng hejianpeng2@huawei.com

xref: #852

cc @arkodg

ratelimit: support SourceIP
6fd3e6a 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain zirain requested a review from a team as a code owner January 30, 2023 07:42
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 30, 2023 •
edited
Loading

Codecov Report

Merging #946 (84c87e3) into main (488c44b) will decrease coverage by 1.60%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #946      +/-   ##
==========================================
- Coverage   63.70%   62.11%   -1.60%     
==========================================
  Files          54       55       +1     
  Lines        7834     8803     +969     
==========================================
+ Hits         4991     5468     +477     
- Misses       2528     2963     +435     
- Partials      315      372      +57
Impacted Files Coverage Δ
internal/infrastructure/kubernetes/deployment.go 54.97% <0.00%> (-31.76%) ⬇️
internal/infrastructure/kubernetes/service.go 40.81% <0.00%> (-31.69%) ⬇️
internal/infrastructure/kubernetes/labels.go 77.77% <0.00%> (-22.23%) ⬇️
internal/infrastructure/kubernetes/configmap.go 53.93% <0.00%> (-21.93%) ⬇️
...ternal/infrastructure/kubernetes/serviceaccount.go 58.62% <0.00%> (-19.35%) ⬇️
internal/infrastructure/kubernetes/infra.go 27.63% <0.00%> (-16.12%) ⬇️
internal/xds/translator/runner/runner.go 75.67% <0.00%> (-10.54%) ⬇️
internal/gatewayapi/route.go 85.23% <0.00%> (-3.95%) ⬇️
internal/xds/translator/ratelimit.go 93.33% <0.00%> (-2.79%) ⬇️
internal/gatewayapi/contexts.go 76.79% <0.00%> (-2.69%) ⬇️
... and 14 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

fix gen-check
f45c992 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
arkodg
arkodg reviewed Jan 30, 2023
View reviewed changes
arkodg
arkodg reviewed Jan 30, 2023
View reviewed changes
@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Jan 30, 2023

thanks for kickstarting the design for ratelimiting based on source IP, added comments

hejianpeng added 2 commits January 31, 2023 10:25
address arkodg's comments
34258dd 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
fix gen
ae66643 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain zirain force-pushed the ratelimit-sourceip branch from 27ee42b to ae66643 Compare January 31, 2023 02:28
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Jan 31, 2023

thanks for kickstarting the design for ratelimiting based on source IP, added comments

updated

arkodg
arkodg reviewed Jan 31, 2023
View reviewed changes
rename to SourceIP
dd43324 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
arkodg
arkodg reviewed Feb 1, 2023
View reviewed changes
@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Feb 1, 2023

this design looks good @zirain , lgtm from my side, lets merge post 0.3.0 so the unimplemented API field doesnt show up in docs ?

arkodg
arkodg reviewed Feb 1, 2023
View reviewed changes
@danehans danehans added this to the 0.4.0-rc.1 milestone Feb 1, 2023
@danehans danehans added documentation Improvements or additions to documentation area/api API-related issues labels Feb 1, 2023
@arkodg
Update docs/latest/design/ratelimit.md
097ef45 
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: zirain <hejianpeng2@huawei.com>
@zirain zirain mentioned this pull request Feb 8, 2023
Closed
addres arko's comment
84c87e3 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain zirain force-pushed the ratelimit-sourceip branch from f4b5b23 to 84c87e3 Compare February 9, 2023 01:37
@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Feb 10, 2023 •
edited
Loading

thanks for updating the doc string !
LGTM from my side, hoping this API PR gets another review before it gets merged
cc @Xunzhuo @danehans @youngnick @skriss @AliceProxy

Xunzhuo
Xunzhuo approved these changes Feb 14, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Xunzhuo Xunzhuo left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, side comment, this defines the sourceIP api field in RateLimitFilter, but have we implemented it in EG?

@Xunzhuo Xunzhuo changed the title ratelimit: support SourceIP ratelimit(api): support SourceIP Feb 14, 2023
@Xunzhuo Xunzhuo merged commit 8133074 into envoyproxy:main Feb 14, 2023
@zirain zirain deleted the ratelimit-sourceip branch February 15, 2023 01:05
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Feb 15, 2023

LGTM, side comment, this defines the sourceIP api field in RateLimitFilter, but have we implemented it in EG?

will implement it later.

@tmsnan tmsnan mentioned this pull request Nov 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arkodg arkodg arkodg left review comments

@Xunzhuo Xunzhuo Xunzhuo approved these changes

Assignees

No one assigned

Labels

area/api API-related issues documentation Improvements or additions to documentation

Projects

None yet

Milestone

0.4.0-rc.1

Development

Successfully merging this pull request may close these issues.

5 participants

@zirain @codecov-commenter @arkodg @Xunzhuo @danehans