Direct response/Redirect with ext_authz

[artificial-aidan]

Title: Unable to run ext_authz filter with direct response and redirect.

Description:
From how I read #8436 I should be able to set up a direct response and have it handled by the authorization filter. But when I set it up as follows no call is made to the ext_authz filter, and it just responds as specified

- 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: local_service
          domains: ["*"]
          typed_per_filter_config:
            envoy.filters.http.ext_authz:
              "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute
              check_settings:
                context_extensions:
                  virtual_host: local_service
          routes:
            - match: { path: "/auth-check" }
              direct_response:
                status: 200
            - match: { prefix: "/old-path" }
              redirect:
                path_redirect: "/new-path"
            - match: { prefix: "/" }
              route:
                cluster: upstream
    http_filters:
      - name: envoy.filters.http.ext_authz
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz
          grpc_service:
            envoy_grpc:
              cluster_name: ext_authz-grpc-service
            timeout: 0.250s
          failure_mode_allow: false
          transport_api_version: V3

[esmet]

I took a look at this and figured out that the ext_authz filter is invoked in this case, but it does nothing because it cannot find a route entry:

Filter::PerRouteFlags Filter::getPerRouteFlags(const Router::RouteConstSharedPtr& route) const {
  if (route == nullptr || route->routeEntry() == nullptr) {
    return PerRouteFlags{true /*skip_check_*/, false /*skip_request_body_buffering_*/};
  }
...

To support direct replies, we'd need to also check for route->directResponseEntry(). I can't think of a reason why direct entries shouldn't work here so I think this is just a bug that should be fixed. Operators can still configure direct response entries that skip ext_authz by providing a per-route config that disables the filter.

[esmet]

/assign

[esmet]

This bit in the ext_authz.cc filter seems to do the trick:

 Filter::PerRouteFlags Filter::getPerRouteFlags(const Router::RouteConstSharedPtr& route) const {
-  if (route == nullptr || route->routeEntry() == nullptr) {
-    return PerRouteFlags{true /*skip_check_*/, false /*skip_request_body_buffering_*/};
+  if (route == nullptr) {
+    // If there's no route, skip authorization checks. This is an optimization to avoid work
+    // if we know the request won't be forwarded upstream anyway.
+    return PerRouteFlags::SkipCheckFlags();
+  }
+
+  if (route->routeEntry() == nullptr && route->directResponseEntry() == nullptr) {
+    // If there's a route, but no route entry nor direct response entry, then skip authorization
+    // checks. This is another optimization.
+    return PerRouteFlags::SkipCheckFlags();
   }

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

[esmet]

Can we unstale this and associate it with a work in progress #14989?

[soulxu]

This is fixed by #17546