diff --git a/.azure-pipelines/pipelines.yml b/.azure-pipelines/pipelines.yml index e5f2904cfb54..6bc20230bafe 100644 --- a/.azure-pipelines/pipelines.yml +++ b/.azure-pipelines/pipelines.yml @@ -244,6 +244,8 @@ stages: matrix: api: CI_TARGET: "bazel.api" + api_compat: + CI_TARGET: "bazel.api_compat" gcc: CI_TARGET: "bazel.gcc" clang_tidy: diff --git a/api/BUILD b/api/BUILD index d61c9e486b81..7ac026f5d296 100644 --- a/api/BUILD +++ b/api/BUILD @@ -254,3 +254,11 @@ proto_library( ":v3_protos", ], ) + +filegroup( + name = "proto_breaking_change_detector_buf_config", + srcs = [ + "buf.yaml", + ], + visibility = ["//visibility:public"], +) diff --git a/api/bazel/repository_locations.bzl b/api/bazel/repository_locations.bzl index 49d7997f15a5..be1e9c9789e4 100644 --- a/api/bazel/repository_locations.bzl +++ b/api/bazel/repository_locations.bzl @@ -122,11 +122,11 @@ REPOSITORY_LOCATIONS_SPEC = 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", + version = "0.53.0", + sha256 = "888bb52d358e34a8d6a57ecff426bed896bdf478ad13c78a70a9e1a9a2d75715", strip_prefix = "buf", urls = ["https://github.com/bufbuild/buf/releases/download/v{version}/buf-Linux-x86_64.tar.gz"], - release_date = "2021-07-30", + release_date = "2021-08-25", use_category = ["api"], tags = ["manual"], ), diff --git a/api/buf.lock b/api/buf.lock new file mode 100644 index 000000000000..046331375ce0 --- /dev/null +++ b/api/buf.lock @@ -0,0 +1,52 @@ +# Generated by buf. DO NOT EDIT. +version: v1 +deps: + - 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: 549da630ffe24b53be3983fcee3bb346 + digest: b1-HVAvWKH61BF6TdZSbHRhrD2SUuC0V7uAlZgCRimGPLI= + create_time: 2021-08-09T14:24:57.923964Z + - 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: 82388a0a0cb04e98a203f95dfed5e84b + digest: b1-lYgUMN58PxyCwvfQoopp40AJ-oHHjWXAzksF7v9U-U4= + create_time: 2021-06-21T22:00:30.152545Z + - remote: buf.build + owner: beta + repository: xds + branch: main + commit: 45f850b92541434cbde4aece01bc7d53 + digest: b1-QZUL5DC6-nVgMMlajH_hlImwghg5HjRsqlEAOl0dZgI= + create_time: 2021-08-09T14:37:06.872899Z + - remote: buf.build + owner: gogo + repository: protobuf + branch: main + commit: 4df00b267f944190a229ce3695781e99 + digest: b1-sjLgsg7CzrkOrIjBDh3s-l0aMjE6oqTj85-OsoopKAw= + create_time: 2021-08-10T00:14:28.345069Z + - remote: buf.build + owner: googleapis + repository: googleapis + branch: main + commit: d1a849b8f8304950832335723096e954 + digest: b1-zJkwX0YeOp1Wa0Jaj_RqMLa2-oEzePH6PJEK8aaMeI4= + create_time: 2021-08-26T15:07:19.652533Z diff --git a/tools/api_proto_breaking_change_detector/buf.yaml b/api/buf.yaml similarity index 53% rename from tools/api_proto_breaking_change_detector/buf.yaml rename to api/buf.yaml index 2f3631072e7c..781f01f972ce 100644 --- a/tools/api_proto_breaking_change_detector/buf.yaml +++ b/api/buf.yaml @@ -1,5 +1,13 @@ version: v1beta1 +deps: + - buf.build/googleapis/googleapis + - buf.build/beta/opencensus + - buf.build/beta/prometheus + - buf.build/beta/opentelemetry + - buf.build/gogo/protobuf + - buf.build/beta/xds breaking: + ignore_unstable_packages: true use: - FIELD_SAME_ONEOF - FIELD_SAME_JSON_NAME @@ -7,7 +15,5 @@ breaking: - 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 diff --git a/bazel/api_binding.bzl b/bazel/api_binding.bzl index 56d7c95f08a8..362e1803a1ef 100644 --- a/bazel/api_binding.bzl +++ b/bazel/api_binding.bzl @@ -9,6 +9,7 @@ def _default_envoy_api_impl(ctx): "tools", "versioning", "contrib", + "buf.yaml", ] for d in api_dirs: ctx.symlink(ctx.path(ctx.attr.envoy_root).dirname.get_child(ctx.attr.reldir).get_child(d), d) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 4f8e196fd256..106c3924923e 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -375,6 +375,16 @@ elif [[ "$CI_TARGET" == "bazel.api" ]]; then # We use custom BAZEL_BUILD_OPTIONS here; the API booster isn't capable of working with libc++ yet. BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS[*]}" python3.8 "${ENVOY_SRCDIR}"/tools/api_boost/api_boost_test.py exit 0 +elif [[ "$CI_TARGET" == "bazel.api_compat" ]]; then + echo "Building buf..." + bazel build @com_github_bufbuild_buf//:buf + BUF_PATH=$(realpath "bazel-source/external/com_github_bufbuild_buf/bin/buf") + echo "Checking API for breaking changes to protobuf backwards compatibility..." + BASE_BRANCH_REF=$("${ENVOY_SRCDIR}"/tools/git/last_github_commit.sh) + COMMIT_TITLE=$(git log -n 1 --pretty='format:%C(auto)%h (%s, %ad)' "${BASE_BRANCH_REF}") + echo -e "\tUsing base commit ${COMMIT_TITLE}" + "${ENVOY_SRCDIR}"/tools/api_proto_breaking_change_detector/detector_ci.sh "${BUF_PATH}" "${BASE_BRANCH_REF}" + exit 0 elif [[ "$CI_TARGET" == "bazel.coverage" || "$CI_TARGET" == "bazel.fuzz_coverage" ]]; then setup_clang_toolchain echo "${CI_TARGET} build with tests ${COVERAGE_TEST_TARGETS[*]}" diff --git a/generated_api_shadow/BUILD b/generated_api_shadow/BUILD index d61c9e486b81..7ac026f5d296 100644 --- a/generated_api_shadow/BUILD +++ b/generated_api_shadow/BUILD @@ -254,3 +254,11 @@ proto_library( ":v3_protos", ], ) + +filegroup( + name = "proto_breaking_change_detector_buf_config", + srcs = [ + "buf.yaml", + ], + visibility = ["//visibility:public"], +) diff --git a/generated_api_shadow/bazel/repository_locations.bzl b/generated_api_shadow/bazel/repository_locations.bzl index 49d7997f15a5..be1e9c9789e4 100644 --- a/generated_api_shadow/bazel/repository_locations.bzl +++ b/generated_api_shadow/bazel/repository_locations.bzl @@ -122,11 +122,11 @@ REPOSITORY_LOCATIONS_SPEC = 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", + version = "0.53.0", + sha256 = "888bb52d358e34a8d6a57ecff426bed896bdf478ad13c78a70a9e1a9a2d75715", strip_prefix = "buf", urls = ["https://github.com/bufbuild/buf/releases/download/v{version}/buf-Linux-x86_64.tar.gz"], - release_date = "2021-07-30", + release_date = "2021-08-25", use_category = ["api"], tags = ["manual"], ), diff --git a/tools/api_proto_breaking_change_detector/BUILD b/tools/api_proto_breaking_change_detector/BUILD index 7eec29a9772b..bce76c323d36 100644 --- a/tools/api_proto_breaking_change_detector/BUILD +++ b/tools/api_proto_breaking_change_detector/BUILD @@ -1,35 +1,56 @@ -load("@rules_python//python:defs.bzl", "py_binary", "py_test") +load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test") licenses(["notice"]) # Apache 2 py_binary( - name = "proto_breaking_change_detector", - srcs = ["detector.py"], + name = "detector", + srcs = [ + "detector.py", + ], data = [ - ":buf.lock", - ":buf.yaml", "@com_github_bufbuild_buf//:buf", + "@envoy_api_canonical//:proto_breaking_change_detector_buf_config", ], main = "detector.py", tags = ["manual"], deps = [ + ":buf_utils", + ":detector_errors", "//tools:run_command", ], ) +py_library( + name = "buf_utils", + srcs = [ + "buf_utils.py", + ], + deps = [ + ":detector_errors", + "//tools/base:utils", + ], +) + +py_library( + name = "detector_errors", + srcs = [ + "detector_errors.py", + ], +) + py_test( - name = "proto_breaking_change_detector_test", + name = "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", + ":detector", + "//tools:run_command", "@rules_python//python/runfiles", ], ) diff --git a/tools/api_proto_breaking_change_detector/buf.lock b/tools/api_proto_breaking_change_detector/buf.lock deleted file mode 100644 index d33cf64e43a5..000000000000 --- a/tools/api_proto_breaking_change_detector/buf.lock +++ /dev/null @@ -1,45 +0,0 @@ -# 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 diff --git a/tools/api_proto_breaking_change_detector/buf_utils.py b/tools/api_proto_breaking_change_detector/buf_utils.py new file mode 100644 index 000000000000..1c279e84a49c --- /dev/null +++ b/tools/api_proto_breaking_change_detector/buf_utils.py @@ -0,0 +1,105 @@ +from pathlib import Path +from typing import List, Union, Tuple + +from detector_errors import ChangeDetectorError, ChangeDetectorInitializeError +from tools.base.utils import cd_and_return +from tools.run_command import run_command + + +def _generate_buf_args(target_path, config_file_loc, additional_args): + buf_args = [] + + # buf requires relative pathing for roots + target_relative = Path(target_path).absolute().relative_to(Path.cwd().absolute()) + + # buf does not accept . as a root; if we are already in the target dir, no need for a --path arg + if str(target_relative) != ".": + buf_args.extend(["--path", str(target_relative)]) + + if config_file_loc: + buf_args.extend(["--config", str(config_file_loc)]) + + buf_args.extend(additional_args or []) + + return buf_args + + +def _cd_into_config_parent(config_file_loc): + config_parent = Path(config_file_loc).parent if config_file_loc else Path.cwd() + return cd_and_return(config_parent) + + +def pull_buf_deps( + buf_path: Union[str, Path], + target_path: Union[str, Path], + config_file_loc: Union[str, Path] = None, + additional_args: List[str] = None) -> None: + """Updates buf.lock file and downloads any BSR dependencies specified in buf.yaml + + Note that in order for dependency downloading to trigger, `buf build` + must be invoked, so `target_path` must contain valid proto syntax. + + Args: + buf_path {Union[str, Path]} -- path to buf binary to use + target_path {Union[str, Path]} -- path to directory containing protos to run buf on + + config_file_loc {Union[str, Path]} -- absolute path to buf.yaml configuration file (if not provided, uses default buf configuration) + additional_args {List[str]} -- additional arguments passed into the buf binary invocations + + Raises: + ChangeDetectorInitializeError: if buf encounters an error while attempting to update the buf.lock file or build afterward + """ + with _cd_into_config_parent(config_file_loc): + buf_args = _generate_buf_args(target_path, config_file_loc, additional_args) + + update_code, _, update_err = run_command(f'{buf_path} mod update') + # for some reason buf prints out the "downloading..." lines on stderr + if update_code != 0: + raise ChangeDetectorInitializeError( + f"Error running `buf mod update`: exit status code {update_code} | stderr: {''.join(update_err)}" + ) + if not Path.cwd().joinpath("buf.lock").exists(): + raise ChangeDetectorInitializeError( + "buf mod update did not generate a buf.lock file (silent error... incorrect config?)" + ) + + run_command(' '.join([f'{buf_path} build', *buf_args])) + + +def check_breaking( + buf_path: Union[str, Path], + target_path: Union[str, Path], + git_ref: str, + git_path: Union[str, Path], + config_file_loc: Union[str, Path] = None, + additional_args: List[str] = None, + subdir: str = None) -> Tuple[int, List[str], List[str]]: + """Runs `buf breaking` to check for breaking changes between the `target_path` protos and the provided initial state + + Args: + buf_path {Union[str, Path]} -- path to buf binary to use + target_path {Union[str, Path]} -- path to directory containing protos to check for breaking changes + git_ref {str} -- git reference to use for the initial state of the protos (typically a commit hash) + git_path {Union[str, Path]} -- absolute path to .git folder for the repository of interest + + subdir {str} -- subdirectory within git repository from which to search for .proto files (default: None, e.g. stay in root) + config_file_loc {Union[str, Path]} -- absolute path to buf.yaml configuration file (if not provided, uses default buf configuration) + additional_args {List[str]} -- additional arguments passed into the buf binary invocations + + Returns: + Tuple[int, List[str], List[str]] -- tuple of (exit status code, stdout, stderr) as provided by run_command. Note stdout/stderr are provided as string lists + """ + with _cd_into_config_parent(config_file_loc): + if not Path(git_path).exists(): + raise ChangeDetectorError(f'path to .git folder {git_path} does not exist') + + buf_args = _generate_buf_args(target_path, config_file_loc, additional_args) + + initial_state_input = f'{git_path}#ref={git_ref}' + + if subdir: + initial_state_input += f',subdir={subdir}' + + final_code, final_out, final_err = run_command( + ' '.join([buf_path, f"breaking --against {initial_state_input}", *buf_args])) + return final_code, final_out, final_err diff --git a/tools/api_proto_breaking_change_detector/detector.py b/tools/api_proto_breaking_change_detector/detector.py index 88fe99ed193a..c5a66be94214 100644 --- a/tools/api_proto_breaking_change_detector/detector.py +++ b/tools/api_proto_breaking_change_detector/detector.py @@ -13,40 +13,22 @@ 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 +from buf_utils import check_breaking, pull_buf_deps +from detector_errors import ChangeDetectorError + 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. + that `is_breaking` does not require any additional invocations + to the breaking change detector. """ pass @@ -54,108 +36,98 @@ 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. - """ + def get_breaking_changes(self) -> List[str]: + """Return a list of strings containing breaking changes output by the tool""" 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, + path_to_changed_dir: str, + git_ref: str, + git_path: str, + subdir: str = None, + buf_path: str = None, + config_file_loc: str = None, 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") + """Initialize the configuration of buf - 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) + This function sets up any necessary config without actually + running buf against any proto files. - 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) + BufWrapper takes a path to a directory containing proto files + as input, and it checks if these proto files break any changes + from a given initial state. - 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" - ) + The initial state is input as a git ref. The constructor expects + a git ref string, as well as an absolute path to a .git folder + for the repository. - with open(lock_location, "r") as f: - self._initial_lock = f.readlines() + Args: + path_to_changed_dir {str} -- absolute path to a directory containing proto files in the after state + buf_path {str} -- path to the buf binary (default: "buf") + git_ref {str} -- git reference to use for the initial state of the protos (typically a commit hash) + git_path {str} -- absolute path to .git folder for the repository of interest + + subdir {str} -- subdirectory within git repository from which to search for .proto files (default: None, e.g. stay in root) + additional_args {List[str]} -- additional arguments passed into the buf binary invocations + config_file_loc {str} -- absolute path to buf.yaml configuration file (if not provided, uses default buf configuration) + """ + if not Path(path_to_changed_dir).is_dir(): + raise ValueError(f"path_to_changed_dir {path_to_changed_dir} is not a valid directory") - copyfile(self._path_to_after, target) + if Path.cwd() not in Path(path_to_changed_dir).parents: + raise ValueError( + f"path_to_changed_dir {path_to_changed_dir} must be a subdirectory of the cwd ({ Path.cwd() })" + ) - 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 not Path(git_path).exists(): + raise ChangeDetectorError(f'path to .git folder {git_path} does not exist') - 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._path_to_changed_dir = path_to_changed_dir + self._additional_args = additional_args + self._buf_path = buf_path or "buf" + self._config_file_loc = config_file_loc + self._git_ref = git_ref + self._git_path = git_path + self._subdir = subdir + self._final_result = None + + pull_buf_deps( + self._buf_path, + self._path_to_changed_dir, + config_file_loc=self._config_file_loc, + additional_args=self._additional_args) - self._initial_result = initial_code, initial_out, initial_err - self._final_result = final_code, final_out, final_err + def run_detector(self) -> None: + self._final_result = check_breaking( + self._buf_path, + self._path_to_changed_dir, + git_ref=self._git_ref, + git_path=self._git_path, + subdir=self._subdir, + config_file_loc=self._config_file_loc, + additional_args=self._additional_args) def is_breaking(self) -> bool: + if not self._final_result: + raise ChangeDetectorError("Must invoke run_detector() before checking if is_breaking()") + final_code, final_out, final_err = self._final_result + final_out, final_err = '\n'.join(final_out), '\n'.join(final_err) + + if final_err != "": + raise ChangeDetectorError(f"Error from buf: {final_err}") if final_code != 0: return True - if final_out != "" or "Failure" in final_out: - return True - if final_err != "" or "Failure" in final_err: + if final_out != "": 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)) + def get_breaking_changes(self) -> List[str]: + _, final_out, _ = self._final_result + return filter(lambda x: len(x) > 0, final_out) if self.is_breaking() else [] diff --git a/tools/api_proto_breaking_change_detector/detector_ci.py b/tools/api_proto_breaking_change_detector/detector_ci.py new file mode 100755 index 000000000000..84278d6d4b07 --- /dev/null +++ b/tools/api_proto_breaking_change_detector/detector_ci.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python3 + +import argparse +import sys +from pathlib import Path + +from detector import BufWrapper + +API_DIR = Path("api").resolve() +GIT_PATH = Path.cwd().joinpath(".git") +CONFIG_FILE_LOC = Path(API_DIR, "buf.yaml") + + +def detect_breaking_changes_git(path_to_buf, ref): + """Returns True if breaking changes were detected in the api folder""" + detector = BufWrapper( + API_DIR, + buf_path=path_to_buf, + config_file_loc=CONFIG_FILE_LOC, + git_ref=ref, + git_path=GIT_PATH, + subdir="api") + detector.run_detector() + breaking = detector.is_breaking() + + if breaking: + print('Breaking changes detected in api protobufs:') + for i, breaking_change in enumerate(detector.get_breaking_changes()): + print(f'\t{i}: {breaking_change}') + print("ERROR: non-backwards-compatible changes detected in api protobufs.") + return breaking + + +if __name__ == '__main__': + parser = argparse.ArgumentParser( + description= + 'Tool to detect breaking changes in api protobufs and enforce backwards compatibility.') + parser.add_argument('buf_path', type=str, help='path to buf binary') + parser.add_argument( + 'git_ref', type=str, help='git reference to check against for breaking changes') + args = parser.parse_args() + + exit_status = detect_breaking_changes_git(args.buf_path, args.git_ref) + sys.exit(exit_status) diff --git a/tools/api_proto_breaking_change_detector/detector_ci.sh b/tools/api_proto_breaking_change_detector/detector_ci.sh new file mode 100755 index 000000000000..7f0ec3271e1e --- /dev/null +++ b/tools/api_proto_breaking_change_detector/detector_ci.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +tools="$(dirname "$(dirname "$(realpath "$0")")")" +root=$(realpath "$tools/..") + +cd "$root" || exit 1 +# to satisfy dependency on run_command (as done in tools/code_format/check_format_test_helper.sh) +export PYTHONPATH="$root" +./tools/api_proto_breaking_change_detector/detector_ci.py "$@" diff --git a/tools/api_proto_breaking_change_detector/detector_errors.py b/tools/api_proto_breaking_change_detector/detector_errors.py new file mode 100644 index 000000000000..724bcfd52fc7 --- /dev/null +++ b/tools/api_proto_breaking_change_detector/detector_errors.py @@ -0,0 +1,6 @@ +class ChangeDetectorError(Exception): + pass + + +class ChangeDetectorInitializeError(ChangeDetectorError): + pass diff --git a/tools/api_proto_breaking_change_detector/detector_test.py b/tools/api_proto_breaking_change_detector/detector_test.py index 874ad4b788bc..1566bf2d500d 100644 --- a/tools/api_proto_breaking_change_detector/detector_test.py +++ b/tools/api_proto_breaking_change_detector/detector_test.py @@ -7,32 +7,24 @@ and ensure that tool behavior is consistent across dependency updates. """ -from pathlib import Path +import tempfile import unittest +from pathlib import Path +from shutil import copyfile, copytree + +from rules_python.python.runfiles import runfiles +from buf_utils import pull_buf_deps from detector import BufWrapper +from tools.base.utils import cd_and_return +from tools.run_command import run_command class BreakingChangeDetectorTests(object): - detector_type = None def run_detector_test(self, testname, is_breaking, expects_changes, additional_args=None): """Runs a test case for an arbitrary breaking change detector type""" - tests_path = Path( - Path(__file__).absolute().parent.parent, "testdata", - "api_proto_breaking_change_detector", "breaking" if is_breaking else "allowed") - - current = Path(tests_path, f"{testname}_current") - changed = Path(tests_path, f"{testname}_next") - - detector_obj = self.detector_type(current, changed, additional_args) - detector_obj.run_detector() - - breaking_response = detector_obj.is_breaking() - self.assertEqual(breaking_response, is_breaking) - - lock_file_changed_response = detector_obj.lock_file_changed() - self.assertEqual(lock_file_changed_response, expects_changes) + pass class TestBreakingChanges(BreakingChangeDetectorTests): @@ -93,7 +85,78 @@ def test_remove_and_reserve_field(self): class BufTests(TestAllowedChanges, TestBreakingChanges, unittest.TestCase): - detector_type = BufWrapper + _buf_path = runfiles.Create().Rlocation("com_github_bufbuild_buf/bin/buf") + + @classmethod + def _run_command_print_error(cls, cmd): + code, out, err = run_command(cmd) + out, err = '\n'.join(out), '\n'.join(err) + if code != 0: + raise Exception( + f"Error running command {cmd}\nExit code: {code} | stdout: {out} | stderr: {err}") + + @classmethod + def setUpClass(cls): + try: + # make temp dir + # buf requires protobuf files to be in a subdirectory of the directory containing the yaml file + cls._temp_dir = tempfile.TemporaryDirectory(dir=Path.cwd()) + cls._config_file_loc = Path(cls._temp_dir.name, "buf.yaml") + + # copy in test data + testdata_path = Path( + Path.cwd(), "tools", "testdata", "api_proto_breaking_change_detector") + copytree(testdata_path, cls._temp_dir.name, dirs_exist_ok=True) + + # copy in buf config + bazel_buf_config_loc = Path.cwd().joinpath( + "external", "envoy_api_canonical", "buf.yaml") + copyfile(bazel_buf_config_loc, cls._config_file_loc) + + # pull buf dependencies and initialize git repo with test data files + with cd_and_return(cls._temp_dir.name): + pull_buf_deps( + cls._buf_path, cls._temp_dir.name, config_file_loc=cls._config_file_loc) + cls._run_command_print_error('git init') + cls._run_command_print_error('git add .') + cls._run_command_print_error("git config user.name 'Bazel Test'") + cls._run_command_print_error("git config user.email '<>'") + cls._run_command_print_error('git commit -m "Initial commit"') + except: + cls.tearDownClass() + raise + + @classmethod + def tearDownClass(cls): + cls._temp_dir.cleanup() + + def tearDown(self): + # undo changes to proto file that were applied in test + with cd_and_return(self._temp_dir.name): + self._run_command_print_error('git reset --hard') + + def run_detector_test(self, testname, is_breaking, expects_changes, additional_args=None): + """Runs a test case for an arbitrary breaking change detector type""" + tests_path = Path(self._temp_dir.name, "breaking" if is_breaking else "allowed") + + target = Path(tests_path, f"{testname}.proto") + changed = Path(tests_path, f"{testname}_changed") + + # make changes to proto file + copyfile(changed, target) + + # buf breaking + detector_obj = BufWrapper( + self._temp_dir.name, + git_ref="HEAD", + git_path=Path(self._temp_dir.name, ".git"), + additional_args=additional_args, + buf_path=self._buf_path, + config_file_loc=self._config_file_loc) + detector_obj.run_detector() + + breaking_response = detector_obj.is_breaking() + self.assertEqual(breaking_response, is_breaking) @unittest.skip("PGV field support not yet added to buf") def test_change_pgv_field(self): diff --git a/tools/base/utils.py b/tools/base/utils.py index cd7526b16271..ca92cdc4e78b 100644 --- a/tools/base/utils.py +++ b/tools/base/utils.py @@ -9,7 +9,8 @@ import tempfile from configparser import ConfigParser from contextlib import ExitStack, contextmanager, redirect_stderr, redirect_stdout -from typing import Callable, Iterator, List, Optional, Union +from pathlib import Path +from typing import Callable, ContextManager, Iterator, List, Optional, Union import yaml @@ -128,3 +129,14 @@ def to_yaml(data: Union[dict, list, str, int], path: Union[pathlib.Path, str]) - path = pathlib.Path(path) path.write_text(yaml.dump(data)) return path + + +@contextmanager +def cd_and_return(path: Union[pathlib.Path, str]) -> ContextManager[None]: + """Changes working directory to given path and returns to previous working directory on exit""" + prev_cwd = Path.cwd() + try: + os.chdir(path) + yield + finally: + os.chdir(prev_cwd) diff --git a/tools/testdata/api_proto_breaking_change_detector/BUILD b/tools/testdata/api_proto_breaking_change_detector/BUILD index 5fb56cefbd2b..0a9739fd8fb2 100644 --- a/tools/testdata/api_proto_breaking_change_detector/BUILD +++ b/tools/testdata/api_proto_breaking_change_detector/BUILD @@ -3,38 +3,36 @@ licenses(["notice"]) # Apache 2 filegroup( name = "proto_breaking_change_detector_testdata", srcs = [ - "allowed/test_add_comment_current", - "allowed/test_add_comment_next", - "allowed/test_add_enum_value_current", - "allowed/test_add_enum_value_next", - "allowed/test_add_field_current", - "allowed/test_add_field_next", - "allowed/test_add_option_current", - "allowed/test_add_option_next", - "allowed/test_force_breaking_change_current", - "allowed/test_force_breaking_change_next", - "allowed/test_remove_and_reserve_field_current", - "allowed/test_remove_and_reserve_field_next", - "breaking/test_change_field_from_oneof_current", - "breaking/test_change_field_from_oneof_next", - "breaking/test_change_field_id_current", - "breaking/test_change_field_id_next", - "breaking/test_change_field_name_current", - "breaking/test_change_field_name_next", - "breaking/test_change_field_plurality_current", - "breaking/test_change_field_plurality_next", - "breaking/test_change_field_to_oneof_current", - "breaking/test_change_field_to_oneof_next", - "breaking/test_change_field_type_current", - "breaking/test_change_field_type_next", - "breaking/test_change_package_name_current", - "breaking/test_change_package_name_next", - "breaking/test_change_pgv_field_current", - "breaking/test_change_pgv_field_next", - "breaking/test_change_pgv_message_current", - "breaking/test_change_pgv_message_next", - "breaking/test_change_pgv_oneof_current", - "breaking/test_change_pgv_oneof_next", + "allowed/test_add_comment.proto", + "allowed/test_add_comment_changed", + "allowed/test_add_enum_value.proto", + "allowed/test_add_enum_value_changed", + "allowed/test_add_field.proto", + "allowed/test_add_field_changed", + "allowed/test_add_option.proto", + "allowed/test_add_option_changed", + "allowed/test_remove_and_reserve_field.proto", + "allowed/test_remove_and_reserve_field_changed", + "breaking/test_change_field_from_oneof.proto", + "breaking/test_change_field_from_oneof_changed", + "breaking/test_change_field_id.proto", + "breaking/test_change_field_id_changed", + "breaking/test_change_field_name.proto", + "breaking/test_change_field_name_changed", + "breaking/test_change_field_plurality.proto", + "breaking/test_change_field_plurality_changed", + "breaking/test_change_field_to_oneof.proto", + "breaking/test_change_field_to_oneof_changed", + "breaking/test_change_field_type.proto", + "breaking/test_change_field_type_changed", + "breaking/test_change_package_name.proto", + "breaking/test_change_package_name_changed", + "breaking/test_change_pgv_field.proto", + "breaking/test_change_pgv_field_changed", + "breaking/test_change_pgv_message.proto", + "breaking/test_change_pgv_message_changed", + "breaking/test_change_pgv_oneof.proto", + "breaking/test_change_pgv_oneof_changed", ], visibility = ["//visibility:public"], ) diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_current b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment.proto similarity index 72% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_current rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment.proto index ff7a78b1fc43..b381c393588e 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_current +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment.proto @@ -1,9 +1,9 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; option java_package = "io.envoyproxy.envoy.protolock"; option java_outer_classname = "EnvoyProtolock"; option java_multiple_files = true; -message CommonExtensionConfig { +message AddCommentMessage { } diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_next b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_changed similarity index 77% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_next rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_changed index 8bd44e75c89b..48b6a550c817 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_next +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_changed @@ -1,10 +1,10 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; option java_package = "io.envoyproxy.envoy.protolock"; option java_outer_classname = "EnvoyProtolock"; option java_multiple_files = true; // Common configuration for all tap extensions. -message CommonExtensionConfig { +message AddCommentMessage { } diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_current b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value.proto similarity index 89% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_current rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value.proto index 0663f21f38ac..56cadb1af24f 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_current +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value.proto @@ -1,5 +1,5 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; message SearchRequest { string query = 1; diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_next b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_changed similarity index 90% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_next rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_changed index 290aea11693c..da3446d09fa0 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_next +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_enum_value_changed @@ -1,5 +1,5 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; message SearchRequest { string query = 1; diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field.proto b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field.proto new file mode 100644 index 000000000000..c16ab3037c87 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; +package test.protos.allowed; + +message AddFieldMessage { + string three = 3; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_next b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_changed similarity index 53% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_next rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_changed index b17fe188e3fe..49e8d2816997 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_next +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_changed @@ -1,7 +1,7 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; -message SampleMessage { +message AddFieldMessage { string three = 3; float four = 4; } diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_current b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option.proto similarity index 73% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_current rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option.proto index f6f3f59b856d..498895fbbb8b 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_current +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option.proto @@ -1,9 +1,9 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; option java_package = "io.envoyproxy.envoy.protolock"; option java_outer_classname = "EnvoyProtolock"; // Common configuration for all tap extensions. -message CommonExtensionConfig { +message AddOptionMessage { } diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_next b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_changed similarity index 77% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_next rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_changed index 8bd44e75c89b..1aca28ad4bed 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_comment_next +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_option_changed @@ -1,10 +1,10 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; option java_package = "io.envoyproxy.envoy.protolock"; option java_outer_classname = "EnvoyProtolock"; option java_multiple_files = true; // Common configuration for all tap extensions. -message CommonExtensionConfig { +message AddOptionMessage { } diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_force_breaking_change_current b/tools/testdata/api_proto_breaking_change_detector/allowed/test_force_breaking_change_current deleted file mode 100644 index 4a38563f01d9..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_force_breaking_change_current +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - string three = 3; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_force_breaking_change_next b/tools/testdata/api_proto_breaking_change_detector/allowed/test_force_breaking_change_next deleted file mode 100644 index 2059fdb4f514..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_force_breaking_change_next +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - float three = 3; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field.proto b/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field.proto new file mode 100644 index 000000000000..22b7ce3a7b08 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; +package test.protos.allowed; + +message RemoveReserveFieldMessage { + string three = 3; + float four = 4; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_next b/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_changed similarity index 55% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_next rename to tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_changed index f2ffa0510ad3..e226ff0130f7 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_next +++ b/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_changed @@ -1,7 +1,7 @@ syntax = "proto3"; -package test.protos.breaking; +package test.protos.allowed; -message SampleMessage { +message RemoveReserveFieldMessage { reserved 4; reserved "four"; diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_current b/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_current deleted file mode 100644 index b17fe188e3fe..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_remove_and_reserve_field_current +++ /dev/null @@ -1,7 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - string three = 3; - float four = 4; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof.proto new file mode 100644 index 000000000000..945e6ab9c8c5 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; +package test.protos.breaking; + +message FromOneofMessage { + oneof test_oneof { + string name = 4; + string sub_message = 9; + float special = 10; + } +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_changed similarity index 85% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_changed index b4965f64fd7a..2aa8f4e02a0e 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_changed @@ -1,7 +1,7 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +message FromOneofMessage { oneof test_oneof { string name = 4; string sub_message = 9; diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_current deleted file mode 100644 index 564601045ee9..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_from_oneof_current +++ /dev/null @@ -1,10 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - oneof test_oneof { - string name = 4; - string sub_message = 9; - float special = 10; - } -} diff --git a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id.proto similarity index 50% rename from tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_current rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id.proto index 4a38563f01d9..15c0521a89e4 100644 --- a/tools/testdata/api_proto_breaking_change_detector/allowed/test_add_field_current +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id.proto @@ -1,6 +1,6 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { - string three = 3; +message ChangeFieldIdMessage { + string three = 3; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_changed similarity index 70% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_changed index 35984ed9637b..9c31f1709402 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_changed @@ -1,6 +1,6 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +message ChangeFieldIdMessage { string three = 42; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name.proto new file mode 100644 index 000000000000..ed0e9b1af7f6 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; +package test.protos.breaking; + +message ChangeFieldNameMessage { + string seventeen = 17; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_changed similarity index 69% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_changed index c6003d138489..7a6e4b181969 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_changed @@ -1,6 +1,6 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +message ChangeFieldNameMessage { string twenty = 17; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_current deleted file mode 100644 index 4471dbbb1a20..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_name_current +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - string seventeen = 17; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality.proto new file mode 100644 index 000000000000..7e15732e1952 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; +package test.protos.breaking; + +message ChangeFieldPluralityMessage { + string from = 1; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_changed similarity index 68% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_changed index 63a5af85d0b2..5c11b9375ba4 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_changed @@ -1,6 +1,6 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +message ChangeFieldPluralityMessage { repeated string from = 1; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_current deleted file mode 100644 index 09e5e4374eef..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_plurality_current +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - string from = 1; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof.proto new file mode 100644 index 000000000000..008ac77757c7 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; +package test.protos.breaking; + +message ToOneofMessage { + oneof test_oneof { + string name = 4; + string sub_message = 9; + } + float loner = 10; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_changed similarity index 86% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_changed index 3336f4e384e9..564d9dd19660 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_changed @@ -1,7 +1,7 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +message ToOneofMessage { oneof test_oneof { string name = 4; string sub_message = 9; diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_current deleted file mode 100644 index 1bb452fdb941..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_to_oneof_current +++ /dev/null @@ -1,10 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - oneof test_oneof { - string name = 4; - string sub_message = 9; - } - float loner = 10; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type.proto similarity index 50% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_current rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type.proto index 4a38563f01d9..3762ae0600e7 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_id_current +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type.proto @@ -1,6 +1,6 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { - string three = 3; +message ChangeFieldTypeMessage { + float pi = 314; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_changed similarity index 68% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_changed index b235c8e224d0..7ce276c75fe6 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_changed @@ -1,6 +1,6 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +message ChangeFieldTypeMessage { string pi = 314; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_current deleted file mode 100644 index 5cae750860e1..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_field_type_current +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - float pi = 314; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name.proto new file mode 100644 index 000000000000..512e1c14d1b6 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; +package test.protos.breaking; + +message ChangePackageNameMessage { + string passcode = 69; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_changed similarity index 66% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_changed index 867e36699374..a8d583e6d117 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_changed @@ -1,6 +1,6 @@ syntax = "proto3"; package wrong.package; -message SampleMessage { +message ChangePackageNameMessage { string passcode = 69; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_current deleted file mode 100644 index f48663822058..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_package_name_current +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - string passcode = 69; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field.proto new file mode 100644 index 000000000000..14d1349b5e45 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; +package test.protos.breaking; + +import "validate/validate.proto"; + +message ChangePGVFieldMessage { + string useremail = 1 [(validate.rules).string.email = true]; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_changed similarity index 63% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_changed index 2715676735f3..7c673f49cd63 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_changed @@ -1,6 +1,8 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +import "validate/validate.proto"; + +message ChangePGVFieldMessage { string useremail = 1 [(validate.rules).string.email = false]; } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_current deleted file mode 100644 index 72b792e819e5..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_field_current +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - string useremail = 1 [(validate.rules).string.email = true]; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message.proto new file mode 100644 index 000000000000..107189d5aec1 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; +package test.protos.breaking; + +import "validate/validate.proto"; + +message ChangePGVMessageMessage { + option (validate.disabled) = true; + + ChangePGVMessageMessage q = 543; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_changed b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_changed new file mode 100644 index 000000000000..6732ead334d6 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_changed @@ -0,0 +1,10 @@ +syntax = "proto3"; +package test.protos.breaking; + +import "validate/validate.proto"; + +message ChangePGVMessageMessage { + option (validate.disabled) = false; + + ChangePGVMessageMessage q = 543; +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_current deleted file mode 100644 index 1df9b09c14b2..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_current +++ /dev/null @@ -1,8 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - option (validate.disabled) = true; - - SampleMessage q = 543; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_next deleted file mode 100644 index 62fd2266f00f..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_message_next +++ /dev/null @@ -1,8 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - option (validate.disabled) = false; - - SampleMessage q = 543; -} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof.proto b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof.proto new file mode 100644 index 000000000000..b93b51a98110 --- /dev/null +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; +package test.protos.breaking; + +import "validate/validate.proto"; + +message ChangePGVOneofMessage { + oneof test_oneof { + option (validate.required) = true; + string name = 4; + string sub_message = 9; + } +} diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_next b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_changed similarity index 60% rename from tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_next rename to tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_changed index 989642b4aca8..f376cd74caac 100644 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_next +++ b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_changed @@ -1,10 +1,12 @@ syntax = "proto3"; package test.protos.breaking; -message SampleMessage { +import "validate/validate.proto"; + +message ChangePGVOneofMessage { oneof test_oneof { option (validate.required) = false; string name = 4; - string sub_message = 9;; + string sub_message = 9; } } diff --git a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_current b/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_current deleted file mode 100644 index 266d6fee1cfd..000000000000 --- a/tools/testdata/api_proto_breaking_change_detector/breaking/test_change_pgv_oneof_current +++ /dev/null @@ -1,10 +0,0 @@ -syntax = "proto3"; -package test.protos.breaking; - -message SampleMessage { - oneof test_oneof { - option (validate.required) = true; - string name = 4; - string sub_message = 9;; - } -} diff --git a/tools/type_whisperer/proto_build_targets_gen.py b/tools/type_whisperer/proto_build_targets_gen.py index 2fd33ac0361c..2da22439946f 100644 --- a/tools/type_whisperer/proto_build_targets_gen.py +++ b/tools/type_whisperer/proto_build_targets_gen.py @@ -66,6 +66,14 @@ ":v3_protos", ], ) + +filegroup( + name = "proto_breaking_change_detector_buf_config", + srcs = [ + "buf.yaml", + ], + visibility = ["//visibility:public"], +) """)