Skip to content

Conversation

@VivekSubr
Copy link
Contributor

@VivekSubr VivekSubr commented Dec 28, 2025

Commit Message:
Additional Description: Adding apis to setsockopt and getsockopt to dynamic modules abi... this is a use case un-supported by wasm filters.
Risk Level: Low, new apis
Testing: Tested manually, verified using pcap, test code is available here: https://github.com/VivekSubr/Client-Server/tree/main/proxy/envoy/dynamic_modules/dscp_filter
Docs Changes: NA? I think dynamic modules apis aren't documented function by function.
Release Notes: Added envoy_dynamic_module_callback_http_set_socket_option and envoy_dynamic_module_callback_http_get_socket_option to dynamic modules abi.

@VivekSubr VivekSubr changed the title adding set socket options in dynamic modules adding socket options apis in dynamic modules Dec 28, 2025
@agrawroh
Copy link
Member

Thanks for the contribution, @VivekSubr! You'll have to also update the ABI Version SHA and expose the RUST bindings for this change :)

/wait

@agrawroh
Copy link
Member

agrawroh commented Jan 1, 2026

@VivekSubr Looks like it's missing coverage:

FAILED: Directories not meeting coverage thresholds:
  ✗ source/extensions/filters/http/dynamic_modules: 93.5% (threshold: 95.2%)

/wait

Signed-off-by: VivekSubr <[email protected]>
Signed-off-by: VivekSubr <[email protected]>
Signed-off-by: VivekSubr <[email protected]>
@VivekSubr
Copy link
Contributor Author

VivekSubr commented Jan 2, 2026

Got all the PR checks to pass, attaching test pcap screenshot for reference,
{207B2DC9-11C4-4A7A-86A4-30DC0DDC7349}

This pcap was taken using this test code: https://github.com/VivekSubr/Client-Server/tree/main/proxy/envoy/dynamic_modules/dscp_filter

This filter, added via dynamic modules, sets http response header using 'x-dscp' header from request.

I think this is good to merge... @agrawroh

agrawroh
agrawroh previously approved these changes Jan 3, 2026
Copy link
Member

@agrawroh agrawroh left a comment

Choose a reason for hiding this comment

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

LGTM with some minor nits. Thanks!

@repokitteh-read-only repokitteh-read-only bot added api deps Approval required for changes to Envoy's external dependencies labels Jan 4, 2026
@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 @mattklein123
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 @agrawroh

🐱

Caused by: #42784 was synchronize by VivekSubr.

see: more, trace.

Signed-off-by: VivekSubr <[email protected]>
Signed-off-by: Vivek Subramanian <[email protected]>
Signed-off-by: VivekSubr <[email protected]>
@VivekSubr
Copy link
Contributor Author

rebased PR, fixed review comments... @agrawroh, @wbpcode please review and approve

@agrawroh agrawroh assigned wbpcode and unassigned mattklein123 Jan 4, 2026
@agrawroh
Copy link
Member

agrawroh commented Jan 4, 2026

cc @wbpcode Could you please take another look as Sr. Extension Maintainer?

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.

5 participants