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.

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

ext_proc: send attributes #31090

Merged

Conversation

jbohanon
Copy link
Contributor

@jbohanon jbohanon commented Nov 28, 2023

Rework of #30781 which was reverted in #31017 due to ASAN fuzzer issues

The ASAN fuzzer issues are unrelated to the net-new code here and will affect any code that calls CEL Parse.

The main differences between this and the original PR:

Commit Message:
Introduce the ability to send attributes in the External Processing Request
Additional Description:
Risk Level: Low
Testing: Integration tests for request/response attributes

Release Notes: N/A
Platform Specific Features: CEL parsing will not work on Windows

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: #31090 was opened by jbohanon.

see: more, trace.

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 @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #31090 was synchronize by jbohanon.

see: more, trace.

@jbohanon jbohanon marked this pull request as ready for review November 30, 2023 21:15
Signed-off-by: Jacob Bohanon <[email protected]>
Signed-off-by: Jacob Bohanon <[email protected]>
Signed-off-by: Jacob Bohanon <[email protected]>
@@ -221,7 +221,8 @@ void Filter::onDestroy() {
}

FilterHeadersStatus Filter::onHeaders(ProcessorState& state,
Http::RequestOrResponseHeaderMap& headers, bool end_stream) {
Http::RequestOrResponseHeaderMap& headers, bool end_stream,
ProtobufWkt::Struct* proto) {
Copy link
Contributor

@yanjunxiang-google yanjunxiang-google Jan 16, 2024

Choose a reason for hiding this comment

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

Should proto be passed by reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use nullptr to indicate that attributes are not configured. This is on Line 243

@yanjunxiang-google
Copy link
Contributor

/lgtm

Copy link

please specify a single label can be specified

🐱

Caused by: a #31090 (comment) was created by @yanjunxiang-google.

see: more, trace.

@jbohanon
Copy link
Contributor Author

It appears that the Mobile/Android check is broken on some unrelated toolchain issues

Signed-off-by: Jacob Bohanon <[email protected]>
@abeyad
Copy link
Contributor

abeyad commented Jan 16, 2024

It appears that the Mobile/Android check is broken on some unrelated toolchain issues

You're right, I'll look into this

Signed-off-by: Jacob Bohanon <[email protected]>
Copy link
Member

@tyxia tyxia left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you for your patience! Nice work

@tyxia
Copy link
Member

tyxia commented Jan 17, 2024

defer to @yanavlasov for final review and merge. PTAL, Thanks

@linsun
Copy link

linsun commented Jan 19, 2024

@yanavlasov a friendly ping for final review and merge when you get a chance? Thx!

@yanavlasov yanavlasov merged commit 6952f54 into envoyproxy:main Jan 24, 2024
53 of 55 checks passed
jbohanon added a commit to solo-io/envoy-fork that referenced this pull request Feb 28, 2024
Introduce the ability to send attributes in the External Processing Request

---------

Signed-off-by: Jacob Bohanon <[email protected]>
jbohanon added a commit to solo-io/envoy-fork that referenced this pull request Mar 1, 2024
Introduce the ability to send attributes in the External Processing Request

---------

Signed-off-by: Jacob Bohanon <[email protected]>
jbohanon added a commit to solo-io/envoy-fork that referenced this pull request Mar 12, 2024
* ext_proc: send attributes (envoyproxy#31090)

Introduce the ability to send attributes in the External Processing Request

---------

Signed-off-by: Jacob Bohanon <[email protected]>

* ext_proc: send and receive dynamic metadata (envoyproxy#30747)

Introduce the ability to send dynamic metadata in the External Processing Request. Also implements the API for returning dynamic metadata as part of the External Processing Response.

---------

Signed-off-by: Jacob Bohanon <[email protected]>

* ext_proc: revise service api for attributes (envoyproxy#32176)


---------

Signed-off-by: Jacob Bohanon <[email protected]>

---------

Signed-off-by: Jacob Bohanon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.