rbac: refactor matchers from shared-ptr to unique-ptr (#40230)

adisuissa · web-flow · commit eb7e8d7a595c · 2025-07-15T11:34:01.000+08:00
Commit Message: rbac: refactor matchers from shared-ptr to unique-ptr
Additional Description:
This PR replaces the creation interface of the RBAC matchers. Instead of
returning a shared-ptr it now returns a unique-ptr.
There is no reason to keep a shared points, because the matchers are
owned by the filter-config which is already shared.

Risk Level: low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Adi Suissa-Peleg &lt;adip@google.com&gt;
diff --git a/source/extensions/filters/common/rbac/matcher_extension.h b/source/extensions/filters/common/rbac/matcher_extension.h
@@ -21,8 +21,8 @@ class MatcherExtensionFactory : public Envoy::Config::TypedFactory {
    * @param config supplies the matcher configuration
    * @return a new MatcherExtension
    */
-  virtual MatcherConstSharedPtr create(const Protobuf::Message& config,
-                                       ProtobufMessage::ValidationVisitor& validation_visitor) PURE;
+  virtual MatcherConstPtr create(const Protobuf::Message& config,
+                                 ProtobufMessage::ValidationVisitor& validation_visitor) PURE;
 
   // @brief the category of the matcher extension type for factory registration.
   std::string category() const override { return "envoy.rbac.matchers"; }
@@ -35,7 +35,7 @@ class MatcherExtensionFactory : public Envoy::Config::TypedFactory {
 template <typename M, typename P>
 class BaseMatcherExtensionFactory : public Filters::Common::RBAC::MatcherExtensionFactory {
 public:
-  Filters::Common::RBAC::MatcherConstSharedPtr
+  Filters::Common::RBAC::MatcherConstPtr
   create(const Protobuf::Message& config,
          ProtobufMessage::ValidationVisitor& validation_visitor) override {
     const auto& matcher_typed_config =
@@ -44,7 +44,7 @@ class BaseMatcherExtensionFactory : public Filters::Common::RBAC::MatcherExtensi
 
     const auto proto_message = MessageUtil::anyConvert<P>(matcher_typed_config.typed_config());
 
-    return std::make_shared<M>(proto_message);
+    return std::make_unique<M>(proto_message);
   }
 
   ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique<P>(); }
diff --git a/source/extensions/filters/common/rbac/matcher_interface.h b/source/extensions/filters/common/rbac/matcher_interface.h
@@ -12,7 +12,7 @@ namespace Common {
 namespace RBAC {
 
 class Matcher;
-using MatcherConstSharedPtr = std::shared_ptr<const Matcher>;
+using MatcherConstPtr = std::unique_ptr<const Matcher>;
 
 /**
  *  Matchers describe the rules for matching either a permission action or principal.
@@ -34,19 +34,19 @@ class Matcher {
                        const StreamInfo::StreamInfo& info) const PURE;
 
   /**
-   * Creates a shared instance of a matcher based off the rules defined in the Permission config
+   * Creates an instance of a matcher based off the rules defined in the Permission config
    * proto message.
    */
-  static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Permission& permission,
-                                      ProtobufMessage::ValidationVisitor& validation_visitor,
-                                      Server::Configuration::CommonFactoryContext& context);
+  static MatcherConstPtr create(const envoy::config::rbac::v3::Permission& permission,
+                                ProtobufMessage::ValidationVisitor& validation_visitor,
+                                Server::Configuration::CommonFactoryContext& context);
 
   /**
-   * Creates a shared instance of a matcher based off the rules defined in the Principal config
+   * Creates an instance of a matcher based off the rules defined in the Principal config
    * proto message.
    */
-  static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Principal& principal,
-                                      Server::Configuration::CommonFactoryContext& context);
+  static MatcherConstPtr create(const envoy::config::rbac::v3::Principal& principal,
+                                Server::Configuration::CommonFactoryContext& context);
 };
 
 } // namespace RBAC
diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc
@@ -13,46 +13,46 @@ namespace Filters {
 namespace Common {
 namespace RBAC {
 
-MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission& permission,
-                                      ProtobufMessage::ValidationVisitor& validation_visitor,
-                                      Server::Configuration::CommonFactoryContext& context) {
+MatcherConstPtr Matcher::create(const envoy::config::rbac::v3::Permission& permission,
+                                ProtobufMessage::ValidationVisitor& validation_visitor,
+                                Server::Configuration::CommonFactoryContext& context) {
   switch (permission.rule_case()) {
   case envoy::config::rbac::v3::Permission::RuleCase::kAndRules:
-    return std::make_shared<const AndMatcher>(permission.and_rules(), validation_visitor, context);
+    return std::make_unique<const AndMatcher>(permission.and_rules(), validation_visitor, context);
   case envoy::config::rbac::v3::Permission::RuleCase::kOrRules:
-    return std::make_shared<const OrMatcher>(permission.or_rules(), validation_visitor, context);
+    return std::make_unique<const OrMatcher>(permission.or_rules(), validation_visitor, context);
   case envoy::config::rbac::v3::Permission::RuleCase::kHeader:
-    return std::make_shared<const HeaderMatcher>(permission.header(), context);
+    return std::make_unique<const HeaderMatcher>(permission.header(), context);
   case envoy::config::rbac::v3::Permission::RuleCase::kDestinationIp:
-    return std::make_shared<const IPMatcher>(permission.destination_ip(),
+    return std::make_unique<const IPMatcher>(permission.destination_ip(),
                                              IPMatcher::Type::DownstreamLocal);
   case envoy::config::rbac::v3::Permission::RuleCase::kDestinationPort:
-    return std::make_shared<const PortMatcher>(permission.destination_port());
+    return std::make_unique<const PortMatcher>(permission.destination_port());
   case envoy::config::rbac::v3::Permission::RuleCase::kDestinationPortRange:
-    return std::make_shared<const PortRangeMatcher>(permission.destination_port_range());
+    return std::make_unique<const PortRangeMatcher>(permission.destination_port_range());
   case envoy::config::rbac::v3::Permission::RuleCase::kAny:
-    return std::make_shared<const AlwaysMatcher>();
+    return std::make_unique<const AlwaysMatcher>();
   case envoy::config::rbac::v3::Permission::RuleCase::kMetadata:
-    return std::make_shared<const MetadataMatcher>(
+    return std::make_unique<const MetadataMatcher>(
         Matchers::MetadataMatcher(permission.metadata(), context),
         envoy::config::rbac::v3::MetadataSource::DYNAMIC);
   case envoy::config::rbac::v3::Permission::RuleCase::kSourcedMetadata:
-    return std::make_shared<const MetadataMatcher>(
+    return std::make_unique<const MetadataMatcher>(
         Matchers::MetadataMatcher(permission.sourced_metadata().metadata_matcher(), context),
         permission.sourced_metadata().metadata_source());
   case envoy::config::rbac::v3::Permission::RuleCase::kNotRule:
-    return std::make_shared<const NotMatcher>(permission.not_rule(), validation_visitor, context);
+    return std::make_unique<const NotMatcher>(permission.not_rule(), validation_visitor, context);
   case envoy::config::rbac::v3::Permission::RuleCase::kRequestedServerName:
-    return std::make_shared<const RequestedServerNameMatcher>(permission.requested_server_name(),
+    return std::make_unique<const RequestedServerNameMatcher>(permission.requested_server_name(),
                                                               context);
   case envoy::config::rbac::v3::Permission::RuleCase::kUrlPath:
-    return std::make_shared<const PathMatcher>(permission.url_path(), context);
+    return std::make_unique<const PathMatcher>(permission.url_path(), context);
   case envoy::config::rbac::v3::Permission::RuleCase::kUriTemplate: {
     auto& factory =
         Config::Utility::getAndCheckFactory<Router::PathMatcherFactory>(permission.uri_template());
     ProtobufTypes::MessagePtr config = Envoy::Config::Utility::translateAnyToFactoryConfig(
         permission.uri_template().typed_config(), validation_visitor, factory);
-    return std::make_shared<const UriTemplateMatcher>(factory.createPathMatcher(*config));
+    return std::make_unique<const UriTemplateMatcher>(factory.createPathMatcher(*config));
   }
   case envoy::config::rbac::v3::Permission::RuleCase::kMatcher: {
     auto& factory =
@@ -65,42 +65,42 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission&
   PANIC_DUE_TO_CORRUPT_ENUM;
 }
 
-MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal& principal,
-                                      Server::Configuration::CommonFactoryContext& context) {
+MatcherConstPtr Matcher::create(const envoy::config::rbac::v3::Principal& principal,
+                                Server::Configuration::CommonFactoryContext& context) {
   switch (principal.identifier_case()) {
   case envoy::config::rbac::v3::Principal::IdentifierCase::kAndIds:
-    return std::make_shared<const AndMatcher>(principal.and_ids(), context);
+    return std::make_unique<const AndMatcher>(principal.and_ids(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kOrIds:
-    return std::make_shared<const OrMatcher>(principal.or_ids(), context);
+    return std::make_unique<const OrMatcher>(principal.or_ids(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kAuthenticated:
-    return std::make_shared<const AuthenticatedMatcher>(principal.authenticated(), context);
+    return std::make_unique<const AuthenticatedMatcher>(principal.authenticated(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kSourceIp:
-    return std::make_shared<const IPMatcher>(principal.source_ip(),
+    return std::make_unique<const IPMatcher>(principal.source_ip(),
                                              IPMatcher::Type::ConnectionRemote);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kDirectRemoteIp:
-    return std::make_shared<const IPMatcher>(principal.direct_remote_ip(),
+    return std::make_unique<const IPMatcher>(principal.direct_remote_ip(),
                                              IPMatcher::Type::DownstreamDirectRemote);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kRemoteIp:
-    return std::make_shared<const IPMatcher>(principal.remote_ip(),
+    return std::make_unique<const IPMatcher>(principal.remote_ip(),
                                              IPMatcher::Type::DownstreamRemote);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kHeader:
-    return std::make_shared<const HeaderMatcher>(principal.header(), context);
+    return std::make_unique<const HeaderMatcher>(principal.header(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kAny:
-    return std::make_shared<const AlwaysMatcher>();
+    return std::make_unique<const AlwaysMatcher>();
   case envoy::config::rbac::v3::Principal::IdentifierCase::kMetadata:
-    return std::make_shared<const MetadataMatcher>(
+    return std::make_unique<const MetadataMatcher>(
         Matchers::MetadataMatcher(principal.metadata(), context),
         envoy::config::rbac::v3::MetadataSource::DYNAMIC);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kSourcedMetadata:
-    return std::make_shared<const MetadataMatcher>(
+    return std::make_unique<const MetadataMatcher>(
         Matchers::MetadataMatcher(principal.sourced_metadata().metadata_matcher(), context),
         principal.sourced_metadata().metadata_source());
   case envoy::config::rbac::v3::Principal::IdentifierCase::kNotId:
-    return std::make_shared<const NotMatcher>(principal.not_id(), context);
+    return std::make_unique<const NotMatcher>(principal.not_id(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kUrlPath:
-    return std::make_shared<const PathMatcher>(principal.url_path(), context);
+    return std::make_unique<const PathMatcher>(principal.url_path(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kFilterState:
-    return std::make_shared<const FilterStateMatcher>(principal.filter_state(), context);
+    return std::make_unique<const FilterStateMatcher>(principal.filter_state(), context);
   case envoy::config::rbac::v3::Principal::IdentifierCase::kCustom:
     return Config::Utility::getAndCheckFactory<PrincipalExtensionFactory>(principal.custom())
         .create(principal.custom(), context);
diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h
@@ -47,7 +47,7 @@ class AndMatcher : public Matcher {
                const StreamInfo::StreamInfo&) const override;
 
 private:
-  std::vector<MatcherConstSharedPtr> matchers_;
+  std::vector<MatcherConstPtr> matchers_;
 };
 
 /**
@@ -73,7 +73,7 @@ class OrMatcher : public Matcher {
                const StreamInfo::StreamInfo&) const override;
 
 private:
-  std::vector<MatcherConstSharedPtr> matchers_;
+  std::vector<MatcherConstPtr> matchers_;
 };
 
 class NotMatcher : public Matcher {
@@ -90,7 +90,7 @@ class NotMatcher : public Matcher {
                const StreamInfo::StreamInfo&) const override;
 
 private:
-  MatcherConstSharedPtr matcher_;
+  MatcherConstPtr matcher_;
 };
 
 /**
diff --git a/source/extensions/filters/common/rbac/principal_extension.h b/source/extensions/filters/common/rbac/principal_extension.h
@@ -20,8 +20,8 @@ class PrincipalExtensionFactory : public Envoy::Config::TypedFactory {
    * @param config supplies the matcher configuration
    * @return a new MatcherExtension
    */
-  virtual MatcherConstSharedPtr create(const envoy::config::core::v3::TypedExtensionConfig& config,
-                                       Server::Configuration::CommonFactoryContext& context) PURE;
+  virtual MatcherConstPtr create(const envoy::config::core::v3::TypedExtensionConfig& config,
+                                 Server::Configuration::CommonFactoryContext& context) PURE;
 
   // @brief the category of the matcher extension type for factory registration.
   std::string category() const override { return "envoy.rbac.principals"; }
@@ -34,13 +34,13 @@ class PrincipalExtensionFactory : public Envoy::Config::TypedFactory {
 template <typename PrincipalType, typename ConfigProto>
 class BasePrincipalExtensionFactory : public PrincipalExtensionFactory {
 public:
-  Filters::Common::RBAC::MatcherConstSharedPtr
+  Filters::Common::RBAC::MatcherConstPtr
   create(const envoy::config::core::v3::TypedExtensionConfig& config,
          Server::Configuration::CommonFactoryContext& context) override {
     ConfigProto typed_config;
     MessageUtil::anyConvertAndValidate(config.typed_config(), typed_config,
                                        context.messageValidationVisitor());
-    return std::make_shared<PrincipalType>(typed_config, context);
+    return std::make_unique<PrincipalType>(typed_config, context);
   }
 
   ProtobufTypes::MessagePtr createEmptyConfigProto() override {
