-
Notifications
You must be signed in to change notification settings - Fork 562
Description
In the current BackendTrafficPolicy
compression implementation, only the type
field is considered during IR translation, while the actual compressor configuration fields (gzip
, brotli
) are ignored. This leads to confusion and prevents users from properly disabling compression.
See related discussions where users were unable to disable compression at the route level:
- Unable to patch out fields using mergeType in BackendTrafficPolicy #6775
- Is it possible to remove BackendTrafficPolicy rules via mergeType? #6143
The compressor
field should be respected when set to null
, but it is currently ignored. Users should be required to provide both the type
and the corresponding brotli
/gzip
attribute. Then, In the buildCompression function, we can add logic to check if brotli
or gzip
is set to null
. Based on that, we can determine the final compression configuration in the XDS.IR.
Current Behavior
compression:
- type: Gzip
gzip: null # This is ignored
The buildCompression function only checks the type field and ignores gzip/brotli configuration. We should also validate the compressor values.
gateway/internal/gatewayapi/backendtrafficpolicy.go
Lines 1177 to 1189 in b1f23d7
func buildCompression(compression []*egv1a1.Compression) []*ir.Compression { | |
if compression == nil { | |
return nil | |
} | |
irCompression := make([]*ir.Compression, 0, len(compression)) | |
for _, c := range compression { | |
irCompression = append(irCompression, &ir.Compression{ | |
Type: c.Type, | |
}) | |
} | |
return irCompression | |
} |
Proposed Change
if (c.Type == egv1a1.GzipCompressorType && c.Gzip != nil) ||
(c.Type == egv1a1.BrotliCompressorType && c.Brotli != nil) {
irCompression = append(irCompression, &ir.Compression{
Type: c.Type,
})
}
This change ensures that compression is applied only when explicitly configured, providing finer-grained control.
It also addresses the cases where users expect to disable compression, as mentioned in #6775 and #6143.
To disable policies at the route level, users will be able to use JSONMerge
. Some example test cases are shown below:
Scenarios: Applying route-level policies with JSONMerge
-
Disable all: The user wants to disable all compression at the route level.
compression: - type: Brotli brotli: null - type: Gzip gzip: null
-
Disable Gzip: Gzip is enabled at the gateway level, but the user wants to disable it.
compression: - type: Gzip gzip: null
-
Switch to Brotli: Gzip is enabled at the gateway level, but the user wants to use Brotli at the route level.
compression: - type: Brotli brotli: {}
-
Disable one: Both Gzip and Brotli are enabled at the gateway level, but the user wants to disable Brotli at the route level.
compression: - type: Brotli brotli: null - type: Gzip gzip: {}
Some other approaches are tested in #6904, which introduces custom logic to override fields after merge, which is not maintainable.
⚠️ Note
This would require users to always specify both thetype
and the corresponding compressor field, introducing a breaking change. Therefore, it may be better to implement a new field such asCompressor
with this logic and deprecate the existingCompression
field.