From a145ed62308744d76347fc76d04ed7cb0718fbc8 Mon Sep 17 00:00:00 2001 From: Venil Noronha Date: Fri, 2 Nov 2018 21:44:49 -0700 Subject: [PATCH 1/2] Add a new TCP cluster rewrite filter This commit adds a new TCP cluster rewrite filter which allows users to rewrite TCP cluster names obtained via TLS SNI by matching via regex configuration. Signed-off-by: Venil Noronha --- WORKSPACE | 2 +- include/istio/mixerclient/check_response.h | 2 +- include/istio/mixerclient/environment.h | 2 +- istio.deps | 4 +- repositories.bzl | 19 ++- src/envoy/BUILD | 1 + src/envoy/tcp/tcp_cluster_rewrite/BUILD | 72 ++++++++ src/envoy/tcp/tcp_cluster_rewrite/config.cc | 73 ++++++++ src/envoy/tcp/tcp_cluster_rewrite/config.h | 56 +++++++ .../tcp/tcp_cluster_rewrite/config_test.cc | 49 ++++++ .../tcp_cluster_rewrite.cc | 69 ++++++++ .../tcp_cluster_rewrite/tcp_cluster_rewrite.h | 81 +++++++++ .../tcp_cluster_rewrite_test.cc | 156 ++++++++++++++++++ src/istio/mixerclient/attribute_compressor.h | 2 +- src/istio/mixerclient/referenced.h | 2 +- .../istio_http_integration_test.cc | 3 +- 16 files changed, 583 insertions(+), 10 deletions(-) create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/BUILD create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/config.cc create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/config.h create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/config_test.cc create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h create mode 100644 src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc diff --git a/WORKSPACE b/WORKSPACE index 212363d7462..deaccaf7849 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -30,7 +30,7 @@ bind( ) # When updating envoy sha manually please update the sha in istio.deps file also -ENVOY_SHA = "de039269f54aa21aa0da21da89a5075aa3db3bb9" +ENVOY_SHA = "45a460fabf34698a875060482de96f7f618bdc9f" http_archive( name = "envoy", diff --git a/include/istio/mixerclient/check_response.h b/include/istio/mixerclient/check_response.h index bd9690533af..86000628b1e 100644 --- a/include/istio/mixerclient/check_response.h +++ b/include/istio/mixerclient/check_response.h @@ -17,7 +17,7 @@ #define ISTIO_MIXERCLIENT_CHECK_RESPONSE_H #include "google/protobuf/stubs/status.h" -#include "mixer/v1/check.pb.h" +#include "mixer/v1/mixer.pb.h" namespace istio { namespace mixerclient { diff --git a/include/istio/mixerclient/environment.h b/include/istio/mixerclient/environment.h index 9e24f9e8d15..499e5bd269b 100644 --- a/include/istio/mixerclient/environment.h +++ b/include/istio/mixerclient/environment.h @@ -18,7 +18,7 @@ #include "check_response.h" #include "google/protobuf/stubs/status.h" -#include "mixer/v1/service.pb.h" +#include "mixer/v1/mixer.pb.h" #include "timer.h" namespace istio { diff --git a/istio.deps b/istio.deps index 2ac5e9820f2..808e154c192 100644 --- a/istio.deps +++ b/istio.deps @@ -4,13 +4,13 @@ "name": "ISTIO_API", "repoName": "api", "file": "repositories.bzl", - "lastStableSHA": "214c7598afb74f7f4dea49f77e45832c49382a15" + "lastStableSHA": "6b9e3a501e6ef254958bf82f7b74c37d64a57a15" }, { "_comment": "", "name": "ENVOY_SHA", "repoName": "envoyproxy/envoy", "file": "WORKSPACE", - "lastStableSHA": "de039269f54aa21aa0da21da89a5075aa3db3bb9" + "lastStableSHA": "45a460fabf34698a875060482de96f7f618bdc9f" } ] diff --git a/repositories.bzl b/repositories.bzl index fb8d3782fa4..c6f017dbec4 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -113,7 +113,7 @@ cc_library( actual = "@googletest_git//:googletest_prod", ) -ISTIO_API = "62c345bd6d6e4c2047dd2dee128b7413231be7b4" +ISTIO_API = "6b9e3a501e6ef254958bf82f7b74c37d64a57a15" def mixerapi_repositories(bind=True): BUILD = """ @@ -192,6 +192,19 @@ cc_proto_library( ], ) +cc_proto_library( + name = "tcp_cluster_rewrite_config_cc_proto", + srcs = glob( + ["envoy/config/filter/network/tcp_cluster_rewrite/v2alpha1/*.proto", ], + ), + default_runtime = "//external:protobuf", + protoc = "//external:protoc", + visibility = ["//visibility:public"], + deps = [ + "//external:cc_gogoproto", + ], +) + filegroup( name = "global_dictionary_file", srcs = ["mixer/v1/global_dictionary.yaml"], @@ -222,6 +235,10 @@ filegroup( name = "jwt_auth_config_cc_proto", actual = "@mixerapi_git//:jwt_auth_config_cc_proto", ) + native.bind( + name = "tcp_cluster_rewrite_config_cc_proto", + actual = "@mixerapi_git//:tcp_cluster_rewrite_config_cc_proto", + ) load(":protobuf.bzl", "protobuf_repositories") load(":cc_gogo_protobuf.bzl", "cc_gogoproto_repositories") diff --git a/src/envoy/BUILD b/src/envoy/BUILD index c6eeb279f2e..9f20000de80 100644 --- a/src/envoy/BUILD +++ b/src/envoy/BUILD @@ -29,6 +29,7 @@ envoy_cc_binary( "//src/envoy/http/jwt_auth:http_filter_factory", "//src/envoy/http/mixer:filter_lib", "//src/envoy/tcp/mixer:filter_lib", + "//src/envoy/tcp/tcp_cluster_rewrite:tcp_cluster_rewrite_lib", "@envoy//source/exe:envoy_main_entry_lib", ], ) diff --git a/src/envoy/tcp/tcp_cluster_rewrite/BUILD b/src/envoy/tcp/tcp_cluster_rewrite/BUILD new file mode 100644 index 00000000000..a163d3da08f --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/BUILD @@ -0,0 +1,72 @@ +# Copyright 2018 Istio Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +################################################################################ +# + +package(default_visibility = ["//visibility:public"]) + +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_binary", + "envoy_cc_library", + "envoy_cc_test", +) + +envoy_cc_library( + name = "tcp_cluster_rewrite_lib", + srcs = ["tcp_cluster_rewrite.cc"], + hdrs = ["tcp_cluster_rewrite.h"], + repository = "@envoy", + deps = [ + "//external:tcp_cluster_rewrite_config_cc_proto", + "@envoy//source/exe:envoy_common_lib", + ], +) + +envoy_cc_library( + name = "config_lib", + srcs = ["config.cc"], + hdrs = ["config.h"], + repository = "@envoy", + deps = [ + ":tcp_cluster_rewrite_lib", + "//src/envoy/utils:utils_lib", + "//external:tcp_cluster_rewrite_config_cc_proto", + "@envoy//source/exe:envoy_common_lib", + ], +) + +envoy_cc_test( + name = "tcp_cluster_rewrite_test", + srcs = ["tcp_cluster_rewrite_test.cc"], + repository = "@envoy", + deps = [ + ":tcp_cluster_rewrite_lib", + ":config_lib", + "@envoy//test/mocks/network:network_mocks", + "@envoy//test/mocks/server:server_mocks", + "@envoy//test/mocks/stream_info:stream_info_mocks", + ], +) + +envoy_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + repository = "@envoy", + deps = [ + ":config_lib", + "@envoy//test/mocks/server:server_mocks", + ], +) diff --git a/src/envoy/tcp/tcp_cluster_rewrite/config.cc b/src/envoy/tcp/tcp_cluster_rewrite/config.cc new file mode 100644 index 00000000000..35644f31a73 --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/config.cc @@ -0,0 +1,73 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "src/envoy/tcp/tcp_cluster_rewrite/config.h" +#include "src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h" + +#include "envoy/registry/registry.h" +#include "envoy/server/filter_config.h" +#include "src/envoy/utils/config.h" + +using namespace ::istio::envoy::config::filter::network::tcp_cluster_rewrite; + +namespace Envoy { +namespace Tcp { +namespace TcpClusterRewrite { + +Network::FilterFactoryCb +TcpClusterRewriteFilterConfigFactory::createFilterFactory( + const Json::Object& config_json, Server::Configuration::FactoryContext&) { + v2alpha1::TcpClusterRewrite config_pb; + if (!Utils::ReadV2Config(config_json, &config_pb) && + !Utils::ReadV1Config(config_json, &config_pb)) { + throw EnvoyException("Failed to parse JSON config"); + } + return createFilterFactory(config_pb); +} + +Network::FilterFactoryCb +TcpClusterRewriteFilterConfigFactory::createFilterFactoryFromProto( + const Protobuf::Message& config, Server::Configuration::FactoryContext&) { + return createFilterFactory( + dynamic_cast(config)); +} + +ProtobufTypes::MessagePtr +TcpClusterRewriteFilterConfigFactory::createEmptyConfigProto() { + return ProtobufTypes::MessagePtr{new v2alpha1::TcpClusterRewrite}; +} + +Network::FilterFactoryCb +TcpClusterRewriteFilterConfigFactory::createFilterFactory( + const v2alpha1::TcpClusterRewrite& config_pb) { + TcpClusterRewriteFilterConfigSharedPtr config( + std::make_shared(config_pb)); + return [config](Network::FilterManager& filter_manager) -> void { + filter_manager.addReadFilter( + std::make_shared(config)); + }; +} + +/** + * Static registration for the TCP cluster rewrite filter. @see RegisterFactory. + */ +static Registry::RegisterFactory< + TcpClusterRewriteFilterConfigFactory, + Server::Configuration::NamedNetworkFilterConfigFactory> + registered_; + +} // namespace TcpClusterRewrite +} // namespace Tcp +} // namespace Envoy diff --git a/src/envoy/tcp/tcp_cluster_rewrite/config.h b/src/envoy/tcp/tcp_cluster_rewrite/config.h new file mode 100644 index 00000000000..8db4f4c3488 --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/config.h @@ -0,0 +1,56 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "envoy/config/filter/network/tcp_cluster_rewrite/v2alpha1/config.pb.h" + +#include "envoy/network/connection.h" +#include "envoy/network/filter.h" +#include "envoy/registry/registry.h" +#include "envoy/server/filter_config.h" + +using namespace ::istio::envoy::config::filter::network::tcp_cluster_rewrite; + +namespace Envoy { +namespace Tcp { +namespace TcpClusterRewrite { + +/** + * Config registration for the TCP cluster rewrite filter. @see + * NamedNetworkFilterConfigFactory. + */ +class TcpClusterRewriteFilterConfigFactory + : public Server::Configuration::NamedNetworkFilterConfigFactory { + public: + Network::FilterFactoryCb createFilterFactory( + const Json::Object&, Server::Configuration::FactoryContext&) override; + + Network::FilterFactoryCb createFilterFactoryFromProto( + const Protobuf::Message&, + Server::Configuration::FactoryContext&) override; + + ProtobufTypes::MessagePtr createEmptyConfigProto() override; + + std::string name() override { return "tcp_cluster_rewrite"; } + + private: + Network::FilterFactoryCb createFilterFactory( + const v2alpha1::TcpClusterRewrite& config_pb); +}; + +} // namespace TcpClusterRewrite +} // namespace Tcp +} // namespace Envoy diff --git a/src/envoy/tcp/tcp_cluster_rewrite/config_test.cc b/src/envoy/tcp/tcp_cluster_rewrite/config_test.cc new file mode 100644 index 00000000000..142cf1e8104 --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/config_test.cc @@ -0,0 +1,49 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "src/envoy/tcp/tcp_cluster_rewrite/config.h" + +#include "test/mocks/server/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace ::istio::envoy::config::filter::network::tcp_cluster_rewrite; +using testing::_; + +namespace Envoy { +namespace Tcp { +namespace TcpClusterRewrite { + +TEST(ConfigTest, ConfigTest) { + NiceMock context; + TcpClusterRewriteFilterConfigFactory factory; + v2alpha1::TcpClusterRewrite config = + *dynamic_cast( + factory.createEmptyConfigProto().get()); + + config.set_cluster_pattern("connection\\.sni"); + config.set_cluster_replacement("replacement.sni"); + + Network::FilterFactoryCb cb = + factory.createFilterFactoryFromProto(config, context); + Network::MockConnection connection; + EXPECT_CALL(connection, addReadFilter(_)); + cb(connection); +} + +} // namespace TcpClusterRewrite +} // namespace Tcp +} // namespace Envoy diff --git a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc new file mode 100644 index 00000000000..24118c113d6 --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc @@ -0,0 +1,69 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h" + +#include "envoy/network/connection.h" + +#include "common/common/assert.h" +#include "common/tcp_proxy/tcp_proxy.h" + +using namespace ::istio::envoy::config::filter::network::tcp_cluster_rewrite; + +namespace Envoy { +namespace Tcp { +namespace TcpClusterRewrite { + +TcpClusterRewriteFilterConfig::TcpClusterRewriteFilterConfig( + const v2alpha1::TcpClusterRewrite& proto_config) { + if (!proto_config.cluster_pattern().empty()) { + should_rewrite_cluster_ = true; + cluster_pattern_ = std::regex(proto_config.cluster_pattern()); + cluster_replacement_ = proto_config.cluster_replacement(); + } else { + should_rewrite_cluster_ = false; + } +} + +Network::FilterStatus TcpClusterRewriteFilter::onNewConnection() { + absl::string_view sni = read_callbacks_->connection().requestedServerName(); + ENVOY_CONN_LOG(trace, + "tcp_cluster_rewrite: new connection with server name {}", + read_callbacks_->connection(), sni); + + if (!sni.empty()) { + // Rewrite the SNI value prior to setting the tcp_proxy cluster name. + std::string cluster_name(absl::StrCat(sni)); + if (config_->shouldRewriteCluster()) { + cluster_name = std::regex_replace(cluster_name, config_->clusterPattern(), + config_->clusterReplacement()); + } + ENVOY_CONN_LOG(trace, "tcp_cluster_rewrite: tcp proxy cluster name {}", + read_callbacks_->connection(), cluster_name); + + // Set the tcp_proxy cluster to the same value as the (rewritten) SNI. The + // data is mutable to allow other filters to change it. + read_callbacks_->connection().streamInfo().filterState().setData( + TcpProxy::PerConnectionCluster::Key, + std::make_unique(cluster_name), + StreamInfo::FilterState::StateType::Mutable); + } + + return Network::FilterStatus::Continue; +} + +} // namespace TcpClusterRewrite +} // namespace Tcp +} // namespace Envoy diff --git a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h new file mode 100644 index 00000000000..1a96eada547 --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h @@ -0,0 +1,81 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include "envoy/config/filter/network/tcp_cluster_rewrite/v2alpha1/config.pb.h" +#include "envoy/network/filter.h" + +#include "common/common/logger.h" + +using namespace ::istio::envoy::config::filter::network::tcp_cluster_rewrite; + +namespace Envoy { +namespace Tcp { +namespace TcpClusterRewrite { + +/** + * Configuration for the TCP cluster rewrite filter. + */ +class TcpClusterRewriteFilterConfig { + public: + TcpClusterRewriteFilterConfig( + const v2alpha1::TcpClusterRewrite& proto_config); + + bool shouldRewriteCluster() const { return should_rewrite_cluster_; } + std::regex clusterPattern() const { return cluster_pattern_; } + std::string clusterReplacement() const { return cluster_replacement_; } + + private: + bool should_rewrite_cluster_; + std::regex cluster_pattern_; + std::string cluster_replacement_; +}; + +typedef std::shared_ptr + TcpClusterRewriteFilterConfigSharedPtr; + +/** + * Implementation of the TCP cluster rewrite filter that sets the upstream + * cluster name from the SNI field in the TLS connection. + */ +class TcpClusterRewriteFilter : public Network::ReadFilter, + Logger::Loggable { + public: + TcpClusterRewriteFilter(TcpClusterRewriteFilterConfigSharedPtr config) + : config_(config) {} + + // Network::ReadFilter + Network::FilterStatus onData(Buffer::Instance&, bool) override { + return Network::FilterStatus::Continue; + } + + Network::FilterStatus onNewConnection() override; + + void initializeReadFilterCallbacks( + Network::ReadFilterCallbacks& callbacks) override { + read_callbacks_ = &callbacks; + } + + private: + TcpClusterRewriteFilterConfigSharedPtr config_; + Network::ReadFilterCallbacks* read_callbacks_{}; +}; + +} // namespace TcpClusterRewrite +} // namespace Tcp +} // namespace Envoy diff --git a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc new file mode 100644 index 00000000000..0ad1ae181fa --- /dev/null +++ b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc @@ -0,0 +1,156 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "common/tcp_proxy/tcp_proxy.h" + +#include "src/envoy/tcp/tcp_cluster_rewrite/config.h" +#include "src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.h" + +#include "test/mocks/network/mocks.h" +#include "test/mocks/server/mocks.h" +#include "test/mocks/stream_info/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace ::istio::envoy::config::filter::network::tcp_cluster_rewrite; +using testing::_; +using testing::Matcher; +using testing::NiceMock; +using testing::Return; +using testing::ReturnRef; + +namespace Envoy { +namespace Tcp { +namespace TcpClusterRewrite { + +class TcpClusterRewriteFilterTest : public testing::Test { + public: + TcpClusterRewriteFilterTest() { + ON_CALL(filter_callbacks_.connection_, streamInfo()) + .WillByDefault(ReturnRef(stream_info_)); + ON_CALL(Const(filter_callbacks_.connection_), streamInfo()) + .WillByDefault(ReturnRef(stream_info_)); + configure(v2alpha1::TcpClusterRewrite()); + } + + void configure(v2alpha1::TcpClusterRewrite proto_config) { + config_ = std::make_unique(proto_config); + filter_ = std::make_unique(config_); + filter_->initializeReadFilterCallbacks(filter_callbacks_); + } + + NiceMock filter_callbacks_; + NiceMock stream_info_; + TcpClusterRewriteFilterConfigSharedPtr config_; + std::unique_ptr filter_; +}; + +TEST_F(TcpClusterRewriteFilterTest, SetTcpProxyClusterOnlyIfSniIsPresent) { + // no sni + { + ON_CALL(filter_callbacks_.connection_, requestedServerName()) + .WillByDefault(Return(EMPTY_STRING)); + filter_->onNewConnection(); + + EXPECT_FALSE( + stream_info_.filterState().hasData( + TcpProxy::PerConnectionCluster::Key)); + } + + // with sni + { + ON_CALL(filter_callbacks_.connection_, requestedServerName()) + .WillByDefault(Return("filter_state_cluster")); + filter_->onNewConnection(); + + EXPECT_TRUE( + stream_info_.filterState().hasData( + TcpProxy::PerConnectionCluster::Key)); + + auto per_connection_cluster = + stream_info_.filterState() + .getDataReadOnly( + TcpProxy::PerConnectionCluster::Key); + EXPECT_EQ(per_connection_cluster.value(), "filter_state_cluster"); + } +} + +TEST_F(TcpClusterRewriteFilterTest, ClusterRewrite) { + // no rewrite + { + ON_CALL(filter_callbacks_.connection_, requestedServerName()) + .WillByDefault(Return("hello.ns1.svc.cluster.local")); + filter_->onNewConnection(); + + EXPECT_TRUE( + stream_info_.filterState().hasData( + TcpProxy::PerConnectionCluster::Key)); + + auto per_connection_cluster = + stream_info_.filterState() + .getDataReadOnly( + TcpProxy::PerConnectionCluster::Key); + EXPECT_EQ(per_connection_cluster.value(), "hello.ns1.svc.cluster.local"); + } + + // with simple rewrite + { + v2alpha1::TcpClusterRewrite proto_config; + proto_config.set_cluster_pattern("\\.global$"); + proto_config.set_cluster_replacement(".svc.cluster.local"); + configure(proto_config); + + ON_CALL(filter_callbacks_.connection_, requestedServerName()) + .WillByDefault(Return("hello.ns1.global")); + filter_->onNewConnection(); + + EXPECT_TRUE( + stream_info_.filterState().hasData( + TcpProxy::PerConnectionCluster::Key)); + + auto per_connection_cluster = + stream_info_.filterState() + .getDataReadOnly( + TcpProxy::PerConnectionCluster::Key); + EXPECT_EQ(per_connection_cluster.value(), "hello.ns1.svc.cluster.local"); + } + + // with regex rewrite + { + v2alpha1::TcpClusterRewrite proto_config; + proto_config.set_cluster_pattern("^.*$"); + proto_config.set_cluster_replacement("another.svc.cluster.local"); + configure(proto_config); + + ON_CALL(filter_callbacks_.connection_, requestedServerName()) + .WillByDefault(Return("hello.ns1.global")); + filter_->onNewConnection(); + + EXPECT_TRUE( + stream_info_.filterState().hasData( + TcpProxy::PerConnectionCluster::Key)); + + auto per_connection_cluster = + stream_info_.filterState() + .getDataReadOnly( + TcpProxy::PerConnectionCluster::Key); + EXPECT_EQ(per_connection_cluster.value(), "another.svc.cluster.local"); + } +} + +} // namespace TcpClusterRewrite +} // namespace Tcp +} // namespace Envoy diff --git a/src/istio/mixerclient/attribute_compressor.h b/src/istio/mixerclient/attribute_compressor.h index f6c9c0203ec..0b0ed899a89 100644 --- a/src/istio/mixerclient/attribute_compressor.h +++ b/src/istio/mixerclient/attribute_compressor.h @@ -19,7 +19,7 @@ #include #include "mixer/v1/attributes.pb.h" -#include "mixer/v1/report.pb.h" +#include "mixer/v1/mixer.pb.h" namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/referenced.h b/src/istio/mixerclient/referenced.h index 698c5bcbf32..002b057efb6 100644 --- a/src/istio/mixerclient/referenced.h +++ b/src/istio/mixerclient/referenced.h @@ -19,7 +19,7 @@ #include #include "include/istio/utils/concat_hash.h" -#include "mixer/v1/check.pb.h" +#include "mixer/v1/mixer.pb.h" namespace istio { namespace mixerclient { diff --git a/test/integration/istio_http_integration_test.cc b/test/integration/istio_http_integration_test.cc index 5e169493ecb..c3f7a25912a 100644 --- a/test/integration/istio_http_integration_test.cc +++ b/test/integration/istio_http_integration_test.cc @@ -22,8 +22,7 @@ #include "fmt/printf.h" #include "gmock/gmock.h" #include "include/istio/utils/attribute_names.h" -#include "mixer/v1/check.pb.h" -#include "mixer/v1/report.pb.h" +#include "mixer/v1/mixer.pb.h" #include "src/envoy/utils/filter_names.h" #include "test/integration/http_protocol_integration.h" From d6331857db188f78d39a00ae8345d393c9ea8167 Mon Sep 17 00:00:00 2001 From: Venil Noronha Date: Mon, 5 Nov 2018 10:12:35 -0800 Subject: [PATCH 2/2] Make TCP cluster rewrite stackable on SNI filter This commit updates the TCP Cluster Rewrite filter to be stackable on the SNI Cluster filter. Signed-off-by: Venil Noronha --- src/envoy/tcp/tcp_cluster_rewrite/config.cc | 3 +- .../tcp_cluster_rewrite.cc | 43 +++++++++------- .../tcp_cluster_rewrite_test.cc | 49 +++++-------------- 3 files changed, 40 insertions(+), 55 deletions(-) diff --git a/src/envoy/tcp/tcp_cluster_rewrite/config.cc b/src/envoy/tcp/tcp_cluster_rewrite/config.cc index 35644f31a73..5decbbb38f0 100644 --- a/src/envoy/tcp/tcp_cluster_rewrite/config.cc +++ b/src/envoy/tcp/tcp_cluster_rewrite/config.cc @@ -30,8 +30,7 @@ Network::FilterFactoryCb TcpClusterRewriteFilterConfigFactory::createFilterFactory( const Json::Object& config_json, Server::Configuration::FactoryContext&) { v2alpha1::TcpClusterRewrite config_pb; - if (!Utils::ReadV2Config(config_json, &config_pb) && - !Utils::ReadV1Config(config_json, &config_pb)) { + if (!Utils::ReadV2Config(config_json, &config_pb)) { throw EnvoyException("Failed to parse JSON config"); } return createFilterFactory(config_pb); diff --git a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc index 24118c113d6..b189c40abd6 100644 --- a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc +++ b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite.cc @@ -38,29 +38,38 @@ TcpClusterRewriteFilterConfig::TcpClusterRewriteFilterConfig( } Network::FilterStatus TcpClusterRewriteFilter::onNewConnection() { - absl::string_view sni = read_callbacks_->connection().requestedServerName(); - ENVOY_CONN_LOG(trace, - "tcp_cluster_rewrite: new connection with server name {}", - read_callbacks_->connection(), sni); - - if (!sni.empty()) { - // Rewrite the SNI value prior to setting the tcp_proxy cluster name. - std::string cluster_name(absl::StrCat(sni)); - if (config_->shouldRewriteCluster()) { - cluster_name = std::regex_replace(cluster_name, config_->clusterPattern(), - config_->clusterReplacement()); - } - ENVOY_CONN_LOG(trace, "tcp_cluster_rewrite: tcp proxy cluster name {}", + if (config_->shouldRewriteCluster() && + read_callbacks_->connection() + .streamInfo() + .filterState() + .hasData( + TcpProxy::PerConnectionCluster::Key)) { + absl::string_view cluster_name = + read_callbacks_->connection() + .streamInfo() + .filterState() + .getDataReadOnly( + TcpProxy::PerConnectionCluster::Key) + .value(); + ENVOY_CONN_LOG(trace, + "tcp_cluster_rewrite: new connection with server name {}", read_callbacks_->connection(), cluster_name); - // Set the tcp_proxy cluster to the same value as the (rewritten) SNI. The - // data is mutable to allow other filters to change it. + // Rewrite the cluster name prior to setting the tcp_proxy cluster name. + std::string final_cluster_name(absl::StrCat(cluster_name)); + final_cluster_name = + std::regex_replace(final_cluster_name, config_->clusterPattern(), + config_->clusterReplacement()); + ENVOY_CONN_LOG(trace, + "tcp_cluster_rewrite: final tcp proxy cluster name {}", + read_callbacks_->connection(), final_cluster_name); + + // The data is mutable to allow other filters to change it. read_callbacks_->connection().streamInfo().filterState().setData( TcpProxy::PerConnectionCluster::Key, - std::make_unique(cluster_name), + std::make_unique(final_cluster_name), StreamInfo::FilterState::StateType::Mutable); } - return Network::FilterStatus::Continue; } diff --git a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc index 0ad1ae181fa..a63ae3d937a 100644 --- a/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc +++ b/src/envoy/tcp/tcp_cluster_rewrite/tcp_cluster_rewrite_test.cc @@ -58,41 +58,14 @@ class TcpClusterRewriteFilterTest : public testing::Test { std::unique_ptr filter_; }; -TEST_F(TcpClusterRewriteFilterTest, SetTcpProxyClusterOnlyIfSniIsPresent) { - // no sni - { - ON_CALL(filter_callbacks_.connection_, requestedServerName()) - .WillByDefault(Return(EMPTY_STRING)); - filter_->onNewConnection(); - - EXPECT_FALSE( - stream_info_.filterState().hasData( - TcpProxy::PerConnectionCluster::Key)); - } - - // with sni - { - ON_CALL(filter_callbacks_.connection_, requestedServerName()) - .WillByDefault(Return("filter_state_cluster")); - filter_->onNewConnection(); - - EXPECT_TRUE( - stream_info_.filterState().hasData( - TcpProxy::PerConnectionCluster::Key)); - - auto per_connection_cluster = - stream_info_.filterState() - .getDataReadOnly( - TcpProxy::PerConnectionCluster::Key); - EXPECT_EQ(per_connection_cluster.value(), "filter_state_cluster"); - } -} - TEST_F(TcpClusterRewriteFilterTest, ClusterRewrite) { // no rewrite { - ON_CALL(filter_callbacks_.connection_, requestedServerName()) - .WillByDefault(Return("hello.ns1.svc.cluster.local")); + stream_info_.filterState().setData( + TcpProxy::PerConnectionCluster::Key, + std::make_unique( + "hello.ns1.svc.cluster.local"), + StreamInfo::FilterState::StateType::Mutable); filter_->onNewConnection(); EXPECT_TRUE( @@ -113,8 +86,10 @@ TEST_F(TcpClusterRewriteFilterTest, ClusterRewrite) { proto_config.set_cluster_replacement(".svc.cluster.local"); configure(proto_config); - ON_CALL(filter_callbacks_.connection_, requestedServerName()) - .WillByDefault(Return("hello.ns1.global")); + stream_info_.filterState().setData( + TcpProxy::PerConnectionCluster::Key, + std::make_unique("hello.ns1.global"), + StreamInfo::FilterState::StateType::Mutable); filter_->onNewConnection(); EXPECT_TRUE( @@ -135,8 +110,10 @@ TEST_F(TcpClusterRewriteFilterTest, ClusterRewrite) { proto_config.set_cluster_replacement("another.svc.cluster.local"); configure(proto_config); - ON_CALL(filter_callbacks_.connection_, requestedServerName()) - .WillByDefault(Return("hello.ns1.global")); + stream_info_.filterState().setData( + TcpProxy::PerConnectionCluster::Key, + std::make_unique("hello.ns1.global"), + StreamInfo::FilterState::StateType::Mutable); filter_->onNewConnection(); EXPECT_TRUE(