Matcher + MatcherTree support for a keep_matching field in OnMatch

api/bazel/repository_locations.bzl

@@ -52,9 +52,9 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         project_desc = "xDS API Working Group (xDS-WG)",
         project_url = "https://github.com/cncf/xds",
         # During the UDPA -> xDS migration, we aren't working with releases.
-        version = "b4127c9b8d78b77423fd25169f05b7476b6ea932",
-        sha256 = "aa5f1596bbef3f277dcf4700e4c1097b34301ae66f3b79cd731e3adfbaff2f8f",
-        release_date = "2024-09-05",
+        version = "2ac532fd44436293585084f8d94c6bdb17835af0",
+        sha256 = "790c4c83b6950bb602fec221f6a529d9f368cdc8852aae7d2592d0d04b015f37",
+        release_date = "2025-05-01",
         strip_prefix = "xds-{version}",
         urls = ["https://github.com/cncf/xds/archive/{version}.tar.gz"],
         use_category = ["api"],

api/envoy/config/common/matcher/v3/matcher.proto

@@ -41,6 +41,17 @@ message Matcher {
       // Protocol-specific action to take.
       core.v3.TypedExtensionConfig action = 2;
     }
+
+    // If true, the action will be taken but the caller will behave as if no
+    // match was found. This applies both to actions directly encoded in the
+    // action field and to actions returned from a nested matcher tree in the
+    // matcher field. A subsequent matcher on_no_match action will be used
+    // instead.
+    //
+    // This field is not supported in all contexts in which the matcher API is
+    // used. If this field is set in a context in which it's not supported,
+    // the resource will be rejected.
+    bool keep_matching = 3;
   }
 
   // A linear list of field matchers.

envoy/matcher/matcher.h

@@ -107,6 +107,7 @@ template <class ActionFactoryContext> class ActionFactory : public Config::Typed
 template <class DataType> struct OnMatch {
   const ActionFactoryCb action_cb_;
   const MatchTreeSharedPtr<DataType> matcher_;
+  bool keep_matching_{};
 };
 template <class DataType> using OnMatchFactoryCb = std::function<OnMatch<DataType>()>;
 
@@ -138,6 +139,8 @@ enum class MatchState {
   MatchComplete,
 };
 
+// Callback to execute against skipped matches' actions.
+template <class DataType> using SkippedMatchCb = std::function<void(const OnMatch<DataType>&)>;
 /**
  * MatchTree provides the interface for performing matches against the data provided by DataType.
  */
@@ -160,7 +163,38 @@ template <class DataType> class MatchTree {
   // matching requirements). If the match couldn't be completed, {false, {}} will be returned.
   // If a match result was determined, {true, action} will be returned. If a match result was
   // determined to be no match, {true, {}} will be returned.
-  virtual MatchResult match(const DataType& matching_data) PURE;
+  virtual MatchResult match(const DataType& matching_data,
+                            SkippedMatchCb<DataType> skipped_match_cb = nullptr) PURE;
+
+protected:
+  // Internally handle recursion & keep_matching logic in matcher implementations.
+  // This should be called against initial matching & on-no-match results.
+  static inline MatchResult
+  handleRecursionAndSkips(const absl::optional<OnMatch<DataType>>& on_match, const DataType& data,
+                          SkippedMatchCb<DataType> skipped_match_cb) {
+    if (!on_match.has_value()) {
+      return {MatchState::MatchComplete, absl::nullopt};
+    }
+    if (on_match->matcher_) {
+      auto nested_result = on_match->matcher_->match(data, skipped_match_cb);
+      // Parent result's keep_matching skips the nested result.
+      if (on_match->keep_matching_ && nested_result.match_state_ == MatchState::MatchComplete &&
+          nested_result.on_match_.has_value()) {
+        if (skipped_match_cb) {
+          skipped_match_cb(*nested_result.on_match_);
+        }
+        return {MatchState::MatchComplete, absl::nullopt};
+      }
+      return nested_result;
+    }
+    if (on_match->action_cb_ && on_match->keep_matching_) {
+      if (skipped_match_cb) {
+        skipped_match_cb(*on_match);
+      }
+      return {MatchState::MatchComplete, absl::nullopt};
+    }
+    return {MatchState::MatchComplete, on_match};
+  }
 };
 
 template <class DataType> using MatchTreeSharedPtr = std::shared_ptr<MatchTree<DataType>>;

source/common/matcher/list_matcher.h

