build: take two on upstream visibility rules (envoyproxy#12692)

Setting things up so downstream folks can just override with visibility:public but upstream avoids core code depending on extensions. Risk Level: medium (of breaking builds, instructions of how to unbreak included) Testing: manually swapped to public, all works. Docs Changes: included Release Notes: overkill Fixes envoyproxy#12444 Signed-off-by: Alyssa Wilk <[email protected]>
NomadXD · Aug 27, 2020 · d245c02 · d245c02
1 parent c555587
commit d245c02
Show file tree

Hide file tree

Showing 28 changed files with 71 additions and 92 deletions.
diff --git a/BUILD b/BUILD
@@ -1,8 +1,3 @@
-load(
-    "@envoy_build_config//:extensions_build_config.bzl",
-    "ADDITIONAL_VISIBILITY",
-)
-
 licenses(["notice"])  # Apache 2
 
 exports_files([
@@ -11,7 +6,7 @@ exports_files([
 ])
 
 # These two definitions exist to help reduce Envoy upstream core code depending on extensions.
-# To avoid visibility problems, one can extend ADDITIONAL_VISIBILITY in source/extensions/extensions_build_config.bzl
+# To avoid visibility problems, see notes in source/extensions/extensions_build_config.bzl
 #
 # TODO(#9953) //test/config_test:__pkg__ should probably be split up and removed.
 # TODO(#9953) the config fuzz tests should be moved somewhere local and //test/config_test and //test/server removed.
@@ -24,13 +19,13 @@ package_group(
         "//test/extensions/...",
         "//test/server",
         "//test/server/config_validation",
-    ] + ADDITIONAL_VISIBILITY,
+    ],
 )
 
 package_group(
     name = "extension_library",
     packages = [
         "//source/extensions/...",
         "//test/extensions/...",
-    ] + ADDITIONAL_VISIBILITY,
+    ],
 )
