Skip to content

Filter out unknown fields in ProtoApiScrubber #43315

Merged
adisuissa merged 23 commits into
envoyproxy:mainfrom
nickshokri:unknown_field_filter
Mar 25, 2026
Merged

Filter out unknown fields in ProtoApiScrubber #43315
adisuissa merged 23 commits into
envoyproxy:mainfrom
nickshokri:unknown_field_filter

Conversation

@nickshokri
Copy link
Copy Markdown
Contributor

Commit Message: Add an unknown field filter in the ProtoApiScrubber's CheckField() method. A max depth cap of 100 was also added to prevent stack overflow.
Additional Description: Note that AI was used for creating the unit tests
Risk Level: Low
Testing: Added unit tests and confirmed they pass
Docs Changes: Added to ProtoApiScrubber documentation the existence of an unknown field filter
Release Notes: Added section under "minor changes"
Platform Specific Features: N/A

@nickshokri nickshokri requested a review from adisuissa as a code owner February 3, 2026 20:42
@repokitteh-read-only
Copy link
Copy Markdown

Hi @nickshokri, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #43315 was opened by nickshokri.

see: more, trace.

Comment thread docs/root/configuration/http/http_filters/proto_api_scrubber_filter.rst Outdated
@repokitteh-read-only
Copy link
Copy Markdown

@sumitkmr2 cannot be assigned to this issue.

🐱

Caused by: a #43315 (comment) was created by @adisuissa.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

nickshokri is not allowed to assign users.

🐱

Caused by: a #43315 (comment) was created by @nickshokri.

see: more, trace.

@nickshokri nickshokri requested a review from adisuissa February 5, 2026 21:51
@yanavlasov
Copy link
Copy Markdown
Contributor

@nickshokri please merge main

@sumitkmr2 please also review the change

/wait

Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

a couple of docs nits

Comment thread docs/root/configuration/http/http_filters/proto_api_scrubber_filter.rst Outdated
@RyanTheOptimist
Copy link
Copy Markdown
Contributor

@phlax friendly ping

@nickshokri nickshokri temporarily deployed to external-contributors March 4, 2026 23:28 — with GitHub Actions Inactive
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #43315 was synchronize by nickshokri.

see: more, trace.

Comment thread changelogs/current.yaml Outdated
Comment thread docs/root/configuration/http/http_filters/proto_api_scrubber_filter.rst Outdated
@nickshokri nickshokri had a problem deploying to external-contributors March 5, 2026 22:07 — with GitHub Actions Error
@nickshokri nickshokri had a problem deploying to external-contributors March 5, 2026 22:15 — with GitHub Actions Error
@nickshokri nickshokri had a problem deploying to external-contributors March 5, 2026 22:17 — with GitHub Actions Error
@nickshokri nickshokri had a problem deploying to external-contributors March 5, 2026 22:20 — with GitHub Actions Error
@nickshokri nickshokri requested a review from phlax March 5, 2026 22:21
@agrawroh
Copy link
Copy Markdown
Member

cc @adisuissa Gentle ping on this for a review.

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks!
Overall LGTM.
/lgtm api
Please consider adding an integration test that will show what are the expected inputs and outputs in the case of scrubbing.

Comment thread changelogs/current.yaml Outdated
@nickshokri
Copy link
Copy Markdown
Contributor Author

@adisuissa friendly ping on this. I'm hoping to get this in by EOW.

adisuissa
adisuissa previously approved these changes Mar 17, 2026
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@adisuissa
Copy link
Copy Markdown
Contributor

Please fix DCO

Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: nick <nickshokri@google.com>
…to unknown_field_filter

Signed-off-by: nick <nickshokri@google.com>
@nickshokri nickshokri had a problem deploying to external-contributors March 24, 2026 18:17 — with GitHub Actions Error
Signed-off-by: nick <nickshokri@google.com>
auto-merge was automatically disabled March 24, 2026 18:47

Head branch was pushed to by a user without write access

@nickshokri nickshokri had a problem deploying to external-contributors March 24, 2026 18:47 — with GitHub Actions Error
@nickshokri nickshokri requested a review from adisuissa March 24, 2026 18:48
Signed-off-by: nick <nickshokri@google.com>
@nickshokri nickshokri temporarily deployed to external-contributors March 24, 2026 19:25 — with GitHub Actions Inactive
adisuissa
adisuissa previously approved these changes Mar 24, 2026
@adisuissa adisuissa enabled auto-merge (squash) March 24, 2026 19:35
Signed-off-by: nick <nickshokri@google.com>
auto-merge was automatically disabled March 24, 2026 20:06

Head branch was pushed to by a user without write access

@nickshokri nickshokri had a problem deploying to external-contributors March 24, 2026 20:07 — with GitHub Actions Error
@nickshokri nickshokri temporarily deployed to external-contributors March 24, 2026 20:07 — with GitHub Actions Inactive
@adisuissa adisuissa enabled auto-merge (squash) March 25, 2026 13:11
@adisuissa adisuissa merged commit 80754fe into envoyproxy:main Mar 25, 2026
31 of 33 checks passed
fishcakez pushed a commit to fishcakez/envoy that referenced this pull request Mar 25, 2026
…voyproxy#43315)

Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: Kuat <kyessenov@users.noreply.github.com>
Co-authored-by: Kuat <kyessenov@users.noreply.github.com>
TAOXUY pushed a commit to TAOXUY/envoy that referenced this pull request Apr 1, 2026
…voyproxy#43315)

Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: Kuat <kyessenov@users.noreply.github.com>
Co-authored-by: Kuat <kyessenov@users.noreply.github.com>
Signed-off-by: Xuyang Tao <taoxuy@google.com>
citrus7 pushed a commit to citrus7/envoy that referenced this pull request Apr 1, 2026
…voyproxy#43315)

Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: Kuat <kyessenov@users.noreply.github.com>
Co-authored-by: Kuat <kyessenov@users.noreply.github.com>
Signed-off-by: Jonathan Wu <jtwu@google.com>
nshipilov pushed a commit to nshipilov/envoy that referenced this pull request Apr 13, 2026
…voyproxy#43315)

Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: Kuat <kyessenov@users.noreply.github.com>
Co-authored-by: Kuat <kyessenov@users.noreply.github.com>
Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>
krinkinmu pushed a commit to grnmeira/envoy that referenced this pull request Apr 20, 2026
…voyproxy#43315)

Signed-off-by: nick <nickshokri@google.com>
Signed-off-by: Kuat <kyessenov@users.noreply.github.com>
Co-authored-by: Kuat <kyessenov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants