Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions packaging/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,18 @@
<regexp>http://(www\.)?antlr.org/license.html</regexp>
<replacement>http://www.antlr.org/license.html</replacement>
</licenseUrlReplacement>
<licenseUrlReplacement>
<regexp>https?://www\.eclipse\.org/legal/epl-2\.0/?</regexp>
<replacement>https://www.eclipse.org/legal/epl-2.0</replacement>
</licenseUrlReplacement>
</licenseUrlReplacements>
<licenseUrlFileNames>
<APACHE-2.0>
https?://www\.apache\.org/licenses/LICENSE-2\.0.*
\Qhttps://aws.amazon.com/apache2.0\E
\Qhttps://opensource.org/licenses/Apache-2.0\E
\Qhttp://www.apache.org/licenses/\E
\Qhttps://repository.jboss.org/licenses/apache-2.0.txt\E
</APACHE-2.0>
<BSD-2-CLAUSE>
https?://(www\.)?opensource.org/licenses/bsd-license.php
Expand Down
6 changes: 3 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@
<aws-secretsmanager-caching.version>2.1.1</aws-secretsmanager-caching.version>
<jansi.version>2.4.0</jansi.version>
<!-- If upgrading, upgrade atlas as well in ql/pom.xml, which brings in some springframework dependencies transitively -->
<spring.version>5.3.39</spring.version>
<spring-boot.version>2.7.18</spring-boot.version>
<spring.ldap.version>2.4.4</spring.ldap.version>
<spring.version>6.2.12</spring.version>
<spring-boot.version>3.4.11</spring-boot.version>
<spring.ldap.version>3.3.4</spring.ldap.version>
<project.build.outputTimestamp>2025-01-01T00:00:00Z</project.build.outputTimestamp>
<keycloak.version>26.0.6</keycloak.version>
<nimbus-oauth.version>11.28</nimbus-oauth.version>
Expand Down
12 changes: 12 additions & 0 deletions standalone-metastore/metastore-rest-catalog/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
<artifactId>hive-standalone-metastore-server</artifactId>
<version>${hive.version}</version>
<scope>provided</scope>
<exclusions>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

scope is provided, why do any exclusions here?

<exclusion>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.hive</groupId>
Expand Down Expand Up @@ -70,6 +76,12 @@
<artifactId>micrometer-registry-prometheus</artifactId>
<version>1.9.17</version>
</dependency>
<dependency>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<version>6.0.0</version>
<scope>provided</scope>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

who is supposed to provide this dependency? we are not building a war and deploying to the app server

</dependency>
<!-- Test dependencies -->
<dependency>
<groupId>org.apache.hive</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import javax.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServlet;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.metastore.ServletSecurity;
import org.apache.hadoop.hive.metastore.ServletSecurity.AuthType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import java.util.Optional;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.rest.HMSCatalogAdapter.Route;
import org.apache.iceberg.rest.HTTPRequest.HTTPMethod;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
*/
package org.apache.iceberg.rest.standalone;

import javax.servlet.http.HttpServlet;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.metastore.ServletServerBuilder;
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
Expand Down Expand Up @@ -54,14 +52,14 @@ public Configuration hadoopConfiguration(ApplicationArguments args) {
}

