diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 12825602848c0..1ba42eaa6ae29 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -351,31 +351,10 @@ def _com_github_zlib_ng_zlib_ng(): def _com_google_cel_cpp(): external_http_archive("com_google_cel_cpp") external_http_archive("rules_antlr") - external_http_archive( - name = "antlr4_runtimes", - build_file_content = """ -package(default_visibility = ["//visibility:public"]) -cc_library( - name = "cpp", - srcs = glob(["runtime/Cpp/runtime/src/**/*.cpp"]), - hdrs = glob(["runtime/Cpp/runtime/src/**/*.h"]), - includes = ["runtime/Cpp/runtime/src"], -) -""", - patch_args = ["-p1"], - # Patches ASAN violation of initialization fiasco - patches = ["@envoy//bazel:antlr.patch"], - ) # Parser dependencies # TODO: upgrade this when cel is upgraded to use the latest version - external_http_archive( - name = "rules_antlr", - sha256 = "7249d1569293d9b239e23c65f6b4c81a07da921738bde0dfeb231ed98be40429", - strip_prefix = "rules_antlr-3cc2f9502a54ceb7b79b37383316b23c4da66f9a", - urls = ["https://github.com/marcohu/rules_antlr/archive/3cc2f9502a54ceb7b79b37383316b23c4da66f9a.tar.gz"], - ) - + external_http_archive(name = "rules_antlr") external_http_archive( name = "antlr4_runtimes", build_file_content = """ @@ -387,12 +366,9 @@ cc_library( includes = ["runtime/Cpp/runtime/src"], ) """, - sha256 = "46f5e1af5f4bd28ade55cb632f9a069656b31fc8c2408f9aa045f9b5f5caad64", patch_args = ["-p1"], # Patches ASAN violation of initialization fiasco patches = ["@envoy//bazel:antlr.patch"], - strip_prefix = "antlr4-4.7.2", - urls = ["https://github.com/antlr/antlr4/archive/4.7.2.tar.gz"], ) def _com_github_nghttp2_nghttp2(): diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 3e44753ab3826..fba8cfebd16f8 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -859,7 +859,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( # See: https://github.com/bazelbuild/rules_rust/issues/386 strip_prefix = "rules_rust-{version}", urls = ["https://github.com/bazelbuild/rules_rust/archive/{version}.tar.gz"], - use_category = ["build"], + use_category = ["test_only"], last_updated = "2020-10-09", ), rules_antlr = dict( @@ -886,8 +886,8 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "ANTLR v4", project_desc = "ANTLR (ANother Tool for Language Recognition) is a powerful parser generator for reading, processing, executing, or translating structured text or binary files", project_url = "https://github.com/antlr/antlr4", - version = "4.7.1", - sha256 = "4d0714f441333a63e50031c9e8e4890c78f3d21e053d46416949803e122a6574", + version = "4.7.2", + sha256 = "46f5e1af5f4bd28ade55cb632f9a069656b31fc8c2408f9aa045f9b5f5caad64", strip_prefix = "antlr4-{version}", urls = ["https://github.com/antlr/antlr4/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], @@ -898,7 +898,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-07-29", + last_updated = "2020-10-09", cpe = "N/A", ), ) diff --git a/docs/root/intro/arch_overview/security/external_deps.rst b/docs/root/intro/arch_overview/security/external_deps.rst index 5a0fb0ff5d785..42a54fe3911f7 100644 --- a/docs/root/intro/arch_overview/security/external_deps.rst +++ b/docs/root/intro/arch_overview/security/external_deps.rst @@ -36,11 +36,6 @@ Observability (extensions) .. include:: external_dep_observability_ext.rst -Test only ---------- - -.. include:: external_dep_test_only.rst - Build ----- @@ -50,3 +45,11 @@ Miscellaneous ------------- .. include:: external_dep_other.rst + +Test only +--------- + +Below we provide the status of the C/C++ dependencies that are only used in tests. Tests also +include additional Java, Rust and Python dependencies that are not tracked below. + +.. include:: external_dep_test_only.rst diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py index 5f17d14681622..7b3e1cac97e03 100755 --- a/tools/code_format/check_format.py +++ b/tools/code_format/check_format.py @@ -110,6 +110,22 @@ EXCEPTION_DENYLIST = ("./source/common/http/http2/codec_impl.h", "./source/common/http/http2/codec_impl.cc") +# We want all URL references to exist in repository_locations.bzl files and have +# metadata that conforms to the schema in ./api/bazel/external_deps.bzl. Below +# we have some exceptions for either infrastructure files or places we fall +# short today (Rust). +# +# Please DO NOT extend this allow list without consulting +# @envoyproxy/dependency-shepherds. +BUILD_URLS_ALLOWLIST = ( + "./generated_api_shadow/bazel/repository_locations.bzl", + "./generated_api_shadow/bazel/envoy_http_archive.bzl", + "./bazel/repository_locations.bzl", + "./bazel/crates.bzl", + "./api/bazel/repository_locations.bzl", + "./api/bazel/envoy_http_archive.bzl", +) + CLANG_FORMAT_PATH = os.getenv("CLANG_FORMAT", "clang-format-10") BUILDIFIER_PATH = paths.getBuildifier() BUILDOZER_PATH = paths.getBuildozer() @@ -404,6 +420,9 @@ def denylistedForExceptions(self, file_path): return (file_path.endswith('.h') and not file_path.startswith("./test/")) or file_path in EXCEPTION_DENYLIST \ or self.isInSubdir(file_path, 'tools/testdata') + def allowlistedForBuildUrls(self, file_path): + return file_path in BUILD_URLS_ALLOWLIST + def isApiFile(self, file_path): return file_path.startswith(self.api_prefix) or file_path.startswith(self.api_shadow_root) @@ -806,6 +825,8 @@ def checkBuildLine(self, line, file_path, reportError): not self.isWorkspaceFile(file_path) and not self.isExternalBuildFile(file_path) and "@envoy//" in line): reportError("Superfluous '@envoy//' prefix") + if not self.allowlistedForBuildUrls(file_path) and (" urls = " in line or " url = " in line): + reportError("Only repository_locations.bzl may contains URL references") def fixBuildLine(self, file_path, line, line_number): if (self.envoy_build_rule_check and not self.isStarlarkFile(file_path) and diff --git a/tools/code_format/check_format_test_helper.py b/tools/code_format/check_format_test_helper.py index 1d98ca7ba17e5..aa90d12848eca 100755 --- a/tools/code_format/check_format_test_helper.py +++ b/tools/code_format/check_format_test_helper.py @@ -262,6 +262,10 @@ def runChecks(): "throw.cc", "Don't introduce throws into exception-free files, use error statuses instead.") errors += checkUnfixableError("pgv_string.proto", "min_bytes is DEPRECATED, Use min_len.") errors += checkFileExpectingOK("commented_throw.cc") + errors += checkUnfixableError("repository_url.bzl", + "Only repository_locations.bzl may contains URL references") + errors += checkUnfixableError("repository_urls.bzl", + "Only repository_locations.bzl may contains URL references") # The following files have errors that can be automatically fixed. errors += checkAndFixError("over_enthusiastic_spaces.cc", diff --git a/tools/dependency/validate.py b/tools/dependency/validate.py index 813d9bd907871..1e6dc88fd3439 100755 --- a/tools/dependency/validate.py +++ b/tools/dependency/validate.py @@ -38,8 +38,27 @@ 'io_bazel_rules_go', 'foreign_cc_platform_utils', 'com_github_golang_protobuf', 'com_google_googleapis' ] -IGNORE_DEPS = set(['envoy', 'envoy_api', 'platforms', 'bazel_tools', 'local_config_cc'] + - UNKNOWN_DEPS) +IGNORE_DEPS = set([ + 'envoy', 'envoy_api', 'envoy_api_canonical', 'platforms', 'bazel_tools', 'local_config_cc', + 'remote_coverage_tools' +] + UNKNOWN_DEPS) + + +# Should a dependency be ignored if it's only used in test? Any changes to this +# allowlist method should be accompanied by an update to the explanation in the +# "Test only" section of +# docs/root/intro/arch_overview/security/external_deps.rst. +def TestOnlyIgnore(dep): + # Rust + if dep.startswith('raze__'): + return True + # Java + if dep.startswith('remotejdk'): + return True + # Python (pip3) + if '_pip3_' in dep: + return True + return False class DependencyError(Exception): @@ -139,11 +158,21 @@ def ValidateTestOnlyDeps(self): DependencyError: on a dependency validation error. """ print('Validating test-only dependencies...') + # Validate that //source doesn't depend on test_only queried_source_deps = self._build_graph.QueryExternalDeps('//source/...') expected_test_only_deps = self._dep_info.DepsByUseCategory('test_only') bad_test_only_deps = expected_test_only_deps.intersection(queried_source_deps) if len(bad_test_only_deps) > 0: raise DependencyError(f'//source depends on test-only dependencies: {bad_test_only_deps}') + # Validate that //test deps additional to those of //source are captured in + # test_only. + test_only_deps = self._build_graph.QueryExternalDeps('//test/...') + source_deps = self._build_graph.QueryExternalDeps('//source/...') + marginal_test_deps = test_only_deps.difference(source_deps) + bad_test_deps = marginal_test_deps.difference(expected_test_only_deps) + unknown_bad_test_deps = [dep for dep in bad_test_deps if not TestOnlyIgnore(dep)] + if len(unknown_bad_test_deps) > 0: + raise DependencyError(f'Missing deps in test_only "use_category": {unknown_bad_test_deps}') def ValidateDataPlaneCoreDeps(self): """Validate dataplane_core dependencies. diff --git a/tools/dependency/validate_test.py b/tools/dependency/validate_test.py index 539504ba02f48..4474c64427607 100755 --- a/tools/dependency/validate_test.py +++ b/tools/dependency/validate_test.py @@ -61,10 +61,14 @@ def test_invalid_build_graph_structure(self): def test_valid_test_only_deps(self): validator = self.BuildValidator({'a': FakeDep('dataplane_core')}, {'//source/...': ['a']}) validator.ValidateTestOnlyDeps() + validator = self.BuildValidator({'a': FakeDep('test_only')}, {'//test/...': ['a', 'b__pip3_']}) + validator.ValidateTestOnlyDeps() def test_invalid_test_only_deps(self): validator = self.BuildValidator({'a': FakeDep('test_only')}, {'//source/...': ['a']}) self.assertRaises(validate.DependencyError, lambda: validator.ValidateTestOnlyDeps()) + validator = self.BuildValidator({'a': FakeDep('test_only')}, {'//test/...': ['b']}) + self.assertRaises(validate.DependencyError, lambda: validator.ValidateTestOnlyDeps()) def test_valid_dataplane_core_deps(self): validator = self.BuildValidator({'a': FakeDep('dataplane_core')}, diff --git a/tools/testdata/check_format/repository_url.bzl b/tools/testdata/check_format/repository_url.bzl new file mode 100644 index 0000000000000..8450c6aa31619 --- /dev/null +++ b/tools/testdata/check_format/repository_url.bzl @@ -0,0 +1,5 @@ +http_archive( + name = "foo", + url = "http://foo.com", + sha256 = "blah", +) diff --git a/tools/testdata/check_format/repository_urls.bzl b/tools/testdata/check_format/repository_urls.bzl new file mode 100644 index 0000000000000..c67406076ab34 --- /dev/null +++ b/tools/testdata/check_format/repository_urls.bzl @@ -0,0 +1,5 @@ +http_archive( + name = "foo", + urls = ["http://foo.com"] + sha256 = "blah", +)