envoyproxy/envoy

proto3 optional for all scalar fields

Open

#14,973 opened on Feb 7, 2021

View on GitHub
 (13 comments) (0 reactions) (0 assignees)C++ (5,373 forks)batch import
api/v3enhancementhelp wantedpriority/high

Repository metrics

Stars
 (27,997 stars)
PR merge metrics
 (Avg merge 8d) (378 merged PRs in 30d)

Description

As discussed in https://github.com/envoyproxy/envoy/pull/14880#issuecomment-772863659, we would like to both:

  • Move to using proto3 optional support over WKTs for scalar fields. This should have both wire, memory, text proto and other overhead wins.
  • Use optional for all scalar fields, since this future proofs client default choices. We can switch from default true to false in Envoy without a breaking API change at any point. It's hard to anticipate always which fields this is needed for, so we can do it everywhere.

There are some things we need to do to make this happen:

  1. Validate that all known code generators support optional in released versions; probably want to make sure this covers C++/Java/Go/Python at least.
  2. Potentially update https://github.com/envoyproxy/protoc-gen-validate so that WKT wrapper flow also works for optional. E.g. if the default value is 3 and the PGV annotation is [(validate.rules).int32.gt = 3], today this would fail PGV validation as PGV is unaware of optional and treats the field as default 0. This would require explicitly providing defaults in an annotation, which is something we have talked about in https://github.com/envoyproxy/envoy/issues/10517 and could fold into this work.
  3. Add a check_format and api/STYLE.md update to enforce the property "all new scalar fields are optional".
  4. Validate that the JSON/YAML value for "unset" makes sense with optional. JSON/YAML stability and usability is a 1st class consideration for the xDS APIs.

@envoyproxy/api-shepherds

Contributor guide