@Bean
public ServletRegistrationBean<HttpServlet> restCatalogServlet(Configuration conf) {
public ServletRegistrationBean<?> restCatalogServlet(Configuration conf) {
return createRestCatalogServlet(conf);
}

/**
* Creates the REST Catalog servlet registration. Shared by production config and tests.
*/
public static ServletRegistrationBean<HttpServlet> createRestCatalogServlet(Configuration conf) {
public static ServletRegistrationBean<?> createRestCatalogServlet(Configuration conf) {
String servletPath = MetastoreConf.getVar(conf, ConfVars.ICEBERG_CATALOG_SERVLET_PATH);
if (servletPath == null || servletPath.isEmpty()) {
servletPath = DEFAULT_SERVLET_PATH;
Expand All @@ -80,9 +78,8 @@ public static ServletRegistrationBean<HttpServlet> createRestCatalogServlet(Conf
if (descriptor == null || descriptor.getServlet() == null) {
throw new IllegalStateException("Failed to create Iceberg REST Catalog servlet");
}

ServletRegistrationBean<HttpServlet> registration =
new ServletRegistrationBean<>(descriptor.getServlet(), "/" + servletPath + "/*");
ServletRegistrationBean<?> registration =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not simply replace the import?

- javax.servlet.http.HttpServlet
+ jakarta.servlet.http.HttpServlet

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At first, IntelliJ was showing red lines when I tried the jakarta import. I didn't see the 'Add to classpath' option initially when I hovered over the error, so I used the generic <?> as a temporary fix to get the code to compile locally. I see how to fix the classpath now so I'll restore the specific HttpServlet with jakarta import

new ServletRegistrationBean(descriptor.getServlet(), "/" + servletPath + "/*");
registration.setName("IcebergRESTCatalog");
registration.setLoadOnStartup(1);

Expand Down
13 changes: 10 additions & 3 deletions standalone-metastore/metastore-server/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@
<dependency>
<groupId>org.apache.thrift</groupId>
<artifactId>libthrift</artifactId>
<version>0.20.0</version>
</dependency>
<dependency>
<groupId>org.datanucleus</groupId>
Expand Down Expand Up @@ -359,17 +360,23 @@
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-util</artifactId>
<version>${jetty.version}</version>
<version>11.0.25</version>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please do not hardcode

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah sure!!

</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
<version>${jetty.version}</version>
<version>11.0.25</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-servlet</artifactId>
<version>${jetty.version}</version>
<version>11.0.25</version>
</dependency>
<dependency>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<version>6.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import javax.servlet.Servlet;
import javax.servlet.ServletRequestEvent;
import javax.servlet.ServletRequestListener;
import jakarta.servlet.Servlet;
import jakarta.servlet.ServletRequestEvent;
import jakarta.servlet.ServletRequestListener;
/**
* TODO:pc remove application logic to a separate interface.
*/
Expand Down Expand Up @@ -382,7 +382,7 @@ public void setThreadFactory(ThreadFactory threadFactory) {

final HttpConnectionFactory http = new HttpConnectionFactory(httpServerConf);

final SslContextFactory sslContextFactory = ServletSecurity.createSslContextFactory(conf);
final SslContextFactory.Server sslContextFactory = ServletSecurity.createSslContextFactory(conf);
if (sslContextFactory != null) {
connector = new ServerConnector(server, sslContextFactory, http);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletOutputStream;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,18 @@
import org.apache.hadoop.security.SecurityUtil;
import org.apache.hadoop.security.UserGroupInformation;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.pac4j.core.context.JEEContext;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why drop this?

import org.pac4j.core.credentials.TokenCredentials;
import org.pac4j.core.credentials.extractor.BearerAuthExtractor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.security.PrivilegedAction;
import java.security.PrivilegedExceptionAction;
import java.util.Arrays;
import java.util.Enumeration;
import java.util.Optional;

/**
* Secures servlet processing.
Expand Down Expand Up @@ -305,10 +301,15 @@ private String extractUserName(HttpServletRequest request, HttpServletResponse r
*/
private String extractBearerToken(HttpServletRequest request,
HttpServletResponse response) {
BearerAuthExtractor extractor = new BearerAuthExtractor();
Optional<TokenCredentials> tokenCredentials = extractor.extract(new JEEContext(
request, response));
return tokenCredentials.map(TokenCredentials::getToken).orElse(null);
String authorization = request.getHeader("Authorization");
if (authorization == null) {
return null;
}
if (authorization.regionMatches(true, 0, "Bearer ", 0, 7)) {
String token = authorization.substring(7).trim();
return token.isEmpty() ? null : token;
}
return null;
}

/**
Expand Down Expand Up @@ -338,7 +339,7 @@ static void loginServerPrincipal(Configuration conf) throws IOException {
* @return null if no ssl in config, an instance otherwise
* @throws IOException if getting password fails
*/
static SslContextFactory createSslContextFactory(Configuration conf) throws IOException {
static SslContextFactory.Server createSslContextFactory(Configuration conf) throws IOException {
final boolean useSsl = MetastoreConf.getBoolVar(conf, MetastoreConf.ConfVars.USE_SSL);
if (!useSsl) {
return null;
Expand All @@ -359,7 +360,7 @@ static SslContextFactory createSslContextFactory(Configuration conf) throws IOEx
if (LOG.isInfoEnabled()) {
LOG.info("HTTP Server SSL: adding excluded protocols: {}", Arrays.toString(excludedProtocols));
}
SslContextFactory factory = new SslContextFactory.Server();
SslContextFactory.Server factory = new SslContextFactory.Server();
factory.addExcludeProtocols(excludedProtocols);
if (LOG.isInfoEnabled()) {
LOG.info("HTTP Server SSL: SslContextFactory.getExcludeProtocols = {}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.servlet.Servlet;
import javax.servlet.http.HttpServlet;
import jakarta.servlet.Servlet;
import jakarta.servlet.http.HttpServlet;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -173,7 +173,7 @@ private Server createServer() {
* @param port The port to bind the connector to
* @return The created ServerConnector
*/
private ServerConnector createConnector(Server server, SslContextFactory sslContextFactory, int port) {
private ServerConnector createConnector(Server server, SslContextFactory.Server sslContextFactory, int port) {
final ServerConnector connector;
HttpConfiguration httpConf = new HttpConfiguration();
// Do not leak information
Expand Down Expand Up @@ -236,7 +236,7 @@ public Server startServer() throws Exception {
}
final Server server = createServer();
// create the connectors
final SslContextFactory sslContextFactory = ServletSecurity.createSslContextFactory(configuration);
final SslContextFactory.Server sslContextFactory = ServletSecurity.createSslContextFactory(configuration);
final ServerConnector[] connectors = new ServerConnector[size];
final ServletContextHandler[] handlers = new ServletContextHandler[size];
Iterator<Map.Entry<Integer, ServletContextHandler>> it = handlersMap.entrySet().iterator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package org.apache.hadoop.hive.metastore.auth;

import java.util.Optional;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponse;

/*
Encapsulates any exceptions thrown by HiveMetastore server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import java.time.Clock;
import java.time.Duration;
import java.util.List;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.hadoop.hive.metastore.auth.HttpAuthenticationException;
import org.checkerframework.checker.index.qual.NonNegative;
import org.checkerframework.checker.nullness.qual.NonNull;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
import org.apache.hadoop.hive.metastore.utils.StringUtils;
import org.apache.hadoop.hive.ql.util.IncrementalObjectSizeEstimator;
import org.apache.hadoop.hive.ql.util.IncrementalObjectSizeEstimator.ObjectEstimator;
import org.eclipse.jetty.util.ConcurrentHashSet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -116,7 +115,7 @@ public class SharedCache {
private AtomicLong cacheUpdateCount = new AtomicLong(0);
private long maxCacheSizeInBytes = -1;
private HashMap<Class<?>, ObjectEstimator> sizeEstimators = null;
private Set<String> tableToUpdateSize = new ConcurrentHashSet<>();
private Set<String> tableToUpdateSize = ConcurrentHashMap.newKeySet();
private ScheduledExecutorService executor = null;
private Map<String, Integer> tableSizeMap = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,12 @@ public Void execute(MultiDataSourceJdbcResource jdbcResource) throws MetaExcepti
// caught and either swallowed or wrapped in MetaException. Also, only a single test fails without this block:
// org.apache.hadoop.hive.metastore.client.TestDatabases.testAlterDatabaseNotNullableFields
// It may worth investigate if this catch block is really needed.
if (e.getMessage() != null && e.getMessage().contains("does not exist")) {
String msg = e.getMessage();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

change is unrelated to the dependency upgrade

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason I included this change is that TestDatabases#testAlterDatabaseNotNullableFields started failing immediately after the move to Spring 6 in my first precommit run.
In the older Spring version ig e.getMessage() included the nested exception text like 'does not exist' but in Spring 6, that detail seems to have moved specifically to the cause exception. "I'm not familiar with spring!!". Because the existing code only checks the top level message, it was failing to catch the 'table missing' case, which broke the test.
I updated it to check getCause().getMessage() as well so it behaves the same way it did before the upgrade.
what do you think?

Throwable cause = e.getCause();
boolean isMissingTable = (msg != null && msg.contains("does not exist")) ||
(cause != null && cause.getMessage() != null && cause.getMessage().contains("does not exist"));

if (isMissingTable) {
LOG.warn("Cannot perform {} since metastore table does not exist", callSig);
} else {
throw new MetaException("Unable to " + callSig + ":" + org.apache.hadoop.util.StringUtils.stringifyException(e));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.function.Function;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.Servlet;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
import org.eclipse.jetty.server.Server;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.util.Map;
import java.util.TreeMap;
import javax.net.ssl.SSLContext;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import java.util.TreeMap;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.metastore.PropertyServlet;
import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
Expand Down
5 changes: 5 additions & 0 deletions standalone-metastore/packaging/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,18 @@
<regexp>http://(www\.)?antlr.org/license.html</regexp>
<replacement>http://www.antlr.org/license.html</replacement>
</licenseUrlReplacement>
<licenseUrlReplacement>
<regexp>https?://www\.eclipse\.org/legal/epl-2\.0/?</regexp>
<replacement>https://www.eclipse.org/legal/epl-2.0</replacement>
</licenseUrlReplacement>
</licenseUrlReplacements>
<licenseUrlFileNames>
<APACHE-2.0>
https?://www\.apache\.org/licenses/LICENSE-2\.0.*
\Qhttps://aws.amazon.com/apache2.0\E
\Qhttps://opensource.org/licenses/Apache-2.0\E
\Qhttp://www.apache.org/licenses/\E
\Qhttps://repository.jboss.org/licenses/apache-2.0.txt\E
</APACHE-2.0>
<BSD-2-CLAUSE>
https?://(www\.)?opensource.org/licenses/bsd-license.php
Expand Down
Loading
Loading