This is an automated email from the ASF dual-hosted git repository.
pifta pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new aff7f87429 HDDS-9010. Fix RootCaRotationPoller not toggling rotation
if previous rotation failed (#5060)
aff7f87429 is described below
commit aff7f8742939c3bb13365c7b1a132dd74a3ffbaa
Author: Galsza <[email protected]>
AuthorDate: Tue Jul 18 14:19:04 2023 +0200
HDDS-9010. Fix RootCaRotationPoller not toggling rotation if previous
rotation failed (#5060)
---
.../client/DefaultCertificateClient.java | 14 +++++---
.../certificate/client/RootCaRotationPoller.java | 18 ++++++++--
.../utils/TestRootCaRotationPoller.java | 40 ++++++++++++++++++++++
3 files changed, 66 insertions(+), 6 deletions(-)
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 e066efef8c..6077895817 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
@@ -205,7 +205,7 @@ public abstract class DefaultCertificateClient implements
CertificateClient {
}
@Override
- public void registerRootCARotationListener(
+ public synchronized void registerRootCARotationListener(
Function<List<X509Certificate>, CompletableFuture<Void>> listener) {
if (securityConfig.isAutoCARotationEnabled()) {
rootCaRotationPoller.addRootCARotationProcessor(listener);
@@ -1333,7 +1333,8 @@ public abstract class DefaultCertificateClient implements
CertificateClient {
return CompletableFuture.completedFuture(null);
}
CertificateRenewerService renewerService =
- new CertificateRenewerService(true);
+ new CertificateRenewerService(
+ true, rootCaRotationPoller::setCertificateRenewalError);
return CompletableFuture.runAsync(renewerService, executorService);
}
@@ -1355,7 +1356,8 @@ public abstract class DefaultCertificateClient implements
CertificateClient {
.setDaemon(true).build());
}
this.executorService.scheduleAtFixedRate(
- new CertificateRenewerService(false),
+ new CertificateRenewerService(false, () -> {
+ }),
timeBeforeGracePeriod, interval, TimeUnit.MILLISECONDS);
getLogger().info("CertificateRenewerService for {} is started with " +
"first delay {} ms and interval {} ms.", component,
@@ -1367,9 +1369,12 @@ public abstract class DefaultCertificateClient
implements CertificateClient {
*/
public class CertificateRenewerService implements Runnable {
private boolean forceRenewal;
+ private Runnable rotationErrorCallback;
- public CertificateRenewerService(boolean forceRenewal) {
+ public CertificateRenewerService(boolean forceRenewal,
+ Runnable rotationErrorCallback) {
this.forceRenewal = forceRenewal;
+ this.rotationErrorCallback = rotationErrorCallback;
}
@Override
@@ -1397,6 +1402,7 @@ public abstract class DefaultCertificateClient implements
CertificateClient {
timeLeft, forceRenewal);
newCertId = renewAndStoreKeyAndCertificate(forceRenewal);
} catch (CertificateException e) {
+ rotationErrorCallback.run();
if (e.errorCode() ==
CertificateException.ErrorCode.ROLLBACK_ERROR) {
if (shutdownCallback != null) {
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/RootCaRotationPoller.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/RootCaRotationPoller.java
index 47cc368bbe..2656eef92d 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/RootCaRotationPoller.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/RootCaRotationPoller.java
@@ -39,6 +39,7 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -55,6 +56,7 @@ public class RootCaRotationPoller implements Runnable,
Closeable {
private final Duration pollingInterval;
private Set<X509Certificate> knownRootCerts;
private final SCMSecurityProtocolClientSideTranslatorPB scmSecureClient;
+ private final AtomicBoolean certificateRenewalError;
public RootCaRotationPoller(SecurityConfig securityConfig,
Set<X509Certificate> initiallyKnownRootCaCerts,
@@ -67,6 +69,7 @@ public class RootCaRotationPoller implements Runnable,
Closeable {
.setDaemon(true).build());
pollingInterval = securityConfig.getRootCaCertificatePollingInterval();
rootCARotationProcessors = new ArrayList<>();
+ certificateRenewalError = new AtomicBoolean(false);
}
private void pollRootCas() {
@@ -86,15 +89,22 @@ public class RootCaRotationPoller implements Runnable,
Closeable {
getPrintableCertIds(knownRootCerts) + ". Root CA Cert ids from " +
"SCM not known by the client: " +
getPrintableCertIds(scmCertsWithoutKnownCerts));
-
+ certificateRenewalError.set(false);
CompletableFuture<Void> allRootCAProcessorFutures =
CompletableFuture.allOf(rootCARotationProcessors.stream()
.map(c -> c.apply(rootCAsFromSCM))
.toArray(CompletableFuture[]::new));
allRootCAProcessorFutures.whenComplete((unused, throwable) -> {
- if (throwable == null) {
+ if (throwable == null && !certificateRenewalError.get()) {
knownRootCerts = new HashSet<>(rootCAsFromSCM);
+ } else {
+ LOG.info("Certificate consumption was unsuccesfull. " +
+ (certificateRenewalError.get() ?
+ "There was a caught exception when trying to sign the " +
+ "certificate" :
+ "There was an unexpected error during cert rotation" +
+ throwable));
}
});
} catch (IOException e) {
@@ -134,6 +144,10 @@ public class RootCaRotationPoller implements Runnable,
Closeable {
}
}
+ public void setCertificateRenewalError() {
+ certificateRenewalError.set(true);
+ }
+
private String getPrintableCertIds(Collection<X509Certificate> certs) {
return certs.stream()
.map(X509Certificate::getSerialNumber)
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCaRotationPoller.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCaRotationPoller.java
index 7f2ed0a813..90ac9daac5 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCaRotationPoller.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestRootCaRotationPoller.java
@@ -42,6 +42,7 @@ import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_ROOTCA_CERTIFICATE_POLLING_INTERVAL;
@@ -51,6 +52,7 @@ import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_ROOTCA_CERTIFICATE
public class TestRootCaRotationPoller {
private SecurityConfig secConf;
+ private GenericTestUtils.LogCapturer logCapturer;
@Mock
private SCMSecurityProtocolClientSideTranslatorPB scmSecurityClient;
@@ -61,6 +63,8 @@ public class TestRootCaRotationPoller {
OzoneConfiguration conf = new OzoneConfiguration();
conf.set(HDDS_X509_ROOTCA_CERTIFICATE_POLLING_INTERVAL, "PT1s");
secConf = new SecurityConfig(conf);
+ logCapturer = GenericTestUtils.LogCapturer.captureLogs(
+ org.slf4j.LoggerFactory.getLogger(RootCaRotationPoller.class));
}
@Test
@@ -116,6 +120,42 @@ public class TestRootCaRotationPoller {
GenericTestUtils.waitFor(atomicBoolean::get, 50, 5000);
}
+ @Test
+ public void testPollerRetriesAfterFailure() throws Exception {
+ X509Certificate knownCert = generateX509Cert(
+ LocalDateTime.now(), Duration.ofSeconds(50));
+ X509Certificate newRootCa = generateX509Cert(
+ LocalDateTime.now(), Duration.ofSeconds(50));
+ HashSet<X509Certificate> knownCerts = new HashSet<>();
+ knownCerts.add(knownCert);
+ List<String> certsFromScm = new ArrayList<>();
+ certsFromScm.add(CertificateCodec.getPEMEncodedString(knownCert));
+ certsFromScm.add(CertificateCodec.getPEMEncodedString(newRootCa));
+ RootCaRotationPoller poller = new RootCaRotationPoller(secConf,
+ knownCerts, scmSecurityClient);
+ poller.run();
+ Mockito.when(scmSecurityClient.getAllRootCaCertificates())
+ .thenReturn(certsFromScm);
+ AtomicBoolean atomicBoolean = new AtomicBoolean();
+ atomicBoolean.set(false);
+ //Set that the first certificate renewal encountered an error
+ AtomicInteger runNumber = new AtomicInteger(1);
+ poller.addRootCARotationProcessor(
+ certificates -> CompletableFuture.supplyAsync(() -> {
+ if (runNumber.getAndIncrement() < 2) {
+ poller.setCertificateRenewalError();
+ }
+ atomicBoolean.set(true);
+ Assertions.assertEquals(certificates.size(), 2);
+ return null;
+ }));
+ //Assert for the poller success even if there was an error at first
+ GenericTestUtils.waitFor(atomicBoolean::get, 50, 5000);
+ //And that we see the error in the logs
+ Assertions.assertTrue(logCapturer.getOutput().contains(
+ "There was a caught exception when trying to sign the certificate"));
+ }
+
private X509Certificate generateX509Cert(
LocalDateTime startDate, Duration certLifetime) throws Exception {
KeyPair keyPair = KeyStoreTestUtil.generateKeyPair("RSA");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
