grpc-gateway
grpc-gateway copied to clipboard
grpc-ecosystem
Feature request: protoc-gen-swagger pluggable validation
Our team makes heave use of https://github.com/envoyproxy/protoc-gen-validate and would like a way for those validations to end up in the swagger spec so we don't need to define them twice.
Hi Patrick! Thanks for your feature request. I'm not sure there's a clean way to do this in the swagger generator, as it would require it to import the protoc-gen-validate definitions, which isn't necessarily something we'll want to do.
Is it correct that you are able to do this at the moment but it requires you to add annotations both for protoc-gen-validate and protoc-gen-swagger?
Hey Johan, thanks for getting back. It does work by adding the annotations twice its just terribly sloppy in the proto code to do so. I'm wondering if there is some way to have the validation be pluggable to other validation libraries?
There isn't any obvious way that I can think of right now, unfortunately. We can keep this open in case anyone has a good idea of how to do it.
what about just going all in on protoc-gen-validate? It's well supported and then you get the validation on the proto end. I can't think of a scenario where you would want to validate the REST and not the proto
Could you elaborate a bit on what you mean by "going all in"?
One way of approaching this to me would be to rewrite protoc-gen-swagger in protoc-gen-star. If you did that you could load both modules into the protoc-gen-star system, but then I have a big questionmark on the flowchart of how to proceed. I don't think there is a system in protoc-gen-star for modules to intercommunicate. Maybe you could speak to how this would work a little bit?
The main concern for me is how we can integrate this with the existing generator without adding a dependency on protoc-gen-validate's proto definitions. It'd need to be done as some sort of plugin to maintain the independence of this project. I don't know of any such dynamic protoc plugins, so I think we'd have to break new ground.
The best solution for your use case may be to fork our swagger generator and add the logic yourself, unfortunately.
I think the grpc ecosystem would greatly benefit from solving this in cohesive manner. I don't think this problem is going away and anyone using validation with the gateway would have the same issue.
I would propose supporting protoc-gen-validate options as input to protoc-gen-swagger and possibly deprecate the protoc-gen-swagger validation options over time.
From a end-user perspective this would simply mean using the protoc-gen-validate options for all validations. This would require them to have those proto definitions in their proto path, but only if they are using validation. Normal swagger generation without validations should work without those proto definitions.
From an execution standpoint you would simply import the generated validation options like what happens here https://github.com/envoyproxy/protoc-gen-validate/blob/master/module/checker.go#L43. This would (probably) require wrapping the current protoc-gen-swagger execution in protoc-gen-star, but thats likely beneficial in general, as we've found that library incredibly useful with our work.
WDYT?
This would be a much less contentious issue if protoc-gen-validate was officially recognised, or there was an official validation solution. I suspect that is never going to happen, because of the consequences for backwards and forwards compatibility. Even considering that, protoc-gen-validate isn't even the only solution in the community, there's also https://github.com/mwitkow/go-proto-validators. Should we choose one and force our users to use it? Should we support all the alternatives that exist?
I don't know if we can commit to it in this repo. So far we've been sticking to definitions in the googleapis, and I think we'd like to keep dependencies to those alone.
Unfortunately, I think I have to agree with @johanbrandhorst on this. Unless Google decides to bless one of the validators as the "official" validator we are unlikely to adopt it in here. That said, if you were to write a generator that emitted the message validation definitions in swagger's format you would be able to use a tool like jq to merge the two together to pretty good effect.
So because google doesn't support it we're going to make people write the validations twice? And when would someone have REST validations and not gRPC ones?
I think we could pretty simply support the validators as people need them. Start with protoc-gen-validate and then if people want another validator supported they could write it. If its a concern, then continue to support the protoc-gen-swagger options for people.
I don't really see any downside from this projects perspective, it would only open up capabilities and congruency with REST options. If folks don't want to use it, nothing changes for them.
i' need something like you, but this project tightly linked with capabilities of other packages in repo, so you can't generate swagger for path variable in streaming request for example. Also swagger only 2.0 and absence of validators. But i don't have much man power to write this from scratch based on protoc-gen-star or protoc-gen-gotemplate now
@pbarker, we are a set of volunteers that are building what we need. Other volunteers are welcome to build what they need. If this is insufficient, I have a contracting rate and my email is in many commits on this repo. Hit me up if you need this and we can talk about what it will take to get it done.
There are plenty of resources on our team to work on this, I'm more just looking to see if this would be allowed to be fixed in the upstream. We really don't want to keep a fork going and we also really don't want to write all of our validations twice ¯_(ツ)_/¯
I'd be happy with a solution that doesn't require this repo to depend on protoc-gen-validate. If we can somehow add some pluggability to the swagger generator it would be fine. It would probably be easiest for you to fork the generator though.