disableCompression: Expose configuration to toggle Envoy GZIP compression on the responses

[chaosbox]

For #6511

This PR adds,

• Listener configuration that exposes a boolean flag to disable compression, by default compression is enabled. This also provides us a way to disable if the users prefer to trade network for CPU, especially when teams want to run lean Envoy instances and rely on horizontal scalability.
• We will run the test build for a while in our cluster to show the actual cost benefit.

Related #310, there had been mentions about disabling compression, the ticket we had raised shows the reason where disabling compression can bring cost benefits.

[github-actions]

Hi @chaosbox! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

[github-actions]

The Contour project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 14d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, the PR is closed

You can:

• Ensure your PR is passing all CI checks. PRs that are fully green are more likely to be reviewed. If you are having trouble with CI checks, reach out to the #contour channel in the Kubernetes Slack workspace.
• Mark this PR as fresh by commenting or pushing a commit
• Close this PR
• Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

[chaosbox]

Ping

[tsaarni]

I've included suggestions inline for some linter nags (link).

Please add the new disableCompression option in "Configuration File" chapter in site/content/docs/main/configuration.md.

Please add also a changelog file.

[tsaarni]

Possible alternative approach could be to leave the compressor filter in the chain but disable it by setting default to false in ResponseDirectionConfig.CommonConfig.Enabled

Enabled: &envoy_config_core_v3.RuntimeFeatureFlag{
	// If compression is enabled add compressor filter
	DefaultValue: wrapperspb.Bool(!b.disableCompression),
	RuntimeKey:   "contour.compression.response.enabled",
},

but since we do not have "per route" setting, which would override the default, it make sense to me that the whole filter is excluded from the chain when disabled, like suggested in this PR currently.

[geomacy]

Yes, given

since we do not have "per route" setting

the thought in this PR is to work at the level of the whole filter. The functionality could be extended as described above if needed in future but that would best be left to another PR.

[geomacy]

Thanks very much for the feedback @tsaarni 🙇 we'll have a look at this and try to get back with updates as soon as possible.

[geomacy]

Many thanks for taking the time to provide the lint fixes. We'll update the docs and add the changelog as soon as possible.

[codecov]

Codecov Report

Attention: Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.

Project coverage is 81.77%. Comparing base (db432cc) to head (72e0e9c).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/contour/serve.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6546   +/-   ##
=======================================
  Coverage   81.76%   81.77%           
=======================================
  Files         133      133           
  Lines       15944    15954   +10     
=======================================
+ Hits        13037    13046    +9     
- Misses       2614     2615    +1     
  Partials      293      293           

Files with missing lines	Coverage Δ
cmd/contour/servecontext.go	85.98% <100.00%> (+0.03%)	⬆️
internal/envoy/v3/listener.go	98.47% <100.00%> (+0.01%)	⬆️
internal/xdscache/v3/listener.go	92.20% <100.00%> (+0.08%)	⬆️
pkg/config/parameters.go	88.03% <ø> (ø)
cmd/contour/serve.go	22.63% <0.00%> (-0.04%)	⬇️

[geomacy]

Hello @tsaarni I have made the changes to the documentation you mentioned.

The first commit 934ca48 is a pure format change to tidy the table borders; the diff therefore looks worse than it is, but is more readable with whitespace diff comparison turned off. The actual addition of the flag to the docs is a one-liner in 2efb693.

[tsaarni]

Looks good! Just small documentation comments.

[tsaarni]

Suggested change

	// DisableCompression disables GZIP compression HTTP filter from the default Listener filters
	// Setting this true will enable Envoy skip "Accept-Encoding: gzip,deflate" request header and always return uncompressed response
	// DisableCompression disables GZIP compression HTTP filter from the default Listener filters.
	// Setting this true will enable Envoy skip "Accept-Encoding: gzip,deflate" request header and always return uncompressed response.

[tsaarni]

The changelog entry could mention where the flag can be found

Suggested change

	Add disableCompression boolean flag to disable GZIP compression HTTP filter from the default Listener filters.
	Add disableCompression boolean flag in the ContourConfiguration CRD and in configuration file to disable GZIP compression HTTP filter, forcing Envoy to always return uncompressed response.

[davinci26]

sorry for commenting so late in the process but I think given that there are a lot of compression algorithms supported by envoy. Some of them much better than gzip like zstd I think we might want to change the api to allow the user to choose the compression algorithm and have an option called nocompression that disables it

[aecay]

@davinci26 that's an interesting idea. It goes beyond what was mentioned in #310 which -- although it's far from a formal design process (so early in contour's lifecylce) -- is at least some semblance of a "blessing" of the API change in this PR. If we were to go for the alternative API you proposed (which is IMO richer/better overall), would it require going through the proposal process since it's not something that has been (afaik) so far envisioned/discussed?