Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compressor: expose generic compressor filter to users #10553

Merged
merged 65 commits into from
May 8, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
47139ba
compressor: expose generic compressor filter to users
Mar 19, 2020
4bc9660
fix naming conflict in compressor_filter_integration_test.cc
Mar 27, 2020
b7cfac0
fix clang-tidy findings
Mar 27, 2020
fd7fab2
coverage: fix naming conflict
Mar 30, 2020
7bfd623
fix bazel compile_time_options build
Mar 30, 2020
153f97e
Merge remote-tracking branch 'upstream/master' into compress
Mar 31, 2020
09f10a9
explicitly deprecate gzip filter
Mar 31, 2020
d9b33cc
fix bazel compile_time_options build
Apr 1, 2020
90d02d5
address review comments
Apr 6, 2020
05a8541
zlib: use Z_DEFAULT_STRATEGY by default instead of Z_FIXED
Apr 7, 2020
c3c26b6
gzip.proto: rework CompressionStrategy and CompressionLevel enums
Apr 7, 2020
ece1755
compressor: make compressor_library required
Apr 7, 2020
3323683
gzip: allow setting of chunk size
Apr 7, 2020
b7e1d28
document the Fixed compression strategy
Apr 8, 2020
85676a2
Merge remote-tracking branch 'upstream/master' into compress
Apr 9, 2020
681d7c4
move to v3 proto
Apr 8, 2020
79051a7
Merge remote-tracking branch 'upstream/master' into compress
Apr 14, 2020
9bcc469
update version history
Apr 14, 2020
21fb4eb
fix pedantic spelling check
Apr 14, 2020
2f48548
make use of PROTOBUF_GET_WRAPPED_OR_DEFAULT
Apr 14, 2020
63085ac
compressor: introduce CompressorPtr type alias
Apr 14, 2020
edbe6b5
rename createCompressorLibraryFromProto to createCompressorFactoryFro…
Apr 14, 2020
da5a305
remove unused header
Apr 14, 2020
76c2b1f
docs: fix references
Apr 14, 2020
bc086c0
avoid using v2 types in v3 configs
Apr 15, 2020
3f89444
Merge remote-tracking branch 'upstream/master' into compress
Apr 15, 2020
801bdcf
fix comments
Apr 15, 2020
8e12669
restructure compressor library extensions
Apr 17, 2020
79ac3b9
Merge remote-tracking branch 'upstream/master' into compress
Apr 17, 2020
2bdbee3
run proto_format/proto_format.sh to fix format check
Apr 17, 2020
d72e015
finalize moving gzip code to new compression extension
Apr 20, 2020
85ae344
Merge remote-tracking branch 'upstream/master' into compress
Apr 20, 2020
d833c1a
run check_format.py fix
Apr 20, 2020
388d8bc
silence clang-tidy warnings
Apr 20, 2020
6ea514a
revert moving decompressor code to reduce patch size
Apr 21, 2020
d43fb14
add granularity to CODEOWNERS
Apr 21, 2020
1498c20
address review comments
Apr 21, 2020
4d09cd2
move gzip fuzzer test one level up
Apr 21, 2020
eabd93d
adjust silencing clang-tidy warnings
Apr 21, 2020
c69e0bb
compression: rework tests
Apr 22, 2020
63a6e46
compressor: add and use test mock for compressor
Apr 22, 2020
cf473a7
rename MockCompressorFilterConfig to TestCompressorFilterConfig
Apr 22, 2020
02024a3
compressor: drop gzip specific test from generic compressor
Apr 22, 2020
590167c
gzip: actually remove friend class
Apr 22, 2020
680ae29
resort to lvalue parameter to avoid usage after std::move casting
Apr 23, 2020
dd93ad6
test: set expectations of compress() call counts for every test case
Apr 23, 2020
e6f9c79
compressor: remove unused declaration from mock
Apr 23, 2020
9aaf9ff
compressor: add test case for CompressorFilterConfig
Apr 23, 2020
d9ee8e0
add compressor filter specific stats prefix
Apr 23, 2020
503c81a
fix code formatting
Apr 23, 2020
b3a49d2
Merge remote-tracking branch 'upstream/master' into compress
Apr 27, 2020
6ede080
drop removed component from CODEOWNERS
Apr 27, 2020
c5149d2
docs: fix doc title
Apr 27, 2020
9b59cac
run check_format.py fix
Apr 27, 2020
a271f28
gzip: do protobuf conversion with JSON round trip
Apr 27, 2020
bc77b25
run proto_format.sh fix
Apr 27, 2020
4a9bff7
reduce gzip compressor's stats prefix to just gzip
Apr 28, 2020
5f683bd
fix reference to the interface class
May 4, 2020
786ab77
avoid changing frozen API in gzip.proto
May 4, 2020
6a9b477
Merge remote-tracking branch 'upstream/master' into compress
May 4, 2020
4b4cfda
compressor: add stats_prefix to compressor filter
May 5, 2020
00fc997
Revert "compressor: add stats_prefix to compressor filter"
May 8, 2020
8c37ff1
Merge remote-tracking branch 'upstream/master' into compress
May 8, 2020
152a6b7
replace CompressorLibrary with TypedExtensionConfig
May 8, 2020
6b23035
reflect config changes in documentation
May 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ extensions/filters/common/original_src @snowp @klarose
/*/extensions/filters/network/thrift_proxy @zuercher @brian-pane
# compressor used by http compression filters
/*/extensions/filters/http/common/compressor @gsagula @rojkov @dio
/*/extensions/filters/http/compressor @rojkov @dio
/*/extensions/filters/http/compressor/gzip @gsagula @rojkov @dio
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this and line 25 be removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the line 27, but 25 is still needed according to check_format.py fix

