remove DFP filter

[zhaohuabing]

This PR removes the DFP filter in the HCM filter chain and moves the host rewrite configuration to the route.

This works because the DFP cluster can resolve DNS asynchronously without the DFP filter.

This PR also fixes: #8353 because now the route can see the original host in the request headers.

[netlify]

✅ Deploy Preview for cerulean-figolla-1f9435 ready!

Name	Link
🔨 Latest commit	3258241
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69d8b0a6d0d81d0008d41544
😎 Deploy Preview	https://deploy-preview-8655--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.32%. Comparing base (f404a9c) to head (3258241).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8655      +/-   ##
==========================================
- Coverage   74.35%   74.32%   -0.04%     
==========================================
  Files         244      244              
  Lines       38792    38696      -96     
==========================================
- Hits        28845    28761      -84     
+ Misses       7950     7944       -6     
+ Partials     1997     1991       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[zirain]

do we need to backport this? if so, better to have a release note.

[zirain]

LGTM, it would be better to have an e2e ensure it worked as expected.

[zhaohuabing]

LGTM, it would be better to have an e2e ensure it worked as expected.

There is no special processing for the dynamic resolver host rewritten anymore, so a dedicated e2e test for this seems unnecessary for now. We can add one in the future if needed.

[zhaohuabing]

/retest

[guydc]

Are the envoy docs out-of-date? They imply that both filter and cluster are needed:

https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/http/http_proxy

Through the combination of both an HTTP filter and custom cluster, Envoy supports HTTP dynamic forward proxy.
[...]
The dynamic forward proxy HTTP filter is used to pause requests if the target DNS host is not already in cache.

Is there any implication of not using the filter and relying on the cluster to do this work, e.g. :

• latency
• distribution of load between different endpoints

Also, does this work in the sub_clusters mode of DFP? While not currently supported by EG, we may want to enabl it at some point in the future, so it's best if the DFP filter is not needed there either.

[guydc]

one note on this change: if previously downstream filters could access the upstream IP, this would no longer be the case. This can be useful in some scenarios: envoyproxy/envoy#43834.

Since EG doesn't enable save_upstream_address by default, it's not likely that users relied on this behavior to access the address in extension filters.

[guydc]

LGTM, it would be better to have an e2e ensure it worked as expected.

There is no special processing for the dynamic resolver host rewritten anymore, so a dedicated e2e test for this seems unnecessary for now. We can add one in the future if needed.

But this flow is still covered by an e2e, right? demonstrating that the rewrite actually happens and that it determines the host that would be looked up ?

[zhaohuabing]

Are the envoy docs out-of-date? They imply that both filter and cluster are needed.

The doc is a bit confusing around the DFP filter vs. cluster.
The filter handles DNS resolution and host rewrite (actually sub_cluster as well, but not used by EG for now, details in the later response), but both can already be covered by the DFP cluster and the router. So the filter isn’t really needed in the request path.
Also, adding the filter means the router isn’t aware of the host rewrite, which results in missing X-ENVOY-ORIGINAL-HOST in access logs.

is there any implication of not using the filter and relying on the cluster to do this work, e.g. :

latency
distribution of load between different endpoints

• latency: cache misses still pay DNS either way; the filter mainly makes the pause happen earlier and more predictably. So there shouldn’t be any meaningful latency difference.
• distribution of load: DNS resolution is about finding the target host and populating the DFP cache. Once the host exists, the cluster still selects from the host set it has. So relying on cluster-side resolution does not inherently change how load is distributed across endpoints for a given resolved target.

Also, does this work in the sub_clusters mode of DFP? While not currently supported by EG, we may want to enabl it at some point in the future, so it's best if the DFP filter is not needed there either.

The sub-clusters are created by the DFP filter, so it's required for the sub-cluster case. We can add the DFP filter for the sub-cluster path in the future if this feature is used.

one note on this change: if previously downstream filters could access the upstream IP, this would no longer be the case. This can be useful in some scenarios: envoyproxy/envoy#43834.

Since EG doesn't enable save_upstream_address by default, it's not likely that users relied on this behavior to access the address in extension filters.

Yes, save_upstream_address is not enabled by EG. We can introudce a knob in the API to turn on it if it's needed in the future. And if it's enabled, the DFP filter should be added to the filter chain.

But this flow is still covered by an e2e, right? demonstrating that the rewrite actually happens and that it determines the host that would be looked up ?

There is no e2e for the general enableEnvoyHeaders test case, I can add one in this PR if we feel it's needed.

[zhaohuabing]

@zirain @guydc a test has been added to cover that X-ORIGINAL-HOST is added by envoy for DFP host rewrite case.

[zhaohuabing]

/retest