build: evaluate using protolock in API build

[mattklein123]

See https://github.com/nilslice/protolock. Should be able to help us codify avoiding accidental breaking changes. The one thing that comes to mind is we are likely going to need some type of mixed mode where certain messages are locked and others are not locked while they are draft. Not sure if that is supported but probably pretty easy to do via some type of comment annotation.

cc @nilslice @rodaine @twoism @htuch

[nilslice]

@mattklein123 are your messages/services ever in draft mode on the master branch? If only in another branch as drafts, the proto.lock file would be checked in and compared against at its (git) committed state.

[mattklein123]

@nilslice we do have draft stuff committed to master. The way we do this currently is to use comment annotations: https://github.com/envoyproxy/envoy/blob/master/tools/protodoc/protodoc.py#L51

I'm wondering if potentially we could do something similar with protolock such that given annotations on a message we could set lock status for that message and submessages? That would allow us to lock non-draft protos but allow other ones to float that are draft/alpha status.

[nilslice]

@mattklein123 if you aren't opposed to dressing up a draft message with a more general annotation, i.e. // @protolock:skip then I'm pretty confident we can make it happen natively in the protolock comparison step.

The behavior there would be that an @protolock:skip annotated message would not be included as an entry to the definitions kept in a proto.lock file. Alternatively, the entry could be included in the lock file for the message, but it has an additional field to indicate that the comparison should skip this message.

[mattklein123]

@nilslice yup that sounds perfect.

[nilslice]

support for skipping messages and services via comment annotation in: nilslice/protolock#15

[nilslice]

Just updating the thread -- protolock finally has sufficiently complete support for oneof and map types, with enum coming soon. I've browsed this repo for integration points but am too unfamiliar with bazel and the build process. If anyone could point me in the right direction, I'd be happy to PR something when the enum support is ready for action.

[nilslice]

protolock now has full support for Enum, Map, and Oneof types within each pertinent rule func. I would be happy to implement the check before builds once I figure out where in Envoy this should occur. cc/ @mattklein123 @rodaine @twoism @htuch any pointers would be much appreciated.

[htuch]

@nilslice we basically want to apply this over all protos in https://github.com/envoyproxy/envoy/tree/master/api. We would want to ignore any in directories containing v2alpha, and a blacklist of some experimental protos. I think we should run this just before/after https://github.com/envoyproxy/envoy/blob/master/ci/do_ci.sh#L137, i.e. as one of the CI API tests.

[nilslice]

@htuch - excellent, thank you for the direction. I'll get a branch up and may ask for further assistance once I take an initial swing.

[htuch]

Moving this to post 1.12.0, since this is in the nice-to-have but not needed bucket.

[github-actions]

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 "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

[htuch]

@YaseenAlk is working on this.