From 7fec609a507371d7176c61aa4623f445543f294f Mon Sep 17 00:00:00 2001 From: Thomas <1607559+thomasvnoort@users.noreply.github.com> Date: Tue, 19 Mar 2024 02:22:28 +0100 Subject: [PATCH] rbac: rules_stat_prefix to emit metrics with a prefix (#31835) This is akin to shadow_rules_stat_prefix but for non-shadowing rules. Since only shadow rules emit dynamic metadata, this prefix only applies to metrics. --------- Signed-off-by: Thomas van Noort --- .../filters/http/rbac/v3/rbac.proto | 7 +- changelogs/current.yaml | 4 + .../http/http_filters/rbac_filter.rst | 7 +- source/common/config/well_known_names.cc | 3 + source/common/config/well_known_names.h | 2 + .../extensions/filters/common/rbac/utility.cc | 11 ++- .../extensions/filters/common/rbac/utility.h | 6 +- .../filters/http/rbac/rbac_filter.cc | 2 +- .../filters/network/rbac/rbac_filter.cc | 2 +- test/common/stats/tag_extractor_impl_test.cc | 11 +++ .../filters/http/rbac/rbac_filter_test.cc | 81 +++++++++++------ .../filters/network/rbac/filter_test.cc | 88 ++++++++++++------- 12 files changed, 152 insertions(+), 72 deletions(-) diff --git a/api/envoy/extensions/filters/http/rbac/v3/rbac.proto b/api/envoy/extensions/filters/http/rbac/v3/rbac.proto index eeb505a17fb7..b14478b70e8f 100644 --- a/api/envoy/extensions/filters/http/rbac/v3/rbac.proto +++ b/api/envoy/extensions/filters/http/rbac/v3/rbac.proto @@ -22,7 +22,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#extension: envoy.filters.http.rbac] // RBAC filter config. -// [#next-free-field: 6] +// [#next-free-field: 7] message RBAC { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.rbac.v2.RBAC"; @@ -34,6 +34,11 @@ message RBAC { config.rbac.v3.RBAC rules = 1 [(udpa.annotations.field_migrate).oneof_promotion = "rules_specifier"]; + // If specified, rules will emit stats with the given prefix. + // This is useful to distinguish the stat when there are more than 1 RBAC filter configured with + // rules. + string rules_stat_prefix = 6; + // The match tree to use when resolving RBAC action for incoming requests. Requests do not // match any matcher will be denied. // If absent, no enforcing RBAC matcher will be applied. diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 092048320fae..aa29fe080592 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -339,6 +339,10 @@ new_features: Update ``aws_request_signing`` filter to support optionally sending the aws signature in query parameters rather than headers, by specifying the :ref:`query_string ` configuration section. +- area: rbac + change: | + Added :ref:`rules_stat_prefix ` + to allow adding custom prefix to the stats emitted by rules. deprecated: - area: listener diff --git a/docs/root/configuration/http/http_filters/rbac_filter.rst b/docs/root/configuration/http/http_filters/rbac_filter.rst index db30b5123a01..4e2e5990392e 100644 --- a/docs/root/configuration/http/http_filters/rbac_filter.rst +++ b/docs/root/configuration/http/http_filters/rbac_filter.rst @@ -33,7 +33,12 @@ The RBAC filter outputs statistics in the ``http..rbac.`` namespace ` comes from the owning HTTP connection manager. -For the shadow rule statistics ``shadow_allowed`` and ``shadow_denied``, the :ref:`shadow_rules_stat_prefix ` +For the rule statistics ``allowed`` and ``denied``, +the :ref:`rules_stat_prefix ` +can be used to add an extra prefix to output the statistics in the ``http..rbac..`` namespace. + +For the shadow rule statistics ``shadow_allowed`` and ``shadow_denied``, +the :ref:`shadow_rules_stat_prefix ` can be used to add an extra prefix to output the statistics in the ``http..rbac..`` namespace. .. csv-table:: diff --git a/source/common/config/well_known_names.cc b/source/common/config/well_known_names.cc index 4e390568c31d..104b182bfb1a 100644 --- a/source/common/config/well_known_names.cc +++ b/source/common/config/well_known_names.cc @@ -207,6 +207,9 @@ TagNameValues::TagNameValues() { // (.).rbac.** addTokenized(RBAC_PREFIX, "$.rbac.**"); + + // http..rbac.(.)* + addTokenized(RBAC_HTTP_PREFIX, "http.*.rbac.$.**"); } void TagNameValues::addRe2(const std::string& name, const std::string& regex, diff --git a/source/common/config/well_known_names.h b/source/common/config/well_known_names.h index a0a40cc79496..8cf5fbd14715 100644 --- a/source/common/config/well_known_names.h +++ b/source/common/config/well_known_names.h @@ -123,6 +123,8 @@ class TagNameValues { const std::string CONNECTION_LIMIT_PREFIX = "envoy.connection_limit_prefix"; // Stats prefix for the RBAC network filter const std::string RBAC_PREFIX = "envoy.rbac_prefix"; + // Stats prefix for the RBAC http filter + const std::string RBAC_HTTP_PREFIX = "envoy.rbac_http_prefix"; // Stats prefix for the TCP Proxy network filter const std::string TCP_PREFIX = "envoy.tcp_prefix"; // Stats prefix for the UDP Proxy network filter diff --git a/source/extensions/filters/common/rbac/utility.cc b/source/extensions/filters/common/rbac/utility.cc index 710342d7b8fd..cc14523ee1fb 100644 --- a/source/extensions/filters/common/rbac/utility.cc +++ b/source/extensions/filters/common/rbac/utility.cc @@ -10,11 +10,14 @@ namespace Filters { namespace Common { namespace RBAC { -RoleBasedAccessControlFilterStats -generateStats(const std::string& prefix, const std::string& shadow_prefix, Stats::Scope& scope) { +RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, + const std::string& rules_prefix, + const std::string& shadow_rules_prefix, + Stats::Scope& scope) { const std::string final_prefix = Envoy::statPrefixJoin(prefix, "rbac."); - return {ENFORCE_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix)) - SHADOW_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix + shadow_prefix))}; + return { + ENFORCE_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix + rules_prefix)) + SHADOW_RBAC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix + shadow_rules_prefix))}; } std::string responseDetail(const std::string& policy_id) { diff --git a/source/extensions/filters/common/rbac/utility.h b/source/extensions/filters/common/rbac/utility.h index 79cd8be7d2bf..fe17def3c6e1 100644 --- a/source/extensions/filters/common/rbac/utility.h +++ b/source/extensions/filters/common/rbac/utility.h @@ -34,8 +34,10 @@ struct RoleBasedAccessControlFilterStats { SHADOW_RBAC_FILTER_STATS(GENERATE_COUNTER_STRUCT) }; -RoleBasedAccessControlFilterStats -generateStats(const std::string& prefix, const std::string& shadow_prefix, Stats::Scope& scope); +RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, + const std::string& rules_prefix, + const std::string& shadow_rules_prefix, + Stats::Scope& scope); template std::unique_ptr diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index bb6950942611..f1ee12283824 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -61,7 +61,7 @@ RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const std::string& stats_prefix, Stats::Scope& scope, Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor) - : stats_(Filters::Common::RBAC::generateStats(stats_prefix, + : stats_(Filters::Common::RBAC::generateStats(stats_prefix, proto_config.rules_stat_prefix(), proto_config.shadow_rules_stat_prefix(), scope)), shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()), engine_(Filters::Common::RBAC::createEngine(proto_config, context, validation_visitor, diff --git a/source/extensions/filters/network/rbac/rbac_filter.cc b/source/extensions/filters/network/rbac/rbac_filter.cc index 500a0c2f617e..7d07de0f6d18 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.cc +++ b/source/extensions/filters/network/rbac/rbac_filter.cc @@ -56,7 +56,7 @@ RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope, Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor& validation_visitor) - : stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), + : stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), "", proto_config.shadow_rules_stat_prefix(), scope)), shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()), engine_(Filters::Common::RBAC::createEngine(proto_config, context, validation_visitor, diff --git a/test/common/stats/tag_extractor_impl_test.cc b/test/common/stats/tag_extractor_impl_test.cc index 43e74080a685..747f9daca0c1 100644 --- a/test/common/stats/tag_extractor_impl_test.cc +++ b/test/common/stats/tag_extractor_impl_test.cc @@ -450,6 +450,17 @@ TEST(TagExtractorTest, DefaultTagExtractors) { rbac_prefix.name_ = tag_names.RBAC_PREFIX; rbac_prefix.value_ = "my_rbac_prefix"; regex_tester.testRegex("my_rbac_prefix.rbac.allowed", "rbac.allowed", {rbac_prefix}); + + // RBAC HTTP Filter Prefix + Tag rbac_http_hcm_prefix; + rbac_http_hcm_prefix.name_ = tag_names.HTTP_CONN_MANAGER_PREFIX; + rbac_http_hcm_prefix.value_ = "hcm_prefix"; + + Tag rbac_http_prefix; + rbac_http_prefix.name_ = tag_names.RBAC_HTTP_PREFIX; + rbac_http_prefix.value_ = "prefix"; + regex_tester.testRegex("http.hcm_prefix.rbac.prefix.allowed", "http.rbac.allowed", + {rbac_http_hcm_prefix, rbac_http_prefix}); } TEST(TagExtractorTest, ExtAuthzTagExtractors) { diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index c8578f2e2622..93cacd425885 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -35,7 +35,8 @@ enum class LogResult { Yes, No, Undecided }; class RoleBasedAccessControlFilterTest : public testing::Test { public: - void setupPolicy(envoy::config::rbac::v3::RBAC::Action action) { + void setupPolicy(envoy::config::rbac::v3::RBAC::Action action, + std::string rules_stat_prefix = "") { envoy::extensions::filters::http::rbac::v3::RBAC config; envoy::config::rbac::v3::Policy policy; @@ -47,6 +48,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { policy.add_principals()->set_any(true); config.mutable_rules()->set_action(action); (*config.mutable_rules()->mutable_policies())["foo"] = policy; + config.set_rules_stat_prefix(rules_stat_prefix); envoy::config::rbac::v3::Policy shadow_policy; auto shadow_policy_rules = shadow_policy.add_permissions()->mutable_or_rules(); @@ -55,7 +57,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { shadow_policy.add_principals()->set_any(true); config.mutable_shadow_rules()->set_action(action); (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy; - config.set_shadow_rules_stat_prefix("prefix_"); + config.set_shadow_rules_stat_prefix("shadow_rules_prefix_"); setupConfig(std::make_shared( config, "test", *store_.rootScope(), context_, @@ -156,7 +158,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { TestUtility::loadFromYaml(fmt::format(shadow_matcher_yaml, action, on_no_match_action), shadow_matcher); *config.mutable_shadow_matcher() = shadow_matcher; - config.set_shadow_rules_stat_prefix("prefix_"); + config.set_shadow_rules_stat_prefix("shadow_rules_prefix_"); setupConfig(std::make_shared( config, "test", *store_.rootScope(), context_, @@ -238,7 +240,7 @@ class RoleBasedAccessControlFilterTest : public testing::Test { *config.mutable_matcher() = matcher; *config.mutable_shadow_matcher() = matcher; - config.set_shadow_rules_stat_prefix("prefix_"); + config.set_shadow_rules_stat_prefix("shadow_rules_prefix_"); setupConfig(std::make_shared( config, "test", *store_.rootScope(), context_, @@ -335,8 +337,9 @@ TEST_F(RoleBasedAccessControlFilterTest, Allowed) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -359,8 +362,9 @@ TEST_F(RoleBasedAccessControlFilterTest, RequestedServerName) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -404,12 +408,16 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) { EXPECT_EQ(1U, config_->stats().shadow_allowed_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); auto filter_meta = req_info_.dynamicMetadata().filter_metadata().at("envoy.filters.http.rbac"); - EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value()); - EXPECT_EQ("bar", filter_meta.fields().at("prefix_shadow_effective_policy_id").string_value()); + EXPECT_EQ("allowed", + filter_meta.fields().at("shadow_rules_prefix_shadow_engine_result").string_value()); + EXPECT_EQ( + "bar", + filter_meta.fields().at("shadow_rules_prefix_shadow_effective_policy_id").string_value()); EXPECT_EQ("rbac_access_denied_matched_policy[none]", callbacks_.details()); checkAccessLogMetadata(LogResult::Undecided); } @@ -496,8 +504,9 @@ TEST_F(RoleBasedAccessControlFilterTest, MatcherAllowed) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -520,8 +529,9 @@ TEST_F(RoleBasedAccessControlFilterTest, RequestedServerNameMatcher) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -565,12 +575,16 @@ TEST_F(RoleBasedAccessControlFilterTest, MatcherDenied) { EXPECT_EQ(1U, config_->stats().shadow_allowed_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); auto filter_meta = req_info_.dynamicMetadata().filter_metadata().at("envoy.filters.http.rbac"); - EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value()); - EXPECT_EQ("bar", filter_meta.fields().at("prefix_shadow_effective_policy_id").string_value()); + EXPECT_EQ("allowed", + filter_meta.fields().at("shadow_rules_prefix_shadow_engine_result").string_value()); + EXPECT_EQ( + "bar", + filter_meta.fields().at("shadow_rules_prefix_shadow_effective_policy_id").string_value()); EXPECT_EQ("rbac_access_denied_matched_policy[none]", callbacks_.details()); checkAccessLogMetadata(LogResult::Undecided); } @@ -618,8 +632,9 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -639,8 +654,9 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldNotLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -660,8 +676,9 @@ TEST_F(RoleBasedAccessControlFilterTest, MatcherShouldLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -681,8 +698,9 @@ TEST_F(RoleBasedAccessControlFilterTest, MatcherShouldNotLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("test.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("test.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("test.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("test.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); Buffer::OwnedImpl data(""); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); @@ -691,6 +709,13 @@ TEST_F(RoleBasedAccessControlFilterTest, MatcherShouldNotLog) { checkAccessLogMetadata(LogResult::No); } +TEST_F(RoleBasedAccessControlFilterTest, RulesStatPrefix) { + setupPolicy(envoy::config::rbac::v3::RBAC::ALLOW, "rules_prefix_"); + + EXPECT_EQ("test.rbac.rules_prefix_.allowed", config_->stats().allowed_.name()); + EXPECT_EQ("test.rbac.rules_prefix_.denied", config_->stats().denied_.name()); +} + // Upstream Ip and Port matcher tests. class UpstreamIpPortMatcherTests : public RoleBasedAccessControlFilterTest { public: diff --git a/test/extensions/filters/network/rbac/filter_test.cc b/test/extensions/filters/network/rbac/filter_test.cc index df60598f379d..78b4d05d32ea 100644 --- a/test/extensions/filters/network/rbac/filter_test.cc +++ b/test/extensions/filters/network/rbac/filter_test.cc @@ -33,7 +33,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { envoy::extensions::filters::network::rbac::v3::RBAC config; config.set_stat_prefix("tcp."); - config.set_shadow_rules_stat_prefix("prefix_"); + config.set_shadow_rules_stat_prefix("shadow_rules_prefix_"); if (with_policy) { envoy::config::rbac::v3::Policy policy; @@ -69,7 +69,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { std::string on_no_match_action = "DENY") { envoy::extensions::filters::network::rbac::v3::RBAC config; config.set_stat_prefix("tcp."); - config.set_shadow_rules_stat_prefix("prefix_"); + config.set_shadow_rules_stat_prefix("shadow_rules_prefix_"); if (with_matcher) { constexpr absl::string_view matcher_yaml = R"EOF( @@ -250,8 +250,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithOneTimeEnforcement) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement) { @@ -270,8 +271,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithContinuousEnforcement EXPECT_EQ(2U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) { @@ -291,8 +293,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerName) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoPolicy) { @@ -309,8 +312,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoPolicy) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) { @@ -330,13 +334,17 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); auto filter_meta = stream_info_.dynamicMetadata().filter_metadata().at(NetworkFilterNames::get().Rbac); - EXPECT_EQ("bar", filter_meta.fields().at("prefix_shadow_effective_policy_id").string_value()); - EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value()); + EXPECT_EQ( + "bar", + filter_meta.fields().at("shadow_rules_prefix_shadow_effective_policy_id").string_value()); + EXPECT_EQ("allowed", + filter_meta.fields().at("shadow_rules_prefix_shadow_engine_result").string_value()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowedWithOneTimeEnforcement) { @@ -355,8 +363,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowedWithOneTimeEnforce EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowedWithContinuousEnforcement) { @@ -375,8 +384,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherAllowedWithContinuousEnfo EXPECT_EQ(2U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerNameMatcher) { @@ -396,8 +406,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, RequestedServerNameMatcher) { EXPECT_EQ(1U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoMatcher) { @@ -414,8 +425,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, AllowedWithNoMatcher) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); } TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherDenied) { @@ -435,13 +447,17 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherDenied) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); auto filter_meta = stream_info_.dynamicMetadata().filter_metadata().at(NetworkFilterNames::get().Rbac); - EXPECT_EQ("bar", filter_meta.fields().at("prefix_shadow_effective_policy_id").string_value()); - EXPECT_EQ("allowed", filter_meta.fields().at("prefix_shadow_engine_result").string_value()); + EXPECT_EQ( + "bar", + filter_meta.fields().at("shadow_rules_prefix_shadow_effective_policy_id").string_value()); + EXPECT_EQ("allowed", + filter_meta.fields().at("shadow_rules_prefix_shadow_engine_result").string_value()); } // Log Tests @@ -456,8 +472,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); checkAccessLogMetadata(true); } @@ -473,8 +490,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, ShouldNotLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); checkAccessLogMetadata(false); } @@ -504,8 +522,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherShouldLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); checkAccessLogMetadata(true); } @@ -521,8 +540,9 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, MatcherShouldNotLog) { EXPECT_EQ(0U, config_->stats().shadow_denied_.value()); EXPECT_EQ("tcp.rbac.allowed", config_->stats().allowed_.name()); EXPECT_EQ("tcp.rbac.denied", config_->stats().denied_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_allowed", config_->stats().shadow_allowed_.name()); - EXPECT_EQ("tcp.rbac.prefix_.shadow_denied", config_->stats().shadow_denied_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_allowed", + config_->stats().shadow_allowed_.name()); + EXPECT_EQ("tcp.rbac.shadow_rules_prefix_.shadow_denied", config_->stats().shadow_denied_.name()); checkAccessLogMetadata(false); }