From a5567b9c2a9f1610ef067b98bfa068cb23c59b7c Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 7 Mar 2017 14:42:58 -0800 Subject: [PATCH 01/23] Not call report if decodeHeaders is not called. (#150) --- src/envoy/mixer/http_filter.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index c59f5f43eea..6a8da6a6f45 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -242,6 +242,8 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { const HeaderMap* response_headers, const AccessLog::RequestInfo& request_info) override { Log().debug("Called Mixer::Instance : {}", __func__); + // If decodeHaeders() is not called, not to call Mixer report. + if (!request_data_) return; // Make sure not to use any class members at the callback. // The class may be gone when it is called. // Log() is a static function so it is OK. From c0c317dc0b2c8d433b5f9e3f349281f2f4f343b2 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 9 Mar 2017 15:16:52 -0800 Subject: [PATCH 02/23] Update mixerclient with sync-ed grpc write and fail-fast. (#155) * Update mixerclient with sync-ed write and fail-fast. * Update to latest test. * Update again --- src/envoy/mixer/repositories.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/mixer/repositories.bzl b/src/envoy/mixer/repositories.bzl index 868f52aabd0..473e2650882 100644 --- a/src/envoy/mixer/repositories.bzl +++ b/src/envoy/mixer/repositories.bzl @@ -15,7 +15,7 @@ ################################################################################ # -MIXER_CLIENT = "456d37e71a7608636ddaf5f3d1acbce015870ebf" +MIXER_CLIENT = "92a305961bcea90ed349ffdbb0ea299c6f6bacad" def mixer_client_repositories(bind=True): native.git_repository( From 25f43e36e7365258eb4498083c8a52d03eeee58d Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 9 Mar 2017 17:54:38 -0800 Subject: [PATCH 03/23] Update envoy to PR553 (#156) * Update envoy to PR553 * Update libevent to 2.1.8 --- src/envoy/repositories.bzl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/envoy/repositories.bzl b/src/envoy/repositories.bzl index ca7e9468ded..e7c5f5584da 100644 --- a/src/envoy/repositories.bzl +++ b/src/envoy/repositories.bzl @@ -62,6 +62,7 @@ event_srcs = [ "evthread.c", "evutil.c", "evutil_rand.c", + "evutil_time.c", "http.c", "listener.c", "log.c", @@ -86,9 +87,7 @@ cc_library( "defer-internal.h", "evbuffer-internal.h", "event-internal.h", - "event.h", "evthread-internal.h", - "evutil.h", "http-internal.h", "iocp-internal.h", "ipv6-internal.h", @@ -134,8 +133,8 @@ cc_library( native.new_http_archive( name = "libevent_git", - url = "https://github.com/libevent/libevent/releases/download/release-2.0.22-stable/libevent-2.0.22-stable.tar.gz", - strip_prefix = "libevent-2.0.22-stable", + url = "https://github.com/libevent/libevent/releases/download/release-2.1.8-stable/libevent-2.1.8-stable.tar.gz", + strip_prefix = "libevent-2.1.8-stable", build_file_content = BUILD, ) @@ -752,6 +751,6 @@ cc_test( native.new_git_repository( name = "envoy_git", remote = "https://github.com/lyft/envoy.git", - commit = "70e5d651b55d356770529e5bee9c6b2707d9cf21", # 3/1/2017 + commit = "9679c08a21988a8e95a4e9b2ba65712ec25eadc1", # https://github.com/lyft/envoy/pull/553 build_file_content = BUILD, ) From 5bf8276f8605cecd2ef8c5f563ce9b0f8497bf51 Mon Sep 17 00:00:00 2001 From: Sebastien Vas Date: Mon, 13 Mar 2017 09:24:07 -0700 Subject: [PATCH 04/23] Uses a specific version of the Shared Pipeline lib (#158) --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 9c59a146e50..35b530333b1 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,6 +1,6 @@ #!groovy -@Library('testutils') +@Library('testutils@stable-838b134') import org.istio.testutils.Utilities import org.istio.testutils.GitUtilities From f0e541fdc224f936e062ad26596688144ba67c7f Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Mon, 13 Mar 2017 12:59:15 -0700 Subject: [PATCH 05/23] Update lyft/envoy commit Id to latest. (#161) * Update lyft/envoy commit Id to latest. * Remove the comment about pull request * Add new line - will delete in next commit. --- src/envoy/repositories.bzl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/envoy/repositories.bzl b/src/envoy/repositories.bzl index e7c5f5584da..a255ed1c39b 100644 --- a/src/envoy/repositories.bzl +++ b/src/envoy/repositories.bzl @@ -751,6 +751,7 @@ cc_test( native.new_git_repository( name = "envoy_git", remote = "https://github.com/lyft/envoy.git", - commit = "9679c08a21988a8e95a4e9b2ba65712ec25eadc1", # https://github.com/lyft/envoy/pull/553 + commit = "9dcac8ca111ecc8da059d1f8d42eb766b44bacd6", build_file_content = BUILD, ) + From 76d96cd4d254570c66d8b5604e47d301f7eddc7d Mon Sep 17 00:00:00 2001 From: Kuat Date: Tue, 14 Mar 2017 12:21:24 -0700 Subject: [PATCH 06/23] Update repositories.bzl (#169) --- src/envoy/repositories.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/repositories.bzl b/src/envoy/repositories.bzl index a255ed1c39b..5a74586ddc8 100644 --- a/src/envoy/repositories.bzl +++ b/src/envoy/repositories.bzl @@ -751,7 +751,7 @@ cc_test( native.new_git_repository( name = "envoy_git", remote = "https://github.com/lyft/envoy.git", - commit = "9dcac8ca111ecc8da059d1f8d42eb766b44bacd6", + commit = "b72309da41fba0c1222a72262b83bedc7294df65", # Mar 13 2017 build_file_content = BUILD, ) From 4d1dbe012056041cdbd047326d765943be0c6258 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 15 Mar 2017 12:10:22 -0700 Subject: [PATCH 07/23] Always set response latency (#172) --- src/envoy/mixer/http_control.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/envoy/mixer/http_control.cc b/src/envoy/mixer/http_control.cc index 3b97d20a35f..50a637ca356 100644 --- a/src/envoy/mixer/http_control.cc +++ b/src/envoy/mixer/http_control.cc @@ -94,10 +94,8 @@ void FillRequestInfoAttributes(const AccessLog::RequestInfo& info, attr->attributes[kResponseSize] = Attributes::Int64Value(info.bytesSent()); } - if (info.duration().count() > 0) { - attr->attributes[kResponseLatency] = Attributes::DurationValue( - std::chrono::duration_cast(info.duration())); - } + attr->attributes[kResponseLatency] = Attributes::DurationValue( + std::chrono::duration_cast(info.duration())); if (info.responseCode().valid()) { attr->attributes[kResponseHttpCode] = From 52ef466a3fb296a4c1e11336fb92ab3f7064f102 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 16 Mar 2017 10:15:18 -0700 Subject: [PATCH 08/23] Update mixerclient to sync_transport change. (#178) --- src/envoy/mixer/repositories.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/mixer/repositories.bzl b/src/envoy/mixer/repositories.bzl index 473e2650882..910055aff48 100644 --- a/src/envoy/mixer/repositories.bzl +++ b/src/envoy/mixer/repositories.bzl @@ -15,7 +15,7 @@ ################################################################################ # -MIXER_CLIENT = "92a305961bcea90ed349ffdbb0ea299c6f6bacad" +MIXER_CLIENT = "e292a59fbbcbeb121d532156c0ac5aad5ab5c9aa" def mixer_client_repositories(bind=True): native.git_repository( From 531cfa007231e3af08b989b0d09ce45f6b375fd9 Mon Sep 17 00:00:00 2001 From: Kuat Date: Thu, 16 Mar 2017 17:03:05 -0700 Subject: [PATCH 09/23] Use opaque config to turn on/off forward attribute and mixer filter (#179) * Modify mixer filter * Swap defaults * Make the filter decoder only * cache mixer disabled decision --- src/envoy/mixer/BUILD | 1 - src/envoy/mixer/forward_attribute_filter.cc | 114 ------------------ src/envoy/mixer/http_filter.cc | 126 ++++++++++++++------ 3 files changed, 87 insertions(+), 154 deletions(-) delete mode 100644 src/envoy/mixer/forward_attribute_filter.cc diff --git a/src/envoy/mixer/BUILD b/src/envoy/mixer/BUILD index 132bc364a1d..6bf9ea64d72 100644 --- a/src/envoy/mixer/BUILD +++ b/src/envoy/mixer/BUILD @@ -31,7 +31,6 @@ cc_proto_library( cc_library( name = "filter_lib", srcs = [ - "forward_attribute_filter.cc", "http_control.cc", "http_control.h", "http_filter.cc", diff --git a/src/envoy/mixer/forward_attribute_filter.cc b/src/envoy/mixer/forward_attribute_filter.cc deleted file mode 100644 index 08d58916f89..00000000000 --- a/src/envoy/mixer/forward_attribute_filter.cc +++ /dev/null @@ -1,114 +0,0 @@ -/* 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 "precompiled/precompiled.h" - -#include "common/common/base64.h" -#include "common/common/logger.h" -#include "common/http/headers.h" -#include "common/http/utility.h" -#include "envoy/server/instance.h" -#include "server/config/network/http_connection_manager.h" -#include "src/envoy/mixer/utils.h" - -namespace Http { -namespace ForwardAttribute { -namespace { - -// The Json object name to specify attributes which will be forwarded -// to the upstream istio proxy. -const std::string kJsonNameAttributes("attributes"); - -} // namespace - -class Config : public Logger::Loggable { - private: - std::string attributes_; - - public: - Config(const Json::Object& config) { - Utils::StringMap attributes = - Utils::ExtractStringMap(config, kJsonNameAttributes); - if (!attributes.empty()) { - std::string serialized_str = Utils::SerializeStringMap(attributes); - attributes_ = - Base64::encode(serialized_str.c_str(), serialized_str.size()); - } - } - - const std::string& attributes() const { return attributes_; } -}; - -typedef std::shared_ptr ConfigPtr; - -class ForwardAttributeFilter : public Http::StreamDecoderFilter { - private: - ConfigPtr config_; - - public: - ForwardAttributeFilter(ConfigPtr config) : config_(config) {} - - FilterHeadersStatus decodeHeaders(HeaderMap& headers, - bool end_stream) override { - if (!config_->attributes().empty()) { - headers.addStatic(Utils::kIstioAttributeHeader, config_->attributes()); - } - return FilterHeadersStatus::Continue; - } - - FilterDataStatus decodeData(Buffer::Instance& data, - bool end_stream) override { - return FilterDataStatus::Continue; - } - - FilterTrailersStatus decodeTrailers(HeaderMap& trailers) override { - return FilterTrailersStatus::Continue; - } - - void setDecoderFilterCallbacks( - StreamDecoderFilterCallbacks& callbacks) override {} -}; - -} // namespace ForwardAttribute -} // namespace Http - -namespace Server { -namespace Configuration { - -class ForwardAttributeConfig : public HttpFilterConfigFactory { - public: - HttpFilterFactoryCb tryCreateFilterFactory( - HttpFilterType type, const std::string& name, const Json::Object& config, - const std::string&, Server::Instance& server) override { - if (type != HttpFilterType::Decoder || name != "forward_attribute") { - return nullptr; - } - - Http::ForwardAttribute::ConfigPtr add_header_config( - new Http::ForwardAttribute::Config(config)); - return [add_header_config]( - Http::FilterChainFactoryCallbacks& callbacks) -> void { - std::shared_ptr instance( - new Http::ForwardAttribute::ForwardAttributeFilter( - add_header_config)); - callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterPtr(instance)); - }; - } -}; - -static RegisterHttpFilterConfigFactory register_; - -} // namespace Configuration -} // namespace server diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index 6a8da6a6f45..59a459e166c 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -15,6 +15,7 @@ #include "precompiled/precompiled.h" +#include "common/common/base64.h" #include "common/common/logger.h" #include "common/http/headers.h" #include "common/http/utility.h" @@ -36,7 +37,17 @@ namespace { const std::string kJsonNameMixerServer("mixer_server"); // The Json object name for static attributes. -const std::string kJsonNameMixerAttributes("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"); + +// Switch to turn off mixer check/report/quota +const std::string kJsonNameMixerSwitch("mixer_control"); // Convert Status::code to HTTP code int HttpCode(int code) { @@ -88,6 +99,7 @@ class Config : public Logger::Loggable { private: std::shared_ptr http_control_; Upstream::ClusterManager& cm_; + std::string forward_attributes_; public: Config(const Json::Object& config, Server::Instance& server) @@ -101,37 +113,75 @@ class Config : public Logger::Loggable { __func__); } - std::map attributes = + Utils::StringMap attributes = + Utils::ExtractStringMap(config, kJsonNameForwardAttributes); + if (!attributes.empty()) { + std::string serialized_str = Utils::SerializeStringMap(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(attributes)); + http_control_ = std::make_shared(mixer_server, + std::move(mixer_attributes)); log().debug("Called Mixer::Config constructor with mixer_server: ", mixer_server); } std::shared_ptr& http_control() { return http_control_; } + const std::string& forward_attributes() const { return forward_attributes_; } }; typedef std::shared_ptr ConfigPtr; -class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { +class Instance : public Http::StreamDecoderFilter, + public Http::AccessLog::Instance { private: std::shared_ptr http_control_; + ConfigPtr config_; std::shared_ptr request_data_; enum State { NotStarted, Calling, Complete, Responded }; State state_; StreamDecoderFilterCallbacks* decoder_callbacks_; - StreamEncoderFilterCallbacks* encoder_callbacks_; bool initiating_call_; int check_status_code_; + bool mixer_disabled_; + + // mixer control switch (off by default) + bool mixer_disabled() { + auto route = decoder_callbacks_->route()->routeEntry(); + if (route != nullptr) { + auto key = route->opaqueConfig().find(kJsonNameMixerSwitch); + if (key != route->opaqueConfig().end() && key->second == "on") { + return false; + } + } + return true; + } + + // attribute forward switch (on by default) + bool forward_disabled() { + auto route = decoder_callbacks_->route()->routeEntry(); + if (route != nullptr) { + auto key = route->opaqueConfig().find(kJsonNameForwardSwitch); + if (key != route->opaqueConfig().end() && key->second == "off") { + return true; + } + } + return false; + } + public: Instance(ConfigPtr config) : http_control_(config->http_control()), + config_(config), state_(NotStarted), initiating_call_(false), check_status_code_(HttpCode(StatusCode::UNKNOWN)) { @@ -149,6 +199,17 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { FilterHeadersStatus decodeHeaders(HeaderMap& headers, bool end_stream) override { Log().debug("Called Mixer::Instance : {}", __func__); + + if (!config_->forward_attributes().empty() && !forward_disabled()) { + headers.addStatic(Utils::kIstioAttributeHeader, + config_->forward_attributes()); + } + + mixer_disabled_ = mixer_disabled(); + if (mixer_disabled_) { + return FilterHeadersStatus::Continue; + } + state_ = Calling; initiating_call_ = true; request_data_ = std::make_shared(); @@ -174,6 +235,10 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override { + if (mixer_disabled_) { + return FilterDataStatus::Continue; + } + Log().debug("Called Mixer::Instance : {} ({}, {})", __func__, data.length(), end_stream); if (state_ == Calling) { @@ -183,6 +248,10 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { } FilterTrailersStatus decodeTrailers(HeaderMap& trailers) override { + if (mixer_disabled_) { + return FilterTrailersStatus::Continue; + } + Log().debug("Called Mixer::Instance : {}", __func__); if (state_ == Calling) { return FilterTrailersStatus::StopIteration; @@ -194,8 +263,10 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { StreamDecoderFilterCallbacks& callbacks) override { Log().debug("Called Mixer::Instance : {}", __func__); decoder_callbacks_ = &callbacks; - decoder_callbacks_->addResetStreamCallback( - [this]() { state_ = Responded; }); + if (!mixer_disabled()) { + decoder_callbacks_->addResetStreamCallback( + [this]() { state_ = Responded; }); + } } void completeCheck(const Status& status) { @@ -215,29 +286,6 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { } } - virtual FilterHeadersStatus encodeHeaders(HeaderMap& headers, - bool end_stream) override { - Log().debug("Called Mixer::Instance : {}", __func__); - return FilterHeadersStatus::Continue; - } - - virtual FilterDataStatus encodeData(Buffer::Instance& data, - bool end_stream) override { - Log().debug("Called Mixer::Instance : {}", __func__); - return FilterDataStatus::Continue; - } - - virtual FilterTrailersStatus encodeTrailers(HeaderMap& trailers) override { - Log().debug("Called Mixer::Instance : {}", __func__); - return FilterTrailersStatus::Continue; - } - - virtual void setEncoderFilterCallbacks( - StreamEncoderFilterCallbacks& callbacks) override { - Log().debug("Called Mixer::Instance : {}", __func__); - encoder_callbacks_ = &callbacks; - } - virtual void log(const HeaderMap* request_headers, const HeaderMap* response_headers, const AccessLog::RequestInfo& request_info) override { @@ -272,19 +320,19 @@ class MixerConfig : public HttpFilterConfigFactory { HttpFilterFactoryCb tryCreateFilterFactory( HttpFilterType type, const std::string& name, const Json::Object& config, const std::string&, Server::Instance& server) override { - if (type != HttpFilterType::Both || name != "mixer") { + if (type != HttpFilterType::Decoder || name != "mixer") { return nullptr; } Http::Mixer::ConfigPtr mixer_config( new Http::Mixer::Config(config, server)); - return - [mixer_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { - std::shared_ptr instance( - new Http::Mixer::Instance(mixer_config)); - callbacks.addStreamFilter(Http::StreamFilterPtr(instance)); - callbacks.addAccessLogHandler(Http::AccessLog::InstancePtr(instance)); - }; + return [mixer_config]( + Http::FilterChainFactoryCallbacks& callbacks) -> void { + std::shared_ptr instance( + new Http::Mixer::Instance(mixer_config)); + callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterPtr(instance)); + callbacks.addAccessLogHandler(Http::AccessLog::InstancePtr(instance)); + }; } }; From 95535f5fb6631adcb453b8f84a2e76d13ea23886 Mon Sep 17 00:00:00 2001 From: Kuat Date: Thu, 16 Mar 2017 18:29:47 -0700 Subject: [PATCH 10/23] Fix a bug in opaque config change and test it out (#182) * Fix a bug and test it out * Update filter type * Update README.md --- src/envoy/mixer/README.md | 39 ++++++++++++----------------- src/envoy/mixer/envoy.conf.template | 15 +++++++---- src/envoy/mixer/http_filter.cc | 6 ++--- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index d7d39f30e03..4e30eb08f56 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -46,26 +46,29 @@ This Proxy will use Envoy and talk to Mixer server. * Then issue HTTP request to proxy. ``` + # request to server-side proxy curl http://localhost:9090/echo -d "hello world" + # request to client-side proxy that gets sent to server-side proxy + curl http://localhost:7070/echo -d "hello world" ``` ## How to configurate HTTP filters -This module has two HTTP filters: -1. mixer filter: intercept all HTTP requests, call the mixer. -2. forward_attribute filter: Forward attributes to the upstream istio/proxy. - ### *mixer* filter: This filter will intercept all HTTP requests and call Mixer. Here is its config: ``` "filters": [ - "type": "both", + "type": "decoder", "name": "mixer", "config": { "mixer_server": "${MIXER_SERVER}", - "attributes" : { + "mixer_attributes" : { + "attribute_name1": "attribute_value1", + "attribute_name2": "attribute_value2" + }, + "forward_attributes" : { "attribute_name1": "attribute_value1", "attribute_name2": "attribute_value2" } @@ -74,26 +77,16 @@ This filter will intercept all HTTP requests and call Mixer. Here is its config: Notes: * mixer_server is required -* attributes: these attributes will be send to the mixer +* mixer_attributes: these attributes will be send to the mixer +* forward_attributes: these attributes will be forwarded to the upstream istio/proxy. -### *forward_attribute* HTTP filter: - -This filer will forward attributes to the upstream istio/proxy. +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: ``` - "filters": [ - "type": "decoder", - "name": "forward_attribute", - "config": { - "attributes": { - "attribute_name1": "attribute_value1", - "attribute_name2": "attribute_value2" - } + "opaque_config": { + "mixer_control": "on", + "mixer_forward": "off" } ``` -Notes: -* attributes: these attributes will be forwarded to the upstream istio/proxy. - - - +This config reverts the behavior by sending requests to mixer server but not forwarding any attributes. diff --git a/src/envoy/mixer/envoy.conf.template b/src/envoy/mixer/envoy.conf.template index c5377632388..86aa0e1dc33 100644 --- a/src/envoy/mixer/envoy.conf.template +++ b/src/envoy/mixer/envoy.conf.template @@ -19,7 +19,11 @@ { "timeout_ms": 0, "prefix": "/", - "cluster": "service1" + "cluster": "service1", + "opaque_config": { + "mixer_control": "on", + "mixer_forward": "off" + } } ] } @@ -32,11 +36,11 @@ ], "filters": [ { - "type": "both", + "type": "decoder", "name": "mixer", "config": { "mixer_server": "${MIXER_SERVER}", - "attributes": { + "mixer_attributes": { "target.uid": "POD222", "target.namespace": "XYZ222" } @@ -85,9 +89,10 @@ "filters": [ { "type": "decoder", - "name": "forward_attribute", + "name": "mixer", "config": { - "attributes": { + "mixer_server": "${MIXER_SERVER}", + "forward_attributes": { "source.uid": "POD11", "source.namespace": "XYZ11" } diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index 59a459e166c..d6ca93e2bb5 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -263,10 +263,8 @@ class Instance : public Http::StreamDecoderFilter, StreamDecoderFilterCallbacks& callbacks) override { Log().debug("Called Mixer::Instance : {}", __func__); decoder_callbacks_ = &callbacks; - if (!mixer_disabled()) { - decoder_callbacks_->addResetStreamCallback( - [this]() { state_ = Responded; }); - } + decoder_callbacks_->addResetStreamCallback( + [this]() { state_ = Responded; }); } void completeCheck(const Status& status) { From 93ea507f3a141bed4bb1240101f77ce6c4844e6e Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 17 Mar 2017 12:38:39 -0700 Subject: [PATCH 11/23] Update mixer client to mixer api with gogoproto. (#184) --- src/envoy/mixer/repositories.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/mixer/repositories.bzl b/src/envoy/mixer/repositories.bzl index 910055aff48..9d06b24a14b 100644 --- a/src/envoy/mixer/repositories.bzl +++ b/src/envoy/mixer/repositories.bzl @@ -15,7 +15,7 @@ ################################################################################ # -MIXER_CLIENT = "e292a59fbbcbeb121d532156c0ac5aad5ab5c9aa" +MIXER_CLIENT = "1d6b587755846fe1b3a44fb53e3ab9ce0534af2c" def mixer_client_repositories(bind=True): native.git_repository( From 20c5bab707136751fd7279da743826a75b950431 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Mon, 20 Mar 2017 11:04:55 -0700 Subject: [PATCH 12/23] Move .bazelrc to tools/bazel.rc (#186) * Move .bazelrc to tools/bazel.rc * Update Jenkinsfile with latest version of pipeline --- .travis.yml | 3 +-- Jenkinsfile | 29 +++++++++++++---------------- .bazelrc => tools/bazel.rc | 0 3 files changed, 14 insertions(+), 18 deletions(-) rename .bazelrc => tools/bazel.rc (100%) diff --git a/.travis.yml b/.travis.yml index 88be42c285b..418e24d2dee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,8 +35,7 @@ before_install: - sudo dpkg -i bazel_${BAZEL_VERSION}-linux-x86_64.deb - sudo apt-get -f install -qqy uuid-dev - cd ${TRAVIS_BUILD_DIR} - - mv .bazelrc .bazelrc.orig - - cat .bazelrc.travis .bazelrc.orig > .bazelrc + - mv .bazelrc.travis .bazelrc script: - script/check-style diff --git a/Jenkinsfile b/Jenkinsfile index 35b530333b1..6575dc498f5 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,6 +1,6 @@ #!groovy -@Library('testutils@stable-838b134') +@Library('testutils@stable-63c264e') import org.istio.testutils.Utilities import org.istio.testutils.GitUtilities @@ -12,20 +12,17 @@ def utils = new Utilities() def bazel = new Bazel() mainFlow(utils) { - pullRequest(utils) { - node { - gitUtils.initialize() - // Proxy does build work correctly with Hazelcast. - // Must use .bazelrc.jenkins - bazel.setVars('', '') - } - - if (utils.runStage('PRESUBMIT')) { - presubmit(gitUtils, bazel) - } - if (utils.runStage('POSTSUBMIT')) { - postsubmit(gitUtils, bazel, utils) - } + node { + gitUtils.initialize() + // Proxy does build work correctly with Hazelcast. + // Must use .bazelrc.jenkins + bazel.setVars('', '') + } + if (utils.runStage('PRESUBMIT')) { + presubmit(gitUtils, bazel) + } + if (utils.runStage('POSTSUBMIT')) { + postsubmit(gitUtils, bazel, utils) } } @@ -62,4 +59,4 @@ def postsubmit(gitUtils, bazel, utils) { utils.publishDockerImages(images, tags, 'release') } } -} \ No newline at end of file +} diff --git a/.bazelrc b/tools/bazel.rc similarity index 100% rename from .bazelrc rename to tools/bazel.rc From 557d0c6a7f1136e98ca1a05494df0b51c5bad7d2 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Mon, 20 Mar 2017 13:42:37 -0700 Subject: [PATCH 13/23] Support apikey based traffic restriction (#189) * b/36368559 support apikey based traffic restriction * Fixed code formatting --- .../api_manager/context/request_context.cc | 14 ++++++ .../src/api_manager/service_control/info.h | 5 +++ .../src/api_manager/service_control/proto.cc | 18 ++++++++ .../api_manager/service_control/proto_test.cc | 18 ++++++++ .../testdata/check_request_android_ios.golden | 43 +++++++++++++++++++ 5 files changed, 98 insertions(+) create mode 100644 contrib/endpoints/src/api_manager/service_control/testdata/check_request_android_ios.golden diff --git a/contrib/endpoints/src/api_manager/context/request_context.cc b/contrib/endpoints/src/api_manager/context/request_context.cc index 75bcb177f9d..24508a4a981 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.cc +++ b/contrib/endpoints/src/api_manager/context/request_context.cc @@ -51,6 +51,16 @@ const char kDefaultApiKeyQueryName1[] = "key"; const char kDefaultApiKeyQueryName2[] = "api_key"; const char kDefaultApiKeyHeaderName[] = "x-api-key"; +// Header for android package name, used for api key restriction check. +const char kXAndroidPackage[] = "x-android-package"; + +// Header for android certificate fingerprint, used for api key restriction +// check. +const char kXAndroidCert[] = "x-android-cert"; + +// Header for IOS bundle identifier, used for api key restriction check. +const char kXIosBundleId[] = "x-ios-bundle-identifier"; + // Default location const char kDefaultLocation[] = "us-central1"; @@ -225,6 +235,10 @@ void RequestContext::FillCheckRequestInfo( service_control::CheckRequestInfo *info) { FillOperationInfo(info); info->allow_unregistered_calls = method()->allow_unregistered_calls(); + + request_->FindHeader(kXAndroidPackage, &info->android_package_name); + request_->FindHeader(kXAndroidCert, &info->android_cert_fingerprint); + request_->FindHeader(kXIosBundleId, &info->ios_bundle_id); } void RequestContext::FillReportRequestInfo( diff --git a/contrib/endpoints/src/api_manager/service_control/info.h b/contrib/endpoints/src/api_manager/service_control/info.h index f203057cc9e..1a62fdfed13 100644 --- a/contrib/endpoints/src/api_manager/service_control/info.h +++ b/contrib/endpoints/src/api_manager/service_control/info.h @@ -74,6 +74,11 @@ struct CheckRequestInfo : public OperationInfo { // Whether the method allow unregistered calls. bool allow_unregistered_calls; + // used for api key restriction check + std::string android_package_name; + std::string android_cert_fingerprint; + std::string ios_bundle_id; + CheckRequestInfo() : allow_unregistered_calls(false) {} }; diff --git a/contrib/endpoints/src/api_manager/service_control/proto.cc b/contrib/endpoints/src/api_manager/service_control/proto.cc index 694f299d22c..cfaa9a5bf53 100644 --- a/contrib/endpoints/src/api_manager/service_control/proto.cc +++ b/contrib/endpoints/src/api_manager/service_control/proto.cc @@ -420,6 +420,12 @@ const char kServiceControlServiceAgent[] = const char kServiceControlUserAgent[] = "servicecontrol.googleapis.com/user_agent"; const char kServiceControlPlatform[] = "servicecontrol.googleapis.com/platform"; +const char kServiceControlAndroidPackageName[] = + "servicecontrol.googleapis.com/android_package_name"; +const char kServiceControlAndroidCertFingerprint[] = + "servicecontrol.googleapis.com/android_cert_fingerprint"; +const char kServiceControlIosBundleId[] = + "servicecontrol.googleapis.com/ios_bundle_id"; // User agent label value // The value for kUserAgent should be configured at service control server. @@ -928,6 +934,18 @@ Status Proto::FillCheckRequest(const CheckRequestInfo& info, (*labels)[kServiceControlUserAgent] = kUserAgent; (*labels)[kServiceControlServiceAgent] = kServiceAgentPrefix + utils::Version::instance().get(); + + if (!info.android_package_name.empty()) { + (*labels)[kServiceControlAndroidPackageName] = info.android_package_name; + } + if (!info.android_cert_fingerprint.empty()) { + (*labels)[kServiceControlAndroidCertFingerprint] = + info.android_cert_fingerprint; + } + if (!info.ios_bundle_id.empty()) { + (*labels)[kServiceControlIosBundleId] = info.ios_bundle_id; + } + return Status::OK; } diff --git a/contrib/endpoints/src/api_manager/service_control/proto_test.cc b/contrib/endpoints/src/api_manager/service_control/proto_test.cc index 609cb54abe7..0a3f05c3668 100644 --- a/contrib/endpoints/src/api_manager/service_control/proto_test.cc +++ b/contrib/endpoints/src/api_manager/service_control/proto_test.cc @@ -161,6 +161,24 @@ TEST_F(ProtoTest, FillGoodCheckRequestTest) { ASSERT_EQ(expected_text, text); } +TEST_F(ProtoTest, FillGoodCheckRequestAndroidIosTest) { + CheckRequestInfo info; + FillOperationInfo(&info); + FillCheckRequestInfo(&info); + + info.android_package_name = "com.google.cloud"; + info.android_cert_fingerprint = "AIzaSyB4Gz8nyaSaWo63IPUcy5d_L8dpKtOTSD0"; + info.ios_bundle_id = "5b40ad6af9a806305a0a56d7cb91b82a27c26909"; + + gasv1::CheckRequest request; + ASSERT_TRUE(scp_.FillCheckRequest(info, &request).ok()); + + std::string text = CheckRequestToString(&request); + std::string expected_text = + ReadTestBaseline("check_request_android_ios.golden"); + ASSERT_EQ(expected_text, text); +} + TEST_F(ProtoTest, FillNoApiKeyCheckRequestTest) { CheckRequestInfo info; info.operation_id = "operation_id"; diff --git a/contrib/endpoints/src/api_manager/service_control/testdata/check_request_android_ios.golden b/contrib/endpoints/src/api_manager/service_control/testdata/check_request_android_ios.golden new file mode 100644 index 00000000000..c847061da4f --- /dev/null +++ b/contrib/endpoints/src/api_manager/service_control/testdata/check_request_android_ios.golden @@ -0,0 +1,43 @@ +service_name: "test_service" +operation { + operation_id: "operation_id" + operation_name: "operation_name" + consumer_id: "api_key:api_key_x" + start_time { + seconds: 100000 + nanos: 100000 + } + end_time { + seconds: 100000 + nanos: 100000 + } + labels { + key: "servicecontrol.googleapis.com/android_cert_fingerprint" + value: "AIzaSyB4Gz8nyaSaWo63IPUcy5d_L8dpKtOTSD0" + } + labels { + key: "servicecontrol.googleapis.com/android_package_name" + value: "com.google.cloud" + } + labels { + key: "servicecontrol.googleapis.com/caller_ip" + value: "1.2.3.4" + } + labels { + key: "servicecontrol.googleapis.com/ios_bundle_id" + value: "5b40ad6af9a806305a0a56d7cb91b82a27c26909" + } + labels { + key: "servicecontrol.googleapis.com/referer" + value: "referer" + } + labels { + key: "servicecontrol.googleapis.com/service_agent" + value: "ESP/{{service_agent_version}}" + } + labels { + key: "servicecontrol.googleapis.com/user_agent" + value: "ESP" + } +} +service_config_id: "2016-09-19r0" From 978b9678d57d1123d7dfec135f5aa6824d26f0d5 Mon Sep 17 00:00:00 2001 From: Kuat Date: Mon, 20 Mar 2017 14:04:37 -0700 Subject: [PATCH 14/23] Fix crash in unreachable/overloaded RDS (#190) --- src/envoy/mixer/http_filter.cc | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index d6ca93e2bb5..8708333de40 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -156,11 +156,14 @@ class Instance : public Http::StreamDecoderFilter, // mixer control switch (off by default) bool mixer_disabled() { - auto route = decoder_callbacks_->route()->routeEntry(); + auto route = decoder_callbacks_->route(); if (route != nullptr) { - auto key = route->opaqueConfig().find(kJsonNameMixerSwitch); - if (key != route->opaqueConfig().end() && key->second == "on") { - return false; + auto entry = route->routeEntry(); + if (entry != nullptr) { + auto key = entry->opaqueConfig().find(kJsonNameMixerSwitch); + if (key != entry->opaqueConfig().end() && key->second == "on") { + return false; + } } } return true; @@ -168,11 +171,14 @@ class Instance : public Http::StreamDecoderFilter, // attribute forward switch (on by default) bool forward_disabled() { - auto route = decoder_callbacks_->route()->routeEntry(); + auto route = decoder_callbacks_->route(); if (route != nullptr) { - auto key = route->opaqueConfig().find(kJsonNameForwardSwitch); - if (key != route->opaqueConfig().end() && key->second == "off") { - return true; + auto entry = route->routeEntry(); + if (entry != nullptr) { + auto key = entry->opaqueConfig().find(kJsonNameForwardSwitch); + if (key != entry->opaqueConfig().end() && key->second == "off") { + return true; + } } } return false; From 955d54a055c08474bf64a063e13eb81b75253b7f Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Mon, 20 Mar 2017 14:12:20 -0700 Subject: [PATCH 15/23] Add mixer client end to end integration test. (#177) * Add mixer client end to end integration test. * Split some repositories into a separate file. * use real mixer for fake mixer_server. * Test repository * use mixer bzl file. * Use mixer repositories * Not to use mixer repository. * Add return line at the end of WORKSPACE. --- BUILD | 4 + WORKSPACE | 16 + src/envoy/mixer/BUILD | 16 +- src/envoy/mixer/integration_test/BUILD | 58 +++ .../mixer/integration_test/attributes.go | 114 ++++++ src/envoy/mixer/integration_test/envoy.conf | 142 ++++++++ src/envoy/mixer/integration_test/envoy.go | 103 ++++++ .../mixer/integration_test/http_client.go | 80 +++++ .../mixer/integration_test/http_server.go | 111 ++++++ .../mixer/integration_test/mixer_server.go | 122 +++++++ .../mixer/integration_test/mixer_test.go | 333 ++++++++++++++++++ .../mixer/integration_test/repositories.bzl | 272 ++++++++++++++ src/envoy/mixer/integration_test/setup.go | 64 ++++ 13 files changed, 1430 insertions(+), 5 deletions(-) create mode 100644 src/envoy/mixer/integration_test/BUILD create mode 100644 src/envoy/mixer/integration_test/attributes.go create mode 100644 src/envoy/mixer/integration_test/envoy.conf create mode 100644 src/envoy/mixer/integration_test/envoy.go create mode 100644 src/envoy/mixer/integration_test/http_client.go create mode 100644 src/envoy/mixer/integration_test/http_server.go create mode 100644 src/envoy/mixer/integration_test/mixer_server.go create mode 100644 src/envoy/mixer/integration_test/mixer_test.go create mode 100644 src/envoy/mixer/integration_test/repositories.bzl create mode 100644 src/envoy/mixer/integration_test/setup.go diff --git a/BUILD b/BUILD index ac06d245289..510620e8246 100644 --- a/BUILD +++ b/BUILD @@ -24,3 +24,7 @@ config_setting( }, visibility = ["//visibility:public"], ) + +load("@io_bazel_rules_go//go:def.bzl", "go_prefix") + +go_prefix("istio.io/proxy") diff --git a/WORKSPACE b/WORKSPACE index ddfa05d578b..4bc7137304c 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -96,3 +96,19 @@ http_file( url = "https://storage.googleapis.com/istio-build/manager/ubuntu_xenial_debug-" + DEBUG_BASE_IMAGE_SHA + ".tar.gz", sha256 = DEBUG_BASE_IMAGE_SHA, ) + +# Following go repositories are for building go integration test for mixer filter. +git_repository( + name = "io_bazel_rules_go", + commit = "9496d79880a7d55b8e4a96f04688d70a374eaaf4", # Mar 3, 2017 (v0.4.1) + remote = "https://github.com/bazelbuild/rules_go.git", +) + +git_repository( + name = "org_pubref_rules_protobuf", + commit = "d42e895387c658eda90276aea018056fcdcb30e4", # Mar 07 2017 (gogo* support) + remote = "https://github.com/pubref/rules_protobuf", +) + +load("//src/envoy/mixer/integration_test:repositories.bzl", "go_mixer_repositories") +go_mixer_repositories() diff --git a/src/envoy/mixer/BUILD b/src/envoy/mixer/BUILD index 6bf9ea64d72..4d57f801626 100644 --- a/src/envoy/mixer/BUILD +++ b/src/envoy/mixer/BUILD @@ -15,7 +15,6 @@ ################################################################################ # - load("@bazel_tools//tools/build_defs/pkg:pkg.bzl", "pkg_tar") load("//src/envoy/mixer:proxy_docker.bzl", "proxy_docker_build") load("@protobuf_git//:protobuf.bzl", "cc_proto_library") @@ -48,6 +47,7 @@ cc_library( cc_binary( name = "envoy", linkstatic = 1, + visibility = [":__subpackages__"], deps = [ ":filter_lib", "@envoy_git//:envoy-main", @@ -80,10 +80,6 @@ pkg_tar( ) proxy_docker_build( - images = [ - {"name": "proxy", "base": "@docker_ubuntu//:xenial"}, - {"name": "proxy_debug", "base": "@ubuntu_xenial_debug//file"}, - ], entrypoint = [ "/usr/local/bin/start_envoy", "-e", @@ -93,6 +89,16 @@ proxy_docker_build( "-t", "/etc/opt/proxy/envoy.conf.template", ], + images = [ + { + "name": "proxy", + "base": "@docker_ubuntu//:xenial", + }, + { + "name": "proxy_debug", + "base": "@ubuntu_xenial_debug//file", + }, + ], ports = ["9090"], repository = "istio", tags = ["manual"], diff --git a/src/envoy/mixer/integration_test/BUILD b/src/envoy/mixer/integration_test/BUILD new file mode 100644 index 00000000000..c45865457ce --- /dev/null +++ b/src/envoy/mixer/integration_test/BUILD @@ -0,0 +1,58 @@ +# Copyright 2016 Google Inc. 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. +# +################################################################################ +# + +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = [ + "attributes.go", + "envoy.go", + "http_client.go", + "http_server.go", + "mixer_server.go", + "setup.go", + ], + deps = [ + "@com_github_gogo_protobuf//types:go_default_library", + "@com_github_golang_glog//:go_default_library", + "@com_github_golang_protobuf//proto:go_default_library", + "@com_github_googleapis_googleapis//:google/rpc", + "@com_github_istio_api//:mixer/v1", + "@com_github_istio_mixer//pkg/api:go_default_library", + "@com_github_istio_mixer//pkg/attribute:go_default_library", + "@com_github_istio_mixer//pkg/pool:go_default_library", + "@com_github_istio_mixer//pkg/tracing:go_default_library", + "@org_golang_google_grpc//:go_default_library", + ], +) + +go_test( + name = "mixer_test", + size = "small", + srcs = [ + "mixer_test.go", + ], + data = [ + "envoy.conf", + "//src/envoy/mixer:envoy", + ], + library = ":go_default_library", + # shared memory path /envoy_shared_memory_0 used by Envoy + # hot start is not working in sandbox mode. + local = True, +) diff --git a/src/envoy/mixer/integration_test/attributes.go b/src/envoy/mixer/integration_test/attributes.go new file mode 100644 index 00000000000..4c97208fd4b --- /dev/null +++ b/src/envoy/mixer/integration_test/attributes.go @@ -0,0 +1,114 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "encoding/json" + "fmt" + "reflect" + + "istio.io/mixer/pkg/attribute" +) + +func verifyStringMap(actual map[string]string, expected map[string]interface{}) error { + for k, v := range expected { + vstring := v.(string) + // "-" make sure the key does not exist. + if vstring == "-" { + if _, ok := actual[k]; ok { + return fmt.Errorf("key %+v is NOT expected", k) + } + } else { + if val, ok := actual[k]; ok { + // "*" only check key exist + if val != vstring && vstring != "*" { + return fmt.Errorf("key %+v value doesn't match. Actual %+v, expected %+v", + k, val, vstring) + } + } else { + return fmt.Errorf("key %+v is expected", k) + } + } + } + return nil +} + +// Please see the comment at top of mixer_test.go for verification rules +func Verify(b *attribute.MutableBag, json_results string) error { + var r map[string]interface{} + if err := json.Unmarshal([]byte(json_results), &r); err != nil { + return fmt.Errorf("unable to decode json %v", err) + } + + all_keys := make(map[string]bool) + for _, k := range b.Names() { + all_keys[k] = true + } + + for k, v := range r { + switch vv := v.(type) { + case string: + // "*" means only checking key. + if vv == "*" { + if _, ok := b.Get(k); !ok { + return fmt.Errorf("attribute %+v is expected", k) + } + } else { + if val, ok := b.Get(k); ok { + if val.(string) != v.(string) { + return fmt.Errorf("attribute %+v value doesn't match. Actual %+v, expected %+v", + k, val.(string), v.(string)) + } + } else { + return fmt.Errorf("attribute %+v is expected", k) + } + } + case float64: + // Json converts all integers to float64, + // Our tests only verify size related attributes which are int64 type + if val, ok := b.Get(k); ok { + vint64 := int64(vv) + if val.(int64) != vint64 { + return fmt.Errorf("attribute %+v value doesn't match. Actual %+v, expected %+v", + k, val.(int64), vint64) + } + } else { + return fmt.Errorf("attribute %+v is expected", k) + } + case map[string]interface{}: + if val, ok := b.Get(k); ok { + if err := verifyStringMap(val.(map[string]string), v.(map[string]interface{})); err != nil { + return fmt.Errorf("attribute %+v StringMap doesn't match: %+v", k, err) + } + } else { + return fmt.Errorf("attribute %+v is expected", k) + } + default: + return fmt.Errorf("attribute %+v is of a type %+v that I don't know how to handle ", + k, reflect.TypeOf(v)) + } + delete(all_keys, k) + + } + + if len(all_keys) > 0 { + var s string + for k, _ := range all_keys { + s += k + ", " + } + return fmt.Errorf("Following attributes are not expected: %s", s) + } + return nil +} diff --git a/src/envoy/mixer/integration_test/envoy.conf b/src/envoy/mixer/integration_test/envoy.conf new file mode 100644 index 00000000000..2083d7f4e5a --- /dev/null +++ b/src/envoy/mixer/integration_test/envoy.conf @@ -0,0 +1,142 @@ +{ + "listeners": [ + { + "port": 29090, + "bind_to_port": true, + "filters": [ + { + "type": "read", + "name": "http_connection_manager", + "config": { + "codec_type": "auto", + "stat_prefix": "ingress_http", + "route_config": { + "virtual_hosts": [ + { + "name": "backend", + "domains": ["*"], + "routes": [ + { + "timeout_ms": 0, + "prefix": "/", + "cluster": "service1", + "opaque_config": { + "mixer_control": "on", + "mixer_forward": "off" + } + } + ] + } + ] + }, + "access_log": [ + { + "path": "/dev/stdout" + } + ], + "filters": [ + { + "type": "decoder", + "name": "mixer", + "config": { + "mixer_server": "localhost:29091", + "mixer_attributes": { + "target.uid": "POD222", + "target.namespace": "XYZ222" + } + } + }, + { + "type": "decoder", + "name": "router", + "config": {} + } + ] + } + } + ] + }, + { + "port": 27070, + "bind_to_port": true, + "filters": [ + { + "type": "read", + "name": "http_connection_manager", + "config": { + "codec_type": "auto", + "stat_prefix": "ingress_http", + "route_config": { + "virtual_hosts": [ + { + "name": "backend", + "domains": ["*"], + "routes": [ + { + "timeout_ms": 0, + "prefix": "/", + "cluster": "service2" + } + ] + } + ] + }, + "access_log": [ + { + "path": "/dev/stdout" + } + ], + "filters": [ + { + "type": "decoder", + "name": "mixer", + "config": { + "mixer_server": "localhost:29091", + "forward_attributes": { + "source.uid": "POD11", + "source.namespace": "XYZ11" + } + } + }, + { + "type": "decoder", + "name": "router", + "config": {} + } + ] + } + } + ] + } + ], + "admin": { + "access_log_path": "/dev/stdout", + "port": 29001 + }, + "cluster_manager": { + "clusters": [ + { + "name": "service1", + "connect_timeout_ms": 5000, + "type": "strict_dns", + "lb_type": "round_robin", + "hosts": [ + { + "url": "tcp://localhost:28080" + } + ] + }, + { + "name": "service2", + "connect_timeout_ms": 5000, + "type": "strict_dns", + "lb_type": "round_robin", + "hosts": [ + { + "url": "tcp://localhost:29090" + } + ] + } + ] + } +} diff --git a/src/envoy/mixer/integration_test/envoy.go b/src/envoy/mixer/integration_test/envoy.go new file mode 100644 index 00000000000..0f9a1fc01c7 --- /dev/null +++ b/src/envoy/mixer/integration_test/envoy.go @@ -0,0 +1,103 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "log" + "os" + "os/exec" + "strings" +) + +func getTestBinRootPath() string { + switch { + // custom path + case os.Getenv("TEST_BIN_ROOT") != "": + return os.Getenv("TEST_BIN_ROOT") + // running under bazel + case os.Getenv("TEST_SRCDIR") != "": + return os.Getenv("TEST_SRCDIR") + "/__main__" + // running with native go + case os.Getenv("GOPATH") != "": + list := strings.Split(os.Getenv("GOPATH"), + string(os.PathListSeparator)) + return list[0] + "/bazel-bin" + default: + return "bazel-bin" + } +} + +func getTestDataRootPath() string { + switch { + // custom path + case os.Getenv("TEST_DATA_ROOT") != "": + return os.Getenv("TEST_DATA_ROOT") + // running under bazel + case os.Getenv("TEST_SRCDIR") != "": + return os.Getenv("TEST_SRCDIR") + "/__main__" + // running with native go + case os.Getenv("GOPATH") != "": + list := strings.Split(os.Getenv("GOPATH"), + string(os.PathListSeparator)) + return list[0] + default: + return "" + } +} + +type Envoy struct { + cmd *exec.Cmd +} + +// Run command and return the merged output from stderr and stdout, error code +func Run(name string, args ...string) (s string, err error) { + log.Println(">", name, strings.Join(args, " ")) + c := exec.Command(name, args...) + bytes, err := c.CombinedOutput() + s = string(bytes) + for _, line := range strings.Split(s, "\n") { + log.Println(line) + } + if err != nil { + log.Println(err) + } + return +} + +func NewEnvoy() (*Envoy, error) { + path := getTestBinRootPath() + "/src/envoy/mixer/envoy" + conf := getTestDataRootPath() + + "/src/envoy/mixer/integration_test/envoy.conf" + log.Printf("Envoy binary: %v\n", path) + log.Printf("Envoy config: %v\n", conf) + + cmd := exec.Command(path, "-c", conf, "-l", "debug") + cmd.Stderr = os.Stderr + cmd.Stdout = os.Stdout + return &Envoy{ + cmd: cmd, + }, nil +} + +func (s *Envoy) Start() error { + return s.cmd.Start() +} + +func (s *Envoy) Stop() error { + log.Printf("Kill Envoy ...\n") + err := s.cmd.Process.Kill() + log.Printf("Kill Envoy ... Done\n") + return err +} diff --git a/src/envoy/mixer/integration_test/http_client.go b/src/envoy/mixer/integration_test/http_client.go new file mode 100644 index 00000000000..0212ddf775e --- /dev/null +++ b/src/envoy/mixer/integration_test/http_client.go @@ -0,0 +1,80 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "io/ioutil" + "log" + "net/http" + "net/url" + "strings" +) + +func HTTPGet(url string) (code int, resp_body string, err error) { + log.Println("HTTP GET", url) + client := &http.Client{} + resp, err := client.Get(url) + if err != nil { + log.Println(err) + return 0, "", err + } + defer resp.Body.Close() + body, _ := ioutil.ReadAll(resp.Body) + resp_body = string(body) + code = resp.StatusCode + log.Println(resp_body) + return code, resp_body, nil +} + +func HTTPPost(url string, content_type string, req_body string) (code int, resp_body string, err error) { + log.Println("HTTP POST", url) + client := &http.Client{} + resp, err := client.Post(url, content_type, strings.NewReader(req_body)) + if err != nil { + log.Println(err) + return 0, "", err + } + defer resp.Body.Close() + body, _ := ioutil.ReadAll(resp.Body) + resp_body = string(body) + code = resp.StatusCode + log.Println(resp_body) + return code, resp_body, nil +} + +func HTTPGetWithHeaders(l string, headers map[string]string) (code int, resp_body string, err error) { + log.Println("HTTP GET with headers: ", l) + client := &http.Client{} + req := http.Request{} + + req.Header = map[string][]string{} + for k, v := range headers { + req.Header[k] = []string{v} + } + req.Method = http.MethodGet + req.URL, _ = url.Parse(l) + + resp, err := client.Do(&req) + if err != nil { + log.Println(err) + return 0, "", err + } + defer resp.Body.Close() + body, _ := ioutil.ReadAll(resp.Body) + resp_body = string(body) + code = resp.StatusCode + log.Println(resp_body) + return code, resp_body, nil +} diff --git a/src/envoy/mixer/integration_test/http_server.go b/src/envoy/mixer/integration_test/http_server.go new file mode 100644 index 00000000000..77677286b58 --- /dev/null +++ b/src/envoy/mixer/integration_test/http_server.go @@ -0,0 +1,111 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "fmt" + "io/ioutil" + "log" + "net" + "net/http" + "strings" + "time" +) + +const ( + FailHeader = "x-istio-backend-fail" + FailBody = "Bad request from backend." +) + +type HttpServer struct { + port uint16 + lis net.Listener +} + +func handler(w http.ResponseWriter, r *http.Request) { + log.Printf("%v %v %v %v\n", r.Method, r.URL, r.Proto, r.RemoteAddr) + for name, headers := range r.Header { + for _, h := range headers { + log.Printf("%v: %v\n", name, h) + } + } + body, err := ioutil.ReadAll(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + // Fail if there is such header. + if r.Header.Get(FailHeader) != "" { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(FailBody)) + return + } + + // echo back the Content-Type and Content-Length in the response + for _, k := range []string{"Content-Type", "Content-Length"} { + if v := r.Header.Get(k); v != "" { + w.Header().Set(k, v) + } + } + w.WriteHeader(http.StatusOK) + w.Write(body) +} + +func NewHttpServer(port uint16) (*HttpServer, error) { + log.Printf("Http server listening on port %v\n", port) + lis, err := net.Listen("tcp", fmt.Sprintf(":%d", port)) + if err != nil { + log.Fatal(err) + return nil, err + } + return &HttpServer{ + port: port, + lis: lis, + }, nil +} + +func (s *HttpServer) Start() { + go func() { + http.HandleFunc("/", handler) + http.Serve(s.lis, nil) + }() + + addr := fmt.Sprintf("http://localhost:%s", s.port) + + const maxAttempts = 10 + for i := 0; i < maxAttempts; i++ { + time.Sleep(time.Second) + client := http.Client{} + log.Println("Pinging the server...") + rsp, err := client.Post( + addr+"/echo", "text/plain", strings.NewReader("PING")) + if err == nil && rsp.StatusCode == http.StatusOK { + log.Println("Got a response...") + png, err := ioutil.ReadAll(rsp.Body) + if err == nil && string(png) == "PING" { + log.Println("Server is up and running...") + return + } + } + log.Println("Will wait a second and try again.") + } +} + +func (s *HttpServer) Stop() { + log.Printf("Close HTTP server\n") + s.lis.Close() + log.Printf("Close HTTP server -- Done\n") +} diff --git a/src/envoy/mixer/integration_test/mixer_server.go b/src/envoy/mixer/integration_test/mixer_server.go new file mode 100644 index 00000000000..e36f04b484a --- /dev/null +++ b/src/envoy/mixer/integration_test/mixer_server.go @@ -0,0 +1,122 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "context" + "fmt" + "log" + "net" + + rpc "github.com/googleapis/googleapis/google/rpc" + "google.golang.org/grpc" + + mixerpb "istio.io/api/mixer/v1" + "istio.io/mixer/pkg/api" + "istio.io/mixer/pkg/attribute" + "istio.io/mixer/pkg/pool" + "istio.io/mixer/pkg/tracing" +) + +type Handler struct { + bag *attribute.MutableBag + ch chan int + r_status rpc.Status +} + +func newHandler() *Handler { + return &Handler{ + bag: nil, + ch: make(chan int, 1), + r_status: rpc.Status{}, + } +} + +func (h *Handler) run(bag *attribute.MutableBag) rpc.Status { + h.bag = attribute.CopyBag(bag) + h.ch <- 1 + return h.r_status +} + +type MixerServer struct { + lis net.Listener + gs *grpc.Server + gp *pool.GoroutinePool + s mixerpb.MixerServer + + check *Handler + report *Handler + quota *Handler +} + +func (ts *MixerServer) Check(ctx context.Context, bag *attribute.MutableBag, + request *mixerpb.CheckRequest, response *mixerpb.CheckResponse) { + response.RequestIndex = request.RequestIndex + response.Result = ts.check.run(bag) +} + +func (ts *MixerServer) Report(ctx context.Context, bag *attribute.MutableBag, + request *mixerpb.ReportRequest, response *mixerpb.ReportResponse) { + response.RequestIndex = request.RequestIndex + response.Result = ts.report.run(bag) +} + +func (ts *MixerServer) Quota(ctx context.Context, bag *attribute.MutableBag, + request *mixerpb.QuotaRequest, response *mixerpb.QuotaResponse) { + response.RequestIndex = request.RequestIndex + response.Result = ts.quota.run(bag) + response.Amount = 0 +} + +func NewMixerServer(port uint16) (*MixerServer, error) { + log.Printf("Mixer server listening on port %v\n", port) + s := &MixerServer{ + check: newHandler(), + report: newHandler(), + quota: newHandler(), + } + + var err error + s.lis, err = net.Listen("tcp", fmt.Sprintf(":%d", port)) + if err != nil { + log.Fatalf("failed to listen: %v", err) + return nil, err + } + + var opts []grpc.ServerOption + opts = append(opts, grpc.MaxConcurrentStreams(32)) + opts = append(opts, grpc.MaxMsgSize(1024*1024)) + s.gs = grpc.NewServer(opts...) + + s.gp = pool.NewGoroutinePool(128, false) + s.gp.AddWorkers(32) + + s.s = api.NewGRPCServer(s, tracing.DisabledTracer(), s.gp) + mixerpb.RegisterMixerServer(s.gs, s.s) + return s, nil +} + +func (s *MixerServer) Start() { + go func() { + _ = s.gs.Serve(s.lis) + log.Printf("Mixer server exited\n") + }() +} + +func (s *MixerServer) Stop() { + log.Printf("Stop Mixer server\n") + s.gs.Stop() + log.Printf("Stop Mixer server -- Done\n") +} diff --git a/src/envoy/mixer/integration_test/mixer_test.go b/src/envoy/mixer/integration_test/mixer_test.go new file mode 100644 index 00000000000..b8f382bd58c --- /dev/null +++ b/src/envoy/mixer/integration_test/mixer_test.go @@ -0,0 +1,333 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "fmt" + "testing" + + rpc "github.com/googleapis/googleapis/google/rpc" +) + +const ( + mixerFailMessage = "Unauthenticated by mixer." +) + +// Attributes verification rules +// 1) If value is *, key must exist, but value is not checked. +// 1) If value is -, key must NOT exist. +// 3) At top level attributes, not inside StringMap, all keys must +// be listed. Extra keys are NOT allowed +// 3) Inside StringMap, not need to list all keys. Extra keys are allowed +// +// Attributes provided from envoy config +// * source.id and source.namespace are forwarded from client proxy +// * target.id and target.namespace are from server proxy +// +// HTTP header "x-istio-attributes" is used to forward attributes between +// proxy. It should be removed before calling mixer and backend. +// +// Check attributes from a good GET request +const checkAttributesOkGet = ` +{ + "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 good GET request +const reportAttributesOkGet = ` +{ + "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": "*" + }, + "request.size": 0, + "response.time": "*", + "response.size": 0, + "response.latency": "*", + "response.http.code": 200, + "response.headers": { + "date": "*", + "content-type": "text/plain; charset=utf-8", + "content-length": "0", + ":status": "200", + "server": "envoy" + } +} +` + +// 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 = ` +{ + "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": "*" + }, + "request.size": 12, + "response.time": "*", + "response.size": 12, + "response.latency": "*", + "response.http.code": 200, + "response.headers": { + "date": "*", + "content-type": "text/plain", + "content-length": "12", + ":status": "200", + "server": "envoy" + } +} +` + +// Check attributes from a fail GET request from mixer +const checkAttributesMixerFail = ` +{ + "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 mixer +const reportAttributesMixerFail = ` +{ + "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": "*" + }, + "request.size": 0, + "response.time": "*", + "response.size": 41, + "response.latency": "*", + "response.http.code": 401, + "response.headers": { + "date": "*", + "content-type": "text/plain", + "content-length": "41", + ":status": "401", + "server": "envoy" + } +} +` + +// 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 = ` +{ + "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": "*" + }, + "request.size": 0, + "response.time": "*", + "response.size": 25, + "response.latency": "*", + "response.http.code": 400, + "response.headers": { + "date": "*", + "content-type": "text/plain; charset=utf-8", + "content-length": "25", + ":status": "400", + "server": "envoy" + } +} +` + +func verifyAttributes( + s *TestSetup, tag string, check string, report 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) + } + _ = <-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", + tag, err, s.mixer.report.bag) + } +} + +func TestMixer(t *testing.T) { + s, err := SetUp() + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + defer s.TearDown() + + // There is a client proxy with filter "forward_attribute" + // and a server proxy with filter "mixer" calling mixer. + // This request will connect to client proxy, to server proxy + // and to the backend. + url := fmt.Sprintf("http://localhost:%d/echo", ClientProxyPort) + + // Issues a GET echo request with 0 size body + if _, _, err := HTTPGet(url); err != nil { + t.Errorf("Failed in GET request: %v", err) + } + verifyAttributes(&s, "OkGet", + checkAttributesOkGet, reportAttributesOkGet, t) + + // Issues a POST echo request with + if _, _, err := HTTPPost(url, "text/plain", "Hello World!"); err != nil { + t.Errorf("Failed in POST request: %v", err) + } + verifyAttributes(&s, "OkPost", + checkAttributesOkPost, reportAttributesOkPost, t) + + // Issues a failed request caused by mixer + s.mixer.check.r_status = rpc.Status{ + Code: int32(rpc.UNAUTHENTICATED), + Message: mixerFailMessage, + } + code, resp_body, err := HTTPGet(url) + s.mixer.check.r_status = rpc.Status{} + if err != nil { + t.Errorf("Failed in GET request: error: %v", err) + } + if code != 401 { + t.Errorf("Status code 401 is expected.") + } + if resp_body != "UNAUTHENTICATED:"+mixerFailMessage { + t.Errorf("Error response body is not expected.") + } + verifyAttributes(&s, "MixerFail", + checkAttributesMixerFail, reportAttributesMixerFail, t) + + // Issues a failed request caused by backend + 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) + } + if code != 400 { + t.Errorf("Status code 400 is expected.") + } + if resp_body != FailBody { + t.Errorf("Error response body is not expected.") + } + verifyAttributes(&s, "BackendFail", + checkAttributesBackendFail, reportAttributesBackendFail, t) +} diff --git a/src/envoy/mixer/integration_test/repositories.bzl b/src/envoy/mixer/integration_test/repositories.bzl new file mode 100644 index 00000000000..7eec527fa39 --- /dev/null +++ b/src/envoy/mixer/integration_test/repositories.bzl @@ -0,0 +1,272 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_repositories", "new_go_repository", "go_repository") +load("@org_pubref_rules_protobuf//protobuf:rules.bzl", "proto_repositories") + +load("@org_pubref_rules_protobuf//gogo:rules.bzl", "gogo_proto_repositories") +load("@org_pubref_rules_protobuf//cpp:rules.bzl", "cpp_proto_repositories") + +def go_istio_api_repositories(use_local=False): + ISTIO_API_BUILD_FILE = """ +# build protos from istio.io/api repo + +package(default_visibility = ["//visibility:public"]) + +load("@io_bazel_rules_go//go:def.bzl", "go_prefix") + +go_prefix("istio.io/api") + +load("@org_pubref_rules_protobuf//gogo:rules.bzl", "gogoslick_proto_library") + +gogoslick_proto_library( + name = "mixer/v1", + importmap = { + "gogoproto/gogo.proto": "github.com/gogo/protobuf/gogoproto", + "google/rpc/status.proto": "github.com/googleapis/googleapis/google/rpc", + "google/protobuf/timestamp.proto": "github.com/gogo/protobuf/types", + "google/protobuf/duration.proto": "github.com/gogo/protobuf/types", + }, + imports = [ + "../../external/com_github_gogo_protobuf", + "../../external/com_github_google_protobuf/src", + "../../external/com_github_googleapis_googleapis", + ], + inputs = [ + "@com_github_google_protobuf//:well_known_protos", + "@com_github_googleapis_googleapis//:status_proto", + "@com_github_gogo_protobuf//gogoproto:go_default_library_protos", + ], + protos = [ + "mixer/v1/attributes.proto", + "mixer/v1/check.proto", + "mixer/v1/quota.proto", + "mixer/v1/report.proto", + "mixer/v1/service.proto", + ], + verbose = 0, + visibility = ["//visibility:public"], + with_grpc = True, + deps = [ + "@com_github_gogo_protobuf//gogoproto:go_default_library", + "@com_github_gogo_protobuf//sortkeys:go_default_library", + "@com_github_gogo_protobuf//types:go_default_library", + "@com_github_googleapis_googleapis//:google/rpc", + ], +) + +DESCRIPTOR_FILE_GROUP = [ + "mixer/v1/config/descriptor/attribute_descriptor.proto", + "mixer/v1/config/descriptor/label_descriptor.proto", + "mixer/v1/config/descriptor/log_entry_descriptor.proto", + "mixer/v1/config/descriptor/metric_descriptor.proto", + "mixer/v1/config/descriptor/monitored_resource_descriptor.proto", + "mixer/v1/config/descriptor/principal_descriptor.proto", + "mixer/v1/config/descriptor/quota_descriptor.proto", + "mixer/v1/config/descriptor/value_type.proto", +] + +gogoslick_proto_library( + name = "mixer/v1/config", + importmap = { + "google/protobuf/struct.proto": "github.com/gogo/protobuf/types", + "mixer/v1/config/descriptor/log_entry_descriptor.proto": "istio.io/api/mixer/v1/config/descriptor", + "mixer/v1/config/descriptor/metric_descriptor.proto": "istio.io/api/mixer/v1/config/descriptor", + "mixer/v1/config/descriptor/monitored_resource_descriptor.proto": "istio.io/api/mixer/v1/config/descriptor", + "mixer/v1/config/descriptor/principal_descriptor.proto": "istio.io/api/mixer/v1/config/descriptor", + "mixer/v1/config/descriptor/quota_descriptor.proto": "istio.io/api/mixer/v1/config/descriptor", + }, + imports = [ + "../../external/com_github_google_protobuf/src", + ], + inputs = DESCRIPTOR_FILE_GROUP + [ + "@com_github_google_protobuf//:well_known_protos", + ], + protos = [ + "mixer/v1/config/cfg.proto", + ], + verbose = 0, + visibility = ["//visibility:public"], + with_grpc = False, + deps = [ + ":mixer/v1/config/descriptor", + "@com_github_gogo_protobuf//sortkeys:go_default_library", + "@com_github_gogo_protobuf//types:go_default_library", + "@com_github_googleapis_googleapis//:google/rpc", + ], +) + +gogoslick_proto_library( + name = "mixer/v1/config/descriptor", + importmap = { + "google/protobuf/duration.proto": "github.com/gogo/protobuf/types", + }, + imports = [ + "../../external/com_github_google_protobuf/src", + ], + inputs = [ + "@com_github_google_protobuf//:well_known_protos", + ], + protos = DESCRIPTOR_FILE_GROUP, + verbose = 0, + visibility = ["//visibility:public"], + with_grpc = False, + deps = [ + "@com_github_gogo_protobuf//types:go_default_library", + ], +) +""" + if use_local: + native.new_local_repository( + name = "com_github_istio_api", + build_file_content = ISTIO_API_BUILD_FILE, + path = "../api", + ) + else: + native.new_git_repository( + name = "com_github_istio_api", + build_file_content = ISTIO_API_BUILD_FILE, + commit = "2cb09827d7f09a6e88eac2c2249dcb45c5419f09", # Mar. 14, 2017 (no releases) + remote = "https://github.com/istio/api.git", + ) + +def go_googleapis_repositories(): + GOOGLEAPIS_BUILD_FILE = """ +package(default_visibility = ["//visibility:public"]) + +load("@io_bazel_rules_go//go:def.bzl", "go_prefix") +go_prefix("github.com/googleapis/googleapis") + +load("@org_pubref_rules_protobuf//gogo:rules.bzl", "gogoslick_proto_library") + +gogoslick_proto_library( + name = "google/rpc", + protos = [ + "google/rpc/code.proto", + "google/rpc/error_details.proto", + "google/rpc/status.proto", + ], + importmap = { + "google/protobuf/any.proto": "github.com/gogo/protobuf/types", + "google/protobuf/duration.proto": "github.com/gogo/protobuf/types", + }, + imports = [ + "../../external/com_github_google_protobuf/src", + ], + inputs = [ + "@com_github_google_protobuf//:well_known_protos", + ], + deps = [ + "@com_github_gogo_protobuf//types:go_default_library", + ], + verbose = 0, +) + +load("@org_pubref_rules_protobuf//cpp:rules.bzl", "cc_proto_library") + +cc_proto_library( + name = "cc_status_proto", + protos = [ + "google/rpc/status.proto", + ], + imports = [ + "../../external/com_github_google_protobuf/src", + ], + verbose = 0, +) + +filegroup( + name = "status_proto", + srcs = [ "google/rpc/status.proto" ], +) + +filegroup( + name = "code_proto", + srcs = [ "google/rpc/code.proto" ], +) +""" + native.new_git_repository( + name = "com_github_googleapis_googleapis", + build_file_content = GOOGLEAPIS_BUILD_FILE, + commit = "13ac2436c5e3d568bd0e938f6ed58b77a48aba15", # Oct 21, 2016 (only release pre-dates sha) + remote = "https://github.com/googleapis/googleapis.git", + ) + +def go_mixer_repositories(use_local_api=False): + go_istio_api_repositories(use_local_api) + go_googleapis_repositories() + + go_repositories() + proto_repositories() + + gogo_proto_repositories() + + new_go_repository( + name = "com_github_golang_glog", + commit = "23def4e6c14b4da8ac2ed8007337bc5eb5007998", # Jan 26, 2016 (no releases) + importpath = "github.com/golang/glog", + ) + + new_go_repository( + name = "com_github_ghodss_yaml", + commit = "04f313413ffd65ce25f2541bfd2b2ceec5c0908c", # Dec 6, 2016 (no releases) + importpath = "github.com/ghodss/yaml", + ) + + new_go_repository( + name = "in_gopkg_yaml_v2", + commit = "14227de293ca979cf205cd88769fe71ed96a97e2", # Jan 24, 2017 (no releases) + importpath = "gopkg.in/yaml.v2", + ) + + new_go_repository( + name = "com_github_golang_protobuf", + commit = "8ee79997227bf9b34611aee7946ae64735e6fd93", # Nov 16, 2016 (no releases) + importpath = "github.com/golang/protobuf", + ) + + new_go_repository( + name = "org_golang_google_grpc", + commit = "708a7f9f3283aa2d4f6132d287d78683babe55c8", # Dec 5, 2016 (v1.0.5) + importpath = "google.golang.org/grpc", + ) + + new_go_repository( + name = "com_github_spf13_cobra", + commit = "35136c09d8da66b901337c6e86fd8e88a1a255bd", # Jan 30, 2017 (no releases) + importpath = "github.com/spf13/cobra", + ) + + new_go_repository( + name = "com_github_spf13_pflag", + commit = "9ff6c6923cfffbcd502984b8e0c80539a94968b7", # Jan 30, 2017 (no releases) + importpath = "github.com/spf13/pflag", + ) + + new_go_repository( + name = "com_github_hashicorp_go_multierror", + commit = "ed905158d87462226a13fe39ddf685ea65f1c11f", # Dec 16, 2016 (no releases) + importpath = "github.com/hashicorp/go-multierror", + ) + + new_go_repository( + name = "com_github_hashicorp_errwrap", + commit = "7554cd9344cec97297fa6649b055a8c98c2a1e55", # Oct 27, 2014 (no releases) + importpath = "github.com/hashicorp/errwrap", + ) + + new_go_repository( + name = "com_github_opentracing_opentracing_go", + commit = "0c3154a3c2ce79d3271985848659870599dfb77c", # Sep 26, 2016 (v1.0.0) + importpath = "github.com/opentracing/opentracing-go", + ) + + new_go_repository( + name = "com_github_opentracing_basictracer", + commit = "1b32af207119a14b1b231d451df3ed04a72efebf", # Sep 29, 2016 (no releases) + importpath = "github.com/opentracing/basictracer-go", + ) + + go_repository( + name = "com_github_istio_mixer", + commit = "064001053b51f73adc3a80ff87ef41a15316c300", + importpath = "github.com/istio/mixer", + ) + diff --git a/src/envoy/mixer/integration_test/setup.go b/src/envoy/mixer/integration_test/setup.go new file mode 100644 index 00000000000..7dc093d2449 --- /dev/null +++ b/src/envoy/mixer/integration_test/setup.go @@ -0,0 +1,64 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "log" +) + +const ( + // These ports should match with used envoy.conf + // Default is using one in this folder. + ServerProxyPort = 29090 + ClientProxyPort = 27070 + MixerPort = 29091 + BackendPort = 28080 +) + +type TestSetup struct { + envoy *Envoy + mixer *MixerServer + backend *HttpServer +} + +func SetUp() (ts TestSetup, err error) { + ts.envoy, err = NewEnvoy() + if err != nil { + log.Printf("unable to create Envoy %v", err) + } else { + ts.envoy.Start() + } + + ts.mixer, err = NewMixerServer(MixerPort) + if err != nil { + log.Printf("unable to create mixer server %v", err) + } else { + ts.mixer.Start() + } + + ts.backend, err = NewHttpServer(BackendPort) + if err != nil { + log.Printf("unable to create HTTP server %v", err) + } else { + ts.backend.Start() + } + return ts, err +} + +func (ts *TestSetup) TearDown() { + ts.envoy.Stop() + ts.mixer.Stop() + ts.backend.Stop() +} From 57419f1a2a4b3d3b00bec59a69458f8911ffe8f7 Mon Sep 17 00:00:00 2001 From: wattli Date: Tue, 21 Mar 2017 13:48:11 -0700 Subject: [PATCH 16/23] Fix broken link (#193) --- src/envoy/mixer/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index 4e30eb08f56..018de6ab206 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -3,7 +3,7 @@ This Proxy will use Envoy and talk to Mixer server. ## Build Mixer server -* Follow https://github.com/istio/mixer/blob/master/doc/devel/development.md to set up environment, and build via: +* Follow https://github.com/istio/mixer/blob/master/doc/dev/development.md to set up environment, and build via: ``` cd $(ISTIO)/mixer From d2bc18a5b81af863c2313c325aa4e9d9acd9455f Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 21 Mar 2017 19:14:25 -0700 Subject: [PATCH 17/23] Make quota call (#192) * hookup quota call * Make quota call. * Update indent. --- src/envoy/mixer/README.md | 4 +- src/envoy/mixer/envoy.conf.template | 3 +- src/envoy/mixer/http_control.cc | 30 ++++++++++- src/envoy/mixer/http_control.h | 2 + src/envoy/mixer/integration_test/envoy.conf | 4 +- .../mixer/integration_test/mixer_server.go | 8 +-- .../mixer/integration_test/mixer_test.go | 52 +++++++++++++++---- src/envoy/mixer/repositories.bzl | 2 +- 8 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index 018de6ab206..5cfb5f57fcb 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -66,7 +66,8 @@ This filter will intercept all HTTP requests and call Mixer. Here is its config: "mixer_server": "${MIXER_SERVER}", "mixer_attributes" : { "attribute_name1": "attribute_value1", - "attribute_name2": "attribute_value2" + "attribute_name2": "attribute_value2", + "quota.name": "RequestCount" }, "forward_attributes" : { "attribute_name1": "attribute_value1", @@ -79,6 +80,7 @@ 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. 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: diff --git a/src/envoy/mixer/envoy.conf.template b/src/envoy/mixer/envoy.conf.template index 86aa0e1dc33..1ca8ffc62b3 100644 --- a/src/envoy/mixer/envoy.conf.template +++ b/src/envoy/mixer/envoy.conf.template @@ -42,7 +42,8 @@ "mixer_server": "${MIXER_SERVER}", "mixer_attributes": { "target.uid": "POD222", - "target.namespace": "XYZ222" + "target.namespace": "XYZ222", + "quota.name": "RequestCount" } } }, diff --git a/src/envoy/mixer/http_control.cc b/src/envoy/mixer/http_control.cc index 50a637ca356..0e015b58ba6 100644 --- a/src/envoy/mixer/http_control.cc +++ b/src/envoy/mixer/http_control.cc @@ -114,6 +114,23 @@ HttpControl::HttpControl(const std::string& mixer_server, ::istio::mixer_client::MixerClientOptions options; options.mixer_server = mixer_server; 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); + } } void HttpControl::FillCheckAttributes(HeaderMap& header_map, Attributes* attr) { @@ -141,7 +158,18 @@ void HttpControl::Check(HttpRequestDataPtr request_data, HeaderMap& headers, FillCheckAttributes(headers, &request_data->attributes); SetStringAttribute(kOriginUser, origin_user, &request_data->attributes); log().debug("Send Check: {}", request_data->attributes.DebugString()); - mixer_client_->Check(request_data->attributes, on_done); + + auto check_on_done = [this, on_done](const Status& status) { + if (status.ok()) { + if (!quota_attributes_.attributes.empty()) { + log().debug("Send Quota: {}", quota_attributes_.DebugString()); + mixer_client_->Quota(quota_attributes_, on_done); + return; // Not to call on_done again. + } + } + on_done(status); + }; + mixer_client_->Check(request_data->attributes, check_on_done); } void HttpControl::Report(HttpRequestDataPtr request_data, diff --git a/src/envoy/mixer/http_control.h b/src/envoy/mixer/http_control.h index e9ddc734f45..9405945d714 100644 --- a/src/envoy/mixer/http_control.h +++ b/src/envoy/mixer/http_control.h @@ -58,6 +58,8 @@ class HttpControl final : public Logger::Loggable { std::unique_ptr<::istio::mixer_client::MixerClient> mixer_client_; // The attributes read from the config file. std::map config_attributes_; + // Quota attributes; extracted from envoy filter config. + ::istio::mixer_client::Attributes quota_attributes_; }; } // namespace Mixer diff --git a/src/envoy/mixer/integration_test/envoy.conf b/src/envoy/mixer/integration_test/envoy.conf index 2083d7f4e5a..4d993753f8b 100644 --- a/src/envoy/mixer/integration_test/envoy.conf +++ b/src/envoy/mixer/integration_test/envoy.conf @@ -42,7 +42,9 @@ "mixer_server": "localhost:29091", "mixer_attributes": { "target.uid": "POD222", - "target.namespace": "XYZ222" + "target.namespace": "XYZ222", + "quota.name": "RequestCount", + "quota.amount": "5" } } }, diff --git a/src/envoy/mixer/integration_test/mixer_server.go b/src/envoy/mixer/integration_test/mixer_server.go index e36f04b484a..fbe5a5bf9c5 100644 --- a/src/envoy/mixer/integration_test/mixer_server.go +++ b/src/envoy/mixer/integration_test/mixer_server.go @@ -56,9 +56,10 @@ type MixerServer struct { gp *pool.GoroutinePool s mixerpb.MixerServer - check *Handler - report *Handler - quota *Handler + check *Handler + report *Handler + quota *Handler + quota_request *mixerpb.QuotaRequest } func (ts *MixerServer) Check(ctx context.Context, bag *attribute.MutableBag, @@ -76,6 +77,7 @@ func (ts *MixerServer) Report(ctx context.Context, bag *attribute.MutableBag, func (ts *MixerServer) Quota(ctx context.Context, bag *attribute.MutableBag, request *mixerpb.QuotaRequest, response *mixerpb.QuotaResponse) { response.RequestIndex = request.RequestIndex + ts.quota_request = request response.Result = ts.quota.run(bag) response.Amount = 0 } diff --git a/src/envoy/mixer/integration_test/mixer_test.go b/src/envoy/mixer/integration_test/mixer_test.go index b8f382bd58c..862391f873a 100644 --- a/src/envoy/mixer/integration_test/mixer_test.go +++ b/src/envoy/mixer/integration_test/mixer_test.go @@ -22,7 +22,8 @@ import ( ) const ( - mixerFailMessage = "Unauthenticated by mixer." + mixerAuthFailMessage = "Unauthenticated by mixer." + mixerQuotaFailMessage = "Not enough quota by mixer." ) // Attributes verification rules @@ -134,14 +135,14 @@ const reportAttributesOkPost = ` }, "request.size": 12, "response.time": "*", - "response.size": 12, + "response.size": 45, "response.latency": "*", - "response.http.code": 200, + "response.http.code": 429, "response.headers": { "date": "*", "content-type": "text/plain", - "content-length": "12", - ":status": "200", + "content-length": "45", + ":status": "429", "server": "envoy" } } @@ -262,6 +263,7 @@ func verifyAttributes( t.Fatalf("Failed to verify %s check: %v\n, Attributes: %+v", tag, err, s.mixer.check.bag) } + _ = <-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", @@ -269,6 +271,18 @@ func verifyAttributes( } } +func verifyQuota(s *TestSetup, tag string, t *testing.T) { + _ = <-s.mixer.quota.ch + if s.mixer.quota_request.Quota != "RequestCount" { + t.Fatalf("Failed to verify %s quota name (=RequestCount): %v\n", + tag, s.mixer.quota_request.Quota) + } + if s.mixer.quota_request.Amount != 5 { + t.Fatalf("Failed to verify %s quota amount (=5): %v\n", + tag, s.mixer.quota_request.Amount) + } +} + func TestMixer(t *testing.T) { s, err := SetUp() if err != nil { @@ -288,20 +302,36 @@ func TestMixer(t *testing.T) { } verifyAttributes(&s, "OkGet", checkAttributesOkGet, reportAttributesOkGet, t) + verifyQuota(&s, "OkGet", t) - // Issues a POST echo request with - if _, _, err := HTTPPost(url, "text/plain", "Hello World!"); err != nil { + // Issues a failed POST request caused by Mixer Quota + s.mixer.quota.r_status = rpc.Status{ + Code: int32(rpc.RESOURCE_EXHAUSTED), + Message: mixerQuotaFailMessage, + } + code, resp_body, err := HTTPPost(url, "text/plain", "Hello World!") + // 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) } + if code != 429 { + t.Errorf("Status code 429 is expected.") + } + if resp_body != "RESOURCE_EXHAUSTED:"+mixerQuotaFailMessage { + t.Errorf("Error response body is not expected.") + } verifyAttributes(&s, "OkPost", checkAttributesOkPost, reportAttributesOkPost, t) + verifyQuota(&s, "OkPost", t) // Issues a failed request caused by mixer s.mixer.check.r_status = rpc.Status{ Code: int32(rpc.UNAUTHENTICATED), - Message: mixerFailMessage, + Message: mixerAuthFailMessage, } - code, resp_body, err := HTTPGet(url) + code, resp_body, err = HTTPGet(url) + // 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) @@ -309,11 +339,12 @@ func TestMixer(t *testing.T) { if code != 401 { t.Errorf("Status code 401 is expected.") } - if resp_body != "UNAUTHENTICATED:"+mixerFailMessage { + if resp_body != "UNAUTHENTICATED:"+mixerAuthFailMessage { t.Errorf("Error response body is not expected.") } verifyAttributes(&s, "MixerFail", checkAttributesMixerFail, reportAttributesMixerFail, t) + // Not quota call due to Mixer failure. // Issues a failed request caused by backend headers := map[string]string{} @@ -330,4 +361,5 @@ func TestMixer(t *testing.T) { } verifyAttributes(&s, "BackendFail", checkAttributesBackendFail, reportAttributesBackendFail, t) + verifyQuota(&s, "BackendFail", t) } diff --git a/src/envoy/mixer/repositories.bzl b/src/envoy/mixer/repositories.bzl index 9d06b24a14b..7edd7e89834 100644 --- a/src/envoy/mixer/repositories.bzl +++ b/src/envoy/mixer/repositories.bzl @@ -15,7 +15,7 @@ ################################################################################ # -MIXER_CLIENT = "1d6b587755846fe1b3a44fb53e3ab9ce0534af2c" +MIXER_CLIENT = "b76b5131c4650cefff4af7e4267883a33d66bca1" def mixer_client_repositories(bind=True): native.git_repository( From 61a53a46cff95afb77fda6b58e128ede73f54e06 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Wed, 22 Mar 2017 16:41:28 -0700 Subject: [PATCH 18/23] Update envoy and update configs (#195) * Update envoy and update configs * Use gcc-4.9 for travis * Use bazel 0.4.5 * Fix SHA of lightstep-tracer-common --- .travis.yml | 26 ++++++----- src/envoy/mixer/envoy.conf.template | 6 +-- src/envoy/mixer/integration_test/envoy.conf | 6 +-- src/envoy/repositories.bzl | 52 ++++++++------------- 4 files changed, 39 insertions(+), 51 deletions(-) diff --git a/.travis.yml b/.travis.yml index 418e24d2dee..b2c2e63a881 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,25 +1,27 @@ sudo: required -dist: xenial + +dist: trusty + +addons: + apt: + sources: + - ubuntu-toolchain-r-test + packages: + - gcc-4.9 + - g++-4.9 + - wget branches: except: - stable -lang: go - -go: - - 1.7.x +language: cpp jdk: - oraclejdk8 env: - - BAZEL_VERSION=0.4.3 - -addons: - apt: - packages: - - wget + - BAZEL_VERSION=0.4.5 cache: directories: @@ -39,7 +41,7 @@ before_install: script: - script/check-style - - bazel --output_base=${HOME}/bazel/outbase test //... + - CC=/usr/bin/gcc-4.9 CXX=/usr/bin/g++-4.9 bazel --output_base=${HOME}/bazel/outbase test //... notifications: slack: istio-dev:wEEEbaabdP5ieCgDOFetA9nX diff --git a/src/envoy/mixer/envoy.conf.template b/src/envoy/mixer/envoy.conf.template index 1ca8ffc62b3..6e20da737ba 100644 --- a/src/envoy/mixer/envoy.conf.template +++ b/src/envoy/mixer/envoy.conf.template @@ -1,7 +1,7 @@ { "listeners": [ { - "port": ${PORT}, + "address": "tcp://0.0.0.0:${PORT}", "bind_to_port": true, "filters": [ { @@ -58,7 +58,7 @@ ] }, { - "port": 7070, + "address": "tcp://0.0.0.0:7070", "bind_to_port": true, "filters": [ { @@ -112,7 +112,7 @@ ], "admin": { "access_log_path": "/dev/stdout", - "port": 9001 + "address": "tcp://0.0.0.0:9001" }, "cluster_manager": { "clusters": [ diff --git a/src/envoy/mixer/integration_test/envoy.conf b/src/envoy/mixer/integration_test/envoy.conf index 4d993753f8b..4374ec1b4ea 100644 --- a/src/envoy/mixer/integration_test/envoy.conf +++ b/src/envoy/mixer/integration_test/envoy.conf @@ -1,7 +1,7 @@ { "listeners": [ { - "port": 29090, + "address": "tcp://0.0.0.0:29090", "bind_to_port": true, "filters": [ { @@ -59,7 +59,7 @@ ] }, { - "port": 27070, + "address": "tcp://0.0.0.0:27070", "bind_to_port": true, "filters": [ { @@ -113,7 +113,7 @@ ], "admin": { "access_log_path": "/dev/stdout", - "port": 29001 + "address": "tcp://0.0.0.0:29001" }, "cluster_manager": { "clusters": [ diff --git a/src/envoy/repositories.bzl b/src/envoy/repositories.bzl index 5a74586ddc8..bb150928ba0 100644 --- a/src/envoy/repositories.bzl +++ b/src/envoy/repositories.bzl @@ -235,25 +235,6 @@ def lightstep_repositories(bind=True): BUILD = """ load("@protobuf_git//:protobuf.bzl", "cc_proto_library") -genrule( - name = "envoy_carrier_pb", - srcs = ["src/c++11/envoy/envoy_carrier.proto"], - outs = ["lightstep/envoy_carrier.proto"], - cmd = "cp $(SRCS) $@", -) - -cc_proto_library( - name = "envoy_carrier_proto", - srcs = ["lightstep/envoy_carrier.proto"], - include = ".", - deps = [ - "//external:cc_wkt_protos", - ], - protoc = "//external:protoc", - default_runtime = "//external:protobuf", - visibility = ["//visibility:public"], -) - cc_library( name = "lightstep_core", srcs = [ @@ -266,7 +247,7 @@ cc_library( "src/c++11/lightstep/impl.h", "src/c++11/lightstep/options.h", "src/c++11/lightstep/propagation.h", - "src/c++11/lightstep/envoy.h", + "src/c++11/lightstep/carrier.h", "src/c++11/lightstep/span.h", "src/c++11/lightstep/tracer.h", "src/c++11/lightstep/util.h", @@ -275,7 +256,7 @@ cc_library( "src/c++11/mapbox_variant/variant.hpp", ], copts = [ - "-DPACKAGE_VERSION='\\"0.19\\"'", + "-DPACKAGE_VERSION='\\"0.36\\"'", "-Iexternal/lightstep_git/src/c++11/lightstep", "-Iexternal/lightstep_git/src/c++11/mapbox_variant", ], @@ -285,7 +266,7 @@ cc_library( visibility = ["//visibility:public"], deps = [ "@lightstep_common_git//:collector_proto", - ":envoy_carrier_proto", + "@lightstep_common_git//:lightstep_carrier_proto", "//external:protobuf", ], )""" @@ -293,16 +274,21 @@ cc_library( COMMON_BUILD = """ load("@protobuf_git//:protobuf.bzl", "cc_proto_library") -genrule( - name = "collector_pb", +cc_proto_library( + name = "collector_proto", srcs = ["collector.proto"], - outs = ["lightstep/collector.proto"], - cmd = "cp $(SRCS) $@", + include = ".", + deps = [ + "//external:cc_wkt_protos", + ], + protoc = "//external:protoc", + default_runtime = "//external:protobuf", + visibility = ["//visibility:public"], ) cc_proto_library( - name = "collector_proto", - srcs = ["lightstep/collector.proto"], + name = "lightstep_carrier_proto", + srcs = ["lightstep_carrier.proto"], include = ".", deps = [ "//external:cc_wkt_protos", @@ -310,19 +296,20 @@ cc_proto_library( protoc = "//external:protoc", default_runtime = "//external:protobuf", visibility = ["//visibility:public"], -)""" +) +""" native.new_git_repository( name = "lightstep_common_git", remote = "https://github.com/lightstep/lightstep-tracer-common.git", - commit = "8d932f7f76cd286691e6179621d0012b0ff1e6aa", + commit = "cbbecd671c1ae1f20ae873c5da688c8c14d04ec3", build_file_content = COMMON_BUILD, ) native.new_git_repository( name = "lightstep_git", remote = "https://github.com/lightstep/lightstep-tracer-cpp.git", - commit = "5a71d623cac17a059041b04fabca4ed86ffff7cc", + commit = "f1dc8f3dfd529350e053fd21273e627f409ae428", # 0.36 build_file_content = BUILD, ) @@ -751,7 +738,6 @@ cc_test( native.new_git_repository( name = "envoy_git", remote = "https://github.com/lyft/envoy.git", - commit = "b72309da41fba0c1222a72262b83bedc7294df65", # Mar 13 2017 + commit = "09f078b016da908ba8b861857f2a12a43933ba40", # Mar 21 2017 build_file_content = BUILD, ) - From 5e601ca3646135d3027568a2c8927e8f291628fc Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Thu, 23 Mar 2017 12:48:29 -0700 Subject: [PATCH 19/23] Enable check cache and refactory mixer config loading (#197) * Refactory the mixer config loading. * fix format * Add integration test. * updated README.md * s/send/sent/ --- src/envoy/mixer/BUILD | 2 + src/envoy/mixer/README.md | 37 +++++-- src/envoy/mixer/config.cc | 96 +++++++++++++++++ src/envoy/mixer/config.h | 53 +++++++++ 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 | 12 ++- .../mixer/integration_test/mixer_server.go | 3 + .../mixer/integration_test/mixer_test.go | 101 +++++++----------- src/envoy/mixer/repositories.bzl | 2 +- src/envoy/mixer/utils.cc | 14 --- src/envoy/mixer/utils.h | 3 - 14 files changed, 261 insertions(+), 147 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..493b2d185cf 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,17 +70,25 @@ 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" + ] } ``` 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. +* 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. -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,17 @@ 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. + + +## 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 sent to the upstream proxy (the server proxy). If the server proxy is calling mixer, these attributes will be sent to the mixer. 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..ee301aba2b5 --- /dev/null +++ b/src/envoy/mixer/config.h @@ -0,0 +1,53 @@ +/* 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..1e9bd153893 100644 --- a/src/envoy/mixer/integration_test/envoy.conf +++ b/src/envoy/mixer/integration_test/envoy.conf @@ -42,10 +42,14 @@ "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", + "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) } 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 3bcb6c4096ce672798f4d544568547b95fcc4bb2 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 24 Mar 2017 12:25:48 -0700 Subject: [PATCH 20/23] Split into separate tests. (#201) --- src/envoy/mixer/integration_test/BUILD | 30 +- .../mixer/integration_test/attributes.go | 16 +- .../integration_test/check_cache_test.go | 40 +++ .../integration_test/check_report_test.go | 154 ++++++++ src/envoy/mixer/integration_test/envoy.go | 35 +- .../{envoy.conf => envoy_conf.go} | 138 +++++-- .../integration_test/failed_request_test.go | 161 +++++++++ .../mixer/integration_test/mixer_server.go | 2 +- .../mixer/integration_test/mixer_test.go | 340 ------------------ .../mixer/integration_test/quota_test.go | 64 ++++ src/envoy/mixer/integration_test/setup.go | 71 ++-- .../mixer/integration_test/test_suite.bzl | 29 ++ 12 files changed, 659 insertions(+), 421 deletions(-) create mode 100644 src/envoy/mixer/integration_test/check_cache_test.go create mode 100644 src/envoy/mixer/integration_test/check_report_test.go rename src/envoy/mixer/integration_test/{envoy.conf => envoy_conf.go} (56%) create mode 100644 src/envoy/mixer/integration_test/failed_request_test.go delete mode 100644 src/envoy/mixer/integration_test/mixer_test.go create mode 100644 src/envoy/mixer/integration_test/quota_test.go create mode 100644 src/envoy/mixer/integration_test/test_suite.bzl diff --git a/src/envoy/mixer/integration_test/BUILD b/src/envoy/mixer/integration_test/BUILD index c45865457ce..a6ff4916db5 100644 --- a/src/envoy/mixer/integration_test/BUILD +++ b/src/envoy/mixer/integration_test/BUILD @@ -1,4 +1,4 @@ -# Copyright 2016 Google Inc. All Rights Reserved. +# 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. @@ -15,13 +15,15 @@ ################################################################################ # -load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +load("@io_bazel_rules_go//go:def.bzl", "go_library") +load(":test_suite.bzl", "go_test_suite") go_library( name = "go_default_library", srcs = [ "attributes.go", "envoy.go", + "envoy_conf.go", "http_client.go", "http_server.go", "mixer_server.go", @@ -41,18 +43,22 @@ go_library( ], ) -go_test( - name = "mixer_test", - size = "small", - srcs = [ - "mixer_test.go", - ], +go_test_suite( data = [ - "envoy.conf", "//src/envoy/mixer:envoy", ], library = ":go_default_library", - # shared memory path /envoy_shared_memory_0 used by Envoy - # hot start is not working in sandbox mode. - local = True, + tags = [ + # Use fixed ports, not in sanbbox, have to be run exclusively. + "exclusive", + # shared memory path /envoy_shared_memory_0 used by Envoy + # hot start is not working in sandbox mode. + "local", + ], + tests = [ + "check_cache_test.go", + "check_report_test.go", + "failed_request_test.go", + "quota_test.go", + ], ) diff --git a/src/envoy/mixer/integration_test/attributes.go b/src/envoy/mixer/integration_test/attributes.go index 4c97208fd4b..6aecd20d8bb 100644 --- a/src/envoy/mixer/integration_test/attributes.go +++ b/src/envoy/mixer/integration_test/attributes.go @@ -45,7 +45,21 @@ func verifyStringMap(actual map[string]string, expected map[string]interface{}) return nil } -// Please see the comment at top of mixer_test.go for verification rules +// Attributes verification rules: +// +// 1) If value is *, key must exist, but value is not checked. +// 1) If value is -, key must NOT exist. +// 3) At top level attributes, not inside StringMap, all keys must +// be listed. Extra keys are NOT allowed +// 3) Inside StringMap, not need to list all keys. Extra keys are allowed +// +// Attributes provided from envoy config +// * source.id and source.namespace are forwarded from client proxy +// * target.id and target.namespace are from server proxy +// +// HTTP header "x-istio-attributes" is used to forward attributes between +// proxy. It should be removed before calling mixer and backend. +// func Verify(b *attribute.MutableBag, json_results string) error { var r map[string]interface{} if err := json.Unmarshal([]byte(json_results), &r); err != nil { diff --git a/src/envoy/mixer/integration_test/check_cache_test.go b/src/envoy/mixer/integration_test/check_cache_test.go new file mode 100644 index 00000000000..ab79320ae82 --- /dev/null +++ b/src/envoy/mixer/integration_test/check_cache_test.go @@ -0,0 +1,40 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "fmt" + "testing" +) + +func TestCheckCache(t *testing.T) { + s, err := SetUp(t, basicConfig+","+checkCacheConfig) + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + defer s.TearDown() + + url := fmt.Sprintf("http://localhost:%d/echo", ClientProxyPort) + + // Issues a GET echo request with 0 size body + tag := "OKGet" + for i := 0; i < 10; i++ { + if _, _, err := HTTPGet(url); err != nil { + t.Errorf("Failed in request %s: %v", tag, err) + } + // Only the first check is called. + s.VerifyCheckCount(tag, 1) + } +} diff --git a/src/envoy/mixer/integration_test/check_report_test.go b/src/envoy/mixer/integration_test/check_report_test.go new file mode 100644 index 00000000000..71d8a3f6bb9 --- /dev/null +++ b/src/envoy/mixer/integration_test/check_report_test.go @@ -0,0 +1,154 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "fmt" + "testing" +) + +// Check attributes from a good GET request +const checkAttributesOkGet = ` +{ + "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 good GET request +const reportAttributesOkGet = ` +{ + "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": "*" + }, + "request.size": 0, + "response.time": "*", + "response.size": 0, + "response.latency": "*", + "response.http.code": 200, + "response.headers": { + "date": "*", + "content-type": "text/plain; charset=utf-8", + "content-length": "0", + ":status": "200", + "server": "envoy" + } +} +` + +// 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 = ` +{ + "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": "*" + }, + "request.size": 12, + "response.time": "*", + "response.size": 12, + "response.latency": "*", + "response.http.code": 200, + "response.headers": { + "date": "*", + "content-type": "text/plain", + "content-length": "12", + ":status": "200", + "server": "envoy" + } +} +` + +func TestCheckReportAttributes(t *testing.T) { + s, err := SetUp(t, basicConfig) + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + defer s.TearDown() + + 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 request %s: %v", tag, err) + } + s.VerifyCheck(tag, checkAttributesOkGet) + s.VerifyReport(tag, reportAttributesOkGet) + + // Issues a POST request. + tag = "OKPost" + if _, _, err := HTTPPost(url, "text/plain", "Hello World!"); err != nil { + t.Errorf("Failed in request %s: %v", tag, err) + } + s.VerifyCheck(tag, checkAttributesOkPost) + s.VerifyReport(tag, reportAttributesOkPost) +} diff --git a/src/envoy/mixer/integration_test/envoy.go b/src/envoy/mixer/integration_test/envoy.go index 0f9a1fc01c7..b34fb5c240a 100644 --- a/src/envoy/mixer/integration_test/envoy.go +++ b/src/envoy/mixer/integration_test/envoy.go @@ -39,24 +39,6 @@ func getTestBinRootPath() string { } } -func getTestDataRootPath() string { - switch { - // custom path - case os.Getenv("TEST_DATA_ROOT") != "": - return os.Getenv("TEST_DATA_ROOT") - // running under bazel - case os.Getenv("TEST_SRCDIR") != "": - return os.Getenv("TEST_SRCDIR") + "/__main__" - // running with native go - case os.Getenv("GOPATH") != "": - list := strings.Split(os.Getenv("GOPATH"), - string(os.PathListSeparator)) - return list[0] - default: - return "" - } -} - type Envoy struct { cmd *exec.Cmd } @@ -76,14 +58,17 @@ func Run(name string, args ...string) (s string, err error) { return } -func NewEnvoy() (*Envoy, error) { - path := getTestBinRootPath() + "/src/envoy/mixer/envoy" - conf := getTestDataRootPath() + - "/src/envoy/mixer/integration_test/envoy.conf" - log.Printf("Envoy binary: %v\n", path) - log.Printf("Envoy config: %v\n", conf) +func NewEnvoy(conf string) (*Envoy, error) { + bin_path := getTestBinRootPath() + "/src/envoy/mixer/envoy" + log.Printf("Envoy binary: %v\n", bin_path) + + conf_path := "/tmp/envoy.conf" + log.Printf("Envoy config: in %v\n%v\n", conf_path, conf) + if err := CreateEnvoyConf(conf_path, conf); err != nil { + return nil, err + } - cmd := exec.Command(path, "-c", conf, "-l", "debug") + cmd := exec.Command(bin_path, "-c", conf_path, "-l", "debug") cmd.Stderr = os.Stderr cmd.Stdout = os.Stdout return &Envoy{ diff --git a/src/envoy/mixer/integration_test/envoy.conf b/src/envoy/mixer/integration_test/envoy_conf.go similarity index 56% rename from src/envoy/mixer/integration_test/envoy.conf rename to src/envoy/mixer/integration_test/envoy_conf.go index 1e9bd153893..166b11543aa 100644 --- a/src/envoy/mixer/integration_test/envoy.conf +++ b/src/envoy/mixer/integration_test/envoy_conf.go @@ -1,7 +1,83 @@ +// 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. +// + +package test + +import ( + "fmt" + "os" + "text/template" +) + +const ( + // These ports should match with used envoy.conf + // Default is using one in this folder. + ServerProxyPort = 29090 + ClientProxyPort = 27070 + MixerPort = 29091 + BackendPort = 28080 + AdminPort = 29001 +) + +type ConfParam struct { + ClientPort int + ServerPort int + AdminPort int + MixerServer string + Backend string + ClientConfig string + ServerConfig string +} + +// A basic config +const basicConfig = ` + "mixer_attributes": { + "target.uid": "POD222", + "target.namespace": "XYZ222" + } +` + +// A config with quota +const quotaConfig = ` + "quota_name": "RequestCount", + "quota_amount": "5" +` + +// A config with check cache keys +const checkCacheConfig = ` + "check_cache_keys": [ + "request.host", + "request.path", + "origin.user" + ] +` + +// The default client proxy mixer config +const defaultClientMixerConfig = ` + "forward_attributes": { + "source.uid": "POD11", + "source.namespace": "XYZ11" + } +` + +// The envoy config template +const envoyConfTempl = ` { "listeners": [ { - "address": "tcp://0.0.0.0:29090", + "address": "tcp://0.0.0.0:{{.ServerPort}}", "bind_to_port": true, "filters": [ { @@ -39,17 +115,8 @@ "type": "decoder", "name": "mixer", "config": { - "mixer_server": "localhost:29091", - "mixer_attributes": { - "target.uid": "POD222", - "target.namespace": "XYZ222" - }, - "quota_name": "RequestCount", - "quota_amount": "5", - "check_cache_keys": [ - "request.host", - "request.path" - ] + "mixer_server": "{{.MixerServer}}", +{{.ServerConfig}} } }, { @@ -63,7 +130,7 @@ ] }, { - "address": "tcp://0.0.0.0:27070", + "address": "tcp://0.0.0.0:{{.ClientPort}}", "bind_to_port": true, "filters": [ { @@ -97,11 +164,8 @@ "type": "decoder", "name": "mixer", "config": { - "mixer_server": "localhost:29091", - "forward_attributes": { - "source.uid": "POD11", - "source.namespace": "XYZ11" - } + "mixer_server": "{{.MixerServer}}", +{{.ClientConfig}} } }, { @@ -117,7 +181,7 @@ ], "admin": { "access_log_path": "/dev/stdout", - "address": "tcp://0.0.0.0:29001" + "address": "tcp://0.0.0.0:{{.AdminPort}}" }, "cluster_manager": { "clusters": [ @@ -128,7 +192,7 @@ "lb_type": "round_robin", "hosts": [ { - "url": "tcp://localhost:28080" + "url": "tcp://{{.Backend}}" } ] }, @@ -139,10 +203,42 @@ "lb_type": "round_robin", "hosts": [ { - "url": "tcp://localhost:29090" + "url": "tcp://localhost:{{.ServerPort}}" } ] } ] } } +` + +func (c *ConfParam) write(path string) error { + tmpl, err := template.New("test").Parse(envoyConfTempl) + if err != nil { + return fmt.Errorf("Failed to parse config template: %v", err) + } + + f, err := os.Create(path) + if err != nil { + return fmt.Errorf("Failed to create file %v: %v", path, err) + } + defer f.Close() + return tmpl.Execute(f, *c) +} + +func getConf() ConfParam { + return ConfParam{ + ClientPort: ClientProxyPort, + ServerPort: ServerProxyPort, + AdminPort: AdminPort, + MixerServer: fmt.Sprintf("localhost:%d", MixerPort), + Backend: fmt.Sprintf("localhost:%d", BackendPort), + ClientConfig: defaultClientMixerConfig, + } +} + +func CreateEnvoyConf(path string, conf string) error { + c := getConf() + c.ServerConfig = conf + return c.write(path) +} diff --git a/src/envoy/mixer/integration_test/failed_request_test.go b/src/envoy/mixer/integration_test/failed_request_test.go new file mode 100644 index 00000000000..9ae99bcf81d --- /dev/null +++ b/src/envoy/mixer/integration_test/failed_request_test.go @@ -0,0 +1,161 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "fmt" + "testing" + + rpc "github.com/googleapis/googleapis/google/rpc" +) + +const ( + mixerAuthFailMessage = "Unauthenticated by mixer." +) + +// Check attributes from a fail GET request from mixer +const checkAttributesMixerFail = ` +{ + "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 mixer +const reportAttributesMixerFail = ` +{ + "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": "*" + }, + "request.size": 0, + "response.time": "*", + "response.size": 41, + "response.latency": "*", + "response.http.code": 401, + "response.headers": { + "date": "*", + "content-type": "text/plain", + "content-length": "41", + ":status": "401", + "server": "envoy" + } +} +` + +// Report attributes from a fail GET request from backend +const reportAttributesBackendFail = ` +{ + "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": "*" + }, + "request.size": 0, + "response.time": "*", + "response.size": 25, + "response.latency": "*", + "response.http.code": 400, + "response.headers": { + "date": "*", + "content-type": "text/plain; charset=utf-8", + "content-length": "25", + ":status": "400", + "server": "envoy" + } +} +` + +func TestFailedRequest(t *testing.T) { + s, err := SetUp(t, basicConfig) + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + defer s.TearDown() + + url := fmt.Sprintf("http://localhost:%d/echo", ClientProxyPort) + + tag := "MixerFail" + s.mixer.check.r_status = rpc.Status{ + Code: int32(rpc.UNAUTHENTICATED), + Message: mixerAuthFailMessage, + } + code, resp_body, err := HTTPGet(url) + // Make sure to restore r_status for next request. + s.mixer.check.r_status = rpc.Status{} + if err != nil { + t.Errorf("Failed in request %s: %v", tag, err) + } + if code != 401 { + t.Errorf("Status code 401 is expected.") + } + if resp_body != "UNAUTHENTICATED:"+mixerAuthFailMessage { + t.Errorf("Error response body is not expected.") + } + s.VerifyCheck(tag, checkAttributesMixerFail) + s.VerifyReport(tag, reportAttributesMixerFail) + + // Issues a failed request caused by backend + tag = "BackendFail" + headers := map[string]string{} + headers[FailHeader] = "Yes" + code, resp_body, err = HTTPGetWithHeaders(url, headers) + if err != nil { + t.Errorf("Failed in request %s: %v", tag, err) + } + if code != 400 { + t.Errorf("Status code 400 is expected.") + } + if resp_body != FailBody { + t.Errorf("Error response body is not expected.") + } + // Same Check attributes as the first one. + s.VerifyCheck(tag, checkAttributesMixerFail) + s.VerifyReport(tag, reportAttributesBackendFail) +} diff --git a/src/envoy/mixer/integration_test/mixer_server.go b/src/envoy/mixer/integration_test/mixer_server.go index 3e851274819..6e3db085e5d 100644 --- a/src/envoy/mixer/integration_test/mixer_server.go +++ b/src/envoy/mixer/integration_test/mixer_server.go @@ -40,7 +40,7 @@ type Handler struct { func newHandler() *Handler { return &Handler{ bag: nil, - ch: make(chan int, 1), + ch: make(chan int, 10), // Allow maximum 10 requests count: 0, r_status: rpc.Status{}, } diff --git a/src/envoy/mixer/integration_test/mixer_test.go b/src/envoy/mixer/integration_test/mixer_test.go deleted file mode 100644 index 99cef6501b3..00000000000 --- a/src/envoy/mixer/integration_test/mixer_test.go +++ /dev/null @@ -1,340 +0,0 @@ -// Copyright 2017 Istio Authors -// -// 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 test - -import ( - "fmt" - "testing" - - rpc "github.com/googleapis/googleapis/google/rpc" -) - -const ( - mixerAuthFailMessage = "Unauthenticated by mixer." - mixerQuotaFailMessage = "Not enough quota by mixer." -) - -// Attributes verification rules -// 1) If value is *, key must exist, but value is not checked. -// 1) If value is -, key must NOT exist. -// 3) At top level attributes, not inside StringMap, all keys must -// be listed. Extra keys are NOT allowed -// 3) Inside StringMap, not need to list all keys. Extra keys are allowed -// -// Attributes provided from envoy config -// * source.id and source.namespace are forwarded from client proxy -// * target.id and target.namespace are from server proxy -// -// HTTP header "x-istio-attributes" is used to forward attributes between -// proxy. It should be removed before calling mixer and backend. -// -// Check attributes from a good GET request -const checkAttributesOkGet = ` -{ - "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 good GET request -const reportAttributesOkGet = ` -{ - "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": "*" - }, - "request.size": 0, - "response.time": "*", - "response.size": 0, - "response.latency": "*", - "response.http.code": 200, - "response.headers": { - "date": "*", - "content-type": "text/plain; charset=utf-8", - "content-length": "0", - ":status": "200", - "server": "envoy" - } -} -` - -// Report attributes from a good POST request -const reportAttributesOkPost = ` -{ - "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": "*" - }, - "request.size": 12, - "response.time": "*", - "response.size": 45, - "response.latency": "*", - "response.http.code": 429, - "response.headers": { - "date": "*", - "content-type": "text/plain", - "content-length": "45", - ":status": "429", - "server": "envoy" - } -} -` - -// Check attributes from a fail GET request from mixer -const checkAttributesMixerFail = ` -{ - "request.host": "localhost:27070", - "request.path": "/echo111", - "request.time": "*", - "source.uid": "POD11", - "source.namespace": "XYZ11", - "target.uid": "POD222", - "target.namespace": "XYZ222", - "request.headers": { - ":method": "GET", - ":path": "/echo111", - ":authority": "localhost:27070", - "x-forwarded-proto": "http", - "x-istio-attributes": "-", - "x-request-id": "*" - } -} -` - -// Report attributes from a fail GET request from mixer -const reportAttributesMixerFail = ` -{ - "request.host": "localhost:27070", - "request.path": "/echo111", - "request.time": "*", - "source.uid": "POD11", - "source.namespace": "XYZ11", - "target.uid": "POD222", - "target.namespace": "XYZ222", - "request.headers": { - ":method": "GET", - ":path": "/echo111", - ":authority": "localhost:27070", - "x-forwarded-proto": "http", - "x-istio-attributes": "-", - "x-request-id": "*" - }, - "request.size": 0, - "response.time": "*", - "response.size": 41, - "response.latency": "*", - "response.http.code": 401, - "response.headers": { - "date": "*", - "content-type": "text/plain", - "content-length": "41", - ":status": "401", - "server": "envoy" - } -} -` - -// Report attributes from a fail GET request from backend -const reportAttributesBackendFail = ` -{ - "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": "*" - }, - "request.size": 0, - "response.time": "*", - "response.size": 25, - "response.latency": "*", - "response.http.code": 400, - "response.headers": { - "date": "*", - "content-type": "text/plain; charset=utf-8", - "content-length": "25", - ":status": "400", - "server": "envoy" - } -} -` - -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", - tag, err, s.mixer.report.bag) - } -} - -func verifyQuota(s *TestSetup, tag string, t *testing.T) { - _ = <-s.mixer.quota.ch - if s.mixer.quota_request.Quota != "RequestCount" { - t.Fatalf("Failed to verify %s quota name (=RequestCount): %v\n", - tag, s.mixer.quota_request.Quota) - } - if s.mixer.quota_request.Amount != 5 { - t.Fatalf("Failed to verify %s quota amount (=5): %v\n", - tag, s.mixer.quota_request.Amount) - } -} - -func TestMixer(t *testing.T) { - s, err := SetUp() - if err != nil { - t.Fatalf("Failed to setup test: %v", err) - } - defer s.TearDown() - - // There is a client proxy with filter "forward_attribute" - // and a server proxy with filter "mixer" calling mixer. - // This request will connect to client proxy, to server proxy - // and to the backend. - 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 request %s: %v", tag, err) - } - 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, - } - code, resp_body, err := HTTPPost(url, "text/plain", "Hello World!") - // Make sure to restore r_status for next request. - s.mixer.quota.r_status = rpc.Status{} - if err != nil { - t.Errorf("Failed in request %s: %v", tag, err) - } - if code != 429 { - t.Errorf("Status code 429 is expected.") - } - if resp_body != "RESOURCE_EXHAUSTED:"+mixerQuotaFailMessage { - t.Errorf("Error response body is not expected.") - } - // 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, - } - code, resp_body, err = HTTPGet(url) - // Make sure to restore r_status for next request. - s.mixer.check.r_status = rpc.Status{} - if err != nil { - t.Errorf("Failed in request %s: %v", tag, err) - } - if code != 401 { - t.Errorf("Status code 401 is expected.") - } - if resp_body != "UNAUTHENTICATED:"+mixerAuthFailMessage { - t.Errorf("Error response body is not expected.") - } - 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 request %s: %v", tag, err) - } - if code != 400 { - t.Errorf("Status code 400 is expected.") - } - if resp_body != FailBody { - t.Errorf("Error response body is not expected.") - } - verifyCheckCount(&s, tag, 2, t) - verifyReportAttributes(&s, tag, reportAttributesBackendFail, t) - verifyQuota(&s, tag, t) -} diff --git a/src/envoy/mixer/integration_test/quota_test.go b/src/envoy/mixer/integration_test/quota_test.go new file mode 100644 index 00000000000..e99afb8e426 --- /dev/null +++ b/src/envoy/mixer/integration_test/quota_test.go @@ -0,0 +1,64 @@ +// Copyright 2017 Istio Authors +// +// 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 test + +import ( + "fmt" + "testing" + + rpc "github.com/googleapis/googleapis/google/rpc" +) + +const ( + mixerQuotaFailMessage = "Not enough quota by mixer." +) + +func TestQuotaCall(t *testing.T) { + s, err := SetUp(t, basicConfig+","+quotaConfig) + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + defer s.TearDown() + + 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 request %s: %v", tag, err) + } + s.VerifyQuota(tag, "RequestCount", 5) + + // Issues a failed POST request caused by Mixer Quota + tag = "QuotaFail" + s.mixer.quota_request = nil + s.mixer.quota.r_status = rpc.Status{ + Code: int32(rpc.RESOURCE_EXHAUSTED), + Message: mixerQuotaFailMessage, + } + code, resp_body, err := HTTPPost(url, "text/plain", "Hello World!") + // Make sure to restore r_status for next request. + s.mixer.quota.r_status = rpc.Status{} + if err != nil { + t.Errorf("Failed in request %s: %v", tag, err) + } + if code != 429 { + t.Errorf("Status code 429 is expected.") + } + if resp_body != "RESOURCE_EXHAUSTED:"+mixerQuotaFailMessage { + t.Errorf("Error response body is not expected.") + } + s.VerifyQuota(tag, "RequestCount", 5) +} diff --git a/src/envoy/mixer/integration_test/setup.go b/src/envoy/mixer/integration_test/setup.go index 7dc093d2449..52bfdd2f711 100644 --- a/src/envoy/mixer/integration_test/setup.go +++ b/src/envoy/mixer/integration_test/setup.go @@ -16,49 +16,78 @@ package test import ( "log" -) - -const ( - // These ports should match with used envoy.conf - // Default is using one in this folder. - ServerProxyPort = 29090 - ClientProxyPort = 27070 - MixerPort = 29091 - BackendPort = 28080 + "testing" ) type TestSetup struct { envoy *Envoy mixer *MixerServer backend *HttpServer + t *testing.T } -func SetUp() (ts TestSetup, err error) { - ts.envoy, err = NewEnvoy() +func SetUp(t *testing.T, conf string) (s TestSetup, err error) { + s.t = t + s.envoy, err = NewEnvoy(conf) if err != nil { log.Printf("unable to create Envoy %v", err) } else { - ts.envoy.Start() + s.envoy.Start() } - ts.mixer, err = NewMixerServer(MixerPort) + s.mixer, err = NewMixerServer(MixerPort) if err != nil { log.Printf("unable to create mixer server %v", err) } else { - ts.mixer.Start() + s.mixer.Start() } - ts.backend, err = NewHttpServer(BackendPort) + s.backend, err = NewHttpServer(BackendPort) if err != nil { log.Printf("unable to create HTTP server %v", err) } else { - ts.backend.Start() + s.backend.Start() + } + return s, err +} + +func (s *TestSetup) TearDown() { + s.envoy.Stop() + s.mixer.Stop() + s.backend.Stop() +} + +func (s *TestSetup) VerifyCheckCount(tag string, expected int) { + if s.mixer.check.count != expected { + s.t.Fatalf("%s check count doesn't match: %v\n, expected: %+v", + tag, s.mixer.check.count, expected) } - return ts, err } -func (ts *TestSetup) TearDown() { - ts.envoy.Stop() - ts.mixer.Stop() - ts.backend.Stop() +func (s *TestSetup) VerifyCheck(tag string, result string) { + _ = <-s.mixer.check.ch + if err := Verify(s.mixer.check.bag, result); err != nil { + s.t.Fatalf("Failed to verify %s check: %v\n, Attributes: %+v", + tag, err, s.mixer.check.bag) + } +} + +func (s *TestSetup) VerifyReport(tag string, result string) { + _ = <-s.mixer.report.ch + if err := Verify(s.mixer.report.bag, result); err != nil { + s.t.Fatalf("Failed to verify %s report: %v\n, Attributes: %+v", + tag, err, s.mixer.report.bag) + } +} + +func (s *TestSetup) VerifyQuota(tag string, name string, amount int64) { + _ = <-s.mixer.quota.ch + if s.mixer.quota_request.Quota != name { + s.t.Fatalf("Failed to verify %s quota name: %v, expected: %v\n", + tag, s.mixer.quota_request.Quota, name) + } + if s.mixer.quota_request.Amount != amount { + s.t.Fatalf("Failed to verify %s quota amount: %v, expected: %v\n", + tag, s.mixer.quota_request.Amount, amount) + } } diff --git a/src/envoy/mixer/integration_test/test_suite.bzl b/src/envoy/mixer/integration_test/test_suite.bzl new file mode 100644 index 00000000000..afa984f4773 --- /dev/null +++ b/src/envoy/mixer/integration_test/test_suite.bzl @@ -0,0 +1,29 @@ +# 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. +# +################################################################################ +# + +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +def go_test_suite(tests, library, data, tags, size="small"): + for test in tests: + go_test( + name = test.split(".")[0], + size = size, + srcs = [test], + data = data, + library = library, + tags = tags, + ) From c93dcc5d7605fe1cc375ac5462499f2316b31870 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Sat, 25 Mar 2017 19:09:45 -0700 Subject: [PATCH 21/23] Update README on how to enable check cache. (#204) * Update README on how to enable check cache. * Update the comment. --- src/envoy/mixer/README.md | 38 ++++++++++++++++++++++++++++---- src/envoy/mixer/config.cc | 31 +++++++++++++------------- src/envoy/mixer/config.h | 1 + src/envoy/mixer/http_control.cc | 30 +++++++++++++++++++++---- src/envoy/mixer/repositories.bzl | 2 +- 5 files changed, 78 insertions(+), 24 deletions(-) diff --git a/src/envoy/mixer/README.md b/src/envoy/mixer/README.md index 493b2d185cf..48229a3631e 100644 --- a/src/envoy/mixer/README.md +++ b/src/envoy/mixer/README.md @@ -73,6 +73,7 @@ This filter will intercept all HTTP requests and call Mixer. Here is its config: }, "quota_name": "RequestCount", "quota_amount": "1", + "check_cache_expiration_in_seconds": "600", "check_cache_keys": [ "request.host", "request.path", @@ -85,16 +86,23 @@ Notes: * mixer_server is required * 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. +* quota_name, quota_amount are used for making quota call. quota_amount defaults to 1. * 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 in the route config: +## HTTP Route opaque config +By default, the mixer filter only forwards attributes and does not call mixer server. This behavior can be changed per HTTP route by supplying an opaque config: ``` - "opaque_config": { + "routes": [ + { + "timeout_ms": 0, + "prefix": "/", + "cluster": "service1", + "opaque_config": { "mixer_control": "on", "mixer_forward": "off" - } + } + } ``` This route opaque config reverts the behavior by sending requests to mixer server but not forwarding any attributes. @@ -111,3 +119,25 @@ 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 sent to the upstream proxy (the server proxy). If the server proxy is calling mixer, these attributes will be sent to the mixer. + + +## How to enable cache for Check calls + +Check calls can be cached. By default, it is not enabled. It can be enabled by supplying non-empty "check_cache_keys" string list in the mixer filter config. Only these attributes in the Check request, their keys and values, are used to calculate the key for the cache lookup. If it is a cache hit, the cached response will be used. +The cached response will be expired in 5 minutes by default. It can be overrided by supplying "check_cache_expiration_in_seconds" in the mixer filter config. The Check response from the mixer has an expiration field. If it is filled, it will be used. By design, the mixer will control the cache expiration time. + +Following is a sample mixer filter config to enable the Check call cache: +``` + "check_cache_expiration_in_seconds": "600", + "check_cache_keys": [ + "request.host", + "request.path", + "source.labels", + "request.headers/:method", + "origin.user" + ] +``` + +For the string map attributes in the above example: +1) "request.headers" attribute is a string map, "request.headers/:method" cache key means only its ":method" key and value are used for cache key. +2) "source.labels" attribute is a string map, "source.labels" cache key means all key value pairs for the string map will be used. diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index 5675a5a311f..8e06b1e9c2b 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -22,21 +22,22 @@ namespace Mixer { namespace { // The Json object name for mixer-server. -const std::string kJsonNameMixerServer("mixer_server"); +const std::string kMixerServer("mixer_server"); // The Json object name for static attributes. -const std::string kJsonNameMixerAttributes("mixer_attributes"); +const std::string kMixerAttributes("mixer_attributes"); // The Json object name to specify attributes which will be forwarded // to the upstream istio proxy. -const std::string kJsonNameForwardAttributes("forward_attributes"); +const std::string kForwardAttributes("forward_attributes"); // The Json object name for quota name and amount. -const std::string kJsonNameQuotaName("quota_name"); -const std::string kJsonNameQuotaAmount("quota_amount"); +const std::string kQuotaName("quota_name"); +const std::string kQuotaAmount("quota_amount"); // The Json object name for check cache keys. -const std::string kJsonNameCheckCacheKeys("check_cache_keys"); +const std::string kCheckCacheKeys("check_cache_keys"); +const std::string kCheckCacheExpiration("check_cache_expiration_in_seconds"); void ReadString(const Json::Object& json, const std::string& name, std::string* value) { @@ -67,28 +68,28 @@ void ReadStringVector(const Json::Object& json, const std::string& name, } // namespace void MixerConfig::Load(const Json::Object& json) { - ReadString(json, kJsonNameMixerServer, &mixer_server); + ReadString(json, kMixerServer, &mixer_server); - ReadStringMap(json, kJsonNameMixerAttributes, &mixer_attributes); - ReadStringMap(json, kJsonNameForwardAttributes, &forward_attributes); + ReadStringMap(json, kMixerAttributes, &mixer_attributes); + ReadStringMap(json, kForwardAttributes, &forward_attributes); - ReadString(json, kJsonNameQuotaName, "a_name); - ReadString(json, kJsonNameQuotaAmount, "a_amount); + ReadString(json, kQuotaName, "a_name); + ReadString(json, kQuotaAmount, "a_amount); - ReadStringVector(json, kJsonNameCheckCacheKeys, &check_cache_keys); + ReadStringVector(json, kCheckCacheKeys, &check_cache_keys); + ReadString(json, kCheckCacheExpiration, &check_cache_expiration); } void MixerConfig::ExtractQuotaAttributes(Attributes* attr) const { if (!quota_name.empty()) { - attr->attributes[ ::istio::mixer_client::kQuotaName] = + attr->attributes[Attributes::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); + attr->attributes[Attributes::kQuotaAmount] = Attributes::Int64Value(amount); } } diff --git a/src/envoy/mixer/config.h b/src/envoy/mixer/config.h index ee301aba2b5..fc08f33c439 100644 --- a/src/envoy/mixer/config.h +++ b/src/envoy/mixer/config.h @@ -41,6 +41,7 @@ struct MixerConfig { // The attribute names for check cache. std::vector check_cache_keys; + std::string check_cache_expiration; // Load the config from envoy config. void Load(const Json::Object& json); diff --git a/src/envoy/mixer/http_control.cc b/src/envoy/mixer/http_control.cc index b44cf707172..8283482dff3 100644 --- a/src/envoy/mixer/http_control.cc +++ b/src/envoy/mixer/http_control.cc @@ -23,8 +23,12 @@ #include "src/envoy/mixer/utils.h" using ::google::protobuf::util::Status; +using ::istio::mixer_client::CheckOptions; using ::istio::mixer_client::Attributes; using ::istio::mixer_client::DoneFunc; +using ::istio::mixer_client::MixerClientOptions; +using ::istio::mixer_client::ReportOptions; +using ::istio::mixer_client::QuotaOptions; namespace Http { namespace Mixer { @@ -45,6 +49,26 @@ const std::string kResponseLatency = "response.latency"; const std::string kResponseSize = "response.size"; const std::string kResponseTime = "response.time"; +// Check cache size: 10000 cache entries. +const int kCheckCacheEntries = 10000; +// Default check cache expired in 5 minutes. +const int kCheckCacheExpirationInSeconds = 300; + +CheckOptions GetCheckOptions(const MixerConfig& config) { + int expiration = kCheckCacheExpirationInSeconds; + if (!config.check_cache_expiration.empty()) { + expiration = std::stoi(config.check_cache_expiration); + } + + // Remove expired items from cache 1 second later. + CheckOptions options(kCheckCacheEntries, expiration * 1000, + (expiration + 1) * 1000); + + options.cache_keys = config.check_cache_keys; + + return options; +} + void SetStringAttribute(const std::string& name, const std::string& value, Attributes* attr) { if (!value.empty()) { @@ -110,11 +134,9 @@ void FillRequestInfoAttributes(const AccessLog::RequestInfo& info, HttpControl::HttpControl(const MixerConfig& mixer_config) : mixer_config_(mixer_config) { - ::istio::mixer_client::MixerClientOptions options; + MixerClientOptions options(GetCheckOptions(mixer_config), ReportOptions(), + QuotaOptions()); 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); mixer_config_.ExtractQuotaAttributes("a_attributes_); diff --git a/src/envoy/mixer/repositories.bzl b/src/envoy/mixer/repositories.bzl index ffa2a20409f..4382e71f921 100644 --- a/src/envoy/mixer/repositories.bzl +++ b/src/envoy/mixer/repositories.bzl @@ -15,7 +15,7 @@ ################################################################################ # -MIXER_CLIENT = "0e6f858bc7b52dc8f143f46ac32afc79d504e8a4" +MIXER_CLIENT = "c5d857e28bfcc53f20f59466b464f99526737545" def mixer_client_repositories(bind=True): native.git_repository( From 8c2bbceb284dd4347806e91dd09a0e78ba78d80b Mon Sep 17 00:00:00 2001 From: htuch Date: Sat, 1 Apr 2017 14:48:36 -0400 Subject: [PATCH 22/23] build: support Envoy native Bazel build. (#210) * build: support Envoy native Bazel build. This patch switches the Envoy build from src/envoy/repositories.bzl to using the upstream native build. See https://github.com/lyft/envoy/pull/663 for the corresponding changes on the Envoy side. * Use Envoy master with BUILD.wip rename merged. * Fix clang-format issues. --- src/envoy/BUILD | 34 ------ src/envoy/mixer/BUILD | 5 +- src/envoy/mixer/http_filter.cc | 18 ++-- src/envoy/repositories.bzl | 189 ++------------------------------- 4 files changed, 22 insertions(+), 224 deletions(-) diff --git a/src/envoy/BUILD b/src/envoy/BUILD index 1c75af50a75..e69de29bb2d 100644 --- a/src/envoy/BUILD +++ b/src/envoy/BUILD @@ -1,34 +0,0 @@ -# Copyright 2016 Google Inc. 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. -# -################################################################################ -# -cc_test( - name = "envoy_test", - data = [ - "@envoy_git//:envoy-testdata", - ], - copts = [ - "-include ./external/envoy_git/test/precompiled/precompiled_test.h", - ], - deps = [ - "@envoy_git//:envoy-test-lib", - "//external:googletest_main", - ], - args = [ - #TODO: Make all test pass - "--gtest_filter=RouterTest.*", - ], - linkstatic=1, -) diff --git a/src/envoy/mixer/BUILD b/src/envoy/mixer/BUILD index aa7c455915d..45c69b7f2d5 100644 --- a/src/envoy/mixer/BUILD +++ b/src/envoy/mixer/BUILD @@ -41,7 +41,7 @@ cc_library( deps = [ ":string_map_proto", "//external:mixer_client_lib", - "@envoy_git//:envoy-common", + "@envoy//source/exe:envoy_common_lib", ], alwayslink = 1, ) @@ -49,10 +49,11 @@ cc_library( cc_binary( name = "envoy", linkstatic = 1, + linkopts = ["-lrt"], visibility = [":__subpackages__"], deps = [ ":filter_lib", - "@envoy_git//:envoy-main", + "@envoy//source/exe:envoy_main_lib", ], ) diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index c46df8adf4b..9f8e4ffb75c 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -316,17 +316,19 @@ class MixerConfig : public HttpFilterConfigFactory { Http::Mixer::ConfigPtr mixer_config( new Http::Mixer::Config(config, server)); - return [mixer_config]( - Http::FilterChainFactoryCallbacks& callbacks) -> void { - std::shared_ptr instance( - new Http::Mixer::Instance(mixer_config)); - callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterPtr(instance)); - callbacks.addAccessLogHandler(Http::AccessLog::InstancePtr(instance)); - }; + return + [mixer_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { + std::shared_ptr instance( + new Http::Mixer::Instance(mixer_config)); + callbacks.addStreamDecoderFilter( + Http::StreamDecoderFilterSharedPtr(instance)); + callbacks.addAccessLogHandler( + Http::AccessLog::InstanceSharedPtr(instance)); + }; } }; static RegisterHttpFilterConfigFactory register_; } // namespace Configuration -} // namespace server +} // namespace Server diff --git a/src/envoy/repositories.bzl b/src/envoy/repositories.bzl index bb150928ba0..bad332e188d 100644 --- a/src/envoy/repositories.bzl +++ b/src/envoy/repositories.bzl @@ -559,185 +559,14 @@ def envoy_repositories(bind=True): rapidjson_repositories(bind) nghttp2_repositories(bind) ares_repositories(bind) - - BUILD = """ -load("@protobuf_git//:protobuf.bzl", "cc_proto_library") - -exports_files(["source/precompiled/precompiled.h"]) - -package(default_visibility = ["//visibility:public"]) - -genrule( - name = "envoy-ratelimit-proto", - srcs = [ - "source/common/ratelimit/ratelimit.proto", - ], - outs = [ - "source/common/generated/ratelimit.proto", - ], - cmd = "cp $(SRCS) $@", -) - -cc_proto_library( - name = "envoy-ratelimit-pb", - srcs = [ - "source/common/generated/ratelimit.proto", - ], - default_runtime = "//external:protobuf", - protoc = "//external:protoc", - include = "source", -) - -genrule( - name = "envoy-test-proto", - srcs = [ - "test/proto/helloworld.proto", - ], - outs = [ - "test/generated/helloworld.proto", - ], - cmd = "cp $(SRCS) $@", -) - -cc_proto_library( - name = "envoy-test-pb", - srcs = [ - "test/generated/helloworld.proto", - ], - default_runtime = "//external:protobuf", - protoc = "//external:protoc", - include = "test", -) - -genrule( - name = "envoy-version", - srcs = glob([ - ".git/**", - ]), - tools = [ - "tools/gen_git_sha.sh", - ], - outs = [ - "source/common/version_generated.cc", - ], - cmd = "touch $@ && $(location tools/gen_git_sha.sh) $$(dirname $(location tools/gen_git_sha.sh)) $@", - local = 1, -) - -cc_library( - name = "envoy-common", - srcs = glob([ - "source/**/*.cc", - "source/**/*.h", - "include/**/*.h", - ], exclude=["source/exe/main.cc"]) + [ - "source/common/version_generated.cc", - ], - copts = [ - "-I./external/envoy_git/source", - "-include ./external/envoy_git/source/precompiled/precompiled.h", - ], - includes = [ - "include", - ], - linkopts = [ - "-lpthread", - "-lanl", - "-lrt", - ], - linkstatic=1, - alwayslink=1, - deps = [ - ":envoy-ratelimit-pb", - "//external:ares", - "//external:libssl", - "//external:nghttp2", - "//external:spdlog", - "//external:tclap", - "//external:lightstep", - "//external:event", - "//external:protobuf", - "//external:http_parser", - "//external:rapidjson", - "//external:event_pthreads", - ], -) - -cc_library( - name = "envoy-main", - srcs = [ - "source/exe/main.cc", - ], - copts = [ - "-I./external/envoy_git/source", - "-include ./external/envoy_git/source/precompiled/precompiled.h", - ], - deps = [ - ":envoy-common", - ], - linkstatic=1, -) - -cc_binary( - name = "envoy", - srcs = [ - "source/exe/main.cc", - ], - copts = [ - "-I./external/envoy_git/source", - "-include ./external/envoy_git/source/precompiled/precompiled.h", - ], - deps = [ - ":envoy-common", - ], - linkstatic=1, -) - -cc_library( - name = "envoy-test-lib", - srcs = glob([ - "test/**/*.cc", - "test/**/*.h", - ]), - copts = [ - "-I./external/envoy_git/source", - "-include ./external/envoy_git/test/precompiled/precompiled_test.h", - ], - includes = [ - "include", - ], - deps = [ - ":envoy-common", - ":envoy-test-pb", - "//external:googletest", - ], - alwayslink=1, -) - -filegroup( - name = "envoy-testdata", - srcs = glob([ - "generated/**/*", - "test/**/*", - ]), -) - -cc_test( - name = "envoy-test", - data = [ - ":envoy-testdata", - ], - deps = [ - ":envoy-test-lib", - ":envoy-test-pb", - "//external:googletest", - ], - linkstatic=1, -)""" - - native.new_git_repository( - name = "envoy_git", + # @boringssl is defined in //:repositories.bzl, but bound to libssl for + # grpc. Rebind to what envoy expects here. + native.bind( + name = "ssl", + actual = "@boringssl//:ssl", + ) + native.git_repository( + name = "envoy", remote = "https://github.com/lyft/envoy.git", - commit = "09f078b016da908ba8b861857f2a12a43933ba40", # Mar 21 2017 - build_file_content = BUILD, + commit = "bf3f23ad439ee83b91015dc4d0d7cb53b14bf1bc", ) From 2fae2d78930494cd68da6cdb4252ad19d9ef5819 Mon Sep 17 00:00:00 2001 From: Sebastien Vas Date: Sat, 1 Apr 2017 13:40:28 -0700 Subject: [PATCH 23/23] Fixes bazel.rc issues (#212) * Fixes bazel rc issues * Update Jenkins to latest pipeline version --- .travis.yml | 3 ++- Jenkinsfile | 2 +- .bazelrc.jenkins => tools/bazel.rc.jenkins | 0 .bazelrc.travis => tools/bazel.rc.travis | 0 4 files changed, 3 insertions(+), 2 deletions(-) rename .bazelrc.jenkins => tools/bazel.rc.jenkins (100%) rename .bazelrc.travis => tools/bazel.rc.travis (100%) diff --git a/.travis.yml b/.travis.yml index b2c2e63a881..845ac9bbd8e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,7 +37,8 @@ before_install: - sudo dpkg -i bazel_${BAZEL_VERSION}-linux-x86_64.deb - sudo apt-get -f install -qqy uuid-dev - cd ${TRAVIS_BUILD_DIR} - - mv .bazelrc.travis .bazelrc + - mv tools/bazel.rc tools/bazel.rc.orig + - cat tools/bazel.rc.travis tools/bazel.rc.orig > tools/bazel.rc script: - script/check-style diff --git a/Jenkinsfile b/Jenkinsfile index 6575dc498f5..c1aa89e3f25 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,6 +1,6 @@ #!groovy -@Library('testutils@stable-63c264e') +@Library('testutils@stable-3e4d089') import org.istio.testutils.Utilities import org.istio.testutils.GitUtilities diff --git a/.bazelrc.jenkins b/tools/bazel.rc.jenkins similarity index 100% rename from .bazelrc.jenkins rename to tools/bazel.rc.jenkins diff --git a/.bazelrc.travis b/tools/bazel.rc.travis similarity index 100% rename from .bazelrc.travis rename to tools/bazel.rc.travis