Merge branch 'main' into dns_details

Signed-off-by: Fredy Wijaya <[email protected]>

source/common/config/utility.cc

@@ -346,5 +346,28 @@ Utility::buildJitteredExponentialBackOffStrategy(
       default_base_interval_ms, default_base_interval_ms * 10, random);
 }
 
+absl::Status Utility::validateTerminalFilters(const std::string& name,
+                                              const std::string& filter_type,
+                                              const std::string& filter_chain_type,
+                                              bool is_terminal_filter,
+                                              bool last_filter_in_current_config) {
+  if (is_terminal_filter && !last_filter_in_current_config) {
+    return absl::InvalidArgumentError(
+        fmt::format("Error: terminal filter named {} of type {} must be the "
+                    "last filter in a {} filter chain.",
+                    name, filter_type, filter_chain_type));
+  } else if (!is_terminal_filter && last_filter_in_current_config) {
+    absl::string_view extra = "";
+    if (filter_chain_type == "router upstream http") {
+      extra = " When upstream_http_filters are specified, they must explicitly end with an "
+              "UpstreamCodec filter.";
+    }
+    return absl::InvalidArgumentError(fmt::format("Error: non-terminal filter named {} of type "
+                                                  "{} is the last filter in a {} filter chain.{}",
+                                                  name, filter_type, filter_chain_type, extra));
+  }
+  return absl::OkStatus();
+}
+
 } // namespace Config
 } // namespace Envoy

source/common/config/utility.h

@@ -424,19 +424,7 @@ class Utility {
                                               const std::string& filter_type,
                                               const std::string& filter_chain_type,
                                               bool is_terminal_filter,
-                                              bool last_filter_in_current_config) {
-    if (is_terminal_filter && !last_filter_in_current_config) {
-      return absl::InvalidArgumentError(
-          fmt::format("Error: terminal filter named {} of type {} must be the "
-                      "last filter in a {} filter chain.",
-                      name, filter_type, filter_chain_type));
-    } else if (!is_terminal_filter && last_filter_in_current_config) {
-      return absl::InvalidArgumentError(fmt::format(
-          "Error: non-terminal filter named {} of type {} is the last filter in a {} filter chain.",
-          name, filter_type, filter_chain_type));
-    }
-    return absl::OkStatus();
-  }
+                                              bool last_filter_in_current_config);
 
   /**
    * Prepares the DNS failure refresh backoff strategy given the cluster configuration.

source/common/quic/envoy_quic_client_session.h

@@ -75,8 +75,7 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl,
 
   // PacketsToReadDelegate
   size_t numPacketsExpectedPerEventLoop() const override {
-    // Do one round of reading per active stream, or to see if there's a new
-    // active stream.
+    // Do one round of reading per active stream, or to see if there's a new active stream.
     return std::max<size_t>(1, GetNumActiveStreams()) * Network::NUM_DATAGRAMS_PER_RECEIVE;
   }
 

source/common/router/config_impl.h

@@ -478,7 +478,6 @@ class RetryPolicyImpl : public RetryPolicy {
   // that should be used when with this policy.
   std::vector<std::pair<Upstream::RetryHostPredicateFactory&, ProtobufTypes::MessagePtr>>
       retry_host_predicate_configs_;
-  Upstream::RetryPrioritySharedPtr retry_priority_;
   // Name and config proto to use to create the RetryPriority to use with this policy. Default
   // initialized when no RetryPriority should be used.
   std::pair<Upstream::RetryPriorityFactory*, ProtobufTypes::MessagePtr> retry_priority_config_;

test/common/config/BUILD

@@ -144,6 +144,7 @@ envoy_cc_test(
         "//test/mocks/upstream:thread_local_cluster_mocks",
         "//test/test_common:environment_lib",
         "//test/test_common:logging_lib",
+        "//test/test_common:status_utility_lib",
         "//test/test_common:test_runtime_lib",
         "//test/test_common:utility_lib",
         "@com_github_cncf_xds//udpa/type/v1:pkg_cc_proto",

test/common/config/utility_test.cc

@@ -20,6 +20,7 @@
 #include "test/mocks/upstream/thread_local_cluster.h"
 #include "test/test_common/environment.h"
 #include "test/test_common/logging.h"
+#include "test/test_common/status_utility.h"
 #include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
@@ -29,6 +30,7 @@
 #include "udpa/type/v1/typed_struct.pb.h"
 #include "xds/type/v3/typed_struct.pb.h"
 
+using Envoy::StatusHelpers::HasStatusMessage;
 using testing::ContainsRegex;
 using testing::Eq;
 using testing::HasSubstr;
@@ -894,6 +896,43 @@ TEST(UtilityTest, GetGrpcControlPlane) {
   }
 }
 
+TEST(UtilityTest, ValidateTerminalFiltersSucceeds) {
+  EXPECT_OK(Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type",
+                                             /*is_terminal_filter=*/true,
+                                             /*last_filter_in_current_config=*/true));
+  EXPECT_OK(Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type",
+                                             /*is_terminal_filter=*/false,
+                                             /*last_filter_in_current_config=*/false));
+}
+
+TEST(UtilityTest, ValidateTerminalFilterFailsWithMisplacedTerminalFilter) {
+  EXPECT_THAT(
+      Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type",
+                                       /*is_terminal_filter=*/true,
+                                       /*last_filter_in_current_config=*/false),
+      HasStatusMessage("Error: terminal filter named filter_name of type filter_type must be the "
+                       "last filter in a chain_type filter chain."));
+}
+
+TEST(UtilityTest, ValidateTerminalFilterFailsWithMissingTerminalFilter) {
+  EXPECT_THAT(Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type",
+                                               /*is_terminal_filter=*/false,
+                                               /*last_filter_in_current_config=*/true),
+              HasStatusMessage("Error: non-terminal filter named filter_name of type "
+                               "filter_type is the last filter in a chain_type filter chain."));
+}
+
+TEST(UtilityTest, ValidateTerminalFilterFailsWithMissingUpstreamTerminalFilter) {
+  EXPECT_THAT(
+      Utility::validateTerminalFilters("filter_name", "filter_type", "router upstream http",
+                                       /*is_terminal_filter=*/false,
+                                       /*last_filter_in_current_config=*/true),
+      HasStatusMessage("Error: non-terminal filter named filter_name of type "
+                       "filter_type is the last filter in a router upstream http filter chain. "
+                       "When upstream_http_filters are specified, they must explicitly end with an "
+                       "UpstreamCodec filter."));
+}
+
 } // namespace
 } // namespace Config
 } // namespace Envoy

test/common/quic/envoy_quic_client_stream_test.cc

@@ -210,6 +210,8 @@ TEST_F(EnvoyQuicClientStreamTest, GetRequestAndHeaderOnlyResponse) {
   const auto result = quic_stream_->encodeHeaders(request_headers_, /*end_stream=*/true);
   EXPECT_TRUE(result.ok());
 
+  quic_stream_->setFlushTimeout(std::chrono::milliseconds(100)); // No-op
+
   EXPECT_CALL(stream_decoder_, decodeHeaders_(_, /*end_stream=*/false))
       .WillOnce(Invoke([](const Http::ResponseHeaderMapPtr& headers, bool) {
         EXPECT_EQ("200", headers->getStatusValue());