@@ -15,21 +15,34 @@ template <class DataType> class ListMatcher : public MatchTree<DataType> {
 public:
   explicit ListMatcher(absl::optional<OnMatch<DataType>> on_no_match) : on_no_match_(on_no_match) {}
 
-  typename MatchTree<DataType>::MatchResult match(const DataType& matching_data) override {
+  using MatchResult = typename MatchTree<DataType>::MatchResult;
+
+  typename MatchTree<DataType>::MatchResult
+  match(const DataType& matching_data,
+        SkippedMatchCb<DataType> skipped_match_cb = nullptr) override {
     for (const auto& matcher : matchers_) {
-      const auto maybe_match = matcher.first->match(matching_data);
+      FieldMatchResult result = matcher.first->match(matching_data);
 
       // One of the matchers don't have enough information, bail on evaluating the match.
-      if (maybe_match.match_state_ == MatchState::UnableToMatch) {
-        return {MatchState::UnableToMatch, {}};
+      if (result.match_state_ == MatchState::UnableToMatch) {
+        return {MatchState::UnableToMatch, absl::nullopt};
+      }
+      if (!result.result()) {
+        continue;
       }
 
-      if (maybe_match.result()) {
-        return {MatchState::MatchComplete, matcher.second};
+      MatchResult processed_result = MatchTree<DataType>::handleRecursionAndSkips(
+          matcher.second, matching_data, skipped_match_cb);
+      // Continue to next matcher if the result is a no-match or is skipped.
+      if (processed_result.match_state_ != MatchState::MatchComplete ||
+          processed_result.on_match_.has_value()) {
+        return processed_result;
       }
     }
 
-    return {MatchState::MatchComplete, on_no_match_};
+    // Return on-no-match, after keep_matching and/or recursion handling.
+    return MatchTree<DataType>::handleRecursionAndSkips(on_no_match_, matching_data,
+                                                        skipped_match_cb);
   }
 
   void addMatcher(FieldMatcherPtr<DataType>&& matcher, OnMatch<DataType> action) {

source/common/matcher/map_matcher.h

@@ -19,7 +19,8 @@ class MapMatcher : public MatchTree<DataType>, Logger::Loggable<Logger::Id::matc
   // Adds a child to the map.
   virtual void addChild(std::string value, OnMatch<DataType>&& on_match) PURE;
 
-  typename MatchTree<DataType>::MatchResult match(const DataType& data) override {
+  typename MatchTree<DataType>::MatchResult
+  match(const DataType& data, SkippedMatchCb<DataType> skipped_match_cb = nullptr) override {
     const auto input = data_input_->get(data);
     ENVOY_LOG(trace, "Attempting to match {}", input);
     if (input.data_availability_ == DataInputGetResult::DataAvailability::NotAvailable) {
@@ -28,27 +29,33 @@ class MapMatcher : public MatchTree<DataType>, Logger::Loggable<Logger::Id::matc
 
     // Returns `on_no_match` when input data is empty. (i.e., is absl::monostate).
     if (absl::holds_alternative<absl::monostate>(input.data_)) {
-      return {MatchState::MatchComplete, on_no_match_};
+      return MatchTree<DataType>::handleRecursionAndSkips(on_no_match_, data, skipped_match_cb);
     }
 
-    const auto result = doMatch(absl::get<std::string>(input.data_));
-    if (result) {
-      if (result->matcher_) {
-        return result->matcher_->match(data);
-      } else {
-        return {MatchState::MatchComplete, OnMatch<DataType>{result->action_cb_, nullptr}};
+    const absl::optional<OnMatch<DataType>> result = doMatch(absl::get<std::string>(input.data_));
+    // No match.
+    if (!result.has_value()) {
+      // Match failed.
+      if (input.data_availability_ ==
+          DataInputGetResult::DataAvailability::MoreDataMightBeAvailable) {
+        return {MatchState::UnableToMatch, absl::nullopt};
       }
-    } else if (input.data_availability_ ==
-               DataInputGetResult::DataAvailability::MoreDataMightBeAvailable) {
-      // It's possible that we were attempting a lookup with a partial value, so delay matching
-      // until we know that we actually failed.
-      return {MatchState::UnableToMatch, absl::nullopt};
+      // No-match, return on_no_match after keep_matching and/or recursion handling.
+      return MatchTree<DataType>::handleRecursionAndSkips(on_no_match_, data, skipped_match_cb);
     }
 
-    return {MatchState::MatchComplete, on_no_match_};
+    // Handle recursion and keep_matching.
+    auto processed_result =
+        MatchTree<DataType>::handleRecursionAndSkips(result, data, skipped_match_cb);
+    // Matched or failed nested matching.
+    if (processed_result.match_state_ != MatchState::MatchComplete ||
+        processed_result.on_match_.has_value()) {
+      return processed_result;
+    }
+    // No-match, return on_no_match after keep_matching and/or recursion handling.
+    return MatchTree<DataType>::handleRecursionAndSkips(on_no_match_, data, skipped_match_cb);
   }
 
-protected:
   template <class DataType2, class ActionFactoryContext> friend class MatchTreeFactory;
   MapMatcher(DataInputPtr<DataType>&& data_input, absl::optional<OnMatch<DataType>> on_no_match,
              absl::Status& creation_status)

source/common/matcher/matcher.h

@@ -43,22 +43,20 @@ struct MaybeMatchResult {
 
 // TODO(snowp): Make this a class that tracks the progress to speed up subsequent traversals.
 template <class DataType>
-static inline MaybeMatchResult evaluateMatch(MatchTree<DataType>& match_tree,
-                                             const DataType& data) {
-  const auto result = match_tree.match(data);
+static inline MaybeMatchResult evaluateMatch(MatchTree<DataType>& match_tree, const DataType& data,
+                                             SkippedMatchCb<DataType> skipped_match_cb = nullptr) {
+  const auto result = match_tree.match(data, skipped_match_cb);
   if (result.match_state_ == MatchState::UnableToMatch) {
-    return MaybeMatchResult{nullptr, MatchState::UnableToMatch};
+    return {nullptr, MatchState::UnableToMatch};
   }
 
   if (!result.on_match_) {
     return {nullptr, MatchState::MatchComplete};
   }
 
-  if (result.on_match_->matcher_) {
-    return evaluateMatch(*result.on_match_->matcher_, data);
-  }
-
-  return MaybeMatchResult{result.on_match_->action_cb_, MatchState::MatchComplete};
+  // Note: does not handle sub-matchers or keep_matching, MatchTree::match(...) implementations are
+  // expected to do so.
+  return {result.on_match_->action_cb_, MatchState::MatchComplete};
 }
 
 template <class DataType> using FieldMatcherFactoryCb = std::function<FieldMatcherPtr<DataType>()>;
@@ -73,8 +71,9 @@ template <class DataType> class AnyMatcher : public MatchTree<DataType> {
   explicit AnyMatcher(absl::optional<OnMatch<DataType>> on_no_match)
       : on_no_match_(std::move(on_no_match)) {}
 
-  typename MatchTree<DataType>::MatchResult match(const DataType&) override {
-    return {MatchState::MatchComplete, on_no_match_};
+  typename MatchTree<DataType>::MatchResult
+  match(const DataType& data, SkippedMatchCb<DataType> skipped_match_cb = nullptr) override {
+    return MatchTree<DataType>::handleRecursionAndSkips(on_no_match_, data, skipped_match_cb);
   }
   const absl::optional<OnMatch<DataType>> on_no_match_;
 };
@@ -153,6 +152,7 @@ class MatchTreeFactory : public OnMatchFactory<DataType> {
                    Server::Configuration::ServerFactoryContext& factory_context,
                    MatchTreeValidationVisitor<DataType>& validation_visitor)
       : action_factory_context_(context), server_factory_context_(factory_context),
+        on_match_validation_visitor_(validation_visitor),
         match_input_factory_(factory_context.messageValidationVisitor(), validation_visitor) {}
 
   // TODO(snowp): Remove this type parameter once we only have one Matcher proto.
@@ -324,9 +324,15 @@ class MatchTreeFactory : public OnMatchFactory<DataType> {
 
   template <class OnMatchType>
   absl::optional<OnMatchFactoryCb<DataType>> createOnMatchBase(const OnMatchType& on_match) {
+    on_match_validation_visitor_.validateOnMatch(on_match);
+    if (const std::vector<absl::Status>& errors = on_match_validation_visitor_.errors();
+        !errors.empty()) {
+      return []() -> OnMatch<DataType> { return OnMatch<DataType>{}; };
+    }
     if (on_match.has_matcher()) {
-      return [matcher_factory = std::move(create(on_match.matcher()))]() {
-        return OnMatch<DataType>{{}, matcher_factory()};
+      return [matcher_factory = std::move(create(on_match.matcher())),
+              keep_matching = on_match.keep_matching()]() {
+        return OnMatch<DataType>{{}, matcher_factory(), keep_matching};
       };
     } else if (on_match.has_action()) {
       auto& factory = Config::Utility::getAndCheckFactory<ActionFactory<ActionFactoryContext>>(
@@ -337,7 +343,9 @@ class MatchTreeFactory : public OnMatchFactory<DataType> {
 
       auto action_factory = factory.createActionFactoryCb(
           *message, action_factory_context_, server_factory_context_.messageValidationVisitor());
-      return [action_factory] { return OnMatch<DataType>{action_factory, {}}; };
+      return [action_factory, keep_matching = on_match.keep_matching()] {
+        return OnMatch<DataType>{action_factory, {}, keep_matching};
+      };
     }
 
     return absl::nullopt;
@@ -367,6 +375,7 @@ class MatchTreeFactory : public OnMatchFactory<DataType> {
   const std::string stats_prefix_;
   ActionFactoryContext& action_factory_context_;
   Server::Configuration::ServerFactoryContext& server_factory_context_;
+  MatchTreeValidationVisitor<DataType>& on_match_validation_visitor_;
   MatchInputFactory<DataType> match_input_factory_;
 };
 } // namespace Matcher

source/common/matcher/prefix_map_matcher.h

@@ -30,6 +30,8 @@ template <class DataType> class PrefixMapMatcher : public MapMatcher<DataType> {
                    absl::optional<OnMatch<DataType>> on_no_match, absl::Status& creation_status)
       : MapMatcher<DataType>(std::move(data_input), std::move(on_no_match), creation_status) {}
 
+  // TODO(ravenblackx): Implement a Reentrant. Re-entry gives access to the
+  // shorter, matching prefixes for callers.
   absl::optional<OnMatch<DataType>> doMatch(const std::string& data) override {
     const auto result = children_.findLongestPrefix(data);
     if (result) {

source/common/matcher/validation_visitor.h

@@ -31,6 +31,17 @@ template <class DataType> class MatchTreeValidationVisitor {
 
   const std::vector<absl::Status>& errors() const { return errors_; }
 
+  template <class OnMatchType> void validateOnMatch(const OnMatchType& on_match) {
+    if (!support_keep_matching_ && on_match.keep_matching()) {
+      errors_.emplace_back(
+          absl::InvalidArgumentError("keep_matching is not supported in this context"));
+    }
+  }
+
+  void setSupportKeepMatching(bool support_keep_matching) {
+    support_keep_matching_ = support_keep_matching;
+  }
+
 protected:
   // Implementations would subclass this to specify the validation logic for data inputs,
   // returning a helpful error message if validation fails.
@@ -39,6 +50,7 @@ template <class DataType> class MatchTreeValidationVisitor {
 
 private:
   std::vector<absl::Status> errors_;
+  bool support_keep_matching_ = false;
 };
 } // namespace Matcher
 } // namespace Envoy

source/extensions/common/matcher/trie_matcher.h

@@ -19,12 +19,12 @@ namespace Matcher {
 using ::Envoy::Matcher::DataInputFactoryCb;
 using ::Envoy::Matcher::DataInputGetResult;
 using ::Envoy::Matcher::DataInputPtr;
-using ::Envoy::Matcher::evaluateMatch;
 using ::Envoy::Matcher::MatchState;
 using ::Envoy::Matcher::MatchTree;
 using ::Envoy::Matcher::OnMatch;
 using ::Envoy::Matcher::OnMatchFactory;
 using ::Envoy::Matcher::OnMatchFactoryCb;
+using ::Envoy::Matcher::SkippedMatchCb;
 
 template <class DataType> struct TrieNode {
   size_t index_;
@@ -71,7 +71,8 @@ template <class DataType> class TrieMatcher : public MatchTree<DataType> {
     }
   }
 
-  typename MatchTree<DataType>::MatchResult match(const DataType& data) override {
+  typename MatchTree<DataType>::MatchResult
+  match(const DataType& data, SkippedMatchCb<DataType> skipped_match_cb = nullptr) override {
     const auto input = data_input_->get(data);
     if (input.data_availability_ != DataInputGetResult::DataAvailability::AllDataAvailable) {
       return {MatchState::UnableToMatch, absl::nullopt};
@@ -93,22 +94,20 @@ template <class DataType> class TrieMatcher : public MatchTree<DataType> {
       if (!first && node.exclusive_) {
         continue;
       }
-      if (node.on_match_->action_cb_) {
-        return {MatchState::MatchComplete, OnMatch<DataType>{node.on_match_->action_cb_, nullptr}};
-      }
-      // Resume any subtree matching to preserve backtracking progress.
-      auto matched = evaluateMatch(*node.on_match_->matcher_, data);
-      if (matched.match_state_ == MatchState::UnableToMatch) {
-        return {MatchState::UnableToMatch, absl::nullopt};
-      }
-      if (matched.match_state_ == MatchState::MatchComplete && matched.result_) {
-        return {MatchState::MatchComplete, OnMatch<DataType>{matched.result_, nullptr}};
+      // handleRecursionAndSkips should only return match-failure, no-match, or an action cb.
+      typename MatchTree<DataType>::MatchResult processed_match =
+          MatchTree<DataType>::handleRecursionAndSkips(*node.on_match_, data, skipped_match_cb);
+
+      if (processed_match.match_state_ != MatchState::MatchComplete ||
+          processed_match.on_match_.has_value()) {
+        return processed_match;
       }
+      // No-match isn't definitive, so continue checking nodes.
       if (first) {
         first = false;
       }
     }
-    return {MatchState::MatchComplete, on_no_match_};
+    return MatchTree<DataType>::handleRecursionAndSkips(on_no_match_, data, skipped_match_cb);
   }
 
 private: