feat(api): Remove gin dependency #4128
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replace gin-gonic/gin with standard net/http.ServeMux (Go 1.22+ patterns) - Implement custom router with middleware chaining support - Migrate all handlers from gin.Context to standard http.HandlerFunc - Rewrite JWT middleware using golang-jwt/jwt/v4 - Fix trusted proxies vs TrustedIPs confusion (store resolved IP in context) - Fix middleware bypass for 404/405 requests - Remove strict JSON validation for backwards compatibility - Fix double middleware execution - All requests now properly go through middleware pipeline
- Replace net.IPNet with netip.Prefix for CIDR networks - Replace net.ParseIP() with netip.ParseAddr() for IP addresses - Replace net.ParseCIDR() with netip.ParsePrefix() for CIDR parsing - Update ResolveClientIP to accept both netip.Prefix and net.IPNet for compatibility - Update convertTrustedProxies to return []netip.Prefix - Update ClientIPMiddleware to use netip.Prefix - Update controllers to use netip for IP parsing and comparison - Maintain backwards compatibility with existing net.IPNet types from config
|
@LaurenceJJones: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
@LaurenceJJones: There are no area labels on this PR. You can add as many areas as you see fit.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
LaurenceJJones
changed the title
- Remove trailing newline from JSON responses (use json.Marshal instead of Encoder) - Return 'incorrect Username or Password' for auth failures (security: don't reveal if machine exists) - Add specific error for empty cookie tokens - Reject certificates signed directly by root CA when intermediate is expected - Update tests to match new error messages
LaurenceJJones
force-pushed
the
remove-gin-dependency
branch
from
a15ceec to
a764762
Compare
- Iterate backwards (right to left) through X-Forwarded-For and X-Real-IP headers - Extract helper functions: isTrustedProxy, findFirstUntrustedIP, isRemoteAddrTrusted - Handle multiple IPs in X-Real-IP header (comma-separated) - Simplify type handling: only accept []netip.Prefix (removed []net.IPNet compatibility) - Fix empty RemoteAddr handling in tests
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4128 +/- ##
==========================================
- Coverage 62.86% 62.64% -0.22%
==========================================
Files 464 471 +7
Lines 33290 33748 +458
==========================================
+ Hits 20927 21143 +216
- Misses 10241 10449 +208
- Partials 2122 2156 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
LaurenceJJones
force-pushed
the
remove-gin-dependency
branch
from
0051f64 to
c652203
Compare
- Fix contextcheck: Add nolint for intentional context.Background() usage - Fix errcheck: Remove unused nolint directives - Fix nilerr: Add nolint for correct false, nil return pattern - Fix revive: Change MachineIDKey to typed context key - Fix unused: Remove unused realm field and isBrokenConnection function - Fix unconvert: Remove unnecessary conversion - Fix ineffassign: Remove unused path variable - Update test: Change expected error from 'cookie token is empty' to 'token not found' to match actual behavior when no cookie is present
LaurenceJJones
force-pushed
the
remove-gin-dependency
branch
from
c652203 to
d7c7a17
Compare
LaurenceJJones
added
kind/enhancement
Access logs should only be written when the logger level allows info-level messages. This ensures that when log level is set to 'error', access logs don't appear in stderr, matching the expected behavior in tests.
LaurenceJJones
changed the title
Set Code: 200 in successful login and refresh token responses to match the swagger spec and fix lp-get-token test that expects code field.
- Refactor StreamDecisionChunked to extract helper functions - Simplify ResolveClientIP and extract helper functions - Fix Unix socket handling to check forwarded headers before fallback - Treat Unix socket (@) as 127.0.0.1 for trusted proxy validation - Reduce verbose comments throughout - Fix all golangci-lint issues (nestif, errcheck, contextcheck, etc.)
- Set route patterns before middleware runs to fix Prometheus cardinality - Fix gzip middleware to properly close original body (prevents socket leaks) - Forward optional ResponseWriter interfaces (Flusher, Hijacker, Pusher, ReaderFrom) - Fix filters map mutation by working on copies - Pre-wrap router middleware chain for better performance - Modernize loops (range over int, maps.Copy)
- Extract findPatternWithVariables helper to reduce nesting - Change matchesPattern to standalone function (unused receiver) - Fix intrange: use range over slice directly - All golangci-lint errors resolved
- Fix nolintlint error by placing directive before function declaration - All golangci-lint errors resolved
- Remove locks and atomic.Value since routes are registered once at startup - Add Build() method to finalize router after all routes are registered - Add built flag to prevent modifications after Build() - Store route patterns in map for routePatternMiddleware - Set unknown routes to 'invalid-endpoint' to bound metrics cardinality - Move startTime in PrometheusMiddleware to just before handler execution
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove Gin dependency and migrate to net/http
Replaced
gin-gonic/ginwith Go's standardnet/http, eliminating 3 direct dependencies (plus ~36 transitive) while maintaining 100% API functionality.Dependencies Removed
github.com/gin-gonic/gingithub.com/gin-contrib/gzipgithub.com/appleboy/gin-jwt/v2Changes
net/http.ServeMux(Go 1.22+ patterns)http.HandlerFunchandlersgolang-jwt/jwt/v4net/netipdraft PR needs to be fully tested and edge cases before approval