dynamic_modules: adds initial object loading logic #35550
Conversation
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
| /** | ||
| * A class for loading and managing dynamic modules. This corresponds to a single dlopen handle. | ||
| * When the DynamicModule object is destroyed, the dlopen handle is closed. | ||
| * | ||
| * This class is supposed to be initialized once in the main thread and can be shared with other | ||
| * threads. | ||
| */ | ||
| class DynamicModule { |
There was a problem hiding this comment.
note: this should be utilized in contrib/golang, and relatively easy, but then the changes in this patch end up across many places, so I chose not to do that right now. - I could take a shot after this PR.
| @@ -345,8 +346,8 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123 | |||
| /*/extensions/filters/http/match_delegate @wbpcode @jstraceski @tyxia | |||
| # Generic proxy and related extensions | |||
| /*/extensions/filters/network/generic_proxy/ @wbpcode @soulxu | |||
|
|
|||
| /*/extensions/health_checkers/common @zuercher @botengyao | |||
There was a problem hiding this comment.
moved this up as it looked like it was misplaced
|
/assign @mattklein123 |
There was a problem hiding this comment.
This looks great and as it stands doesn't pose a risk (yet) as it isn't compiled into the final Envoy build at this time. 👍 to the approach of adding this little by little and starting with the initial object loading logic as the foundation for future PRs which will include code from https://github.com/mathetake/envoy-dynamic-modules. I really like the approach and simplicity of the ABI as defined in https://github.com/mathetake/envoy-dynamic-modules/tree/main/abi.
I didn't want to approve without this being reviewed by @mattklein123 who is the one assigned to this pull request.
| /** | ||
| * A class for loading and managing dynamic modules. This corresponds to a single dlopen handle. | ||
| * When the DynamicModule object is destroyed, the dlopen handle is closed. | ||
| * | ||
| * This class is supposed to be initialized once in the main thread and can be shared with other | ||
| * threads. | ||
| */ | ||
| class DynamicModule { |
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
|
/retest |
|
(CI is flaky as usual for unrelated reasons - the same coverage upload failure is happening to other PRs) |
Commit Message: dynamic_modules: adds initial object loading logic Additional Description: This is the very first commit of the dynamic loading feature discussed among community members. This is the effort to upstream the playground repository https://github.com/mathetake/envoy-dynamic-modules as an Envoy core extension. Series of commits will follow this little by little. envoyproxy#2053, envoyproxy#24230, envoyproxy#32502 Risk Level: N/A (not compiled into the final build yet) Testing: unit Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> Signed-off-by: Martin Duke <martin.h.duke@gmail.com>
Commit Message: dynamic_modules: adds initial object loading logic Additional Description: This is the very first commit of the dynamic loading feature discussed among community members. This is the effort to upstream the playground repository https://github.com/mathetake/envoy-dynamic-modules as an Envoy core extension. Series of commits will follow this little by little. envoyproxy#2053, envoyproxy#24230, envoyproxy#32502 Risk Level: N/A (not compiled into the final build yet) Testing: unit Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> Signed-off-by: asingh-g <abhisinghx@google.com>
| absl::StatusOr<DynamicModuleSharedPtr> result = newDynamicModule("invalid_name", false); | ||
| EXPECT_FALSE(result.ok()); | ||
| EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument); | ||
| } |
There was a problem hiding this comment.
Just drive by review: you should add a test which loads a file which exists, but is not a valid library.
Commit Message: dynamic_modules: adds initial object loading logic
Additional Description:
This is the very first commit of the dynamic loading feature discussed among community
members. This is the effort to upstream the playground repository
https://github.com/mathetake/envoy-dynamic-modules as an Envoy core extension.
Series of commits will follow this little by little.
#2053, #24230, #32502
Risk Level: N/A (not compiled into the final build yet)
Testing: unit
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]