feat: add protovalidate interceptor to default chain#35
Conversation
Add ProtoValidateInterceptor and ProtoValidateStreamInterceptor using go-grpc-middleware's protovalidate interceptor with buf.build/go/protovalidate. Default on in the interceptor chain (after TraceId, before Prometheus metrics). Zero cost if no validation annotations in proto. Users can add custom validator options via SetProtoValidateOptions() during init. Uses GlobalValidator by default for shared caching.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 8 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds protovalidate-based request validation to the gRPC server interceptor chain. It introduces three new exported functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds request validation to the default gRPC server interceptor chains by integrating Buf’s protovalidate via go-grpc-middleware, with optional init-time customization.
Changes:
- Introduces
ProtoValidateInterceptor()andProtoValidateStreamInterceptor()backed byprotovalidate.GlobalValidatorby default. - Adds
SetProtoValidateOptions(...)and wires protovalidate into the default unary and stream server interceptor chains (before Prometheus metrics). - Updates module dependencies/docs to include protovalidate.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| README.md | Documents new protovalidate interceptors and init-time configuration API. |
| interceptors.go | Implements protovalidate interceptors/options and inserts them into default chains. |
| go.mod | Adds protovalidate dependency (and indirect transitive deps). |
| go.sum | Records checksums for newly added dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interceptors.go`:
- Around line 260-266: The stream interceptor is discarding the error from
protovalidate.New() in ProtoValidateStreamInterceptor; extract validator
creation into a helper (e.g., createProtoValidator or initProtoValidator) that
calls protovalidate.New(protoValidateOpts...) and panics if it returns an error,
then have ProtoValidateStreamInterceptor call that helper and pass the returned
validator to protovalidate_middleware.StreamServerInterceptor (falling back to
protovalidate.GlobalValidator only if no opts exist).
- Around line 250-256: The ProtoValidateInterceptor currently discards the error
from protovalidate.New(protoValidateOpts...), which can produce a nil validator
and cause runtime panics; update ProtoValidateInterceptor to check the returned
error from protovalidate.New (when protoValidateOpts is non-empty) and panic (or
log.Fatal) with a clear message if err != nil so initialization fails fast;
otherwise continue to return protovalidate_middleware.UnaryServerInterceptor(v)
on success, and keep the existing fallback to
protovalidate_middleware.UnaryServerInterceptor(protovalidate.GlobalValidator)
when protoValidateOpts is empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7b76428-d8f7-4f30-8f88-466a16dfb2f6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
README.mdgo.modinterceptors.go
- Cache validator via sync.Once so it's created once, not per call - Log error and fall back to GlobalValidator if custom options fail - Extract getProtoValidator() helper used by both unary and stream
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
ProtoValidateInterceptor()andProtoValidateStreamInterceptor()using go-grpc-middleware's protovalidate interceptorprotovalidate.GlobalValidatorby default (shared caching, zero config)SetProtoValidateOptions()for custom constraints during initInvalidArgumentgRPC status on validation failureChain order: ResponseTimeLogging → TraceId → ProtoValidate → Prometheus → ServerError → NewRelic → PanicRecovery
Test plan
go test -race ./...passesmake lint— 0 issuesSummary by CodeRabbit
Release Notes
New Features
Documentation