Stable Envoy API versions

[htuch]

We've had feedback from folks (in particular gRPC-LB) that the current Breaking Change Policy is insufficient for handling long-term compatibility at the wire and generated proto level.

Specifically:

1. Coupling API field validity and deprecation to Envoy release cycles is an implicit API state that mostly makes sense for Envoy but not for independent clients. Even Structured version and feature reporting #5405 introduces too much pain for non-Envoy clients.
2. Upgrading to oneof and other wire compatible changes breaks Go code generation for protobuf. There are no safe mutations to existing proto fields.

The proposal here is that we switch to semantic versioning by making use of package namespaces. Any API that is not marked as alpha, e.g. v2alpha, becomes immutable modulo field addition. Deprecation would happen by moving to v3, v4, etc.

What do folks think? CC @envoyproxy/maintainers @alyssawilk

[snowp]

One concern that springs to mind is the issues we've had with moving service between packages, since that breaks gRPC wire compatibility due to the :path header. Would this make changes to any protobufs that's referenced by a service very painful?

[htuch]

@snowp package namespace changes would imply service endpoint changes. The management server would need to support both during a transition period.

[moderation]

Not sure if this is relevant for this particular issue but CockroachDB recently switched away from SemVer - Why we're switching to calendar versioning. Along the same lines the SemVer specification is getting an upgrade - What’s next for SemVer.

[mattklein123]

What do folks think?

I'm not thrilled about this since I do think it will increase deprecation overhead/velocity quite a bit. I think I'm not fully understanding the problem that gRPC is facing. Can you potentially describe in more detail?

[htuch]

@markdroth can you provide a summary for your team?

[dfawley]

API stability is an extremely important factor for anyone outside Envoy looking to adopt the xDS protocol. gRPC needs the ability to easily express to its users what version(s) of the protocol it works with. At this point, we cannot say "gRPC works with xDS v2", because features present in the initial release of v2 may have been removed and replaced -- or may be in the future -- in accordance with the breaking change policy. This means if gRPC uses the old fields, it won't work with new servers and vice-versa.

