Skip to content

Commit

Permalink
Integrate with api_spec parser. (envoyproxy#260)
Browse files Browse the repository at this point in the history
  • Loading branch information
@qiwzhang
qiwzhang authored Nov 20, 2017
1 parent 190c807 commit 1d9ab8a
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 21 deletions.
1 change: 1 addition & 0 deletions mixerclient/control/src/http/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ cc_library(
],
visibility = ["//visibility:public"],
deps = [
"//api_spec:api_spec_lib",
"//control/include/http:headers_only",
"//control/src:common_lib",
"//control/src/utils:utils_lib",
Expand Down
2 changes: 2 additions & 0 deletions mixerclient/control/src/http/request_handler_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ CancelFunc RequestHandlerImpl::Check(CheckData* check_data,
AttributesBuilder builder(&request_context_);
builder.ExtractForwardedAttributes(check_data);
builder.ExtractCheckAttributes(check_data);

service_context_->AddApiAttributes(check_data, &request_context_);
}

if (service_context_->client_context()->config().has_forward_attributes()) {
Expand Down
166 changes: 145 additions & 21 deletions mixerclient/control/src/http/request_handler_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
#include "client_context.h"
#include "control/src/mock_mixer_client.h"
#include "controller_impl.h"
#include "google/protobuf/text_format.h"
#include "gtest/gtest.h"
#include "mock_check_data.h"
#include "mock_report_data.h"

using ::google::protobuf::TextFormat;
using ::google::protobuf::util::Status;
using ::istio::mixer::v1::Attributes;
using ::istio::mixer::v1::config::client::ServiceConfig;
Expand All @@ -37,28 +39,41 @@ namespace istio {
namespace mixer_control {
namespace http {

// The default client config
const char kDefaultClientConfig[] = R"(
service_configs {
key: ":default"
value {
enable_mixer_check: true
mixer_attributes {
attributes {
key: "route0-key"
value {
string_value: "route0-value"
}
}
}
}
}
default_destination_service: ":default"
mixer_attributes {
attributes {
key: "global-key"
value {
string_value: "global-value"
}
}
}
)";

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;
ASSERT_TRUE(
TextFormat::ParseFromString(kDefaultClientConfig, &client_config_));

mock_client_ = new ::testing::NiceMock<MockMixerClient>;
client_context_ = std::make_shared<ClientContext>(
Expand All @@ -68,6 +83,10 @@ class RequestHandlerImplTest : public ::testing::Test {
std::unique_ptr<Controller>(new ControllerImpl(client_context_));
}

void SetServiceConfig(const std::string& name, const ServiceConfig& config) {
(*client_config_.mutable_service_configs())[name] = config;
}

std::shared_ptr<ClientContext> client_context_;
HttpClientConfig client_config_;
std::vector<Requirement> legacy_quotas_;
Expand Down Expand Up @@ -115,7 +134,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) {
[](const Status& status) { EXPECT_TRUE(status.ok()); });
}

TEST_F(RequestHandlerImplTest, TestHandlerLegacyRoute) {
TEST_F(RequestHandlerImplTest, TestLegacyRoute) {
::testing::NiceMock<MockCheckData> mock_data;
EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1);
EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1);
Expand Down Expand Up @@ -147,7 +166,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerLegacyRoute) {
handler->Check(&mock_data, nullptr, nullptr);
}

TEST_F(RequestHandlerImplTest, TestHandlerDefaultRoute) {
TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) {
::testing::NiceMock<MockCheckData> mock_data;
EXPECT_CALL(mock_data, GetSourceIpPort(_, _)).Times(1);
EXPECT_CALL(mock_data, GetSourceUser(_)).Times(1);
Expand All @@ -160,15 +179,120 @@ TEST_F(RequestHandlerImplTest, TestHandlerDefaultRoute) {
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(map["route0-key"].string_value(), "route0-value");
return nullptr;
}));

// destionation.server is empty, will use default one
Controller::PerRouteConfig config;
auto handler = controller_->CreateRequestHandler(config);
handler->Check(&mock_data, nullptr, nullptr);
}

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

ServiceConfig route_config;
route_config.set_enable_mixer_check(true);
auto map3 = route_config.mutable_mixer_attributes()->mutable_attributes();
(*map3)["route1-key"].set_string_value("route1-value");
SetServiceConfig("route1", route_config);

// 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["route1-key"].string_value(), "route1-value");
return nullptr;
}));

// destionation.server is empty, will use default one
Controller::PerRouteConfig config;
config.destination_service = "route1";
auto handler = controller_->CreateRequestHandler(config);
handler->Check(&mock_data, nullptr, nullptr);
}

