envoyproxy / java-control-plane

Java implementation of an Envoy gRPC control plane
Apache License 2.0
293 stars 136 forks source link

server: Implement protobuf validation #107

Closed rmichela closed 5 years ago

rmichela commented 5 years ago
codecov-io commented 5 years ago

Codecov Report

Merging #107 into master will decrease coverage by 0.44%. The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #107      +/-   ##
============================================
- Coverage     92.88%   92.43%   -0.45%     
  Complexity      150      150              
============================================
  Files            19       19              
  Lines           576      582       +6     
  Branches         48       48              
============================================
+ Hits            535      538       +3     
- Misses           32       35       +3     
  Partials          9        9
Impacted Files Coverage Δ Complexity Δ
...nvoyproxy/controlplane/server/DiscoveryServer.java 93.93% <50%> (-2.1%) 14 <0> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8e19277...445007d. Read the comment docs.

rmichela commented 5 years ago

@snowp @joeyb can you please take a look?

joeyb commented 5 years ago

Closing this for now since DiscoveryRequest doesn't actually have any validation rules to validate right now.

If validation rules are added and we re-open this, then we also need to consider the behavior when those validation rules fail. As-is, if we just reject the request then Envoy is likely going to just create another identical request and we'll end up in a failure loop.