-
Notifications
You must be signed in to change notification settings - Fork 4
feat(proxyproto): support serve http(s) server through proxy servers and load balancers #40
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
Conversation
Reviewer's Guide by SourceryThis pull request introduces proxy protocol support for HTTP(S) servers enabling them to operate behind proxy servers and load balancers. The changes include documentation updates, dependency bumps, and a new proxyproto package that provides utilities to parse both v1 and v2 of the proxy protocol, wrap network connections and listeners, and integrate proxy protocol handling in server startup functions. Class diagram for the proxyproto packageclassDiagram
class Header {
+net.Addr LocalAddr
+net.Addr RemoteAddr
+int Version
+Command Command
+Protocol Protocol
+[]byte RawTLVs
}
class conn {
-net.Conn Conn
-*bufio.Reader r
-*Header h
-bool isLoaded
-sync.Once once
+Read(b []byte) int
+LocalAddr() net.Addr
+RemoteAddr() net.Addr
-tryUseProxy()
}
class listener {
-net.Listener Listener
+Accept() net.Conn
}
%% Relationships
conn ..> Header : uses
listener ..> conn : wraps
%% Global functions
class NewConn {
<<function>>
}
class NewListener {
<<function>>
}
class ListenAndServe {
<<function>>
}
class ListenAndServeTLS {
<<function>>
}
%% Functions are not methods of any class, so they are shown as standalone elements
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Here's the code health analysis summary for commits Analysis Summary
|
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.
Hey @cnlangzi - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Detected hardcoded secret key for cookie signing. (link)
- Detected hardcoded secret key for cookie verification. (link)
Overall Comments:
- Consider adding a benchmark to
conn_test.go
to measure the performance impact of usingproxyproto.Conn
.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🔴 Security: 2 blocking issues
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
+ Coverage 92.01% 92.10% +0.09%
==========================================
Files 38 43 +5
Lines 1427 1647 +220
==========================================
+ Hits 1313 1517 +204
- Misses 78 91 +13
- Partials 36 39 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Changed
Fixed
Added
cookie
,hsts
andproxyproto
sections on README.mdTests
Tasks to complete before merging PR:
make unit-test
to check for any regressions 📋make lint
to check for any issuesSummary by Sourcery
New Features: