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

httpproxy: Add support for ip-based filtering #5008

Merged
merged 5 commits into from
Apr 25, 2023

Conversation

ecordell
Copy link
Contributor

@ecordell ecordell commented Jan 25, 2023

Configures Envoy's envoy.filters.http.rbac per route via HTTPProxy.

See #4990 for design

Fixes #3693

@ecordell ecordell requested a review from a team as a code owner January 25, 2023 23:55
@ecordell ecordell requested review from skriss and sunjayBhatia and removed request for a team January 25, 2023 23:55
@ecordell ecordell force-pushed the ipfilter branch 3 times, most recently from d860bd4 to 3d143ce Compare January 26, 2023 02:12
Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

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

this looks great so far, awesome to have an implementation to play with

@sunjayBhatia sunjayBhatia requested a review from tsaarni February 1, 2023 22:28
@sunjayBhatia sunjayBhatia added the release-note/major A major change that needs more than a paragraph of explanation in the release notes. label Feb 1, 2023
@github-actions
Copy link

The Contour project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 14d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the PR is closed

You can:

  • Mark this PR as fresh by commenting or pushing a commit
  • Close this PR
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 16, 2023
@ecordell
Copy link
Contributor Author

I'll rebase this when the design is finalized

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 17, 2023
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

The Contour project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 14d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the PR is closed

You can:

  • Mark this PR as fresh by commenting or pushing a commit
  • Close this PR
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 3, 2023
@ecordell ecordell force-pushed the ipfilter branch 2 times, most recently from 13d0876 to d7a4308 Compare March 20, 2023 22:56
@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 21, 2023
@skriss
Copy link
Member

skriss commented Mar 24, 2023

@ecordell thanks for the PR! One thing to put on your radar is that we will want to add some user documentation as part of this PR -- should be a new page in site/content/docs/main/config, and added to the TOC via site/data/docs/main-toc.md.

@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Merging #5008 (18a54cc) into main (7572a41) will increase coverage by 0.11%.
The diff coverage is 93.84%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5008      +/-   ##
==========================================
+ Coverage   77.78%   77.89%   +0.11%     
==========================================
  Files         138      138              
  Lines       18068    18198     +130     
==========================================
+ Hits        14054    14176     +122     
- Misses       3742     3748       +6     
- Partials      272      274       +2     
Impacted Files Coverage Δ
internal/dag/dag.go 96.64% <ø> (ø)
internal/dag/httpproxy_processor.go 92.09% <87.30%> (-0.24%) ⬇️
internal/envoy/v3/listener.go 98.43% <100.00%> (+0.01%) ⬆️
internal/envoy/v3/route.go 80.15% <100.00%> (+1.71%) ⬆️

apis/projectcontour/v1/httpproxy.go Outdated Show resolved Hide resolved
apis/projectcontour/v1/httpproxy.go Show resolved Hide resolved
test/e2e/httpproxy/ip_filtering_test.go Outdated Show resolved Hide resolved
test/e2e/httpproxy/ip_filtering_test.go Outdated Show resolved Hide resolved
test/e2e/httpproxy/ip_filtering_test.go Outdated Show resolved Hide resolved
@ecordell
Copy link
Contributor Author

@skriss thanks for the initial review! addressed everything that you noted.

@sunjayBhatia sunjayBhatia self-requested a review April 3, 2023 17:04
@skriss
Copy link
Member

skriss commented Apr 10, 2023

@ecordell just a friendly ping that the end of this release cycle is rapidly approaching; if you could address the above comments and fix the small merge conflict then we can take another look here. Thanks!

@ecordell
Copy link
Contributor Author

@skriss Addressed the comments and rebased, please let me know if you see anything else I need to do to get this into the next release

@ecordell ecordell force-pushed the ipfilter branch 3 times, most recently from b791b1a to fff01bd Compare April 18, 2023 12:12
@ecordell
Copy link
Contributor Author

@sunjayBhatia @skriss any final comments? I want to make sure I've covered everything so this can get into the next release, feel free to ping me in kube slack (same username) if there's anything else you'd like me to address.

site/content/docs/main/config/ip-filtering.md Show resolved Hide resolved
@@ -2,6 +2,9 @@ module github.com/projectcontour/contour

go 1.19

// remove once https://github.com/cert-manager/cert-manager/issues/5953 is fixed
replace github.com/Venafi/vcert/v4 => github.com/jetstack/vcert/v4 v4.9.6-0.20230127103832-3aa3dfd6613d
Copy link
Member

Choose a reason for hiding this comment

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

huh never end up running go list ... very often but confirmed this is an issue

@skriss
Copy link
Member

skriss commented Apr 20, 2023

@skriss Addressed the comments and rebased, please let me know if you see anything else I need to do to get this into the next release

Thanks @ecordell! Looks like @sunjayBhatia approved and I will take a final look as soon as I can (at KubeCon EU right now). We definitely want to get this into the upcoming release as well.

Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

thanks for the updates @ecordell, LGTM. One last tiny merge conflict to resolve and this should be ready for merge

Configures Envoy's envoy.filters.http.rbac per
route via HTTPProxy.

See API docs in this commit for details.

Fixes projectcontour#3693

Signed-off-by: Evan Cordell <[email protected]>
Signed-off-by: Evan Cordell <[email protected]>
@skriss skriss merged commit f3eb153 into projectcontour:main Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/major A major change that needs more than a paragraph of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add IP allowlist and blocklist functionality
4 participants