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

KAFKA-16437 - Upgrade to Jakarta and Jetty 12 (KIP-1032) #16754

Merged
merged 14 commits into from
Dec 11, 2024
Merged

KAFKA-16437 - Upgrade to Jakarta and Jetty 12 (KIP-1032) #16754

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5a191a0
KAFKA-16437 - Upgrade to Jakarta and Jetty 12.x
cshannon Oct 16, 2024
96d2eac
Fix SSLUtilsTest
cshannon Oct 16, 2024
fafc05c
Merge branch 'trunk' into KAFKA-16437
cshannon Oct 17, 2024
fa0910c
Merge branch 'trunk' into KAFKA-16437
cshannon Oct 22, 2024
d108f99
Merge branch 'trunk' into KAFKA-16437
cshannon Nov 11, 2024
ed76855
Merge branch 'trunk' into KAFKA-16437
cshannon Nov 26, 2024
acad443
Merge branch 'trunk' into KAFKA-16437
cshannon Dec 4, 2024
ab01616
Bump jetty to 12.0.15
cshannon Dec 4, 2024
b3f5c11
Address PR comments
cshannon Dec 5, 2024
d35bec7
Merge branch 'trunk' into KAFKA-16437
cshannon Dec 5, 2024
2cc3897
empty commit to run tests
cshannon Dec 9, 2024
d2688d6
Merge branch 'trunk' into KAFKA-16437
cshannon Dec 9, 2024
2652325
address comments
cshannon Dec 10, 2024
4a9aff5
empty commit to re-run tests
cshannon Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 42 additions & 20 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,7 @@ project(':generator') {
implementation libs.argparse4j
implementation libs.jacksonDatabind
implementation libs.jacksonJDK8Datatypes
implementation libs.jacksonJaxrsJsonProvider
implementation libs.jacksonJakartarsJsonProvider

implementation 'org.eclipse.jgit:org.eclipse.jgit:6.4.0.202211300538-r'
// SSH support for JGit based on Apache MINA sshd
Expand Down Expand Up @@ -1865,7 +1865,7 @@ project(':clients') {
compileOnly libs.jose4j // for SASL/OAUTHBEARER JWT validation; only used by broker

testImplementation libs.bcpkix
testImplementation libs.jacksonJaxrsJsonProvider
testImplementation libs.jacksonJakartarsJsonProvider
testImplementation libs.jose4j
testImplementation libs.junitJupiter
testImplementation libs.reload4j
Expand Down Expand Up @@ -2428,7 +2428,7 @@ project(':tools') {
implementation libs.joptSimple

implementation libs.jose4j // for SASL/OAUTHBEARER JWT validation
implementation libs.jacksonJaxrsJsonProvider
implementation libs.jacksonJakartarsJsonProvider

testImplementation project(':clients')
testImplementation project(':clients').sourceSets.test.output
Expand Down Expand Up @@ -2497,14 +2497,20 @@ project(':trogdor') {
implementation libs.slf4jApi
runtimeOnly libs.reload4j

implementation libs.jacksonJaxrsJsonProvider
implementation libs.jacksonJakartarsJsonProvider
implementation libs.jerseyContainerServlet
implementation libs.jerseyHk2
implementation libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
implementation libs.activation // Jersey dependency that was available in the JDK before Java 9
implementation libs.jettyServer
implementation libs.jettyServlet
implementation libs.jettyServlets
implementation (libs.jettyServer) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyServlet) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyServlets) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}

implementation project(':group-coordinator')
implementation project(':group-coordinator:group-coordinator-api')
Expand Down Expand Up @@ -2554,7 +2560,7 @@ project(':shell') {
implementation project(':raft')

implementation libs.jose4j // for SASL/OAUTHBEARER JWT validation
implementation libs.jacksonJaxrsJsonProvider
implementation libs.jacksonJakartarsJsonProvider

testImplementation project(':clients')
testImplementation project(':clients').sourceSets.test.output
Expand Down Expand Up @@ -3378,7 +3384,7 @@ project(':connect:api') {
api project(':clients')
implementation libs.slf4jApi
runtimeOnly libs.reload4j
implementation libs.jaxrsApi
implementation libs.jakartaRsApi

testImplementation libs.junitJupiter
testImplementation project(':clients').sourceSets.test.output
Expand Down Expand Up @@ -3501,15 +3507,23 @@ project(':connect:runtime') {
implementation libs.reload4j
implementation libs.jose4j // for SASL/OAUTHBEARER JWT validation
implementation libs.jacksonAnnotations
implementation libs.jacksonJaxrsJsonProvider
implementation libs.jacksonJakartarsJsonProvider
implementation libs.jerseyContainerServlet
implementation libs.jerseyHk2
implementation libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
implementation libs.activation // Jersey dependency that was available in the JDK before Java 9
implementation libs.jettyServer
implementation libs.jettyServlet
implementation libs.jettyServlets
implementation libs.jettyClient
implementation (libs.jettyServer) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyServlet) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyServlets) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyClient) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation libs.classgraph
implementation libs.mavenArtifact
implementation libs.swaggerAnnotations
Expand Down Expand Up @@ -3672,7 +3686,7 @@ project(':connect:basic-auth-extension') {
implementation project(':connect:api')
implementation libs.slf4jApi
runtimeOnly libs.reload4j
implementation libs.jaxrsApi
implementation libs.jakartaRsApi
implementation libs.jaxAnnotationApi

testImplementation libs.bcpkix
Expand Down Expand Up @@ -3718,15 +3732,23 @@ project(':connect:mirror') {
implementation libs.slf4jApi
runtimeOnly libs.reload4j
implementation libs.jacksonAnnotations
implementation libs.jacksonJaxrsJsonProvider
implementation libs.jacksonJakartarsJsonProvider
implementation libs.jerseyContainerServlet
implementation libs.jerseyHk2
implementation libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
implementation libs.activation // Jersey dependency that was available in the JDK before Java 9
implementation libs.jettyServer
implementation libs.jettyServlet
implementation libs.jettyServlets
implementation libs.jettyClient
implementation (libs.jettyServer) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyServlet) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyServlets) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation (libs.jettyClient) {
exclude group: 'org.slf4j', module: 'slf4j-api'
}
implementation libs.swaggerAnnotations

testImplementation libs.junitJupiter
Expand Down
24 changes: 12 additions & 12 deletions checkstyle/import-control.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,8 @@

<subpackage name="trogdor">
<allow pkg="com.fasterxml.jackson" />
<allow pkg="javax.servlet" />
<allow pkg="javax.ws.rs" />
<allow pkg="jakarta.servlet" />
<allow pkg="jakarta.ws.rs" />
<allow pkg="net.sourceforge.argparse4j" />
<allow pkg="org.apache.kafka.clients" />
<allow pkg="org.apache.kafka.clients.admin" />
Expand Down Expand Up @@ -528,7 +528,7 @@

<subpackage name="rest">
<allow pkg="org.apache.kafka.connect.health" />
<allow pkg="javax.ws.rs" />
<allow pkg="jakarta.ws.rs" />
<allow pkg= "javax.security.auth"/>
<subpackage name="basic">
<allow pkg="org.apache.kafka.connect.rest"/>
Expand All @@ -554,8 +554,8 @@
<allow pkg="org.apache.kafka.server.config" />
<allow pkg="kafka.server" />
<subpackage name="rest">
<allow pkg="javax.ws.rs" />
<allow pkg="javax.inject" />
<allow pkg="jakarta.ws.rs" />
<allow pkg="jakarta.inject" />
<allow pkg="org.glassfish.jersey" />
<allow pkg="org.glassfish.hk2" />
</subpackage>
Expand All @@ -570,10 +570,10 @@

<subpackage name="rest">
<allow pkg="org.eclipse.jetty" />
<allow pkg="javax.ws.rs" />
<allow pkg="javax.inject" />
<allow pkg="jakarta.ws.rs" />
<allow pkg="jakarta.inject" />
<allow pkg="org.glassfish.hk2" />
<allow pkg="javax.servlet" />
<allow pkg="jakarta.servlet" />
<allow pkg="org.glassfish.jersey" />
<allow pkg="com.fasterxml.jackson" />
<allow pkg="org.apache.http"/>
Expand All @@ -587,7 +587,7 @@
</subpackage>

<subpackage name="distributed">
<allow pkg="javax.ws.rs.core" />
<allow pkg="jakarta.ws.rs.core" />
</subpackage>
</subpackage>

Expand Down Expand Up @@ -619,8 +619,8 @@
<allow pkg="org.apache.kafka.common.test" />
<allow pkg="kafka.zk" />
<allow pkg="kafka.utils" />
<allow class="javax.servlet.http.HttpServletResponse" />
<allow class="javax.ws.rs.core.Response" />
<allow class="jakarta.servlet.http.HttpServletResponse" />
<allow class="jakarta.ws.rs.core.Response" />
<allow pkg="com.fasterxml.jackson.core.type" />
<allow pkg="org.apache.kafka.metadata" />
<allow pkg="org.eclipse.jetty.client"/>
Expand All @@ -632,7 +632,7 @@
<subpackage name="integration">
<allow pkg="org.apache.kafka.connect.util.clusters" />
<allow pkg="org.apache.kafka.connect" />
<allow pkg="javax.ws.rs" />
<allow pkg="jakarta.ws.rs" />
<allow pkg="org.apache.http"/>
<allow pkg="org.eclipse.jetty.util"/>
<!-- for tests -->
Expand Down
2 changes: 1 addition & 1 deletion connect/api/src/main/java/org/apache/kafka/connect/rest/ConnectRestExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public interface ConnectRestExtension extends Configurable, Versioned, Closeable
* will invoke this method after registering the default Connect resources. If the implementations attempt
* to re-register any of the Connect resources, it will be ignored and will be logged.
*
* @param restPluginContext The context provides access to JAX-RS {@link javax.ws.rs.core.Configurable} and {@link
* @param restPluginContext The context provides access to JAX-RS {@link jakarta.ws.rs.core.Configurable} and {@link
* ConnectClusterState}.The custom JAX-RS resources can be registered via the {@link
* ConnectRestExtensionContext#configurable()}
*/
Expand Down
9 changes: 5 additions & 4 deletions connect/api/src/main/java/org/apache/kafka/connect/rest/ConnectRestExtensionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,20 @@

import org.apache.kafka.connect.health.ConnectClusterState;

import javax.ws.rs.core.Configurable;
import jakarta.ws.rs.core.Configurable;


/**
* The interface provides the ability for {@link ConnectRestExtension} implementations to access the JAX-RS
* {@link javax.ws.rs.core.Configurable} and cluster state {@link ConnectClusterState}. The implementation for the interface is provided
* {@link jakarta.ws.rs.core.Configurable} and cluster state {@link ConnectClusterState}. The implementation for the interface is provided
* by the Connect framework.
*/
public interface ConnectRestExtensionContext {

/**
* Provides an implementation of {@link javax.ws.rs.core.Configurable} that can be used to register JAX-RS resources.
* Provides an implementation of {@link jakarta.ws.rs.core.Configurable} that can be used to register JAX-RS resources.
*
* @return the JAX-RS {@link javax.ws.rs.core.Configurable}; never {@code null}
* @return the JAX-RS {@link jakarta.ws.rs.core.Configurable}; never {@code null}
*/
Configurable<? extends Configurable<?>> configurable();

Expand Down
2 changes: 1 addition & 1 deletion ...va/org/apache/kafka/connect/rest/basic/auth/extension/BasicAuthSecurityRestExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
*</Pre>
*
* <p>This is a reference implementation of the {@link ConnectRestExtension} interface. It registers an implementation of {@link
* javax.ws.rs.container.ContainerRequestFilter} that does JAAS based authentication of incoming Basic Auth credentials. {@link
* jakarta.ws.rs.container.ContainerRequestFilter} that does JAAS based authentication of incoming Basic Auth credentials. {@link
* ConnectRestExtension} implementations are loaded via the plugin class loader using {@link java.util.ServiceLoader} mechanism and hence
* the packaged jar includes {@code META-INF/services/org.apache.kafka.connect.rest.extension.ConnectRestExtension} with the entry
* {@code org.apache.kafka.connect.extension.auth.jaas.BasicAuthSecurityRestExtension}
Expand Down
13 changes: 7 additions & 6 deletions ...src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,13 @@
import javax.security.auth.login.Configuration;
import javax.security.auth.login.LoginContext;
import javax.security.auth.login.LoginException;
import javax.ws.rs.HttpMethod;
import javax.ws.rs.Priorities;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerRequestFilter;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;

import jakarta.ws.rs.HttpMethod;
import jakarta.ws.rs.Priorities;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.SecurityContext;

@Priority(Priorities.AUTHENTICATION)
public class JaasBasicAuthFilter implements ContainerRequestFilter {
Expand Down
3 changes: 2 additions & 1 deletion ...rg/apache/kafka/connect/rest/basic/auth/extension/BasicAuthSecurityRestExtensionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
import java.util.function.Supplier;

import javax.security.auth.login.Configuration;
import javax.ws.rs.core.Configurable;

import jakarta.ws.rs.core.Configurable;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
Expand Down
11 changes: 6 additions & 5 deletions ...test/java/org/apache/kafka/connect/rest/basic/auth/extension/JaasBasicAuthFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@
import javax.security.auth.callback.Callback;
import javax.security.auth.callback.CallbackHandler;
import javax.security.auth.callback.ChoiceCallback;
import javax.ws.rs.HttpMethod;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;
import javax.ws.rs.core.UriInfo;

import jakarta.ws.rs.HttpMethod;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.SecurityContext;
import jakarta.ws.rs.core.UriInfo;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down
10 changes: 5 additions & 5 deletions .../src/main/java/org/apache/kafka/connect/mirror/rest/resources/InternalMirrorResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@

import java.util.Map;

import javax.inject.Inject;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.Path;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.UriInfo;
import jakarta.inject.Inject;
import jakarta.ws.rs.NotFoundException;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.UriInfo;

@Path("/{source}/{target}/connectors")
public class InternalMirrorResource extends InternalClusterResource {
Expand Down
7 changes: 6 additions & 1 deletion ...test/java/org/apache/kafka/connect/mirror/integration/DedicatedMirrorIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,12 @@ public void testMultiNodeCluster() throws Exception {
// Cluster aliases
final String a = "A";
// Use a convoluted cluster name to ensure URL encoding/decoding works
final String b = "B- ._~:/?#[]@!$&'()*+;=\"<>%{}|\\^`618";
// TODO: Jetty 12 throws a 400 amiguous error with the previous test string here
// It seems like at least some encoded characters are not allowed anymore based on
// https://github.com/jetty/jetty.project/issues/11890#issuecomment-2156442947
// See https://github.com/jetty/jetty.project/issues/11890
//final String b = "B- ._~:/?#[]@!$&'()*+;=\"<>%{}|\\^`618";
final String b = "B";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you keep most of these suspicious characters, and just remove the ones which are ambiguous?
From reading the linked issue, it might just be the / character, and the remainder are still valid.

It looks like this test was sensitive enough to find the underlying change in the library, so I'd like to keep it sensitive in case something like this happens again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there's actually a few characters that are not allowed, control characters, backslash, etc. I went ahead and removed a few problem characters so it works now. I also updated the comments for the section. You can see the exact rules in the Rejecting Suspicious Sequences section as part of https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#uri-path-canonicalization

final String ab = a + "->" + b;
final String ba = b + "->" + a;
final String testTopicPrefix = "test-topic-";
Expand Down
2 changes: 1 addition & 1 deletion connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,7 @@ private void awaitStopTask(ConnectorTaskId taskId, long timeout) {
}

if (!task.awaitStop(timeout)) {
log.error("Graceful stop of task {} failed.", task.id());
log.error(" Graceful stop of task {} failed.", task.id());
cshannon marked this conversation as resolved.
Show resolved Hide resolved
task.cancel();
} else {
log.debug("Graceful stop of task {} succeeded.", task.id());
Expand Down
5 changes: 3 additions & 2 deletions ...runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@

import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriBuilder;

import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriBuilder;

import static org.apache.kafka.clients.consumer.ConsumerConfig.GROUP_ID_CONFIG;
import static org.apache.kafka.common.utils.Utils.UncheckedCloseable;
Expand Down
4 changes: 2 additions & 2 deletions .../runtime/src/main/java/org/apache/kafka/connect/runtime/rest/ConnectRestConfigurable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import java.util.Map;
import java.util.Objects;

import javax.ws.rs.core.Configurable;
import javax.ws.rs.core.Configuration;
import jakarta.ws.rs.core.Configurable;
import jakarta.ws.rs.core.Configuration;

/**
* The implementation delegates to {@link ResourceConfig} so that we can handle duplicate
Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/org/apache/kafka/connect/runtime/rest/ConnectRestExtensionContextImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.apache.kafka.connect.health.ConnectClusterState;
import org.apache.kafka.connect.rest.ConnectRestExtensionContext;

import javax.ws.rs.core.Configurable;
import jakarta.ws.rs.core.Configurable;

public class ConnectRestExtensionContextImpl implements ConnectRestExtensionContext {

Expand Down
7 changes: 4 additions & 3 deletions ...ect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/HerderRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriBuilder;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriBuilder;

public class HerderRequestHandler {

Expand Down Expand Up @@ -113,6 +113,7 @@ public <T, U> T completeOrForwardRequest(FutureCallback<T> cb,
}
String forwardUrl = uriBuilder.build().toString();
log.debug("Forwarding request {} {} {}", forwardUrl, method, body);
// TODO, we may need to set the reqest timeout as Idle timeout on the HttpClient
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: reqest -> request

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is valid to me, as the default idle timeout for HttpClient is 30 seconds, and the default request timeout is 90 seconds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to configure the idle timeout I guess the question is the best way to do so. I think we'd either need to pass it as an argument to the request() methods or add it to the AbstractConfig for RestClient so it has access to it when constructing the HttpClient. I guess there's also the question if it should always match the request timeout or it could be configured as a separate config. Maybe this could be done as a follow on task after this is merged.

return translator.translate(restClient.httpRequest(forwardUrl, method, headers, body, resultType));
} else {
log.error("Request '{} {}' failed because it couldn't find the target Connect worker within two hops (between workers).",
Expand Down
Loading
Loading