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

add support for proxy protocol on HTTP listener #1777

Merged
merged 2 commits into from
Jan 19, 2021
Merged

add support for proxy protocol on HTTP listener #1777

merged 2 commits into from
Jan 19, 2021

Conversation

calebdoxsey
Copy link
Contributor

@calebdoxsey calebdoxsey commented Jan 15, 2021 •
edited
Loading

Summary

This PR adds a new config option for requiring incoming HTTP requests to use the HAProxy proxy protocol (either version 1 or version 2).

Checklist:

  • add related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • ready for review

@calebdoxsey calebdoxsey requested a review from a team as a code owner January 15, 2021 19:01
@codeclimate
Copy link

codeclimate bot commented Jan 15, 2021 •
edited
Loading

Code Climate has analyzed commit 7835b37 and detected 0 issues on this pull request.

View more on Code Climate.

@calebdoxsey
Copy link
Contributor Author

@travisgroth does require_proxy_protocol: true sound good for the option name?

@codecov
Copy link

codecov bot commented Jan 15, 2021 •
edited
Loading

Codecov Report

Merging #1777 (7835b37) into master (10912ad) will increase coverage by 0.2%.
The diff coverage is 57.1%.

@@           Coverage Diff            @@
##           master   #1777     +/-   ##
========================================
+ Coverage    57.4%   57.6%   +0.2%     
========================================
  Files         135     135             
  Lines        9698    9709     +11     
========================================
+ Hits         5571    5602     +31     
+ Misses       3586    3570     -16     
+ Partials      541     537      -4
Impacted Files Coverage Δ
config/options.go 48.5% <ø> (ø)
internal/controlplane/xds_listeners.go 57.9% <57.1%> (+5.4%) ⬆️
internal/tcptunnel/tcptunnel.go 46.3% <0.0%> (-0.9%) ⬇️
pkg/storage/redis/redis.go 75.1% <0.0%> (+1.0%) ⬆️
internal/autocert/manager.go 66.8% <0.0%> (+1.3%) ⬆️
internal/controlplane/xds.go 76.9% <0.0%> (+4.8%) ⬆️

@travisgroth
Copy link
Contributor

@calebdoxsey just for the sake of a shorter option, maybe use_proxy_protocol? It also maps to the same setting in nginx ingress.

@calebdoxsey calebdoxsey merged commit 09747aa into master Jan 19, 2021
@calebdoxsey calebdoxsey deleted the cdoxsey/242 branch January 19, 2021 12:57
calebdoxsey added a commit that referenced this pull request Jan 19, 2021
@calebdoxsey
reduce memory usage by handling http/2 coalescing via a lua script (#…
b2ceaa9 
…1779)

* add support for proxy protocol on HTTP listener (#1777)

* add support for proxy protocol on HTTP listener

* rename option, add doc

* reduce memory usage by handling http/2 coalescing via a lua script

* move script to file

* use wellknown

* fix integration test
contrun added a commit to contrun/pomerium that referenced this pull request Mar 31, 2021
@contrun
change require_proxy_protocol to use_proxy_protocol
19b8346 
I set `use_proxy_protocol` to be true in my yaml config. Envoy didn't use proxy protocol albeit. Both the documents and pomerium#1777 hint the name should be use_proxy_protocol.
@contrun contrun mentioned this pull request Mar 31, 2021
Merged
calebdoxsey pushed a commit that referenced this pull request Mar 31, 2021
@contrun
change require_proxy_protocol to use_proxy_protocol (#2043)
9980206 
I set `use_proxy_protocol` to be true in my yaml config. Envoy didn't use proxy protocol albeit. Both the documents and #1777 hint the name should be use_proxy_protocol.
github-actions bot pushed a commit that referenced this pull request Apr 5, 2021
@contrun @github-actions
change require_proxy_protocol to use_proxy_protocol (#2043)
4aab29a 
I set `use_proxy_protocol` to be true in my yaml config. Envoy didn't use proxy protocol albeit. Both the documents and #1777 hint the name should be use_proxy_protocol.
desimone pushed a commit that referenced this pull request Apr 6, 2021
@github-actions @contrun
change require_proxy_protocol to use_proxy_protocol (#2043) (#2058)
403718c 
I set `use_proxy_protocol` to be true in my yaml config. Envoy didn't use proxy protocol albeit. Both the documents and #1777 hint the name should be use_proxy_protocol.

Co-authored-by: contrun <uuuuuu@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@travisgroth travisgroth travisgroth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@calebdoxsey @travisgroth