Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proto: unify envoy_proto_library/api_proto_library. #4233

Merged
merged 9 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions api/bazel/api_build_system.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,16 @@ def api_proto_library_internal(visibility = ["//visibility:private"], **kwargs):
# gRPC stub generation.
# TODO(htuch): Automatically generate go_proto_library and go_grpc_library
# from api_proto_library.
def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], deps = [], has_services = 0, require_py = 1):
def api_proto_library(
name,
visibility = ["//visibility:private"],
srcs = [],
deps = [],
external_proto_deps = [],
external_cc_proto_deps = [],
has_services = 0,
linkstatic = None,
require_py = 1):
# This is now vestigial, since there are no direct consumers in
# the data plane API. However, we want to maintain native proto_library support
# in the proto graph to (1) support future C++ use of native rules with
Expand All @@ -99,15 +108,14 @@ def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], de
native.proto_library(
name = name,
srcs = srcs,
deps = deps + [
deps = deps + external_proto_deps + [
"@com_google_protobuf//:any_proto",
"@com_google_protobuf//:descriptor_proto",
"@com_google_protobuf//:duration_proto",
"@com_google_protobuf//:empty_proto",
"@com_google_protobuf//:struct_proto",
"@com_google_protobuf//:timestamp_proto",
"@com_google_protobuf//:wrappers_proto",
"@googleapis//:api_httpbody_protos_proto",
"@googleapis//:http_api_protos_proto",
"@googleapis//:rpc_status_protos_lib",
"@com_github_gogo_protobuf//:gogo_proto",
Expand All @@ -123,17 +131,29 @@ def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], de
pgv_cc_proto_library(
name = _Suffix(name, _CC_SUFFIX),
srcs = srcs,
linkstatic = linkstatic,
deps = [_LibrarySuffix(d, _CC_SUFFIX) for d in deps],
external_deps = [
external_deps = external_cc_proto_deps + [
"@com_google_protobuf//:cc_wkt_protos",
"@googleapis//:http_api_protos",
"@googleapis//:rpc_status_protos",
"@com_github_gogo_protobuf//:gogo_proto_cc",
],
visibility = ["//visibility:public"],
)
py_export_suffixes = []
if (require_py == 1):
api_py_proto_library(name, srcs, deps, has_services)
py_export_suffixes = ["_py", "_py_genproto"]

# Allow unlimited visibility for consumers
export_suffixes = ["", "_cc", "_cc_validate", "_cc_proto", "_cc_proto_genproto"] + py_export_suffixes
for s in export_suffixes:
native.alias(
name = name + "_export" + s,
actual = name + s,
visibility = ["//visibility:public"],
)

def api_cc_test(name, srcs, proto_deps):
native.cc_test(
Expand Down
53 changes: 11 additions & 42 deletions bazel/envoy_build_system.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("@com_google_protobuf//:protobuf.bzl", "cc_proto_library", "py_proto_library")
load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library")

def envoy_package():
native.package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -398,55 +399,23 @@ def _proto_header(proto_path):
return None

# Envoy proto targets should be specified with this function.
def envoy_proto_library(
name,
srcs = [],
deps = [],
external_deps = [],
generate_python = True):
# Ideally this would be native.{proto_library, cc_proto_library}.
# Unfortunately, this doesn't work with http_api_protos due to the PGV
# requirement to also use them in the non-native protobuf.bzl
# cc_proto_library; you end up with the same file built twice. So, also
# using protobuf.bzl cc_proto_library here.
cc_proto_deps = []
py_proto_deps = ["@com_google_protobuf//:protobuf_python"]

def envoy_proto_library(name, external_deps = [], **kwargs):
external_proto_deps = []
external_cc_proto_deps = []
if "api_httpbody_protos" in external_deps:
cc_proto_deps.append("@googleapis//:api_httpbody_protos")
py_proto_deps.append("@googleapis//:api_httpbody_protos_py")

if "http_api_protos" in external_deps:
cc_proto_deps.append("@googleapis//:http_api_protos")
py_proto_deps.append("@googleapis//:http_api_protos_py")

if "well_known_protos" in external_deps:
# WKT is already included for Python as part of standard deps above.
cc_proto_deps.append("@com_google_protobuf//:cc_wkt_protos")

cc_proto_library(
name = name,
srcs = srcs,
default_runtime = "@com_google_protobuf//:protobuf",
protoc = "@com_google_protobuf//:protoc",
deps = deps + cc_proto_deps,
external_cc_proto_deps.append("@googleapis//:api_httpbody_protos")
external_proto_deps.append("@googleapis//:api_httpbody_protos_proto")
return api_proto_library(
name,
external_cc_proto_deps = external_cc_proto_deps,
external_proto_deps = external_proto_deps,
# Avoid generating .so, we don't need it, can interfere with builds
# such as OSS-Fuzz.
linkstatic = 1,
alwayslink = 1,
visibility = ["//visibility:public"],
**kwargs
)

if generate_python:
py_proto_library(
name = name + "_py",
srcs = srcs,
default_runtime = "@com_google_protobuf//:protobuf_python",
protoc = "@com_google_protobuf//:protoc",
deps = deps + py_proto_deps,
visibility = ["//visibility:public"],
)

# Envoy proto descriptor targets should be specified with this function.
# This is used for testing only.
def envoy_proto_descriptor(name, out, srcs = [], external_deps = []):
Expand Down
2 changes: 1 addition & 1 deletion source/common/ratelimit/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ envoy_cc_library(
srcs = ["ratelimit_impl.cc"],
hdrs = ["ratelimit_impl.h"],
deps = [
":ratelimit_proto",
":ratelimit_proto_cc",
"//include/envoy/grpc:async_client_interface",
"//include/envoy/grpc:async_client_manager_interface",
"//include/envoy/ratelimit:ratelimit_interface",
Expand Down
3 changes: 1 addition & 2 deletions test/common/access_log/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ envoy_cc_library(
envoy_proto_library(
name = "access_log_formatter_fuzz_proto",
srcs = ["access_log_formatter_fuzz.proto"],
generate_python = 0,
deps = ["//test/fuzz:common_proto"],
)

Expand All @@ -33,7 +32,7 @@ envoy_cc_fuzz_test(
srcs = ["access_log_formatter_fuzz_test.cc"],
corpus = "access_log_formatter_corpus",
deps = [
":access_log_formatter_fuzz_proto",
":access_log_formatter_fuzz_proto_cc",
"//source/common/access_log:access_log_formatter_lib",
"//test/fuzz:utility_lib",
],
Expand Down
10 changes: 5 additions & 5 deletions test/common/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ envoy_cc_test(
"//test/mocks/http:http_mocks",
"//test/mocks/tracing:tracing_mocks",
"//test/mocks/upstream:upstream_mocks",
"//test/proto:helloworld_proto",
"//test/proto:helloworld_proto_cc",
],
)

Expand All @@ -40,7 +40,7 @@ envoy_cc_test(
deps = [
"//source/common/buffer:buffer_lib",
"//source/common/grpc:codec_lib",
"//test/proto:helloworld_proto",
"//test/proto:helloworld_proto_cc",
],
)

Expand All @@ -51,7 +51,7 @@ envoy_cc_test(
"//source/common/grpc:common_lib",
"//source/common/http:headers_lib",
"//test/mocks/upstream:upstream_mocks",
"//test/proto:helloworld_proto",
"//test/proto:helloworld_proto_cc",
"//test/test_common:utility_lib",
],
)
Expand All @@ -66,7 +66,7 @@ envoy_cc_test(
"//source/common/tracing:http_tracer_lib",
"//test/mocks/grpc:grpc_mocks",
"//test/mocks/tracing:tracing_mocks",
"//test/proto:helloworld_proto",
"//test/proto:helloworld_proto_cc",
"//test/test_common:test_time_lib",
] + envoy_select_google_grpc(["//source/common/grpc:google_async_client_lib"]),
)
Expand Down Expand Up @@ -99,7 +99,7 @@ envoy_cc_test_library(
"//source/common/http/http2:conn_pool_lib",
"//test/integration:integration_lib",
"//test/mocks/local_info:local_info_mocks",
"//test/proto:helloworld_proto",
"//test/proto:helloworld_proto_cc",
"//test/test_common:test_time_lib",
],
)
Expand Down
9 changes: 3 additions & 6 deletions test/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ envoy_cc_test_library(
envoy_proto_library(
name = "conn_manager_impl_fuzz_proto",
srcs = ["conn_manager_impl_fuzz.proto"],
external_deps = ["well_known_protos"],
generate_python = 0,
deps = [
"//test/fuzz:common_proto",
],
Expand All @@ -108,7 +106,7 @@ envoy_cc_fuzz_test(
srcs = ["conn_manager_impl_fuzz_test.cc"],
corpus = "conn_manager_impl_corpus",
deps = [
":conn_manager_impl_fuzz_proto",
":conn_manager_impl_fuzz_proto_cc",
"//source/common/common:empty_string",
"//source/common/http:conn_manager_lib",
"//source/common/http:date_provider_lib",
Expand Down Expand Up @@ -211,15 +209,14 @@ envoy_cc_test(
envoy_proto_library(
name = "header_map_impl_fuzz_proto",
srcs = ["header_map_impl_fuzz.proto"],
external_deps = ["well_known_protos"],
)

envoy_cc_fuzz_test(
name = "header_map_impl_fuzz_test",
srcs = ["header_map_impl_fuzz_test.cc"],
corpus = "header_map_impl_corpus",
deps = [
":header_map_impl_fuzz_proto",
":header_map_impl_fuzz_proto_cc",
"//source/common/http:header_map_lib",
],
)
Expand Down Expand Up @@ -254,7 +251,7 @@ envoy_cc_fuzz_test(
srcs = ["utility_fuzz_test.cc"],
corpus = "utility_corpus",
deps = [
":utility_fuzz_proto",
":utility_fuzz_proto_cc",
"//source/common/http:utility_lib",
"//test/test_common:utility_lib",
],
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/conn_manager_impl_corpus/example
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ actions {
stream_action {
stream_id: 0
response {
continue_100_headers {}
continue_headers {}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/conn_manager_impl_fuzz.proto
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ message RequestAction {
// TODO(htuch): Model and fuzz encoder filter buffering/resumption and different status returns.
message ResponseAction {
oneof response_action_selector {
test.fuzz.Headers continue_100_headers = 1;
test.fuzz.Headers continue_headers = 1;
test.fuzz.Headers headers = 2;
uint32 data = 3;
test.fuzz.Headers trailers = 4;
Expand Down
4 changes: 2 additions & 2 deletions test/common/http/conn_manager_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,10 @@ class FuzzStream {
const test::common::http::ResponseAction& response_action) {
const bool end_stream = response_action.end_stream();
switch (response_action.response_action_selector_case()) {
case test::common::http::ResponseAction::kContinue100Headers: {
case test::common::http::ResponseAction::kContinueHeaders: {
if (state == StreamState::PendingHeaders) {
auto headers = std::make_unique<TestHeaderMapImpl>(
Fuzz::fromHeaders(response_action.continue_100_headers()));
Fuzz::fromHeaders(response_action.continue_headers()));
headers->setReferenceKey(Headers::get().Status, "100");
decoder_filter_->callbacks_->encode100ContinueHeaders(std::move(headers));
}
Expand Down
4 changes: 1 addition & 3 deletions test/common/http/http2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ envoy_package()
envoy_proto_library(
name = "codec_impl_fuzz_proto",
srcs = ["codec_impl_fuzz.proto"],
external_deps = ["well_known_protos"],
generate_python = 0,
deps = ["//test/fuzz:common_proto"],
)

Expand All @@ -23,7 +21,7 @@ envoy_cc_fuzz_test(
srcs = ["codec_impl_fuzz_test.cc"],
corpus = "codec_impl_corpus",
deps = [
":codec_impl_fuzz_proto",
":codec_impl_fuzz_proto_cc",
"//source/common/http:header_map_lib",
"//source/common/http/http2:codec_lib",
"//test/fuzz:utility_lib",
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/http2/codec_impl_corpus/100-continue
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ actions {
stream_action {
stream_id: 0
response {
continue_100_headers {
continue_headers {
headers {
key: ":status"
value: "100"
Expand Down
4 changes: 2 additions & 2 deletions test/common/http/http2/codec_impl_corpus/example
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ actions {
stream_action {
stream_id: 3
request {
reset: 0
reset_stream: 0
}
}
}
Expand Down Expand Up @@ -252,7 +252,7 @@ actions {
stream_action {
stream_id: 4
response {
reset: 0
reset_stream: 0
}
}
}
4 changes: 2 additions & 2 deletions test/common/http/http2/codec_impl_fuzz.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ message NewStream {

message DirectionalAction {
oneof directional_action_selector {
test.fuzz.Headers continue_100_headers = 1;
test.fuzz.Headers continue_headers = 1;
test.fuzz.Headers headers = 2;
uint32 data = 3;
test.fuzz.Headers trailers = 4;
uint32 reset = 5;
uint32 reset_stream = 5;
bool read_disable = 6;
}
bool end_stream = 7;
Expand Down
9 changes: 4 additions & 5 deletions test/common/http/http2/codec_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,9 @@ class Stream : public LinkedObject<Stream> {
const test::common::http::http2::DirectionalAction& directional_action) {
const bool end_stream = directional_action.end_stream();
switch (directional_action.directional_action_selector_case()) {
case test::common::http::http2::DirectionalAction::kContinue100Headers: {
case test::common::http::http2::DirectionalAction::kContinueHeaders: {
if (state == StreamState::PendingHeaders) {
Http::TestHeaderMapImpl headers =
Fuzz::fromHeaders(directional_action.continue_100_headers());
Http::TestHeaderMapImpl headers = Fuzz::fromHeaders(directional_action.continue_headers());
headers.setReferenceKey(Headers::get().Status, "100");
encoder.encode100ContinueHeaders(headers);
}
Expand Down Expand Up @@ -130,10 +129,10 @@ class Stream : public LinkedObject<Stream> {
}
break;
}
case test::common::http::http2::DirectionalAction::kReset: {
case test::common::http::http2::DirectionalAction::kResetStream: {
if (state != StreamState::Closed) {
encoder.getStream().resetStream(
static_cast<Http::StreamResetReason>(directional_action.reset()));
static_cast<Http::StreamResetReason>(directional_action.reset_stream()));
request_state_ = response_state_ = StreamState::Closed;
}
break;
Expand Down
2 changes: 2 additions & 0 deletions test/common/ratelimit/ratelimit_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class RateLimitGrpcClientTest : public testing::TestWithParam<bool> {
Grpc::AsyncClientPtr{async_client_}, absl::optional<std::chrono::milliseconds>(),
"envoy.service.ratelimit.v2.RateLimitService.ShouldRateLimit"));
} else {
// Force link time dependency on deprecated message type.
pb::lyft::ratelimit::RateLimit _ignore;
client_.reset(new GrpcClientImpl(Grpc::AsyncClientPtr{async_client_},
absl::optional<std::chrono::milliseconds>(),
"pb.lyft.ratelimit.RateLimitService.ShouldRateLimit"));
Expand Down
Loading