diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java index 22bdf80a7aa9..c2183b21c0a8 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java @@ -1395,6 +1395,17 @@ public CertificateRenewerService(boolean forceRenewal, @Override public void run() { + try { + renewCertificateIfNeeded(); + } catch (RuntimeException e) { + // This task is scheduled at a fixed rate: an exception escaping it cancels every future + // execution, and the component stops renewing its certificate without any further notice. + getLogger().error("Certificate renewal for {} failed unexpectedly, keeping the renewal " + + "schedule.", component, e); + } + } + + private void renewCertificateIfNeeded() { // Lock to protect the certificate renew process, to make sure there is // only one renew process is ongoing at one time. // Certificate renew steps: diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java index ee2a52b2f634..2a9d44902958 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/client/TestDefaultCertificateClient.java @@ -25,6 +25,7 @@ import static org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient.InitResponse.FAILURE; import static org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateCodec.getPEMEncodedString; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -49,6 +50,7 @@ import java.security.cert.X509Certificate; import java.time.Duration; import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.RandomStringUtils; @@ -586,4 +588,52 @@ protected String signAndStoreCertificate(CertificateSignRequest request, Path ce .count(); assertThat(monitorThreadCount).isEqualTo(0L); } + + /** + * A renewal that fails with an unchecked exception must not let it escape the renewer task. + * The task is scheduled at a fixed rate, so an escaping exception cancels every further + * execution and the component stops renewing its certificate until it is restarted. + */ + @Test + public void testRenewerContainsUnexpectedFailure(@TempDir File metaDir) + throws Exception { + OzoneConfiguration ozoneConf = new OzoneConfiguration(); + ozoneConf.set(HDDS_METADATA_DIR_NAME, metaDir.getPath()); + SecurityConfig conf = new SecurityConfig(ozoneConf); + String compName = "test-unexpected-failure"; + + CertificateCodec certCodec = new CertificateCodec(conf, compName); + X509Certificate cert = generateX509Cert(null); + certCodec.writeCertificate(cert); + String certId = cert.getSerialNumber().toString(); + + AtomicInteger attempts = new AtomicInteger(); + DefaultCertificateClient client = new DefaultCertificateClient( + conf, null, mock(Logger.class), certId, compName, "", null, null) { + + @Override + protected SCMGetCertResponseProto sign(CertificateSignRequest request) { + return null; + } + + @Override + protected String signAndStoreCertificate(CertificateSignRequest request, Path certificatePath, boolean renew) { + return null; + } + + @Override + public String renewAndStoreKeyAndCertificate(boolean force) { + attempts.incrementAndGet(); + throw new IllegalStateException("renewal failed unexpectedly"); + } + }; + + try { + // Runs exactly what the scheduled task runs. + assertDoesNotThrow(client.new CertificateRenewerService(true, () -> { })::run); + assertThat(attempts.get()).isPositive(); + } finally { + client.close(); + } + } }