From 9674201fbd9e425faeeca0276c3b4e9bb34ace97 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 22 Mar 2017 16:47:49 -0700 Subject: [PATCH 1/5] Refactory the mixer config loading. --- src/envoy/mixer/BUILD | 2 + src/envoy/mixer/README.md | 20 +++-- src/envoy/mixer/config.cc | 96 +++++++++++++++++++++ src/envoy/mixer/config.h | 52 +++++++++++ src/envoy/mixer/envoy.conf.template | 12 ++- src/envoy/mixer/http_control.cc | 29 ++----- src/envoy/mixer/http_control.h | 8 +- src/envoy/mixer/http_filter.cc | 36 +++----- src/envoy/mixer/integration_test/envoy.conf | 8 +- src/envoy/mixer/repositories.bzl | 2 +- src/envoy/mixer/utils.cc | 14 --- src/envoy/mixer/utils.h | 3 - 12 files changed, 200 insertions(+), 82 deletions(-) create mode 100644 src/envoy/mixer/config.cc create mode 100644 src/envoy/mixer/config.h diff --git a/src/envoy/mixer/BUILD b/src/envoy/mixer/BUILD index 4d57f801626..aa7c455915d 100644 --- a/src/envoy/mixer/BUILD +++ b/src/envoy/mixer/BUILD @@ -30,6 +30,8 @@ cc_proto_library( cc_library( name = "filter_lib", srcs = [ + "config.cc", + "config.h", "http_control.cc", "http_control.h", "http_filter.cc", diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index 5cfb5f57fcb..fd0469d290a 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -52,9 +52,7 @@ This Proxy will use Envoy and talk to Mixer server. curl http://localhost:7070/echo -d "hello world" ``` -## How to configurate HTTP filters - -### *mixer* filter: +## How to configurate HTTP Mixer filters This filter will intercept all HTTP requests and call Mixer. Here is its config: @@ -72,7 +70,14 @@ This filter will intercept all HTTP requests and call Mixer. Here is its config: "forward_attributes" : { "attribute_name1": "attribute_value1", "attribute_name2": "attribute_value2" - } + }, + "quota_name": "RequestCount", + "quota_amount": "1", + "check_cache_keys": [ + "request.host", + "request.path", + "origin.user" + ] } ``` @@ -80,9 +85,10 @@ Notes: * mixer_server is required * mixer_attributes: these attributes will be send to the mixer * forward_attributes: these attributes will be forwarded to the upstream istio/proxy. -* "quota.name" and "quota.amount" are used for quota call. "quota.amount" is default to 1 if missing. +* quota_name, quota_amount are used for making quota call. quota_amount is default to 1 if missing. +* check_cache_keys is to cache check calls. If missing or empty, check calls are not cached. -By default, mixer filter forwards attributes and does not invoke mixer server. You can customize this behavior per HTTP route by supplying an opaque config: +By default, mixer filter forwards attributes and does not invoke mixer server. You can customize this behavior per HTTP route by supplying an opaque config in the route config: ``` "opaque_config": { @@ -91,4 +97,4 @@ By default, mixer filter forwards attributes and does not invoke mixer server. Y } ``` -This config reverts the behavior by sending requests to mixer server but not forwarding any attributes. +This route opaque config reverts the behavior by sending requests to mixer server but not forwarding any attributes. diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc new file mode 100644 index 00000000000..5675a5a311f --- /dev/null +++ b/src/envoy/mixer/config.cc @@ -0,0 +1,96 @@ +/* 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 "src/envoy/mixer/config.h" + +using ::istio::mixer_client::Attributes; + +namespace Http { +namespace Mixer { +namespace { + +// The Json object name for mixer-server. +const std::string kJsonNameMixerServer("mixer_server"); + +// The Json object name for static attributes. +const std::string kJsonNameMixerAttributes("mixer_attributes"); + +// The Json object name to specify attributes which will be forwarded +// to the upstream istio proxy. +const std::string kJsonNameForwardAttributes("forward_attributes"); + +// The Json object name for quota name and amount. +const std::string kJsonNameQuotaName("quota_name"); +const std::string kJsonNameQuotaAmount("quota_amount"); + +// The Json object name for check cache keys. +const std::string kJsonNameCheckCacheKeys("check_cache_keys"); + +void ReadString(const Json::Object& json, const std::string& name, + std::string* value) { + if (json.hasObject(name)) { + *value = json.getString(name); + } +} + +void ReadStringMap(const Json::Object& json, const std::string& name, + std::map* map) { + if (json.hasObject(name)) { + json.getObject(name)->iterate( + [map](const std::string& key, const Json::Object& obj) -> bool { + (*map)[key] = obj.asString(); + return true; + }); + } +} + +void ReadStringVector(const Json::Object& json, const std::string& name, + std::vector* value) { + if (json.hasObject(name)) { + auto v = json.getStringArray(name); + value->swap(v); + } +} + +} // namespace + +void MixerConfig::Load(const Json::Object& json) { + ReadString(json, kJsonNameMixerServer, &mixer_server); + + ReadStringMap(json, kJsonNameMixerAttributes, &mixer_attributes); + ReadStringMap(json, kJsonNameForwardAttributes, &forward_attributes); + + ReadString(json, kJsonNameQuotaName, "a_name); + ReadString(json, kJsonNameQuotaAmount, "a_amount); + + ReadStringVector(json, kJsonNameCheckCacheKeys, &check_cache_keys); +} + +void MixerConfig::ExtractQuotaAttributes(Attributes* attr) const { + if (!quota_name.empty()) { + attr->attributes[ ::istio::mixer_client::kQuotaName] = + Attributes::StringValue(quota_name); + + int64_t amount = 1; // default amount to 1. + if (!quota_amount.empty()) { + amount = std::stoi(quota_amount); + } + attr->attributes[ ::istio::mixer_client::kQuotaAmount] = + Attributes::Int64Value(amount); + } +} + +} // namespace Mixer +} // namespace Http diff --git a/src/envoy/mixer/config.h b/src/envoy/mixer/config.h new file mode 100644 index 00000000000..8f687bd6bcf --- /dev/null +++ b/src/envoy/mixer/config.h @@ -0,0 +1,52 @@ +/* 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. + */ + +#pragma once + +#include "precompiled/precompiled.h" +#include "envoy/json/json_object.h" +#include "include/attribute.h" + +namespace Http { +namespace Mixer { + +// A config for mixer filter +struct MixerConfig { + // the mixer server address + std::string mixer_server; + + // These static attributes will be send to mixer in both + // Check and Report. + std::map mixer_attributes; + + // These attributes will be forwarded to upstream. + std::map forward_attributes; + + // Quota attributes. + std::string quota_name; + std::string quota_amount; + + // The attribute names for check cache. + std::vector check_cache_keys; + + // Load the config from envoy config. + void Load(const Json::Object& json); + + // Extract quota attributes. + void ExtractQuotaAttributes(::istio::mixer_client::Attributes* attr) const; +}; + +} // namespace Mixer +} // namespace Http diff --git a/src/envoy/mixer/envoy.conf.template b/src/envoy/mixer/envoy.conf.template index 6e20da737ba..c7574734ff3 100644 --- a/src/envoy/mixer/envoy.conf.template +++ b/src/envoy/mixer/envoy.conf.template @@ -42,9 +42,15 @@ "mixer_server": "${MIXER_SERVER}", "mixer_attributes": { "target.uid": "POD222", - "target.namespace": "XYZ222", - "quota.name": "RequestCount" - } + "target.namespace": "XYZ222" + }, + "quota_name": "RequestCount", + "quota_amount": "1", + "check_cache_keys": [ + "request.host", + "request.path", + "origin.user" + ] } }, { diff --git a/src/envoy/mixer/http_control.cc b/src/envoy/mixer/http_control.cc index 0e015b58ba6..b44cf707172 100644 --- a/src/envoy/mixer/http_control.cc +++ b/src/envoy/mixer/http_control.cc @@ -108,29 +108,16 @@ void FillRequestInfoAttributes(const AccessLog::RequestInfo& info, } // namespace -HttpControl::HttpControl(const std::string& mixer_server, - std::map&& attributes) - : config_attributes_(std::move(attributes)) { +HttpControl::HttpControl(const MixerConfig& mixer_config) + : mixer_config_(mixer_config) { ::istio::mixer_client::MixerClientOptions options; - options.mixer_server = mixer_server; + options.mixer_server = mixer_config_.mixer_server; + options.check_options.cache_keys.insert( + mixer_config_.check_cache_keys.begin(), + mixer_config_.check_cache_keys.end()); mixer_client_ = ::istio::mixer_client::CreateMixerClient(options); - // Extract quota attributes - auto it = config_attributes_.find(::istio::mixer_client::kQuotaName); - if (it != config_attributes_.end()) { - quota_attributes_.attributes[ ::istio::mixer_client::kQuotaName] = - Attributes::StringValue(it->second); - config_attributes_.erase(it); - - int64_t amount = 1; // default amount to 1. - it = config_attributes_.find(::istio::mixer_client::kQuotaAmount); - if (it != config_attributes_.end()) { - amount = std::stoi(it->second); - config_attributes_.erase(it); - } - quota_attributes_.attributes[ ::istio::mixer_client::kQuotaAmount] = - Attributes::Int64Value(amount); - } + mixer_config_.ExtractQuotaAttributes("a_attributes_); } void HttpControl::FillCheckAttributes(HeaderMap& header_map, Attributes* attr) { @@ -148,7 +135,7 @@ void HttpControl::FillCheckAttributes(HeaderMap& header_map, Attributes* attr) { FillRequestHeaderAttributes(header_map, attr); - for (const auto& attribute : config_attributes_) { + for (const auto& attribute : mixer_config_.mixer_attributes) { SetStringAttribute(attribute.first, attribute.second, attr); } } diff --git a/src/envoy/mixer/http_control.h b/src/envoy/mixer/http_control.h index 9405945d714..c473facef3c 100644 --- a/src/envoy/mixer/http_control.h +++ b/src/envoy/mixer/http_control.h @@ -21,6 +21,7 @@ #include "common/http/headers.h" #include "envoy/http/access_log.h" #include "include/client.h" +#include "src/envoy/mixer/config.h" namespace Http { namespace Mixer { @@ -37,8 +38,7 @@ typedef std::shared_ptr HttpRequestDataPtr; class HttpControl final : public Logger::Loggable { public: // The constructor. - HttpControl(const std::string& mixer_server, - std::map&& attributes); + HttpControl(const MixerConfig& mixer_config); // Make mixer check call. void Check(HttpRequestDataPtr request_data, HeaderMap& headers, @@ -56,8 +56,8 @@ class HttpControl final : public Logger::Loggable { // The mixer client std::unique_ptr<::istio::mixer_client::MixerClient> mixer_client_; - // The attributes read from the config file. - std::map config_attributes_; + // The mixer config + const MixerConfig& mixer_config_; // Quota attributes; extracted from envoy filter config. ::istio::mixer_client::Attributes quota_attributes_; }; diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index 8708333de40..c46df8adf4b 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -22,6 +22,7 @@ #include "envoy/server/instance.h" #include "envoy/ssl/connection.h" #include "server/config/network/http_connection_manager.h" +#include "src/envoy/mixer/config.h" #include "src/envoy/mixer/http_control.h" #include "src/envoy/mixer/utils.h" @@ -33,16 +34,6 @@ namespace Http { namespace Mixer { namespace { -// The Json object name for mixer-server. -const std::string kJsonNameMixerServer("mixer_server"); - -// The Json object name for static attributes. -const std::string kJsonNameMixerAttributes("mixer_attributes"); - -// The Json object name to specify attributes which will be forwarded -// to the upstream istio proxy. -const std::string kJsonNameForwardAttributes("forward_attributes"); - // Switch to turn off attribute forwarding const std::string kJsonNameForwardSwitch("mixer_forward"); @@ -100,35 +91,30 @@ class Config : public Logger::Loggable { std::shared_ptr http_control_; Upstream::ClusterManager& cm_; std::string forward_attributes_; + MixerConfig mixer_config_; public: Config(const Json::Object& config, Server::Instance& server) : cm_(server.clusterManager()) { - std::string mixer_server; - if (config.hasObject(kJsonNameMixerServer)) { - mixer_server = config.getString(kJsonNameMixerServer); - } else { + mixer_config_.Load(config); + if (mixer_config_.mixer_server.empty()) { log().error( "mixer_server is required but not specified in the config: {}", __func__); + } else { + log().debug("Called Mixer::Config constructor with mixer_server: ", + mixer_config_.mixer_server); } - Utils::StringMap attributes = - Utils::ExtractStringMap(config, kJsonNameForwardAttributes); - if (!attributes.empty()) { - std::string serialized_str = Utils::SerializeStringMap(attributes); + if (!mixer_config_.forward_attributes.empty()) { + std::string serialized_str = + Utils::SerializeStringMap(mixer_config_.forward_attributes); forward_attributes_ = Base64::encode(serialized_str.c_str(), serialized_str.size()); log().debug("Mixer forward attributes set: ", serialized_str); } - std::map mixer_attributes = - Utils::ExtractStringMap(config, kJsonNameMixerAttributes); - - http_control_ = std::make_shared(mixer_server, - std::move(mixer_attributes)); - log().debug("Called Mixer::Config constructor with mixer_server: ", - mixer_server); + http_control_ = std::make_shared(mixer_config_); } std::shared_ptr& http_control() { return http_control_; } diff --git a/src/envoy/mixer/integration_test/envoy.conf b/src/envoy/mixer/integration_test/envoy.conf index 4374ec1b4ea..d4c68db1c38 100644 --- a/src/envoy/mixer/integration_test/envoy.conf +++ b/src/envoy/mixer/integration_test/envoy.conf @@ -42,10 +42,10 @@ "mixer_server": "localhost:29091", "mixer_attributes": { "target.uid": "POD222", - "target.namespace": "XYZ222", - "quota.name": "RequestCount", - "quota.amount": "5" - } + "target.namespace": "XYZ222" + }, + "quota_name": "RequestCount", + "quota_amount": "5" } }, { diff --git a/src/envoy/mixer/repositories.bzl b/src/envoy/mixer/repositories.bzl index 7edd7e89834..ffa2a20409f 100644 --- a/src/envoy/mixer/repositories.bzl +++ b/src/envoy/mixer/repositories.bzl @@ -15,7 +15,7 @@ ################################################################################ # -MIXER_CLIENT = "b76b5131c4650cefff4af7e4267883a33d66bca1" +MIXER_CLIENT = "0e6f858bc7b52dc8f143f46ac32afc79d504e8a4" def mixer_client_repositories(bind=True): native.git_repository( diff --git a/src/envoy/mixer/utils.cc b/src/envoy/mixer/utils.cc index 6dc3a78356d..32b09667208 100644 --- a/src/envoy/mixer/utils.cc +++ b/src/envoy/mixer/utils.cc @@ -21,20 +21,6 @@ namespace Utils { const LowerCaseString kIstioAttributeHeader("x-istio-attributes"); -StringMap ExtractStringMap(const Json::Object& json, const std::string& name) { - StringMap map; - if (json.hasObject(name)) { - Json::ObjectPtr json_obj = json.getObject(name); - Json::Object* raw_obj = json_obj.get(); - json_obj->iterate( - [&map, raw_obj](const std::string& key, const Json::Object&) -> bool { - map[key] = raw_obj->getString(key); - return true; - }); - } - return map; -} - std::string SerializeStringMap(const StringMap& string_map) { ::istio::proxy::mixer::StringMap pb; ::google::protobuf::Map* map_pb = pb.mutable_map(); diff --git a/src/envoy/mixer/utils.h b/src/envoy/mixer/utils.h index 9273a9a2d8b..e94ed91706c 100644 --- a/src/envoy/mixer/utils.h +++ b/src/envoy/mixer/utils.h @@ -29,9 +29,6 @@ extern const LowerCaseString kIstioAttributeHeader; // The string map. typedef std::map StringMap; -// Extracts name/value attributes from a json object. -StringMap ExtractStringMap(const Json::Object& json, const std::string& name); - // Serialize a string map to string. std::string SerializeStringMap(const StringMap& map); From 67abd31e98c182b7fdcf8b99278834e73db8beb6 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 22 Mar 2017 17:07:39 -0700 Subject: [PATCH 2/5] fix format --- src/envoy/mixer/config.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/envoy/mixer/config.h b/src/envoy/mixer/config.h index 8f687bd6bcf..ee301aba2b5 100644 --- a/src/envoy/mixer/config.h +++ b/src/envoy/mixer/config.h @@ -16,6 +16,7 @@ #pragma once #include "precompiled/precompiled.h" + #include "envoy/json/json_object.h" #include "include/attribute.h" From 136b5a43b053571d14ba3d808a93519f6dd83b91 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 22 Mar 2017 18:05:40 -0700 Subject: [PATCH 3/5] Add integration test. --- src/envoy/mixer/integration_test/envoy.conf | 6 +- .../mixer/integration_test/mixer_server.go | 3 + .../mixer/integration_test/mixer_test.go | 101 +++++++----------- 3 files changed, 46 insertions(+), 64 deletions(-) diff --git a/src/envoy/mixer/integration_test/envoy.conf b/src/envoy/mixer/integration_test/envoy.conf index d4c68db1c38..1e9bd153893 100644 --- a/src/envoy/mixer/integration_test/envoy.conf +++ b/src/envoy/mixer/integration_test/envoy.conf @@ -45,7 +45,11 @@ "target.namespace": "XYZ222" }, "quota_name": "RequestCount", - "quota_amount": "5" + "quota_amount": "5", + "check_cache_keys": [ + "request.host", + "request.path" + ] } }, { diff --git a/src/envoy/mixer/integration_test/mixer_server.go b/src/envoy/mixer/integration_test/mixer_server.go index fbe5a5bf9c5..3e851274819 100644 --- a/src/envoy/mixer/integration_test/mixer_server.go +++ b/src/envoy/mixer/integration_test/mixer_server.go @@ -33,6 +33,7 @@ import ( type Handler struct { bag *attribute.MutableBag ch chan int + count int r_status rpc.Status } @@ -40,6 +41,7 @@ func newHandler() *Handler { return &Handler{ bag: nil, ch: make(chan int, 1), + count: 0, r_status: rpc.Status{}, } } @@ -47,6 +49,7 @@ func newHandler() *Handler { func (h *Handler) run(bag *attribute.MutableBag) rpc.Status { h.bag = attribute.CopyBag(bag) h.ch <- 1 + h.count++ return h.r_status } diff --git a/src/envoy/mixer/integration_test/mixer_test.go b/src/envoy/mixer/integration_test/mixer_test.go index 862391f873a..99cef6501b3 100644 --- a/src/envoy/mixer/integration_test/mixer_test.go +++ b/src/envoy/mixer/integration_test/mixer_test.go @@ -94,27 +94,6 @@ const reportAttributesOkGet = ` } ` -// Check attributes from a good POST request -const checkAttributesOkPost = ` -{ - "request.host": "localhost:27070", - "request.path": "/echo", - "request.time": "*", - "source.uid": "POD11", - "source.namespace": "XYZ11", - "target.uid": "POD222", - "target.namespace": "XYZ222", - "request.headers": { - ":method": "POST", - ":path": "/echo", - ":authority": "localhost:27070", - "x-forwarded-proto": "http", - "x-istio-attributes": "-", - "x-request-id": "*" - } -} -` - // Report attributes from a good POST request const reportAttributesOkPost = ` { @@ -152,7 +131,7 @@ const reportAttributesOkPost = ` const checkAttributesMixerFail = ` { "request.host": "localhost:27070", - "request.path": "/echo", + "request.path": "/echo111", "request.time": "*", "source.uid": "POD11", "source.namespace": "XYZ11", @@ -160,7 +139,7 @@ const checkAttributesMixerFail = ` "target.namespace": "XYZ222", "request.headers": { ":method": "GET", - ":path": "/echo", + ":path": "/echo111", ":authority": "localhost:27070", "x-forwarded-proto": "http", "x-istio-attributes": "-", @@ -173,7 +152,7 @@ const checkAttributesMixerFail = ` const reportAttributesMixerFail = ` { "request.host": "localhost:27070", - "request.path": "/echo", + "request.path": "/echo111", "request.time": "*", "source.uid": "POD11", "source.namespace": "XYZ11", @@ -181,7 +160,7 @@ const reportAttributesMixerFail = ` "target.namespace": "XYZ222", "request.headers": { ":method": "GET", - ":path": "/echo", + ":path": "/echo111", ":authority": "localhost:27070", "x-forwarded-proto": "http", "x-istio-attributes": "-", @@ -202,27 +181,6 @@ const reportAttributesMixerFail = ` } ` -// Check attributes from a fail GET request from backend -const checkAttributesBackendFail = ` -{ - "request.host": "localhost:27070", - "request.path": "/echo", - "request.time": "*", - "source.uid": "POD11", - "source.namespace": "XYZ11", - "target.uid": "POD222", - "target.namespace": "XYZ222", - "request.headers": { - ":method": "GET", - ":path": "/echo", - ":authority": "localhost:27070", - "x-forwarded-proto": "http", - "x-istio-attributes": "-", - "x-request-id": "*" - } -} -` - // Report attributes from a fail GET request from backend const reportAttributesBackendFail = ` { @@ -256,14 +214,22 @@ const reportAttributesBackendFail = ` } ` -func verifyAttributes( - s *TestSetup, tag string, check string, report string, t *testing.T) { +func verifyCheckCount(s *TestSetup, tag string, count int, t *testing.T) { + if s.mixer.check.count != count { + t.Fatalf("%s check count doesn't match: %v\n, expected: %+v", + tag, s.mixer.check.count, count) + } +} + +func verifyCheckAttributes(s *TestSetup, tag string, check string, t *testing.T) { _ = <-s.mixer.check.ch if err := Verify(s.mixer.check.bag, check); err != nil { t.Fatalf("Failed to verify %s check: %v\n, Attributes: %+v", tag, err, s.mixer.check.bag) } +} +func verifyReportAttributes(s *TestSetup, tag string, report string, t *testing.T) { _ = <-s.mixer.report.ch if err := Verify(s.mixer.report.bag, report); err != nil { t.Fatalf("Failed to verify %s report: %v\n, Attributes: %+v", @@ -297,14 +263,16 @@ func TestMixer(t *testing.T) { url := fmt.Sprintf("http://localhost:%d/echo", ClientProxyPort) // Issues a GET echo request with 0 size body + tag := "OKGet" if _, _, err := HTTPGet(url); err != nil { - t.Errorf("Failed in GET request: %v", err) + t.Errorf("Failed in request %s: %v", tag, err) } - verifyAttributes(&s, "OkGet", - checkAttributesOkGet, reportAttributesOkGet, t) - verifyQuota(&s, "OkGet", t) + verifyCheckAttributes(&s, tag, checkAttributesOkGet, t) + verifyReportAttributes(&s, tag, reportAttributesOkGet, t) + verifyQuota(&s, tag, t) // Issues a failed POST request caused by Mixer Quota + tag = "QuotaFail" s.mixer.quota.r_status = rpc.Status{ Code: int32(rpc.RESOURCE_EXHAUSTED), Message: mixerQuotaFailMessage, @@ -313,7 +281,7 @@ func TestMixer(t *testing.T) { // Make sure to restore r_status for next request. s.mixer.quota.r_status = rpc.Status{} if err != nil { - t.Errorf("Failed in POST request: %v", err) + t.Errorf("Failed in request %s: %v", tag, err) } if code != 429 { t.Errorf("Status code 429 is expected.") @@ -321,11 +289,15 @@ func TestMixer(t *testing.T) { if resp_body != "RESOURCE_EXHAUSTED:"+mixerQuotaFailMessage { t.Errorf("Error response body is not expected.") } - verifyAttributes(&s, "OkPost", - checkAttributesOkPost, reportAttributesOkPost, t) - verifyQuota(&s, "OkPost", t) + // Use cached check. so server check count should remain 1. + verifyCheckCount(&s, tag, 1, t) + verifyReportAttributes(&s, tag, reportAttributesOkPost, t) + verifyQuota(&s, tag, t) // Issues a failed request caused by mixer + // Use a different path to avoid check cache + url = fmt.Sprintf("http://localhost:%d/echo111", ClientProxyPort) + tag = "MixerFail" s.mixer.check.r_status = rpc.Status{ Code: int32(rpc.UNAUTHENTICATED), Message: mixerAuthFailMessage, @@ -334,7 +306,7 @@ func TestMixer(t *testing.T) { // Make sure to restore r_status for next request. s.mixer.check.r_status = rpc.Status{} if err != nil { - t.Errorf("Failed in GET request: error: %v", err) + t.Errorf("Failed in request %s: %v", tag, err) } if code != 401 { t.Errorf("Status code 401 is expected.") @@ -342,16 +314,19 @@ func TestMixer(t *testing.T) { if resp_body != "UNAUTHENTICATED:"+mixerAuthFailMessage { t.Errorf("Error response body is not expected.") } - verifyAttributes(&s, "MixerFail", - checkAttributesMixerFail, reportAttributesMixerFail, t) + verifyCheckAttributes(&s, tag, checkAttributesMixerFail, t) + verifyReportAttributes(&s, tag, reportAttributesMixerFail, t) // Not quota call due to Mixer failure. // Issues a failed request caused by backend + // Use the first path to use check cache + url = fmt.Sprintf("http://localhost:%d/echo", ClientProxyPort) + tag = "BackendFail" headers := map[string]string{} headers[FailHeader] = "Yes" code, resp_body, err = HTTPGetWithHeaders(url, headers) if err != nil { - t.Errorf("Failed in GET request: error: %v", err) + t.Errorf("Failed in request %s: %v", tag, err) } if code != 400 { t.Errorf("Status code 400 is expected.") @@ -359,7 +334,7 @@ func TestMixer(t *testing.T) { if resp_body != FailBody { t.Errorf("Error response body is not expected.") } - verifyAttributes(&s, "BackendFail", - checkAttributesBackendFail, reportAttributesBackendFail, t) - verifyQuota(&s, "BackendFail", t) + verifyCheckCount(&s, tag, 2, t) + verifyReportAttributes(&s, tag, reportAttributesBackendFail, t) + verifyQuota(&s, tag, t) } From a15c82c609b81f307b2890db9cea083a354df7b6 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 23 Mar 2017 11:30:17 -0700 Subject: [PATCH 4/5] updated README.md --- src/envoy/mixer/README.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index fd0469d290a..b5c803125d2 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -83,8 +83,8 @@ This filter will intercept all HTTP requests and call Mixer. Here is its config: Notes: * mixer_server is required -* mixer_attributes: these attributes will be send to the mixer -* forward_attributes: these attributes will be forwarded to the upstream istio/proxy. +* mixer_attributes: these attributes will be send to the mixer in both Check and Report calls. +* forward_attributes: these attributes will be forwarded to the upstream istio/proxy. It will send them to mixer in Check and Report calls. * quota_name, quota_amount are used for making quota call. quota_amount is default to 1 if missing. * check_cache_keys is to cache check calls. If missing or empty, check calls are not cached. @@ -98,3 +98,16 @@ By default, mixer filter forwards attributes and does not invoke mixer server. Y ``` This route opaque config reverts the behavior by sending requests to mixer server but not forwarding any attributes. + + +## How to enable quota (rate limiting) + +Quota (rate limiting) is enforced by the mixer. Mixer needs to be configured with Quota in its global config and service config. Its quota config will have +"quota name", its limit within a window. If "Quota" is added but param is missing, the default config is: quota name is "RequestCount", the limit is 10 with 1 second window. Essentially, it is imposing 10 qps rate limiting. + +Mixer client can be configured to make Quota call for all requests. If "quota_name" is specified in the mixer filter config, mixer client will call Quota with the specified quota name. If "quota_amount" is specified, it will call with that amount, otherwise the used amount is 1. + + +## How to pass some attributes from client proxy to mixer. + +Usually client proxy is not configured to call mixer (it can be enabled in the route opaque_config). Client proxy can pass some attributes to mixer by using "forward_attributes" field. Its attributes will be send to the upstream proxy (the server proxy). If the server proxy is calling mixer, these attributes will be send to the mixer. From a17ebd13ee3cefed53dc77d58aabce76b729ef84 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 23 Mar 2017 11:51:00 -0700 Subject: [PATCH 5/5] s/send/sent/ --- src/envoy/mixer/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index b5c803125d2..493b2d185cf 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -83,7 +83,7 @@ This filter will intercept all HTTP requests and call Mixer. Here is its config: Notes: * mixer_server is required -* mixer_attributes: these attributes will be send to the mixer in both Check and Report calls. +* mixer_attributes: these attributes will be sent to the mixer in both Check and Report calls. * forward_attributes: these attributes will be forwarded to the upstream istio/proxy. It will send them to mixer in Check and Report calls. * quota_name, quota_amount are used for making quota call. quota_amount is default to 1 if missing. * check_cache_keys is to cache check calls. If missing or empty, check calls are not cached. @@ -110,4 +110,4 @@ Mixer client can be configured to make Quota call for all requests. If "quota_n ## How to pass some attributes from client proxy to mixer. -Usually client proxy is not configured to call mixer (it can be enabled in the route opaque_config). Client proxy can pass some attributes to mixer by using "forward_attributes" field. Its attributes will be send to the upstream proxy (the server proxy). If the server proxy is calling mixer, these attributes will be send to the mixer. +Usually client proxy is not configured to call mixer (it can be enabled in the route opaque_config). Client proxy can pass some attributes to mixer by using "forward_attributes" field. Its attributes will be sent to the upstream proxy (the server proxy). If the server proxy is calling mixer, these attributes will be sent to the mixer.