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

feat: add interceptor for bufbuild/protovalidate #614

Merged
merged 10 commits into from
Aug 15, 2023
Merged

feat: add interceptor for bufbuild/protovalidate #614

merged 10 commits into from
Aug 15, 2023

Conversation

gvencadze
Copy link
Contributor

Interceptor for validating incoming requests using bufbuild/protovalidate with Google's Common Expression Language (CEL) support (+ legacy protoc-gen-validate)

Changes

  1. Add interceptor that validates incoming requests using bufbuild/protovalidate-go under the hood
  2. For unit-testing purposes I've added new proto contract that uses annotations from protovalidate, because it doesn't generate Validate() interface, so I can't reuse already existing contract and codegen
  3. Add protovalidate to buf.yaml as depedency, I've tried to use it as local, but without any success. (If usage of protoc for codegen is important, I'll rework this moment)

Verification

I've created sample project, with some proto APIs and check both modes of protovalidate - legacy (like in protoc-gen-validate) and new powered by Google's Common Expression Language (CEL)

I've added examples only for new mode

Copy link
Collaborator

@johanbrandhorst johanbrandhorst 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 this, this is great!

interceptors/protovalidate/doc.go Show resolved Hide resolved
interceptors/protovalidate/doc.go Show resolved Hide resolved
interceptors/protovalidate/doc.go Outdated Show resolved Hide resolved
interceptors/protovalidate/example_stream_test.go Outdated Show resolved Hide resolved
interceptors/protovalidate/protovalidate_test.go Outdated Show resolved Hide resolved
testing/testvalidate/interceptor_suite.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Perfect, thanks a lot for this :)

Makefile Outdated Show resolved Hide resolved
@johanbrandhorst
Copy link
Collaborator

The lint error seems to imply we need to rerun the generation script?

https://github.com/grpc-ecosystem/go-grpc-middleware/actions/runs/5872315640/job/15923608916?pr=614

@johanbrandhorst johanbrandhorst merged commit eef4b06 into grpc-ecosystem:main Aug 15, 2023
6 checks passed
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

@akshayjshah
Copy link
Contributor

@johanbrandhorst and @gvencadze - awesome to see this has already landed! Are you open to a follow-up PR that adds an unimplemented Option interface to this package, so that the two exported constructors have some room to evolve without breaking back-compat?

@johanbrandhorst
Copy link
Collaborator

Sounds reasonable, this hasn't hit a released version yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants