fapifta commented on code in PR #5649:
URL: https://github.com/apache/ozone/pull/5649#discussion_r1418853692
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java:
##########
@@ -18,357 +18,374 @@
package org.apache.hadoop.ozone.container.ozoneimpl;
-import org.apache.hadoop.fs.FileUtil;
import org.apache.hadoop.hdds.HddsConfigKeys;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
import
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
import
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandResponseProto;
-import org.apache.hadoop.hdds.scm.container.ContainerID;
-import org.apache.hadoop.hdds.scm.pipeline.MockPipeline;
+import org.apache.hadoop.hdds.scm.XceiverClientManager;
+import org.apache.hadoop.hdds.scm.XceiverClientManager.ScmClientConfig;
+import org.apache.hadoop.hdds.scm.client.ClientTrustManager;
import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient;
import org.apache.hadoop.hdds.security.token.ContainerTokenIdentifier;
import org.apache.hadoop.hdds.security.token.ContainerTokenSecretManager;
import
org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClientTestImpl;
-import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.client.SecretKeyTestClient;
-import org.apache.hadoop.ozone.container.ContainerTestHelper;
import org.apache.hadoop.hdds.scm.XceiverClientGrpc;
import org.apache.hadoop.hdds.scm.XceiverClientSpi;
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
import org.apache.hadoop.ozone.container.common.ContainerTestUtils;
+import org.apache.hadoop.ozone.container.common.statemachine.StateContext;
import org.apache.hadoop.ozone.container.replication.SimpleContainerDownloader;
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.token.Token;
-import org.apache.ozone.test.GenericTestUtils;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.rules.TestRule;
-import org.junit.rules.Timeout;
-import org.apache.ozone.test.JUnit5AwareTimeout;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.ozone.test.GenericTestUtils.LogCapturer;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import java.io.File;
+import java.io.IOException;
import java.nio.file.Path;
import java.security.cert.CertificateExpiredException;
-import java.time.Duration;
+import java.security.cert.X509Certificate;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.UUID;
-import java.util.concurrent.TimeUnit;
-import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_KEY_DIR_NAME;
-import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_KEY_DIR_NAME_DEFAULT;
-import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_KEY_LEN;
-import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_ACK_TIMEOUT;
-import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static java.time.Duration.ofSeconds;
+import static java.util.Collections.singletonList;
+import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_EXPIRY_TIME;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_GRPC_TLS_ENABLED;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_GRPC_TLS_TEST_CERT;
import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_DEFAULT_DURATION;
import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_GRACE_DURATION_TOKEN_CHECKS_ENABLED;
import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY;
+import static org.apache.hadoop.hdds.scm.container.ContainerID.valueOf;
+import static org.apache.hadoop.hdds.scm.pipeline.MockPipeline.createPipeline;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.HDDS_DATANODE_CONTAINER_DB_DIR;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY;
+import static
org.apache.hadoop.ozone.container.ContainerTestHelper.getCloseContainer;
+import static
org.apache.hadoop.ozone.container.ContainerTestHelper.getCreateContainerSecureRequest;
+import static
org.apache.hadoop.ozone.container.ContainerTestHelper.getTestContainerID;
import static
org.apache.hadoop.ozone.container.replication.CopyContainerCompression.NO_COMPRESSION;
+import static org.apache.ozone.test.GenericTestUtils.LogCapturer.captureLogs;
+import static org.apache.ozone.test.GenericTestUtils.waitFor;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.slf4j.LoggerFactory.getLogger;
/**
* Tests ozone containers via secure grpc/netty.
*/
-@RunWith(Parameterized.class)
+@Timeout(300)
public class TestOzoneContainerWithTLS {
- private static final Logger LOG = LoggerFactory.getLogger(
- TestOzoneContainerWithTLS.class);
- /**
- * Set the timeout for every test.
- */
- @Rule
- public TestRule testTimeout = new JUnit5AwareTimeout(Timeout.seconds(300));
- @Rule
- public TemporaryFolder tempFolder = new TemporaryFolder();
+ private static final int CERT_LIFETIME = 10; // seconds
+ private static final int ROOT_CERT_LIFE_TIME = 20; // seconds
+ private String clusterID;
private OzoneConfiguration conf;
- private ContainerTokenSecretManager secretManager;
+ private DatanodeDetails dn;
+ private Pipeline pipeline;
private CertificateClientTestImpl caClient;
- private SecretKeyClient secretKeyClient;
- private boolean containerTokenEnabled;
- private int certLifetime = 15 * 1000; // 15s
-
- public TestOzoneContainerWithTLS(boolean enableToken) {
- this.containerTokenEnabled = enableToken;
- }
+ private SecretKeyClient keyClient;
+ private ContainerTokenSecretManager secretManager;
- @Parameterized.Parameters
- public static Collection<Object[]> enableBlockToken() {
- return Arrays.asList(new Object[][] {
- {false},
- {true}
- });
- }
+ @TempDir
+ private File tempFolder;
- @Before
+ @BeforeEach
public void setup() throws Exception {
conf = new OzoneConfiguration();
- String ozoneMetaPath =
- GenericTestUtils.getTempPath("ozoneMeta");
- File ozoneMetaFile = new File(ozoneMetaPath);
- conf.set(OZONE_METADATA_DIRS, ozoneMetaPath);
-
- FileUtil.fullyDelete(ozoneMetaFile);
- String keyDirName = conf.get(HDDS_KEY_DIR_NAME,
- HDDS_KEY_DIR_NAME_DEFAULT);
-
- File ozoneKeyDir = new File(ozoneMetaFile, keyDirName);
- ozoneKeyDir.mkdirs();
+ conf.set(OZONE_METADATA_DIRS, tempFolder.getPath() + "/ozoneMeta");
+ conf.set(HDDS_DATANODE_DIR_KEY, tempFolder.getPath() + "/dn");
+ conf.set(HDDS_DATANODE_CONTAINER_DB_DIR,
+ tempFolder.getPath() + "/container.db");
conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
- conf.setBoolean(HddsConfigKeys.HDDS_GRPC_TLS_ENABLED, true);
-
- conf.setBoolean(HddsConfigKeys.HDDS_GRPC_TLS_TEST_CERT, true);
+ conf.setBoolean(HDDS_GRPC_TLS_ENABLED, true);
+ conf.setBoolean(HDDS_GRPC_TLS_TEST_CERT, true);
conf.setBoolean(HDDS_X509_GRACE_DURATION_TOKEN_CHECKS_ENABLED, false);
- conf.setInt(HDDS_KEY_LEN, 1024);
-
- // certificate lives for 10s
- conf.set(HDDS_X509_DEFAULT_DURATION,
- Duration.ofMillis(certLifetime).toString());
- conf.set(HDDS_X509_RENEW_GRACE_DURATION, "PT2S");
- conf.set(HDDS_X509_CA_ROTATION_CHECK_INTERNAL, "PT1S"); // 1s
- conf.set(HDDS_X509_CA_ROTATION_ACK_TIMEOUT, "PT1S"); // 1s
-
- long expiryTime = conf.getTimeDuration(
- HddsConfigKeys.HDDS_BLOCK_TOKEN_EXPIRY_TIME, "1s",
- TimeUnit.MILLISECONDS);
-
- caClient = new CertificateClientTestImpl(conf);
- secretKeyClient = new SecretKeyTestClient();
- secretManager = new ContainerTokenSecretManager(expiryTime,
- secretKeyClient);
+ conf.set(HDDS_X509_DEFAULT_DURATION, ofSeconds(CERT_LIFETIME).toString());
+ conf.set(HddsConfigKeys.HDDS_X509_MAX_DURATION,
+ ofSeconds(ROOT_CERT_LIFE_TIME).toString());
+ conf.set(HDDS_X509_RENEW_GRACE_DURATION, ofSeconds(2).toString());
+ conf.setInt(HDDS_BLOCK_TOKEN_EXPIRY_TIME, 1000);
+
+ clusterID = UUID.randomUUID().toString();
+ caClient = new CertificateClientTestImpl(conf, false);
+ keyClient = new SecretKeyTestClient();
+ secretManager = new ContainerTokenSecretManager(1000, keyClient);
+
+ dn = aDatanode();
+ pipeline = createPipeline(singletonList(dn));
}
- @Test(expected = CertificateExpiredException.class)
- public void testCertificateLifetime() throws Exception {
- // Sleep to wait for certificate expire
- LocalDateTime now = LocalDateTime.now();
- now = now.plusSeconds(certLifetime / 1000);
- caClient.getCertificate().checkValidity(Date.from(
- now.atZone(ZoneId.systemDefault()).toInstant()));
+ @Test
+ public void testCertificateLifetime() {
+ Date atTime = Date.from(LocalDateTime.now()
+ .plusSeconds(CERT_LIFETIME)
+ .atZone(ZoneId.systemDefault())
+ .toInstant());
+
+ assertThrows(CertificateExpiredException.class,
+ () -> caClient.getCertificate().checkValidity(atTime));
}
- @Test
- public void testCreateOzoneContainer() throws Exception {
- LOG.info("testCreateOzoneContainer with TLS and containerToken enabled:
{}",
- containerTokenEnabled);
+ @ParameterizedTest(name = "Container token enabled: {0}")
+ @ValueSource(booleans = {false, true})
+ public void testCreateOzoneContainer(boolean containerTokenEnabled)
+ throws Exception {
conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED,
containerTokenEnabled);
+ OzoneContainer container = createAndStartOzoneContainerInstance();
- long containerId = ContainerTestHelper.getTestContainerID();
- OzoneContainer container = null;
-
System.out.println(System.getProperties().getProperty("java.library.path"));
- DatanodeDetails dn = MockDatanodeDetails.randomDatanodeDetails();
- try {
- Pipeline pipeline = MockPipeline.createSingleNodePipeline();
- conf.set(HDDS_DATANODE_DIR_KEY, tempFolder.getRoot().getPath());
- conf.setInt(OzoneConfigKeys.DFS_CONTAINER_IPC_PORT,
- pipeline.getFirstNode().getPort(DatanodeDetails.Port.Name.STANDALONE)
- .getValue());
- conf.setBoolean(
- OzoneConfigKeys.DFS_CONTAINER_IPC_RANDOM_PORT, false);
-
- container = new OzoneContainer(dn, conf, ContainerTestUtils
- .getMockContext(dn, conf), caClient, secretKeyClient);
- //Set scmId and manually start ozone container.
- container.start(UUID.randomUUID().toString());
-
- try (XceiverClientGrpc client = new XceiverClientGrpc(pipeline, conf,
- Collections.singletonList(caClient.getCACertificate()))) {
-
- if (containerTokenEnabled) {
- client.connect();
- createSecureContainer(client, containerId,
- secretManager.generateToken(
- UserGroupInformation.getCurrentUser().getUserName(),
- ContainerID.valueOf(containerId)));
- } else {
- client.connect();
- createContainer(client, containerId);
- }
- }
+ try (XceiverClientGrpc client =
+ new XceiverClientGrpc(pipeline, conf, aClientTrustManager())) {
+ client.connect();
+
+ createContainer(client, containerTokenEnabled, getTestContainerID());
} finally {
if (container != null) {
container.stop();
}
}
}
- @Test
- public void testContainerDownload() throws Exception {
- DatanodeDetails dn = MockDatanodeDetails.createDatanodeDetails(
- UUID.randomUUID().toString(), "localhost", "0.0.0.0",
- "/default-rack");
- Pipeline pipeline = MockPipeline.createSingleNodePipeline();
- conf.set(HDDS_DATANODE_DIR_KEY, tempFolder.newFolder().getPath());
- conf.setInt(OzoneConfigKeys.DFS_CONTAINER_IPC_PORT,
- pipeline.getFirstNode().getPort(DatanodeDetails.Port.Name.STANDALONE)
- .getValue());
- conf.setBoolean(OzoneConfigKeys.DFS_CONTAINER_IPC_RANDOM_PORT, false);
+ @ParameterizedTest(name = "Container token enabled: {0}")
+ @ValueSource(booleans = {false, true})
+ public void testContainerDownload(boolean containerTokenEnabled)
+ throws Exception {
+ conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED,
+ containerTokenEnabled);
+ OzoneContainer container = createAndStartOzoneContainerInstance();
- OzoneContainer container = null;
+ ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class);
+ XceiverClientManager clientManager =
+ new XceiverClientManager(conf, scmClientConf, aClientTrustManager());
+ XceiverClientSpi client = null;
try {
- container = new OzoneContainer(dn, conf, ContainerTestUtils
- .getMockContext(dn, conf), caClient, secretKeyClient);
+ client = clientManager.acquireClient(pipeline);
+ // at this point we have an established connection from the client to
+ // the container, and we do not expect a new SSL handshake while we are
+ // running container ops until the renewal, however it may happen, as
+ // the protocol can do a renegotiation at any time, so this dynamic
+ // introduces a very low chance of flakiness.
+ // The downloader client when it connects first, will do a failing
+ // handshake that we are expecting because before downloading, we wait
+ // for the expiration without renewing the certificate.
+ List<Long> containers = new ArrayList<>();
+ List<DatanodeDetails> sourceDatanodes = new ArrayList<>();
+ sourceDatanodes.add(dn);
- // Set scmId and manually start ozone container.
- container.start(UUID.randomUUID().toString());
+ containers.add(createAndCloseContainer(client, containerTokenEnabled));
+ letCertExpire();
+ containers.add(createAndCloseContainer(client, containerTokenEnabled));
+ assertDownloadContainerFails(containers.get(0), sourceDatanodes);
- // Create containers
- long containerId = ContainerTestHelper.getTestContainerID();
- List<Long> containerIdList = new ArrayList<>();
- XceiverClientGrpc client = new XceiverClientGrpc(pipeline, conf,
- Collections.singletonList(caClient.getCACertificate()));
- client.connect();
- if (containerTokenEnabled) {
- Token<ContainerTokenIdentifier> token = secretManager.generateToken(
- UserGroupInformation.getCurrentUser().getUserName(),
- ContainerID.valueOf(containerId));
- createSecureContainer(client, containerId, token);
- closeSecureContainer(client, containerId, token);
- } else {
- createContainer(client, containerId);
- closeContainer(client, containerId);
+ caClient.renewKey();
+ containers.add(createAndCloseContainer(client, containerTokenEnabled));
+ assertDownloadContainerWorks(containers, sourceDatanodes);
+ } finally {
+ if (container != null) {
+ container.stop();
}
- containerIdList.add(containerId++);
+ if (client != null) {
+ clientManager.releaseClient(client, true);
+ }
+ }
+ }
- // Wait certificate to expire
- GenericTestUtils.waitFor(() ->
- caClient.getCertificate().getNotAfter().before(new Date()),
- 100, certLifetime);
+ @Test
+ public void testLongLivingClientWithCertRenews() throws Exception {
+ LogCapturer logs = captureLogs(getLogger(ClientTrustManager.class));
+ OzoneContainer container = createAndStartOzoneContainerInstance();
- List<DatanodeDetails> sourceDatanodes = new ArrayList<>();
- sourceDatanodes.add(dn);
- if (containerTokenEnabled) {
- // old client still function well after certificate expired
- Token<ContainerTokenIdentifier> token = secretManager.generateToken(
- UserGroupInformation.getCurrentUser().getUserName(),
- ContainerID.valueOf(containerId));
- createSecureContainer(client, containerId, token);
- closeSecureContainer(client, containerId++, token);
- } else {
- createContainer(client, containerId);
- closeContainer(client, containerId++);
- }
+ ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class);
+ scmClientConf.setStaleThreshold(500);
+ XceiverClientManager clientManager =
+ new XceiverClientManager(conf, scmClientConf, aClientTrustManager());
+ assertClientTrustManagerLoading(true, logs, "Client loaded certificates.");
- // Download newly created container will fail because of cert expired
- GenericTestUtils.LogCapturer logCapture = GenericTestUtils.LogCapturer
- .captureLogs(SimpleContainerDownloader.LOG);
- SimpleContainerDownloader downloader =
- new SimpleContainerDownloader(conf, caClient);
- Path file = downloader.getContainerDataFromReplicas(
- containerId, sourceDatanodes, null, NO_COMPRESSION);
- downloader.close();
- Assert.assertNull(file);
- Assert.assertTrue(logCapture.getOutput().contains(
- "java.security.cert.CertificateExpiredException"));
+ XceiverClientSpi client = null;
+ try {
+ client = clientManager.acquireClient(pipeline);
+ createAndCloseContainer(client, false);
+ assertClientTrustManagerLoading(false, logs,
+ "Check client did not reloaded certificates.");
+
+ letCACertExpire();
+
+ // works based off of initial SSL handshake
+ createAndCloseContainer(client, false);
+ assertClientTrustManagerLoading(false, logs,
+ "Check client did not reloaded certificates.");
+
+ clientManager.releaseClient(client, true);
+ client = clientManager.acquireClient(pipeline);
+ assertClientTrustManagerLoading(false, logs,
+ "Check second client creation does not reload certificates.");
+ // aim is to only load certs at first start, and in case of a
verification
+ // failure
+
+ try {
+ // fails after a new client is created, as we just have expired certs
+ // in the source (caClient), so container ops still fail even though
+ // a retry/reload happens, but we expect it to happen before the
failure
+ createAndCloseContainer(client, false);
+ assertClientTrustManagerFailedAndRetried(logs);
Review Comment:
Yes you are right, moved it into the catch clause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
