rbac: support to configure the shadow rule stat with a custom prefix.

api/envoy/extensions/filters/http/rbac/v3/rbac.proto

@@ -29,6 +29,11 @@ message RBAC {
   // but will emit stats and logs and can be used for rule testing.
   // If absent, no shadow RBAC policy will be applied.
   config.rbac.v3.RBAC shadow_rules = 2;
+
+  // If specified, shadow 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
+  // shadow rules.
+  string shadow_rules_stat_prefix = 3;
 }
 
 message RBACPerRoute {

api/envoy/extensions/filters/network/rbac/v3/rbac.proto

@@ -21,6 +21,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 //
 // Header should not be used in rules/shadow_rules in RBAC network filter as
 // this information is only available in :ref:`RBAC http filter <config_http_filters_rbac>`.
+// [#next-free-field: 6]
 message RBAC {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.network.rbac.v2.RBAC";
@@ -45,6 +46,11 @@ message RBAC {
   // If absent, no shadow RBAC policy will be applied.
   config.rbac.v3.RBAC shadow_rules = 2;
 
+  // If specified, shadow 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
+  // shadow rules.
+  string shadow_rules_stat_prefix = 5;
+
   // The prefix to use when emitting statistics.
   string stat_prefix = 3 [(validate.rules).string = {min_len: 1}];
 

docs/root/version_history/current.rst

@@ -49,6 +49,7 @@ Minor Behavior Changes
   response HEADERS frame with the END_HEADERS flag set from upstream server.
 * oauth filter: added the optional parameter :ref:`auth_scopes <envoy_v3_api_field_extensions.filters.http.oauth2.v3alpha.OAuth2Config.auth_scopes>` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider.
 * perf: allow reading more bytes per operation from raw sockets to improve performance.
+* rbac: added :ref:`shadow_rules_stat_prefix <envoy_v3_api_field_extensions.filters.http.rbac.v3.RBAC.shadow_rules_stat_prefix>` to allow adding custom prefix to the stats emitted by shadow rules.
 * router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats <config_http_conn_man_headers_custom_request_headers>`.
 * router: made the path rewrite available without finalizing headers, so the filter could calculate the current value of the final url.
 * tracing: added `upstream_cluster.name` tag that resolves to resolve to :ref:`alt_stat_name <envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` if provided (and otherwise the cluster name).

source/extensions/filters/http/rbac/rbac_filter.cc

@@ -18,6 +18,7 @@ RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig(
     const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config,
     const std::string& stats_prefix, Stats::Scope& scope)
     : stats_(Filters::Common::RBAC::generateStats(stats_prefix, scope)),
+      shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()),
       engine_(Filters::Common::RBAC::createEngine(proto_config)),
       shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)) {}
 
@@ -91,14 +92,10 @@ RoleBasedAccessControlFilter::decodeHeaders(Http::RequestHeaderMap& headers, boo
 
     auto& fields = *metrics.mutable_fields();
     if (!effective_policy_id.empty()) {
-      *fields[Filters::Common::RBAC::DynamicMetadataKeysSingleton::get()
-                  .ShadowEffectivePolicyIdField]
-           .mutable_string_value() = effective_policy_id;
+      *fields[config_->shadowEffectivePolicyIdField()].mutable_string_value() = effective_policy_id;
     }
 
-    *fields[Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEngineResultField]
-         .mutable_string_value() = shadow_resp_code;
-
+    *fields[config_->shadowEngineResultField()].mutable_string_value() = shadow_resp_code;
     callbacks_->streamInfo().setDynamicMetadata(HttpFilterNames::get().Rbac, metrics);
   }
 

source/extensions/filters/http/rbac/rbac_filter.h

@@ -43,6 +43,14 @@ class RoleBasedAccessControlFilterConfig {
       const std::string& stats_prefix, Stats::Scope& scope);
 
   Filters::Common::RBAC::RoleBasedAccessControlFilterStats& stats() { return stats_; }
+  std::string shadowEffectivePolicyIdField() const {
+    return shadow_rules_stat_prefix_ +
+           Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEffectivePolicyIdField;
+  }
+  std::string shadowEngineResultField() const {
+    return shadow_rules_stat_prefix_ +
+           Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEngineResultField;
+  }
 
   const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl*
   engine(const Router::RouteConstSharedPtr route,
@@ -56,6 +64,7 @@ class RoleBasedAccessControlFilterConfig {
   }
 
   Filters::Common::RBAC::RoleBasedAccessControlFilterStats stats_;
+  const std::string shadow_rules_stat_prefix_;
 
   std::unique_ptr<const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> engine_;
   std::unique_ptr<const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> shadow_engine_;

source/extensions/filters/network/rbac/rbac_filter.cc

@@ -16,6 +16,7 @@ namespace RBACFilter {
 RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig(
     const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope)
     : stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), scope)),
+      shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()),
       engine_(Filters::Common::RBAC::createEngine(proto_config)),
       shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)),
       enforcement_type_(proto_config.enforcement_type()) {}
@@ -86,11 +87,9 @@ void RoleBasedAccessControlFilter::setDynamicMetadata(std::string shadow_engine_
   ProtobufWkt::Struct metrics;
   auto& fields = *metrics.mutable_fields();
   if (!shadow_policy_id.empty()) {
-    *fields[Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEffectivePolicyIdField]
-         .mutable_string_value() = shadow_policy_id;
+    *fields[config_->shadowEffectivePolicyIdField()].mutable_string_value() = shadow_policy_id;
   }
-  *fields[Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEngineResultField]
-       .mutable_string_value() = shadow_engine_result;
+  *fields[config_->shadowEngineResultField()].mutable_string_value() = shadow_engine_result;
   callbacks_->connection().streamInfo().setDynamicMetadata(NetworkFilterNames::get().Rbac, metrics);
 }
 

source/extensions/filters/network/rbac/rbac_filter.h

@@ -31,6 +31,14 @@ class RoleBasedAccessControlFilterConfig {
       const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope);
 
   Filters::Common::RBAC::RoleBasedAccessControlFilterStats& stats() { return stats_; }
+  std::string shadowEffectivePolicyIdField() const {
+    return shadow_rules_stat_prefix_ +
+           Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEffectivePolicyIdField;
+  }
+  std::string shadowEngineResultField() const {
+    return shadow_rules_stat_prefix_ +
+           Filters::Common::RBAC::DynamicMetadataKeysSingleton::get().ShadowEngineResultField;
+  }
 
   const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl*
   engine(Filters::Common::RBAC::EnforcementMode mode) const {
@@ -44,9 +52,10 @@ class RoleBasedAccessControlFilterConfig {
 
 private:
   Filters::Common::RBAC::RoleBasedAccessControlFilterStats stats_;
+  const std::string shadow_rules_stat_prefix_;
 
-  std::unique_ptr<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> engine_;
-  std::unique_ptr<Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> shadow_engine_;
+  std::unique_ptr<const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> engine_;
+  std::unique_ptr<const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl> shadow_engine_;
   const envoy::extensions::filters::network::rbac::v3::RBAC::EnforcementType enforcement_type_;
 };
 

test/extensions/filters/http/rbac/rbac_filter_test.cc

@@ -49,6 +49,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_");
 
     return std::make_shared<RoleBasedAccessControlFilterConfig>(config, "test", store_);
   }
@@ -184,8 +185,8 @@ TEST_F(RoleBasedAccessControlFilterTest, Denied) {
   EXPECT_EQ(1U, config_->stats().shadow_allowed_.value());
 
   auto filter_meta = req_info_.dynamicMetadata().filter_metadata().at(HttpFilterNames::get().Rbac);
-  EXPECT_EQ("allowed", filter_meta.fields().at("shadow_engine_result").string_value());
-  EXPECT_EQ("bar", filter_meta.fields().at("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("prefix_shadow_effective_policy_id").string_value());
   EXPECT_EQ("rbac_access_denied_matched_policy[none]", callbacks_.details());
   checkAccessLogMetadata(LogResult::Undecided);
 }

test/extensions/filters/network/rbac/filter_test.cc

@@ -45,6 +45,7 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test {
       shadow_policy_rules->add_rules()->set_destination_port(456);
       shadow_policy.add_principals()->set_any(true);
       config.mutable_shadow_rules()->set_action(action);
+      config.set_shadow_rules_stat_prefix("prefix_");
       (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy;
     }
 
@@ -189,8 +190,8 @@ TEST_F(RoleBasedAccessControlNetworkFilterTest, Denied) {
 
   auto filter_meta =
       stream_info_.dynamicMetadata().filter_metadata().at(NetworkFilterNames::get().Rbac);
-  EXPECT_EQ("bar", filter_meta.fields().at("shadow_effective_policy_id").string_value());
-  EXPECT_EQ("allowed", filter_meta.fields().at("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("prefix_shadow_engine_result").string_value());
 }
 
 // Log Tests