In addition to those concerns, gRPC-Go has twice in the past two months (#6118, #5432) been broken by wire-compatible changes to the proto format that break the go generated API. This is not only disruptive to our development, but it also affects our users: they will need to vendor these protos at the right version. The problem with this is, if they are developing a project that has other dependencies on the xDS API, they may not be able to find a single version that satisfies all of their dependencies (some may require newer versions and some may require older versions). Multiple copies of the same proto definitions are not permitted, and will result in init-time panics.

[mattklein123]

re: the Go issues, I think that is more straightforward. We can just not do the things that break Go such as add a oneof after the fact (and anything else).

The other concern is much trickier. I understand it, but adhering to it will greatly reduce our velocity. Can we discuss this on the next community call and brainstorm possible outcomes?

[dfawley]

@mattklein123 In order to maintain backward compatibility for the Go generated API, the other condition also needs to be met. Removing a field will break the generated code API (in addition to breaking wire compatibility).

I have no desire to reduce your velocity. New features can be added with a clear "experimental" tag to be exempted from these requirements. gRPC and other users requiring a stable API would not use those features until they are finalized.

Discussing this further at the next community meeting sounds fine.

[htuch]

I agree that discussing this in the community meeting would be the most efficient way to move forward. Doug, can you and interested gRPC folks join https://calendar.google.com/event?action=TEMPLATE&tmeid=NzI4c2E5YWJjbDRnMmp1amdsOW5uNTJsMjZfMjAxOTAzMjZUMTYwMDAwWiA4YnNmYzhtMmRmOG1hZDJqdGdmNmY4b25qNEBn&tmsrc=8bsfc8m2df8mad2jtgf6f8onj4%40group.calendar.google.com&scp=ALL on 3/26?

…

On Wed, Mar 13, 2019 at 5:17 PM Doug Fawley ***@***.***> wrote: @mattklein123 <https://github.com/mattklein123> In order to maintain backward compatibility for the Go generated API, the other condition also needs to be met. Removing a field will break the generated code API (in addition to breaking wire compatibility). I have no desire to reduce your velocity. New features can be added with a clear "experimental" tag to be exempted from these requirements. gRPC and other users requiring a stable API would not use those features until they are finalized. Discussing this further at the next community meeting sounds fine. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#6271 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKaLvxhDKFmbTIcMkTUR0nSoXuZJsUQ1ks5vWWrngaJpZM4brnnv> .

[dfawley]

@htuch, yes, I plan to attend, and I have invited a few others. Thanks for sharing the link.

[alyssawilk]

Resharing Doug's doc which was presented at the community call via my chromium account (so folks can read and comment)
https://docs.google.com/document/d/1nAomWSjvSSFCNZ1wQJ7D01f5VGDGLn5zcNxcDozYqYI/edit?ts=5c9a516b#heading=h.hv6zo3sm8vz7

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[htuch]

We now have a design document to share:

• Full design document
• TL;DR summary

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[htuch]

It looks like we have general buy-in on the proposal, so the following calendar is what I think I can commit to:

Before end of Q2:

• TDS (runTime Discovery Service (TDS) #6708)
• Unknown field support for dynamic config, stats tracking, etc. (Log and count unknown proto fields #6818 and bootstrap and xDS should have separate support for allow unknown fields #6651).
• Add presubmit check forcing sign-off from @envoyproxy/api-shepherds for any changes to api/. @envoyproxy/udpa-wg will be tagged on all such PRs for optional review.
• Manual inspection of all API changes to avoid any breaking changes at either wire or language stub level.
• Minor/patch version proto annotations added to v2 API.
• Semantic version reporting in xDS for core APIs.
• Introduce v3alpha for some relatively isolated example package. Write a translation module in Envoy implementation, providing the pattern for the "translate old versions of API to new on API ingest".
• Node enhancements for user agent and client feature reporting.

Before end of Q3:

• Node enhancements for extension version reporting, corresponding filter interface changes to support this.
• TBD tool enforcement of minor/patch version updates when material proto changes occur.
• TBD tool to enforce ODR and ensure transitive upgrades work as expected.
• TBD tool to enforce package dependency constraints, e.g. verify the property that stable APIs do not depend on experimental APIs, that the latest stable version of any API only depends on the latest stable versions (including transitively) of other APIs. Also, to ensure we have well formed package namespace.
• TBD tooling to support reducing the manual stub/code toil when moving from vN to v(N+1) where most fields are the same.
• Finalize incremental xDS APIs (config: partial rejections in incremental xDS #5739).
• protolock presubmit (build: evaluate using protolock in API build #3368, build: evaluate using protolock in API build #3368)
• Extend protolock to handle PGV annotations.

At the end of Q3, we will start the annual release calendar for new API major versions, and the APIs that were in v3alpha will be frozen as v3, v4alpha will begin, etc. Support for v2 APIs that have newer major versions will be removed at the end of 2020 Q3.

Field will continue to be deprecated via annotation according to https://github.com/envoyproxy/envoy/blob/master//CONTRIBUTING.md#breaking-change-policy. However, we will not remove any further fields from the v2 fields from now on. Deprecated fields will become disabled by default following the existing schedule, this may be overriden by TDS. The goal of deprecation across major versions will be to allow advanced notion of significant structural changes to the API when moving from vN to v(N+1), but no fields will be removed in vN.

@mattklein123 @alyssawilk @dfawley @envoyproxy/api-shepherds @envoyproxy/maintainers (and anyone else who cares about API stability!) does this sound like a reasonable plan-of-record?

[mattklein123]

@htuch +1 looks great to me. Thanks for driving.

[htuch]

See #8085 for the v3alpha and v3 release plans. The burndown is at https://github.com/envoyproxy/envoy/issues?q=is%3Aopen+is%3Aissue+label%3A%22v3+API%22+milestone%3A1.12.0

[htuch]

I'm going to consider this issue resolved via the introduction of v3, additional improvements, e.g. minor versioning, can be tracked via independent issues.