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

Load arbitrary interceptors in the gateway #7839

Merged
8 commits merged into from
Sep 20, 2021
Merged

Load arbitrary interceptors in the gateway #7839

8 commits merged into from
Sep 20, 2021

Conversation

npepinpe
Copy link
Member

@npepinpe npepinpe commented Sep 16, 2021 •
edited
Loading

Description

This PR allows loading arbitrary interceptors in the gateway, which will wrap every call and allow for users to specify custom logic before and after each call.

Interceptors are specified in the configuration of the gateway under zeebe.gateway.interceptors, or zeebe.broker.gateway.interceptors. They're mapped as a list, and will be called in the order in which they are defined in the configuration. So item 0 will be called first, then item 1, etc., with the actual service at the end. NOTE: the monitoring interceptor, if enabled, remains the top level one and will wrap all further interceptors.

Interceptors can be loaded from the class path, or via an external JAR by specifying their jarPath configuration property. If so, they will have an isolated class loader, which will ensure that, should they bundle dependencies that would collide with Zeebe's, there will be no collision and each side (the interceptor and the gateway) will use the right dependency.

Related issues

closes #7755

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/0.25) to the PR, in case that fails you need to create backports manually.

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually
  • The change has been verified by a QA run
  • The impact of the changes is verified by a benchmark

Documentation:

  • The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
  • New content is added to the release announcement

@npepinpe
Copy link
Member Author

npepinpe commented Sep 16, 2021 •
edited
Loading

This is a pretty big PR, honestly (though most of it is test code 😄 - and I'm still missing some tests!). I'm happy to try and split it, but I wasn't sure where would be best. I tried to still split by commits though, so you should be able to review each sort of separately. It's not completely finished, but I figured it might be good to have a look at it already.

As you can see from the test TestInterceptor, doing per message authorization is kind of a pain - I'm still not sure this is the right approach. The authorization is really expected to be per operation/call, so it's awkward. Let's discuss if this is the way to move forward (regarding authorization), or if we just add a simple internal gRPC query service which pipes the calls through the broker client (it should be easy, that's what the other QueryService is doing anyway).

NOTE: it seems blocking is fine in the listeners, so says their docs, so we might want to encourage that in the future.

To tackle asynchronous issues and the TCL, I added a InterceptorUtil which authors can use to get a context specific executor, which they can use for async code, and ensure their callbacks are called back with the right TCL. I'm happy to drop this though and push for users to block in their code 🤷‍♂️

@npepinpe npepinpe mentioned this pull request Sep 16, 2021
Closed
This was linked to issues Sep 16, 2021
Add support for custom client command authorization #7490
Closed
Add custom gRPC interceptors support #7755
Closed
Base automatically changed from np-gateway-query to develop September 16, 2021 20:26
@npepinpe npepinpe removed a link to an issue Sep 17, 2021
Add support for custom client command authorization #7490
Closed
@npepinpe npepinpe marked this pull request as ready for review September 17, 2021 09:09
@npepinpe
refactor(util): update reflection utility
f203df8 
Updates ReflectUtil#newInstance to avoid usage of deprecated APIs.
@npepinpe
refactor(util): add call with TCL
070d68e 
Adds a call method which also ensures the TCL is correctly set.
@npepinpe
refactor(broker): use ReflectUtil#newInstance
2d9dea6 
Use update ReflectUtil#newInstance instead of custom implementation.
Also updates test to make test exporter classes visible to ReflectUtil.
@npepinpe
feat(gateway): add configuration for interceptors
3cdeea5 
Adds user configuration for arbitrary interceptors as part of the
general gateway configuration. Interceptors are specified as a list of
interceptor configuration, in the order in which they will be called by
the server.
@npepinpe
feat(gateway): load arbitrary interceptors
0105464 
Loads arbitrary interceptors as part of the gateway startup, and wraps
the gateway service with them in a chain.

Interceptors can be loaded from external JARs, and will be loaded with
isolated class loaders to avoid dependency collisions.

Each interceptor is decorated with a special internal interceptor which
forwards the intercept call, ensuring that for that call, the thread
context class loader is correctly set.

An executor is provided in the context, accessible via InterceptorUtil,
which lets users use asynchronous code and pipe it back to it to ensure
their call sets the right thread context class loader.
@npepinpe
test(gateway): add unit test for interceptors
3203aa4
@npepinpe
test(gateway): add integration tests
6457f53 
Adds an integration test for interceptors loaded from the class path,
with a dummy interceptor which will deny DEPLOYMENT calls with a
specific error code and message.
menski
menski approved these changes Sep 20, 2021
View reviewed changes
Copy link
Contributor

@menski menski left a comment

Choose a reason for hiding this comment

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

looks good to me, feel free to squash and merge

@npepinpe
Copy link
Member Author

bors merge

@ghost
Copy link

ghost commented Sep 20, 2021

Build succeeded:

@ghost ghost merged commit ea10262 into develop Sep 20, 2021
@ghost ghost deleted the 7755-load-interceptors branch September 20, 2021 08:08
This was referenced Oct 7, 2021
Closed
Merged
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@menski menski menski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add custom gRPC interceptors support
2 participants
@npepinpe @menski