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

Add service-loaded extension points for channel initialization #13565

Merged
merged 16 commits into from
Nov 2, 2023

Conversation

chrisvest
Copy link
Contributor

@chrisvest chrisvest commented Aug 24, 2023

Motivation:

Netty is used by many libraries and frameworks that (reasonably) hide the fact that they use Netty under the covers. When Netty is hidden, it is hard, or sometimes impossible, to modify the channel pipelines, attributes, or options, from the outside. It might not even be clear if a framework or library makes use of Netty at all. However, we sometimes want to apply some changes, or make some checks, to most or all channels that are initialized by Netty, regardless of the framework or library that is using Netty in the given case.

Some examples of use-cases are:

  • Web application firewalls.
  • Server-side request forgery filters.
  • Intrusion detection.
  • Metrics gathering.

To address these use-cases in a way that don't require integrators to somehow find every Netty usage in a process, we introduce a service-loaded extension point that hooks into the channel initialization process.

Modification:

A new abstract class, ChannelInitializerExtension, is added. Implementations of this interface can be found and service-loaded at runtime by AbstractBootstrap, with the help of the ChannelInitializerExtensions utility class.

The ChannelInitializerExtension class offers three callback methods, one for the initialization of each of the different "kinds" of channels:

  • Server listener channels.
  • Server child channels.
  • Client channels.

The extensions are disabled by default for security reasons, and require opt in by setting the io.netty.bootstrap.extensions system property to serviceload.
To see what extensions are available, the system property can be set to log, and we will load and log (at INFO level) what extensions are available, but not actually run them.

Result:

We have extension points for use cases that are cutting-across different Netty uses, and require low-level Netty access. This makes it relatively easy to implement such use cases without requiring deep access to the Netty internals of arbitrary libraries and frameworks, and without even needing to know which libraries and frameworks are actually using Netty. The only restriction to this, is the use of shading with class-relocation, which in each case would require their own META-INF/services file. Thankfully, most libraries and frameworks that shade Netty, also offer non-shading versions.

@chrisvest
Copy link
Contributor Author

FYI @vietj

@chrisvest
Copy link
Contributor Author

@mostroverkhov @normanmaurer Addressed your comments.

@chrisvest
Copy link
Contributor Author

@mostroverkhov Did some more cleanup based on your comments.
@normanmaurer I've addressed your comments, please take a look.

