[router] Warn on unknown retry_on policy tokens

source/common/router/retry_policy_impl.cc

@@ -2,11 +2,14 @@
 
 #include <memory>
 
+#include "source/common/common/logger.h"
 #include "source/common/config/utility.h"
 #include "source/common/router/reset_header_parser.h"
 #include "source/common/router/retry_state_impl.h"
 #include "source/common/upstream/retry_factory.h"
 
+#include "absl/strings/str_join.h"
+
 namespace Envoy {
 namespace Router {
 
@@ -40,6 +43,13 @@ RetryPolicyImpl::RetryPolicyImpl(const envoy::config::route::v3::RetryPolicy& re
   num_retries_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(retry_policy, num_retries, 1);
   retry_on_ = RetryStateImpl::parseRetryOn(retry_policy.retry_on()).first;
   retry_on_ |= RetryStateImpl::parseRetryGrpcOn(retry_policy.retry_on()).first;
+  const auto unknown_tokens = RetryStateImpl::getUnknownRetryOnTokens(retry_policy.retry_on());
+  if (!unknown_tokens.empty()) {
+    ENVOY_LOG_EVERY_POW_2_MISC(warn,
+                               "Unknown retry_on policy token(s) [{}] in retry_on config '{}'. "
+                               "These tokens will be silently ignored.",
+                               absl::StrJoin(unknown_tokens, ", "), retry_policy.retry_on());
+  }
 
   for (const auto& host_predicate : retry_policy.retry_host_predicate()) {
     auto& factory = Envoy::Config::Utility::getAndCheckFactory<Upstream::RetryHostPredicateFactory>(

source/common/router/retry_state_impl.cc

@@ -237,6 +237,16 @@ std::pair<uint32_t, bool> RetryStateImpl::parseRetryGrpcOn(absl::string_view ret
   return {ret, all_fields_valid};
 }
 
+std::vector<std::string> RetryStateImpl::getUnknownRetryOnTokens(absl::string_view config) {
+  std::vector<std::string> unknown;
+  for (const auto& token : StringUtil::splitToken(config, ",", false, true)) {
+    if (!parseRetryOn(token).second && !parseRetryGrpcOn(token).second) {
+      unknown.emplace_back(token);
+    }
+  }
+  return unknown;
+}
+
 absl::optional<std::chrono::milliseconds>
 RetryStateImpl::parseResetInterval(const Http::ResponseHeaderMap& response_headers) const {
   for (const auto& reset_header : reset_headers_) {

source/common/router/retry_state_impl.h

@@ -2,6 +2,7 @@
 
 #include <cstdint>
 #include <string>
+#include <vector>
 
 #include "envoy/common/random_generator.h"
 #include "envoy/event/timer.h"
@@ -51,6 +52,14 @@ class RetryStateImpl : public RetryState {
    */
   static std::pair<uint32_t, bool> parseRetryGrpcOn(absl::string_view retry_grpc_on_header);
 
+  /**
+   * Returns any tokens from @param config that are not recognized by either
+   * the HTTP or gRPC retry policy parsers.
+   * @param config is the comma-separated retry_on field value.
+   * @return vector of unrecognized token strings.
+   */
+  static std::vector<std::string> getUnknownRetryOnTokens(absl::string_view config);
+
   // Router::RetryState
   bool enabled() override { return retry_on_ != 0; }
   absl::optional<std::chrono::milliseconds>

test/common/router/retry_state_impl_test.cc

@@ -1445,6 +1445,35 @@ TEST_F(RouterRetryStateImplTest, ParseRetryGrpcOn) {
   EXPECT_FALSE(result.second);
 }
 
+TEST_F(RouterRetryStateImplTest, GetUnknownRetryOnTokens) {
+  // All valid HTTP tokens: no unknowns.
+  EXPECT_TRUE(RetryStateImpl::getUnknownRetryOnTokens("5xx,gateway-error,connect-failure").empty());
+
+  // All valid gRPC tokens: no unknowns.
+  EXPECT_TRUE(RetryStateImpl::getUnknownRetryOnTokens("cancelled,deadline-exceeded").empty());
+
+  // Mixed valid HTTP and gRPC tokens: no unknowns.
+  EXPECT_TRUE(RetryStateImpl::getUnknownRetryOnTokens("5xx,cancelled").empty());
+
+  // One unknown token among valid ones.
+  auto unknown = RetryStateImpl::getUnknownRetryOnTokens("5xx,typo,connect-failure");
+  EXPECT_EQ(unknown.size(), 1);
+  EXPECT_EQ(unknown[0], "typo");
+
+  // Multiple unknown tokens.
+  unknown = RetryStateImpl::getUnknownRetryOnTokens("bad1,5xx,bad2");
+  EXPECT_EQ(unknown.size(), 2);
+  EXPECT_EQ(unknown[0], "bad1");
+  EXPECT_EQ(unknown[1], "bad2");
+
+  // All unknown tokens.
+  unknown = RetryStateImpl::getUnknownRetryOnTokens("foo,bar");
+  EXPECT_EQ(unknown.size(), 2);
+
+  // Empty config: no unknowns.
+  EXPECT_TRUE(RetryStateImpl::getUnknownRetryOnTokens("").empty());
+}
+
 TEST_F(RouterRetryStateImplTest, RemoveAllRetryHeaders) {
   // Make sure retry related headers are removed when the policy is enabled.
   {