Skip to content

Require Both Type and Compressor Field for Compression in BackendTrafficPolicy config #6924

@sudiptob2

Description

@sudiptob2

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:

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.

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

  1. Disable all: The user wants to disable all compression at the route level.

    compression:
      - type: Brotli
        brotli: null
      - type: Gzip
        gzip: null
  2. Disable Gzip: Gzip is enabled at the gateway level, but the user wants to disable it.

    compression:
      - type: Gzip
        gzip: null
  3. 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: {}
  4. 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 the type and the corresponding compressor field, introducing a breaking change. Therefore, it may be better to implement a new field such as Compressor with this logic and deprecate the existing Compression field.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/apiAPI-related issueskind/decisionA record of a decision made by the community.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions