Skip to content

Commit d245c02

Browse files
alyssawilkalyssawilk
authored
build: take two on upstream visibility rules (#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 #12444 Signed-off-by: Alyssa Wilk <[email protected]>
1 parent c555587 commit d245c02

File tree

28 files changed

+71
-92
lines changed

28 files changed

+71
-92
lines changed

‎BUILD‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
load(
2-
"@envoy_build_config//:extensions_build_config.bzl",
3-
"ADDITIONAL_VISIBILITY",
4-
)
5-
61
licenses(["notice"]) # Apache 2
72

83
exports_files([
@@ -11,7 +6,7 @@ exports_files([
116
])
127

138
# These two definitions exist to help reduce Envoy upstream core code depending on extensions.
14-
# To avoid visibility problems, one can extend ADDITIONAL_VISIBILITY in source/extensions/extensions_build_config.bzl
9+
# To avoid visibility problems, see notes in source/extensions/extensions_build_config.bzl
1510
#
1611
# TODO(#9953) //test/config_test:__pkg__ should probably be split up and removed.
1712
# 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(
2419
"//test/extensions/...",
2520
"//test/server",
2621
"//test/server/config_validation",
27-
] + ADDITIONAL_VISIBILITY,
22+
],
2823
)
2924

3025
package_group(
3126
name = "extension_library",
3227
packages = [
3328
"//source/extensions/...",
3429
"//test/extensions/...",
35-
] + ADDITIONAL_VISIBILITY,
30+
],
3631
)