TEST_F(RequestHandlerImplTest, TestDefaultRouteQuota) {
::testing::NiceMock<MockCheckData> mock_data;

ServiceConfig route_config;
route_config.set_enable_mixer_check(true);
auto quota = route_config.add_quota_spec()->add_rules()->add_quotas();
quota->set_quota("route0-quota");
quota->set_charge(10);
SetServiceConfig(":default", route_config);

// 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(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);
EXPECT_EQ(quotas[1].quota, "route0-quota");
EXPECT_EQ(quotas[1].charge, 10);
return nullptr;
}));

// destionation.server is empty, will use default one
Controller::PerRouteConfig config;
auto handler = controller_->CreateRequestHandler(config);
handler->Check(&mock_data, nullptr, nullptr);
}

TEST_F(RequestHandlerImplTest, TestDefaultRouteApiSpec) {
::testing::NiceMock<MockCheckData> mock_data;
EXPECT_CALL(mock_data, FindRequestHeader(_, _))
.WillRepeatedly(
Invoke([](CheckData::HeaderType type, std::string* value) -> bool {
if (type == CheckData::HEADER_PATH) {
*value = "/books/120";
return true;
}
if (type == CheckData::HEADER_METHOD) {
*value = "GET";
return true;
}
return false;
}));

ServiceConfig route_config;
route_config.set_enable_mixer_check(true);
auto api_spec = route_config.add_http_api_spec();
auto map1 = api_spec->mutable_attributes()->mutable_attributes();
(*map1)["api.name"].set_string_value("test-name");
auto pattern = api_spec->add_patterns();
auto map2 = pattern->mutable_attributes()->mutable_attributes();
(*map2)["api.operation"].set_string_value("test-method");
pattern->set_http_method("GET");
pattern->set_uri_template("/books/*");
SetServiceConfig(":default", route_config);

// 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["api.name"].string_value(), "test-name");
EXPECT_EQ(map["api.operation"].string_value(), "test-method");
return nullptr;
}));

// destionation.server is empty, will use default one
Controller::PerRouteConfig config;
auto handler = controller_->CreateRequestHandler(config);
handler->Check(&mock_data, nullptr, nullptr);
Expand Down
21 changes: 21 additions & 0 deletions mixerclient/control/src/http/service_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ ServiceContext::ServiceContext(std::shared_ptr<ClientContext> client_context,
service_config_.mutable_mixer_attributes()->MergeFrom(
client_context->config().mixer_attributes());

// Build api_spec parsers
for (const auto& api_spec : service_config_.http_api_spec()) {
api_spec_parsers_.push_back(
std::move(::istio::api_spec::HttpApiSpecParser::Create(api_spec)));
}

// Build quota parser
for (const auto& quota : service_config_.quota_spec()) {
quota_parsers_.push_back(
Expand All @@ -42,6 +48,21 @@ void ServiceContext::AddStaticAttributes(RequestContext* request) const {
}
}

void ServiceContext::AddApiAttributes(CheckData* check_data,
RequestContext* request) const {
if (api_spec_parsers_.size() == 0) {
return;
}
std::string http_method;
std::string path;
if (check_data->FindRequestHeader(CheckData::HEADER_METHOD, &http_method) &&
check_data->FindRequestHeader(CheckData::HEADER_PATH, &path)) {
for (const auto& parser : api_spec_parsers_) {
parser->AddAttributes(http_method, path, &request->attributes);
}
}
}

// Add quota requirements from quota configs.
void ServiceContext::AddQuotas(RequestContext* request) const {
for (const auto& parser : quota_parsers_) {
Expand Down
8 changes: 8 additions & 0 deletions mixerclient/control/src/http/service_context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#ifndef MIXERCONTROL_HTTP_SERVICE_CONTEXT_H
#define MIXERCONTROL_HTTP_SERVICE_CONTEXT_H

#include "api_spec/include/http_api_spec_parser.h"
#include "client_context.h"
#include "google/protobuf/stubs/status.h"
#include "mixer/v1/attributes.pb.h"
Expand All @@ -39,6 +40,9 @@ class ServiceContext {
// Add static mixer attributes.
void AddStaticAttributes(RequestContext* request) const;

// Add api attributes from api_spec.
void AddApiAttributes(CheckData* check_data, RequestContext* request) const;

// Add quota requirements from quota configs.
void AddQuotas(RequestContext* request) const;

Expand All @@ -53,6 +57,10 @@ class ServiceContext {
// The client context object.
std::shared_ptr<ClientContext> client_context_;

// Api spec parsers to generate api attributes.
std::vector<std::unique_ptr<::istio::api_spec::HttpApiSpecParser>>
api_spec_parsers_;

// The quota parsers for each quota config.
std::vector<std::unique_ptr<::istio::quota::ConfigParser>> quota_parsers_;

Expand Down

0 comments on commit 1d9ab8a

Please sign in to comment.