From 883dc3fca66b3667a533d07fc6903f0504423926 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 28 Mar 2019 08:16:24 +0000 Subject: [PATCH 1/6] update envoy to latest Signed-off-by: Lizan Zhou --- WORKSPACE | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 1b3aa623ddb..2f2c16397e6 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -35,8 +35,8 @@ bind( # When updating envoy sha manually please update the sha in istio.deps file also # # Determine SHA256 `wget https://github.com/envoyproxy/envoy/archive/COMMIT.tar.gz && sha256sum COMMIT.tar.gz` -ENVOY_SHA = "925810d00b0d3095a8e67fd4e04e0f597ed188bb" -ENVOY_SHA256 = "26d1f14e881455546cf0e222ec92a8e1e5f65cb2c5761d63c66598b39cd9c47d" +ENVOY_SHA = "805683f835bd63e4b7b9d89059aa0d3783924a93" +ENVOY_SHA256 = "75a029fb3904c17f47c7f723e2a04468bfc007bb4cfc74fe21f82cf799d8a904" http_archive( name = "envoy", @@ -57,7 +57,7 @@ cc_configure() load("@envoy_api//bazel:repositories.bzl", "api_dependencies") api_dependencies() -load("@io_bazel_rules_go//go:def.bzl", "go_rules_dependencies", "go_register_toolchains") +load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains") go_rules_dependencies() go_register_toolchains() From 1578cf8b11f57565d7470704138228cb7dbd3e92 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Fri, 8 Mar 2019 06:32:34 -0800 Subject: [PATCH 2/6] update envoy with latest build fixes (#2147) --- WORKSPACE | 18 +++++------------- istio.deps | 2 +- protobuf.bzl | 4 ++-- src/envoy/http/mixer/control.cc | 4 ++-- src/envoy/tcp/mixer/control.cc | 4 ++-- 5 files changed, 12 insertions(+), 20 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 2f2c16397e6..ad244b4264b 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -45,7 +45,8 @@ http_archive( sha256 = ENVOY_SHA256, ) -load("@envoy//bazel:repositories.bzl", "envoy_dependencies") +load("@envoy//bazel:repositories.bzl", "GO_VERSION", "envoy_dependencies") + envoy_dependencies() load("@rules_foreign_cc//:workspace_definitions.bzl", "rules_foreign_cc_dependencies") @@ -57,17 +58,8 @@ cc_configure() load("@envoy_api//bazel:repositories.bzl", "api_dependencies") api_dependencies() -load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains") -go_rules_dependencies() -go_register_toolchains() +load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") -# Nov 28, 2017 (bazel 0.8.0 support) -RULES_PROTOBUF_SHA = "563b674a2ce6650d459732932ea2bc98c9c9a9bf" -RULES_PROTOBUF_SHA256 = "338e0d65cd709c6a6f9b5702466e641d536479be8b564d1e12a5d1de22a5cff6" +go_rules_dependencies() -http_archive( - name = "org_pubref_rules_protobuf", - strip_prefix = "rules_protobuf-" + RULES_PROTOBUF_SHA, - url = "https://github.com/pubref/rules_protobuf/archive/" + RULES_PROTOBUF_SHA + ".tar.gz", - sha256 = RULES_PROTOBUF_SHA256, -) +go_register_toolchains(go_version = GO_VERSION) diff --git a/istio.deps b/istio.deps index ae6bb2cb60f..386c3a40093 100644 --- a/istio.deps +++ b/istio.deps @@ -11,6 +11,6 @@ "name": "ENVOY_SHA", "repoName": "envoyproxy/envoy", "file": "WORKSPACE", - "lastStableSHA": "8912fa36acdf4367d37998d98cead376762d2b49" + "lastStableSHA": "805683f835bd63e4b7b9d89059aa0d3783924a93" } ] diff --git a/protobuf.bzl b/protobuf.bzl index 786bb25f222..da07cbb58e8 100644 --- a/protobuf.bzl +++ b/protobuf.bzl @@ -17,8 +17,8 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # Match SHA used by Envoy -PROTOBUF_SHA = "7492b5681231c79f0265793fa57dc780ae2481d6" -PROTOBUF_SHA256 = "46f1da3a6a6db66dd240cf95a5553198f7c6e98e6ac942fceb8a1cf03291d96e" +PROTOBUF_SHA = "582743bf40c5d3639a70f98f183914a2c0cd0680" +PROTOBUF_SHA256 = "cf9e2fb1d2cd30ec9d51ff1749045208bd641f290f64b85046485934b0e03783" def protobuf_repositories(load_repo=True, bind=True): if load_repo: diff --git a/src/envoy/http/mixer/control.cc b/src/envoy/http/mixer/control.cc index 32b7a384ab8..a74ff98ad5d 100644 --- a/src/envoy/http/mixer/control.cc +++ b/src/envoy/http/mixer/control.cc @@ -30,10 +30,10 @@ Control::Control(ControlDataSharedPtr control_data, : control_data_(control_data), check_client_factory_(Utils::GrpcClientFactoryForCluster( control_data_->config().check_cluster(), cm, scope, - dispatcher.timeSystem())), + dispatcher.timeSource())), report_client_factory_(Utils::GrpcClientFactoryForCluster( control_data_->config().report_cluster(), cm, scope, - dispatcher.timeSystem())), + dispatcher.timeSource())), stats_obj_(dispatcher, control_data_->stats(), control_data_->config() .config_pb() diff --git a/src/envoy/tcp/mixer/control.cc b/src/envoy/tcp/mixer/control.cc index 8b25c2fad1f..e384577049b 100644 --- a/src/envoy/tcp/mixer/control.cc +++ b/src/envoy/tcp/mixer/control.cc @@ -33,10 +33,10 @@ Control::Control(ControlDataSharedPtr control_data, dispatcher_(dispatcher), check_client_factory_(Utils::GrpcClientFactoryForCluster( control_data_->config().check_cluster(), cm, scope, - dispatcher.timeSystem())), + dispatcher.timeSource())), report_client_factory_(Utils::GrpcClientFactoryForCluster( control_data_->config().report_cluster(), cm, scope, - dispatcher.timeSystem())), + dispatcher.timeSource())), stats_obj_(dispatcher, control_data_->stats(), control_data_->config() .config_pb() From 6f62df69f1df5aa8907c13a3534ad2dfb33d84dd Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Wed, 13 Mar 2019 00:53:15 +0000 Subject: [PATCH 3/6] fix build Signed-off-by: Lizan Zhou --- test/integration/int_client.cc | 10 +++++----- test/integration/int_server.cc | 16 ++++++++-------- test/integration/int_server.h | 2 -- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/test/integration/int_client.cc b/test/integration/int_client.cc index 1c344741eca..dd174ba2914 100644 --- a/test/integration/int_client.cc +++ b/test/integration/int_client.cc @@ -85,7 +85,8 @@ class ClientStream : public Envoy::Http::StreamDecoder, // Envoy::Http::StreamCallbacks // - virtual void onResetStream(Envoy::Http::StreamResetReason reason) override { + virtual void onResetStream(Envoy::Http::StreamResetReason reason, + absl::string_view) override { // TODO test with h2 to see if we get any of these and whether the // connection error handling is enough to handle it. switch (reason) { @@ -446,8 +447,7 @@ Client::Client(const std::string &name) stats_(), thread_(nullptr), time_system_(), - api_(std::chrono::milliseconds(1), - Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_), + api_(Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_), dispatcher_{api_.allocateDispatcher()} {} Client::~Client() { @@ -565,8 +565,8 @@ LoadGenerator::LoadGenerator( ++responses_received_; uint64_t status = 0; - if (!Envoy::StringUtil::atoul(response->Status()->value().c_str(), - status)) { + if (!Envoy::StringUtil::atoull(response->Status()->value().c_str(), + status)) { ENVOY_LOG(error, "Connection({}:{}) received response with bad status", connection.name(), connection.id()); } else if (200 <= status && status < 300) { diff --git a/test/integration/int_server.cc b/test/integration/int_server.cc index 78ab9042535..5e2a79d0258 100644 --- a/test/integration/int_server.cc +++ b/test/integration/int_server.cc @@ -211,7 +211,8 @@ class ServerStreamImpl : public ServerStream, // Envoy::Http::StreamCallbacks // - virtual void onResetStream(Envoy::Http::StreamResetReason reason) override { + virtual void onResetStream(Envoy::Http::StreamResetReason reason, + absl::string_view) override { // TODO test with h2 to see if we get these and whether the connection error // handling is enough to handle it. switch (reason) { @@ -306,18 +307,19 @@ ServerConnection::ServerConnection( close_callback_(close_callback) { // TODO make use of network_connection_->socketOptions() and possibly http // settings; + constexpr uint32_t max_request_headers_kb = 2U; switch (http_type) { case Envoy::Http::CodecClient::Type::HTTP1: http_connection_ = std::make_unique( - network_connection, *this, Envoy::Http::Http1Settings()); + network_connection, *this, Envoy::Http::Http1Settings(), + max_request_headers_kb); break; case Envoy::Http::CodecClient::Type::HTTP2: { Envoy::Http::Http2Settings settings; settings.allow_connect_ = true; settings.allow_metadata_ = true; - constexpr uint32_t max_request_headers_kb = 2U; http_connection_ = std::make_unique( network_connection, *this, scope, settings, @@ -330,7 +332,8 @@ ServerConnection::ServerConnection( name_, id_, static_cast(http_type) + 1); http_connection_ = std::make_unique( - network_connection, *this, Envoy::Http::Http1Settings()); + network_connection, *this, Envoy::Http::Http1Settings(), + max_request_headers_kb); break; } } @@ -609,8 +612,7 @@ Server::Server(const std::string &name, : name_(name), stats_(), time_system_(), - api_(std::chrono::milliseconds(1), - Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_), + api_(Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_), dispatcher_(api_.allocateDispatcher()), connection_handler_(new Envoy::Server::ConnectionHandlerImpl( ENVOY_LOGGER(), *dispatcher_)), @@ -713,8 +715,6 @@ uint64_t Server::listenerTag() const { return 0; } const std::string &Server::name() const { return name_; } -bool Server::reverseWriteFilterOrder() const { return true; } - const Envoy::Network::FilterChain *Server::findFilterChain( const Envoy::Network::ConnectionSocket &) const { return &server_filter_chain_; diff --git a/test/integration/int_server.h b/test/integration/int_server.h index b7fe7653378..2e2789b84c2 100644 --- a/test/integration/int_server.h +++ b/test/integration/int_server.h @@ -359,8 +359,6 @@ class Server : public Envoy::Network::FilterChainManager, virtual const std::string &name() const override; - virtual bool reverseWriteFilterOrder() const override; - // // Envoy::Network::FilterChainManager // From 6e6f86a9d6c9cfccbc48f1a78d6977023c0039b4 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 28 Mar 2019 14:46:07 +0000 Subject: [PATCH 4/6] fix build Signed-off-by: Lizan Zhou --- src/envoy/http/authn/authenticator_base_test.cc | 10 +++++----- src/envoy/utils/utils.cc | 13 ++++++++----- test/integration/int_client.cc | 3 ++- test/integration/int_server.cc | 3 ++- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/envoy/http/authn/authenticator_base_test.cc b/src/envoy/http/authn/authenticator_base_test.cc index 1a476f065e7..41c09f610d7 100644 --- a/src/envoy/http/authn/authenticator_base_test.cc +++ b/src/envoy/http/authn/authenticator_base_test.cc @@ -88,7 +88,7 @@ class ValidateX509Test : public testing::TestWithParam, virtual ~ValidateX509Test() {} NiceMock connection_{}; - NiceMock ssl_{}; + NiceMock ssl_{}; Envoy::Http::HeaderMapImpl header_{}; FilterConfig filter_config_{}; FilterContext filter_context_{ @@ -142,7 +142,7 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerCert) { EXPECT_CALL(Const(ssl_), peerCertificatePresented()) .Times(1) .WillOnce(Return(true)); - EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return("foo")); + EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return(std::vector{"foo"})); EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_)); // When client certificate is present on mTLS, authenticated attribute should // be extracted. @@ -156,7 +156,7 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerSpiffeCert) { .WillOnce(Return(true)); EXPECT_CALL(ssl_, uriSanPeerCertificate()) .Times(1) - .WillOnce(Return("spiffe://foo")); + .WillOnce(Return(std::vector{"spiffe://foo"})); EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_)); // When client certificate is present on mTLS, authenticated attribute should @@ -171,7 +171,7 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerMalformedSpiffeCert) { .WillOnce(Return(true)); EXPECT_CALL(ssl_, uriSanPeerCertificate()) .Times(1) - .WillOnce(Return("spiffe:foo")); + .WillOnce(Return(std::vector{"spiffe:foo"})); EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_)); // When client certificate is present on mTLS and the spiffe subject format is @@ -193,7 +193,7 @@ class ValidateJwtTest : public testing::Test, // StrictMock request_info_{}; envoy::api::v2::core::Metadata dynamic_metadata_; NiceMock connection_{}; - // NiceMock ssl_{}; + // NiceMock ssl_{}; Envoy::Http::HeaderMapImpl header_{}; FilterConfig filter_config_{}; FilterContext filter_context_{dynamic_metadata_, header_, &connection_, diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index d598d12bc0e..3ec653ae12d 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -94,18 +94,21 @@ bool GetDestinationUID(const envoy::api::v2::core::Metadata& metadata, bool GetPrincipal(const Network::Connection* connection, bool peer, std::string* principal) { if (connection) { - Ssl::Connection* ssl = const_cast(connection->ssl()); + Ssl::ConnectionInfo* ssl = const_cast(connection->ssl()); if (ssl != nullptr) { - std::string result; + std::vector sans; + if (peer) { - result = ssl->uriSanPeerCertificate(); + sans = ssl->uriSanPeerCertificate(); } else { - result = ssl->uriSanLocalCertificate(); + sans = ssl->uriSanLocalCertificate(); } - if (result.empty()) { // empty result is not allowed + if (sans.empty()) { // empty result is not allowed return false; } + + std::string result = sans[0]; if (result.length() >= kSPIFFEPrefix.length() && result.compare(0, kSPIFFEPrefix.length(), kSPIFFEPrefix) == 0) { // Strip out the prefix "spiffe://" in the identity. diff --git a/test/integration/int_client.cc b/test/integration/int_client.cc index dd174ba2914..0db233326a4 100644 --- a/test/integration/int_client.cc +++ b/test/integration/int_client.cc @@ -447,7 +447,8 @@ Client::Client(const std::string &name) stats_(), thread_(nullptr), time_system_(), - api_(Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_), + api_(Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_, + Envoy::Filesystem::fileSystemForTest()), dispatcher_{api_.allocateDispatcher()} {} Client::~Client() { diff --git a/test/integration/int_server.cc b/test/integration/int_server.cc index 5e2a79d0258..e5a7416bec5 100644 --- a/test/integration/int_server.cc +++ b/test/integration/int_server.cc @@ -612,7 +612,8 @@ Server::Server(const std::string &name, : name_(name), stats_(), time_system_(), - api_(Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_), + api_(Envoy::Thread::ThreadFactorySingleton::get(), stats_, time_system_, + Envoy::Filesystem::fileSystemForTest()), dispatcher_(api_.allocateDispatcher()), connection_handler_(new Envoy::Server::ConnectionHandlerImpl( ENVOY_LOGGER(), *dispatcher_)), From 3f6618b58205442585467ca13afbf168f16e7f00 Mon Sep 17 00:00:00 2001 From: Mandar U Jog Date: Thu, 28 Mar 2019 16:07:51 +0000 Subject: [PATCH 5/6] fix formatting --- src/envoy/http/authn/authenticator_base_test.cc | 4 +++- src/envoy/utils/utils.cc | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/envoy/http/authn/authenticator_base_test.cc b/src/envoy/http/authn/authenticator_base_test.cc index 41c09f610d7..eb59538ad82 100644 --- a/src/envoy/http/authn/authenticator_base_test.cc +++ b/src/envoy/http/authn/authenticator_base_test.cc @@ -142,7 +142,9 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerCert) { EXPECT_CALL(Const(ssl_), peerCertificatePresented()) .Times(1) .WillOnce(Return(true)); - EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return(std::vector{"foo"})); + EXPECT_CALL(ssl_, uriSanPeerCertificate()) + .Times(1) + .WillOnce(Return(std::vector{"foo"})); EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_)); // When client certificate is present on mTLS, authenticated attribute should // be extracted. diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index 3ec653ae12d..b45e042a1e7 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -94,7 +94,8 @@ bool GetDestinationUID(const envoy::api::v2::core::Metadata& metadata, bool GetPrincipal(const Network::Connection* connection, bool peer, std::string* principal) { if (connection) { - Ssl::ConnectionInfo* ssl = const_cast(connection->ssl()); + Ssl::ConnectionInfo* ssl = + const_cast(connection->ssl()); if (ssl != nullptr) { std::vector sans; From eef6690db70d92c18cb398a2bca7a141468e12a6 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 14 Mar 2019 01:08:31 +0000 Subject: [PATCH 6/6] fix status match Signed-off-by: Lizan Zhou --- src/istio/mixerclient/BUILD | 1 + src/istio/mixerclient/status_util.cc | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/istio/mixerclient/BUILD b/src/istio/mixerclient/BUILD index 7a0f0194135..c7072a3ddd6 100644 --- a/src/istio/mixerclient/BUILD +++ b/src/istio/mixerclient/BUILD @@ -63,6 +63,7 @@ cc_library( "//include/istio/utils:simple_lru_cache", "//src/istio/prefetch:quota_prefetch_lib", "//src/istio/utils:utils_lib", + "@com_google_absl//absl/strings", ], ) diff --git a/src/istio/mixerclient/status_util.cc b/src/istio/mixerclient/status_util.cc index bee88008827..01ab0e5c517 100644 --- a/src/istio/mixerclient/status_util.cc +++ b/src/istio/mixerclient/status_util.cc @@ -14,14 +14,15 @@ */ #include "src/istio/mixerclient/status_util.h" +#include "absl/strings/match.h" +#include "absl/strings/string_view.h" namespace istio { namespace mixerclient { -static ::google::protobuf::StringPiece TIMEOUT_MESSAGE( - "upstream request timeout"); -static ::google::protobuf::StringPiece SEND_ERROR_MESSAGE( - "upstream connect error or disconnect/reset before headers"); +static constexpr absl::string_view TIMEOUT_MESSAGE{"upstream request timeout"}; +static constexpr absl::string_view SEND_ERROR_MESSAGE{ + "upstream connect error or disconnect/reset before headers"}; TransportResult TransportStatus( const ::google::protobuf::util::Status &status) { @@ -31,10 +32,13 @@ TransportResult TransportStatus( if (::google::protobuf::util::error::Code::UNAVAILABLE == status.error_code()) { - if (TIMEOUT_MESSAGE == status.error_message()) { + absl::string_view error_message{status.error_message().data(), + static_cast( + status.error_message().length())}; + if (absl::StartsWith(error_message, TIMEOUT_MESSAGE)) { return TransportResult::RESPONSE_TIMEOUT; } - if (SEND_ERROR_MESSAGE == status.error_message()) { + if (absl::StartsWith(error_message, SEND_ERROR_MESSAGE)) { return TransportResult::SEND_ERROR; } }