diff --git a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/S3GatewayService.java b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/S3GatewayService.java index 02c3a365a4a1..4cbeddf0dce1 100644 --- a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/S3GatewayService.java +++ b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/S3GatewayService.java @@ -42,8 +42,6 @@ public void start(OzoneConfiguration conf) throws Exception { public void stop() throws Exception { Preconditions.assertNotNull(s3g, "S3 Gateway not running"); s3g.stop(); - // TODO (HDDS-11539): Remove this workaround once the @PreDestroy issue is fixed - OzoneClientCache.closeClient(); } @Override diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientCache.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientCache.java index ba7038d025f4..75afe46a92cf 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientCache.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientCache.java @@ -26,10 +26,13 @@ import java.security.cert.CertificateException; import java.util.Collections; import java.util.List; +import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; -import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.inject.Singleton; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.security.SecurityConfig; +import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.client.OzoneClient; @@ -37,30 +40,49 @@ import org.apache.hadoop.ozone.om.helpers.ServiceInfoEx; import org.apache.hadoop.ozone.om.protocol.S3Auth; import org.apache.hadoop.ozone.om.protocolPB.GrpcOmTransport; +import org.apache.ratis.util.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Cached ozone client for s3 requests. */ -@ApplicationScoped -public final class OzoneClientCache { +@Singleton +public class OzoneClientCache { + private static final Logger LOG = LoggerFactory.getLogger(OzoneClientCache.class); - // single, cached OzoneClient established on first connection - // for s3g gRPC OmTransport, OmRequest - OmResponse channel - private static OzoneClientCache instance; + + private final OzoneConfiguration conf; private OzoneClient client; - private SecurityConfig secConfig; - private OzoneClientCache(OzoneConfiguration ozoneConfiguration) - throws IOException { + @Inject + OzoneClientCache(OzoneConfiguration conf) { + this.conf = conf; + LOG.debug("{}: Created", this); + } + + @PostConstruct + public void initialize() throws IOException { + conf.set("ozone.om.group.rights", "NONE"); // Set the expected OM version if not set via config. - ozoneConfiguration.setIfUnset(OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY, + conf.setIfUnset(OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY, OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_DEFAULT); + conf.setBoolean(S3Auth.S3_AUTH_CHECK, true); + + client = createClient(conf); + Preconditions.assertTrue(conf.getBoolean(S3Auth.S3_AUTH_CHECK, false), S3Auth.S3_AUTH_CHECK); + + LOG.debug("{}: Initialized", this); + } + + public OzoneClient getClient() { + return client; + } + + public static OzoneClient createClient(OzoneConfiguration ozoneConfiguration) throws IOException { String omServiceID = OmUtils.getOzoneManagerServiceId(ozoneConfiguration); - secConfig = new SecurityConfig(ozoneConfiguration); - client = null; + SecurityConfig secConfig = new SecurityConfig(ozoneConfiguration); try { if (secConfig.isGrpcTlsEnabled()) { if (ozoneConfiguration @@ -70,43 +92,23 @@ private OzoneClientCache(OzoneConfiguration ozoneConfiguration) // Grpc transport selected // need to get certificate for TLS through // hadoop rpc first via ServiceInfo - setCertificate(omServiceID, - ozoneConfiguration); + setCertificate(secConfig, omServiceID, ozoneConfiguration); } } if (omServiceID == null) { - client = OzoneClientFactory.getRpcClient(ozoneConfiguration); + return OzoneClientFactory.getRpcClient(ozoneConfiguration); } else { // As in HA case, we need to pass om service ID. - client = OzoneClientFactory.getRpcClient(omServiceID, + return OzoneClientFactory.getRpcClient(omServiceID, ozoneConfiguration); } } catch (IOException e) { LOG.warn("cannot create OzoneClient", e); throw e; } - // S3 Gateway should always set the S3 Auth. - ozoneConfiguration.setBoolean(S3Auth.S3_AUTH_CHECK, true); - } - - public static OzoneClient getOzoneClientInstance(OzoneConfiguration - ozoneConfiguration) - throws IOException { - if (instance == null) { - instance = new OzoneClientCache(ozoneConfiguration); - } - return instance.client; - } - - public static void closeClient() throws IOException { - if (instance != null) { - instance.client.close(); - instance = null; - } } - private void setCertificate(String omServiceID, - OzoneConfiguration conf) + private static void setCertificate(SecurityConfig secConfig, String omServiceID, OzoneConfiguration conf) throws IOException { // create local copy of config incase exception occurs @@ -119,6 +121,7 @@ private void setCertificate(String omServiceID, // get certificates with service list request config.set(OZONE_OM_TRANSPORT_CLASS, OZONE_OM_TRANSPORT_CLASS_DEFAULT); + config.setBoolean(S3Auth.S3_AUTH_CHECK, false); if (omServiceID == null) { certClient = OzoneClientFactory.getRpcClient(config); @@ -161,7 +164,8 @@ private void setCertificate(String omServiceID, } @PreDestroy - public void destroy() throws IOException { - OzoneClientCache.closeClient(); + public void cleanup() { + LOG.debug("{}: Closing cached client", this); + IOUtils.close(LOG, client); } } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java index 1de4308ea6f8..7404019d0035 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java @@ -22,10 +22,6 @@ import javax.enterprise.context.RequestScoped; import javax.enterprise.inject.Produces; import javax.inject.Inject; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.container.ContainerRequestContext; -import javax.ws.rs.core.Context; -import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.client.OzoneClient; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,42 +38,17 @@ public class OzoneClientProducer { private OzoneClient client; @Inject - private OzoneConfiguration ozoneConfiguration; - - @Context - private ContainerRequestContext context; + private OzoneClientCache clientCache; @Produces - public synchronized OzoneClient createClient() throws WebApplicationException, - IOException { - ozoneConfiguration.set("ozone.om.group.rights", "NONE"); - client = getClient(ozoneConfiguration); + public OzoneClient createClient() { + client = clientCache.getClient(); return client; } @PreDestroy public void destroy() throws IOException { + LOG.debug("{}: Clearing thread-local auth", this); client.getObjectStore().getClientProxy().clearThreadLocalS3Auth(); } - - private OzoneClient getClient(OzoneConfiguration config) - throws IOException { - OzoneClient ozoneClient = null; - try { - ozoneClient = - OzoneClientCache.getOzoneClientInstance(ozoneConfiguration); - } catch (Exception e) { - // For any other critical errors during object creation throw Internal - // error. - if (LOG.isDebugEnabled()) { - LOG.debug("Error during Client Creation: ", e); - } - throw e; - } - return ozoneClient; - } - - public synchronized void setOzoneConfiguration(OzoneConfiguration config) { - this.ozoneConfiguration = config; - } } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java index e35c77653515..9a1a6af21ea4 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java @@ -18,10 +18,13 @@ package org.apache.hadoop.ozone.s3secret; import com.google.common.annotations.VisibleForTesting; +import java.io.IOException; import java.util.Map; +import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.core.Context; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.audit.AuditAction; import org.apache.hadoop.ozone.audit.AuditEventStatus; import org.apache.hadoop.ozone.audit.AuditLogger; @@ -29,6 +32,8 @@ import org.apache.hadoop.ozone.audit.AuditMessage; import org.apache.hadoop.ozone.audit.Auditor; import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.om.protocol.S3Auth; +import org.apache.hadoop.ozone.s3.OzoneClientCache; import org.apache.hadoop.ozone.s3.util.AuditUtils; /** @@ -36,15 +41,26 @@ */ public class S3SecretEndpointBase implements Auditor { + private final OzoneConfiguration conf; + private OzoneClient client; + @Context private ContainerRequestContext context; - @Inject - private OzoneClient client; - protected static final AuditLogger AUDIT = new AuditLogger(AuditLoggerType.S3GLOGGER); + @Inject + S3SecretEndpointBase(OzoneConfiguration conf) { + this.conf = new OzoneConfiguration(conf); + this.conf.setBoolean(S3Auth.S3_AUTH_CHECK, false); + } + + @PostConstruct + void initialize() throws IOException { + client = OzoneClientCache.createClient(conf); + } + protected String userNameFromRequest() { return context.getSecurityContext().getUserPrincipal().getName(); } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java index f1e201f4796d..648c9d26ed00 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java @@ -22,11 +22,13 @@ import jakarta.annotation.Nullable; import java.io.IOException; +import javax.inject.Inject; import javax.ws.rs.DELETE; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.core.Response; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.audit.S3GAction; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.S3SecretValue; @@ -43,6 +45,11 @@ public class S3SecretManagementEndpoint extends S3SecretEndpointBase { private static final Logger LOG = LoggerFactory.getLogger(S3SecretManagementEndpoint.class); + @Inject + S3SecretManagementEndpoint(OzoneConfiguration conf) { + super(conf); + } + @PUT public Response generate() throws IOException { return generateInternal(null); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientCache.java similarity index 70% rename from hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java rename to hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientCache.java index fa469f121526..f6b901d235b3 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientCache.java @@ -27,35 +27,28 @@ import org.junit.jupiter.api.Test; /** - * Test class for @{@link OzoneClientProducer}. + * Test class for @{@link OzoneClientCache}. */ -public class TestOzoneClientProducer { +public class TestOzoneClientCache { - private OzoneClientProducer producer; - - public TestOzoneClientProducer() - throws Exception { - producer = new OzoneClientProducer(); + @Test + public void testGetClientFailure() { OzoneConfiguration config = new OzoneConfiguration(); config.setBoolean(OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY, true); config.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, ""); - producer.setOzoneConfiguration(config); - } + OzoneClientCache subject = new OzoneClientCache(config); - @Test - public void testGetClientFailure() { - assertThrows(IOException.class, () -> producer.createClient(), - "testGetClientFailure"); + assertThrows(IOException.class, subject::initialize); } @Test public void testGetClientFailureWithMultipleServiceIds() { OzoneConfiguration configuration = new OzoneConfiguration(); configuration.set(OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY, "ozone1,ozone2"); - producer.setOzoneConfiguration(configuration); - IOException testGetClientFailure = assertThrows(IOException.class, () -> - producer.createClient(), "testGetClientFailureWithMultipleServiceIds"); - assertThat(testGetClientFailure.getMessage()) + OzoneClientCache subject = new OzoneClientCache(configuration); + + IOException e = assertThrows(IOException.class, subject::initialize); + assertThat(e.getMessage()) .contains("More than 1 OzoneManager ServiceID"); } @@ -64,13 +57,12 @@ public void testGetClientFailureWithMultipleServiceIdsAndInternalServiceId() { OzoneConfiguration configuration = new OzoneConfiguration(); configuration.set(OMConfigKeys.OZONE_OM_INTERNAL_SERVICE_ID, "ozone1"); configuration.set(OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY, "ozone1,ozone2"); - producer.setOzoneConfiguration(configuration); + OzoneClientCache subject = new OzoneClientCache(configuration); + // Still test will fail, as config is not complete. But it should pass // the service id check. - IOException testGetClientFailure = assertThrows(IOException.class, () -> - producer.createClient(), - "testGetClientFailureWithMultipleServiceIdsAndInternalServiceId"); - assertThat(testGetClientFailure.getMessage()) + IOException e = assertThrows(IOException.class, subject::initialize); + assertThat(e.getMessage()) .doesNotContain("More than 1 OzoneManager ServiceID"); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretGenerate.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretGenerate.java index 3fcd1d31d54b..72879c45f835 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretGenerate.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretGenerate.java @@ -79,7 +79,7 @@ void setUp() { when(uriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>()); when(context.getUriInfo()).thenReturn(uriInfo); - endpoint = new S3SecretManagementEndpoint(); + endpoint = new S3SecretManagementEndpoint(conf); endpoint.setClient(client); endpoint.setContext(context); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretRevoke.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretRevoke.java index 324197f5f65c..500cb7272dd6 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretRevoke.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretRevoke.java @@ -37,6 +37,7 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.SecurityContext; import javax.ws.rs.core.UriInfo; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.client.ObjectStoreStub; import org.apache.hadoop.ozone.client.OzoneClient; import org.apache.hadoop.ozone.client.OzoneClientStub; @@ -70,13 +71,14 @@ public class TestSecretRevoke { @BeforeEach void setUp() { + OzoneConfiguration conf = new OzoneConfiguration(); OzoneClient client = new OzoneClientStub(objectStore); when(uriInfo.getPathParameters()).thenReturn(new MultivaluedHashMap<>()); when(uriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>()); when(context.getUriInfo()).thenReturn(uriInfo); - endpoint = new S3SecretManagementEndpoint(); + endpoint = new S3SecretManagementEndpoint(conf); endpoint.setClient(client); endpoint.setContext(context); }