Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GrpcFieldExtraction: Supports extracting fields of type map<string, string> in addition to string #35162

Merged
merged 4 commits into from
Aug 13, 2024
Merged

GrpcFieldExtraction: Supports extracting fields of type map<string, string> in addition to string #35162

merged 4 commits into from
Aug 13, 2024

Conversation

sissisuna
Copy link
Contributor

@sissisuna sissisuna commented Jul 12, 2024 •
edited
Loading

Commit Message: GrpcFieldExtraction: Supports extracting fields of type map<string, string> in addition to string
Additional Description:
Risk Level: Low
Testing: Unit test
Docs Changes: Inline with the filter API proto.
Release Notes: This change is backward compatible and no behavior change is expected for existing users.
Platform Specific Features:

@repokitteh-read-only RepoKitteh - Read Only
Copy link

Hi @sissisuna, 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: #35162 was opened by sissisuna.

see: more, trace.

@repokitteh-read-only RepoKitteh - Read Only
Copy link

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: #35162 was opened by sissisuna.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added api deps Approval required for changes to Envoy's external dependencies labels Jul 12, 2024
@repokitteh-read-only RepoKitteh - Read Only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @abeyad
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @mattklein123

🐱

Caused by: #35162 was opened by sissisuna.

see: more, trace.

@sissisuna
Copy link
Contributor Author

/retest

@sissisuna sissisuna marked this pull request as ready for review July 15, 2024 18:52
mattklein123
mattklein123 requested changes Jul 16, 2024
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM. Please add a release note, thanks.

/wait

@sissisuna
Copy link
Contributor Author

sissisuna commented Jul 16, 2024 •
edited
Loading

LGTM. Please add a release note, thanks.

/wait

@mattklein123 Thanks for reviewing the change! The release notes are already part of the description as "Release Notes: This change is backward compatible and no behavior change is expected for existing users.", can you please let me know if anything else is missing or if I need to add it to an additional place?

@sissisuna
GrpcFieldExtraction: Supports extracting fields of type `map<string, …
d300abe 
…string>` in addition to string.

Risk Level: Low
Testing: Unit tests
Docs Changes: Inline with the filter API proto.
Release Notes: This change is backward compatible and no behavior change is expected for existing users.

Signed-off-by: Xi Wu <xiwuxw@google.com>
@sissisuna
Copy link
Contributor Author

/retest

2 similar comments
@sissisuna
Copy link
Contributor Author

/retest

@sissisuna
Copy link
Contributor Author

/retest

abeyad
abeyad previously approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@abeyad abeyad left a comment

Choose a reason for hiding this comment

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

/lgtm api

mattklein123
mattklein123 requested changes Jul 29, 2024
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

You need to add a release note here: https://github.com/envoyproxy/envoy/blob/main/changelogs/current.yaml

Please look at other changes and the contributing guide to learn more.

/wait

@sissisuna
Merge remote-tracking branch 'upstream/main' into gfx
ef7ee84 
Signed-off-by: Xi Wu <xiwuxw@google.com>
@sissisuna
Copy link
Contributor Author

/retest

@sissisuna
Copy link
Contributor Author

/retest

1 similar comment
@sissisuna
Copy link
Contributor Author

/retest

mattklein123
mattklein123 previously approved these changes Aug 7, 2024
View reviewed changes
@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Aug 7, 2024
@mattklein123
Copy link
Member

/retest

@mattklein123 mattklein123 enabled auto-merge (squash) August 7, 2024 14:43
@sissisuna
Copy link
Contributor Author

/retest

@mattklein123
Copy link
Member

Sorry please merge main.

/wait

@sissisuna
Merge remote-tracking branch 'upstream/main' into gfx
2a3c5c9 
Signed-off-by: Xi Wu <xiwuxw@google.com>
auto-merge was automatically disabled August 12, 2024 23:29

Head branch was pushed to by a user without write access

@repokitteh-read-only repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Aug 12, 2024
@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Aug 13, 2024
@mattklein123 mattklein123 merged commit 1cd67c5 into envoyproxy:main Aug 13, 2024
48 of 50 checks passed
asingh-g pushed a commit to asingh-g/envoy that referenced this pull request Aug 20, 2024
@sissisuna @asingh-g
GrpcFieldExtraction: Supports extracting fields of type `map<string, …
bb4517e 
…string>` in addition to string (envoyproxy#35162)

Commit Message: GrpcFieldExtraction: Supports extracting fields of type
`map<string, string>` in addition to string
Additional Description:
Risk Level: Low
Testing: Unit test
Docs Changes: Inline with the filter API proto.
Release Notes: This change is backward compatible and no behavior change
is expected for existing users.
Platform Specific Features:

---------

Signed-off-by: Xi Wu <xiwuxw@google.com>
Signed-off-by: asingh-g <abhisinghx@google.com>
@botengyao botengyao mentioned this pull request Aug 26, 2024
Merged
wbpcode pushed a commit that referenced this pull request Aug 26, 2024
@botengyao
grpc_field_extraction: fix empty ListValue indication (#35842)
77357ca 
<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
envoy-security@googlegroups.com where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->


ProtobufWkt::Value is introduced by #35162

Commit Message:
Additional Description:
Risk Level: low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:

Signed-off-by: Boteng Yao <boteng@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@abeyad abeyad abeyad left review comments

@yanavlasov yanavlasov Awaiting requested review from yanavlasov yanavlasov is a code owner

Assignees

@abeyad abeyad

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sissisuna @mattklein123 @abeyad