tooling: Add buf bazel dependency and tests to evaluate it (#17515)

Follow-up to #17375 where it was agreed that protolock is not actively maintained enough to depend on. This PR "migrates" the tests from that PR to use buf instead, and also cleans some of the code per a few of the review comments. Still a few outstanding points:

- buf build on the envoy/api folder requires several protobuf dependencies such as udpa to be available to buf to consume. Suggested solution by buf is to point buf's config to necessary BSR modules that the buf team is hosting.
  - These lines are commented out in this PR as I had some trouble automating it for the tests, and it is not necessary for the tests to pass
  - May introduce issues if buf is not pointing to the same version of modules that bazel builds for envoy. May need to introduce some way to couple them, or (ideally) find a way to run the breaking change detector without building the dependencies
- Currently bazel is using a binary release of buf (for linux). Goal is to move to building it from source in the near future
- It may be in our interest to expand the list of API-breaking-change rules (buf provides an extensive list of rules we could adopt)

Risk Level: Low

Testing: Tests that evaluate buf against "allowed" and "breaking" protobuf API changes. Currently 4 tests are skipped - 3 of them are PGV-related (we need to communicate our desired PGV rules to the buf team so they may add them in the near future). The 4th is a test I had originally written to evaluate protolock but may not apply to buf ("forcing" a breaking change) - refer to comments

Docs Changes:
Release Notes:
Platform Specific Features: buf binary imported by bazel is linux-only. Hopefully the ["manual"] tags attribute prevents any issues for non-linux users

Signed-off-by: Yaseen Alkhafaji <[email protected]>

api/bazel/repositories.bzl

@@ -47,6 +47,11 @@ def api_dependencies():
         name = "opentelemetry_proto",
         build_file_content = OPENTELEMETRY_LOGS_BUILD_CONTENT,
     )
+    external_http_archive(
+        name = "com_github_bufbuild_buf",
+        build_file_content = BUF_BUILD_CONTENT,
+        tags = ["manual"],
+    )
 
 PROMETHEUSMETRICS_BUILD_CONTENT = """
 load("@envoy_api//bazel:api_build_system.bzl", "api_cc_py_proto_library")
@@ -150,3 +155,17 @@ go_proto_library(
     visibility = ["//visibility:public"],
 )
 """
+
+BUF_BUILD_CONTENT = """
+package(
+    default_visibility = ["//visibility:public"],
+)
+
+filegroup(
+    name = "buf",
+    srcs = [
+        "@com_github_bufbuild_buf//:bin/buf",
+    ],
+    tags = ["manual"], # buf is downloaded as a linux binary; tagged manual to prevent build for non-linux users
+)
+"""

api/bazel/repository_locations.bzl

@@ -118,4 +118,16 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         urls = ["https://github.com/open-telemetry/opentelemetry-proto/archive/v{version}.tar.gz"],
         use_category = ["api"],
     ),
+    com_github_bufbuild_buf = dict(
+        project_name = "buf",
+        project_desc = "A new way of working with Protocol Buffers.",  # Used for breaking change detection in API protobufs
+        project_url = "https://buf.build",
+        version = "0.48.2",
+        sha256 = "ee0ea6c4a7bbb016d79b056905c0a1f018e7c5e47b37038c993a77b1bc732c0d",
+        strip_prefix = "buf",
+        urls = ["https://github.com/bufbuild/buf/releases/download/v{version}/buf-Linux-x86_64.tar.gz"],
+        release_date = "2021-07-30",
+        use_category = ["api"],
+        tags = ["manual"],
+    ),
 )

tools/api_proto_breaking_change_detector/BUILD

@@ -0,0 +1,35 @@
+load("@rules_python//python:defs.bzl", "py_binary", "py_test")
+
+licenses(["notice"])  # Apache 2
+
+py_binary(
+    name = "proto_breaking_change_detector",
+    srcs = ["detector.py"],
+    data = [
+        ":buf.lock",
+        ":buf.yaml",
+        "@com_github_bufbuild_buf//:buf",
+    ],
+    main = "detector.py",
+    tags = ["manual"],
+    deps = [
+        "//tools:run_command",
+    ],
+)
+
+py_test(
+    name = "proto_breaking_change_detector_test",
+    srcs = ["detector_test.py"],
+    data = [
+        "//tools/testdata/api_proto_breaking_change_detector:proto_breaking_change_detector_testdata",
+        "@com_github_bufbuild_buf//:buf",
+    ],
+    main = "detector_test.py",
+    python_version = "PY3",
+    srcs_version = "PY3",
+    tags = ["manual"],
+    deps = [
+        ":proto_breaking_change_detector",
+        "@rules_python//python/runfiles",
+    ],
+)

tools/api_proto_breaking_change_detector/buf.lock

@@ -0,0 +1,45 @@
+# Generated by buf. DO NOT EDIT.
+version: v1
+deps:
+  - remote: buf.build
+    owner: beta
+    repository: googleapis
+    branch: main
+    commit: 1c473ad9220a49bca9320f4cc690eba5
+    digest: b1-unlhrcI3tnJd0JEGuOb692LZ_tY_gCGq6mK1bgCn1Pg=
+    create_time: 2021-06-23T20:16:47.788079Z
+  - remote: buf.build
+    owner: beta
+    repository: opencensus
+    branch: main
+    commit: 5f5f8259293649d68707d2e5b6285748
+    digest: b1-myYwcdM0Xu05qIwhiy4eWEcARYUuZZ1vTbYvrrHu1mU=
+    create_time: 2021-03-03T20:50:42.079743Z
+  - remote: buf.build
+    owner: beta
+    repository: opentelemetry
+    branch: main
+    commit: ee82722300c04a618e1c9a2373ce2958
+    digest: b1-MwMH-u3ygagogGt6GnJMsqSyxDL-6RKCThaBQRdT_28=
+    create_time: 2021-06-23T20:17:01.385563Z
+  - remote: buf.build
+    owner: beta
+    repository: prometheus
+    branch: main
+    commit: a91b42d18a994cd4b07b37f365f87cf9
+    digest: b1-uKmv58fyoNwJI855qg7UEagfdyUl6XNPsFAdDoi57f4=
+    create_time: 2021-06-23T20:16:58.410272Z
+  - remote: buf.build
+    owner: beta
+    repository: protoc-gen-validate
+    branch: main
+    commit: 3aa3a142febe4d198171026cddadb18b
+    digest: b1-MkZlf7zGl1xvJCsBhk-Kh46tvCHpRNpGNUOtryA9-ng=
+    create_time: 2021-03-30T19:57:01.168017Z
+  - remote: buf.build
+    owner: beta
+    repository: udpa
+    branch: main
+    commit: 64457162aab743c5a695a8cccbd93d8d
+    digest: b1-15-sblwZI7jDyUli6FyJsBTy8dRe6mHTA2B2VTcIm4g=
+    create_time: 2021-03-30T19:57:09.877302Z

tools/api_proto_breaking_change_detector/buf.yaml

@@ -0,0 +1,13 @@
+version: v1beta1
+breaking:
+  use:
+    - FIELD_SAME_ONEOF
+    - FIELD_SAME_JSON_NAME
+    - FIELD_SAME_NAME
+    - FIELD_SAME_TYPE
+    - FIELD_SAME_LABEL
+    - FILE_SAME_PACKAGE
+
+    # needed for allowing removal/reserving of fields
+    - FIELD_NO_DELETE_UNLESS_NUMBER_RESERVED
+    - FIELD_NO_DELETE_UNLESS_NAME_RESERVED

tools/api_proto_breaking_change_detector/detector.py

