prefix_rewrite not being respected with caching enabled in Envoy 1.33

[ashishb-solo]

If you are reporting any crash or any potential security issue, do not
open an issue in this repo. Please report the issue via emailing
[email protected] where the issue will be triaged appropriately.

Title: prefix_rewrite not being respected with caching enabled in Envoy 1.33

Description:

As of Envoy 1.33 and above, when caching is enabled on a filter chain, the prefix_rewrite configuration on the route is not being respected. Requests to the upstream are coming in with the original path provided by the downstream client instead.

I'm fairly certain this behaviour was introduced in this commit.

Repro steps:

Run an echo server on port 8090 (docker run -p 8090:8080 -e LOG_HTTP_HEADERS=true -e LOG_HTTP_BODY=true jmalloc/echo-server) and run Envoy with the below configuration.

• With Envoy 1.32.5, the response is GET / HTTP/1.1 (which is correct).
• In Envoy 1.33.2, the response is GET /test HTTP/1.1 which is incorrect (/test should be removed in accordance with the prefix_rewrite configuration on the route).
• If the envoy.filters.http.cache filter is removed from the configuration, then Envoy 1.33 behaves the same as 1.32 (the upstream correctly sees / and not /test).

# required for toggling runtime reloadable features
layered_runtime:
  layers:
  - name: admin
    admin_layer: {}

# taken from https://www.envoyproxy.io/docs/envoy/latest/start/quick-start/configuration-static
# or view-source:https://www.envoyproxy.io/docs/envoy/latest/_downloads/92dcb9714fb6bc288d042029b34c0de4/envoy-demo.yaml
admin:
  address:
    socket_address: { address: 127.0.0.1, port_value: 9901 }

static_resources:

  listeners:
  - name: listener_0
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 10000
    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
          stat_prefix: ingress_http
          access_log:
          - name: envoy.access_loggers.stdout
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
              # log_format:
              #   text_format_source:
              #     inline_string: "test\n"
          http_filters:
          - name: envoy.filters.http.cache
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.cache.v3.CacheConfig
              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
          route_config:
            name: listener-::-8080-routes
            virtual_hosts:
              - name: my-virtual-host
                domains:
                  - test-domain
                  - cache-hit.example.com
                routes:
                  - match:
                      prefix: /test
                    route:
                      cluster: my-cluster
                      prefix_rewrite: /
                    name: my-virtual-host
                typed_per_filter_config:
                  envoy.filters.http.ext_authz:
                    '@type': type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute
                    disabled: true

  clusters:
  - name: my-cluster
    type: LOGICAL_DNS
    # Comment out the following line to test on v6 networks
    dns_lookup_family: V4_ONLY
    load_assignment:
      cluster_name: my-cluster
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: 127.0.0.1
                port_value: 8090

[Implausiblyfun]

I think that technically this will still get the right lookups as key_ stores the path on the request.

However if any other header mutations take place and they affect a vary header then I think you could get to a space where some get responses would be cached but never retrieved from cache right?

[ashishb-solo]

I haven't thought too much about the implications for the process of performing the caching operation itself. I was thinking more about how this affects upstreams, since they will be serving different requests based on the version of Envoy that is used (ie. / vs /test). You probably are right about that though, but I haven't really looked into that portion of the issue yet.

[phlax]

cc @toddmgreer @jmarantz @penguingao @mpwarres @capoferro as codeowners
cc @ravenblackx as he may know more

[ravenblackx]

Would highly recommend not using the cache filter at all until the new version lands. Before or after that commit (which you are correct to call out as the cause), it's a disaster, in different ways.

The new version (#37990) is also a disaster, that will require weird custom configuration to get it to do anything sensible, but it at least can be coaxed into doing something sensible. The whole premise of trying to fit cache behavior into the constraints of an envoy filter is unfortunately a bad one.

I also would not recommend waiting for the new version of the filter because its rate of progress is glacial, so my recommendation at this time would be to either do caching some other way, or don't do caching at all.

Possible workarounds at this time if you really want to use the cache filter (which are also what will likely end up being needed with the new version) involve routing through an internal listener; you could either:

1. put the cache on the real listener with very little config, and send everything from there to an internal listener, in which you configure your prefix_rewrite and any other filters, or
2. do everything else first, on the real listener, including prefix_rewrite, but have an internal listener be the global upstream cluster for that listener, and then have the cache filter and actual upstream routing on the internal listener.

Both of these options are a pain and will interfere with other things that other filters do, if you have a complicated filter chain.