Skip to content

Commit

Permalink
Integrate with quota parser (envoyproxy#258)
Browse files Browse the repository at this point in the history
* Integrate with quota parser

* Add optimization for service context lookup.
  • Loading branch information
qiwzhang authored Nov 17, 2017
1 parent 6412ed7 commit 190c807
Show file tree
Hide file tree
Showing 21 changed files with 310 additions and 76 deletions.
2 changes: 1 addition & 1 deletion mixerclient/control/include/http/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Controller {
std::string destination_service;

// if not NULL, legacy per-route config for 0.2 and before.
const ::istio::mixer::v1::config::client::ServiceConfig* legacy_config;
const ::istio::mixer::v1::config::client::ServiceConfig* legacy_config{};
};

// Creates a HTTP request handler.
Expand Down
1 change: 1 addition & 0 deletions mixerclient/control/src/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ cc_library(
visibility = [":__subpackages__"],
deps = [
"//:mixer_client_lib",
"//quota:config_parser_lib",
"//external:mixer_client_config_cc_proto",
],
)
Expand Down
9 changes: 4 additions & 5 deletions mixerclient/control/src/client_context_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ ClientContextBase::ClientContextBase(const TransportConfig& config,
mixer_client_ = ::istio::mixer_client::CreateMixerClient(options);
}

CancelFunc ClientContextBase::SendCheck(
TransportCheckFunc transport, DoneFunc on_done,
const std::vector<::istio::quota::Requirement>& quotas,
RequestContext* request) {
CancelFunc ClientContextBase::SendCheck(TransportCheckFunc transport,
DoneFunc on_done,
RequestContext* request) {
// Intercept the callback to save check status in request_context
auto local_on_done = [request, on_done](const Status& status) {
// save the check status code
Expand All @@ -83,7 +82,7 @@ CancelFunc ClientContextBase::SendCheck(
// TODO: add debug message
// GOOGLE_LOG(INFO) << "Check attributes: " <<
// request->attributes.DebugString();
return mixer_client_->Check(request->attributes, quotas, transport,
return mixer_client_->Check(request->attributes, request->quotas, transport,
local_on_done);
}

Expand Down
4 changes: 1 addition & 3 deletions mixerclient/control/src/client_context_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ class ClientContextBase {
// Use mixer client object to make a Check call.
::istio::mixer_client::CancelFunc SendCheck(
::istio::mixer_client::TransportCheckFunc transport,
::istio::mixer_client::DoneFunc on_done,
const std::vector<::istio::quota::Requirement>& quotas,
RequestContext* request);
::istio::mixer_client::DoneFunc on_done, RequestContext* request);

// Use mixer client object to make a Report call.
void SendReport(const RequestContext& request);
Expand Down
4 changes: 3 additions & 1 deletion mixerclient/control/src/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ cc_library(
srcs = [
"attributes_builder.cc",
"attributes_builder.h",
"client_context.cc",
"client_context.h",
"controller_impl.cc",
"controller_impl.h",
"request_handler_impl.cc",
"request_handler_impl.h",
"service_context.h"
"service_context.cc",
"service_context.h",
],
visibility = ["//visibility:public"],
deps = [
Expand Down
67 changes: 67 additions & 0 deletions mixerclient/control/src/http/client_context.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/* Copyright 2017 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 "client_context.h"

using ::istio::mixer::v1::config::client::ServiceConfig;

namespace istio {
namespace mixer_control {
namespace http {

ClientContext::ClientContext(const Controller::Options& data)
: ClientContextBase(data.config.transport(), data.env),
config_(data.config),
legacy_quotas_(data.legacy_quotas) {}

ClientContext::ClientContext(
std::unique_ptr<::istio::mixer_client::MixerClient> mixer_client,
const ::istio::mixer::v1::config::client::HttpClientConfig& config,
const std::vector<::istio::quota::Requirement>& legacy_quotas)
: ClientContextBase(std::move(mixer_client)),
config_(config),
legacy_quotas_(legacy_quotas) {}

void ClientContext::AddLegacyQuotas(
std::vector<::istio::quota::Requirement>* quotas) const {
quotas->insert(quotas->end(), legacy_quotas_.begin(), legacy_quotas_.end());
}

const std::string& ClientContext::GetServiceName(
const std::string& service_name) const {
if (service_name.empty()) {
return config_.default_destination_service();
}
const auto& config_map = config_.service_configs();
auto it = config_map.find(service_name);
if (it == config_map.end()) {
return config_.default_destination_service();
}
return service_name;
}

// Get the service config by the name.
const ServiceConfig& ClientContext::GetServiceConfig(
const std::string& service_name) const {
const auto& config_map = config_.service_configs();
auto it = config_map.find(service_name);
// The name should be a valid one checked by GetServiceName()
GOOGLE_CHECK(it != config_map.end());
return it->second;
}

} // namespace http
} // namespace mixer_control
} // namespace istio
25 changes: 13 additions & 12 deletions mixerclient/control/src/http/client_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,29 @@ namespace http {
// * the mixer client object to call Check/Report with cache.
class ClientContext : public ClientContextBase {
public:
ClientContext(const Controller::Options& data)
: ClientContextBase(data.config.transport(), data.env),
config_(data.config),
legacy_quotas_(data.legacy_quotas) {}

ClientContext(const Controller::Options& data);
// A constructor for unit-test to pass in a mock mixer_client
ClientContext(
std::unique_ptr<::istio::mixer_client::MixerClient> mixer_client,
const ::istio::mixer::v1::config::client::HttpClientConfig& config,
const std::vector<::istio::quota::Requirement>& legacy_quotas)
: ClientContextBase(std::move(mixer_client)),
config_(config),
legacy_quotas_(legacy_quotas) {}
const std::vector<::istio::quota::Requirement>& legacy_quotas);

// Retrieve mixer client config.
const ::istio::mixer::v1::config::client::HttpClientConfig& config() const {
return config_;
}

const std::vector<::istio::quota::Requirement>& legacy_quotas() const {
return legacy_quotas_;
}
// Append the legacy quotas.
void AddLegacyQuotas(std::vector<::istio::quota::Requirement>* quotas) const;

// Get valid service name in the config map.
// If input service name is in the map, use it, otherwise, use the default
// one.
const std::string& GetServiceName(const std::string& service_name) const;

// Get the service config by the name.
const ::istio::mixer::v1::config::client::ServiceConfig& GetServiceConfig(
const std::string& service_name) const;

private:
// The http client config.
Expand Down
21 changes: 14 additions & 7 deletions mixerclient/control/src/http/controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,21 @@ std::shared_ptr<ServiceContext> ControllerImpl::GetServiceContext(
return std::make_shared<ServiceContext>(client_context_,
*config.legacy_config);
}
auto config_map = client_context_->config().service_configs();
auto it = config_map.find(config.destination_service);
if (it == config_map.end()) {
it = config_map.find(
client_context_->config().default_destination_service());

const std::string& origin_name = config.destination_service;
auto service_context = service_context_map_[origin_name];
if (!service_context) {
// Get the valid service name from service_configs map.
auto valid_name = client_context_->GetServiceName(origin_name);
service_context = service_context_map_[valid_name];
if (!service_context) {
service_context = std::make_shared<ServiceContext>(
client_context_, client_context_->GetServiceConfig(valid_name));
service_context_map_[valid_name] = service_context;
}
service_context_map_[origin_name] = service_context;
}
// TODO: cache this service context.
return std::make_shared<ServiceContext>(client_context_, it->second);
return service_context;
}

std::unique_ptr<Controller> Controller::Create(const Options& data) {
Expand Down
5 changes: 5 additions & 0 deletions mixerclient/control/src/http/controller_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#define MIXERCONTROL_HTTP_CONTROLLER_IMPL_H

#include <memory>
#include <unordered_map>

#include "client_context.h"
#include "control/include/http/controller.h"
Expand Down Expand Up @@ -45,6 +46,10 @@ class ControllerImpl : public Controller {

// The client context object to hold client config and client cache.
std::shared_ptr<ClientContext> client_context_;

// The map to cache service context. key is destination.service
std::unordered_map<std::string, std::shared_ptr<ServiceContext>>
service_context_map_;
};

} // namespace http
Expand Down
9 changes: 5 additions & 4 deletions mixerclient/control/src/http/request_handler_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ CancelFunc RequestHandlerImpl::Check(CheckData* check_data,
return nullptr;
}

std::vector<Requirement> quotas(
service_context_->client_context()->legacy_quotas());
return service_context_->client_context()->SendCheck(
transport, on_done, quotas, &request_context_);
service_context_->client_context()->AddLegacyQuotas(&request_context_.quotas);
service_context_->AddQuotas(&request_context_);

return service_context_->client_context()->SendCheck(transport, on_done,
&request_context_);
}

// Make remote report call.
Expand Down
61 changes: 54 additions & 7 deletions mixerclient/control/src/http/request_handler_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ namespace http {
class RequestHandlerImplTest : public ::testing::Test {
public:
void SetUp() {
// add a global mixer attribute
auto map1 = client_config_.mutable_mixer_attributes()->mutable_attributes();
(*map1)["global-key"].set_string_value("global-value");

// add a legacy quota
legacy_quotas_.push_back({"legacy-quota", 10});

// add default route config with:
// * a mixer attribute
// * a route quota
ServiceConfig route_config;
route_config.set_enable_mixer_check(true);
auto map3 = route_config.mutable_mixer_attributes()->mutable_attributes();
(*map3)["route-key"].set_string_value("route-value");
auto quota = route_config.add_quota_spec()->add_rules()->add_quotas();
quota->set_quota("route-quota");
quota->set_charge(20);
client_config_.set_default_destination_service(":default");
(*client_config_.mutable_service_configs())[":default"] = route_config;

mock_client_ = new ::testing::NiceMock<MockMixerClient>;
client_context_ = std::make_shared<ClientContext>(
std::unique_ptr<MixerClient>(mock_client_), client_config_,
Expand Down Expand Up @@ -95,7 +115,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) {
[](const Status& status) { EXPECT_TRUE(status.ok()); });
}

TEST_F(RequestHandlerImplTest, TestHandlerMixerAttributes) {
TEST_F(RequestHandlerImplTest, TestHandlerLegacyRoute) {
::testing::NiceMock<MockCheckData> mock_data;
EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1);
EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1);
Expand All @@ -107,9 +127,11 @@ TEST_F(RequestHandlerImplTest, TestHandlerMixerAttributes) {
TransportCheckFunc transport,
DoneFunc on_done) -> CancelFunc {
auto map = attributes.attributes();
EXPECT_EQ(map["key1"].string_value(), "value1");
EXPECT_EQ(map["key2"].string_value(), "value2");
EXPECT_EQ(quotas.size(), 0);
EXPECT_EQ(map["global-key"].string_value(), "global-value");
EXPECT_EQ(map["legacy-key"].string_value(), "legacy-value");
EXPECT_EQ(quotas.size(), 1);
EXPECT_EQ(quotas[0].quota, "legacy-quota");
EXPECT_EQ(quotas[0].charge, 10);
return nullptr;
}));

Expand All @@ -118,11 +140,36 @@ TEST_F(RequestHandlerImplTest, TestHandlerMixerAttributes) {
Controller::PerRouteConfig config;
config.legacy_config = &legacy;

auto map1 = client_config_.mutable_mixer_attributes()->mutable_attributes();
(*map1)["key1"].set_string_value("value1");
auto map2 = legacy.mutable_mixer_attributes()->mutable_attributes();
(*map2)["key2"].set_string_value("value2");
(*map2)["legacy-key"].set_string_value("legacy-value");

auto handler = controller_->CreateRequestHandler(config);
handler->Check(&mock_data, nullptr, nullptr);
}

TEST_F(RequestHandlerImplTest, TestHandlerDefaultRoute) {
::testing::NiceMock<MockCheckData> mock_data;
EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1);
EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1);

// Check should be called.
EXPECT_CALL(*mock_client_, Check(_, _, _, _))
.WillOnce(Invoke([](const Attributes& attributes,
const std::vector<Requirement>& quotas,
TransportCheckFunc transport,
DoneFunc on_done) -> CancelFunc {
auto map = attributes.attributes();
EXPECT_EQ(map["global-key"].string_value(), "global-value");
EXPECT_EQ(map["route-key"].string_value(), "route-value");
EXPECT_EQ(quotas.size(), 2);
EXPECT_EQ(quotas[0].quota, "legacy-quota");
EXPECT_EQ(quotas[0].charge, 10);
EXPECT_EQ(quotas[1].quota, "route-quota");
EXPECT_EQ(quotas[1].charge, 20);
return nullptr;
}));

Controller::PerRouteConfig config;
auto handler = controller_->CreateRequestHandler(config);
handler->Check(&mock_data, nullptr, nullptr);
}
Expand Down
54 changes: 54 additions & 0 deletions mixerclient/control/src/http/service_context.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* Copyright 2017 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 "service_context.h"

using ::istio::mixer::v1::config::client::ServiceConfig;

namespace istio {
namespace mixer_control {
namespace http {

ServiceContext::ServiceContext(std::shared_ptr<ClientContext> client_context,
const ServiceConfig& config)
: client_context_(client_context), service_config_(config) {
// Merge client config mixer attributes.
service_config_.mutable_mixer_attributes()->MergeFrom(
client_context->config().mixer_attributes());

// Build quota parser
for (const auto& quota : service_config_.quota_spec()) {
quota_parsers_.push_back(
std::move(::istio::quota::ConfigParser::Create(quota)));
}
}

// Add static mixer attributes.
void ServiceContext::AddStaticAttributes(RequestContext* request) const {
if (service_config_.has_mixer_attributes()) {
request->attributes.MergeFrom(service_config_.mixer_attributes());
}
}

// Add quota requirements from quota configs.
void ServiceContext::AddQuotas(RequestContext* request) const {
for (const auto& parser : quota_parsers_) {
parser->GetRequirements(request->attributes, &request->quotas);
}
}

} // namespace http
} // namespace mixer_control
} // namespace istio
Loading

0 comments on commit 190c807

Please sign in to comment.