From 83185113d9de9774d288093f3196c5f8d66210c5 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Mon, 13 Aug 2018 12:34:58 -0700 Subject: [PATCH 1/9] Generate source.namespace in proxy. The next step is to change Mixer to not to generate source.namespace if it's already in the Check call. --- include/istio/control/tcp/check_data.h | 4 +++ include/istio/utils/attribute_names.h | 1 + src/envoy/http/authn/http_filter_test.cc | 12 ++++++--- src/envoy/tcp/mixer/filter.cc | 5 ++++ src/envoy/tcp/mixer/filter.h | 2 ++ src/envoy/utils/BUILD | 1 + src/envoy/utils/authn.cc | 6 +++++ src/envoy/utils/authn_test.cc | 10 ++++--- src/envoy/utils/utils.cc | 20 ++++++++++++++ src/envoy/utils/utils.h | 4 +++ src/envoy/utils/utils_test.cc | 27 +++++++++++++++++++ src/istio/control/http/attributes_builder.cc | 1 + .../control/http/attributes_builder_test.cc | 24 ++++++++++++----- src/istio/control/tcp/attributes_builder.cc | 4 +++ .../control/tcp/attributes_builder_test.cc | 21 ++++++++++++--- src/istio/control/tcp/mock_check_data.h | 2 ++ src/istio/utils/attribute_names.cc | 1 + 17 files changed, 130 insertions(+), 15 deletions(-) diff --git a/include/istio/control/tcp/check_data.h b/include/istio/control/tcp/check_data.h index 73cc2ab14de..ddc421f2b92 100644 --- a/include/istio/control/tcp/check_data.h +++ b/include/istio/control/tcp/check_data.h @@ -34,6 +34,10 @@ class CheckData { // If SSL is used, get peer or local certificate SAN URI. virtual bool GetPrincipal(bool peer, std::string* user) const = 0; + // Get source namespace from the principal. + virtual bool GetSourceNamespace(const std::string& principal, + std::string* ns) const = 0; + // Returns true if connection is mutual TLS enabled. virtual bool IsMutualTLS() const = 0; diff --git a/include/istio/utils/attribute_names.h b/include/istio/utils/attribute_names.h index 84e7415837d..67c6df64c98 100644 --- a/include/istio/utils/attribute_names.h +++ b/include/istio/utils/attribute_names.h @@ -27,6 +27,7 @@ struct AttributeName { // https://github.com/istio/istio/issues/4689 static const char kSourceUser[]; static const char kSourcePrincipal[]; + static const char kSourceNamespace[]; static const char kDestinationPrincipal[]; static const char kRequestHeaders[]; diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index 08376bfb9a3..20382d94ec0 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -72,7 +72,7 @@ std::unique_ptr createAlwaysPassAuthenticator( _local(FilterContext *filter_context) : AuthenticatorBase(filter_context) {} bool run(Payload *) override { // Set some data to verify authentication result later. - auto payload = TestUtilities::CreateX509Payload("foo"); + auto payload = TestUtilities::CreateX509Payload("sa/foo/ns/test_ns/"); filter_context()->setPeerResult(&payload); return true; } @@ -180,16 +180,22 @@ TEST_F(AuthenticationFilterTest, AllPass) { ProtobufWkt::Struct expected_data; ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(R"( + fields { + key: "source.namespace" + value { + string_value: "test_ns" + } + } fields { key: "source.principal" value { - string_value: "foo" + string_value: "sa/foo/ns/test_ns/" } } fields { key: "source.user" value { - string_value: "foo" + string_value: "sa/foo/ns/test_ns/" } })", &expected_data)); diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index 105db9d19b6..fe161687c77 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -154,6 +154,11 @@ bool Filter::GetPrincipal(bool peer, std::string* user) const { return Utils::GetPrincipal(&filter_callbacks_->connection(), peer, user); } +bool Filter::GetSourceNamespace(const std::string& principal, + std::string* ns) const { + return Utils::GetSourceNamespace(principal, ns); +} + bool Filter::IsMutualTLS() const { return Utils::IsMutualTLS(&filter_callbacks_->connection()); } diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h index a06daa9f852..cc3c3b8aa33 100644 --- a/src/envoy/tcp/mixer/filter.h +++ b/src/envoy/tcp/mixer/filter.h @@ -52,6 +52,8 @@ class Filter : public Network::Filter, // CheckData virtual functions. bool GetSourceIpPort(std::string* str_ip, int* port) const override; bool GetPrincipal(bool peer, std::string* user) const override; + bool GetSourceNamespace(const std::string& principal, + std::string* ns) const override; bool IsMutualTLS() const override; bool GetRequestedServerName(std::string* name) const override; diff --git a/src/envoy/utils/BUILD b/src/envoy/utils/BUILD index adf671a1b20..b3ed537cd99 100644 --- a/src/envoy/utils/BUILD +++ b/src/envoy/utils/BUILD @@ -32,6 +32,7 @@ envoy_cc_library( repository = "@envoy", visibility = ["//visibility:public"], deps = [ + ":utils_lib", "//include/istio/utils:attribute_names_header", "//src/istio/authn:context_proto", "//src/istio/utils:attribute_names_lib", diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index a28f8ceba8f..151d78cb567 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -17,6 +17,7 @@ #include "common/common/base64.h" #include "include/istio/utils/attribute_names.h" #include "src/envoy/utils/filter_names.h" +#include "src/envoy/utils/utils.h" #include "src/istio/authn/context.pb.h" using istio::authn::Result; @@ -47,6 +48,11 @@ void Authentication::SaveAuthAttributesToStruct( result.peer_user()); setKeyValue(data, istio::utils::AttributeName::kSourcePrincipal, result.peer_user()); + std::string source_ns(""); + if (GetSourceNamespace(result.peer_user(), &source_ns)) { + setKeyValue(data, istio::utils::AttributeName::kSourceNamespace, + source_ns); + } } if (result.has_origin()) { const auto& origin = result.origin(); diff --git a/src/envoy/utils/authn_test.cc b/src/envoy/utils/authn_test.cc index cc97855cda9..c4001dd3740 100644 --- a/src/envoy/utils/authn_test.cc +++ b/src/envoy/utils/authn_test.cc @@ -41,7 +41,7 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { EXPECT_TRUE(data.mutable_fields()->empty()); result.set_principal("principal"); - result.set_peer_user("peeruser"); + result.set_peer_user("sa/peeruser/ns/abc/"); auto origin = result.mutable_origin(); origin->add_audiences("audiences0"); origin->add_audiences("audiences1"); @@ -62,11 +62,15 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { "principal"); EXPECT_EQ( data.fields().at(istio::utils::AttributeName::kSourceUser).string_value(), - "peeruser"); + "sa/peeruser/ns/abc/"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kSourcePrincipal) .string_value(), - "peeruser"); + "sa/peeruser/ns/abc/"); + EXPECT_EQ(data.fields() + .at(istio::utils::AttributeName::kSourceNamespace) + .string_value(), + "abc"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kRequestAuthAudiences) .string_value(), diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index 6197cacedf5..6ed80d9dad9 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -33,6 +33,8 @@ const std::string kPerHostMetadataKey("istio"); // Attribute field for per-host data override const std::string kMetadataDestinationUID("uid"); +const std::string kNamespacePrefix("ns/"); + } // namespace void ExtractHeaders(const Http::HeaderMap& header_map, @@ -118,6 +120,24 @@ bool GetPrincipal(const Network::Connection* connection, bool peer, return false; } +bool GetSourceNamespace(const std::string& principal, + std::string* source_namespace) { + if (source_namespace) { + size_t begin = principal.find(kNamespacePrefix); + if (begin == std::string::npos) { + return false; + } + begin += kNamespacePrefix.length(); + size_t end = principal.find("/", begin); + if (end == std::string::npos) { + return false; + } + *source_namespace = principal.substr(begin, end - begin); + return true; + } + return false; +} + bool IsMutualTLS(const Network::Connection* connection) { return connection != nullptr && connection->ssl() != nullptr && connection->ssl()->peerCertificatePresented(); diff --git a/src/envoy/utils/utils.h b/src/envoy/utils/utils.h index 6ffde52c3c9..c7e8ba52062 100644 --- a/src/envoy/utils/utils.h +++ b/src/envoy/utils/utils.h @@ -41,6 +41,10 @@ bool GetDestinationUID(const envoy::api::v2::core::Metadata& metadata, bool GetPrincipal(const Network::Connection* connection, bool peer, std::string* principal); +// Get source.namespace attribute from principal. +bool GetSourceNamespace(const std::string& principal, + std::string* source_namespace); + // Returns true if connection is mutual TLS enabled. bool IsMutualTLS(const Network::Connection* connection); diff --git a/src/envoy/utils/utils_test.cc b/src/envoy/utils/utils_test.cc index 8d38b995e77..f7094d593fa 100644 --- a/src/envoy/utils/utils_test.cc +++ b/src/envoy/utils/utils_test.cc @@ -44,4 +44,31 @@ TEST(UtilsTest, ParseMessageWithUnknownField) { EXPECT_EQ(http_config.default_destination_service(), "service.svc.cluster.local"); } + +TEST(UtilsTest, GetSourceNamespace) { + std::string ns = ""; + EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("", &ns)); + EXPECT_EQ("", ns); + + EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("ns/abc", &ns)); + EXPECT_EQ("", ns); + + EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("ns/", &ns)); + EXPECT_EQ("", ns); + + EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("s/abc/", &ns)); + EXPECT_EQ("", ns); + + EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("NS/abc/", &ns)); + EXPECT_EQ("", ns); + + EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("abc", &ns)); + EXPECT_EQ("", ns); + + EXPECT_EQ(true, Envoy::Utils::GetSourceNamespace("ns/abc/", &ns)); + EXPECT_EQ("abc", ns); + + EXPECT_EQ(true, Envoy::Utils::GetSourceNamespace("ns//", &ns)); + EXPECT_EQ("", ns); +} } // namespace diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 91835c7df67..ac1e00fc92a 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -90,6 +90,7 @@ void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { utils::AttributeName::kRequestAuthPrincipal, utils::AttributeName::kSourceUser, utils::AttributeName::kSourcePrincipal, + utils::AttributeName::kSourceNamespace, utils::AttributeName::kRequestAuthAudiences, utils::AttributeName::kRequestAuthPresenter, utils::AttributeName::kRequestAuthRawClaims, diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 305fac3b44b..dca943d9e9a 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -224,16 +224,22 @@ attributes { string_value: "www.google.com" } } +attributes { + key: "source.namespace" + value { + string_value: "test_ns" + } +} attributes { key: "source.principal" value { - string_value: "test_user" + string_value: "sa/test_user/ns/test_ns/" } } attributes { key: "source.user" value { - string_value: "test_user" + string_value: "sa/test_user/ns/test_ns/" } } attributes { @@ -489,16 +495,22 @@ fields { string_value: "test_raw_claims" } } +fields { + key: "source.namespace" + value { + string_value: "test_ns" + } +} fields { key: "source.principal" value { - string_value: "test_user" + string_value: "sa/test_user/ns/test_ns/" } } fields { key: "source.user" value { - string_value: "test_user" + string_value: "sa/test_user/ns/test_ns/" } } )"; @@ -556,7 +568,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { if (peer) { - *user = "test_user"; + *user = "sa/test_user/ns/test_ns/"; } else { *user = "destination_user"; } @@ -630,7 +642,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { if (peer) { - *user = "test_user"; + *user = "sa/test_user/ns/test_ns/"; } else { *user = "destination_user"; } diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index e61bc0e3917..40cf18dbf62 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -49,6 +49,10 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { // over. https://github.com/istio/istio/issues/4689 builder.AddString(utils::AttributeName::kSourceUser, source_user); builder.AddString(utils::AttributeName::kSourcePrincipal, source_user); + std::string source_ns(""); + if (check_data->GetSourceNamespace(source_user, &source_ns)) { + builder.AddString(utils::AttributeName::kSourceNamespace, source_ns); + } } std::string destination_principal; diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index 52ee3a984a2..a936e461cac 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -73,16 +73,22 @@ attributes { string_value: "www.google.com" } } +attributes { + key: "source.namespace" + value { + string_value: "test_ns" + } +} attributes { key: "source.principal" value { - string_value: "test_user" + string_value: "sa/test_user/ns/test_ns/" } } attributes { key: "source.user" value { - string_value: "test_user" + string_value: "sa/test_user/ns/test_ns/" } } attributes { @@ -372,12 +378,21 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string* user) -> bool { if (peer) { - *user = "test_user"; + *user = "sa/test_user/ns/test_ns/"; } else { *user = "destination_user"; } return true; })); + EXPECT_CALL(mock_data, GetSourceNamespace(_, _)) + .WillRepeatedly( + Invoke([](const std::string& principal, std::string* ns) -> bool { + if (principal == "sa/test_user/ns/test_ns/") { + *ns = "test_ns"; + return true; + } + return false; + })); EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5")); EXPECT_CALL(mock_data, GetRequestedServerName(_)) .WillOnce(Invoke([](std::string* name) -> bool { diff --git a/src/istio/control/tcp/mock_check_data.h b/src/istio/control/tcp/mock_check_data.h index 16cb7752553..20954111bb9 100644 --- a/src/istio/control/tcp/mock_check_data.h +++ b/src/istio/control/tcp/mock_check_data.h @@ -28,6 +28,8 @@ class MockCheckData : public CheckData { public: MOCK_CONST_METHOD2(GetSourceIpPort, bool(std::string* ip, int* port)); MOCK_CONST_METHOD2(GetPrincipal, bool(bool peer, std::string* user)); + MOCK_CONST_METHOD2(GetSourceNamespace, + bool(const std::string&, std::string* ns)); MOCK_CONST_METHOD0(IsMutualTLS, bool()); MOCK_CONST_METHOD1(GetRequestedServerName, bool(std::string* name)); MOCK_CONST_METHOD0(GetConnectionId, std::string()); diff --git a/src/istio/utils/attribute_names.cc b/src/istio/utils/attribute_names.cc index d42e0666b38..eead2de2f58 100644 --- a/src/istio/utils/attribute_names.cc +++ b/src/istio/utils/attribute_names.cc @@ -21,6 +21,7 @@ namespace utils { // Define attribute names const char AttributeName::kSourceUser[] = "source.user"; const char AttributeName::kSourcePrincipal[] = "source.principal"; +const char AttributeName::kSourceNamespace[] = "source.namespace"; const char AttributeName::kDestinationPrincipal[] = "destination.principal"; const char AttributeName::kRequestHeaders[] = "request.headers"; From e26b74edd38cef0ce6e3912a7bbe10cd63087778 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Mon, 13 Aug 2018 16:05:05 -0700 Subject: [PATCH 2/9] add comment. --- src/envoy/utils/utils.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index 6ed80d9dad9..03352e1da92 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -123,6 +123,7 @@ bool GetPrincipal(const Network::Connection* connection, bool peer, bool GetSourceNamespace(const std::string& principal, std::string* source_namespace) { if (source_namespace) { + // The namespace is a substring in principal with format: "ns//". size_t begin = principal.find(kNamespacePrefix); if (begin == std::string::npos) { return false; From 5d716b2b2e6073429452e2b352a3866611a0ab68 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Tue, 14 Aug 2018 15:54:21 -0700 Subject: [PATCH 3/9] address comments. --- include/istio/control/tcp/check_data.h | 4 -- src/envoy/http/authn/http_filter_test.cc | 7 +- src/envoy/tcp/mixer/filter.cc | 5 -- src/envoy/tcp/mixer/filter.h | 2 - src/envoy/utils/BUILD | 1 + src/envoy/utils/authn.cc | 4 +- src/envoy/utils/authn_test.cc | 6 +- src/envoy/utils/utils.cc | 21 ------ src/envoy/utils/utils.h | 4 -- src/envoy/utils/utils_test.cc | 27 -------- src/istio/control/tcp/BUILD | 2 + src/istio/control/tcp/attributes_builder.cc | 3 +- .../control/tcp/attributes_builder_test.cc | 9 +-- src/istio/utils/BUILD | 14 ++++ src/istio/utils/utils.cc | 61 ++++++++++++++++ src/istio/utils/utils.h | 28 ++++++++ src/istio/utils/utils_test.cc | 69 +++++++++++++++++++ 17 files changed, 191 insertions(+), 76 deletions(-) create mode 100644 src/istio/utils/utils.cc create mode 100644 src/istio/utils/utils.h create mode 100644 src/istio/utils/utils_test.cc diff --git a/include/istio/control/tcp/check_data.h b/include/istio/control/tcp/check_data.h index ddc421f2b92..73cc2ab14de 100644 --- a/include/istio/control/tcp/check_data.h +++ b/include/istio/control/tcp/check_data.h @@ -34,10 +34,6 @@ class CheckData { // If SSL is used, get peer or local certificate SAN URI. virtual bool GetPrincipal(bool peer, std::string* user) const = 0; - // Get source namespace from the principal. - virtual bool GetSourceNamespace(const std::string& principal, - std::string* ns) const = 0; - // Returns true if connection is mutual TLS enabled. virtual bool IsMutualTLS() const = 0; diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index 20382d94ec0..68d1edbbb74 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -72,7 +72,8 @@ std::unique_ptr createAlwaysPassAuthenticator( _local(FilterContext *filter_context) : AuthenticatorBase(filter_context) {} bool run(Payload *) override { // Set some data to verify authentication result later. - auto payload = TestUtilities::CreateX509Payload("sa/foo/ns/test_ns/"); + auto payload = TestUtilities::CreateX509Payload( + "cluster.local/sa/test_user/ns/test_ns/"); filter_context()->setPeerResult(&payload); return true; } @@ -189,13 +190,13 @@ TEST_F(AuthenticationFilterTest, AllPass) { fields { key: "source.principal" value { - string_value: "sa/foo/ns/test_ns/" + string_value: "cluster.local/sa/test_user/ns/test_ns/" } } fields { key: "source.user" value { - string_value: "sa/foo/ns/test_ns/" + string_value: "cluster.local/sa/test_user/ns/test_ns/" } })", &expected_data)); diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index fe161687c77..105db9d19b6 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -154,11 +154,6 @@ bool Filter::GetPrincipal(bool peer, std::string* user) const { return Utils::GetPrincipal(&filter_callbacks_->connection(), peer, user); } -bool Filter::GetSourceNamespace(const std::string& principal, - std::string* ns) const { - return Utils::GetSourceNamespace(principal, ns); -} - bool Filter::IsMutualTLS() const { return Utils::IsMutualTLS(&filter_callbacks_->connection()); } diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h index cc3c3b8aa33..a06daa9f852 100644 --- a/src/envoy/tcp/mixer/filter.h +++ b/src/envoy/tcp/mixer/filter.h @@ -52,8 +52,6 @@ class Filter : public Network::Filter, // CheckData virtual functions. bool GetSourceIpPort(std::string* str_ip, int* port) const override; bool GetPrincipal(bool peer, std::string* user) const override; - bool GetSourceNamespace(const std::string& principal, - std::string* ns) const override; bool IsMutualTLS() const override; bool GetRequestedServerName(std::string* name) const override; diff --git a/src/envoy/utils/BUILD b/src/envoy/utils/BUILD index b3ed537cd99..3a3a9d1a11d 100644 --- a/src/envoy/utils/BUILD +++ b/src/envoy/utils/BUILD @@ -36,6 +36,7 @@ envoy_cc_library( "//include/istio/utils:attribute_names_header", "//src/istio/authn:context_proto", "//src/istio/utils:attribute_names_lib", + "//src/istio/utils:utils_lib", ":filter_names_lib", "@envoy//source/exe:envoy_common_lib", ], diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index 151d78cb567..9c915eaca3d 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -17,8 +17,8 @@ #include "common/common/base64.h" #include "include/istio/utils/attribute_names.h" #include "src/envoy/utils/filter_names.h" -#include "src/envoy/utils/utils.h" #include "src/istio/authn/context.pb.h" +#include "src/istio/utils/utils.h" using istio::authn::Result; @@ -49,7 +49,7 @@ void Authentication::SaveAuthAttributesToStruct( setKeyValue(data, istio::utils::AttributeName::kSourcePrincipal, result.peer_user()); std::string source_ns(""); - if (GetSourceNamespace(result.peer_user(), &source_ns)) { + if (istio::utils::GetSourceNamespace(result.peer_user(), &source_ns)) { setKeyValue(data, istio::utils::AttributeName::kSourceNamespace, source_ns); } diff --git a/src/envoy/utils/authn_test.cc b/src/envoy/utils/authn_test.cc index c4001dd3740..93ab445c623 100644 --- a/src/envoy/utils/authn_test.cc +++ b/src/envoy/utils/authn_test.cc @@ -41,7 +41,7 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { EXPECT_TRUE(data.mutable_fields()->empty()); result.set_principal("principal"); - result.set_peer_user("sa/peeruser/ns/abc/"); + result.set_peer_user("cluster.local/sa/peeruser/ns/abc/"); auto origin = result.mutable_origin(); origin->add_audiences("audiences0"); origin->add_audiences("audiences1"); @@ -62,11 +62,11 @@ TEST_F(AuthenticationTest, SaveAuthAttributesToStruct) { "principal"); EXPECT_EQ( data.fields().at(istio::utils::AttributeName::kSourceUser).string_value(), - "sa/peeruser/ns/abc/"); + "cluster.local/sa/peeruser/ns/abc/"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kSourcePrincipal) .string_value(), - "sa/peeruser/ns/abc/"); + "cluster.local/sa/peeruser/ns/abc/"); EXPECT_EQ(data.fields() .at(istio::utils::AttributeName::kSourceNamespace) .string_value(), diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index 03352e1da92..6197cacedf5 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -33,8 +33,6 @@ const std::string kPerHostMetadataKey("istio"); // Attribute field for per-host data override const std::string kMetadataDestinationUID("uid"); -const std::string kNamespacePrefix("ns/"); - } // namespace void ExtractHeaders(const Http::HeaderMap& header_map, @@ -120,25 +118,6 @@ bool GetPrincipal(const Network::Connection* connection, bool peer, return false; } -bool GetSourceNamespace(const std::string& principal, - std::string* source_namespace) { - if (source_namespace) { - // The namespace is a substring in principal with format: "ns//". - size_t begin = principal.find(kNamespacePrefix); - if (begin == std::string::npos) { - return false; - } - begin += kNamespacePrefix.length(); - size_t end = principal.find("/", begin); - if (end == std::string::npos) { - return false; - } - *source_namespace = principal.substr(begin, end - begin); - return true; - } - return false; -} - bool IsMutualTLS(const Network::Connection* connection) { return connection != nullptr && connection->ssl() != nullptr && connection->ssl()->peerCertificatePresented(); diff --git a/src/envoy/utils/utils.h b/src/envoy/utils/utils.h index c7e8ba52062..6ffde52c3c9 100644 --- a/src/envoy/utils/utils.h +++ b/src/envoy/utils/utils.h @@ -41,10 +41,6 @@ bool GetDestinationUID(const envoy::api::v2::core::Metadata& metadata, bool GetPrincipal(const Network::Connection* connection, bool peer, std::string* principal); -// Get source.namespace attribute from principal. -bool GetSourceNamespace(const std::string& principal, - std::string* source_namespace); - // Returns true if connection is mutual TLS enabled. bool IsMutualTLS(const Network::Connection* connection); diff --git a/src/envoy/utils/utils_test.cc b/src/envoy/utils/utils_test.cc index f7094d593fa..8d38b995e77 100644 --- a/src/envoy/utils/utils_test.cc +++ b/src/envoy/utils/utils_test.cc @@ -44,31 +44,4 @@ TEST(UtilsTest, ParseMessageWithUnknownField) { EXPECT_EQ(http_config.default_destination_service(), "service.svc.cluster.local"); } - -TEST(UtilsTest, GetSourceNamespace) { - std::string ns = ""; - EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("", &ns)); - EXPECT_EQ("", ns); - - EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("ns/abc", &ns)); - EXPECT_EQ("", ns); - - EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("ns/", &ns)); - EXPECT_EQ("", ns); - - EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("s/abc/", &ns)); - EXPECT_EQ("", ns); - - EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("NS/abc/", &ns)); - EXPECT_EQ("", ns); - - EXPECT_EQ(false, Envoy::Utils::GetSourceNamespace("abc", &ns)); - EXPECT_EQ("", ns); - - EXPECT_EQ(true, Envoy::Utils::GetSourceNamespace("ns/abc/", &ns)); - EXPECT_EQ("abc", ns); - - EXPECT_EQ(true, Envoy::Utils::GetSourceNamespace("ns//", &ns)); - EXPECT_EQ("", ns); -} } // namespace diff --git a/src/istio/control/tcp/BUILD b/src/istio/control/tcp/BUILD index 5794abd1771..438f7de4d02 100644 --- a/src/istio/control/tcp/BUILD +++ b/src/istio/control/tcp/BUILD @@ -31,6 +31,7 @@ cc_library( "//include/istio/utils:attribute_names_header", "//src/istio/control:common_lib", "//src/istio/utils:attribute_names_lib", + "//src/istio/utils:utils_lib", ], ) @@ -45,6 +46,7 @@ cc_test( linkstatic = 1, deps = [ ":control_lib", + "//src/istio/utils:utils_lib", "//external:googletest_main", ], ) diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index 40cf18dbf62..595a5d9d7a6 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -14,6 +14,7 @@ */ #include "src/istio/control/tcp/attributes_builder.h" +#include "src/istio/utils/utils.h" #include "include/istio/utils/attribute_names.h" #include "include/istio/utils/attributes_builder.h" @@ -50,7 +51,7 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { builder.AddString(utils::AttributeName::kSourceUser, source_user); builder.AddString(utils::AttributeName::kSourcePrincipal, source_user); std::string source_ns(""); - if (check_data->GetSourceNamespace(source_user, &source_ns)) { + if (utils::GetSourceNamespace(source_user, &source_ns)) { builder.AddString(utils::AttributeName::kSourceNamespace, source_ns); } } diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index a936e461cac..f56b7438810 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -22,6 +22,7 @@ #include "include/istio/utils/attributes_builder.h" #include "src/istio/control/tcp/mock_check_data.h" #include "src/istio/control/tcp/mock_report_data.h" +#include "src/istio/utils/utils.h" using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; @@ -82,13 +83,13 @@ attributes { attributes { key: "source.principal" value { - string_value: "sa/test_user/ns/test_ns/" + string_value: "cluster.local/sa/test_user/ns/test_ns/" } } attributes { key: "source.user" value { - string_value: "sa/test_user/ns/test_ns/" + string_value: "cluster.local/sa/test_user/ns/test_ns/" } } attributes { @@ -378,7 +379,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string* user) -> bool { if (peer) { - *user = "sa/test_user/ns/test_ns/"; + *user = "cluster.local/sa/test_user/ns/test_ns/"; } else { *user = "destination_user"; } @@ -387,7 +388,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetSourceNamespace(_, _)) .WillRepeatedly( Invoke([](const std::string& principal, std::string* ns) -> bool { - if (principal == "sa/test_user/ns/test_ns/") { + if (principal == "cluster.local/sa/test_user/ns/test_ns/") { *ns = "test_ns"; return true; } diff --git a/src/istio/utils/BUILD b/src/istio/utils/BUILD index a1ec20a2851..1eac6ef4e5e 100644 --- a/src/istio/utils/BUILD +++ b/src/istio/utils/BUILD @@ -19,6 +19,10 @@ cc_library( srcs = [ "protobuf.cc", "status.cc", + "utils.cc" + ], + hdrs = [ + "utils.h", ], visibility = ["//visibility:public"], deps = [ @@ -27,6 +31,16 @@ cc_library( ], ) +cc_test( + name = "utils_test", + size = "small", + srcs = ["utils_test.cc"], + deps = [ + ":utils_lib", + "//external:googletest_main", + ], +) + cc_library( name = "md5_lib", srcs = ["md5.cc"], diff --git a/src/istio/utils/utils.cc b/src/istio/utils/utils.cc new file mode 100644 index 00000000000..4ef7b4bf3d0 --- /dev/null +++ b/src/istio/utils/utils.cc @@ -0,0 +1,61 @@ +/* Copyright 2018 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/istio/utils/utils.h" + +#include +#include + +namespace istio { +namespace utils { + +namespace { +const std::string kNamespaceKey("ns"); +const char kDelimiter = '/'; +} // namespace + +bool GetSourceNamespace(const std::string& principal, + std::string* source_namespace) { + if (source_namespace) { + // The namespace is a substring in principal with format: + // "/ns//sa/". '/' is not allowed to + // appear in actual content except as delimiter between tokens. + // The following algorithm extracts the namespace based on the above format + // but is a little more flexible. It assumes that the principal begins with + // a string followed by pairs separated by '/'. + + // Spilt the principal into tokens separated by kDelimiter. + std::stringstream ss(principal); + std::vector tokens; + std::string token; + while (std::getline(ss, token, kDelimiter)) { + tokens.push_back(token); + } + + // Skip the first string and check remaining pairs. + for (int i = 1; i < tokens.size(); i += 2) { + if (tokens[i] == kNamespaceKey) { + // Found the namespace key, treat the following token as namespace. + int j = i + 1; + *source_namespace = (j < tokens.size() ? tokens[j] : ""); + return true; + } + } + } + return false; +} + +} // namespace utils +} // namespace istio diff --git a/src/istio/utils/utils.h b/src/istio/utils/utils.h new file mode 100644 index 00000000000..063c18ca402 --- /dev/null +++ b/src/istio/utils/utils.h @@ -0,0 +1,28 @@ +/* Copyright 2018 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 + +namespace istio { +namespace utils { + +// Get source.namespace attribute from principal. +bool GetSourceNamespace(const std::string& principal, + std::string* source_namespace); + +} // namespace utils +} // namespace istio diff --git a/src/istio/utils/utils_test.cc b/src/istio/utils/utils_test.cc new file mode 100644 index 00000000000..303bfd51294 --- /dev/null +++ b/src/istio/utils/utils_test.cc @@ -0,0 +1,69 @@ +/* Copyright 2018 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/istio/utils/utils.h" +#include "gtest/gtest.h" + +namespace istio { +namespace utils { +namespace { + +class UtilsTest : public ::testing::Test { + protected: + void checkFalse(const std::string& principal) { + std::string output_ns = "none"; + EXPECT_FALSE(GetSourceNamespace(principal, &output_ns)); + EXPECT_EQ(output_ns, output_ns); + } + + void checkTrue(const std::string& principal, const std::string& ns) { + std::string output_ns = "none"; + EXPECT_TRUE(GetSourceNamespace(principal, &output_ns)); + EXPECT_EQ(ns, output_ns); + } +}; + +TEST_F(UtilsTest, GetSourceNamespace) { + checkFalse(""); + checkFalse("ns/abc"); + checkFalse("sa/user/ns/abc"); + checkFalse("ns/abc/sa/user"); + checkFalse("cluster.local"); + checkFalse("cluster.local/"); + checkFalse("cluster.local/sa/user"); + checkFalse("cluster.local/sa/user_ns/"); + checkFalse("cluster.local/sa/user_ns/abc/xyz"); + checkFalse("cluster.local/NS/abc"); + + checkTrue("cluster.local/ns", ""); + checkTrue("cluster.local/ns/", ""); + checkTrue("cluster.local/ns//", ""); + checkTrue("cluster.local/sa/user/ns", ""); + checkTrue("cluster.local/sa/user/ns/", ""); + checkTrue("cluster.local/ns//sa/user", ""); + + checkTrue("cluster.local/ns/abc_ns", "abc_ns"); + checkTrue("cluster.local/ns/abc_ns/", "abc_ns"); + checkTrue("cluster.local/ns/abc_ns/sa/user_ns", "abc_ns"); + checkTrue("cluster.local/ns/abc_ns/sa/user_ns/other/xyz", "abc_ns"); + checkTrue("cluster.local/sa/user_ns/ns/abc", "abc"); + checkTrue("cluster.local/sa/user_ns/ns/abc/", "abc"); + checkTrue("cluster.local/sa/user_ns/ns/abc_ns", "abc_ns"); + checkTrue("cluster.local/sa/user_ns/ns/abc_ns/", "abc_ns"); +} + +} // namespace +} // namespace utils +} // namespace istio From ce969b2b8febe94c4e1ead26bb4b9544ebe07682 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Thu, 16 Aug 2018 14:12:08 -0700 Subject: [PATCH 4/9] address comment --- src/envoy/http/authn/http_filter_test.cc | 2 +- .../http/authn/origin_authenticator_test.cc | 2 +- .../http/authn/peer_authenticator_test.cc | 2 +- .../http/jwt_auth/jwt_authenticator_test.cc | 2 +- .../http/jwt_auth/token_extractor_test.cc | 2 +- .../api_spec/http_api_spec_parser_test.cc | 2 +- .../control/http/attributes_builder_test.cc | 2 +- .../control/http/request_handler_impl_test.cc | 2 +- .../control/tcp/attributes_builder_test.cc | 11 +----- src/istio/control/tcp/mock_check_data.h | 2 -- .../control/tcp/request_handler_impl_test.cc | 2 +- src/istio/mixerclient/client_impl_test.cc | 2 +- src/istio/mixerclient/quota_cache_test.cc | 2 +- src/istio/mixerclient/report_batch_test.cc | 2 +- src/istio/utils/utils.cc | 36 ++++++++++++------- src/istio/utils/utils_test.cc | 4 +-- 16 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index 68d1edbbb74..aca52b5e365 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -31,12 +31,12 @@ using Envoy::Http::Istio::AuthN::FilterContext; using istio::authn::Payload; using istio::authn::Result; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; -using testing::_; using testing::AtLeast; using testing::Invoke; using testing::NiceMock; using testing::ReturnRef; using testing::StrictMock; +using testing::_; namespace iaapi = istio::authentication::v1alpha1; diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index a2a2cd7e858..0a4211692f0 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -27,13 +27,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; using istio::authn::Result; -using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; +using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index cce09dbdc99..60f04a827ec 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -26,13 +26,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; -using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; +using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index fd35224e7fe..e3acc734f89 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -22,9 +22,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; -using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; +using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/token_extractor_test.cc b/src/envoy/http/jwt_auth/token_extractor_test.cc index 8c5dac0d69f..f4befee63b3 100644 --- a/src/envoy/http/jwt_auth/token_extractor_test.cc +++ b/src/envoy/http/jwt_auth/token_extractor_test.cc @@ -19,9 +19,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; -using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; +using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/istio/api_spec/http_api_spec_parser_test.cc b/src/istio/api_spec/http_api_spec_parser_test.cc index db46b2acbef..ff7a409a066 100644 --- a/src/istio/api_spec/http_api_spec_parser_test.cc +++ b/src/istio/api_spec/http_api_spec_parser_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::config::client::HTTPAPISpec; using ::istio::utils::AttributesBuilder; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace api_spec { diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index dca943d9e9a..023071b9c4f 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -29,8 +29,8 @@ using ::google::protobuf::util::MessageDifferencer; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::Attributes_StringMap; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc index dcbbaf3d49f..f52300b9564 100644 --- a/src/istio/control/http/request_handler_impl_test.cc +++ b/src/istio/control/http/request_handler_impl_test.cc @@ -35,8 +35,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index f56b7438810..6ee76fe6ef7 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -27,9 +27,9 @@ using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; -using ::testing::_; using ::testing::Invoke; using ::testing::Return; +using ::testing::_; namespace istio { namespace control { @@ -385,15 +385,6 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { } return true; })); - EXPECT_CALL(mock_data, GetSourceNamespace(_, _)) - .WillRepeatedly( - Invoke([](const std::string& principal, std::string* ns) -> bool { - if (principal == "cluster.local/sa/test_user/ns/test_ns/") { - *ns = "test_ns"; - return true; - } - return false; - })); EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5")); EXPECT_CALL(mock_data, GetRequestedServerName(_)) .WillOnce(Invoke([](std::string* name) -> bool { diff --git a/src/istio/control/tcp/mock_check_data.h b/src/istio/control/tcp/mock_check_data.h index 20954111bb9..16cb7752553 100644 --- a/src/istio/control/tcp/mock_check_data.h +++ b/src/istio/control/tcp/mock_check_data.h @@ -28,8 +28,6 @@ class MockCheckData : public CheckData { public: MOCK_CONST_METHOD2(GetSourceIpPort, bool(std::string* ip, int* port)); MOCK_CONST_METHOD2(GetPrincipal, bool(bool peer, std::string* user)); - MOCK_CONST_METHOD2(GetSourceNamespace, - bool(const std::string&, std::string* ns)); MOCK_CONST_METHOD0(IsMutualTLS, bool()); MOCK_CONST_METHOD1(GetRequestedServerName, bool(std::string* name)); MOCK_CONST_METHOD0(GetConnectionId, std::string()); diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index 259fd06d7ce..9f978b171ce 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -31,8 +31,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc index ee38af82753..f15022239ce 100644 --- a/src/istio/mixerclient/client_impl_test.cc +++ b/src/istio/mixerclient/client_impl_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixerclient::CheckResponseInfo; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/quota_cache_test.cc b/src/istio/mixerclient/quota_cache_test.cc index 9f691a3dd19..9d613db8874 100644 --- a/src/istio/mixerclient/quota_cache_test.cc +++ b/src/istio/mixerclient/quota_cache_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/report_batch_test.cc b/src/istio/mixerclient/report_batch_test.cc index 8893d8e4535..aec33ab251b 100644 --- a/src/istio/mixerclient/report_batch_test.cc +++ b/src/istio/mixerclient/report_batch_test.cc @@ -23,8 +23,8 @@ using ::google::protobuf::util::error::Code; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace mixerclient { diff --git a/src/istio/utils/utils.cc b/src/istio/utils/utils.cc index 4ef7b4bf3d0..548913fb8d7 100644 --- a/src/istio/utils/utils.cc +++ b/src/istio/utils/utils.cc @@ -36,22 +36,34 @@ bool GetSourceNamespace(const std::string& principal, // but is a little more flexible. It assumes that the principal begins with // a string followed by pairs separated by '/'. - // Spilt the principal into tokens separated by kDelimiter. - std::stringstream ss(principal); - std::vector tokens; - std::string token; - while (std::getline(ss, token, kDelimiter)) { - tokens.push_back(token); + // Skip the first string and check remaining pairs. + size_t begin = principal.find(kDelimiter); + if (begin == std::string::npos) { + return false; } + begin += 1; - // Skip the first string and check remaining pairs. - for (int i = 1; i < tokens.size(); i += 2) { - if (tokens[i] == kNamespaceKey) { - // Found the namespace key, treat the following token as namespace. - int j = i + 1; - *source_namespace = (j < tokens.size() ? tokens[j] : ""); + while (1) { + size_t key_end = principal.find(kDelimiter, begin); + if (key_end == std::string::npos) { + return false; + } + + size_t value_begin = key_end + 1; + size_t value_end = principal.find(kDelimiter, value_begin); + + if (principal.compare(begin, key_end - begin, kNamespaceKey) == 0) { + size_t len = (value_end == std::string::npos ? value_end + : value_end - value_begin); + *source_namespace = principal.substr(value_begin, len); return true; } + + if (value_end == std::string::npos) { + return false; + } + + begin = value_end + 1; } } return false; diff --git a/src/istio/utils/utils_test.cc b/src/istio/utils/utils_test.cc index 303bfd51294..a84e8444ad2 100644 --- a/src/istio/utils/utils_test.cc +++ b/src/istio/utils/utils_test.cc @@ -42,15 +42,15 @@ TEST_F(UtilsTest, GetSourceNamespace) { checkFalse("ns/abc/sa/user"); checkFalse("cluster.local"); checkFalse("cluster.local/"); + checkFalse("cluster.local/ns"); checkFalse("cluster.local/sa/user"); + checkFalse("cluster.local/sa/user/ns"); checkFalse("cluster.local/sa/user_ns/"); checkFalse("cluster.local/sa/user_ns/abc/xyz"); checkFalse("cluster.local/NS/abc"); - checkTrue("cluster.local/ns", ""); checkTrue("cluster.local/ns/", ""); checkTrue("cluster.local/ns//", ""); - checkTrue("cluster.local/sa/user/ns", ""); checkTrue("cluster.local/sa/user/ns/", ""); checkTrue("cluster.local/ns//sa/user", ""); From 6e8144e3152fa63470c657fcf048cf898ecde4a9 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Thu, 16 Aug 2018 14:24:54 -0700 Subject: [PATCH 5/9] fix format --- src/envoy/http/authn/http_filter_test.cc | 2 +- src/envoy/http/authn/origin_authenticator_test.cc | 2 +- src/envoy/http/authn/peer_authenticator_test.cc | 2 +- src/envoy/http/jwt_auth/jwt_authenticator_test.cc | 2 +- src/envoy/http/jwt_auth/token_extractor_test.cc | 2 +- src/istio/api_spec/http_api_spec_parser_test.cc | 2 +- src/istio/control/http/attributes_builder_test.cc | 2 +- src/istio/control/http/request_handler_impl_test.cc | 2 +- src/istio/control/tcp/attributes_builder_test.cc | 2 +- src/istio/control/tcp/request_handler_impl_test.cc | 2 +- src/istio/mixerclient/client_impl_test.cc | 2 +- src/istio/mixerclient/quota_cache_test.cc | 2 +- src/istio/mixerclient/report_batch_test.cc | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index aca52b5e365..68d1edbbb74 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -31,12 +31,12 @@ using Envoy::Http::Istio::AuthN::FilterContext; using istio::authn::Payload; using istio::authn::Result; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; +using testing::_; using testing::AtLeast; using testing::Invoke; using testing::NiceMock; using testing::ReturnRef; using testing::StrictMock; -using testing::_; namespace iaapi = istio::authentication::v1alpha1; diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index 0a4211692f0..a2a2cd7e858 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -27,13 +27,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; using istio::authn::Result; +using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; -using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index 60f04a827ec..cce09dbdc99 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -26,13 +26,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; +using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; -using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index e3acc734f89..fd35224e7fe 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -22,9 +22,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/token_extractor_test.cc b/src/envoy/http/jwt_auth/token_extractor_test.cc index f4befee63b3..8c5dac0d69f 100644 --- a/src/envoy/http/jwt_auth/token_extractor_test.cc +++ b/src/envoy/http/jwt_auth/token_extractor_test.cc @@ -19,9 +19,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/istio/api_spec/http_api_spec_parser_test.cc b/src/istio/api_spec/http_api_spec_parser_test.cc index ff7a409a066..db46b2acbef 100644 --- a/src/istio/api_spec/http_api_spec_parser_test.cc +++ b/src/istio/api_spec/http_api_spec_parser_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::config::client::HTTPAPISpec; using ::istio::utils::AttributesBuilder; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace api_spec { diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 023071b9c4f..dca943d9e9a 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -29,8 +29,8 @@ using ::google::protobuf::util::MessageDifferencer; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::Attributes_StringMap; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc index f52300b9564..dcbbaf3d49f 100644 --- a/src/istio/control/http/request_handler_impl_test.cc +++ b/src/istio/control/http/request_handler_impl_test.cc @@ -35,8 +35,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index 6ee76fe6ef7..6de42b7cf95 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -27,9 +27,9 @@ using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; +using ::testing::_; using ::testing::Invoke; using ::testing::Return; -using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index 9f978b171ce..259fd06d7ce 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -31,8 +31,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc index f15022239ce..ee38af82753 100644 --- a/src/istio/mixerclient/client_impl_test.cc +++ b/src/istio/mixerclient/client_impl_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixerclient::CheckResponseInfo; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/quota_cache_test.cc b/src/istio/mixerclient/quota_cache_test.cc index 9d613db8874..9f691a3dd19 100644 --- a/src/istio/mixerclient/quota_cache_test.cc +++ b/src/istio/mixerclient/quota_cache_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/report_batch_test.cc b/src/istio/mixerclient/report_batch_test.cc index aec33ab251b..8893d8e4535 100644 --- a/src/istio/mixerclient/report_batch_test.cc +++ b/src/istio/mixerclient/report_batch_test.cc @@ -23,8 +23,8 @@ using ::google::protobuf::util::error::Code; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { From 927aab8311166b371a28cd9970f4bc4970a56d19 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Fri, 17 Aug 2018 10:58:54 -0700 Subject: [PATCH 6/9] simplify the code. --- src/envoy/http/authn/http_filter_test.cc | 2 +- .../http/authn/origin_authenticator_test.cc | 2 +- .../http/authn/peer_authenticator_test.cc | 2 +- .../http/jwt_auth/jwt_authenticator_test.cc | 2 +- .../http/jwt_auth/token_extractor_test.cc | 2 +- .../api_spec/http_api_spec_parser_test.cc | 2 +- .../control/http/attributes_builder_test.cc | 2 +- .../control/http/request_handler_impl_test.cc | 2 +- .../control/tcp/attributes_builder_test.cc | 2 +- .../control/tcp/request_handler_impl_test.cc | 2 +- src/istio/mixerclient/client_impl_test.cc | 2 +- src/istio/mixerclient/quota_cache_test.cc | 2 +- src/istio/mixerclient/report_batch_test.cc | 2 +- src/istio/utils/utils.cc | 38 ++++--------------- src/istio/utils/utils_test.cc | 3 -- 15 files changed, 20 insertions(+), 47 deletions(-) diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index 68d1edbbb74..aca52b5e365 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -31,12 +31,12 @@ using Envoy::Http::Istio::AuthN::FilterContext; using istio::authn::Payload; using istio::authn::Result; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; -using testing::_; using testing::AtLeast; using testing::Invoke; using testing::NiceMock; using testing::ReturnRef; using testing::StrictMock; +using testing::_; namespace iaapi = istio::authentication::v1alpha1; diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index a2a2cd7e858..0a4211692f0 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -27,13 +27,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; using istio::authn::Result; -using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; +using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index cce09dbdc99..60f04a827ec 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -26,13 +26,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; -using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; +using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index fd35224e7fe..e3acc734f89 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -22,9 +22,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; -using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; +using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/token_extractor_test.cc b/src/envoy/http/jwt_auth/token_extractor_test.cc index 8c5dac0d69f..f4befee63b3 100644 --- a/src/envoy/http/jwt_auth/token_extractor_test.cc +++ b/src/envoy/http/jwt_auth/token_extractor_test.cc @@ -19,9 +19,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; -using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; +using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/istio/api_spec/http_api_spec_parser_test.cc b/src/istio/api_spec/http_api_spec_parser_test.cc index db46b2acbef..ff7a409a066 100644 --- a/src/istio/api_spec/http_api_spec_parser_test.cc +++ b/src/istio/api_spec/http_api_spec_parser_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::config::client::HTTPAPISpec; using ::istio::utils::AttributesBuilder; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace api_spec { diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index dca943d9e9a..023071b9c4f 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -29,8 +29,8 @@ using ::google::protobuf::util::MessageDifferencer; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::Attributes_StringMap; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc index dcbbaf3d49f..f52300b9564 100644 --- a/src/istio/control/http/request_handler_impl_test.cc +++ b/src/istio/control/http/request_handler_impl_test.cc @@ -35,8 +35,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index 6de42b7cf95..6ee76fe6ef7 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -27,9 +27,9 @@ using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; -using ::testing::_; using ::testing::Invoke; using ::testing::Return; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index 259fd06d7ce..9f978b171ce 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -31,8 +31,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc index ee38af82753..f15022239ce 100644 --- a/src/istio/mixerclient/client_impl_test.cc +++ b/src/istio/mixerclient/client_impl_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixerclient::CheckResponseInfo; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/quota_cache_test.cc b/src/istio/mixerclient/quota_cache_test.cc index 9f691a3dd19..9d613db8874 100644 --- a/src/istio/mixerclient/quota_cache_test.cc +++ b/src/istio/mixerclient/quota_cache_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; using ::istio::quota_config::Requirement; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/report_batch_test.cc b/src/istio/mixerclient/report_batch_test.cc index 8893d8e4535..aec33ab251b 100644 --- a/src/istio/mixerclient/report_batch_test.cc +++ b/src/istio/mixerclient/report_batch_test.cc @@ -23,8 +23,8 @@ using ::google::protobuf::util::error::Code; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; -using ::testing::_; using ::testing::Invoke; +using ::testing::_; namespace istio { namespace mixerclient { diff --git a/src/istio/utils/utils.cc b/src/istio/utils/utils.cc index 548913fb8d7..0d6151a9ca8 100644 --- a/src/istio/utils/utils.cc +++ b/src/istio/utils/utils.cc @@ -22,7 +22,7 @@ namespace istio { namespace utils { namespace { -const std::string kNamespaceKey("ns"); +const std::string kNamespaceKey("/ns/"); const char kDelimiter = '/'; } // namespace @@ -32,39 +32,15 @@ bool GetSourceNamespace(const std::string& principal, // The namespace is a substring in principal with format: // "/ns//sa/". '/' is not allowed to // appear in actual content except as delimiter between tokens. - // The following algorithm extracts the namespace based on the above format - // but is a little more flexible. It assumes that the principal begins with - // a string followed by pairs separated by '/'. - - // Skip the first string and check remaining pairs. - size_t begin = principal.find(kDelimiter); + size_t begin = principal.find(kNamespaceKey); if (begin == std::string::npos) { return false; } - begin += 1; - - while (1) { - size_t key_end = principal.find(kDelimiter, begin); - if (key_end == std::string::npos) { - return false; - } - - size_t value_begin = key_end + 1; - size_t value_end = principal.find(kDelimiter, value_begin); - - if (principal.compare(begin, key_end - begin, kNamespaceKey) == 0) { - size_t len = (value_end == std::string::npos ? value_end - : value_end - value_begin); - *source_namespace = principal.substr(value_begin, len); - return true; - } - - if (value_end == std::string::npos) { - return false; - } - - begin = value_end + 1; - } + begin += kNamespaceKey.length(); + size_t end = principal.find(kDelimiter, begin); + size_t len = (end == std::string::npos ? end : end - begin); + *source_namespace = principal.substr(begin, len); + return true; } return false; } diff --git a/src/istio/utils/utils_test.cc b/src/istio/utils/utils_test.cc index a84e8444ad2..dd3c34feff4 100644 --- a/src/istio/utils/utils_test.cc +++ b/src/istio/utils/utils_test.cc @@ -37,9 +37,6 @@ class UtilsTest : public ::testing::Test { TEST_F(UtilsTest, GetSourceNamespace) { checkFalse(""); - checkFalse("ns/abc"); - checkFalse("sa/user/ns/abc"); - checkFalse("ns/abc/sa/user"); checkFalse("cluster.local"); checkFalse("cluster.local/"); checkFalse("cluster.local/ns"); From e6b6eceb92954c4334a6ba939233847f99a01a56 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Tue, 21 Aug 2018 19:06:23 -0700 Subject: [PATCH 7/9] make check. --- src/envoy/http/authn/http_filter_test.cc | 2 +- src/envoy/http/authn/origin_authenticator_test.cc | 2 +- src/envoy/http/authn/peer_authenticator_test.cc | 2 +- src/envoy/http/jwt_auth/jwt_authenticator_test.cc | 2 +- src/envoy/http/jwt_auth/token_extractor_test.cc | 2 +- src/istio/api_spec/http_api_spec_parser_test.cc | 2 +- src/istio/control/http/attributes_builder_test.cc | 2 +- src/istio/control/http/request_handler_impl_test.cc | 2 +- src/istio/control/tcp/attributes_builder_test.cc | 2 +- src/istio/control/tcp/request_handler_impl_test.cc | 2 +- src/istio/mixerclient/client_impl_test.cc | 2 +- src/istio/mixerclient/quota_cache_test.cc | 2 +- src/istio/mixerclient/report_batch_test.cc | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index aca52b5e365..68d1edbbb74 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -31,12 +31,12 @@ using Envoy::Http::Istio::AuthN::FilterContext; using istio::authn::Payload; using istio::authn::Result; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; +using testing::_; using testing::AtLeast; using testing::Invoke; using testing::NiceMock; using testing::ReturnRef; using testing::StrictMock; -using testing::_; namespace iaapi = istio::authentication::v1alpha1; diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index 0a4211692f0..a2a2cd7e858 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -27,13 +27,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; using istio::authn::Result; +using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; -using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index 60f04a827ec..cce09dbdc99 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -26,13 +26,13 @@ namespace iaapi = istio::authentication::v1alpha1; using istio::authn::Payload; +using testing::_; using testing::DoAll; using testing::MockFunction; using testing::NiceMock; using testing::Return; using testing::SetArgPointee; using testing::StrictMock; -using testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index e3acc734f89..fd35224e7fe 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -22,9 +22,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/envoy/http/jwt_auth/token_extractor_test.cc b/src/envoy/http/jwt_auth/token_extractor_test.cc index f4befee63b3..8c5dac0d69f 100644 --- a/src/envoy/http/jwt_auth/token_extractor_test.cc +++ b/src/envoy/http/jwt_auth/token_extractor_test.cc @@ -19,9 +19,9 @@ using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; +using ::testing::_; using ::testing::Invoke; using ::testing::NiceMock; -using ::testing::_; namespace Envoy { namespace Http { diff --git a/src/istio/api_spec/http_api_spec_parser_test.cc b/src/istio/api_spec/http_api_spec_parser_test.cc index ff7a409a066..db46b2acbef 100644 --- a/src/istio/api_spec/http_api_spec_parser_test.cc +++ b/src/istio/api_spec/http_api_spec_parser_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::config::client::HTTPAPISpec; using ::istio::utils::AttributesBuilder; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace api_spec { diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 023071b9c4f..dca943d9e9a 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -29,8 +29,8 @@ using ::google::protobuf::util::MessageDifferencer; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::Attributes_StringMap; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc index f52300b9564..dcbbaf3d49f 100644 --- a/src/istio/control/http/request_handler_impl_test.cc +++ b/src/istio/control/http/request_handler_impl_test.cc @@ -35,8 +35,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index 6ee76fe6ef7..6de42b7cf95 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -27,9 +27,9 @@ using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; +using ::testing::_; using ::testing::Invoke; using ::testing::Return; -using ::testing::_; namespace istio { namespace control { diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index 9f978b171ce..259fd06d7ce 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -31,8 +31,8 @@ using ::istio::mixerclient::MixerClient; using ::istio::mixerclient::TransportCheckFunc; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace control { diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc index f15022239ce..ee38af82753 100644 --- a/src/istio/mixerclient/client_impl_test.cc +++ b/src/istio/mixerclient/client_impl_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixerclient::CheckResponseInfo; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/quota_cache_test.cc b/src/istio/mixerclient/quota_cache_test.cc index 9d613db8874..9f691a3dd19 100644 --- a/src/istio/mixerclient/quota_cache_test.cc +++ b/src/istio/mixerclient/quota_cache_test.cc @@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; using ::istio::quota_config::Requirement; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { diff --git a/src/istio/mixerclient/report_batch_test.cc b/src/istio/mixerclient/report_batch_test.cc index aec33ab251b..8893d8e4535 100644 --- a/src/istio/mixerclient/report_batch_test.cc +++ b/src/istio/mixerclient/report_batch_test.cc @@ -23,8 +23,8 @@ using ::google::protobuf::util::error::Code; using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; -using ::testing::Invoke; using ::testing::_; +using ::testing::Invoke; namespace istio { namespace mixerclient { From 5e60c2858ae71324362c3cb8c06c0eb413331b35 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Tue, 21 Aug 2018 19:11:57 -0700 Subject: [PATCH 8/9] small update --- .../control/http/attributes_builder_test.cc | 16 ++++++++-------- src/istio/control/tcp/attributes_builder_test.cc | 8 ++++---- src/istio/utils/utils_test.cc | 2 ++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index dca943d9e9a..a03c2469dec 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -227,19 +227,19 @@ attributes { attributes { key: "source.namespace" value { - string_value: "test_ns" + string_value: "ns_ns" } } attributes { key: "source.principal" value { - string_value: "sa/test_user/ns/test_ns/" + string_value: "sa/test_user/ns/ns_ns/" } } attributes { key: "source.user" value { - string_value: "sa/test_user/ns/test_ns/" + string_value: "sa/test_user/ns/ns_ns/" } } attributes { @@ -498,19 +498,19 @@ fields { fields { key: "source.namespace" value { - string_value: "test_ns" + string_value: "ns_ns" } } fields { key: "source.principal" value { - string_value: "sa/test_user/ns/test_ns/" + string_value: "sa/test_user/ns/ns_ns/" } } fields { key: "source.user" value { - string_value: "sa/test_user/ns/test_ns/" + string_value: "sa/test_user/ns/ns_ns/" } } )"; @@ -568,7 +568,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { if (peer) { - *user = "sa/test_user/ns/test_ns/"; + *user = "sa/test_user/ns/ns_ns/"; } else { *user = "destination_user"; } @@ -642,7 +642,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { if (peer) { - *user = "sa/test_user/ns/test_ns/"; + *user = "sa/test_user/ns/ns_ns/"; } else { *user = "destination_user"; } diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index 6de42b7cf95..faa78ccc06e 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -77,19 +77,19 @@ attributes { attributes { key: "source.namespace" value { - string_value: "test_ns" + string_value: "ns_ns" } } attributes { key: "source.principal" value { - string_value: "cluster.local/sa/test_user/ns/test_ns/" + string_value: "cluster.local/sa/test_user/ns/ns_ns/" } } attributes { key: "source.user" value { - string_value: "cluster.local/sa/test_user/ns/test_ns/" + string_value: "cluster.local/sa/test_user/ns/ns_ns/" } } attributes { @@ -379,7 +379,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetPrincipal(_, _)) .WillRepeatedly(Invoke([](bool peer, std::string* user) -> bool { if (peer) { - *user = "cluster.local/sa/test_user/ns/test_ns/"; + *user = "cluster.local/sa/test_user/ns/ns_ns/"; } else { *user = "destination_user"; } diff --git a/src/istio/utils/utils_test.cc b/src/istio/utils/utils_test.cc index dd3c34feff4..4282e4edccd 100644 --- a/src/istio/utils/utils_test.cc +++ b/src/istio/utils/utils_test.cc @@ -50,7 +50,9 @@ TEST_F(UtilsTest, GetSourceNamespace) { checkTrue("cluster.local/ns//", ""); checkTrue("cluster.local/sa/user/ns/", ""); checkTrue("cluster.local/ns//sa/user", ""); + checkTrue("cluster.local/ns//ns/ns", ""); + checkTrue("cluster.local/ns/ns/ns/ns", "ns"); checkTrue("cluster.local/ns/abc_ns", "abc_ns"); checkTrue("cluster.local/ns/abc_ns/", "abc_ns"); checkTrue("cluster.local/ns/abc_ns/sa/user_ns", "abc_ns"); From b2d79f18604126d80e69ab49ca045128a1f66f5d Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Wed, 22 Aug 2018 16:50:23 -0700 Subject: [PATCH 9/9] fix test. --- src/istio/control/http/attributes_builder_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index a03c2469dec..5f2d2429b3b 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -139,7 +139,7 @@ attributes { attributes { key: "source.principal" value { - string_value: "test_user" + string_value: "sa/test_user/ns/ns_ns/" } } )";