dynamic modules: add asyncDataSource for the module binary using local.filename#43333
Conversation
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
this should be helpful to distribute dynamic module without customize the image. cc @agrawroh @mathetake what do you think about this API? |
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
I don't remember exactly what happens there but are we going to have a same issue as Wasm remote fetch bug or are we good here? |
I think bug is bug, although we don't care wasm too much, but we can fix bug for dynamic module. The key is do we want this feature. I inclined to keep it because my previously referred reasons. But still open to listen you and @agrawroh 's suggestion. |
|
No I meant I remember there was a fundamental limitation in remote resource but maybe I am mistaken. If you see no problem then I think this is useful |
|
+1 This would be a really useful feature to have! |
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
/retest |
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
/retest |
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
Could we split the local.filename support and the remote fetcher and keep the simplest local.filename support in this PR because it would be easier to get agreement on that. |
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
you can move test/extensions/filters/http/dynamic_modules/config_test.cc test to test/extensions/dynamic_modules/http/config_test.cc because we put all tests of dynamic modules there. |
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
| // via ``local.filename``. | ||
| // | ||
| // When both ``name`` and ``module`` are set, ``module`` takes precedence. | ||
| config.core.v3.AsyncDataSource module = 6; |
There was a problem hiding this comment.
I named it module instead of source as per the patterns we have in the existing code where a datasource signifies what it is fetching. Reading dynamic_module_config.module also seemed better than dynamic_module_config.source which might mean configSource and not moduleSource.
|
Thank you again for the review @wbpcode. Apologies that I wasn't able to understand that we wanted to split out remote mode from this PR. |
I think this PR is cleanup and all previous comments from Takeshi were addressed
…inary (#43818) **Commit Message**: dynamic_modules: add remote HTTP support for asyncDataSource module binary **Additional Description**: Extends the asyncDataSource support (added in #43333 for local.filename) to support fetching dynamic module binaries from remote HTTP sources via module.remote in DynamicModuleConfig. The module is downloaded asynchronously during listener initialization with SHA256 verification, written to a temporary file, and loaded via dlopen. If the remote fetch fails, the filter is not installed and requests pass through (fail-open). **Risk Level**: Low **Testing**: Added unit tests + Manually tested **Docs Changes**: API docs already a part of this PR **Release Notes**: Added --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…l.filename (envoyproxy#43333) **Commit Message**: dynamic modules: add asyncDataSource for the module binary using local.filename **Additional Description**: The PR adds support for fetching dynamic module binary from one of the RemoteDataSource. This currently implements local.filename support. Remote fetching will be added in follow up PRs. **Risk Level**: Low **Testing**: Added unit tests + Manually tested different scenarios. **Docs Changes**: API docs already a part of this PR **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --- The API usage is like below: ``` http_filters: - name: envoy.extensions.filters.http.dynamic_modules typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_modules.v3.DynamicModuleFilter filter_name: "hello" filter_config: "@type": "type.googleapis.com/google.protobuf.StringValue" value: | dynamic_module_config: module: local: filename: "/path/to/libhello.so" do_not_close: true ``` *AI Disclaimer*: Some of the UTs have been generated using Claude Code --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: bjmask <11672696+bjmask@users.noreply.github.com>
…inary (envoyproxy#43818) **Commit Message**: dynamic_modules: add remote HTTP support for asyncDataSource module binary **Additional Description**: Extends the asyncDataSource support (added in envoyproxy#43333 for local.filename) to support fetching dynamic module binaries from remote HTTP sources via module.remote in DynamicModuleConfig. The module is downloaded asynchronously during listener initialization with SHA256 verification, written to a temporary file, and loaded via dlopen. If the remote fetch fails, the filter is not installed and requests pass through (fail-open). **Risk Level**: Low **Testing**: Added unit tests + Manually tested **Docs Changes**: API docs already a part of this PR **Release Notes**: Added --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: bjmask <11672696+bjmask@users.noreply.github.com>
…l.filename (envoyproxy#43333) **Commit Message**: dynamic modules: add asyncDataSource for the module binary using local.filename **Additional Description**: The PR adds support for fetching dynamic module binary from one of the RemoteDataSource. This currently implements local.filename support. Remote fetching will be added in follow up PRs. **Risk Level**: Low **Testing**: Added unit tests + Manually tested different scenarios. **Docs Changes**: API docs already a part of this PR **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --- The API usage is like below: ``` http_filters: - name: envoy.extensions.filters.http.dynamic_modules typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_modules.v3.DynamicModuleFilter filter_name: "hello" filter_config: "@type": "type.googleapis.com/google.protobuf.StringValue" value: | dynamic_module_config: module: local: filename: "/path/to/libhello.so" do_not_close: true ``` *AI Disclaimer*: Some of the UTs have been generated using Claude Code --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
**Commit Message**: dynamic_modules: remote source: add caching for fetched modules **Motivation** Follow up from #43818 and #43333 Next PR: `nack_on_cache_miss` support (NACK on cache miss + background fetch) **Additional Description**: `newDynamicModuleFromBytes` already writes fetched modules to a deterministic path based on SHA256. This PR adds a check before the async fetch: if the file already exists on disk, load it directly and skip the HTTP fetch entirely. It is a simple approach that forms base for the next PR. **Risk Level**: Low **Testing**: Added unit tests **Docs Changes**: NA **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…l.filename (envoyproxy#43333) **Commit Message**: dynamic modules: add asyncDataSource for the module binary using local.filename **Additional Description**: The PR adds support for fetching dynamic module binary from one of the RemoteDataSource. This currently implements local.filename support. Remote fetching will be added in follow up PRs. **Risk Level**: Low **Testing**: Added unit tests + Manually tested different scenarios. **Docs Changes**: API docs already a part of this PR **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --- The API usage is like below: ``` http_filters: - name: envoy.extensions.filters.http.dynamic_modules typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_modules.v3.DynamicModuleFilter filter_name: "hello" filter_config: "@type": "type.googleapis.com/google.protobuf.StringValue" value: | dynamic_module_config: module: local: filename: "/path/to/libhello.so" do_not_close: true ``` *AI Disclaimer*: Some of the UTs have been generated using Claude Code --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…inary (envoyproxy#43818) **Commit Message**: dynamic_modules: add remote HTTP support for asyncDataSource module binary **Additional Description**: Extends the asyncDataSource support (added in envoyproxy#43333 for local.filename) to support fetching dynamic module binaries from remote HTTP sources via module.remote in DynamicModuleConfig. The module is downloaded asynchronously during listener initialization with SHA256 verification, written to a temporary file, and loaded via dlopen. If the remote fetch fails, the filter is not installed and requests pass through (fail-open). **Risk Level**: Low **Testing**: Added unit tests + Manually tested **Docs Changes**: API docs already a part of this PR **Release Notes**: Added --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…yproxy#43974) **Commit Message**: dynamic_modules: remote source: add caching for fetched modules **Motivation** Follow up from envoyproxy#43818 and envoyproxy#43333 Next PR: `nack_on_cache_miss` support (NACK on cache miss + background fetch) **Additional Description**: `newDynamicModuleFromBytes` already writes fetched modules to a deterministic path based on SHA256. This PR adds a check before the async fetch: if the file already exists on disk, load it directly and skip the HTTP fetch entirely. It is a simple approach that forms base for the next PR. **Risk Level**: Low **Testing**: Added unit tests **Docs Changes**: NA **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…yproxy#43974) **Commit Message**: dynamic_modules: remote source: add caching for fetched modules **Motivation** Follow up from envoyproxy#43818 and envoyproxy#43333 Next PR: `nack_on_cache_miss` support (NACK on cache miss + background fetch) **Additional Description**: `newDynamicModuleFromBytes` already writes fetched modules to a deterministic path based on SHA256. This PR adds a check before the async fetch: if the file already exists on disk, load it directly and skip the HTTP fetch entirely. It is a simple approach that forms base for the next PR. **Risk Level**: Low **Testing**: Added unit tests **Docs Changes**: NA **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>
…yproxy#43974) **Commit Message**: dynamic_modules: remote source: add caching for fetched modules **Motivation** Follow up from envoyproxy#43818 and envoyproxy#43333 Next PR: `nack_on_cache_miss` support (NACK on cache miss + background fetch) **Additional Description**: `newDynamicModuleFromBytes` already writes fetched modules to a deterministic path based on SHA256. This PR adds a check before the async fetch: if the file already exists on disk, load it directly and skip the HTTP fetch entirely. It is a simple approach that forms base for the next PR. **Risk Level**: Low **Testing**: Added unit tests **Docs Changes**: NA **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>
…yproxy#43974) **Commit Message**: dynamic_modules: remote source: add caching for fetched modules **Motivation** Follow up from envoyproxy#43818 and envoyproxy#43333 Next PR: `nack_on_cache_miss` support (NACK on cache miss + background fetch) **Additional Description**: `newDynamicModuleFromBytes` already writes fetched modules to a deterministic path based on SHA256. This PR adds a check before the async fetch: if the file already exists on disk, load it directly and skip the HTTP fetch entirely. It is a simple approach that forms base for the next PR. **Risk Level**: Low **Testing**: Added unit tests **Docs Changes**: NA **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…l.filename (envoyproxy#43333) **Commit Message**: dynamic modules: add asyncDataSource for the module binary using local.filename **Additional Description**: The PR adds support for fetching dynamic module binary from one of the RemoteDataSource. This currently implements local.filename support. Remote fetching will be added in follow up PRs. **Risk Level**: Low **Testing**: Added unit tests + Manually tested different scenarios. **Docs Changes**: API docs already a part of this PR **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --- The API usage is like below: ``` http_filters: - name: envoy.extensions.filters.http.dynamic_modules typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_modules.v3.DynamicModuleFilter filter_name: "hello" filter_config: "@type": "type.googleapis.com/google.protobuf.StringValue" value: | dynamic_module_config: module: local: filename: "/path/to/libhello.so" do_not_close: true ``` *AI Disclaimer*: Some of the UTs have been generated using Claude Code --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…inary (envoyproxy#43818) **Commit Message**: dynamic_modules: add remote HTTP support for asyncDataSource module binary **Additional Description**: Extends the asyncDataSource support (added in envoyproxy#43333 for local.filename) to support fetching dynamic module binaries from remote HTTP sources via module.remote in DynamicModuleConfig. The module is downloaded asynchronously during listener initialization with SHA256 verification, written to a temporary file, and loaded via dlopen. If the remote fetch fails, the filter is not installed and requests pass through (fail-open). **Risk Level**: Low **Testing**: Added unit tests + Manually tested **Docs Changes**: API docs already a part of this PR **Release Notes**: Added --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…yproxy#43974) **Commit Message**: dynamic_modules: remote source: add caching for fetched modules **Motivation** Follow up from envoyproxy#43818 and envoyproxy#43333 Next PR: `nack_on_cache_miss` support (NACK on cache miss + background fetch) **Additional Description**: `newDynamicModuleFromBytes` already writes fetched modules to a deterministic path based on SHA256. This PR adds a check before the async fetch: if the file already exists on disk, load it directly and skip the HTTP fetch entirely. It is a simple approach that forms base for the next PR. **Risk Level**: Low **Testing**: Added unit tests **Docs Changes**: NA **Release Notes**: Added Platform Specific Features: NA [Optional Runtime guard:] Not needed [Optional Fixes #Issue]: NA [Optional Fixes commit #PR or SHA]: NA [Optional Deprecated:]: NA [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] NA --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Commit Message: dynamic modules: add asyncDataSource for the module binary using local.filename
Additional Description: The PR adds support for fetching dynamic module binary from one of the RemoteDataSource. This currently implements local.filename support. Remote fetching will be added in follow up PRs.
Risk Level: Low
Testing: Added unit tests + Manually tested different scenarios.
Docs Changes: API docs already a part of this PR
Release Notes: Added
Platform Specific Features: NA
[Optional Runtime guard:] Not needed
[Optional Fixes #Issue]: NA
[Optional Fixes commit #PR or SHA]: NA
[Optional Deprecated:]: NA
[Optional API Considerations:] NA
The API usage is like below:
AI Disclaimer: Some of the UTs have been generated using Claude Code