From e753ca25b6f106e232bd530843035217009458fc Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 22 Nov 2022 12:31:42 -0500 Subject: [PATCH 1/4] attempting to remove protoc Signed-off-by: Alyssa Wilk --- api/bazel/repository_locations.bzl | 12 ++++++------ mobile/envoy_build_config/extension_registry.cc | 1 + mobile/test/cc/unit/envoy_config_test.cc | 4 ++++ source/exe/process_wide.cc | 6 ++++-- source/exe/process_wide.h | 2 +- test/integration/BUILD | 1 + test/integration/base_integration_test.cc | 2 ++ test/test_runner.cc | 2 +- 8 files changed, 20 insertions(+), 10 deletions(-) diff --git a/api/bazel/repository_locations.bzl b/api/bazel/repository_locations.bzl index 66f7af0222c4f..a0ee69003a8d2 100644 --- a/api/bazel/repository_locations.bzl +++ b/api/bazel/repository_locations.bzl @@ -13,15 +13,15 @@ REPOSITORY_LOCATIONS_SPEC = dict( license_url = "https://github.com/bazelbuild/bazel-skylib/blob/{version}/LICENSE", ), com_envoyproxy_protoc_gen_validate = dict( + release_date = "2022-09-01", project_name = "protoc-gen-validate (PGV)", project_desc = "protoc plugin to generate polyglot message validators", - project_url = "https://github.com/bufbuild/protoc-gen-validate", - version = "0.6.13", - sha256 = "cfa8741c939387969550a305f237e627db26e3ca00c69c0d6a5148843d14655a", - release_date = "2022-10-03", - strip_prefix = "protoc-gen-validate-{version}", - urls = ["https://github.com/bufbuild/protoc-gen-validate/archive/v{version}.tar.gz"], + project_url = "https://github.com/alyssawilk/protoc-gen-validate/tree/minimal", use_category = ["api"], + sha256 = "ee0e6d9dbf5d19c7d327444c07f1a07cf4ec64bd3bc1fe68890685a723bde52a", + version = "2682ad06cca00550030e177834f58a2bc06eb61e", + urls = ["https://github.com/alyssawilk/protoc-gen-validate/archive/refs/heads/minimal.zip"], + strip_prefix = "protoc-gen-validate-minimal", implied_untracked_deps = [ "com_github_iancoleman_strcase", "com_github_lyft_protoc_gen_star", diff --git a/mobile/envoy_build_config/extension_registry.cc b/mobile/envoy_build_config/extension_registry.cc index f214129b5f2d9..9d9eecb11f77a 100644 --- a/mobile/envoy_build_config/extension_registry.cc +++ b/mobile/envoy_build_config/extension_registry.cc @@ -26,6 +26,7 @@ #include "extension_registry_platform_additions.h" #include "library/common/extensions/cert_validator/platform_bridge/config.h" +#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h" #include "library/common/extensions/filters/http/assertion/config.h" #include "library/common/extensions/filters/http/local_error/config.h" #include "library/common/extensions/filters/http/network_configuration/config.h" diff --git a/mobile/test/cc/unit/envoy_config_test.cc b/mobile/test/cc/unit/envoy_config_test.cc index abd0c4bac0ec1..4ab1df5252b11 100644 --- a/mobile/test/cc/unit/envoy_config_test.cc +++ b/mobile/test/cc/unit/envoy_config_test.cc @@ -1,6 +1,7 @@ #include #include +#include "extension_registry.h" #include "test/test_common/utility.h" #include "absl/strings/str_replace.h" @@ -15,6 +16,7 @@ #if defined(__APPLE__) #include "source/extensions/network/dns_resolver/apple/apple_dns_impl.h" #endif +#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h" using testing::HasSubstr; using testing::Not; @@ -322,6 +324,8 @@ TEST(TestConfig, RemainingTemplatesThrows) { } TEST(TestConfig, EnablePlatformCertificatesValidation) { + envoy_mobile::extensions::cert_validator::platform_bridge::PlatformBridgeCertValidator validator; + UNREFERENCED_PARAMETER(validator); // Simply used to avoid unknown type errors. EngineBuilder engine_builder; envoy::config::bootstrap::v3::Bootstrap bootstrap; engine_builder.enablePlatformCertificatesValidation(false); diff --git a/source/exe/process_wide.cc b/source/exe/process_wide.cc index 9e93e9c48f4b4..212f80c53c15a 100644 --- a/source/exe/process_wide.cc +++ b/source/exe/process_wide.cc @@ -21,7 +21,7 @@ struct InitData { InitData& processWideInitData() { MUTABLE_CONSTRUCT_ON_FIRST_USE(InitData); }; } // namespace -ProcessWide::ProcessWide() { +ProcessWide::ProcessWide(bool validate_proto_descriptors) { // Note that the following lock has the dual use of making sure that initialization is complete // before a second caller can enter and leave this function. auto& init_data = processWideInitData(); @@ -31,7 +31,9 @@ ProcessWide::ProcessWide() { // TODO(mattklein123): Audit the following as not all of these have to be re-initialized in the // edge case where something does init/destroy/init/destroy. Event::Libevent::Global::initialize(); - Envoy::Server::validateProtoDescriptors(); + if (validate_proto_descriptors) { + Envoy::Server::validateProtoDescriptors(); + } Http::Http2::initializeNghttp2Logging(); // We do not initialize Google gRPC here -- we instead instantiate diff --git a/source/exe/process_wide.h b/source/exe/process_wide.h index 375faad54ee06..2d0147d063bef 100644 --- a/source/exe/process_wide.h +++ b/source/exe/process_wide.h @@ -8,7 +8,7 @@ namespace Envoy { // e.g. c-ares. There should only ever be a single instance of this. class ProcessWide { public: - ProcessWide(); + ProcessWide(bool validate_proto_descriptors = true); ~ProcessWide(); }; diff --git a/test/integration/BUILD b/test/integration/BUILD index 6e058a241e81a..dd5bb88d16aee 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -892,6 +892,7 @@ envoy_cc_test_library( "base_integration_test.h", ], deps = [ + "//source/server:proto_descriptors_lib", "//source/extensions/request_id/uuid:config", ":autonomous_upstream_lib", ":fake_upstream_lib", diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc index b805eb2cf69c8..df479f1576219 100644 --- a/test/integration/base_integration_test.cc +++ b/test/integration/base_integration_test.cc @@ -20,6 +20,7 @@ #include "source/common/network/utility.h" #include "source/extensions/transport_sockets/tls/context_config_impl.h" #include "source/extensions/transport_sockets/tls/ssl_socket.h" +#include "source/server/proto_descriptors.h" #include "test/integration/utility.h" #include "test/test_common/environment.h" @@ -47,6 +48,7 @@ BaseIntegrationTest::BaseIntegrationTest(const InstanceConstSharedPtrFn& upstrea version_(version), upstream_address_fn_(upstream_address_fn), config_helper_(version, *api_, config), default_log_level_(TestEnvironment::getOptions().logLevel()) { + Envoy::Server::validateProtoDescriptors(); // This is a hack, but there are situations where we disconnect fake upstream connections and // then we expect the server connection pool to get the disconnect before the next test starts. // This does not always happen. This pause should allow the server to pick up the disconnect diff --git a/test/test_runner.cc b/test/test_runner.cc index c6b4dbc359d2d..ed1ca1b845ef1 100644 --- a/test/test_runner.cc +++ b/test/test_runner.cc @@ -70,7 +70,7 @@ int TestRunner::RunTests(int argc, char** argv) { ::testing::InitGoogleMock(&argc, argv); // We hold on to process_wide to provide RAII cleanup of process-wide // state. - ProcessWide process_wide; + ProcessWide process_wide(false); // Add a test-listener so we can call a hook where we can do a quiescence // check after each method. See // https://github.com/google/googletest/blob/master/googletest/docs/advanced.md From 946f31d0f1bf76b0e0e9827af5840e1a0f0af0db Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 22 Nov 2022 14:22:13 -0500 Subject: [PATCH 2/4] more protoc removals Signed-off-by: Alyssa Wilk --- api/bazel/repository_locations.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/bazel/repository_locations.bzl b/api/bazel/repository_locations.bzl index a0ee69003a8d2..551ccdbdebc95 100644 --- a/api/bazel/repository_locations.bzl +++ b/api/bazel/repository_locations.bzl @@ -18,7 +18,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_desc = "protoc plugin to generate polyglot message validators", project_url = "https://github.com/alyssawilk/protoc-gen-validate/tree/minimal", use_category = ["api"], - sha256 = "ee0e6d9dbf5d19c7d327444c07f1a07cf4ec64bd3bc1fe68890685a723bde52a", + sha256 = "4c8eaf5e58f2fc64a8a9075081b1ce4b5aea3967a5448d9a4f7763c7f4a41c00", version = "2682ad06cca00550030e177834f58a2bc06eb61e", urls = ["https://github.com/alyssawilk/protoc-gen-validate/archive/refs/heads/minimal.zip"], strip_prefix = "protoc-gen-validate-minimal", From 052a6925ac726ceff634de37e1a9827091a4b0f0 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 22 Nov 2022 14:23:34 -0500 Subject: [PATCH 3/4] cleanup Signed-off-by: Alyssa Wilk --- mobile/envoy_build_config/extension_registry.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/mobile/envoy_build_config/extension_registry.cc b/mobile/envoy_build_config/extension_registry.cc index 9d9eecb11f77a..f214129b5f2d9 100644 --- a/mobile/envoy_build_config/extension_registry.cc +++ b/mobile/envoy_build_config/extension_registry.cc @@ -26,7 +26,6 @@ #include "extension_registry_platform_additions.h" #include "library/common/extensions/cert_validator/platform_bridge/config.h" -#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h" #include "library/common/extensions/filters/http/assertion/config.h" #include "library/common/extensions/filters/http/local_error/config.h" #include "library/common/extensions/filters/http/network_configuration/config.h" From 7fcd3cc3fbc52f4f8474ed4d4e0424bbccd98e25 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 22 Nov 2022 16:15:57 -0500 Subject: [PATCH 4/4] mobile fixes Signed-off-by: Alyssa Wilk --- .../extensions/cert_validator/platform_bridge/config.h | 8 +++++++- mobile/test/cc/unit/envoy_config_test.cc | 4 ---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/mobile/library/common/extensions/cert_validator/platform_bridge/config.h b/mobile/library/common/extensions/cert_validator/platform_bridge/config.h index 51b9865bb644e..435f5bc949a63 100644 --- a/mobile/library/common/extensions/cert_validator/platform_bridge/config.h +++ b/mobile/library/common/extensions/cert_validator/platform_bridge/config.h @@ -3,13 +3,14 @@ #include "source/extensions/transport_sockets/tls/cert_validator/factory.h" #include "library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.h" +#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h" namespace Envoy { namespace Extensions { namespace TransportSockets { namespace Tls { -class PlatformBridgeCertValidatorFactory : public CertValidatorFactory { +class PlatformBridgeCertValidatorFactory : public CertValidatorFactory, public Config::TypedFactory { public: CertValidatorPtr createCertValidator(const Envoy::Ssl::CertificateValidationContextConfig* config, SslStats& stats, TimeSource& time_source) override; @@ -17,6 +18,11 @@ class PlatformBridgeCertValidatorFactory : public CertValidatorFactory { std::string name() const override { return "envoy_mobile.cert_validator.platform_bridge_cert_validator"; } + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } + std::string category() const override { return "envoy.tls.cert_validator"; } + private: const envoy_cert_validator* platform_validator_ = nullptr; diff --git a/mobile/test/cc/unit/envoy_config_test.cc b/mobile/test/cc/unit/envoy_config_test.cc index 4ab1df5252b11..abd0c4bac0ec1 100644 --- a/mobile/test/cc/unit/envoy_config_test.cc +++ b/mobile/test/cc/unit/envoy_config_test.cc @@ -1,7 +1,6 @@ #include #include -#include "extension_registry.h" #include "test/test_common/utility.h" #include "absl/strings/str_replace.h" @@ -16,7 +15,6 @@ #if defined(__APPLE__) #include "source/extensions/network/dns_resolver/apple/apple_dns_impl.h" #endif -#include "library/common/extensions/cert_validator/platform_bridge/platform_bridge.pb.h" using testing::HasSubstr; using testing::Not; @@ -324,8 +322,6 @@ TEST(TestConfig, RemainingTemplatesThrows) { } TEST(TestConfig, EnablePlatformCertificatesValidation) { - envoy_mobile::extensions::cert_validator::platform_bridge::PlatformBridgeCertValidator validator; - UNREFERENCED_PARAMETER(validator); // Simply used to avoid unknown type errors. EngineBuilder engine_builder; envoy::config::bootstrap::v3::Bootstrap bootstrap; engine_builder.enablePlatformCertificatesValidation(false);