deps: Bump protobuf and grpc#42435
Conversation
Signed-off-by: Jonh Wendell <jwendell@redhat.com>
|
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to |
| # Following attribute will be enabled once the filter is registered. | ||
| # extension_names = ["envoy.filters.http.proto_api_scrubber"], | ||
| rbe_pool = "6gig", | ||
| # FIXME This test is failing with newer protobuf |
There was a problem hiding this comment.
Do you know the cause of failure? I want to make sure we are not breaking this filter.
@sumitkmr2 @pranavrautgoogle can you help diagnosing the test failure with the new protobuf, please?
There was a problem hiding this comment.
It's failing here: https://github.com/envoyproxy/envoy/blob/main/test/extensions/filters/http/grpc_field_extraction/message_converter/message_converter_test_lib.h#L36
I was hoping that @sumitkmr2 (the author of that test) could help on this.
There was a problem hiding this comment.
I've pushed a fix to this test and removed the FIXME.
There was a problem hiding this comment.
Apologies, I was on leave. The fix looks good to me, thanks @jwendell for the fix!
| } | ||
|
|
||
| - if (in_->BytesAvailable() < static_cast<pb::int64>(current_message_size_)) { | ||
| + if (in_->BytesAvailable() < static_cast<int64_t>(current_message_size_)) { |
There was a problem hiding this comment.
Yes, I can go ahead and make this PR upstream today
|
/wait-any |
Protobuf 33.1 changed DebugString() to produce unparseable output (random prefixes, whitespace) for security reasons. The test was using DebugString() to serialize a matcher proto then parsing it with TextFormat::ParseFromString(), which now fails silently. Replace DebugString() with TextFormat::PrintToString() which produces proper text proto format. Signed-off-by: Jonh Wendell <jwendell@redhat.com>
|
ping @envoyproxy/envoy-maintainers |
|
@yanavlasov i think this waiting for further review |
agrawroh
left a comment
There was a problem hiding this comment.
It looks good to me. I'm okay taking a patch in grpc_httpjson_transcoding until it gets upstreamed (we can have a tracking ticket).
Will let @yanavlasov give a final approval.
Signed-off-by: Jonh Wendell <jwendell@redhat.com> Signed-off-by: dinesh-murugiah <dinesh.murugiah@freshworks.com>
…37 behavior under Protobuf 30+) (#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in #42435 and #42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
…37 behavior under Protobuf 30+) (envoyproxy#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in envoyproxy#42435 and envoyproxy#42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit envoyproxy#43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> (cherry picked from commit 697e419) Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
…37 behavior under Protobuf 30+) (#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in #42435 and #42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> (cherry picked from commit 697e419) Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
…37 behavior under Protobuf 30+) (envoyproxy#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in envoyproxy#42435 and envoyproxy#42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit envoyproxy#43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: bjmask <11672696+bjmask@users.noreply.github.com>
…37 behavior under Protobuf 30+) (envoyproxy#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in envoyproxy#42435 and envoyproxy#42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit envoyproxy#43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Jonh Wendell <jwendell@redhat.com> Signed-off-by: Gustavo <grnmeira@gmail.com>
…37 behavior under Protobuf 30+) (envoyproxy#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in envoyproxy#42435 and envoyproxy#42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit envoyproxy#43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
…37 behavior under Protobuf 30+) (envoyproxy#43508) Commit Message: Envoy 1.37.0 updated protobuf from 29.3 to 33.2. That dependency change altered debug-string output semantics and broke behavior that worked in 1.36.x for ext_proc consumers parsing `xds.virtual_host_metadata`. This behavior change was introduced in envoyproxy#42435 and envoyproxy#42827. This PR switches message serialization from debug-string APIs to protobuf text-format APIs so message-valued CEL attributes are machine-parseable again, as recommended by protobuf programming guides. https://protobuf.dev/programming-guides/deserialize-debug **What broke in 1.37.0** - In both 1.36.x and 1.37.0, message-valued CEL results were stringified via ShortDebugString(). - In 1.36.x (protobuf 29.3), that output was parseable. - In 1.37.0 (protobuf 33.2), debug strings are now prefixed with `goo.gle/debugonly` / `goo.gle/debugstr`, which is intentionally non-parseable as textproto. - Result: ext_proc implementations that parse `xds.virtual_host_metadata` as textproto started failing after upgrading to 1.37.0. **Note**: xds metadata attributes in ext_proc requests are **not diagnostic log fields**; they are machine-to-machine payload data consumed by external processors. External processors parse these values to make policy and routing decisions, so serialization must be stable and machine-readable by standard protobuf tooling. `DebugString` is a diagnostic format with no compatibility contract, and in protobuf 30+ it may include `goo.gle/debug...` prefixes that intentionally break parsing. For this path, protobuf `TextFormat` is the correct format, not debug serialization. **Example** Given virtual host metadata: ```yaml { "metadata": { "filterMetadata": { "envoy-gateway": { "resources": [ { "kind": "Gateway", "name": "eg", "namespace": "default", "sectionName": "http" } ] } } } } ``` Before (1.37.0 + protobuf 30+ path), ext_proc may receive: `goo.gle/debugonly filter_metadata { key: "envoy-gateway" value { ... } }` Parsing fails at token 1. After this PR: `filter_metadata { key: "envoy-gateway" value { ... } }` This is protobuf text format and parses via TextFormat::ParseFromString(...). **Compatibility notes** - This restores 1.36.x-like effective behavior for ext_proc users (parseable metadata strings), while using the correct serialization API. - Minor formatting differences vs 1.36.x debug output are possible (whitespace/order), but output is now stable machine-readable textproto. - Scope includes other call sites using Expr::print(...) for message-valued CEL results (for example rate-limit descriptor CEL stringification). Additional Description: Risk Level: low Testing: unit and integration test Docs Changes: Release Notes: yes Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit envoyproxy#43466] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
No description provided.