This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit e2ec2702519a7b8797cd203dcc83c9e7c2a87b20 Author: Vitalii Diravka <[email protected]> AuthorDate: Mon Mar 4 00:05:14 2019 +0200 DRILL-7051: Upgrade jetty - upgrade Jetty dependencies to 9.3 version - adaptation to the new Jetty API (SessionHandler, LoginService, AbstractLoginService) - add JavaDocs and code refactoring closes #1681 --- .../drill/yarn/appMaster/http/WebServer.java | 31 +++++----- exec/java-exec/pom.xml | 15 ++--- .../drill/exec/server/rest/StorageResources.java | 9 +-- .../apache/drill/exec/server/rest/WebServer.java | 8 ++- .../server/rest/auth/DrillRestLoginService.java | 5 +- .../server/rest/auth/DrillSpnegoAuthenticator.java | 13 ++-- .../server/rest/auth/DrillSpnegoLoginService.java | 10 +-- .../rest/spnego/TestSpnegoAuthentication.java | 18 +++--- .../apache/drill/test/TestGracefulShutdown.java | 15 +++-- pom.xml | 72 ++++++++++++++-------- 10 files changed, 99 insertions(+), 97 deletions(-) diff --git a/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/http/WebServer.java b/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/http/WebServer.java index 0920eb6..5ba31bc 100644 --- a/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/http/WebServer.java +++ b/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/http/WebServer.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.Date; import java.util.Set; +import javax.servlet.ServletRequest; import javax.servlet.http.HttpSession; import javax.servlet.http.HttpSessionEvent; import javax.servlet.http.HttpSessionListener; @@ -174,9 +175,8 @@ public class WebServer implements AutoCloseable { // Security, if requested. if (AMSecurityManagerImpl.isEnabled()) { - servletContextHandler.setSecurityHandler(createSecurityHandler(config)); - servletContextHandler.setSessionHandler(createSessionHandler(config, - servletContextHandler.getSecurityHandler())); + servletContextHandler.setSecurityHandler(createSecurityHandler()); + servletContextHandler.setSessionHandler(createSessionHandler(config, servletContextHandler.getSecurityHandler())); } } @@ -249,12 +249,11 @@ public class WebServer implements AutoCloseable { } @Override - public UserIdentity login(String username, Object credentials) { + public UserIdentity login(String username, Object credentials, ServletRequest request) { if (!securityMgr.login(username, (String) credentials)) { return null; } - return new DefaultUserIdentity(null, new AMUserPrincipal(username), - new String[] { ADMIN_ROLE }); + return new DefaultUserIdentity(null, new AMUserPrincipal(username), new String[] { ADMIN_ROLE }); } @Override @@ -290,12 +289,12 @@ public class WebServer implements AutoCloseable { } /** - * @return - * @return - * @see http://www.eclipse.org/jetty/documentation/current/embedded-examples.html + * It creates handler with security constraint combinations for runtime efficiency + * @see <a href="http://www.eclipse.org/jetty/documentation/current/embedded-examples.html";>Eclipse Jetty Documentation</a> + * + * @return security handler with precomputed constraint combinations */ - - private ConstraintSecurityHandler createSecurityHandler(Config config) { + private ConstraintSecurityHandler createSecurityHandler() { ConstraintSecurityHandler security = new ConstraintSecurityHandler(); Set<String> knownRoles = ImmutableSet.of(ADMIN_ROLE); @@ -310,8 +309,11 @@ public class WebServer implements AutoCloseable { } /** - * @return A {@link SessionHandler} which contains a - * {@link HashSessionManager} + * It creates A {@link SessionHandler} which contains a {@link HashSessionManager} + * + * @param config Drill configs + * @param securityHandler Set of initparameters that are used by the Authentication + * @return session handler */ private SessionHandler createSessionHandler(Config config, final SecurityHandler securityHandler) { @@ -366,14 +368,13 @@ public class WebServer implements AutoCloseable { * certificate is generated and used. * <p> * This is a shameless copy of - * {@link org.apache.drill.exec.server.rest.Webserver#createHttpsConnector( )}. + * {@link org.apache.drill.exec.server.rest.WebServer#createHttpsConnector(int, int, int)}. * The two should be merged at some point. The primary issue is that the Drill * version is tightly coupled to Drillbit configuration. * * @return Initialized {@link ServerConnector} for HTTPS connections. * @throws Exception */ - private ServerConnector createHttpsConnector(Config config) throws Exception { LOG.info("Setting up HTTPS connector for web server"); diff --git a/exec/java-exec/pom.xml b/exec/java-exec/pom.xml index a341ab9..1d0cb75 100644 --- a/exec/java-exec/pom.xml +++ b/exec/java-exec/pom.xml @@ -126,17 +126,14 @@ <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-server</artifactId> - <version>9.1.5.v20140505</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-servlet</artifactId> - <version>9.1.5.v20140505</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-servlets</artifactId> - <version>9.1.5.v20140505</version> <exclusions> <exclusion> <artifactId>jetty-continuation</artifactId> @@ -210,10 +207,6 @@ </exclusions> </dependency> <dependency> - <groupId>org.glassfish.jersey.inject</groupId> - <artifactId>jersey-hk2</artifactId> - </dependency> - <dependency> <groupId>org.apache.calcite</groupId> <artifactId>calcite-core</artifactId> </dependency> @@ -359,12 +352,12 @@ <artifactId>commons-codec</artifactId> </exclusion> <exclusion> - <groupId>io.netty</groupId> - <artifactId>netty</artifactId> + <groupId>io.netty</groupId> + <artifactId>netty</artifactId> </exclusion> <exclusion> - <groupId>io.netty</groupId> - <artifactId>netty-all</artifactId> + <groupId>io.netty</groupId> + <artifactId>netty-all</artifactId> </exclusion> </exclusions> </dependency> diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java index 04090cc..5dee285 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java @@ -86,8 +86,9 @@ public class StorageResources { .header(HttpHeaders.CONTENT_DISPOSITION, String.format("attachment;filename=\"%s_storage_plugins.%s\"", pluginGroup, format)) .build() - : Response.status(Response.Status.NOT_FOUND.getStatusCode(), - String.format("Unknown file type %s for %s Storage Plugin Configs", format, pluginGroup)) + : Response.status(Response.Status.NOT_FOUND.getStatusCode()) + .entity(String.format("Unknown \"%s\" file format for \"%s\" Storage Plugin configs group", + format, pluginGroup)) .build(); } @@ -146,8 +147,8 @@ public class StorageResources { ? Response.ok(getPluginConfig(name)) .header(HttpHeaders.CONTENT_DISPOSITION, String.format("attachment;filename=\"%s.%s\"", name, format)) .build() - : Response.status(Response.Status.NOT_FOUND.getStatusCode(), - String.format("Unknown file type %s for Storage Plugin Config: %s", format, name)) + : Response.status(Response.Status.NOT_FOUND.getStatusCode()) + .entity(String.format("Unknown \"%s\" file format for \"%s\" Storage Plugin config", format, name)) .build(); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java index 3fa2f3c..b912a4c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java @@ -83,7 +83,6 @@ import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; import java.math.BigInteger; -import java.net.BindException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -187,7 +186,7 @@ public class WebServer implements AutoCloseable { try { embeddedJetty.start(); return; - } catch (BindException e) { + } catch (IOException e) { if (portHunt) { logger.info("Failed to start on port {}, trying port {}", port, ++port, e); } else { @@ -269,7 +268,10 @@ public class WebServer implements AutoCloseable { } /** - * @return A {@link SessionHandler} which contains a {@link HashSessionManager} + * It creates A {@link SessionHandler} which contains a {@link HashSessionManager} + * + * @param securityHandler Set of initparameters that are used by the Authentication + * @return session handler */ private SessionHandler createSessionHandler(final SecurityHandler securityHandler) { SessionManager sessionManager = new HashSessionManager(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillRestLoginService.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillRestLoginService.java index 998026b..33fe52c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillRestLoginService.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillRestLoginService.java @@ -31,6 +31,7 @@ import org.eclipse.jetty.security.LoginService; import org.eclipse.jetty.server.UserIdentity; import javax.security.auth.Subject; +import javax.servlet.ServletRequest; import java.security.Principal; /** @@ -62,7 +63,7 @@ public class DrillRestLoginService implements LoginService { } @Override - public UserIdentity login(String username, Object credentials) { + public UserIdentity login(String username, Object credentials, ServletRequest request) { if (!(credentials instanceof String)) { return null; } @@ -126,7 +127,7 @@ public class DrillRestLoginService implements LoginService { @Override public void logout(UserIdentity user) { // no-op - if(logger.isTraceEnabled()) { + if (logger.isTraceEnabled()) { logger.trace("Web user {} logged out.", user.getUserPrincipal().getName()); } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java index 24a684b..d60aaf5 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java @@ -28,9 +28,7 @@ import org.eclipse.jetty.security.authentication.DeferredAuthentication; import org.eclipse.jetty.security.authentication.SessionAuthentication; import org.eclipse.jetty.security.authentication.SpnegoAuthenticator; import org.eclipse.jetty.server.Authentication; -import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.UserIdentity; import javax.servlet.ServletRequest; @@ -150,15 +148,12 @@ public class DrillSpnegoAuthenticator extends SpnegoAuthenticator { newUri = WebServerConstants.WEBSERVER_ROOT_PATH; } } - response.setContentLength(0); - final HttpChannel channel = HttpChannel.getCurrentHttpChannel(); - final Response base_response = channel.getResponse(); - final Request base_request = channel.getRequest(); - final int redirectCode = - base_request.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? 302 : 303; + Request baseRequest = Request.getBaseRequest(req); + int redirectCode = + baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? 302 : 303; try { - base_response.sendRedirect(redirectCode, res.encodeRedirectURL(newUri)); + baseRequest.getResponse().sendRedirect(redirectCode, res.encodeRedirectURL(newUri)); } catch (IOException e) { logger.error("DrillSpnegoAuthenticator: Failed while using the redirect URL {} from client {}", newUri, req.getRemoteAddr(), e); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoLoginService.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoLoginService.java index c890190..429aa3a 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoLoginService.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoLoginService.java @@ -37,6 +37,7 @@ import org.ietf.jgss.GSSName; import org.ietf.jgss.Oid; import javax.security.auth.Subject; +import javax.servlet.ServletRequest; import java.io.IOException; import java.lang.reflect.Field; import java.security.Principal; @@ -78,16 +79,11 @@ public class DrillSpnegoLoginService extends SpnegoLoginService { } @Override - public UserIdentity login(final String username, final Object credentials) { + public UserIdentity login(final String username, final Object credentials, ServletRequest request) { UserIdentity identity = null; try { - identity = loggedInUgi.doAs(new PrivilegedExceptionAction<UserIdentity>() { - @Override - public UserIdentity run() { - return spnegoLogin(credentials); - } - }); + identity = loggedInUgi.doAs((PrivilegedExceptionAction<UserIdentity>) () -> spnegoLogin(credentials)); } catch (Exception e) { logger.error("Failed to login using SPNEGO", e); } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/spnego/TestSpnegoAuthentication.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/spnego/TestSpnegoAuthentication.java index cbf45ac..e4f82a1 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/spnego/TestSpnegoAuthentication.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/spnego/TestSpnegoAuthentication.java @@ -56,6 +56,8 @@ import java.lang.reflect.Field; import java.security.PrivilegedExceptionAction; import static junit.framework.TestCase.fail; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; /** @@ -88,7 +90,6 @@ public class TestSpnegoAuthentication { /** * Both SPNEGO and FORM mechanism is enabled for WebServer in configuration. Test to see if the respective security * handlers are created successfully or not. - * @throws Exception */ @Test public void testSPNEGOAndFORMEnabled() throws Exception { @@ -119,7 +120,6 @@ public class TestSpnegoAuthentication { /** * Validate if FORM security handler is created successfully when only form is configured as auth mechanism - * @throws Exception */ @Test public void testOnlyFORMEnabled() throws Exception { @@ -151,7 +151,6 @@ public class TestSpnegoAuthentication { /** * Validate failure in creating FORM security handler when PAM authenticator is absent. PAM authenticator is provided * via {@link PlainFactory#getAuthenticator()} - * @throws Exception */ @Test public void testFORMEnabledWithPlainDisabled() throws Exception { @@ -185,7 +184,6 @@ public class TestSpnegoAuthentication { /** * Validate only SPNEGO security handler is configured properly when enabled via configuration - * @throws Exception */ @Test public void testOnlySPNEGOEnabled() throws Exception { @@ -219,7 +217,6 @@ public class TestSpnegoAuthentication { * Validate when none of the security mechanism is specified in the * {@link ExecConstants#HTTP_AUTHENTICATION_MECHANISMS}, FORM security handler is still configured correctly when * authentication is enabled along with PAM authenticator module. - * @throws Exception */ @Test public void testConfigBackwardCompatibility() throws Exception { @@ -244,9 +241,8 @@ public class TestSpnegoAuthentication { } /** - * Validate successful {@link DrillSpnegoLoginService#login(String, Object)} when provided with client token for a - * configured service principal. - * @throws Exception + * Validate successful {@link DrillSpnegoLoginService#login(String, Object, javax.servlet.ServletRequest)} + * when provided with client token for a configured service principal. */ @Test public void testDrillSpnegoLoginService() throws Exception { @@ -305,11 +301,11 @@ public class TestSpnegoAuthentication { final DrillSpnegoLoginService loginService = new DrillSpnegoLoginService(drillbitContext); // Authenticate the client using its SPNEGO token - final UserIdentity user = loginService.login(null, token); + final UserIdentity user = loginService.login(null, token, null); // Validate the UserIdentity of authenticated client - assertTrue(user != null); - assertTrue(user.getUserPrincipal().getName().equals(spnegoHelper.CLIENT_SHORT_NAME)); + assertNotNull(user); + assertEquals(user.getUserPrincipal().getName(), spnegoHelper.CLIENT_SHORT_NAME); assertTrue(user.isUserInRole("authenticated", null)); } diff --git a/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java b/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java index d74f1e6..c5d508b 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java +++ b/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java @@ -140,7 +140,7 @@ public class TestGracefulShutdown extends BaseTestQuery { Thread.sleep(100L); } - if (waitAndAssertDrillbitCount(cluster, zkRefresh)) { + if (waitAndAssertDrillbitCount(cluster, zkRefresh, drillbits.length)) { return; } Assert.fail("Timed out"); @@ -178,7 +178,7 @@ public class TestGracefulShutdown extends BaseTestQuery { throw new RuntimeException("Failed : HTTP error code : " + conn.getResponseCode()); } - if (waitAndAssertDrillbitCount(cluster, zkRefresh)) { + if (waitAndAssertDrillbitCount(cluster, zkRefresh, drillbits.length)) { return; } Assert.fail("Timed out"); @@ -240,24 +240,23 @@ public class TestGracefulShutdown extends BaseTestQuery { assertFalse("First drillbit instance should have a temporary Javascript dir deleted", originalDrillbitTempDir.exists()); } - private static File getWebServerTempDirPath(Drillbit drillbit) throws IllegalAccessException { + private File getWebServerTempDirPath(Drillbit drillbit) throws IllegalAccessException { Field webServerField = FieldUtils.getField(drillbit.getClass(), "webServer", true); WebServer webServerHandle = (WebServer) FieldUtils.readField(webServerField, drillbit, true); - File webServerTempDirPath = webServerHandle.getOrCreateTmpJavaScriptDir(); - return webServerTempDirPath; + return webServerHandle.getOrCreateTmpJavaScriptDir(); } - private static boolean waitAndAssertDrillbitCount(ClusterFixture cluster, int zkRefresh) throws InterruptedException { + private boolean waitAndAssertDrillbitCount(ClusterFixture cluster, int zkRefresh, int bitsNum) + throws InterruptedException { while (true) { Collection<DrillbitEndpoint> drillbitEndpoints = cluster.drillbit() .getContext() .getClusterCoordinator() .getAvailableEndpoints(); - if (drillbitEndpoints.size() == 2) { + if (drillbitEndpoints.size() == bitsNum - 1) { return true; } - Thread.sleep(zkRefresh); } } diff --git a/pom.xml b/pom.xml index 5991089..c2eefd8 100644 --- a/pom.xml +++ b/pom.xml @@ -85,7 +85,8 @@ <reflections.version>0.9.10</reflections.version> <avro.version>1.8.2</avro.version> <metrics.version>4.0.2</metrics.version> - <jersey.version>2.28</jersey.version> + <jetty.version>9.3.25.v20180904</jetty.version> + <jersey.version>2.25.1</jersey.version> <asm.version>7.0</asm.version> <excludedGroups /> <memoryMb>4096</memoryMb> @@ -2452,6 +2453,49 @@ <version>1.60</version> </dependency> + <!--Eclipse Jetty dependecies--> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-server</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-servlet</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-servlets</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-security</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-util</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-io</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-webapp</artifactId> + <version>${jetty.version}</version> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-xml</artifactId> + <version>${jetty.version}</version> + </dependency> + <!--Eclipse Jetty dependecies--> + <!--GlassFish Jersey dependecies--> <dependency> <groupId>org.glassfish.jersey.ext</groupId> @@ -2483,34 +2527,8 @@ <artifactId>jersey-container-servlet</artifactId> <version>${jersey.version}</version> </dependency> - <dependency> - <groupId>org.glassfish.jersey.inject</groupId> - <artifactId>jersey-hk2</artifactId> - <version>${jersey.version}</version> - </dependency> <!--/GlassFish Jersey dependecies--> - <!--Javax Servlet dependecies--> - <dependency> - <groupId>javax.servlet.jsp</groupId> - <artifactId>javax.servlet.jsp-api</artifactId> - <version>2.3.3</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>javax.servlet.jsp</groupId> - <artifactId>jsp-api</artifactId> - <version>2.2</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>javax.servlet</groupId> - <artifactId>javax.servlet-api</artifactId> - <version>4.0.1</version> - <scope>provided</scope> - </dependency> - <!--/Javax Servlet dependecies--> - <!-- Test Dependencies --> <dependency> <groupId>org.apache.hadoop</groupId>