# jwt_authn http filter extension
/*/extensions/filters/http/jwt_authn @qiwzhang @lizan
# grpc_http1_reverse_bridge http filter extension
Expand Down
2 changes: 2 additions & 0 deletions api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ proto_library(
"//envoy/config/filter/http/aws_request_signing/v2alpha:pkg",
"//envoy/config/filter/http/buffer/v2:pkg",
"//envoy/config/filter/http/cache/v2alpha:pkg",
"//envoy/config/filter/http/compressor/gzip/v2:pkg",
"//envoy/config/filter/http/compressor/v2:pkg",
"//envoy/config/filter/http/cors/v2:pkg",
"//envoy/config/filter/http/csrf/v2:pkg",
Expand Down Expand Up @@ -170,6 +171,7 @@ proto_library(
"//envoy/extensions/filters/http/aws_request_signing/v3:pkg",
"//envoy/extensions/filters/http/buffer/v3:pkg",
"//envoy/extensions/filters/http/cache/v3alpha:pkg",
"//envoy/extensions/filters/http/compressor/gzip/v3:pkg",
"//envoy/extensions/filters/http/compressor/v3:pkg",
"//envoy/extensions/filters/http/cors/v3:pkg",
"//envoy/extensions/filters/http/csrf/v3:pkg",
Expand Down
9 changes: 9 additions & 0 deletions api/envoy/config/filter/http/compressor/gzip/v2/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# DO NOT EDIT. This file is generated by tools/proto_sync.py.

load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")

licenses(["notice"]) # Apache 2

api_proto_package(
deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
)
67 changes: 67 additions & 0 deletions api/envoy/config/filter/http/compressor/gzip/v2/gzip.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
syntax = "proto3";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this relate to https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/http/gzip/v2/gzip.proto? Which one should service operators configure and when? Is the former deprecated?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly, the former is deprecated. We need to make that clear as @htuch points out, have text in deprecated.rst, and also make sure the deprecated feature warning/stat is incremented so that operators know thy need to upgrade.


package envoy.config.filter.http.compressor.gzip.v2;

import "google/protobuf/wrappers.proto";

import "udpa/annotations/migrate.proto";
import "udpa/annotations/status.proto";
import "validate/validate.proto";

option java_package = "io.envoyproxy.envoy.config.filter.http.compressor.gzip.v2";
option java_outer_classname = "GzipProto";
option java_multiple_files = true;
option (udpa.annotations.file_migrate).move_to_package =
"envoy.extensions.filters.http.compressor.gzip.v3";
option (udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: Gzip]
// [#extension: envoy.filters.http.compressor.gzip]

// [#next-free-field: 10]
message Gzip {
enum CompressionStrategy {
// The DEFAULT value translates directly to zlib's Z_DEFAULT_STRATEGY. It doesn't
// equate to any other strategy.
DEFAULT = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on what the DEFAULTs are here and below? These should be fixed in the API.


FILTERED = 1;

HUFFMAN = 2;

RLE = 3;
}

message CompressionLevel {
enum Enum {
DEFAULT = 0;
BEST = 1;
SPEED = 2;
}
}
htuch marked this conversation as resolved.
Show resolved Hide resolved

// Value from 1 to 9 that controls the amount of internal memory used by zlib. Higher values
// use more memory, but are faster and produce better compression results. The default value is 5.
google.protobuf.UInt32Value memory_level = 1 [(validate.rules).uint32 = {lte: 9 gte: 1}];

// A value used for selecting the zlib compression level. This setting will affect speed and
// amount of compression applied to the content. "BEST" provides higher compression at the cost of
// higher latency, "SPEED" provides lower compression with minimum impact on response time.
// "DEFAULT" provides an optimal result between speed and compression. This field will be set to
// "DEFAULT" if not specified.
CompressionLevel.Enum compression_level = 3 [(validate.rules).enum = {defined_only: true}];

// A value used for selecting the zlib compression strategy which is directly related to the
// characteristics of the content. Most of the time "DEFAULT" will be the best choice, though
// there are situations which changing this parameter might produce better results. For example,
// run-length encoding (RLE) is typically used when the content is known for having sequences
// which same data occurs many consecutive times. For more information about each strategy, please
// refer to zlib manual.
CompressionStrategy compression_strategy = 4 [(validate.rules).enum = {defined_only: true}];

// Value from 9 to 15 that represents the base two logarithmic of the compressor's window size.
// Larger window results in better compression at the expense of memory usage. The default is 12
// which will produce a 4096 bytes window. For more details about this parameter, please refer to
// zlib manual > deflateInit2.
google.protobuf.UInt32Value window_bits = 9 [(validate.rules).uint32 = {lte: 15 gte: 9}];
}
23 changes: 22 additions & 1 deletion api/envoy/config/filter/http/compressor/v2/compressor.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ package envoy.config.filter.http.compressor.v2;

import "envoy/api/v2/core/base.proto";

import "google/protobuf/any.proto";
import "google/protobuf/wrappers.proto";

import "udpa/annotations/migrate.proto";
import "udpa/annotations/status.proto";
import "validate/validate.proto";

option java_package = "io.envoyproxy.envoy.config.filter.http.compressor.v2";
option java_outer_classname = "CompressorProto";
Expand All @@ -17,8 +19,10 @@ option (udpa.annotations.file_migrate).move_to_package =
option (udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: Compressor]
// Compressor :ref:`configuration overview <config_http_filters_compressor>`.
// [#extension: envoy.filters.http.compressor]

// [#next-free-field: 6]
// [#next-free-field: 7]
message Compressor {
// Minimum response length, in bytes, which will trigger compression. The default value is 30.
google.protobuf.UInt32Value content_length = 1;
Expand All @@ -45,4 +49,21 @@ message Compressor {
// Runtime flag that controls whether the filter is enabled or not. If set to false, the
// filter will operate as a pass-through filter. If not specified, defaults to enabled.
api.v2.core.RuntimeFeatureFlag runtime_enabled = 5;

// A compressor library to use for compression. Currently only
// :ref:`envoy.filters.http.compressor.gzip<envoy_api_msg_config.filter.http.compressor.gzip.v2.Gzip>`
// is included in Envoy.
//
// This field is ignored when used in the context of the deprecated "envoy.filters.http.gzip"
// filter.
CompressorLibrary compressor_library = 6;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it makes sense to have a default here. Can we require this and then make the gzip config explicit for clarity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this field is required now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it required via annotation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... This message is used also in the deprecated envoy.filters.http.gzip filter. I guess there are two options:

  1. revert gzip: make use of generalized compression filter #10306 completely or partially (drop the compressor field from the Gzip message);
  2. duplicate compressor.proto for envoy.filters.http.gzip.

}

message CompressorLibrary {
// The name of the compressor library to instantiate for the filter.
string name = 1 [(validate.rules).string = {min_bytes: 1}];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this for new extensions as the extension will be looked up via the typed_config.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, removed.


// Compressor library specific configuration. See the supported libraries for further
// documentation.
google.protobuf.Any typed_config = 2;
}
12 changes: 12 additions & 0 deletions api/envoy/extensions/filters/http/compressor/gzip/v3/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# DO NOT EDIT. This file is generated by tools/proto_sync.py.

load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")

licenses(["notice"]) # Apache 2

api_proto_package(
deps = [
"//envoy/config/filter/http/compressor/gzip/v2:pkg",
"@com_github_cncf_udpa//udpa/annotations:pkg",
],
)
71 changes: 71 additions & 0 deletions api/envoy/extensions/filters/http/compressor/gzip/v3/gzip.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
syntax = "proto3";

package envoy.extensions.filters.http.compressor.gzip.v3;

import "google/protobuf/wrappers.proto";

import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
import "validate/validate.proto";

option java_package = "io.envoyproxy.envoy.extensions.filters.http.compressor.gzip.v3";
option java_outer_classname = "GzipProto";
option java_multiple_files = true;
option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE;

// [#protodoc-title: Gzip]
// [#extension: envoy.filters.http.compressor.gzip]

// [#next-free-field: 10]
message Gzip {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.compressor.gzip.v2.Gzip";

enum CompressionStrategy {
// The DEFAULT value translates directly to zlib's Z_DEFAULT_STRATEGY. It doesn't
// equate to any other strategy.
DEFAULT = 0;

FILTERED = 1;

HUFFMAN = 2;

RLE = 3;
}

message CompressionLevel {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.compressor.gzip.v2.Gzip.CompressionLevel";

enum Enum {
DEFAULT = 0;
BEST = 1;
SPEED = 2;
}
}

// Value from 1 to 9 that controls the amount of internal memory used by zlib. Higher values
// use more memory, but are faster and produce better compression results. The default value is 5.
google.protobuf.UInt32Value memory_level = 1 [(validate.rules).uint32 = {lte: 9 gte: 1}];

// A value used for selecting the zlib compression level. This setting will affect speed and
// amount of compression applied to the content. "BEST" provides higher compression at the cost of
// higher latency, "SPEED" provides lower compression with minimum impact on response time.
// "DEFAULT" provides an optimal result between speed and compression. This field will be set to
// "DEFAULT" if not specified.
CompressionLevel.Enum compression_level = 3 [(validate.rules).enum = {defined_only: true}];

// A value used for selecting the zlib compression strategy which is directly related to the
// characteristics of the content. Most of the time "DEFAULT" will be the best choice, though
// there are situations which changing this parameter might produce better results. For example,
// run-length encoding (RLE) is typically used when the content is known for having sequences
// which same data occurs many consecutive times. For more information about each strategy, please
// refer to zlib manual.
CompressionStrategy compression_strategy = 4 [(validate.rules).enum = {defined_only: true}];

// Value from 9 to 15 that represents the base two logarithmic of the compressor's window size.
// Larger window results in better compression at the expense of memory usage. The default is 12
// which will produce a 4096 bytes window. For more details about this parameter, please refer to
// zlib manual > deflateInit2.
google.protobuf.UInt32Value window_bits = 9 [(validate.rules).uint32 = {lte: 15 gte: 9}];
}
26 changes: 25 additions & 1 deletion api/envoy/extensions/filters/http/compressor/v3/compressor.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,23 @@ package envoy.extensions.filters.http.compressor.v3;

import "envoy/config/core/v3/base.proto";

import "google/protobuf/any.proto";
import "google/protobuf/wrappers.proto";

import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
import "validate/validate.proto";

option java_package = "io.envoyproxy.envoy.extensions.filters.http.compressor.v3";
option java_outer_classname = "CompressorProto";
option java_multiple_files = true;
option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE;

// [#protodoc-title: Compressor]
// Compressor :ref:`configuration overview <config_http_filters_compressor>`.
// [#extension: envoy.filters.http.compressor]

// [#next-free-field: 6]
// [#next-free-field: 7]
message Compressor {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.compressor.v2.Compressor";
Expand Down Expand Up @@ -46,4 +50,24 @@ message Compressor {
// Runtime flag that controls whether the filter is enabled or not. If set to false, the
// filter will operate as a pass-through filter. If not specified, defaults to enabled.
config.core.v3.RuntimeFeatureFlag runtime_enabled = 5;

// A compressor library to use for compression. Currently only
// :ref:`envoy.filters.http.compressor.gzip<envoy_api_msg_extensions.filters.http.compressor.gzip.v3.Gzip>`
// is included in Envoy.
//
// This field is ignored when used in the context of the deprecated "envoy.filters.http.gzip"
// filter.
CompressorLibrary compressor_library = 6;
}

message CompressorLibrary {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.compressor.v2.CompressorLibrary";

// The name of the compressor library to instantiate for the filter.
string name = 1 [(validate.rules).string = {min_bytes: 1}];

// Compressor library specific configuration. See the supported libraries for further
// documentation.
google.protobuf.Any typed_config = 2;
}
1 change: 1 addition & 0 deletions api/versioning/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ proto_library(
"//envoy/config/filter/http/aws_request_signing/v2alpha:pkg",
"//envoy/config/filter/http/buffer/v2:pkg",
"//envoy/config/filter/http/cache/v2alpha:pkg",
"//envoy/config/filter/http/compressor/gzip/v2:pkg",
"//envoy/config/filter/http/compressor/v2:pkg",
"//envoy/config/filter/http/cors/v2:pkg",
"//envoy/config/filter/http/csrf/v2:pkg",
Expand Down
1 change: 1 addition & 0 deletions docs/root/api-v2/config/filter/http/http.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ HTTP filters
*/v2/*
*/v2alpha/*
*/v2alpha1/*
compressor/*/v2/*
1 change: 1 addition & 0 deletions docs/root/api-v3/config/filter/http/http.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ HTTP filters

*/empty/*
../../../extensions/filters/http/*/v3/*
../../../extensions/filters/http/compressor/*/v3/*
Loading