Response map HTTP filter for custom upstream responses #14221
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. | ||
|
|
||
| load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| api_proto_package( | ||
| deps = [ | ||
| "//envoy/config/accesslog/v3:pkg", | ||
| "//envoy/config/core/v3:pkg", | ||
| "@com_github_cncf_udpa//udpa/annotations:pkg", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| syntax = "proto3"; | ||
|
Member
There was a problem hiding this comment. This is not where extension config should live, move to
Contributor
Author
There was a problem hiding this comment. The intention here is for this to be non-extension config (for source/common/response_map/*) and to have separate extension config for the response map filter (source/extensions/http/filters/response_map/...). Is this idea correct or misguided?
Contributor
Author
There was a problem hiding this comment. I looked harder at how the API is organized and I think I understand. My feeling now is that this should live under |
||
|
|
||
| package envoy.config.response_map.v3; | ||
|
|
||
| import "envoy/config/accesslog/v3/accesslog.proto"; | ||
| import "envoy/config/core/v3/base.proto"; | ||
| import "envoy/config/core/v3/substitution_format_string.proto"; | ||
|
|
||
| import "google/protobuf/struct.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.config.response_map.v3"; | ||
| option java_outer_classname = "ResponseMapProto"; | ||
| option java_multiple_files = true; | ||
| option (udpa.annotations.file_status).package_version_status = ACTIVE; | ||
|
|
||
| // [#protodoc-title: ResponseMap] | ||
| // Response map filter :ref:`configuration overview <config_response_map>`. | ||
|
|
||
| // The configuration to filter and change HTTP responses. | ||
| // [#next-free-field: 6] | ||
| message ResponseMapper { | ||
|
Member
There was a problem hiding this comment. This isn't different from ResponseMapper in HCM, why not just use it?
Contributor
Author
There was a problem hiding this comment. The idea is to have a common config that could be shared across HCM and any other use case (like the response map http filter). I think we should deprecate the response mapper on the HCM to use this proto instead. I didn't do that here because I wasn't sure of the right/approved migration path. What do you think?
Contributor
Author
There was a problem hiding this comment. I captured my thoughts here https://docs.google.com/document/d/1THyRnepx89FbK2VVaqoEHXv2knbm_lsLx2XgDZ7FxDw and added a link to the PR description
Member
There was a problem hiding this comment. Thanks, can you give us comment access to the doc? |
||
| // Filter to determine if this mapper should apply. | ||
| accesslog.v3.AccessLogFilter filter = 1 [(validate.rules).message = {required: true}]; | ||
|
Contributor
There was a problem hiding this comment. @lizan can I ask you to do an API pass before I dig into the code?
Member
There was a problem hiding this comment. @snowp can you weigh in here re: the new matcher API fits here? I think the access log filter works well for this purpose but just want to double check.
Contributor
There was a problem hiding this comment. I would think that this could be accomplished with the new matching API, though we would most likely spend some time to reach parity in terms of supporting all the same conditionals that the access log filter supports. The matching API would offer some benefit in terms of better performance when lots of matchers are used, due to being able to structure it more as a tree than as a list of complex matchers. I think this falls into the category that we'd use the matching API if it was available today but since that work is ongoing it becomes a question of time lines. Maybe @mattklein123 has thoughts on whether we're okay with relying on the access log filters here. I could imagine the access log filter eventually being subsumed by the new matching work, but that would be a longer term thing
Contributor
There was a problem hiding this comment. Yeah, I'd initially hoped the new matcher code could be used, since using access filters just strikes me as suboptimal and I'd like the perf wins. Looks like it'd also do away with much of the shared code, so probably the way to go (if we opt to have a standalone filter for per-route) is stick with this for now, and move both the HCM and filter matchers over to the new API when it's more feature complete. |
||
|
|
||
| // The new response status code if specified. | ||
| google.protobuf.UInt32Value status_code = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}]; | ||
|
|
||
| // The new local reply text if specified. It will be used in the `%LOCAL_REPLY_BODY%` | ||
| // command operator in the `body_format`. | ||
| core.v3.DataSource body = 3; | ||
|
|
||
| // A per mapper `body_format` to use when this mapper is matched. | ||
| core.v3.SubstitutionFormatString body_format_override = 4; | ||
|
|
||
| // HTTP headers to add to a matched response. This allows the response mapper to append, to add | ||
| // or to override headers of any matched response before it is sent to a downstream client. | ||
| repeated core.v3.HeaderValueOption headers_to_add = 5 | ||
| [(validate.rules).repeated = {max_items: 1000}]; | ||
| } | ||
|
|
||
| // The configuration to customize HTTP responses read by Envoy. | ||
| message ResponseMap { | ||
|
Member
There was a problem hiding this comment. This is same as LocalReplyConfig in HCM as well?
Contributor
Author
There was a problem hiding this comment. Yep. From the comment above, I intended for there to be a common proto shared across use cases and that we should (eventually) migrate HCM to use this common type. I would appreciate thoughts/guidance on a migration strategy. |
||
| // Configuration of list of mappers which allows to filter and change HTTP response. | ||
| // The mappers will be checked by the specified order until one is matched. | ||
| repeated ResponseMapper mappers = 1; | ||
|
|
||
| // The configuration to form response body from the :ref:`command operators <config_access_log_command_operators>` | ||
| // and to specify response content type as one of: plain/text or application/json. | ||
| // | ||
| // Example one: plain/text body_format. | ||
| // | ||
| // .. code-block:: | ||
| // | ||
| // text_format: %LOCAL_REPLY_BODY%:%RESPONSE_CODE%:path=$REQ(:path)% | ||
| // | ||
| // The following response body in `plain/text` format will be generated for a request with | ||
| // local reply body of "upstream connection error", response_code=503 and path=/foo. | ||
| // | ||
| // .. code-block:: | ||
| // | ||
| // upstream connection error:503:path=/foo | ||
| // | ||
| // Example two: application/json body_format. | ||
| // | ||
| // .. code-block:: | ||
| // | ||
| // json_format: | ||
| // status: %RESPONSE_CODE% | ||
| // message: %LOCAL_REPLY_BODY% | ||
| // path: $REQ(:path)% | ||
| // | ||
| // The following response body in "application/json" format would be generated for a request with | ||
| // local reply body of "upstream connection error", response_code=503 and path=/foo. | ||
| // | ||
| // .. code-block:: json | ||
| // | ||
| // { | ||
| // "status": 503, | ||
| // "message": "upstream connection error", | ||
| // "path": "/foo" | ||
| // } | ||
| // | ||
| core.v3.SubstitutionFormatString body_format = 2; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. | ||
|
|
||
| load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| api_proto_package( | ||
| deps = [ | ||
| "//envoy/config/response_map/v3:pkg", | ||
| "@com_github_cncf_udpa//udpa/annotations:pkg", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package envoy.extensions.filters.http.response_map.v3; | ||
|
|
||
| import "envoy/config/response_map/v3/response_map.proto"; | ||
|
|
||
| import "google/protobuf/struct.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.response_map.v3"; | ||
| option java_outer_classname = "ResponseMapProto"; | ||
| option java_multiple_files = true; | ||
| option (udpa.annotations.file_status).package_version_status = ACTIVE; | ||
|
|
||
| // [#protodoc-title: ResponseMap] | ||
| // Response map filter :ref:`configuration overview <config_http_filters_response_map>`. | ||
| // [#extension: envoy.filters.http.response_map] | ||
|
|
||
| // The filter configuration for response mapping. | ||
| message ResponseMap { | ||
| // The response map to use to customize HTTP responses read by Envoy. | ||
| config.response_map.v3.ResponseMap response_map = 1 [(validate.rules).message = {required: true}]; | ||
| } | ||
|
|
||
| // Extra settings on a per virtualhost/route/weighted-cluster level. | ||
| message ResponseMapPerRoute { | ||
| oneof override { | ||
| option (validate.required) = true; | ||
|
|
||
| // Disable the response map filter for this particular vhost or route. | ||
| // If disabled is specified in multiple per-filter-configs, the most specific one will be used. | ||
| bool disabled = 1 [(validate.rules).bool = {const: true}]; | ||
|
|
||
| // Override the global configuration of the response map filter with this new config. | ||
| config.response_map.v3.ResponseMap response_map = 2 | ||
| [(validate.rules).message = {required: true}]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where this is used, shall we just relax the min_len: 1 constraint on text_format if we want an empty format string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use it here https://github.com/envoyproxy/envoy/pull/14221/files#diff-7a3f06d418ccdf0de0f51a473b6d3450baa5faf9ac8362aa6fe33bc1284b5152R27
We could just relax the constraint, but my initial idea was to avoid making changes to an existing field and figured the explicit "empty" option was clear enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would slightly prefer relax the existing one, or we can have a field
fixed_textwhich just do what PlainStringFormatterImpl does.