diff --git a/bazel/README.md b/bazel/README.md
@@ -654,10 +654,8 @@ local_repository(
 ## Extra extensions
 
 If you are building your own Envoy extensions or custom Envoy builds and encounter visibility
-problems with, you may need to adjust the default visibility rules.
-By default, Envoy extensions are set up to only be visible to code within the
-[//source/extensions](../source/extensions/), or the Envoy server target. To adjust this,
-add any additional targets you need to `ADDITIONAL_VISIBILITY` in
+problems with, you may need to adjust the default visibility rules to be public,
+as documented in
 [extensions_build_config.bzl](../source/extensions/extensions_build_config.bzl).
 See the instructions above about how to create your own custom version of
 [extensions_build_config.bzl](../source/extensions/extensions_build_config.bzl).

diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl
@@ -32,14 +32,16 @@ load(
     _envoy_py_test_binary = "envoy_py_test_binary",
     _envoy_sh_test = "envoy_sh_test",
 )
+load(
+    "@envoy_build_config//:extensions_build_config.bzl",
+    "EXTENSION_PACKAGE_VISIBILITY",
+)
 
 def envoy_package():
     native.package(default_visibility = ["//visibility:public"])
 
 def envoy_extension_package():
-    # TODO(rgs1): revert this to //:extension_library once
-    # https://github.com/envoyproxy/envoy/issues/12444 is fixed.
-    native.package(default_visibility = ["//visibility:public"])
+    native.package(default_visibility = EXTENSION_PACKAGE_VISIBILITY)
 
 # A genrule variant that can output a directory. This is useful when doing things like
 # generating a fuzz corpus mechanically.

diff --git a/bazel/envoy_library.bzl b/bazel/envoy_library.bzl
@@ -9,6 +9,10 @@ load(
     "envoy_linkstatic",
 )
 load("@envoy_api//bazel:api_build_system.bzl", "api_cc_py_proto_library")
+load(
+    "@envoy_build_config//:extensions_build_config.bzl",
+    "EXTENSION_CONFIG_VISIBILITY",
+)
 
 # As above, but wrapped in list form for adding to dep lists. This smell seems needed as
 # SelectorValue values have to match the attribute type. See
@@ -70,14 +74,15 @@ def envoy_cc_extension(
         undocumented = False,
         status = "stable",
         tags = [],
-        # TODO(rgs1): revert this to //:extension_config once
-        # https://github.com/envoyproxy/envoy/issues/12444 is fixed.
-        visibility = ["//visibility:public"],
+        extra_visibility = [],
+        visibility = EXTENSION_CONFIG_VISIBILITY,
         **kwargs):
     if security_posture not in EXTENSION_SECURITY_POSTURES:
         fail("Unknown extension security posture: " + security_posture)
     if status not in EXTENSION_STATUS_VALUES:
         fail("Unknown extension status: " + status)
+    if "//visibility:public" not in visibility:
+        visibility = visibility + extra_visibility
     envoy_cc_library(name, tags = tags, visibility = visibility, **kwargs)
 
 # Envoy C++ library targets should be specified with this function.

diff --git a/ci/filter_example_setup.sh b/ci/filter_example_setup.sh
@@ -5,7 +5,7 @@
 set -e
 
 # This is the hash on https://github.com/envoyproxy/envoy-filter-example.git we pin to.
-ENVOY_FILTER_EXAMPLE_GITSHA="777342f20d93b3a50b641556749ad41502a63d09"
+ENVOY_FILTER_EXAMPLE_GITSHA="493e2e5bee10bbed1c3c097e09d83d7f672a9f2e"
 ENVOY_FILTER_EXAMPLE_SRCDIR="${BUILD_DIR}/envoy-filter-example"
 
 export ENVOY_FILTER_EXAMPLE_TESTS="//:echo2_integration_test //http-filter-example:http_filter_integration_test //:envoy_binary_test"

diff --git a/source/extensions/access_loggers/file/BUILD b/source/extensions/access_loggers/file/BUILD
@@ -27,12 +27,11 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) determine if this is core or should be cleaned up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         ":file_access_log_lib",
         "//include/envoy/registry",

diff --git a/source/extensions/access_loggers/grpc/BUILD b/source/extensions/access_loggers/grpc/BUILD
@@ -97,13 +97,12 @@ envoy_cc_extension(
     name = "http_config",
     srcs = ["http_config.cc"],
     hdrs = ["http_config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/common/access_log:__subpackages__",
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         ":config_utils",
         "//include/envoy/server:access_log_config_interface",
@@ -120,13 +119,12 @@ envoy_cc_extension(
     name = "tcp_config",
     srcs = ["tcp_config.cc"],
     hdrs = ["tcp_config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/common/access_log:__subpackages__",
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         ":config_utils",
         "//include/envoy/server:access_log_config_interface",

diff --git a/source/extensions/common/crypto/BUILD b/source/extensions/common/crypto/BUILD
@@ -21,14 +21,13 @@ envoy_cc_extension(
     external_deps = [
         "ssl",
     ],
-    security_posture = "unknown",
-    undocumented = True,
     # Legacy test use. TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/common/config:__subpackages__",
         "//test/common/crypto:__subpackages__",
     ],
+    security_posture = "unknown",
+    undocumented = True,
     deps = [
         "//include/envoy/buffer:buffer_interface",
         "//source/common/common:assert_lib",

diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl
@@ -202,8 +202,7 @@ EXTENSIONS = {
 
 }
 
-# This can be used to extend the visibility rules for Envoy extensions
-# (//:extension_config and //:extension_library in //BUILD)
-# if downstream Envoy builds need to directly reference envoy extensions.
-ADDITIONAL_VISIBILITY = [
-]
+# These can be changed to ["//visibility:public"], for  downstream builds which
+# need to directly reference Envoy extensions.
+EXTENSION_CONFIG_VISIBILITY = ["//:extension_config"]
+EXTENSION_PACKAGE_VISIBILITY = ["//:extension_library"]
diff --git a/source/extensions/filters/http/buffer/BUILD b/source/extensions/filters/http/buffer/BUILD
@@ -39,6 +39,7 @@ envoy_cc_extension(
     hdrs = ["config.h"],
     security_posture = "robust_to_untrusted_downstream",
     # Legacy test use. TODO(#9953) clean up.
+    visibility = ["//visibility:public"],
     deps = [
         "//include/envoy/registry",
         "//source/extensions/filters/http:well_known_names",

diff --git a/source/extensions/filters/http/common/BUILD b/source/extensions/filters/http/common/BUILD
@@ -21,6 +21,7 @@ envoy_cc_library(
 envoy_cc_library(
     name = "factory_base_lib",
     hdrs = ["factory_base.h"],
+    visibility = ["//visibility:public"],
     deps = [
         "//include/envoy/server:filter_config_interface",
     ],

diff --git a/source/extensions/filters/http/cors/BUILD b/source/extensions/filters/http/cors/BUILD
@@ -31,12 +31,11 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//include/envoy/server:filter_config_interface",

diff --git a/source/extensions/filters/http/grpc_http1_bridge/BUILD b/source/extensions/filters/http/grpc_http1_bridge/BUILD
@@ -33,14 +33,13 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "unknown",
     # Legacy test use. TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//source/exe:__pkg__",
         "//test/integration:__subpackages__",
         "//test/server:__subpackages__",
     ],
+    security_posture = "unknown",
     deps = [
         "//include/envoy/registry",
         "//include/envoy/server:filter_config_interface",

diff --git a/source/extensions/filters/http/health_check/BUILD b/source/extensions/filters/http/health_check/BUILD
@@ -37,14 +37,13 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # Legacy test use. TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/common/filter/http:__subpackages__",
         "//test/integration:__subpackages__",
         "//test/server:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//source/common/http:header_utility_lib",

diff --git a/source/extensions/filters/http/ip_tagging/BUILD b/source/extensions/filters/http/ip_tagging/BUILD
@@ -33,12 +33,11 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//source/common/protobuf:utility_lib",

diff --git a/source/extensions/filters/http/on_demand/BUILD b/source/extensions/filters/http/on_demand/BUILD
@@ -30,13 +30,12 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) classify and clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/common/access_log:__subpackages__",
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//source/extensions/filters/http:well_known_names",

diff --git a/source/extensions/filters/http/rbac/BUILD b/source/extensions/filters/http/rbac/BUILD
@@ -13,12 +13,11 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//source/extensions/filters/http:well_known_names",

diff --git a/source/extensions/filters/listener/original_dst/BUILD b/source/extensions/filters/listener/original_dst/BUILD
@@ -28,12 +28,11 @@ envoy_cc_library(
 envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         ":original_dst_lib",
         "//include/envoy/registry",

diff --git a/source/extensions/filters/listener/proxy_protocol/BUILD b/source/extensions/filters/listener/proxy_protocol/BUILD
@@ -39,12 +39,11 @@ envoy_cc_library(
 envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//include/envoy/server:filter_config_interface",

diff --git a/source/extensions/filters/listener/tls_inspector/BUILD b/source/extensions/filters/listener/tls_inspector/BUILD
@@ -19,8 +19,7 @@ envoy_cc_library(
     external_deps = ["ssl"],
     # TODO(#9953) clean up.
     visibility = [
-        "//:extension_config",
-        "//test/integration:__subpackages__",
+        "//visibility:public",
     ],
     deps = [
         "//include/envoy/event:dispatcher_interface",
@@ -37,12 +36,11 @@ envoy_cc_library(
 envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
-    security_posture = "robust_to_untrusted_downstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream",
     deps = [
         "//include/envoy/registry",
         "//include/envoy/server:filter_config_interface",

diff --git a/source/extensions/filters/network/echo/BUILD b/source/extensions/filters/network/echo/BUILD
@@ -28,12 +28,11 @@ envoy_cc_library(
 envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
-    security_posture = "unknown",
     # TODO(#9953) move echo integration test to extensions.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "unknown",
     deps = [
         ":echo",
         "//include/envoy/registry",

diff --git a/source/extensions/filters/network/redis_proxy/BUILD b/source/extensions/filters/network/redis_proxy/BUILD
@@ -120,12 +120,11 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "requires_trusted_downstream_and_upstream",
     # TODO(#9953) clean up.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "requires_trusted_downstream_and_upstream",
     deps = [
         "//include/envoy/upstream:upstream_interface",
         "//source/extensions/common/redis:cluster_refresh_manager_lib",

diff --git a/source/extensions/internal_redirect/allow_listed_routes/BUILD b/source/extensions/internal_redirect/allow_listed_routes/BUILD
@@ -24,12 +24,11 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream_and_upstream",
     # TODO(#9953) clean up by moving the redirect test to extensions.
-    visibility = [
-        "//:extension_config",
+    extra_visibility = [
         "//test/integration:__subpackages__",
     ],
+    security_posture = "robust_to_untrusted_downstream_and_upstream",
     deps = [
         ":allow_listed_routes_lib",
         "//include/envoy/registry",