diff --git a/include/istio/control/tcp/controller.h b/include/istio/control/tcp/controller.h index ff2e40a203e..fa2ce3255fe 100644 --- a/include/istio/control/tcp/controller.h +++ b/include/istio/control/tcp/controller.h @@ -18,6 +18,7 @@ #include "include/istio/control/tcp/request_handler.h" #include "include/istio/mixerclient/client.h" +#include "include/istio/utils/local_attributes.h" #include "mixer/v1/config/client/client_config.pb.h" namespace istio { @@ -39,14 +40,17 @@ class Controller { // * mixer_config: the mixer client config. // * some functions provided by the environment (Envoy) struct Options { - Options(const ::istio::mixer::v1::config::client::TcpClientConfig& config) - : config(config) {} + Options(const ::istio::mixer::v1::config::client::TcpClientConfig& config, + const ::istio::utils::LocalNode& local_node) + : config(config), local_node(local_node) {} // Mixer filter config const ::istio::mixer::v1::config::client::TcpClientConfig& config; // Some plaform functions for mixer client library. ::istio::mixerclient::Environment env; + + const ::istio::utils::LocalNode& local_node; }; // The factory function to create a new instance of the controller. diff --git a/include/istio/utils/local_attributes.h b/include/istio/utils/local_attributes.h index 76c1c3ee3ff..0935e3c93b3 100644 --- a/include/istio/utils/local_attributes.h +++ b/include/istio/utils/local_attributes.h @@ -49,6 +49,9 @@ void CreateLocalAttributes(const LocalNode& local, bool SerializeForwardedAttributes(const LocalNode& local, std::string* serialized_forward_attributes); +// check if this listener is outbound based on "context.reporter.kind" attribute +bool IsOutbound(const ::istio::mixer::v1::Attributes& attributes); + } // namespace utils } // namespace istio diff --git a/src/envoy/tcp/mixer/BUILD b/src/envoy/tcp/mixer/BUILD index f9cba6d787e..ecf05489ce1 100644 --- a/src/envoy/tcp/mixer/BUILD +++ b/src/envoy/tcp/mixer/BUILD @@ -36,6 +36,7 @@ envoy_cc_library( deps = [ "//src/envoy/utils:utils_lib", "//src/istio/control/tcp:control_lib", + "//src/istio/utils:utils_lib", "@envoy//source/exe:envoy_common_lib", ], ) diff --git a/src/envoy/tcp/mixer/control.cc b/src/envoy/tcp/mixer/control.cc index 6f8bee5fd64..85789069ec7 100644 --- a/src/envoy/tcp/mixer/control.cc +++ b/src/envoy/tcp/mixer/control.cc @@ -14,9 +14,12 @@ */ #include "src/envoy/tcp/mixer/control.h" +#include "include/istio/utils/local_attributes.h" #include "src/envoy/utils/mixer_control.h" +using ::istio::mixer::v1::Attributes; using ::istio::mixerclient::Statistics; +using ::istio::utils::LocalNode; namespace Envoy { namespace Tcp { @@ -25,7 +28,8 @@ namespace Mixer { Control::Control(const Config& config, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, Stats::Scope& scope, - Utils::MixerFilterStats& stats, const std::string& uuid) + Utils::MixerFilterStats& stats, const std::string& uuid, + const LocalInfo::LocalInfo& local_info) : config_(config), dispatcher_(dispatcher), check_client_factory_(Utils::GrpcClientFactoryForCluster( @@ -36,10 +40,16 @@ Control::Control(const Config& config, Upstream::ClusterManager& cm, config_.config_pb().transport().stats_update_interval(), [this](Statistics* stat) -> bool { return GetStats(stat); }), uuid_(uuid) { - Utils::SerializeForwardedAttributes(config_.config_pb().transport(), - &serialized_forward_attributes_); + auto& logger = Logger::Registry::getLog(Logger::Id::config); + LocalNode local_node; + if (!Utils::ExtractNodeInfo(local_info.node(), &local_node)) { + ENVOY_LOG_TO_LOGGER(logger, warn, "Unable to get node metadata"); + } + ::istio::utils::SerializeForwardedAttributes(local_node, + &serialized_forward_attributes_); - ::istio::control::tcp::Controller::Options options(config_.config_pb()); + ::istio::control::tcp::Controller::Options options(config_.config_pb(), + local_node); Utils::CreateEnvironment(dispatcher, random, *check_client_factory_, *report_client_factory_, diff --git a/src/envoy/tcp/mixer/control.h b/src/envoy/tcp/mixer/control.h index 203858619ad..9e08a167f7a 100644 --- a/src/envoy/tcp/mixer/control.h +++ b/src/envoy/tcp/mixer/control.h @@ -15,11 +15,14 @@ #pragma once +#include "common/common/logger.h" #include "envoy/event/dispatcher.h" +#include "envoy/local_info/local_info.h" #include "envoy/runtime/runtime.h" #include "envoy/thread_local/thread_local.h" #include "envoy/upstream/cluster_manager.h" #include "include/istio/control/tcp/controller.h" +#include "include/istio/utils/local_attributes.h" #include "src/envoy/tcp/mixer/config.h" #include "src/envoy/utils/stats.h" @@ -33,7 +36,7 @@ class Control final : public ThreadLocal::ThreadLocalObject { Control(const Config& config, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, Stats::Scope& scope, Utils::MixerFilterStats& stats, - const std::string& uuid); + const std::string& uuid, const LocalInfo::LocalInfo& local_info); ::istio::control::tcp::Controller* controller() { return controller_.get(); } diff --git a/src/envoy/tcp/mixer/control_factory.h b/src/envoy/tcp/mixer/control_factory.h index 763d0e65cfe..a33c08151c6 100644 --- a/src/envoy/tcp/mixer/control_factory.h +++ b/src/envoy/tcp/mixer/control_factory.h @@ -15,6 +15,7 @@ #pragma once +#include "envoy/local_info/local_info.h" #include "src/envoy/tcp/mixer/control.h" namespace Envoy { @@ -38,10 +39,13 @@ class ControlFactory : public Logger::Loggable { uuid_(context.random().uuid()) { Runtime::RandomGenerator& random = context.random(); Stats::Scope& scope = context.scope(); - tls_->set([this, &random, &scope](Event::Dispatcher& dispatcher) + const LocalInfo::LocalInfo& local_info = context.localInfo(); + + tls_->set([this, &random, &scope, + &local_info](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr { - return ThreadLocal::ThreadLocalObjectSharedPtr( - new Control(*config_, cm_, dispatcher, random, scope, stats_, uuid_)); + return ThreadLocal::ThreadLocalObjectSharedPtr(new Control( + *config_, cm_, dispatcher, random, scope, stats_, uuid_, local_info)); }); } diff --git a/src/istio/control/client_context_base.cc b/src/istio/control/client_context_base.cc index e814fd56e00..e489faf18a2 100644 --- a/src/istio/control/client_context_base.cc +++ b/src/istio/control/client_context_base.cc @@ -31,6 +31,8 @@ using ::istio::mixerclient::QuotaOptions; using ::istio::mixerclient::ReportOptions; using ::istio::mixerclient::Statistics; using ::istio::mixerclient::TransportCheckFunc; +using ::istio::utils::CreateLocalAttributes; +using ::istio::utils::LocalNode; namespace istio { namespace control { @@ -69,11 +71,14 @@ ReportOptions GetReportOptions(const TransportConfig& config) { } // namespace ClientContextBase::ClientContextBase(const TransportConfig& config, - const Environment& env) { + const Environment& env, bool outbound, + const LocalNode& local_node) + : outbound_(outbound) { MixerClientOptions options(GetCheckOptions(config), GetReportOptions(config), GetQuotaOptions(config)); options.env = env; mixer_client_ = ::istio::mixerclient::CreateMixerClient(options); + CreateLocalAttributes(local_node, &local_attributes_); } CancelFunc ClientContextBase::SendCheck(TransportCheckFunc transport, @@ -111,5 +116,20 @@ void ClientContextBase::GetStatistics(Statistics* stat) const { mixer_client_->GetStatistics(stat); } +void ClientContextBase::AddLocalNodeAttributes( + ::istio::mixer::v1::Attributes* request) const { + if (outbound_) { + request->MergeFrom(local_attributes_.outbound); + } else { + request->MergeFrom(local_attributes_.inbound); + } +} + +void ClientContextBase::AddLocalNodeForwardAttribues( + ::istio::mixer::v1::Attributes* request) const { + if (outbound_) { + request->MergeFrom(local_attributes_.forward); + } +} } // namespace control } // namespace istio diff --git a/src/istio/control/client_context_base.h b/src/istio/control/client_context_base.h index 54a6a89ace4..560c090083c 100644 --- a/src/istio/control/client_context_base.h +++ b/src/istio/control/client_context_base.h @@ -17,6 +17,8 @@ #define ISTIO_CONTROL_CLIENT_CONTEXT_BASE_H #include "include/istio/mixerclient/client.h" +#include "include/istio/utils/attribute_names.h" +#include "include/istio/utils/local_attributes.h" #include "mixer/v1/config/client/client_config.pb.h" #include "request_context.h" @@ -29,12 +31,16 @@ class ClientContextBase { public: ClientContextBase( const ::istio::mixer::v1::config::client::TransportConfig& config, - const ::istio::mixerclient::Environment& env); + const ::istio::mixerclient::Environment& env, bool outbound, + const ::istio::utils::LocalNode& local_node); // A constructor for unit-test to pass in a mock mixer_client ClientContextBase( - std::unique_ptr<::istio::mixerclient::MixerClient> mixer_client) - : mixer_client_(std::move(mixer_client)) {} + std::unique_ptr<::istio::mixerclient::MixerClient> mixer_client, + bool outbound, ::istio::utils::LocalAttributes& local_attributes) + : mixer_client_(std::move(mixer_client)), + outbound_(outbound), + local_attributes_(local_attributes) {} // virtual destrutor virtual ~ClientContextBase() {} @@ -49,9 +55,20 @@ class ClientContextBase { // Get statistics. void GetStatistics(::istio::mixerclient::Statistics* stat) const; + void AddLocalNodeAttributes(::istio::mixer::v1::Attributes* request) const; + + void AddLocalNodeForwardAttribues( + ::istio::mixer::v1::Attributes* request) const; + private: // The mixer client object with check cache and report batch features. std::unique_ptr<::istio::mixerclient::MixerClient> mixer_client_; + + // If this is an outbound client context. + bool outbound_; + + // local attributes - owned by the client context. + ::istio::utils::LocalAttributes local_attributes_; }; } // namespace control diff --git a/src/istio/control/http/client_context.cc b/src/istio/control/http/client_context.cc index 925ae7313ee..e3acf7fc895 100644 --- a/src/istio/control/http/client_context.cc +++ b/src/istio/control/http/client_context.cc @@ -19,51 +19,27 @@ using ::istio::mixer::v1::Attributes_AttributeValue; using ::istio::mixer::v1::config::client::ServiceConfig; using ::istio::utils::AttributeName; -using ::istio::utils::CreateLocalAttributes; namespace istio { namespace control { namespace http { -const char* kReporterOutbound = "outbound"; - -namespace { - -// isOutbound returns true if this is an outbound listener configuration. -// It relies on pilot setting context.reporter.kind == outbound; -static bool isOutbound( - const ::istio::mixer::v1::config::client::HttpClientConfig& config) { - bool outbound = false; - const auto& attributes_map = config.mixer_attributes().attributes(); - const auto it = attributes_map.find(AttributeName::kContextReporterKind); - if (it != attributes_map.end()) { - const Attributes_AttributeValue& value = it->second; - if (kReporterOutbound == value.string_value()) { - outbound = true; - } - } - return outbound; -} - -} // namespace ClientContext::ClientContext(const Controller::Options& data) - : ClientContextBase(data.config.transport(), data.env), + : ClientContextBase( + data.config.transport(), data.env, + ::istio::utils::IsOutbound(data.config.mixer_attributes()), + data.local_node), config_(data.config), - service_config_cache_size_(data.service_config_cache_size), - outbound_(isOutbound(data.config)) { - CreateLocalAttributes(data.local_node, &local_attributes_); -} + service_config_cache_size_(data.service_config_cache_size) {} ClientContext::ClientContext( std::unique_ptr<::istio::mixerclient::MixerClient> mixer_client, const ::istio::mixer::v1::config::client::HttpClientConfig& config, int service_config_cache_size, ::istio::utils::LocalAttributes& local_attributes, bool outbound) - : ClientContextBase(std::move(mixer_client)), + : ClientContextBase(std::move(mixer_client), outbound, local_attributes), config_(config), - service_config_cache_size_(service_config_cache_size), - local_attributes_(local_attributes), - outbound_(outbound) {} + service_config_cache_size_(service_config_cache_size) {} const std::string& ClientContext::GetServiceName( const std::string& service_name) const { @@ -89,22 +65,6 @@ const ServiceConfig* ClientContext::GetServiceConfig( return nullptr; } -void ClientContext::AddLocalNodeAttributes( - ::istio::mixer::v1::Attributes* request) const { - if (outbound_) { - request->MergeFrom(local_attributes_.outbound); - } else { - request->MergeFrom(local_attributes_.inbound); - } -} - -void ClientContext::AddLocalNodeForwardAttribues( - ::istio::mixer::v1::Attributes* request) const { - if (outbound_) { - request->MergeFrom(local_attributes_.forward); - } -} - } // namespace http } // namespace control } // namespace istio diff --git a/src/istio/control/http/client_context.h b/src/istio/control/http/client_context.h index 0a5ab57c3a6..acb5609f192 100644 --- a/src/istio/control/http/client_context.h +++ b/src/istio/control/http/client_context.h @@ -55,27 +55,12 @@ class ClientContext : public ClientContextBase { // Get the service config cache size int service_config_cache_size() const { return service_config_cache_size_; } - // AddLocalNodeAttributes adds source.* attributes for outbound mixer filter - // and adds destination.* attributes for inbound mixer filter. - void AddLocalNodeAttributes(::istio::mixer::v1::Attributes* request) const; - - // AddLocalNodeForwardAttribues add forward attributes for outbound mixer - // filter. - void AddLocalNodeForwardAttribues( - ::istio::mixer::v1::Attributes* request) const; - private: // The http client config. const ::istio::mixer::v1::config::client::HttpClientConfig& config_; // The service config cache size int service_config_cache_size_; - - // local attributes - owned by the client context. - ::istio::utils::LocalAttributes local_attributes_; - - // if this client context is for an inbound listener or outbound listener. - bool outbound_; }; } // namespace http diff --git a/src/istio/control/tcp/client_context.h b/src/istio/control/tcp/client_context.h index e093b9745ef..86296d3f1d9 100644 --- a/src/istio/control/tcp/client_context.h +++ b/src/istio/control/tcp/client_context.h @@ -18,6 +18,7 @@ #include "include/istio/control/tcp/controller.h" #include "include/istio/quota_config/config_parser.h" +#include "include/istio/utils/local_attributes.h" #include "src/istio/control/client_context_base.h" #include "src/istio/control/request_context.h" @@ -31,7 +32,10 @@ namespace tcp { class ClientContext : public ClientContextBase { public: ClientContext(const Controller::Options& data) - : ClientContextBase(data.config.transport(), data.env), + : ClientContextBase( + data.config.transport(), data.env, + ::istio::utils::IsOutbound(data.config.mixer_attributes()), + data.local_node), config_(data.config) { BuildQuotaParser(); } @@ -39,13 +43,17 @@ class ClientContext : public ClientContextBase { // A constructor for unit-test to pass in a mock mixer_client ClientContext( std::unique_ptr<::istio::mixerclient::MixerClient> mixer_client, - const ::istio::mixer::v1::config::client::TcpClientConfig& config) - : ClientContextBase(std::move(mixer_client)), config_(config) { + const ::istio::mixer::v1::config::client::TcpClientConfig& config, + bool outbound, ::istio::utils::LocalAttributes& local_attributes) + : ClientContextBase(std::move(mixer_client), outbound, local_attributes), + config_(config) { BuildQuotaParser(); } // Add static mixer attributes. void AddStaticAttributes(RequestContext* request) const { + AddLocalNodeAttributes(&request->attributes); + if (config_.has_mixer_attributes()) { request->attributes.MergeFrom(config_.mixer_attributes()); } diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index 259fd06d7ce..eb8b9559c94 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -13,6 +13,7 @@ * limitations under the License. */ +#include "google/protobuf/text_format.h" #include "gtest/gtest.h" #include "src/istio/control/mock_mixer_client.h" #include "src/istio/control/tcp/client_context.h" @@ -20,6 +21,7 @@ #include "src/istio/control/tcp/mock_check_data.h" #include "src/istio/control/tcp/mock_report_data.h" +using ::google::protobuf::TextFormat; using ::google::protobuf::util::Status; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::config::client::TcpClientConfig; @@ -30,6 +32,7 @@ using ::istio::mixerclient::DoneFunc; using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; +using ::istio::utils::LocalAttributes; using ::testing::_; using ::testing::Invoke; @@ -38,6 +41,36 @@ namespace istio { namespace control { namespace tcp { +namespace { +// local inbound +const char kLocalInbound[] = R"( +attributes { + key: "destination.uid" + value { + string_value: "kubernetes://client-84469dc8d7-jbbxt.default" + } +} +)"; + +const char kLocalOutbound[] = R"( +attributes { + key: "source.uid" + value { + string_value: "kubernetes://client-84469dc8d7-jbbxt.default" + } +} +)"; + +const char kLocalForward[] = R"( +attributes { + key: "source.uid" + value { + string_value: "kubernetes://client-84469dc8d7-jbbxt.default" + } +} +)"; +} // namespace + class RequestHandlerImplTest : public ::testing::Test { public: void SetUp() { @@ -50,9 +83,14 @@ class RequestHandlerImplTest : public ::testing::Test { quota->set_quota("quota"); quota->set_charge(5); + LocalAttributes la; + ASSERT_TRUE(TextFormat::ParseFromString(kLocalInbound, &la.inbound)); + ASSERT_TRUE(TextFormat::ParseFromString(kLocalOutbound, &la.outbound)); + ASSERT_TRUE(TextFormat::ParseFromString(kLocalForward, &la.forward)); + mock_client_ = new ::testing::NiceMock; client_context_ = std::make_shared( - std::unique_ptr(mock_client_), client_config_); + std::unique_ptr(mock_client_), client_config_, false, la); controller_ = std::unique_ptr(new ControllerImpl(client_context_)); } diff --git a/src/istio/utils/local_attributes.cc b/src/istio/utils/local_attributes.cc index da1dfac87ce..3b209ea7f19 100644 --- a/src/istio/utils/local_attributes.cc +++ b/src/istio/utils/local_attributes.cc @@ -20,6 +20,10 @@ namespace istio { namespace utils { +namespace { +const char kReporterOutbound[] = "outbound"; +} // namespace + // create Local attributes object and return a pointer to it. // Should be freed by the caller. void CreateLocalAttributes(const LocalNode& local, @@ -49,5 +53,20 @@ bool SerializeForwardedAttributes(const LocalNode& local, return attributes.SerializeToString(serialized_forward_attributes); } +// check if this listener is outbound based on "context.reporter.kind" attribute +bool IsOutbound(const ::istio::mixer::v1::Attributes& attributes) { + bool outbound = false; + const auto& attributes_map = attributes.attributes(); + const auto it = + attributes_map.find(::istio::utils::AttributeName::kContextReporterKind); + if (it != attributes_map.end()) { + const ::istio::mixer::v1::Attributes_AttributeValue& value = it->second; + if (kReporterOutbound == value.string_value()) { + outbound = true; + } + } + return outbound; +} + } // namespace utils } // namespace istio