Almost complete rewrite of cache filter

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

@@ -20,7 +20,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: HTTP Cache Filter]
 
 // [#extension: envoy.filters.http.cache]
-// [#next-free-field: 7]
+// [#next-free-field: 8]
 message CacheConfig {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.http.cache.v2alpha.CacheConfig";
@@ -93,4 +93,16 @@ message CacheConfig {
   // causes the cache to validate with its upstream even if the lookup is a hit. Setting this
   // to true will ignore these headers.
   bool ignore_request_cache_control_header = 6;
+
+  // If this is set, requests sent upstream to populate the cache will go to the
+  // specified cluster rather than the cluster selected by the vhost and route.
+  //
+  // If you have actions to be taken by the router filter - either
+  // ``upstream_http_filters`` or one of the ``RouteConfiguration`` actions such as
+  // ``response_headers_to_add`` - then the cache's side-channel going directly to the
+  // routed cluster will bypass these actions. You can set ``override_upstream_cluster``
+  // to an internal listener which duplicates the relevant ``RouteConfiguration``, to
+  // replicate the desired behavior on the side-channel upstream request issued by the
+  // cache.
+  string override_upstream_cluster = 7;
 }

changelogs/current.yaml

@@ -2,6 +2,23 @@ date: Pending
 
 behavior_changes:
 # *Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required*
+- area: cache_filter
+  change: |
+    CacheFilter (WIP) has been completely reworked. Any existing cache implementations
+    will need to be modified to fit the new API. The new cache API is much simpler, as
+    individual cache implementations no longer need to comprehend various http headers,
+    only read and write at keys. Cache filter now handles "thundering herds" - if
+    multiple requests for the same resource arrive before the cache is populated,
+    now only one request goes upstream, and there is only one insert to the cache.
+    Range requests now convert to an upstream request for the entire resource, to
+    populate the cache.
+    Surprising behavior change may result if there are any active filters upstream
+    of a CacheFilter, including if RouteConfiguration does any actions
+    (e.g. adding headers) - it is recommended that for anything other than the most
+    simplistic configuration (for which the CacheFilter should be the furthest
+    upstream filter), a CacheFilter should be configured to make its requests
+    to an InternalListener which duplicates the RouteConfiguration and any filter
+    chain upstream of the CacheFilter. This is recognized as far from ideal.
 
 minor_behavior_changes:
 # *Changes that may cause incompatibilities for some users, but should not for most*

docs/root/configuration/http/http_filters/_include/http-cache-configuration-internal-listener.yaml

@@ -0,0 +1,113 @@
+bootstrap_extensions:
+- name: envoy.bootstrap.internal_listener
+  typed_config:
+    "@type": type.googleapis.com/envoy.extensions.bootstrap.internal_listener.v3.InternalListener
+static_resources:
+  listeners:
+  - address:
+      socket_address:
+        address: 0.0.0.0
+        port_value: 8000
+    filter_chains:
+    - filters:
+      - name: envoy.filters.network.http_connection_manager
+        typed_config:
+          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+          codec_type: AUTO
+          stat_prefix: ingress_http
+          route_config:
+            name: local_route
+            virtual_hosts:
+            - name: backend
+              response_headers_to_add:
+              - header:
+                  key: x-something
+                  value: something
+              domains:
+              - "*"
+              routes:
+              - match:
+                  prefix: "/service/1"
+                route:
+                  cluster: service1
+              - match:
+                  prefix: "/service/2"
+                route:
+                  cluster: service2
+          http_filters:
+          - name: "envoy.filters.http.cache"
+            typed_config:
+              "@type": "type.googleapis.com/envoy.extensions.filters.http.cache.v3.CacheConfig"
+              override_upstream_cluster: cache_internal_listener_cluster
+              typed_config:
+                "@type": "type.googleapis.com/envoy.extensions.http.cache.simple_http_cache.v3.SimpleHttpCacheConfig"
+          - name: envoy.filters.http.router
+            typed_config:
+              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+  - name: cache_internal_listener
+    internal_listener: {}
+    filter_chains:
+    - filters:
+      - name: envoy.filters.network.http_connection_manager
+        typed_config:
+          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+          codec_type: AUTO
+          stat_prefix: cache_internal_listener
+          route_config:
+            name: local_route
+            virtual_hosts:
+            - name: backend
+              response_headers_to_add:
+              - header:
+                  key: x-something
+                  value: something
+              domains:
+              - "*"
+              routes:
+              - match:
+                  prefix: "/service/1"
+                route:
+                  cluster: service1
+              - match:
+                  prefix: "/service/2"
+                route:
+                  cluster: service2
+          http_filters:
+          - name: envoy.filters.http.router
+            typed_config:
+              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+
+  clusters:
+  - name: service1
+    type: STRICT_DNS
+    lb_policy: ROUND_ROBIN
+    load_assignment:
+      cluster_name: service1
+      endpoints:
+      - lb_endpoints:
+        - endpoint:
+            address:
+              socket_address:
+                address: service1
+                port_value: 8000
+  - name: service2
+    type: STRICT_DNS
+    lb_policy: ROUND_ROBIN
+    load_assignment:
+      cluster_name: service2
+      endpoints:
+      - lb_endpoints:
+        - endpoint:
+            address:
+              socket_address:
+                address: service2
+                port_value: 8000
+  - name: cache_internal_listener_cluster
+    load_assignment:
+      cluster_name: cache_internal_listener_cluster
+      endpoints:
+      - lb_endpoints:
+        - endpoint:
+            address:
+              envoy_internal_address:
+                server_listener_name: cache_internal_listener

docs/root/configuration/http/http_filters/cache_filter.rst

@@ -13,12 +13,25 @@ Cache filter
   upstream than the cache filter, while non-cacheable requests still go through the
   listener filter chain. It is therefore recommended for consistency that only the
   router filter should be further upstream in the listener filter chain than the
-  cache filter.
+  cache filter, and even then only if the router filter does not perform any mutations
+  such as if ``request_headers_to_add`` is set.
 
 .. image:: /_static/cache-filter-chain.svg
    :width: 80%
    :align: center
 
+* For more complex filter chains where some filters must be upstream of the cache
+  filter for correct behavior, or if the router filter is configured to perform
+  mutations via
+  :ref:`RouteConfiguration <envoy_v3_api_field_config.route.v3.RouteConfiguration.request_headers_to_add>`
+  the recommended way to configure this so that it works correctly is to configure
+  an internal listener which duplicates the part of the filter chain that is
+  upstream of the cache filter, and the ``RouteConfiguration``.
+
+.. image:: /_static/cache-filter-internal-listener.svg
+   :width: 80%
+   :align: center
+
 The HTTP Cache filter implements most of the complexity of HTTP caching semantics.
 
 For HTTP Requests:
@@ -50,6 +63,16 @@ Example filter configuration with a ``SimpleHttpCache`` cache implementation:
    :lineno-start: 29
    :caption: :download:`http-cache-configuration.yaml <_include/http-cache-configuration.yaml>`
 
+The more complicated filter chain configuration required if mutations occur upstream of the cache filter
+involves duplicating the full route config into an internal listener (unfortunately this is currently unavoidable):
+
+.. literalinclude:: _include/http-cache-configuration-internal-listener.yaml
+   :language: yaml
+   :lines: 38-113
+   :linenos:
+   :lineno-start: 38
+   :caption: :download:`http-cache-configuration-internal-listener.yaml <_include/http-cache-configuration-internal-listener.yaml>`
+
 .. seealso::
 
    :ref:`Envoy Cache Sandbox <install_sandboxes_cache_filter>`

source/extensions/filters/http/cache/BUILD

@@ -12,25 +12,87 @@ licenses(["notice"])  # Apache 2
 
 envoy_extension_package()
 
+envoy_cc_library(
+    name = "http_source_interface",
+    hdrs = ["http_source.h"],
+    deps = [
+        "//envoy/buffer:buffer_interface",
+        "//envoy/http:header_map_interface",
+        "//source/extensions/filters/http/cache:range_utils_lib",
+        "@com_google_absl//absl/functional:any_invocable",
+    ],
+)
+
+envoy_cc_library(
+    name = "upstream_request_lib",
+    srcs = ["upstream_request_impl.cc"],
+    hdrs = [
+        "upstream_request.h",
+        "upstream_request_impl.h",
+    ],
+    deps = [
+        ":http_source_interface",
+        ":range_utils_lib",
+        ":stats",
+        "//source/common/buffer:watermark_buffer_lib",
+        "//source/common/common:cancel_wrapper_lib",
+        "//source/common/common:logger_lib",
+        "@com_google_absl//absl/types:variant",
+    ],
+)
+
+envoy_cc_library(
+    name = "cache_sessions_lib",
+    srcs = [
+        "cache_sessions.cc",
+    ],
+    hdrs = [
+        "cache_sessions.h",
+    ],
+    deps = [
+        ":http_cache_lib",
+        ":stats",
+        ":upstream_request_lib",
+        "//source/common/http:utility_lib",
+    ],
+)
+
+envoy_cc_library(
+    name = "cache_sessions_impl_lib",
+    srcs = [
+        "cache_sessions_impl.cc",
+    ],
+    hdrs = [
+        "cache_sessions_impl.h",
+    ],
+    deps = [
+        ":cache_sessions_lib",
+        ":cacheability_utils_lib",
+        ":upstream_request_lib",
+        "//source/common/common:cancel_wrapper_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "cache_filter_lib",
     srcs = [
         "cache_filter.cc",
-        "upstream_request.cc",
     ],
     hdrs = [
         "cache_filter.h",
-        "filter_state.h",
-        "upstream_request.h",
     ],
     deps = [
         ":cache_custom_headers",
         ":cache_entry_utils_lib",
-        ":cache_filter_logging_info_lib",
         ":cache_headers_utils_lib",
-        ":cache_insert_queue_lib",
+        ":cache_sessions_impl_lib",
+        ":cache_sessions_lib",
         ":cacheability_utils_lib",
         ":http_cache_lib",
+        ":stats",
+        ":upstream_request_lib",
+        "//source/common/buffer:buffer_lib",
+        "//source/common/common:cancel_wrapper_lib",
         "//source/common/common:enum_to_int",
         "//source/common/common:logger_lib",
         "//source/common/common:macros",
@@ -65,24 +127,13 @@ envoy_cc_library(
     ],
 )
 
-envoy_cc_library(
-    name = "cache_insert_queue_lib",
-    srcs = ["cache_insert_queue.cc"],
-    hdrs = ["cache_insert_queue.h"],
-    deps = [
-        ":http_cache_lib",
-        "//source/common/buffer:buffer_lib",
-    ],
-)
-
 envoy_cc_library(
     name = "cache_policy_lib",
     hdrs = ["cache_policy.h"],
     deps = [
         ":cache_headers_utils_lib",
         ":http_cache_lib",
         "//source/common/http:header_map_lib",
-        "//source/common/stream_info:filter_state_lib",
     ],
 )
 
@@ -91,6 +142,15 @@ envoy_proto_library(
     srcs = ["key.proto"],
 )
 
+envoy_cc_library(
+    name = "cache_progress_receiver_interface",
+    hdrs = ["cache_progress_receiver.h"],
+    deps = [
+        "//envoy/http:header_map_interface",
+        "//source/extensions/filters/http/cache:range_utils_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "http_cache_lib",
     srcs = ["http_cache.cc"],
@@ -99,6 +159,8 @@ envoy_cc_library(
         ":cache_custom_headers",
         ":cache_entry_utils_lib",
         ":cache_headers_utils_lib",
+        ":cache_progress_receiver_interface",
+        ":http_source_interface",
         ":key_cc_proto",
         ":range_utils_lib",
         "//envoy/buffer:buffer_interface",
@@ -137,6 +199,7 @@ envoy_cc_library(
     hdrs = ["cache_headers_utils.h"],
     deps = [
         ":cache_custom_headers",
+        ":key_cc_proto",
         "//envoy/common:time_interface",
         "//envoy/http:header_map_interface",
         "//source/common/common:matchers_lib",
@@ -159,12 +222,12 @@ envoy_cc_library(
     ],
 )
 
-envoy_cc_library(
-    name = "cache_filter_logging_info_lib",
-    srcs = ["cache_filter_logging_info.cc"],
-    hdrs = ["cache_filter_logging_info.h"],
+envoy_cc_extension(
+    name = "stats",
+    srcs = ["stats.cc"],
+    hdrs = ["stats.h"],
     deps = [
-        "//source/common/stream_info:filter_state_lib",
+        ":cache_entry_utils_lib",
     ],
 )
 
@@ -174,6 +237,8 @@ envoy_cc_extension(
     hdrs = ["config.h"],
     deps = [
         ":cache_filter_lib",
+        ":cache_sessions_lib",
+        ":stats",
         "//source/extensions/filters/http/common:factory_base_lib",
         "@envoy_api//envoy/extensions/filters/http/cache/v3:pkg_cc_proto",
     ],