*
* @return The priority.
*/
public double priority() {
Copy link
Member

Choose a reason for hiding this comment

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

  1. Curious why double instead of int/long? To allow users sneaking in any position, even if other extensions use 0 and 1.0?
  2. Usually, the recommended approach for ServiceLoader providers to implement them in a way that order doesn't matter. Even if we add priority, users can easily abuse it by always specifying highest/lowest priority for their extensions, resulting in the same undefined ordering as ServiceLoader provides by default. Consider adding it later, if there is a use-case. As we learn how users use extensions, there might be a better approach instead of a double. For example, in the future we can add smth like before(Class)/after(Class) if there is a need to execute one extension before/after another.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usually, the recommended approach for ServiceLoader providers to implement them in a way that order doesn't matter.

That is still the recommended approach, hence a default implementation is provided, returning a default value.

For example, in the future we can add smth like before(Class)/after(Class) if there is a need to execute one extension before/after another.

Such methods can create ambiguous partial orderings. A value with inherent total ordering cannot.

Copy link
Member

Choose a reason for hiding this comment

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

@idelpivnitskiy does this work for you ?

Copy link
Member

Choose a reason for hiding this comment

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

Don't have a strong opinion here, we can defer adding priorities or keep it as-is.

Such methods can create ambiguous partial orderings. A value with inherent total ordering cannot.

The total ordering requires developers to know what all other providers exist and even after they assign a priority value for their provider, it doesn't mean that the other provider can not change its priority later. With before/after it's easier to target explicit providers. For example, a security provider can always be after logging provider. On the other hand, before/after required explicit dependency on another provider to access its class. So, each approach has pros and cons.

@chrisvest
Copy link
Contributor Author

I had a discussion about this with @idelpivnitskiy and we realized that the whole deal with isApplicable and ApplicableInfo can be removed, because the post* methods have the same information and more, and at that point they can choose if they want to do something or not. Removing the "applicability check" simplified the code a fair bit.

I have also extended the use of the system property to add support for a "log" value that loads and logs what extensions are available, without actually running them. This way, operators can do dry-runs and make more informed decisions about wether to enable extensions or not.

Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

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

Thank you!

@chrisvest
Copy link
Contributor Author

I have added a commit so the DnsNameResolver and OCSP validator both add a marker attribute to the pipelines they create.
It turns out that in practice, extensions likely want to recognize pipelines and give them special treatment. At least the DNS one.

@chrisvest
Copy link
Contributor Author

@ejona86 I followed what gRPC does with the class loader, while also having what I think is a reasonable default. Please take a look.

@chrisvest
Copy link
Contributor Author

@ejona86 @normanmaurer Addressed your comments. Please take a look, thanks!

**Motivation:**

Netty is used by many libraries and frameworks that (reasonably) hide the fact that they use Netty under the covers.
When Netty is hidden, it is hard, or sometimes impossible, to modify the channel pipelines, attributes, or options, from the outside.
It might not even be clear if a framework or library makes use of Netty at all.
However, we sometimes want to apply some changes, or make some checks, to most or all channels that are initialized by Netty, regardless of the framework or library that is using Netty in the given case.

Some examples of use-cases are:
- Web application firewalls.
- Server-side request forgery filters.
- Intrusion detection.
- Metrics gathering.

To address these use-cases in a way that don't require integrators to somehow find every Netty usage in a process, we introduce a service-loaded extension point that hooks into the channel initialization process.

**Modification:**

A new interface, `ChannelInitializerExtension`, is added.
By default, implementations of this interface are found and service-loaded at runtime by `AbstractBootstrap`, with the help of hte `ChannelInitializerExtensions` utility class.

The interface offers three callback methods, one for the initialization of each of the different "kinds" of channels:
- Server listener channels.
- Server child channels.
- Client channels.

The extension also gets the opportunity to decide if it is applicable to a particular context or use-case, through the `isApplicable` method.
The method takes an `ApplicableInfo` object, which is created by `AbstractBootstrap`.
The `ApplicableInfo` is a parameter object, and a `final` class, which allow us to add information to it in the future by adding more methods to it, without breaking backwards compatibility.
To begin with, we only expose the concrete bootstrap class through the `ApplicableInfo`, so extensions can tell if they are being considered for a server, or a client usage.

By default, we service-load all extensions that are available at runtime.
However, integrators can disable the use of extensions on a per-bootstrap basis with the new `disableChannelInitializerExtensions` method.

Extensions can also be disabled for the entire JVM process by setting the `io.netty.bootstrap.extensions` system property to `empty` or `false`.
This system property is intentionally defined as a string, so we can offer more controls in the future.
For instance, we may wish to offer injecting different loader mechanisms, in which case the property could denote a class name.

**Result:**

We have extension points for use cases that are cutting-across different Netty uses, and require low-level Netty access.
This makes it relatively easy to implement such use cases without requiring deep access to the Netty internals of arbitrary libraries and frameworks, and without even needing to know which libraries and frameworks are actually using Netty.
The only restriction to this, is the use of shading with class-relocation, which in each case would require their own `META-INF/services` file.
Thankfully, most libraries and frameworks that shade Netty, also offer non-shading versions.
Also remove ApplicableInfo, and add a logging value for the system property.
This simplifies the code.
These are two cases where Netty itself might create Bootstrap instances, and extensions will likely want to be able to recognize those cases.
This would otherwise be difficult to do, since the handles used might be internal types or hidden behind channel initializers.
…ic class loader

However, also pick the loader for the (Server)Bootstrap loader as the default.
The thread context class loader will be used as a fallback if no extensions are found by the first class loader.
@chrisvest
Copy link
Contributor Author

@normanmaurer @ejona86 Rebased this. Please take a look.

Copy link
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

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

LGTM... Let's wait and see if @ejona86 has more comments

@chrisvest
Copy link
Contributor Author

Doesn't look like Eric has more comments, so I'm merging this.

@chrisvest chrisvest merged commit d2a7264 into netty:4.1 Nov 2, 2023
14 checks passed
@chrisvest chrisvest deleted the 41-ch-init-ext branch November 2, 2023 17:51
@chrisvest chrisvest added this to the 4.1.101.Final milestone Nov 2, 2023
chrisvest added a commit that referenced this pull request Nov 2, 2023
**Motivation:**

Netty is used by many libraries and frameworks that (reasonably) hide
the fact that they use Netty under the covers. When Netty is hidden, it
is hard, or sometimes impossible, to modify the channel pipelines,
attributes, or options, from the outside. It might not even be clear if
a framework or library makes use of Netty at all. However, we sometimes
want to apply some changes, or make some checks, to most or all channels
that are initialized by Netty, regardless of the framework or library
that is using Netty in the given case.

Some examples of use-cases are:
- Web application firewalls.
- Server-side request forgery filters.
- Intrusion detection.
- Metrics gathering.

To address these use-cases in a way that don't require integrators to
somehow find every Netty usage in a process, we introduce a
service-loaded extension point that hooks into the channel
initialization process.

**Modification:**

A new abstract class, `ChannelInitializerExtension`, is added.
Implementations of this interface can be found and service-loaded at
runtime by `AbstractBootstrap`, with the help of the
`ChannelInitializerExtensions` utility class.

The `ChannelInitializerExtension` class offers three callback methods,
one for the initialization of each of the different "kinds" of channels:
- Server listener channels.
- Server child channels.
- Client channels.

The extensions are disabled by default for security reasons, and require
opt in by setting the `io.netty5.bootstrap.extensions` system property to
`serviceload`.
To see what extensions are available, the system property can be set to
`log`, and we will load and log (at INFO level) what extensions are
available, but not actually run them.

**Result:**

We have extension points for use cases that are cutting-across different
Netty uses, and require low-level Netty access. This makes it relatively
easy to implement such use cases without requiring deep access to the
Netty internals of arbitrary libraries and frameworks, and without even
needing to know which libraries and frameworks are actually using Netty.
The only restriction to this, is the use of shading with
class-relocation, which in each case would require their own
`META-INF/services` file. Thankfully, most libraries and frameworks that
shade Netty, also offer non-shading versions.
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants