[ProtoApiScrubber] Initialize and store descriptor in FilterConfig to be used later for scrubbing. by sumitkmr2 · Pull Request #39815 · envoyproxy/envoy

sumitkmr2 · 2025-06-10T08:25:18Z

Commit Message: Initialize and store descriptor in FilterConfig to be used later for scrubbing.
Additional Description: N/A
Risk Level: NONE
Testing: UTs added
Docs Changes: NONE
Release Notes: N/A
Platform Specific Features: NONE
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

# Conflicts: # source/extensions/filters/http/proto_api_scrubber/BUILD # source/extensions/filters/http/proto_api_scrubber/filter_config.cc # source/extensions/filters/http/proto_api_scrubber/filter_config.h Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

# Conflicts: # source/extensions/filters/http/proto_api_scrubber/filter_config.cc # source/extensions/filters/http/proto_api_scrubber/filter_config.h # test/extensions/filters/http/proto_api_scrubber/filter_config_test.cc Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

# Conflicts: # source/extensions/filters/http/proto_api_scrubber/filter.cc # source/extensions/filters/http/proto_api_scrubber/filter.h Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

# Conflicts: # source/extensions/filters/http/proto_api_scrubber/filter.cc # source/extensions/filters/http/proto_api_scrubber/filter.h Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

repokitteh-read-only · 2025-06-10T08:25:24Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #39815 was opened by sumitkmr2.

see: more, trace.

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

# Conflicts: # test/extensions/filters/http/proto_api_scrubber/filter_test.cc Signed-off-by: Sumit Kumar <sumitkmr@google.com>

adisuissa · 2025-06-13T21:18:58Z

+    }
+    break;
+  }
+


style-nit: please remove empty lines in switch-case.

adisuissa · 2025-06-13T21:19:20Z