@@ -0,0 +1,161 @@
+""" Protocol Buffer Breaking Change Detector
+
+This tool is used to detect "breaking changes" in protobuf files, to
+ensure proper backwards-compatibility in protobuf API updates. The tool
+can check for breaking changes of a single API by taking 2 .proto file
+paths as input (before and after) and outputting a bool `is_breaking`.
+
+The breaking change detector creates a temporary directory, copies in
+each file to compute a protobuf "state", computes a diff of the "before"
+and "after" states, and runs the diff against a set of rules to determine
+if there was a breaking change.
+
+The tool is currently implemented with buf (https://buf.build/)
+"""
+
+import tempfile
+from rules_python.python.runfiles import runfiles
+from tools.run_command import run_command
+from shutil import copyfile
+from pathlib import Path
+import os
+from typing import List
+
+
+class ProtoBreakingChangeDetector(object):
+    """Abstract breaking change detector interface"""
+
+    def __init__(self, path_to_before: str, path_to_after: str) -> None:
+        """Initialize the configuration of the breaking change detector
+
+        This function sets up any necessary config without actually
+        running the detector against any proto files.
+
+        Takes in a single protobuf as 2 files, in a ``before`` state
+        and an ``after`` state, and checks if the ``after`` state
+        violates any breaking change rules.
+
+        Args:
+            path_to_before {str} -- absolute path to the .proto file in the before state
+            path_to_after {str} -- absolute path to the .proto file in the after state
+        """
+        pass
+
+    def run_detector(self) -> None:
+        """Run the breaking change detector to detect rule violations
+
+        This method should populate the detector's internal data such
+        that `is_breaking` and `lock_file_changed` do not require any
+        additional invocations to the breaking change detector.
+        """
+        pass
+
+    def is_breaking(self) -> bool:
+        """Return True if breaking changes were detected in the given protos"""
+        pass
+
+    def lock_file_changed(self) -> bool:
+        """Return True if the detector state file changed after being run
+
+        This function assumes that the detector uses a lock file to
+        compare "before" and "after" states of protobufs, which is
+        admittedly an implementation detail. It is mostly used for
+        testing, to ensure that the breaking change detector is
+        checking all of the protobuf features we are interested in.
+        """
+        pass
+
+
+class ChangeDetectorError(Exception):
+    pass
+
+
+class ChangeDetectorInitializeError(ChangeDetectorError):
+    pass
+
+
+BUF_STATE_FILE = "tmp.json"
+
+
+class BufWrapper(ProtoBreakingChangeDetector):
+    """Breaking change detector implemented with buf"""
+
+    def __init__(
+            self,
+            path_to_before: str,
+            path_to_after: str,
+            additional_args: List[str] = None) -> None:
+        if not Path(path_to_before).is_file():
+            raise ValueError(f"path_to_before {path_to_before} does not exist")
+
+        if not Path(path_to_after).is_file():
+            raise ValueError(f"path_to_after {path_to_after} does not exist")
+
+        self._path_to_before = path_to_before
+        self._path_to_after = path_to_after
+        self._additional_args = additional_args
+
+    def run_detector(self) -> None:
+        # buf requires protobuf files to be in a subdirectory of the yaml file
+        with tempfile.TemporaryDirectory(prefix=str(Path(".").absolute()) + os.sep) as temp_dir:
+            buf_path = runfiles.Create().Rlocation("com_github_bufbuild_buf/bin/buf")
+
+            buf_config_loc = Path(".", "tools", "api_proto_breaking_change_detector")
+
+            yaml_file_loc = Path(".", "buf.yaml")
+            copyfile(Path(buf_config_loc, "buf.yaml"), yaml_file_loc)
+
+            target = Path(temp_dir, f"{Path(self._path_to_before).stem}.proto")
+
+            buf_args = [
+                "--path",
+                # buf requires relative pathing for roots
+                str(target.relative_to(Path(".").absolute())),
+                "--config",
+                str(yaml_file_loc),
+            ]
+            buf_args.extend(self._additional_args or [])
+
+            copyfile(self._path_to_before, target)
+
+            lock_location = Path(temp_dir, BUF_STATE_FILE)
+
+            initial_code, initial_out, initial_err = run_command(
+                ' '.join([buf_path, f"build -o {lock_location}", *buf_args]))
+            initial_out, initial_err = ''.join(initial_out), ''.join(initial_err)
+
+            if initial_code != 0 or len(initial_out) > 0 or len(initial_err) > 0:
+                raise ChangeDetectorInitializeError(
+                    f"Unexpected error during init:\n\tExit Status Code: {initial_code}\n\tstdout: {initial_out}\n\t stderr: {initial_err}\n"
+                )
+
+            with open(lock_location, "r") as f:
+                self._initial_lock = f.readlines()
+
+            copyfile(self._path_to_after, target)
+
+            final_code, final_out, final_err = run_command(
+                ' '.join([buf_path, f"breaking --against {lock_location}", *buf_args]))
+            final_out, final_err = ''.join(final_out), ''.join(final_err)
+
+            if len(final_out) == len(final_err) == final_code == 0:
+                _, _, _ = run_command(' '.join([buf_path, f"build -o {lock_location}", *buf_args]))
+            with open(lock_location, "r") as f:
+                self._final_lock = f.readlines()
+
+            self._initial_result = initial_code, initial_out, initial_err
+            self._final_result = final_code, final_out, final_err
+
+    def is_breaking(self) -> bool:
+        final_code, final_out, final_err = self._final_result
+
+        if final_code != 0:
+            return True
+        if final_out != "" or "Failure" in final_out:
+            return True
+        if final_err != "" or "Failure" in final_err:
+            return True
+        return False
+
+    def lock_file_changed(self) -> bool:
+        return any(before != after for before, after in zip(self._initial_lock, self._final_lock))