From 91c2423b62ed2b8334b725b73dd8aa8376d74963 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Thu, 14 Jun 2018 19:02:52 -0700 Subject: [PATCH 1/7] rbac: add metadata support to rbac filter. Signed-off-by: Yangmin Zhu --- api/envoy/api/v2/core/base.proto | 72 ++++++++++ api/envoy/config/rbac/v2alpha/BUILD | 2 + api/envoy/config/rbac/v2alpha/rbac.proto | 7 + source/common/config/metadata.cc | 55 ++++++++ source/common/config/metadata.h | 9 ++ .../extensions/filters/common/rbac/engine.h | 5 +- .../filters/common/rbac/engine_impl.cc | 7 +- .../filters/common/rbac/engine_impl.h | 4 +- .../filters/common/rbac/matchers.cc | 41 ++++-- .../extensions/filters/common/rbac/matchers.h | 46 ++++--- .../filters/http/rbac/rbac_filter.cc | 6 +- test/common/config/metadata_test.cc | 123 ++++++++++++++++++ .../filters/common/rbac/engine_impl_test.cc | 10 +- .../filters/common/rbac/matchers_test.cc | 31 ++++- test/extensions/filters/common/rbac/mocks.h | 4 +- .../filters/http/rbac/rbac_filter_test.cc | 2 +- 16 files changed, 374 insertions(+), 50 deletions(-) diff --git a/api/envoy/api/v2/core/base.proto b/api/envoy/api/v2/core/base.proto index ade2f025845e4..051655a4a00f6 100644 --- a/api/envoy/api/v2/core/base.proto +++ b/api/envoy/api/v2/core/base.proto @@ -95,6 +95,78 @@ message Metadata { map filter_metadata = 1; } +// MetadataMatcher provides a general interface to check if a given value is matched in +// :ref:`Metadata `. It uses `filter` and `path` to retrieve the value +// from the Metadata and then check if it's matched to one of the specified values. +// +// An example use of MetadataMatcher is specifying additional metadata in envoy.filters.http.rbac to +// enforce access control based on dynamic metadata in a request. +message MetadataMatcher { + // Specifies the value to match. Only primitive value is supported. + message Value { + // Specifies how to match a value. Only have effect on primitive value. + // For non-primitive value, it's always not matched. + oneof match_specifier { + option (validate.required) = true; + + // If specified, value match will be performed based on null value. It's matched if and only + // if the target value is a NullValue and this field is set to true. + bool null_match = 1; + + // If specified, value match will be performed based on double value. It's matched if and only + // if the target value is a double value and is equal to this field. + double number_match = 2; + + // If specified, value match will be performed based on string value. It's matched if and only + // if the target value is a string value and is equal to this field. + // + // Examples: + // + // * *abc* will only match to string value *abc*. + string exact_match = 3; + + // If specified, value match will be performed based on the prefix of the string value. + // It's matched if and only if the target value is a string value and has the prefix specified + // in this field. + // Note: empty prefix is not allowed, please use present_match instead. + // + // Examples: + // + // * *abc* will match to string value *abc.xyz* + string prefix_match = 4 [(validate.rules).string.min_bytes = 1]; + + // If specified, value match will be performed based on the suffix of the string value. + // It's matched if and only if the target value is a string value and has the suffix specified + // in this field. + // Note: empty suffix is not allowed, please use present_match instead. + // + // Examples: + // + // * *abc* will match to string value *xyz.abc* + string suffix_match = 5 [(validate.rules).string.min_bytes = 1]; + + // If specified, value match will be performed based on bool value. It's matched if and only + // if the target value is a bool value and is equal to this field. + bool bool_match = 6; + + // If specified, value match will be performed based on whether the path is referring to a + // valid primitive value in the metadata. + // Note: If the path is referring to a non-primitive value, it's always not matched. + bool present_match = 7; + } + } + + // Required. The filter name to retrieve the Struct from the Metadata. + string filter = 1 [(validate.rules).string.min_bytes = 1]; + + // Required. The multi-key path to retrieve the Value from the Struct. + repeated string path = 2 [(validate.rules).repeated .min_items = 1]; + + // Required. A set of values to match. The MetadataMatcher is matched if at least one value is + // matched, in other words, it's matched with OR semantics. + repeated Value values = 3 [(validate.rules).repeated .min_items = 1]; +} + // Runtime derived uint32 with a default when not specified. message RuntimeUInt32 { // Default value if runtime value is not available. diff --git a/api/envoy/config/rbac/v2alpha/BUILD b/api/envoy/config/rbac/v2alpha/BUILD index 396982264e3a3..0ff71cbd9c5e3 100644 --- a/api/envoy/config/rbac/v2alpha/BUILD +++ b/api/envoy/config/rbac/v2alpha/BUILD @@ -8,6 +8,7 @@ api_proto_library( visibility = ["//visibility:public"], deps = [ "//envoy/api/v2/core:address", + "//envoy/api/v2/core:base", "//envoy/api/v2/route", "//envoy/type:string_match", ], @@ -18,6 +19,7 @@ api_go_proto_library( proto = ":rbac", deps = [ "//envoy/api/v2/core:address_go_proto", + "//envoy/api/v2/core:base_go_proto", "//envoy/api/v2/route:route_go_proto", "//envoy/type:string_match_go_proto", ], diff --git a/api/envoy/config/rbac/v2alpha/rbac.proto b/api/envoy/config/rbac/v2alpha/rbac.proto index cb9e53b5d9b12..772e7375f7d33 100644 --- a/api/envoy/config/rbac/v2alpha/rbac.proto +++ b/api/envoy/config/rbac/v2alpha/rbac.proto @@ -2,6 +2,7 @@ syntax = "proto3"; import "validate/validate.proto"; import "envoy/api/v2/core/address.proto"; +import "envoy/api/v2/core/base.proto"; import "envoy/api/v2/route/route.proto"; package envoy.config.rbac.v2alpha; @@ -111,6 +112,9 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32.lte = 65535]; + + // A metadata that describes additional information about the action. + envoy.api.v2.core.MetadataMatcher metadata = 7; } } @@ -150,5 +154,8 @@ message Principal { // A header (or psuedo-header such as :path or :method) on the incoming HTTP request. envoy.api.v2.route.HeaderMatcher header = 6; + + // A metadata that describes additional information about the principal. + envoy.api.v2.core.MetadataMatcher metadata = 7; } } diff --git a/source/common/config/metadata.cc b/source/common/config/metadata.cc index 22a2d9f05be7a..6e18a06b246b0 100644 --- a/source/common/config/metadata.cc +++ b/source/common/config/metadata.cc @@ -1,5 +1,7 @@ #include "common/config/metadata.h" +#include "absl/strings/match.h" + namespace Envoy { namespace Config { @@ -47,5 +49,58 @@ ProtobufWkt::Value& Metadata::mutableMetadataValue(envoy::api::v2::core::Metadat return (*(*metadata.mutable_filter_metadata())[filter].mutable_fields())[key]; } +bool Metadata::match(const envoy::api::v2::core::MetadataMatcher& matcher, + const envoy::api::v2::core::Metadata& metadata) { + const std::vector path(matcher.path().begin(), matcher.path().end()); + const auto& value = metadataValue(metadata, matcher.filter(), path); + for (const envoy::api::v2::core::MetadataMatcher::Value& m : matcher.values()) { + switch (m.match_specifier_case()) { + case envoy::api::v2::core::MetadataMatcher_Value::kNullMatch: + if (value.kind_case() == ProtobufWkt::Value::kNullValue && m.null_match()) { + return true; + } + break; + case envoy::api::v2::core::MetadataMatcher_Value::kNumberMatch: + if (value.kind_case() == ProtobufWkt::Value::kNumberValue && + m.number_match() == value.number_value()) { + return true; + } + break; + case envoy::api::v2::core::MetadataMatcher_Value::kExactMatch: + if (value.kind_case() == ProtobufWkt::Value::kStringValue && + m.exact_match() == value.string_value()) { + return true; + } + break; + case envoy::api::v2::core::MetadataMatcher_Value::kPrefixMatch: + if (value.kind_case() == ProtobufWkt::Value::kStringValue && + absl::StartsWith(value.string_value(), m.prefix_match())) { + return true; + } + break; + case envoy::api::v2::core::MetadataMatcher_Value::kSuffixMatch: + if (value.kind_case() == ProtobufWkt::Value::kStringValue && + absl::EndsWith(value.string_value(), m.suffix_match())) { + return true; + } + break; + case envoy::api::v2::core::MetadataMatcher_Value::kBoolMatch: + if (value.kind_case() == ProtobufWkt::Value::kBoolValue && + m.bool_match() == value.bool_value()) { + return true; + } + break; + case envoy::api::v2::core::MetadataMatcher_Value::kPresentMatch: + if (value.kind_case() != ProtobufWkt::Value::KIND_NOT_SET && m.present_match()) { + return true; + } + break; + default: + break; + } + } + return false; +} + } // namespace Config } // namespace Envoy diff --git a/source/common/config/metadata.h b/source/common/config/metadata.h index 21432c9a7d739..07c9eb0579dec 100644 --- a/source/common/config/metadata.h +++ b/source/common/config/metadata.h @@ -44,6 +44,15 @@ class Metadata { static ProtobufWkt::Value& mutableMetadataValue(envoy::api::v2::core::Metadata& metadata, const std::string& filter, const std::string& key); + + /** + * Check whether the metadata is matched to the matcher. + * @param matcher the matcher with the match condition. + * @param metadata the metadata to check. + * @return true if it's matched otherwise false. + */ + static bool match(const envoy::api::v2::core::MetadataMatcher& matcher, + const envoy::api::v2::core::Metadata& metadata); }; } // namespace Config diff --git a/source/extensions/filters/common/rbac/engine.h b/source/extensions/filters/common/rbac/engine.h index a122f58af5471..75860a9573a24 100644 --- a/source/extensions/filters/common/rbac/engine.h +++ b/source/extensions/filters/common/rbac/engine.h @@ -23,9 +23,10 @@ class RoleBasedAccessControlEngine { * @param connection the downstream connection used to identify the action/principal. * @param headers the headers of the incoming request used to identify the action/principal. An * empty map should be used if there are no headers available. + * @param metadata the metadata with additional information about the action/principal. */ - virtual bool allowed(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const PURE; + virtual bool allowed(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const PURE; }; } // namespace RBAC diff --git a/source/extensions/filters/common/rbac/engine_impl.cc b/source/extensions/filters/common/rbac/engine_impl.cc index adf62d3eb7ac1..1608476f8efff 100644 --- a/source/extensions/filters/common/rbac/engine_impl.cc +++ b/source/extensions/filters/common/rbac/engine_impl.cc @@ -15,11 +15,12 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( } } -bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const { +bool RoleBasedAccessControlEngineImpl::allowed( + const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const { bool matched = false; for (const auto& policy : policies_) { - if (policy.matches(connection, headers)) { + if (policy.matches(connection, headers, metadata)) { matched = true; break; } diff --git a/source/extensions/filters/common/rbac/engine_impl.h b/source/extensions/filters/common/rbac/engine_impl.h index d0747e5342eb4..6be4d18798d27 100644 --- a/source/extensions/filters/common/rbac/engine_impl.h +++ b/source/extensions/filters/common/rbac/engine_impl.h @@ -15,8 +15,8 @@ class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine { public: RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v2alpha::RBAC& rules); - bool allowed(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool allowed(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const override; private: const bool allowed_if_matched_; diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index 9cead82116c70..be4dcade29f3e 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -1,6 +1,7 @@ #include "extensions/filters/common/rbac/matchers.h" #include "common/common/assert.h" +#include "common/config/metadata.h" namespace Envoy { namespace Extensions { @@ -22,6 +23,8 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v2alpha::Permis return std::make_shared(permission.destination_port()); case envoy::config::rbac::v2alpha::Permission::RuleCase::kAny: return std::make_shared(); + case envoy::config::rbac::v2alpha::Permission::RuleCase::kMetadata: + return std::make_shared(permission.metadata()); default: NOT_REACHED; } @@ -41,6 +44,8 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v2alpha::Princi return std::make_shared(principal.header()); case envoy::config::rbac::v2alpha::Principal::IdentifierCase::kAny: return std::make_shared(); + case envoy::config::rbac::v2alpha::Principal::IdentifierCase::kMetadata: + return std::make_shared(principal.metadata()); default: NOT_REACHED; } @@ -59,9 +64,10 @@ AndMatcher::AndMatcher(const envoy::config::rbac::v2alpha::Principal_Set& set) { } bool AndMatcher::matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const { + const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const { for (const auto& matcher : matchers_) { - if (!matcher->matches(connection, headers)) { + if (!matcher->matches(connection, headers, metadata)) { return false; } } @@ -84,9 +90,10 @@ OrMatcher::OrMatcher( } bool OrMatcher::matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const { + const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const { for (const auto& matcher : matchers_) { - if (matcher->matches(connection, headers)) { + if (matcher->matches(connection, headers, metadata)) { return true; } } @@ -94,27 +101,28 @@ bool OrMatcher::matches(const Network::Connection& connection, return false; } -bool HeaderMatcher::matches(const Network::Connection&, - const Envoy::Http::HeaderMap& headers) const { +bool HeaderMatcher::matches(const Network::Connection&, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const { return Envoy::Http::HeaderUtility::matchHeaders(headers, header_); } -bool IPMatcher::matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap&) const { +bool IPMatcher::matches(const Network::Connection& connection, const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata&) const { const Envoy::Network::Address::InstanceConstSharedPtr& ip = destination_ ? connection.localAddress() : connection.remoteAddress(); return range_.isInRange(*ip.get()); } -bool PortMatcher::matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap&) const { +bool PortMatcher::matches(const Network::Connection& connection, const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata&) const { const Envoy::Network::Address::Ip* ip = connection.localAddress().get()->ip(); return ip && ip->port() == port_; } bool AuthenticatedMatcher::matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap&) const { + const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata&) const { const auto* ssl = connection.ssl(); if (!ssl) { // connection was not authenticated return false; @@ -128,9 +136,16 @@ bool AuthenticatedMatcher::matches(const Network::Connection& connection, return principal == name_; } +bool MetadataMatcher::matches(const Network::Connection&, const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata& metadata) const { + return Envoy::Config::Metadata::match(matcher_, metadata); +} + bool PolicyMatcher::matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const { - return permissions_.matches(connection, headers) && principals_.matches(connection, headers); + const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const { + return permissions_.matches(connection, headers, metadata) && + principals_.matches(connection, headers, metadata); } } // namespace RBAC diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index 2c09288a08354..cc96e6eb89c77 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -32,8 +32,8 @@ class Matcher { * @param headers the request headers used to match against. An empty map should be used if * there are none headers available. */ - virtual bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const PURE; + virtual bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const PURE; /** * Creates a shared instance of a matcher based off the rules defined in the Permission config @@ -53,7 +53,8 @@ class Matcher { */ class AlwaysMatcher : public Matcher { public: - bool matches(const Network::Connection&, const Envoy::Http::HeaderMap&) const override { + bool matches(const Network::Connection&, const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata&) const override { return true; } }; @@ -67,8 +68,8 @@ class AndMatcher : public Matcher { AndMatcher(const envoy::config::rbac::v2alpha::Permission_Set& rules); AndMatcher(const envoy::config::rbac::v2alpha::Principal_Set& ids); - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: std::vector matchers_; @@ -85,8 +86,8 @@ class OrMatcher : public Matcher { OrMatcher(const Protobuf::RepeatedPtrField<::envoy::config::rbac::v2alpha::Permission>& rules); OrMatcher(const Protobuf::RepeatedPtrField<::envoy::config::rbac::v2alpha::Principal>& ids); - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: std::vector matchers_; @@ -100,8 +101,8 @@ class HeaderMatcher : public Matcher { public: HeaderMatcher(const envoy::api::v2::route::HeaderMatcher& matcher) : header_(matcher) {} - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: const Envoy::Http::HeaderUtility::HeaderData header_; @@ -116,8 +117,8 @@ class IPMatcher : public Matcher { IPMatcher(const envoy::api::v2::core::CidrRange& range, bool destination) : range_(Network::Address::CidrRange::create(range)), destination_(destination) {} - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: const Network::Address::CidrRange range_; @@ -131,8 +132,8 @@ class PortMatcher : public Matcher { public: PortMatcher(const uint32_t port) : port_(port) {} - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: const uint32_t port_; @@ -147,8 +148,8 @@ class AuthenticatedMatcher : public Matcher { AuthenticatedMatcher(const envoy::config::rbac::v2alpha::Principal_Authenticated& auth) : name_(auth.name()) {} - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: const std::string name_; @@ -163,14 +164,25 @@ class PolicyMatcher : public Matcher { PolicyMatcher(const envoy::config::rbac::v2alpha::Policy& policy) : permissions_(policy.permissions()), principals_(policy.principals()) {} - bool matches(const Network::Connection& connection, - const Envoy::Http::HeaderMap& headers) const override; + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata&) const override; private: const OrMatcher permissions_; const OrMatcher principals_; }; +class MetadataMatcher : public Matcher { +public: + MetadataMatcher(const envoy::api::v2::core::MetadataMatcher& matcher) : matcher_(matcher) {} + + bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, + const envoy::api::v2::core::Metadata& metadata) const override; + +private: + const envoy::api::v2::core::MetadataMatcher matcher_; +}; + } // namespace RBAC } // namespace Common } // namespace Filters diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index 9ef629f6e6841..78b0db13851f9 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -68,7 +68,8 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head const absl::optional& shadow_engine = config_->engine(callbacks_->route(), EnforcementMode::Shadow); if (shadow_engine.has_value()) { - if (shadow_engine->allowed(*callbacks_->connection(), headers)) { + if (shadow_engine->allowed(*callbacks_->connection(), headers, + callbacks_->requestInfo().dynamicMetadata())) { config_->stats().shadow_allowed_.inc(); } else { config_->stats().shadow_denied_.inc(); @@ -78,7 +79,8 @@ Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::Head const absl::optional& engine = config_->engine(callbacks_->route(), EnforcementMode::Enforced); if (engine.has_value()) { - if (engine->allowed(*callbacks_->connection(), headers)) { + if (engine->allowed(*callbacks_->connection(), headers, + callbacks_->requestInfo().dynamicMetadata())) { config_->stats().allowed_.inc(); return Http::FilterHeadersStatus::Continue; } else { diff --git a/test/common/config/metadata_test.cc b/test/common/config/metadata_test.cc index 072624a3f2f0c..7e006643009d5 100644 --- a/test/common/config/metadata_test.cc +++ b/test/common/config/metadata_test.cc @@ -43,6 +43,129 @@ TEST(MetadataTest, MetadataValuePath) { ProtobufWkt::Value::KindCase::KIND_NOT_SET); } +TEST(MetadataTest, MatchNullValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_null_value(ProtobufWkt::NullValue::NULL_VALUE); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_null_match(false); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_null_match(true); + EXPECT_TRUE(Metadata::match(matcher, metadata)); +} + +TEST(MetadataTest, MatchNumberValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_number_value(9); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_number_match(1); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_number_match(9); + EXPECT_TRUE(Metadata::match(matcher, metadata)); +} + +TEST(MetadataTest, MatchStringExactValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_string_value("prod"); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_exact_match("prod"); + EXPECT_TRUE(Metadata::match(matcher, metadata)); +} + +TEST(MetadataTest, MatchStringPrefixValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_string_value("prodabc"); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_prefix_match("prodx"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_prefix_match("prod"); + EXPECT_TRUE(Metadata::match(matcher, metadata)); +} + +TEST(MetadataTest, MatchStringSuffixValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_string_value("abcprod"); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_suffix_match("prodx"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_suffix_match("prod"); + EXPECT_TRUE(Metadata::match(matcher, metadata)); + ; +} + +TEST(MetadataTest, MatchBoolValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_bool_value(true); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_bool_match(false); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_bool_match(true); + EXPECT_TRUE(Metadata::match(matcher, metadata)); +} + +TEST(MetadataTest, MatchPresentValue) { + envoy::api::v2::core::Metadata metadata; + Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); + Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_number_value(1); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_present_match(false); + EXPECT_FALSE(Metadata::match(matcher, metadata)); + matcher.add_values()->set_present_match(true); + EXPECT_TRUE(Metadata::match(matcher, metadata)); + + matcher.clear_path(); + matcher.add_path("unknown"); + EXPECT_FALSE(Metadata::match(matcher, metadata)); +} + } // namespace } // namespace Config } // namespace Envoy diff --git a/test/extensions/filters/common/rbac/engine_impl_test.cc b/test/extensions/filters/common/rbac/engine_impl_test.cc index 85e8f891ea538..b4f441f44cab1 100644 --- a/test/extensions/filters/common/rbac/engine_impl_test.cc +++ b/test/extensions/filters/common/rbac/engine_impl_test.cc @@ -20,10 +20,12 @@ namespace Common { namespace RBAC { namespace { -void checkEngine(const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, - const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), - const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl()) { - EXPECT_EQ(expected, engine.allowed(connection, headers)); +void checkEngine( + const RBAC::RoleBasedAccessControlEngineImpl& engine, bool expected, + const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), + const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl(), + const envoy::api::v2::core::Metadata& metadata = envoy::api::v2::core::Metadata()) { + EXPECT_EQ(expected, engine.allowed(connection, headers, metadata)); } TEST(RoleBasedAccessControlEngineImpl, Disabled) { diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 680293acb8f0c..8e9404450b240 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -20,10 +20,12 @@ namespace Common { namespace RBAC { namespace { -void checkMatcher(const RBAC::Matcher& matcher, bool expected, - const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), - const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl()) { - EXPECT_EQ(expected, matcher.matches(connection, headers)); +void checkMatcher( + const RBAC::Matcher& matcher, bool expected, + const Envoy::Network::Connection& connection = Envoy::Network::MockConnection(), + const Envoy::Http::HeaderMap& headers = Envoy::Http::HeaderMapImpl(), + const envoy::api::v2::core::Metadata& metadata = envoy::api::v2::core::Metadata()) { + EXPECT_EQ(expected, matcher.matches(connection, headers, metadata)); } TEST(AlwaysMatcher, AlwaysMatches) { checkMatcher(RBAC::AlwaysMatcher(), true); } @@ -210,6 +212,27 @@ TEST(AuthenticatedMatcher, NoSSL) { checkMatcher(AuthenticatedMatcher({}), false, conn); } +TEST(MetadataMatcher, MetadataMatcher) { + Envoy::Network::MockConnection conn; + Envoy::Http::HeaderMapImpl header; + + auto label = MessageUtil::keyValueStruct("label", "prod"); + envoy::api::v2::core::Metadata metadata; + metadata.mutable_filter_metadata()->insert( + Protobuf::MapPair("other", label)); + metadata.mutable_filter_metadata()->insert( + Protobuf::MapPair("rbac", label)); + + envoy::api::v2::core::MetadataMatcher matcher; + matcher.set_filter("rbac"); + matcher.add_path("label"); + + matcher.add_values()->set_exact_match("test"); + checkMatcher(MetadataMatcher(matcher), false, conn, header, metadata); + matcher.add_values()->set_exact_match("prod"); + checkMatcher(MetadataMatcher(matcher), true, conn, header, metadata); +} + TEST(PolicyMatcher, PolicyMatcher) { envoy::config::rbac::v2alpha::Policy policy; policy.add_permissions()->set_destination_port(123); diff --git a/test/extensions/filters/common/rbac/mocks.h b/test/extensions/filters/common/rbac/mocks.h index e95f63f21d256..a289b6cea8c37 100644 --- a/test/extensions/filters/common/rbac/mocks.h +++ b/test/extensions/filters/common/rbac/mocks.h @@ -15,8 +15,8 @@ class MockEngine : public RoleBasedAccessControlEngineImpl { MockEngine(const envoy::config::rbac::v2alpha::RBAC& rules) : RoleBasedAccessControlEngineImpl(rules){}; - MOCK_CONST_METHOD2(allowed, - bool(const Envoy::Network::Connection&, const Envoy::Http::HeaderMap&)); + MOCK_CONST_METHOD3(allowed, bool(const Envoy::Network::Connection&, const Envoy::Http::HeaderMap&, + const envoy::api::v2::core::Metadata&)); }; } // namespace RBAC diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index 58253bd94878b..8e5cc2b6cb87a 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -102,7 +102,7 @@ TEST_F(RoleBasedAccessControlFilterTest, RouteLocalOverride) { NiceMock engine{route_config.rbac().rules()}; NiceMock per_route_config_{route_config}; - EXPECT_CALL(engine, allowed(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(engine, allowed(_, _, _)).WillRepeatedly(Return(true)); EXPECT_CALL(per_route_config_, engine()).WillRepeatedly(ReturnRef(engine)); EXPECT_CALL(callbacks_.route_->route_entry_, perFilterConfig(HttpFilterNames::get().RBAC)) From 75fcb1f91c415eddb09259f5e1283edb3573bc9b Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Mon, 18 Jun 2018 12:12:55 -0700 Subject: [PATCH 2/7] create seperate directory and namespace for matchers. Signed-off-by: Yangmin Zhu --- api/envoy/api/v2/core/base.proto | 72 -------- api/envoy/config/rbac/v2alpha/BUILD | 6 +- api/envoy/config/rbac/v2alpha/rbac.proto | 6 +- api/envoy/type/BUILD | 11 -- api/envoy/type/matchers/BUILD | 43 +++++ api/envoy/type/matchers/metadata.proto | 59 ++++++ api/envoy/type/matchers/number.proto | 23 +++ api/envoy/type/matchers/string.proto | 49 +++++ api/envoy/type/range.proto | 10 + api/envoy/type/string_match.proto | 30 --- source/common/common/BUILD | 14 ++ source/common/common/matchers.cc | 99 ++++++++++ source/common/common/matchers.h | 75 ++++++++ source/common/config/metadata.cc | 55 ------ source/common/config/metadata.h | 9 - source/extensions/filters/common/rbac/BUILD | 3 + .../extensions/filters/common/rbac/engine.h | 1 + .../filters/common/rbac/matchers.cc | 3 +- .../extensions/filters/common/rbac/matchers.h | 7 +- test/common/common/BUILD | 14 ++ test/common/common/matchers_test.cc | 171 ++++++++++++++++++ test/common/config/metadata_test.cc | 123 ------------- .../filters/common/rbac/matchers_test.cc | 6 +- 23 files changed, 575 insertions(+), 314 deletions(-) create mode 100644 api/envoy/type/matchers/BUILD create mode 100644 api/envoy/type/matchers/metadata.proto create mode 100644 api/envoy/type/matchers/number.proto create mode 100644 api/envoy/type/matchers/string.proto delete mode 100644 api/envoy/type/string_match.proto create mode 100644 source/common/common/matchers.cc create mode 100644 source/common/common/matchers.h create mode 100644 test/common/common/matchers_test.cc diff --git a/api/envoy/api/v2/core/base.proto b/api/envoy/api/v2/core/base.proto index 051655a4a00f6..ade2f025845e4 100644 --- a/api/envoy/api/v2/core/base.proto +++ b/api/envoy/api/v2/core/base.proto @@ -95,78 +95,6 @@ message Metadata { map filter_metadata = 1; } -// MetadataMatcher provides a general interface to check if a given value is matched in -// :ref:`Metadata `. It uses `filter` and `path` to retrieve the value -// from the Metadata and then check if it's matched to one of the specified values. -// -// An example use of MetadataMatcher is specifying additional metadata in envoy.filters.http.rbac to -// enforce access control based on dynamic metadata in a request. -message MetadataMatcher { - // Specifies the value to match. Only primitive value is supported. - message Value { - // Specifies how to match a value. Only have effect on primitive value. - // For non-primitive value, it's always not matched. - oneof match_specifier { - option (validate.required) = true; - - // If specified, value match will be performed based on null value. It's matched if and only - // if the target value is a NullValue and this field is set to true. - bool null_match = 1; - - // If specified, value match will be performed based on double value. It's matched if and only - // if the target value is a double value and is equal to this field. - double number_match = 2; - - // If specified, value match will be performed based on string value. It's matched if and only - // if the target value is a string value and is equal to this field. - // - // Examples: - // - // * *abc* will only match to string value *abc*. - string exact_match = 3; - - // If specified, value match will be performed based on the prefix of the string value. - // It's matched if and only if the target value is a string value and has the prefix specified - // in this field. - // Note: empty prefix is not allowed, please use present_match instead. - // - // Examples: - // - // * *abc* will match to string value *abc.xyz* - string prefix_match = 4 [(validate.rules).string.min_bytes = 1]; - - // If specified, value match will be performed based on the suffix of the string value. - // It's matched if and only if the target value is a string value and has the suffix specified - // in this field. - // Note: empty suffix is not allowed, please use present_match instead. - // - // Examples: - // - // * *abc* will match to string value *xyz.abc* - string suffix_match = 5 [(validate.rules).string.min_bytes = 1]; - - // If specified, value match will be performed based on bool value. It's matched if and only - // if the target value is a bool value and is equal to this field. - bool bool_match = 6; - - // If specified, value match will be performed based on whether the path is referring to a - // valid primitive value in the metadata. - // Note: If the path is referring to a non-primitive value, it's always not matched. - bool present_match = 7; - } - } - - // Required. The filter name to retrieve the Struct from the Metadata. - string filter = 1 [(validate.rules).string.min_bytes = 1]; - - // Required. The multi-key path to retrieve the Value from the Struct. - repeated string path = 2 [(validate.rules).repeated .min_items = 1]; - - // Required. A set of values to match. The MetadataMatcher is matched if at least one value is - // matched, in other words, it's matched with OR semantics. - repeated Value values = 3 [(validate.rules).repeated .min_items = 1]; -} - // Runtime derived uint32 with a default when not specified. message RuntimeUInt32 { // Default value if runtime value is not available. diff --git a/api/envoy/config/rbac/v2alpha/BUILD b/api/envoy/config/rbac/v2alpha/BUILD index 0ff71cbd9c5e3..8de09c3e44fbc 100644 --- a/api/envoy/config/rbac/v2alpha/BUILD +++ b/api/envoy/config/rbac/v2alpha/BUILD @@ -8,9 +8,8 @@ api_proto_library( visibility = ["//visibility:public"], deps = [ "//envoy/api/v2/core:address", - "//envoy/api/v2/core:base", "//envoy/api/v2/route", - "//envoy/type:string_match", + "//envoy/type/matchers:metadata", ], ) @@ -19,8 +18,7 @@ api_go_proto_library( proto = ":rbac", deps = [ "//envoy/api/v2/core:address_go_proto", - "//envoy/api/v2/core:base_go_proto", "//envoy/api/v2/route:route_go_proto", - "//envoy/type:string_match_go_proto", + "//envoy/type/matchers:metadata_go_proto", ], ) diff --git a/api/envoy/config/rbac/v2alpha/rbac.proto b/api/envoy/config/rbac/v2alpha/rbac.proto index 772e7375f7d33..a72b3dba7ab60 100644 --- a/api/envoy/config/rbac/v2alpha/rbac.proto +++ b/api/envoy/config/rbac/v2alpha/rbac.proto @@ -2,8 +2,8 @@ syntax = "proto3"; import "validate/validate.proto"; import "envoy/api/v2/core/address.proto"; -import "envoy/api/v2/core/base.proto"; import "envoy/api/v2/route/route.proto"; +import "envoy/type/matchers/metadata.proto"; package envoy.config.rbac.v2alpha; option go_package = "v2alpha"; @@ -114,7 +114,7 @@ message Permission { uint32 destination_port = 6 [(validate.rules).uint32.lte = 65535]; // A metadata that describes additional information about the action. - envoy.api.v2.core.MetadataMatcher metadata = 7; + envoy.type.matchers.MetadataMatcher metadata = 7; } } @@ -156,6 +156,6 @@ message Principal { envoy.api.v2.route.HeaderMatcher header = 6; // A metadata that describes additional information about the principal. - envoy.api.v2.core.MetadataMatcher metadata = 7; + envoy.type.matchers.MetadataMatcher metadata = 7; } } diff --git a/api/envoy/type/BUILD b/api/envoy/type/BUILD index 4859476efbd9d..557bbecc6c3b4 100644 --- a/api/envoy/type/BUILD +++ b/api/envoy/type/BUILD @@ -23,14 +23,3 @@ api_go_proto_library( name = "range", proto = ":range", ) - -api_proto_library( - name = "string_match", - srcs = ["string_match.proto"], - visibility = ["//visibility:public"], -) - -api_go_proto_library( - name = "string_match", - proto = ":string_match", -) diff --git a/api/envoy/type/matchers/BUILD b/api/envoy/type/matchers/BUILD new file mode 100644 index 0000000000000..73766b4c616bc --- /dev/null +++ b/api/envoy/type/matchers/BUILD @@ -0,0 +1,43 @@ +load("//bazel:api_build_system.bzl", "api_go_proto_library", "api_proto_library") + +licenses(["notice"]) # Apache 2 + +api_proto_library( + name = "metadata", + srcs = ["metadata.proto"], + visibility = ["//visibility:public"], + deps = [ + ":number", + ":string", + ], +) + +api_go_proto_library( + name = "metadata", + proto = ":metadata", +) + +api_proto_library( + name = "number", + srcs = ["number.proto"], + visibility = ["//visibility:public"], + deps = [ + "//envoy/type:range", + ], +) + +api_go_proto_library( + name = "number", + proto = ":number", +) + +api_proto_library( + name = "string", + srcs = ["string.proto"], + visibility = ["//visibility:public"], +) + +api_go_proto_library( + name = "string", + proto = ":string", +) diff --git a/api/envoy/type/matchers/metadata.proto b/api/envoy/type/matchers/metadata.proto new file mode 100644 index 0000000000000..ade8a84635047 --- /dev/null +++ b/api/envoy/type/matchers/metadata.proto @@ -0,0 +1,59 @@ +syntax = "proto3"; + +package envoy.type.matchers; +option go_package = "matchers"; + +import "envoy/type/matchers/string.proto"; +import "envoy/type/matchers/number.proto"; + +import "validate/validate.proto"; + +// [#protodoc-title: MetadataMatcher] + +// MetadataMatcher provides a general interface to check if a given value is matched in +// :ref:`Metadata `. It uses `filter` and `path` to retrieve the value +// from the Metadata and then check if it's matched to one of the specified values. +// +// An example use of MetadataMatcher is specifying additional metadata in envoy.filters.http.rbac to +// enforce access control based on dynamic metadata in a request. +message MetadataMatcher { + // Specifies the value to match. Only primitive value is supported. For non-primitive value, the + // result is always not matched. + message Value { + // Specifies how to match a value. Only have effect on primitive value. + oneof match_pattern { + option (validate.required) = true; + + // If specified, it's matched if and only if the target value is a NullValue and this field is + // also set to true. + bool null_match = 1; + + // If specified, it's matched if and only if the target value is a double value and is matched + // to this field. + DoubleMatcher double_match = 2; + + // If specified, it's matched if and only if the target value is a string value and is matched + // to this field. + StringMatcher string_match = 3; + + // If specified, it's matched if and only if the target value is a bool value and is equal to + // this field. + bool bool_match = 4; + + // If specified, value match will be performed based on whether the path is referring to a + // valid primitive value in the metadata. If the path is referring to a non-primitive value, + // the result is always not matched. + bool present_match = 7; + } + } + + // Required. The filter name to retrieve the Struct from the Metadata. + string filter = 1 [(validate.rules).string.min_bytes = 1]; + + // Required. The multi-key path to retrieve the Value from the Struct. + repeated string path = 2 [(validate.rules).repeated .min_items = 1]; + + // Required. A set of values to match. The MetadataMatcher is matched if at least one value is + // matched, in other words, it's matched with OR semantics. + repeated Value values = 3 [(validate.rules).repeated .min_items = 1]; +} diff --git a/api/envoy/type/matchers/number.proto b/api/envoy/type/matchers/number.proto new file mode 100644 index 0000000000000..8c55e174803ef --- /dev/null +++ b/api/envoy/type/matchers/number.proto @@ -0,0 +1,23 @@ +syntax = "proto3"; + +package envoy.type.matchers; +option go_package = "matchers"; + +import "envoy/type/range.proto"; + +import "validate/validate.proto"; + +// [#protodoc-title: NumberMatcher] + +// Specifies the way to match a double value. +message DoubleMatcher { + oneof match_pattern { + option (validate.required) = true; + + // If specified, the input double value must be in the range specified here. + envoy.type.DoubleRange range = 1; + + // If specified, the input double value must be equal to the value specified here. + double exact = 2; + } +} diff --git a/api/envoy/type/matchers/string.proto b/api/envoy/type/matchers/string.proto new file mode 100644 index 0000000000000..42dd0bde9d456 --- /dev/null +++ b/api/envoy/type/matchers/string.proto @@ -0,0 +1,49 @@ +syntax = "proto3"; + +package envoy.type.matchers; +option go_package = "matchers"; + +import "validate/validate.proto"; + +// [#protodoc-title: StringMatcher] + +// Specifies the way to match a string. +message StringMatcher { + oneof match_pattern { + option (validate.required) = true; + + // The input string must match exactly the string specified here. + // + // Examples: + // + // * *abc* only matches the value *abc*. + string exact = 1; + + // The input string must have the prefix specified here. + // Note: empty prefix is not allowed, please use regex instead. + // + // Examples: + // + // * *abc* matches the value *abc.xyz* + string prefix = 2 [(validate.rules).string.min_bytes = 1]; + + // The input string must have the suffix specified here. + // Note: empty prefix is not allowed, please use regex instead. + // + // Examples: + // + // * *abc* matches the value *xyz.abc* + string suffix = 3 [(validate.rules).string.min_bytes = 1]; + + // The input string must match the regular expression specified here. + // The regex grammar is defined `here + // `_. + // + // Examples: + // + // * The regex *\d{3}* matches the value *123* + // * The regex *\d{3}* does not match the value *1234* + // * The regex *\d{3}* does not match the value *123.456* + string regex = 4; + } +} diff --git a/api/envoy/type/range.proto b/api/envoy/type/range.proto index fd6045e7fd289..115091ddf9f69 100644 --- a/api/envoy/type/range.proto +++ b/api/envoy/type/range.proto @@ -18,3 +18,13 @@ message Int64Range { // end of the range (exclusive) int64 end = 2; } + +// Specifies the double start and end of the range using half-open interval semantics [start, +// end). +message DoubleRange { + // start of the range (inclusive) + double start = 1; + + // end of the range (exclusive) + double end = 2; +} diff --git a/api/envoy/type/string_match.proto b/api/envoy/type/string_match.proto deleted file mode 100644 index c1e2468ad5899..0000000000000 --- a/api/envoy/type/string_match.proto +++ /dev/null @@ -1,30 +0,0 @@ -syntax = "proto3"; - -package envoy.type; -option go_package = "envoy_type"; - -import "gogoproto/gogo.proto"; - -option (gogoproto.equal_all) = true; - -// [#protodoc-title: StringMatch] - -// Specifies the way to match a string. -message StringMatch { - oneof match_pattern { - // The input string must match exactly the string specified here. - // Or it is a "*", which means that it matches any string. - string simple = 1; - - // The input string must have the prefix specified here. - string prefix = 2; - - // The input string must have the suffix specified here. - string suffix = 3; - - // The input string must match the regular expression specified here. - // The regex grammar is defined `here - // `_. - string regex = 4; - } -} diff --git a/source/common/common/BUILD b/source/common/common/BUILD index fc0e43d110f1b..7105a23847040 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -113,6 +113,20 @@ envoy_cc_library( hdrs = ["macros.h"], ) +envoy_cc_library( + name = "matchers_lib", + srcs = ["matchers.cc"], + hdrs = ["matchers.h"], + deps = [ + ":utility_lib", + "//source/common/config:metadata_lib", + "//source/common/protobuf", + "@envoy_api//envoy/type/matchers:metadata_cc", + "@envoy_api//envoy/type/matchers:number_cc", + "@envoy_api//envoy/type/matchers:string_cc", + ], +) + envoy_cc_library( name = "non_copyable", hdrs = ["non_copyable.h"], diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc new file mode 100644 index 0000000000000..f0022bd12f61b --- /dev/null +++ b/source/common/common/matchers.cc @@ -0,0 +1,99 @@ +#include "common/common/matchers.h" + +#include "envoy/api/v2/core/base.pb.h" + +#include "common/config/metadata.h" +#include "common/protobuf/protobuf.h" + +#include "absl/strings/match.h" + +namespace Envoy { +namespace Matchers { + +bool DoubleMatcher::match(double value) const { + switch (matcher_.match_pattern_case()) { + case envoy::type::matchers::DoubleMatcher::kRange: + return matcher_.range().start() <= value && value < matcher_.range().end(); + case envoy::type::matchers::DoubleMatcher::kExact: + return matcher_.exact() == value; + default: + return false; + }; +} + +bool StringMatcher::match(const std::string& value) const { + switch (matcher_.match_pattern_case()) { + case envoy::type::matchers::StringMatcher::kExact: + return matcher_.exact() == value; + case envoy::type::matchers::StringMatcher::kPrefix: + return absl::StartsWith(value, matcher_.prefix()); + case envoy::type::matchers::StringMatcher::kSuffix: + return absl::EndsWith(value, matcher_.suffix()); + case envoy::type::matchers::StringMatcher::kRegex: + return std::regex_match(value, regex_); + default: + return false; + } +} + +MetadataMatcher::MetadataMatcher(const envoy::type::matchers::MetadataMatcher& matcher) + : matcher_(matcher), path_(matcher.path().begin(), matcher.path().end()) { + for (const envoy::type::matchers::MetadataMatcher::Value& m : matcher_.values()) { + switch (m.match_pattern_case()) { + case envoy::type::matchers::MetadataMatcher_Value::kNullMatch: + null_matcher_ |= m.null_match(); + break; + case envoy::type::matchers::MetadataMatcher_Value::kDoubleMatch: + double_matcher_.push_back(DoubleMatcher(m.double_match())); + break; + case envoy::type::matchers::MetadataMatcher_Value::kStringMatch: + string_matcher_.push_back(StringMatcher(m.string_match())); + break; + case envoy::type::matchers::MetadataMatcher_Value::kBoolMatch: + if (m.bool_match()) { + bool_matcher_allow_true_ = true; + } else { + bool_matcher_allow_false_ = true; + } + break; + case envoy::type::matchers::MetadataMatcher_Value::kPresentMatch: + present_matcher_ |= m.present_match(); + break; + default: + break; + } + } +} + +bool MetadataMatcher::match(const envoy::api::v2::core::Metadata& metadata) const { + const auto& value = Envoy::Config::Metadata::metadataValue(metadata, matcher_.filter(), path_); + if (present_matcher_ && value.kind_case() != ProtobufWkt::Value::KIND_NOT_SET) { + return true; + } + switch (value.kind_case()) { + case ProtobufWkt::Value::kNullValue: + return null_matcher_; + case ProtobufWkt::Value::kNumberValue: + for (const auto& m : double_matcher_) { + if (m.match(value.number_value())) { + return true; + } + } + return false; + case ProtobufWkt::Value::kStringValue: + for (const auto& m : string_matcher_) { + if (m.match(value.string_value())) { + return true; + } + } + return false; + case ProtobufWkt::Value::kBoolValue: + return (bool_matcher_allow_true_ && value.bool_value()) || + (bool_matcher_allow_false_ && !value.bool_value()); + default: + return false; + } +} + +} // namespace Matchers +} // namespace Envoy diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h new file mode 100644 index 0000000000000..4b83105054b13 --- /dev/null +++ b/source/common/common/matchers.h @@ -0,0 +1,75 @@ +#pragma once + +#include + +#include "envoy/api/v2/core/base.pb.h" +#include "envoy/type/matchers/metadata.pb.h" +#include "envoy/type/matchers/number.pb.h" +#include "envoy/type/matchers/string.pb.h" + +#include "common/common/utility.h" + +namespace Envoy { +namespace Matchers { + +class DoubleMatcher { +public: + DoubleMatcher(const envoy::type::matchers::DoubleMatcher& matcher) : matcher_(matcher) {} + + /** + * Check whether the value is matched to the matcher. + * @param value the double value to check. + * @return true if it's matched otherwise false. + */ + bool match(double value) const; + +private: + const envoy::type::matchers::DoubleMatcher matcher_; +}; + +class StringMatcher { +public: + StringMatcher(const envoy::type::matchers::StringMatcher& matcher) : matcher_(matcher) { + if (matcher.match_pattern_case() == envoy::type::matchers::StringMatcher::kRegex) { + regex_ = RegexUtil::parseRegex(matcher_.regex()); + } + } + + /** + * Check whether the value is matched to the matcher. + * @param value the string to check. + * @return true if it's matched otherwise false. + */ + bool match(const std::string& value) const; + +private: + const envoy::type::matchers::StringMatcher matcher_; + std::regex regex_; +}; + +class MetadataMatcher { +public: + MetadataMatcher(const envoy::type::matchers::MetadataMatcher& matcher); + + /** + * Check whether the metadata is matched to the matcher. + * @param metadata the metadata to check. + * @return true if it's matched otherwise false. + */ + bool match(const envoy::api::v2::core::Metadata& metadata) const; + +private: + const envoy::type::matchers::MetadataMatcher matcher_; + const std::vector path_; + + bool null_matcher_{false}; + bool bool_matcher_allow_true_{false}; + bool bool_matcher_allow_false_{false}; + bool present_matcher_{false}; + + std::vector double_matcher_; + std::vector string_matcher_; +}; + +} // namespace Matchers +} // namespace Envoy diff --git a/source/common/config/metadata.cc b/source/common/config/metadata.cc index 6e18a06b246b0..22a2d9f05be7a 100644 --- a/source/common/config/metadata.cc +++ b/source/common/config/metadata.cc @@ -1,7 +1,5 @@ #include "common/config/metadata.h" -#include "absl/strings/match.h" - namespace Envoy { namespace Config { @@ -49,58 +47,5 @@ ProtobufWkt::Value& Metadata::mutableMetadataValue(envoy::api::v2::core::Metadat return (*(*metadata.mutable_filter_metadata())[filter].mutable_fields())[key]; } -bool Metadata::match(const envoy::api::v2::core::MetadataMatcher& matcher, - const envoy::api::v2::core::Metadata& metadata) { - const std::vector path(matcher.path().begin(), matcher.path().end()); - const auto& value = metadataValue(metadata, matcher.filter(), path); - for (const envoy::api::v2::core::MetadataMatcher::Value& m : matcher.values()) { - switch (m.match_specifier_case()) { - case envoy::api::v2::core::MetadataMatcher_Value::kNullMatch: - if (value.kind_case() == ProtobufWkt::Value::kNullValue && m.null_match()) { - return true; - } - break; - case envoy::api::v2::core::MetadataMatcher_Value::kNumberMatch: - if (value.kind_case() == ProtobufWkt::Value::kNumberValue && - m.number_match() == value.number_value()) { - return true; - } - break; - case envoy::api::v2::core::MetadataMatcher_Value::kExactMatch: - if (value.kind_case() == ProtobufWkt::Value::kStringValue && - m.exact_match() == value.string_value()) { - return true; - } - break; - case envoy::api::v2::core::MetadataMatcher_Value::kPrefixMatch: - if (value.kind_case() == ProtobufWkt::Value::kStringValue && - absl::StartsWith(value.string_value(), m.prefix_match())) { - return true; - } - break; - case envoy::api::v2::core::MetadataMatcher_Value::kSuffixMatch: - if (value.kind_case() == ProtobufWkt::Value::kStringValue && - absl::EndsWith(value.string_value(), m.suffix_match())) { - return true; - } - break; - case envoy::api::v2::core::MetadataMatcher_Value::kBoolMatch: - if (value.kind_case() == ProtobufWkt::Value::kBoolValue && - m.bool_match() == value.bool_value()) { - return true; - } - break; - case envoy::api::v2::core::MetadataMatcher_Value::kPresentMatch: - if (value.kind_case() != ProtobufWkt::Value::KIND_NOT_SET && m.present_match()) { - return true; - } - break; - default: - break; - } - } - return false; -} - } // namespace Config } // namespace Envoy diff --git a/source/common/config/metadata.h b/source/common/config/metadata.h index 07c9eb0579dec..21432c9a7d739 100644 --- a/source/common/config/metadata.h +++ b/source/common/config/metadata.h @@ -44,15 +44,6 @@ class Metadata { static ProtobufWkt::Value& mutableMetadataValue(envoy::api::v2::core::Metadata& metadata, const std::string& filter, const std::string& key); - - /** - * Check whether the metadata is matched to the matcher. - * @param matcher the matcher with the match condition. - * @param metadata the metadata to check. - * @return true if it's matched otherwise false. - */ - static bool match(const envoy::api::v2::core::MetadataMatcher& matcher, - const envoy::api::v2::core::Metadata& metadata); }; } // namespace Config diff --git a/source/extensions/filters/common/rbac/BUILD b/source/extensions/filters/common/rbac/BUILD index 59c0cf7beb7dd..0170fc2d7b8f1 100644 --- a/source/extensions/filters/common/rbac/BUILD +++ b/source/extensions/filters/common/rbac/BUILD @@ -16,8 +16,10 @@ envoy_cc_library( "//include/envoy/http:header_map_interface", "//include/envoy/network:connection_interface", "//source/common/common:assert_lib", + "//source/common/common:matchers_lib", "//source/common/http:header_utility_lib", "//source/common/network:cidr_range_lib", + "@envoy_api//envoy/api/v2/core:base_cc", "@envoy_api//envoy/config/rbac/v2alpha:rbac_cc", ], ) @@ -39,6 +41,7 @@ envoy_cc_library( deps = [ "//source/extensions/filters/common/rbac:engine_interface", "//source/extensions/filters/common/rbac:matchers_lib", + "@envoy_api//envoy/api/v2/core:base_cc", "@envoy_api//envoy/config/filter/http/rbac/v2:rbac_cc", ], ) diff --git a/source/extensions/filters/common/rbac/engine.h b/source/extensions/filters/common/rbac/engine.h index 75860a9573a24..1281f6319bcf2 100644 --- a/source/extensions/filters/common/rbac/engine.h +++ b/source/extensions/filters/common/rbac/engine.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/api/v2/core/base.pb.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/network/connection.h" diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index be4dcade29f3e..bb223c42afcab 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -1,7 +1,6 @@ #include "extensions/filters/common/rbac/matchers.h" #include "common/common/assert.h" -#include "common/config/metadata.h" namespace Envoy { namespace Extensions { @@ -138,7 +137,7 @@ bool AuthenticatedMatcher::matches(const Network::Connection& connection, bool MetadataMatcher::matches(const Network::Connection&, const Envoy::Http::HeaderMap&, const envoy::api::v2::core::Metadata& metadata) const { - return Envoy::Config::Metadata::match(matcher_, metadata); + return matcher_.match(metadata); } bool PolicyMatcher::matches(const Network::Connection& connection, diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index cc96e6eb89c77..e8a4861331595 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -2,10 +2,12 @@ #include +#include "envoy/api/v2/core/base.pb.h" #include "envoy/config/rbac/v2alpha/rbac.pb.h" #include "envoy/http/header_map.h" #include "envoy/network/connection.h" +#include "common/common/matchers.h" #include "common/http/header_utility.h" #include "common/network/cidr_range.h" @@ -31,6 +33,7 @@ class Matcher { * @param connection the downstream connection used to match against. * @param headers the request headers used to match against. An empty map should be used if * there are none headers available. + * @param metadata the additional information about the action/principal. */ virtual bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, const envoy::api::v2::core::Metadata& metadata) const PURE; @@ -174,13 +177,13 @@ class PolicyMatcher : public Matcher { class MetadataMatcher : public Matcher { public: - MetadataMatcher(const envoy::api::v2::core::MetadataMatcher& matcher) : matcher_(matcher) {} + MetadataMatcher(const Envoy::Matchers::MetadataMatcher& matcher) : matcher_(matcher) {} bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, const envoy::api::v2::core::Metadata& metadata) const override; private: - const envoy::api::v2::core::MetadataMatcher matcher_; + const Envoy::Matchers::MetadataMatcher matcher_; }; } // namespace RBAC diff --git a/test/common/common/BUILD b/test/common/common/BUILD index aa7f952e6fcd5..75c8eb9adaeab 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -71,6 +71,20 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "matchers_test", + srcs = ["matchers_test.cc"], + deps = [ + "//source/common/common:matchers_lib", + "//source/common/config:metadata_lib", + "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/api/v2/core:base_cc", + "@envoy_api//envoy/type/matchers:metadata_cc", + "@envoy_api//envoy/type/matchers:number_cc", + "@envoy_api//envoy/type/matchers:string_cc", + ], +) + envoy_cc_test( name = "utility_test", srcs = ["utility_test.cc"], diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc new file mode 100644 index 0000000000000..fe6f3e1512bf4 --- /dev/null +++ b/test/common/common/matchers_test.cc @@ -0,0 +1,171 @@ +#include "envoy/api/v2/core/base.pb.h" +#include "envoy/type/matchers/metadata.pb.h" +#include "envoy/type/matchers/number.pb.h" +#include "envoy/type/matchers/string.pb.h" + +#include "common/common/matchers.h" +#include "common/config/metadata.h" +#include "common/protobuf/protobuf.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Matcher { +namespace { + +TEST(MetadataTest, MatchNullValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_null_value(ProtobufWkt::NullValue::NULL_VALUE); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->set_null_match(false); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->set_null_match(true); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); +} + +TEST(MetadataTest, MatchDoubleValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_number_value(9); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_double_match()->set_exact(1); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_double_match()->set_exact(9); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + + matcher.clear_values(); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + auto r = matcher.add_values()->mutable_double_match()->mutable_range(); + r->set_start(9.1); + r->set_end(10); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + + r = matcher.add_values()->mutable_double_match()->mutable_range(); + r->set_start(8.9); + r->set_end(9); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + + r = matcher.add_values()->mutable_double_match()->mutable_range(); + r->set_start(9); + r->set_end(9.1); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); +} + +TEST(MetadataTest, MatchStringExactValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_string_value("prod"); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_string_match()->set_exact("prod"); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); +} + +TEST(MetadataTest, MatchStringPrefixValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_string_value("prodabc"); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_string_match()->set_prefix("prodx"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_string_match()->set_prefix("prod"); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); +} + +TEST(MetadataTest, MatchStringSuffixValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_string_value("abcprod"); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_string_match()->set_suffix("prodx"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->mutable_string_match()->set_suffix("prod"); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + ; +} + +TEST(MetadataTest, MatchBoolValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_bool_value(true); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->set_bool_match(false); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->set_bool_match(true); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); +} + +TEST(MetadataTest, MatchPresentValue) { + envoy::api::v2::core::Metadata metadata; + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label") + .set_string_value("test"); + Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") + .set_number_value(1); + + envoy::type::matchers::MetadataMatcher matcher; + matcher.set_filter("envoy.filter.b"); + matcher.add_path("label"); + + matcher.add_values()->mutable_string_match()->set_exact("test"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->set_present_match(false); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + matcher.add_values()->set_present_match(true); + EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); + + matcher.clear_path(); + matcher.add_path("unknown"); + EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); +} + +} // namespace +} // namespace Matcher +} // namespace Envoy diff --git a/test/common/config/metadata_test.cc b/test/common/config/metadata_test.cc index 7e006643009d5..072624a3f2f0c 100644 --- a/test/common/config/metadata_test.cc +++ b/test/common/config/metadata_test.cc @@ -43,129 +43,6 @@ TEST(MetadataTest, MetadataValuePath) { ProtobufWkt::Value::KindCase::KIND_NOT_SET); } -TEST(MetadataTest, MatchNullValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") - .set_null_value(ProtobufWkt::NullValue::NULL_VALUE); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_null_match(false); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_null_match(true); - EXPECT_TRUE(Metadata::match(matcher, metadata)); -} - -TEST(MetadataTest, MatchNumberValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_number_value(9); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_number_match(1); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_number_match(9); - EXPECT_TRUE(Metadata::match(matcher, metadata)); -} - -TEST(MetadataTest, MatchStringExactValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_string_value("prod"); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_exact_match("prod"); - EXPECT_TRUE(Metadata::match(matcher, metadata)); -} - -TEST(MetadataTest, MatchStringPrefixValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_string_value("prodabc"); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_prefix_match("prodx"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_prefix_match("prod"); - EXPECT_TRUE(Metadata::match(matcher, metadata)); -} - -TEST(MetadataTest, MatchStringSuffixValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_string_value("abcprod"); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_suffix_match("prodx"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_suffix_match("prod"); - EXPECT_TRUE(Metadata::match(matcher, metadata)); - ; -} - -TEST(MetadataTest, MatchBoolValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_bool_value(true); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_bool_match(false); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_bool_match(true); - EXPECT_TRUE(Metadata::match(matcher, metadata)); -} - -TEST(MetadataTest, MatchPresentValue) { - envoy::api::v2::core::Metadata metadata; - Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label").set_string_value("test"); - Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label").set_number_value(1); - - envoy::api::v2::core::MetadataMatcher matcher; - matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); - - matcher.add_values()->set_exact_match("test"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_present_match(false); - EXPECT_FALSE(Metadata::match(matcher, metadata)); - matcher.add_values()->set_present_match(true); - EXPECT_TRUE(Metadata::match(matcher, metadata)); - - matcher.clear_path(); - matcher.add_path("unknown"); - EXPECT_FALSE(Metadata::match(matcher, metadata)); -} - } // namespace } // namespace Config } // namespace Envoy diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 8e9404450b240..08e4dff6e20d0 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -223,13 +223,13 @@ TEST(MetadataMatcher, MetadataMatcher) { metadata.mutable_filter_metadata()->insert( Protobuf::MapPair("rbac", label)); - envoy::api::v2::core::MetadataMatcher matcher; + envoy::type::matchers::MetadataMatcher matcher; matcher.set_filter("rbac"); matcher.add_path("label"); - matcher.add_values()->set_exact_match("test"); + matcher.add_values()->mutable_string_match()->set_exact("test"); checkMatcher(MetadataMatcher(matcher), false, conn, header, metadata); - matcher.add_values()->set_exact_match("prod"); + matcher.add_values()->mutable_string_match()->set_exact("prod"); checkMatcher(MetadataMatcher(matcher), true, conn, header, metadata); } From 5d009dc83c454fcde8e1b908980ebdadf540d3ab Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Mon, 18 Jun 2018 15:28:23 -0700 Subject: [PATCH 3/7] fix api and docs. Signed-off-by: Yangmin Zhu --- api/docs/BUILD | 3 +++ api/envoy/type/matchers/BUILD | 7 +++++++ docs/build.sh | 3 +++ docs/root/api-v2/types/types.rst | 3 +++ 4 files changed, 16 insertions(+) diff --git a/api/docs/BUILD b/api/docs/BUILD index 39359024f21c3..823cd32f2a478 100644 --- a/api/docs/BUILD +++ b/api/docs/BUILD @@ -65,5 +65,8 @@ proto_library( "//envoy/service/metrics/v2:metrics_service", "//envoy/type:percent", "//envoy/type:range", + "//envoy/type/matchers:metadata", + "//envoy/type/matchers:number", + "//envoy/type/matchers:string", ], ) diff --git a/api/envoy/type/matchers/BUILD b/api/envoy/type/matchers/BUILD index 73766b4c616bc..235626653e8f1 100644 --- a/api/envoy/type/matchers/BUILD +++ b/api/envoy/type/matchers/BUILD @@ -15,6 +15,10 @@ api_proto_library( api_go_proto_library( name = "metadata", proto = ":metadata", + deps = [ + ":number_go_proto", + ":string_go_proto", + ], ) api_proto_library( @@ -29,6 +33,9 @@ api_proto_library( api_go_proto_library( name = "number", proto = ":number", + deps = [ + "//envoy/type:range_go_proto", + ], ) api_proto_library( diff --git a/docs/build.sh b/docs/build.sh index 4920bd5b969d7..be20527f0d01b 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -107,6 +107,9 @@ PROTO_RST=" /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/external_auth.proto.rst /envoy/type/percent/envoy/type/percent.proto.rst /envoy/type/range/envoy/type/range.proto.rst + /envoy/type/matchers/metadata/envoy/type/matchers/metadata.proto.rst + /envoy/type/matchers/number/envoy/type/matchers/number.proto.rst + /envoy/type/matchers/string/envoy/type/matchers/string.proto.rst " # Dump all the generated RST so they can be added to PROTO_RST easily. diff --git a/docs/root/api-v2/types/types.rst b/docs/root/api-v2/types/types.rst index 116d6c3cb519c..2531a6e34e75d 100644 --- a/docs/root/api-v2/types/types.rst +++ b/docs/root/api-v2/types/types.rst @@ -7,3 +7,6 @@ Types ../type/percent.proto ../type/range.proto + ../type/matchers/metadata.proto + ../type/matchers/number.proto + ../type/matchers/string.proto From 4301c1df145b8cf6819ca3887999964f68000ac7 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Tue, 19 Jun 2018 13:06:52 -0700 Subject: [PATCH 4/7] address comments. Signed-off-by: Yangmin Zhu --- api/envoy/config/rbac/v2alpha/rbac.proto | 4 +-- api/envoy/type/matchers/metadata.proto | 7 ++--- api/envoy/type/matchers/number.proto | 4 +-- api/envoy/type/matchers/string.proto | 4 +-- source/common/common/matchers.cc | 26 +++++++++---------- source/common/common/matchers.h | 14 +++++----- test/common/common/matchers_test.cc | 14 +++++----- .../filters/common/rbac/matchers_test.cc | 2 +- 8 files changed, 38 insertions(+), 37 deletions(-) diff --git a/api/envoy/config/rbac/v2alpha/rbac.proto b/api/envoy/config/rbac/v2alpha/rbac.proto index a72b3dba7ab60..e57772b87043b 100644 --- a/api/envoy/config/rbac/v2alpha/rbac.proto +++ b/api/envoy/config/rbac/v2alpha/rbac.proto @@ -114,7 +114,7 @@ message Permission { uint32 destination_port = 6 [(validate.rules).uint32.lte = 65535]; // A metadata that describes additional information about the action. - envoy.type.matchers.MetadataMatcher metadata = 7; + envoy.type.matcher.MetadataMatcher metadata = 7; } } @@ -156,6 +156,6 @@ message Principal { envoy.api.v2.route.HeaderMatcher header = 6; // A metadata that describes additional information about the principal. - envoy.type.matchers.MetadataMatcher metadata = 7; + envoy.type.matcher.MetadataMatcher metadata = 7; } } diff --git a/api/envoy/type/matchers/metadata.proto b/api/envoy/type/matchers/metadata.proto index ade8a84635047..341e889202b06 100644 --- a/api/envoy/type/matchers/metadata.proto +++ b/api/envoy/type/matchers/metadata.proto @@ -1,7 +1,7 @@ syntax = "proto3"; -package envoy.type.matchers; -option go_package = "matchers"; +package envoy.type.matcher; +option go_package = "matcher"; import "envoy/type/matchers/string.proto"; import "envoy/type/matchers/number.proto"; @@ -43,7 +43,7 @@ message MetadataMatcher { // If specified, value match will be performed based on whether the path is referring to a // valid primitive value in the metadata. If the path is referring to a non-primitive value, // the result is always not matched. - bool present_match = 7; + bool present_match = 5; } } @@ -51,6 +51,7 @@ message MetadataMatcher { string filter = 1 [(validate.rules).string.min_bytes = 1]; // Required. The multi-key path to retrieve the Value from the Struct. + // Note: Array is not supported and will result a not match discarding any specified values. repeated string path = 2 [(validate.rules).repeated .min_items = 1]; // Required. A set of values to match. The MetadataMatcher is matched if at least one value is diff --git a/api/envoy/type/matchers/number.proto b/api/envoy/type/matchers/number.proto index 8c55e174803ef..92fc6a78fb755 100644 --- a/api/envoy/type/matchers/number.proto +++ b/api/envoy/type/matchers/number.proto @@ -1,7 +1,7 @@ syntax = "proto3"; -package envoy.type.matchers; -option go_package = "matchers"; +package envoy.type.matcher; +option go_package = "matcher"; import "envoy/type/range.proto"; diff --git a/api/envoy/type/matchers/string.proto b/api/envoy/type/matchers/string.proto index 42dd0bde9d456..afb419a613b39 100644 --- a/api/envoy/type/matchers/string.proto +++ b/api/envoy/type/matchers/string.proto @@ -1,7 +1,7 @@ syntax = "proto3"; -package envoy.type.matchers; -option go_package = "matchers"; +package envoy.type.matcher; +option go_package = "matcher"; import "validate/validate.proto"; diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index f0022bd12f61b..47687a6b22641 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -12,9 +12,9 @@ namespace Matchers { bool DoubleMatcher::match(double value) const { switch (matcher_.match_pattern_case()) { - case envoy::type::matchers::DoubleMatcher::kRange: + case envoy::type::matcher::DoubleMatcher::kRange: return matcher_.range().start() <= value && value < matcher_.range().end(); - case envoy::type::matchers::DoubleMatcher::kExact: + case envoy::type::matcher::DoubleMatcher::kExact: return matcher_.exact() == value; default: return false; @@ -23,40 +23,40 @@ bool DoubleMatcher::match(double value) const { bool StringMatcher::match(const std::string& value) const { switch (matcher_.match_pattern_case()) { - case envoy::type::matchers::StringMatcher::kExact: + case envoy::type::matcher::StringMatcher::kExact: return matcher_.exact() == value; - case envoy::type::matchers::StringMatcher::kPrefix: + case envoy::type::matcher::StringMatcher::kPrefix: return absl::StartsWith(value, matcher_.prefix()); - case envoy::type::matchers::StringMatcher::kSuffix: + case envoy::type::matcher::StringMatcher::kSuffix: return absl::EndsWith(value, matcher_.suffix()); - case envoy::type::matchers::StringMatcher::kRegex: + case envoy::type::matcher::StringMatcher::kRegex: return std::regex_match(value, regex_); default: return false; } } -MetadataMatcher::MetadataMatcher(const envoy::type::matchers::MetadataMatcher& matcher) +MetadataMatcher::MetadataMatcher(const envoy::type::matcher::MetadataMatcher& matcher) : matcher_(matcher), path_(matcher.path().begin(), matcher.path().end()) { - for (const envoy::type::matchers::MetadataMatcher::Value& m : matcher_.values()) { + for (const envoy::type::matcher::MetadataMatcher::Value& m : matcher_.values()) { switch (m.match_pattern_case()) { - case envoy::type::matchers::MetadataMatcher_Value::kNullMatch: + case envoy::type::matcher::MetadataMatcher_Value::kNullMatch: null_matcher_ |= m.null_match(); break; - case envoy::type::matchers::MetadataMatcher_Value::kDoubleMatch: + case envoy::type::matcher::MetadataMatcher_Value::kDoubleMatch: double_matcher_.push_back(DoubleMatcher(m.double_match())); break; - case envoy::type::matchers::MetadataMatcher_Value::kStringMatch: + case envoy::type::matcher::MetadataMatcher_Value::kStringMatch: string_matcher_.push_back(StringMatcher(m.string_match())); break; - case envoy::type::matchers::MetadataMatcher_Value::kBoolMatch: + case envoy::type::matcher::MetadataMatcher_Value::kBoolMatch: if (m.bool_match()) { bool_matcher_allow_true_ = true; } else { bool_matcher_allow_false_ = true; } break; - case envoy::type::matchers::MetadataMatcher_Value::kPresentMatch: + case envoy::type::matcher::MetadataMatcher_Value::kPresentMatch: present_matcher_ |= m.present_match(); break; default: diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index 4b83105054b13..b8f7de675a899 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -14,7 +14,7 @@ namespace Matchers { class DoubleMatcher { public: - DoubleMatcher(const envoy::type::matchers::DoubleMatcher& matcher) : matcher_(matcher) {} + DoubleMatcher(const envoy::type::matcher::DoubleMatcher& matcher) : matcher_(matcher) {} /** * Check whether the value is matched to the matcher. @@ -24,13 +24,13 @@ class DoubleMatcher { bool match(double value) const; private: - const envoy::type::matchers::DoubleMatcher matcher_; + const envoy::type::matcher::DoubleMatcher matcher_; }; class StringMatcher { public: - StringMatcher(const envoy::type::matchers::StringMatcher& matcher) : matcher_(matcher) { - if (matcher.match_pattern_case() == envoy::type::matchers::StringMatcher::kRegex) { + StringMatcher(const envoy::type::matcher::StringMatcher& matcher) : matcher_(matcher) { + if (matcher.match_pattern_case() == envoy::type::matcher::StringMatcher::kRegex) { regex_ = RegexUtil::parseRegex(matcher_.regex()); } } @@ -43,13 +43,13 @@ class StringMatcher { bool match(const std::string& value) const; private: - const envoy::type::matchers::StringMatcher matcher_; + const envoy::type::matcher::StringMatcher matcher_; std::regex regex_; }; class MetadataMatcher { public: - MetadataMatcher(const envoy::type::matchers::MetadataMatcher& matcher); + MetadataMatcher(const envoy::type::matcher::MetadataMatcher& matcher); /** * Check whether the metadata is matched to the matcher. @@ -59,7 +59,7 @@ class MetadataMatcher { bool match(const envoy::api::v2::core::Metadata& metadata) const; private: - const envoy::type::matchers::MetadataMatcher matcher_; + const envoy::type::matcher::MetadataMatcher matcher_; const std::vector path_; bool null_matcher_{false}; diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index fe6f3e1512bf4..332e511a6ecb7 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -20,7 +20,7 @@ TEST(MetadataTest, MatchNullValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_null_value(ProtobufWkt::NullValue::NULL_VALUE); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); @@ -39,7 +39,7 @@ TEST(MetadataTest, MatchDoubleValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_number_value(9); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); @@ -75,7 +75,7 @@ TEST(MetadataTest, MatchStringExactValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_string_value("prod"); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); @@ -92,7 +92,7 @@ TEST(MetadataTest, MatchStringPrefixValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_string_value("prodabc"); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); @@ -111,7 +111,7 @@ TEST(MetadataTest, MatchStringSuffixValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_string_value("abcprod"); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); @@ -131,7 +131,7 @@ TEST(MetadataTest, MatchBoolValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_bool_value(true); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); @@ -150,7 +150,7 @@ TEST(MetadataTest, MatchPresentValue) { Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label") .set_number_value(1); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); matcher.add_path("label"); diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 08e4dff6e20d0..267671af68af4 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -223,7 +223,7 @@ TEST(MetadataMatcher, MetadataMatcher) { metadata.mutable_filter_metadata()->insert( Protobuf::MapPair("rbac", label)); - envoy::type::matchers::MetadataMatcher matcher; + envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("rbac"); matcher.add_path("label"); From 5d4e01d47012bb8d45bb721a9abdf2e8276969eb Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Wed, 20 Jun 2018 13:29:48 -0700 Subject: [PATCH 5/7] rename matchers -> matcher, add NullMatch and PathSegment. Signed-off-by: Yangmin Zhu --- api/docs/BUILD | 6 ++-- api/envoy/config/rbac/v2alpha/BUILD | 4 +-- api/envoy/config/rbac/v2alpha/rbac.proto | 2 +- api/envoy/type/{matchers => matcher}/BUILD | 0 .../type/{matchers => matcher}/metadata.proto | 29 ++++++++++++++----- .../type/{matchers => matcher}/number.proto | 0 .../type/{matchers => matcher}/string.proto | 0 docs/build.sh | 6 ++-- docs/root/api-v2/types/types.rst | 6 ++-- source/common/common/BUILD | 6 ++-- source/common/common/matchers.cc | 7 +++-- source/common/common/matchers.h | 8 ++--- test/common/common/BUILD | 6 ++-- test/common/common/matchers_test.cc | 26 ++++++++--------- .../filters/common/rbac/matchers_test.cc | 2 +- 15 files changed, 61 insertions(+), 47 deletions(-) rename api/envoy/type/{matchers => matcher}/BUILD (100%) rename api/envoy/type/{matchers => matcher}/metadata.proto (73%) rename api/envoy/type/{matchers => matcher}/number.proto (100%) rename api/envoy/type/{matchers => matcher}/string.proto (100%) diff --git a/api/docs/BUILD b/api/docs/BUILD index 823cd32f2a478..fcb7e8ca6cc2f 100644 --- a/api/docs/BUILD +++ b/api/docs/BUILD @@ -65,8 +65,8 @@ proto_library( "//envoy/service/metrics/v2:metrics_service", "//envoy/type:percent", "//envoy/type:range", - "//envoy/type/matchers:metadata", - "//envoy/type/matchers:number", - "//envoy/type/matchers:string", + "//envoy/type/matcher:metadata", + "//envoy/type/matcher:number", + "//envoy/type/matcher:string", ], ) diff --git a/api/envoy/config/rbac/v2alpha/BUILD b/api/envoy/config/rbac/v2alpha/BUILD index 8de09c3e44fbc..b889a3dfaa0ab 100644 --- a/api/envoy/config/rbac/v2alpha/BUILD +++ b/api/envoy/config/rbac/v2alpha/BUILD @@ -9,7 +9,7 @@ api_proto_library( deps = [ "//envoy/api/v2/core:address", "//envoy/api/v2/route", - "//envoy/type/matchers:metadata", + "//envoy/type/matcher:metadata", ], ) @@ -19,6 +19,6 @@ api_go_proto_library( deps = [ "//envoy/api/v2/core:address_go_proto", "//envoy/api/v2/route:route_go_proto", - "//envoy/type/matchers:metadata_go_proto", + "//envoy/type/matcher:metadata_go_proto", ], ) diff --git a/api/envoy/config/rbac/v2alpha/rbac.proto b/api/envoy/config/rbac/v2alpha/rbac.proto index e57772b87043b..1542dd03d3369 100644 --- a/api/envoy/config/rbac/v2alpha/rbac.proto +++ b/api/envoy/config/rbac/v2alpha/rbac.proto @@ -3,7 +3,7 @@ syntax = "proto3"; import "validate/validate.proto"; import "envoy/api/v2/core/address.proto"; import "envoy/api/v2/route/route.proto"; -import "envoy/type/matchers/metadata.proto"; +import "envoy/type/matcher/metadata.proto"; package envoy.config.rbac.v2alpha; option go_package = "v2alpha"; diff --git a/api/envoy/type/matchers/BUILD b/api/envoy/type/matcher/BUILD similarity index 100% rename from api/envoy/type/matchers/BUILD rename to api/envoy/type/matcher/BUILD diff --git a/api/envoy/type/matchers/metadata.proto b/api/envoy/type/matcher/metadata.proto similarity index 73% rename from api/envoy/type/matchers/metadata.proto rename to api/envoy/type/matcher/metadata.proto index 341e889202b06..c64727dd1be31 100644 --- a/api/envoy/type/matchers/metadata.proto +++ b/api/envoy/type/matcher/metadata.proto @@ -3,8 +3,8 @@ syntax = "proto3"; package envoy.type.matcher; option go_package = "matcher"; -import "envoy/type/matchers/string.proto"; -import "envoy/type/matchers/number.proto"; +import "envoy/type/matcher/string.proto"; +import "envoy/type/matcher/number.proto"; import "validate/validate.proto"; @@ -17,16 +17,30 @@ import "validate/validate.proto"; // An example use of MetadataMatcher is specifying additional metadata in envoy.filters.http.rbac to // enforce access control based on dynamic metadata in a request. message MetadataMatcher { + // Specifies the segment in a path to retrieve value from Metadata. + // Note: Array is not supported for now and will result a not match directly. + message PathSegment { + oneof segment { + option (validate.required) = true; + + // If specified, use the key to retrieve the value in a Struct. + string key = 1 [(validate.rules).string.min_bytes = 1]; + } + } + // Specifies the value to match. Only primitive value is supported. For non-primitive value, the // result is always not matched. message Value { + // NullMatch is an empty message to specify a null value. + message NullMatch { + } + // Specifies how to match a value. Only have effect on primitive value. oneof match_pattern { option (validate.required) = true; - // If specified, it's matched if and only if the target value is a NullValue and this field is - // also set to true. - bool null_match = 1; + // If specified, it's matched if and only if the target value is a NullValue. + NullMatch null_match = 1; // If specified, it's matched if and only if the target value is a double value and is matched // to this field. @@ -50,9 +64,8 @@ message MetadataMatcher { // Required. The filter name to retrieve the Struct from the Metadata. string filter = 1 [(validate.rules).string.min_bytes = 1]; - // Required. The multi-key path to retrieve the Value from the Struct. - // Note: Array is not supported and will result a not match discarding any specified values. - repeated string path = 2 [(validate.rules).repeated .min_items = 1]; + // Required. The path to retrieve the Value from the Struct. + repeated PathSegment path = 2 [(validate.rules).repeated .min_items = 1]; // Required. A set of values to match. The MetadataMatcher is matched if at least one value is // matched, in other words, it's matched with OR semantics. diff --git a/api/envoy/type/matchers/number.proto b/api/envoy/type/matcher/number.proto similarity index 100% rename from api/envoy/type/matchers/number.proto rename to api/envoy/type/matcher/number.proto diff --git a/api/envoy/type/matchers/string.proto b/api/envoy/type/matcher/string.proto similarity index 100% rename from api/envoy/type/matchers/string.proto rename to api/envoy/type/matcher/string.proto diff --git a/docs/build.sh b/docs/build.sh index be20527f0d01b..5776a3c97f7d8 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -107,9 +107,9 @@ PROTO_RST=" /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/external_auth.proto.rst /envoy/type/percent/envoy/type/percent.proto.rst /envoy/type/range/envoy/type/range.proto.rst - /envoy/type/matchers/metadata/envoy/type/matchers/metadata.proto.rst - /envoy/type/matchers/number/envoy/type/matchers/number.proto.rst - /envoy/type/matchers/string/envoy/type/matchers/string.proto.rst + /envoy/type/matcher/metadata/envoy/type/matcher/metadata.proto.rst + /envoy/type/matcher/number/envoy/type/matcher/number.proto.rst + /envoy/type/matcher/string/envoy/type/matcher/string.proto.rst " # Dump all the generated RST so they can be added to PROTO_RST easily. diff --git a/docs/root/api-v2/types/types.rst b/docs/root/api-v2/types/types.rst index 2531a6e34e75d..cab15e217698e 100644 --- a/docs/root/api-v2/types/types.rst +++ b/docs/root/api-v2/types/types.rst @@ -7,6 +7,6 @@ Types ../type/percent.proto ../type/range.proto - ../type/matchers/metadata.proto - ../type/matchers/number.proto - ../type/matchers/string.proto + ../type/matcher/metadata.proto + ../type/matcher/number.proto + ../type/matcher/string.proto diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 7105a23847040..a66f9727f1325 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -121,9 +121,9 @@ envoy_cc_library( ":utility_lib", "//source/common/config:metadata_lib", "//source/common/protobuf", - "@envoy_api//envoy/type/matchers:metadata_cc", - "@envoy_api//envoy/type/matchers:number_cc", - "@envoy_api//envoy/type/matchers:string_cc", + "@envoy_api//envoy/type/matcher:metadata_cc", + "@envoy_api//envoy/type/matcher:number_cc", + "@envoy_api//envoy/type/matcher:string_cc", ], ) diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index 47687a6b22641..dede043926d82 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -37,11 +37,14 @@ bool StringMatcher::match(const std::string& value) const { } MetadataMatcher::MetadataMatcher(const envoy::type::matcher::MetadataMatcher& matcher) - : matcher_(matcher), path_(matcher.path().begin(), matcher.path().end()) { + : matcher_(matcher) { + for (const auto& seg : matcher.path()) { + path_.push_back(seg.key()); + } for (const envoy::type::matcher::MetadataMatcher::Value& m : matcher_.values()) { switch (m.match_pattern_case()) { case envoy::type::matcher::MetadataMatcher_Value::kNullMatch: - null_matcher_ |= m.null_match(); + null_matcher_ = true; break; case envoy::type::matcher::MetadataMatcher_Value::kDoubleMatch: double_matcher_.push_back(DoubleMatcher(m.double_match())); diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index b8f7de675a899..75fed75fe6c63 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -3,9 +3,9 @@ #include #include "envoy/api/v2/core/base.pb.h" -#include "envoy/type/matchers/metadata.pb.h" -#include "envoy/type/matchers/number.pb.h" -#include "envoy/type/matchers/string.pb.h" +#include "envoy/type/matcher/metadata.pb.h" +#include "envoy/type/matcher/number.pb.h" +#include "envoy/type/matcher/string.pb.h" #include "common/common/utility.h" @@ -60,7 +60,7 @@ class MetadataMatcher { private: const envoy::type::matcher::MetadataMatcher matcher_; - const std::vector path_; + std::vector path_; bool null_matcher_{false}; bool bool_matcher_allow_true_{false}; diff --git a/test/common/common/BUILD b/test/common/common/BUILD index 75c8eb9adaeab..82b9818292eeb 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -79,9 +79,9 @@ envoy_cc_test( "//source/common/config:metadata_lib", "//source/common/protobuf:utility_lib", "@envoy_api//envoy/api/v2/core:base_cc", - "@envoy_api//envoy/type/matchers:metadata_cc", - "@envoy_api//envoy/type/matchers:number_cc", - "@envoy_api//envoy/type/matchers:string_cc", + "@envoy_api//envoy/type/matcher:metadata_cc", + "@envoy_api//envoy/type/matcher:number_cc", + "@envoy_api//envoy/type/matcher:string_cc", ], ) diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index 332e511a6ecb7..177cb83f5726f 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -1,7 +1,7 @@ #include "envoy/api/v2/core/base.pb.h" -#include "envoy/type/matchers/metadata.pb.h" -#include "envoy/type/matchers/number.pb.h" -#include "envoy/type/matchers/string.pb.h" +#include "envoy/type/matcher/metadata.pb.h" +#include "envoy/type/matcher/number.pb.h" +#include "envoy/type/matcher/string.pb.h" #include "common/common/matchers.h" #include "common/config/metadata.h" @@ -22,13 +22,11 @@ TEST(MetadataTest, MatchNullValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->set_null_match(false); - EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->set_null_match(true); + matcher.add_values()->mutable_null_match(); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); } @@ -41,7 +39,7 @@ TEST(MetadataTest, MatchDoubleValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -77,7 +75,7 @@ TEST(MetadataTest, MatchStringExactValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -94,7 +92,7 @@ TEST(MetadataTest, MatchStringPrefixValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -113,7 +111,7 @@ TEST(MetadataTest, MatchStringSuffixValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -133,7 +131,7 @@ TEST(MetadataTest, MatchBoolValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -152,7 +150,7 @@ TEST(MetadataTest, MatchPresentValue) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("envoy.filter.b"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -162,7 +160,7 @@ TEST(MetadataTest, MatchPresentValue) { EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); matcher.clear_path(); - matcher.add_path("unknown"); + matcher.add_path()->set_key("unknown"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); } diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 267671af68af4..22a905a35d73c 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -225,7 +225,7 @@ TEST(MetadataMatcher, MetadataMatcher) { envoy::type::matcher::MetadataMatcher matcher; matcher.set_filter("rbac"); - matcher.add_path("label"); + matcher.add_path()->set_key("label"); matcher.add_values()->mutable_string_match()->set_exact("test"); checkMatcher(MetadataMatcher(matcher), false, conn, header, metadata); From 530deddaeaee949ea06b965606893fea1ab09841 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Mon, 25 Jun 2018 12:22:17 -0700 Subject: [PATCH 6/7] address comments, update docs and small updates. Signed-off-by: Yangmin Zhu --- api/envoy/config/rbac/v2alpha/rbac.proto | 4 +- api/envoy/type/matcher/metadata.proto | 83 +++++++++++++++++++----- api/envoy/type/matcher/number.proto | 1 + source/common/common/matchers.cc | 12 ++-- 4 files changed, 79 insertions(+), 21 deletions(-) diff --git a/api/envoy/config/rbac/v2alpha/rbac.proto b/api/envoy/config/rbac/v2alpha/rbac.proto index 1542dd03d3369..5f332cd27a35f 100644 --- a/api/envoy/config/rbac/v2alpha/rbac.proto +++ b/api/envoy/config/rbac/v2alpha/rbac.proto @@ -113,7 +113,7 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32.lte = 65535]; - // A metadata that describes additional information about the action. + // Metadata that describes additional information about the action. envoy.type.matcher.MetadataMatcher metadata = 7; } } @@ -155,7 +155,7 @@ message Principal { // A header (or psuedo-header such as :path or :method) on the incoming HTTP request. envoy.api.v2.route.HeaderMatcher header = 6; - // A metadata that describes additional information about the principal. + // Metadata that describes additional information about the principal. envoy.type.matcher.MetadataMatcher metadata = 7; } } diff --git a/api/envoy/type/matcher/metadata.proto b/api/envoy/type/matcher/metadata.proto index c64727dd1be31..905e426ff3308 100644 --- a/api/envoy/type/matcher/metadata.proto +++ b/api/envoy/type/matcher/metadata.proto @@ -14,11 +14,64 @@ import "validate/validate.proto"; // :ref:`Metadata `. It uses `filter` and `path` to retrieve the value // from the Metadata and then check if it's matched to one of the specified values. // +// For example, for the following Metadata: +// +// .. code-block:: yaml +// +// filter_metadata: +// envoy.filters.http.rbac: +// fields: +// a: +// struct_value: +// fields: +// b: +// struct_value: +// fields: +// c: +// string_value: pro +// t: +// list_value: +// values: +// - string_value: m +// - string_value: n +// +// The following MetadataMatcher is matched as the path [a, b, c] will retrieve a string value "pro" +// from the Metadata which is matched to the prefix match specified in values. +// +// .. code-block:: yaml +// +// filter: envoy.filters.http.rbac +// path: +// - key: a +// - key: b +// - key: c +// values: +// - string_match: +// exact: ex +// - string_match: +// prefix: pr +// +// The following MetadataMatcher is not matched as the path [a, t] is pointing to a list value in +// the Metadata which is not supported for now. +// +// .. code-block:: yaml +// +// filter: envoy.filters.http.rbac +// path: +// - key: a +// - key: t +// values: +// - string_match: +// exact: m +// // An example use of MetadataMatcher is specifying additional metadata in envoy.filters.http.rbac to -// enforce access control based on dynamic metadata in a request. +// enforce access control based on dynamic metadata in a request. See :ref:`Permission +// ` and :ref:`Principal +// `. message MetadataMatcher { // Specifies the segment in a path to retrieve value from Metadata. - // Note: Array is not supported for now and will result a not match directly. + // Note: Currently it's not supported to retrieve a value from a list in Metadata. This means it + // will always be not matched if the associated value of the key is a list. message PathSegment { oneof segment { option (validate.required) = true; @@ -28,30 +81,30 @@ message MetadataMatcher { } } - // Specifies the value to match. Only primitive value is supported. For non-primitive value, the + // Specifies the value to match. Only primitive value are supported. For non-primitive values, the // result is always not matched. message Value { // NullMatch is an empty message to specify a null value. message NullMatch { } - // Specifies how to match a value. Only have effect on primitive value. + // Specifies how to match a value. oneof match_pattern { option (validate.required) = true; - // If specified, it's matched if and only if the target value is a NullValue. + // If specified, a match occurs if and only if the target value is a NullValue. NullMatch null_match = 1; - // If specified, it's matched if and only if the target value is a double value and is matched - // to this field. + // If specified, a match occurs if and only if the target value is a double value and is + // matched to this field. DoubleMatcher double_match = 2; - // If specified, it's matched if and only if the target value is a string value and is matched - // to this field. + // If specified, a match occurs if and only if the target value is a string value and is + // matched to this field. StringMatcher string_match = 3; - // If specified, it's matched if and only if the target value is a bool value and is equal to - // this field. + // If specified, a match occurs if and only if the target value is a bool value and is equal + // to this field. bool bool_match = 4; // If specified, value match will be performed based on whether the path is referring to a @@ -61,13 +114,13 @@ message MetadataMatcher { } } - // Required. The filter name to retrieve the Struct from the Metadata. + // The filter name to retrieve the Struct from the Metadata. string filter = 1 [(validate.rules).string.min_bytes = 1]; - // Required. The path to retrieve the Value from the Struct. + // The path to retrieve the Value from the Struct. repeated PathSegment path = 2 [(validate.rules).repeated .min_items = 1]; - // Required. A set of values to match. The MetadataMatcher is matched if at least one value is - // matched, in other words, it's matched with OR semantics. + // A set of values to match. The MetadataMatcher is matched if at least one value is + // matched, in other words, a match occurs with OR semantics. repeated Value values = 3 [(validate.rules).repeated .min_items = 1]; } diff --git a/api/envoy/type/matcher/number.proto b/api/envoy/type/matcher/number.proto index 92fc6a78fb755..9cf4ff1f10458 100644 --- a/api/envoy/type/matcher/number.proto +++ b/api/envoy/type/matcher/number.proto @@ -15,6 +15,7 @@ message DoubleMatcher { option (validate.required) = true; // If specified, the input double value must be in the range specified here. + // Note: The range is using half-open interval semantics [start, end). envoy.type.DoubleRange range = 1; // If specified, the input double value must be equal to the value specified here. diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index dede043926d82..a39c76c0d44c6 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -17,7 +17,7 @@ bool DoubleMatcher::match(double value) const { case envoy::type::matcher::DoubleMatcher::kExact: return matcher_.exact() == value; default: - return false; + NOT_REACHED; }; } @@ -32,7 +32,7 @@ bool StringMatcher::match(const std::string& value) const { case envoy::type::matcher::StringMatcher::kRegex: return std::regex_match(value, regex_); default: - return false; + NOT_REACHED; } } @@ -63,7 +63,7 @@ MetadataMatcher::MetadataMatcher(const envoy::type::matcher::MetadataMatcher& ma present_matcher_ |= m.present_match(); break; default: - break; + NOT_REACHED; } } } @@ -93,8 +93,12 @@ bool MetadataMatcher::match(const envoy::api::v2::core::Metadata& metadata) cons case ProtobufWkt::Value::kBoolValue: return (bool_matcher_allow_true_ && value.bool_value()) || (bool_matcher_allow_false_ && !value.bool_value()); - default: + case ProtobufWkt::Value::kListValue: + case ProtobufWkt::Value::kStructValue: + case ProtobufWkt::Value::KIND_NOT_SET: return false; + default: + NOT_REACHED; } } From a9467d4b26ac3c3f3343974307790ae55cb83cf3 Mon Sep 17 00:00:00 2001 From: Yangmin Zhu Date: Mon, 25 Jun 2018 19:08:06 -0700 Subject: [PATCH 7/7] address comments, make metadata.value not repeated. Signed-off-by: Yangmin Zhu --- api/envoy/type/matcher/metadata.proto | 19 +++--- source/common/common/BUILD | 1 + source/common/common/matchers.cc | 60 +++++++------------ source/common/common/matchers.h | 9 +-- test/common/common/matchers_test.cc | 46 +++++++------- .../filters/common/rbac/matchers_test.cc | 4 +- 6 files changed, 60 insertions(+), 79 deletions(-) diff --git a/api/envoy/type/matcher/metadata.proto b/api/envoy/type/matcher/metadata.proto index 905e426ff3308..f899bc1305251 100644 --- a/api/envoy/type/matcher/metadata.proto +++ b/api/envoy/type/matcher/metadata.proto @@ -12,7 +12,7 @@ import "validate/validate.proto"; // MetadataMatcher provides a general interface to check if a given value is matched in // :ref:`Metadata `. It uses `filter` and `path` to retrieve the value -// from the Metadata and then check if it's matched to one of the specified values. +// from the Metadata and then check if it's matched to the specified value. // // For example, for the following Metadata: // @@ -36,7 +36,7 @@ import "validate/validate.proto"; // - string_value: n // // The following MetadataMatcher is matched as the path [a, b, c] will retrieve a string value "pro" -// from the Metadata which is matched to the prefix match specified in values. +// from the Metadata which is matched to the specified prefix match. // // .. code-block:: yaml // @@ -45,10 +45,8 @@ import "validate/validate.proto"; // - key: a // - key: b // - key: c -// values: -// - string_match: -// exact: ex -// - string_match: +// value: +// string_match: // prefix: pr // // The following MetadataMatcher is not matched as the path [a, t] is pointing to a list value in @@ -60,8 +58,8 @@ import "validate/validate.proto"; // path: // - key: a // - key: t -// values: -// - string_match: +// value: +// string_match: // exact: m // // An example use of MetadataMatcher is specifying additional metadata in envoy.filters.http.rbac to @@ -120,7 +118,6 @@ message MetadataMatcher { // The path to retrieve the Value from the Struct. repeated PathSegment path = 2 [(validate.rules).repeated .min_items = 1]; - // A set of values to match. The MetadataMatcher is matched if at least one value is - // matched, in other words, a match occurs with OR semantics. - repeated Value values = 3 [(validate.rules).repeated .min_items = 1]; + // The MetadataMatcher is matched if the value retrieved by path is matched to this value. + Value value = 3 [(validate.rules).message.required = true]; } diff --git a/source/common/common/BUILD b/source/common/common/BUILD index a66f9727f1325..9d070f59939bd 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -117,6 +117,7 @@ envoy_cc_library( name = "matchers_lib", srcs = ["matchers.cc"], hdrs = ["matchers.h"], + external_deps = ["abseil_optional"], deps = [ ":utility_lib", "//source/common/config:metadata_lib", diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index a39c76c0d44c6..d95e3a0e8dcb1 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -41,30 +41,25 @@ MetadataMatcher::MetadataMatcher(const envoy::type::matcher::MetadataMatcher& ma for (const auto& seg : matcher.path()) { path_.push_back(seg.key()); } - for (const envoy::type::matcher::MetadataMatcher::Value& m : matcher_.values()) { - switch (m.match_pattern_case()) { - case envoy::type::matcher::MetadataMatcher_Value::kNullMatch: - null_matcher_ = true; - break; - case envoy::type::matcher::MetadataMatcher_Value::kDoubleMatch: - double_matcher_.push_back(DoubleMatcher(m.double_match())); - break; - case envoy::type::matcher::MetadataMatcher_Value::kStringMatch: - string_matcher_.push_back(StringMatcher(m.string_match())); - break; - case envoy::type::matcher::MetadataMatcher_Value::kBoolMatch: - if (m.bool_match()) { - bool_matcher_allow_true_ = true; - } else { - bool_matcher_allow_false_ = true; - } - break; - case envoy::type::matcher::MetadataMatcher_Value::kPresentMatch: - present_matcher_ |= m.present_match(); - break; - default: - NOT_REACHED; - } + const auto& v = matcher_.value(); + switch (v.match_pattern_case()) { + case envoy::type::matcher::MetadataMatcher_Value::kNullMatch: + null_matcher_ = true; + break; + case envoy::type::matcher::MetadataMatcher_Value::kDoubleMatch: + double_matcher_.emplace(v.double_match()); + break; + case envoy::type::matcher::MetadataMatcher_Value::kStringMatch: + string_matcher_.emplace(v.string_match()); + break; + case envoy::type::matcher::MetadataMatcher_Value::kBoolMatch: + bool_matcher_.emplace(v.bool_match()); + break; + case envoy::type::matcher::MetadataMatcher_Value::kPresentMatch: + present_matcher_ = v.present_match(); + break; + default: + NOT_REACHED; } } @@ -77,22 +72,11 @@ bool MetadataMatcher::match(const envoy::api::v2::core::Metadata& metadata) cons case ProtobufWkt::Value::kNullValue: return null_matcher_; case ProtobufWkt::Value::kNumberValue: - for (const auto& m : double_matcher_) { - if (m.match(value.number_value())) { - return true; - } - } - return false; + return double_matcher_.has_value() && double_matcher_->match(value.number_value()); case ProtobufWkt::Value::kStringValue: - for (const auto& m : string_matcher_) { - if (m.match(value.string_value())) { - return true; - } - } - return false; + return string_matcher_.has_value() && string_matcher_->match(value.string_value()); case ProtobufWkt::Value::kBoolValue: - return (bool_matcher_allow_true_ && value.bool_value()) || - (bool_matcher_allow_false_ && !value.bool_value()); + return (bool_matcher_.has_value() && *bool_matcher_ == value.bool_value()); case ProtobufWkt::Value::kListValue: case ProtobufWkt::Value::kStructValue: case ProtobufWkt::Value::KIND_NOT_SET: diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index 75fed75fe6c63..36197b7f7c1d6 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -9,6 +9,8 @@ #include "common/common/utility.h" +#include "absl/types/optional.h" + namespace Envoy { namespace Matchers { @@ -63,12 +65,11 @@ class MetadataMatcher { std::vector path_; bool null_matcher_{false}; - bool bool_matcher_allow_true_{false}; - bool bool_matcher_allow_false_{false}; + absl::optional bool_matcher_; bool present_matcher_{false}; - std::vector double_matcher_; - std::vector string_matcher_; + absl::optional double_matcher_; + absl::optional string_matcher_; }; } // namespace Matchers diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index 177cb83f5726f..5e081276dbf7b 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -24,9 +24,9 @@ TEST(MetadataTest, MatchNullValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_null_match(); + matcher.mutable_value()->mutable_null_match(); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); } @@ -41,26 +41,24 @@ TEST(MetadataTest, MatchDoubleValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_double_match()->set_exact(1); + matcher.mutable_value()->mutable_double_match()->set_exact(1); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_double_match()->set_exact(9); + matcher.mutable_value()->mutable_double_match()->set_exact(9); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.clear_values(); - EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - auto r = matcher.add_values()->mutable_double_match()->mutable_range(); + auto r = matcher.mutable_value()->mutable_double_match()->mutable_range(); r->set_start(9.1); r->set_end(10); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - r = matcher.add_values()->mutable_double_match()->mutable_range(); + r = matcher.mutable_value()->mutable_double_match()->mutable_range(); r->set_start(8.9); r->set_end(9); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - r = matcher.add_values()->mutable_double_match()->mutable_range(); + r = matcher.mutable_value()->mutable_double_match()->mutable_range(); r->set_start(9); r->set_end(9.1); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); @@ -77,9 +75,9 @@ TEST(MetadataTest, MatchStringExactValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_string_match()->set_exact("prod"); + matcher.mutable_value()->mutable_string_match()->set_exact("prod"); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); } @@ -94,11 +92,11 @@ TEST(MetadataTest, MatchStringPrefixValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_string_match()->set_prefix("prodx"); + matcher.mutable_value()->mutable_string_match()->set_prefix("prodx"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_string_match()->set_prefix("prod"); + matcher.mutable_value()->mutable_string_match()->set_prefix("prod"); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); } @@ -113,11 +111,11 @@ TEST(MetadataTest, MatchStringSuffixValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_string_match()->set_suffix("prodx"); + matcher.mutable_value()->mutable_string_match()->set_suffix("prodx"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->mutable_string_match()->set_suffix("prod"); + matcher.mutable_value()->mutable_string_match()->set_suffix("prod"); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); ; } @@ -133,11 +131,11 @@ TEST(MetadataTest, MatchBoolValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->set_bool_match(false); + matcher.mutable_value()->set_bool_match(false); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->set_bool_match(true); + matcher.mutable_value()->set_bool_match(true); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); } @@ -152,11 +150,11 @@ TEST(MetadataTest, MatchPresentValue) { matcher.set_filter("envoy.filter.b"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->set_present_match(false); + matcher.mutable_value()->set_present_match(false); EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); - matcher.add_values()->set_present_match(true); + matcher.mutable_value()->set_present_match(true); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata)); matcher.clear_path(); diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 22a905a35d73c..53cdb0f1229ef 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -227,9 +227,9 @@ TEST(MetadataMatcher, MetadataMatcher) { matcher.set_filter("rbac"); matcher.add_path()->set_key("label"); - matcher.add_values()->mutable_string_match()->set_exact("test"); + matcher.mutable_value()->mutable_string_match()->set_exact("test"); checkMatcher(MetadataMatcher(matcher), false, conn, header, metadata); - matcher.add_values()->mutable_string_match()->set_exact("prod"); + matcher.mutable_value()->mutable_string_match()->set_exact("prod"); checkMatcher(MetadataMatcher(matcher), true, conn, header, metadata); }