dependencies: fix some of the fallout from Wasm merge. (#13569)

* Move use of URLs/sha256 in repositories.bzl to
  repository_locations.bzl.

* Add a check_format rule to validate we don't add URLs outside of
  repository_locations.bzl in the future.

* Extend build graph validator (validate.py) to catch any test deps
  not covered by "test_only". Exceptions made for Rust/Java/Pip3.

Risk level: Low (build only)
Testing: Additional validate.py and check_format unit tests.

Signed-off-by: Harvey Tuch <[email protected]>

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():

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",
     ),
 )

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

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

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",

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.

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')},

tools/testdata/check_format/repository_url.bzl

@@ -0,0 +1,5 @@
+http_archive(
+    name = "foo",
+    url = "http://foo.com",
+    sha256 = "blah",
+)

tools/testdata/check_format/repository_urls.bzl

@@ -0,0 +1,5 @@
+http_archive(
+    name = "foo",
+    urls = ["http://foo.com"]
+    sha256 = "blah",
+)