+    break;
+  }
+
+  default: {


style-nit: remove curly brackets when not needed.

I added those since some style-guides expect all the switch cases to be enclosed in curly braces. Removed now.

adisuissa · 2025-06-13T21:20:38Z

+    auto file_or_error = api.fileSystem().fileReadToEnd(data_source.filename());
+    if (!file_or_error.status().ok() || !descriptor_set.ParseFromString(file_or_error.value())) {
+      std::cout << "File_or_Error_Status: " << file_or_error.status() << std::endl;
+      std::cout << "File_or_Error_Status message: " << file_or_error.status().message()


Seems that this doesn't capture the case where file_or_error is valid (but ParseFromString fails).
Please fix to capture the correct error.

Done, thanks for pointing out.

adisuissa · 2025-06-13T21:21:47Z

+absl::Status ProtoApiScrubberFilterConfig::initializeDescriptorPool(
+    Api::Api& api, const ::envoy::config::core::v3::DataSource& data_source) {
+  Envoy::Protobuf::FileDescriptorSet descriptor_set;
+  auto pool = std::make_unique<Envoy::Protobuf::DescriptorPool>();


nit: move after the switch block.

adisuissa · 2025-06-13T21:26:15Z

+  }
+
+  for (const auto& file : descriptor_set.file()) {
+    pool->BuildFile(file);


Can this fail?
If so, should a failure be returned?

Yes, it can fail. Added the error handling. Thanks for pointing it out.

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

sumitkmr2

Addressed comments, thanks!

sumitkmr2 · 2025-06-16T05:53:36Z

+absl::Status ProtoApiScrubberFilterConfig::initializeDescriptorPool(
+    Api::Api& api, const ::envoy::config::core::v3::DataSource& data_source) {
+  Envoy::Protobuf::FileDescriptorSet descriptor_set;
+  auto pool = std::make_unique<Envoy::Protobuf::DescriptorPool>();


sumitkmr2 · 2025-06-16T05:58:12Z

+    auto file_or_error = api.fileSystem().fileReadToEnd(data_source.filename());
+    if (!file_or_error.status().ok() || !descriptor_set.ParseFromString(file_or_error.value())) {
+      std::cout << "File_or_Error_Status: " << file_or_error.status() << std::endl;
+      std::cout << "File_or_Error_Status message: " << file_or_error.status().message()


Done, thanks for pointing out.

sumitkmr2 · 2025-06-16T05:59:55Z

+    break;
+  }
+
+  default: {


I added those since some style-guides expect all the switch cases to be enclosed in curly braces. Removed now.

sumitkmr2 · 2025-06-16T06:24:24Z

+    }
+    break;
+  }
+


sumitkmr2 · 2025-06-16T06:24:52Z

+  }
+
+  for (const auto& file : descriptor_set.file()) {
+    pool->BuildFile(file);


Yes, it can fail. Added the error handling. Thanks for pointing it out.

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

adisuissa

LGTM, thanks.

BTW: just saw that there's a missing coverage (https://storage.googleapis.com/envoy-pr/d7f4e90/coverage/source/extensions/filters/http/proto_api_scrubber/filter.cc.html). Can you add some tests that validate that this code is WAI?

sumitkmr2 · 2025-06-17T05:15:33Z

LGTM, thanks.

BTW: just saw that there's a missing coverage (https://storage.googleapis.com/envoy-pr/d7f4e90/coverage/source/extensions/filters/http/proto_api_scrubber/filter.cc.html). Can you add some tests that validate that this code is WAI?

Yeah sure, I'll add test to cover these lines as well.

sumitkmr2 added 30 commits May 8, 2025 11:02

Add directory, class and method structure for ProtoApiScrubber filter

5bd3abe

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix formatting and update CODEOWNERS

736aa59

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add docs file

c7f3843

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add doc file in TOC

c42d95e

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix doc formatting

823133e

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix doc formatting

bd43f1f

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix filter name in docs

e2698c5

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix filter name in docs

a243458

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Remove #not-implemented-hide tag to fix the docs build break

203d808

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add integration test and simplify project skeleton code

9d91c23

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Remove unnecessary imports

65ea54a

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Addressing comments

19f5dac

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Removing documentation changes.

ef944d4

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add filter config parsing and storing logic

c6fb0c9

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Replacing exceptions by absl::Status

2c28702

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Polishing changes

e10f977

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Formatting changes

672ad11

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Remove untested dummy files

7fd7322

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Improve test coverage

5bcff55

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Improve test coverage

7e35414

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Clean up build files

a331208

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix build errors

a2b8033

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Revert build_config.bzl change

2d4662e

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Scrubbing passthrough changes

16c3cdf

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add message converter from-to envoybuffer-streamMessage

c9a830b

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add more UTs

9378196

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Merge branch 'main' into scrubbing

d1c3dee

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Refactoring filter_config.cc

e7b981f

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

sumitkmr2 added 10 commits June 9, 2025 06:37

Replace with ExceptionFreeFactoryBase

71e9957

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Address comments

7dbb3c8

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Merge branch 'config_build_fix' into scrubbing

7627692

# Conflicts: # source/extensions/filters/http/proto_api_scrubber/filter.cc # source/extensions/filters/http/proto_api_scrubber/filter.h Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Merge conflict

7999fdf

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix code formatting

bf22cde

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Address comments

121424c

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Merge branch 'main' into scrubbing

fd46146

# Conflicts: # source/extensions/filters/http/proto_api_scrubber/filter.cc # source/extensions/filters/http/proto_api_scrubber/filter.h Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add back request_message_converter_ to state

7b0c215

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Increase coverage

185c86f

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Merge branch 'scrubbing' into descriptor

e04aea6

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

sumitkmr2 added 4 commits June 11, 2025 04:24

Merge branch 'main' into descriptor

bc994a6

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Try fixing spell error

0dade8b

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Fix tests

86ce72e

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Cleanup code

8395a3a

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

sumitkmr2 mentioned this pull request Jun 12, 2025

ProtoApiScrubber: Add support to handle streaming requests #39681

Merged

sumitkmr2 marked this pull request as ready for review June 12, 2025 07:50

sumitkmr2 requested a review from adisuissa as a code owner June 12, 2025 07:50

Merge branch 'main' into descriptor

bd52e8b

# Conflicts: # test/extensions/filters/http/proto_api_scrubber/filter_test.cc Signed-off-by: Sumit Kumar <sumitkmr@google.com>

KBaichoo assigned adisuissa Jun 13, 2025

adisuissa reviewed Jun 13, 2025

View reviewed changes

Address comments

ead0a37

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

sumitkmr2 commented Jun 16, 2025

View reviewed changes

sumitkmr2 added 4 commits June 16, 2025 06:39

Add tests

186d9df

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add more tests

a07a1c4

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Update error message

a8f3e10

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

Add test for build file failure

d7f4e90

Signed-off-by: Sumit Kumar <sumitkmr@google.com>

adisuissa approved these changes Jun 16, 2025

View reviewed changes

adisuissa merged commit 48a242c into envoyproxy:main Jun 16, 2025

Uh oh!

Conversation

sumitkmr2 commented Jun 10, 2025

Uh oh!

repokitteh-read-only Bot commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sumitkmr2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

sumitkmr2 commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants