local reply modification: support non-string values in Substitution format string in json_format

[kfaseela]

Description:

We configure “local reply modification” to modify status codes for certain local replies (“local” means that Envoy generates these replies, not an upstream host). Documentation here: https://www.envoyproxy.io/docs/envoy/v1.22.2/configuration/http/http_conn_man/local_reply.html

We also add a body with content-type application/problem+json according to RFC 7807 (https://www.rfc-editor.org/rfc/rfc7807.html). For this, we use a “SubstitutionFormatString” in “json_format” (documented here: https://www.envoyproxy.io/docs/envoy/v1.22.2/api-v3/config/core/v3/substitution_format_string.proto#envoy-v3-api-field-config-core-v3-substitutionformatstring-json-format).

In this documentation there is an example that shows the JSON object to be created with a “status” field formatted as an integer:

                   status_code: 401
                    body_format_override:
                      json_format: 
                        title: "Bad request"
                        status: 400
                        cause: "MANDATORY_IE_INCORRECT"
                       detail: "%RESPONSE_CODE_DETAILS%"

However, when we configure a json_format with an integer value, Envoy rejects the configuration because it expects a string. If we put a string there, we get a string value in the JSON output. The RFC mentioned above specifies an integer value for the “status” field (see chapter 3.1 of the RFC 7807, and that is our problem. The screenshot below shows the error from Envoy (on envoylint.com):

We have checked the Envoy issues on Github to see if somebody else reported this problem already but did not find an existing bug. We did find however something that is related where somebody fixed a very similar issue in the access log: https://github.com/envoyproxy/envoy/issues/8374

We are using Envoy 1.22.0/1.22.2 (latest released version).

[wbpcode]

Yeah, it seems that it's a bug indeed (?). For now, only string/struct/list are supported for the Json formatter and this is not matched with the docs.

[wbpcode]

cc @mattklein123

[wbpcode]

Looks like the "%RESPONSE_CODE%" can be handled as number, and the "400" can only be treated as string.

[kfaseela]

Looks like the "%RESPONSE_CODE%" can be handled as number, and the "400" can only be treated as string.

We cannot use %RESPONSE_CODE% when modifying the local reply because we need to replace the current value of %RESPONSE_CODE% with a value of our own

[kfaseela]

@wbpcode I am not an envoy export, but if this is something easy to fix (like similar issue in the access log: https://github.com/envoyproxy/envoy/issues/8374), do you think we can add beginner label to the issue?

[ZiyangXiao]

Hi I'm a beginner of envoy, can I try this?

[kfaseela]

Hi I'm a beginner of envoy, can I try this?

Would be happy if you do so 🙂

[mattklein123]

I briefly looked at this and I don't think this is very difficult to implement, since retaining type information has already been implemented. Effectively we just need to implement support for integer values and then have a formatter for integers which spits out the correct typed integer.

Here is the relevant code to get started with: https://github.com/envoyproxy/envoy/blob/cb78092e9219da3fd77e49aa1273435696c72921/source/common/formatter/substitution_formatter.cc#L173

[ZiyangXiao]

I briefly looked at this and I don't think this is very difficult to implement, since retaining type information has already been implemented. Effectively we just need to implement support for integer values and then have a formatter for integers which spits out the correct typed integer.

Here is the relevant code to get started with:

https://github.com/envoyproxy/envoy/blob/cb78092e9219da3fd77e49aa1273435696c72921/source/common/formatter/substitution_formatter.cc#L173

Thanks for your suggestions! I'm starting learning and working on this.

[kfaseela]

Thank you so much @mattklein123 @ZiyangXiao !

[kfaseela]

/assign @ZiyangXiao

[repokitteh-read-only[bot]]

@ZiyangXiao cannot be assigned to this issue.

:cat:

Caused by: a https://github.com/envoyproxy/envoy/issues/21941#issuecomment-1188649623 was created by @kfaseela. see: [more](https://github.com/envoyproxy/envoy/issues/21941#issuecomment-1188649623), [trace](https://prod.repokitteh.app/traces/ui/envoyproxy/envoy/b6a29590-072b-11ed-8cf9-dd9a7e517188).

[kfaseela]

22396 fixes the issue.. Thanks a lot @ZiyangXiao