‎bazel/README.md‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -654,10 +654,8 @@ local_repository(
654654
## Extra extensions
655655

656656
If you are building your own Envoy extensions or custom Envoy builds and encounter visibility
657-
problems with, you may need to adjust the default visibility rules.
658-
By default, Envoy extensions are set up to only be visible to code within the
659-
[//source/extensions](../source/extensions/), or the Envoy server target. To adjust this,
660-
add any additional targets you need to `ADDITIONAL_VISIBILITY` in
657+
problems with, you may need to adjust the default visibility rules to be public,
658+
as documented in
661659
[extensions_build_config.bzl](../source/extensions/extensions_build_config.bzl).
662660
See the instructions above about how to create your own custom version of
663661
[extensions_build_config.bzl](../source/extensions/extensions_build_config.bzl).

‎bazel/envoy_build_system.bzl‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,16 @@ load(
3232
_envoy_py_test_binary = "envoy_py_test_binary",
3333
_envoy_sh_test = "envoy_sh_test",
3434
)
35+
load(
36+
"@envoy_build_config//:extensions_build_config.bzl",
37+
"EXTENSION_PACKAGE_VISIBILITY",
38+
)
3539

3640
def envoy_package():
3741
native.package(default_visibility = ["//visibility:public"])
3842

3943
def envoy_extension_package():
40-
# TODO(rgs1): revert this to //:extension_library once
41-
# https://github.com/envoyproxy/envoy/issues/12444 is fixed.
42-
native.package(default_visibility = ["//visibility:public"])
44+
native.package(default_visibility = EXTENSION_PACKAGE_VISIBILITY)
4345

4446
# A genrule variant that can output a directory. This is useful when doing things like
4547
# generating a fuzz corpus mechanically.

‎bazel/envoy_library.bzl‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ load(
99
"envoy_linkstatic",
1010
)
1111
load("@envoy_api//bazel:api_build_system.bzl", "api_cc_py_proto_library")
12+
load(
13+
"@envoy_build_config//:extensions_build_config.bzl",
14+
"EXTENSION_CONFIG_VISIBILITY",
15+
)
1216

1317
# As above, but wrapped in list form for adding to dep lists. This smell seems needed as
1418
# SelectorValue values have to match the attribute type. See
@@ -70,14 +74,15 @@ def envoy_cc_extension(
7074
undocumented = False,
7175
status = "stable",
7276
tags = [],
73-
# TODO(rgs1): revert this to //:extension_config once
74-
# https://github.com/envoyproxy/envoy/issues/12444 is fixed.
75-
visibility = ["//visibility:public"],
77+
extra_visibility = [],
78+
visibility = EXTENSION_CONFIG_VISIBILITY,
7679
**kwargs):
7780
if security_posture not in EXTENSION_SECURITY_POSTURES:
7881
fail("Unknown extension security posture: " + security_posture)
7982
if status not in EXTENSION_STATUS_VALUES:
8083
fail("Unknown extension status: " + status)
84+
if "//visibility:public" not in visibility:
85+
visibility = visibility + extra_visibility
8186
envoy_cc_library(name, tags = tags, visibility = visibility, **kwargs)
8287

8388
# Envoy C++ library targets should be specified with this function.

‎ci/filter_example_setup.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
set -e
66

77
# This is the hash on https://github.com/envoyproxy/envoy-filter-example.git we pin to.
8-
ENVOY_FILTER_EXAMPLE_GITSHA="777342f20d93b3a50b641556749ad41502a63d09"
8+
ENVOY_FILTER_EXAMPLE_GITSHA="493e2e5bee10bbed1c3c097e09d83d7f672a9f2e"
99
ENVOY_FILTER_EXAMPLE_SRCDIR="${BUILD_DIR}/envoy-filter-example"
1010

1111
export ENVOY_FILTER_EXAMPLE_TESTS="//:echo2_integration_test //http-filter-example:http_filter_integration_test //:envoy_binary_test"

‎source/extensions/access_loggers/file/BUILD‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,11 @@ envoy_cc_extension(
2727
name = "config",
2828
srcs = ["config.cc"],
2929
hdrs = ["config.h"],
30-
security_posture = "robust_to_untrusted_downstream",
3130
# TODO(#9953) determine if this is core or should be cleaned up.
32-
visibility = [
33-
"//:extension_config",
31+
extra_visibility = [
3432
"//test:__subpackages__",
3533
],
34+
security_posture = "robust_to_untrusted_downstream",
3635
deps = [
3736
":file_access_log_lib",
3837
"//include/envoy/registry",

‎source/extensions/access_loggers/grpc/BUILD‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,12 @@ envoy_cc_extension(
9797
name = "http_config",
9898
srcs = ["http_config.cc"],
9999
hdrs = ["http_config.h"],
100-
security_posture = "robust_to_untrusted_downstream",
101100
# TODO(#9953) clean up.
102-
visibility = [
103-
"//:extension_config",
101+
extra_visibility = [
104102
"//test/common/access_log:__subpackages__",
105103
"//test/integration:__subpackages__",
106104
],
105+
security_posture = "robust_to_untrusted_downstream",
107106
deps = [
108107
":config_utils",
109108
"//include/envoy/server:access_log_config_interface",
@@ -120,13 +119,12 @@ envoy_cc_extension(
120119
name = "tcp_config",
121120
srcs = ["tcp_config.cc"],
122121
hdrs = ["tcp_config.h"],
123-
security_posture = "robust_to_untrusted_downstream",
124122
# TODO(#9953) clean up.
125-
visibility = [
126-
"//:extension_config",
123+
extra_visibility = [
127124
"//test/common/access_log:__subpackages__",
128125
"//test/integration:__subpackages__",
129126
],
127+
security_posture = "robust_to_untrusted_downstream",
130128
deps = [
131129
":config_utils",
132130
"//include/envoy/server:access_log_config_interface",

‎source/extensions/common/crypto/BUILD‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@ envoy_cc_extension(
2121
external_deps = [
2222
"ssl",
2323
],
24-
security_posture = "unknown",
25-
undocumented = True,
2624
# Legacy test use. TODO(#9953) clean up.
27-
visibility = [
28-
"//:extension_config",
25+
extra_visibility = [
2926
"//test/common/config:__subpackages__",
3027
"//test/common/crypto:__subpackages__",
3128
],
29+
security_posture = "unknown",
30+
undocumented = True,
3231
deps = [
3332
"//include/envoy/buffer:buffer_interface",
3433
"//source/common/common:assert_lib",

‎source/extensions/extensions_build_config.bzl‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,7 @@ EXTENSIONS = {
202202

203203
}
204204

205-
# This can be used to extend the visibility rules for Envoy extensions
206-
# (//:extension_config and //:extension_library in //BUILD)
207-
# if downstream Envoy builds need to directly reference envoy extensions.
208-
ADDITIONAL_VISIBILITY = [
209-
]
205+
# These can be changed to ["//visibility:public"], for downstream builds which
206+
# need to directly reference Envoy extensions.
207+
EXTENSION_CONFIG_VISIBILITY = ["//:extension_config"]
208+
EXTENSION_PACKAGE_VISIBILITY = ["//:extension_library"]

‎source/extensions/filters/http/buffer/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ envoy_cc_extension(
3939
hdrs = ["config.h"],
4040
security_posture = "robust_to_untrusted_downstream",
4141
# Legacy test use. TODO(#9953) clean up.
42+
visibility = ["//visibility:public"],
4243
deps = [
4344
"//include/envoy/registry",
4445
"//source/extensions/filters/http:well_known_names",

0 commit comments

Comments
 (0)