Skip to content

Commit 14626b1

Browse files
committed
Resume lease renewal after restart.
Secret renewals are resumed after restarting secrets by re-registering LeaseRenewalScheduler. Also, guard concurrent renewals that might have been in progress while the renewal scheduler has been in progress. Closes gh-867
1 parent 114f944 commit 14626b1

File tree

2 files changed

+91
-1
lines changed

2 files changed

+91
-1
lines changed

spring-vault-core/src/main/java/org/springframework/vault/core/lease/SecretLeaseContainer.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ void restartSecrets() {
549549
Lease previousLease = getPreviousLease(previousLeases, requestedSecret);
550550

551551
try {
552+
this.renewals.put(requestedSecret, renewalScheduler);
552553
doStart(requestedSecret, renewalScheduler, (secrets, lease) -> {
553554
onSecretsRotated(requestedSecret, previousLease, lease, secrets.getRequiredData());
554555
}, () -> {
@@ -807,7 +808,19 @@ private Lease doRenew(Lease lease) {
807808
protected void onLeaseExpired(RequestedSecret requestedSecret, Lease lease) {
808809

809810
if (requestedSecret.getMode() == Mode.ROTATE) {
810-
doStart(requestedSecret, this.renewals.get(requestedSecret), (secrets, currentLease) -> {
811+
LeaseRenewalScheduler renewalScheduler = this.renewals.get(requestedSecret);
812+
// prevent races for concurrent renewals of the same secret using different
813+
// leases
814+
if (renewalScheduler == null || !renewalScheduler.leaseEquals(lease)) {
815+
logger.debug(String.format(
816+
"Skipping rotation after renewal expiry for secret %s with lease %s as no LeaseRenewalScheduler is found. This can happen if leases have been restarted while concurrent expiry processing.",
817+
requestedSecret.getPath(), lease.getLeaseId()));
818+
819+
super.onLeaseExpired(requestedSecret, lease);
820+
return;
821+
}
822+
823+
doStart(requestedSecret, renewalScheduler, (secrets, currentLease) -> {
811824
onSecretsRotated(requestedSecret, lease, currentLease, secrets.getRequiredData());
812825
}, () -> super.onLeaseExpired(requestedSecret, lease));
813826
}
@@ -1023,6 +1036,10 @@ private boolean isLeaseRotateOnly(Lease lease, RequestedSecret requestedSecret)
10231036
&& requestedSecret.getMode() == Mode.ROTATE;
10241037
}
10251038

1039+
boolean leaseEquals(Lease lease) {
1040+
return getLease() == lease;
1041+
}
1042+
10261043
}
10271044

10281045
private class LeaseAuthenticationEventListener implements AuthenticationListener, AuthenticationErrorListener {

spring-vault-core/src/test/java/org/springframework/vault/core/lease/SecretLeaseContainerUnitTests.java

+73
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.springframework.http.HttpStatus;
3838
import org.springframework.scheduling.TaskScheduler;
3939
import org.springframework.scheduling.Trigger;
40+
import org.springframework.test.util.ReflectionTestUtils;
4041
import org.springframework.vault.VaultException;
4142
import org.springframework.vault.core.RestOperationsCallback;
4243
import org.springframework.vault.core.VaultOperations;
@@ -479,6 +480,78 @@ void shouldNotRotateExpiringLease() {
479480
assertThat(((SecretLeaseCreatedEvent) events.get(1)).getSecrets()).containsOnlyKeys("foo");
480481
}
481482

483+
@SuppressWarnings("unchecked")
484+
@Test
485+
void secretShouldContinueOnRestartSecrets() {
486+
487+
when(this.taskScheduler.schedule(any(Runnable.class), any(Trigger.class))).thenReturn(this.scheduledFuture);
488+
489+
VaultResponse first = createSecrets();
490+
VaultResponse second = createSecrets();
491+
second.setData(Collections.singletonMap("foo", (Object) "bar"));
492+
493+
when(this.vaultOperations.read(this.requestedSecret.getPath())).thenReturn(first, second);
494+
when(this.vaultOperations.doWithSession(any(RestOperationsCallback.class)))
495+
.thenReturn(Lease.of("after_restart", Duration.ofSeconds(1), true));
496+
497+
RequestedSecret secret = RequestedSecret.rotating("my-secret");
498+
499+
this.secretLeaseContainer.setExpiryThreshold(Duration.ofSeconds(1));
500+
this.secretLeaseContainer.addRequestedSecret(secret);
501+
this.secretLeaseContainer.start();
502+
503+
ArgumentCaptor<Runnable> captor = ArgumentCaptor.forClass(Runnable.class);
504+
this.secretLeaseContainer.restartSecrets();
505+
506+
verify(this.taskScheduler, times(2)).schedule(captor.capture(), any(Trigger.class));
507+
captor.getAllValues().get(0).run(); // old lease run
508+
captor.getAllValues().get(1).run(); // new lease run
509+
510+
// one from restartSecrets, the second from the scheduler detecting expiry
511+
verify(this.leaseListenerAdapter, times(2)).onLeaseEvent(any(SecretLeaseRotatedEvent.class));
512+
verify(this.leaseListenerAdapter, never()).onLeaseEvent(any(SecretLeaseExpiredEvent.class));
513+
verify(this.leaseListenerAdapter, never()).onLeaseEvent(any(SecretLeaseErrorEvent.class));
514+
}
515+
516+
@SuppressWarnings("unchecked")
517+
@Test
518+
void concurrentRenewalAfterRestartSecretShouldContinueOnRestartSecrets() {
519+
520+
when(this.taskScheduler.schedule(any(Runnable.class), any(Trigger.class))).thenReturn(this.scheduledFuture);
521+
522+
VaultResponse first = createSecrets();
523+
VaultResponse second = createSecrets();
524+
second.setData(Collections.singletonMap("foo", (Object) "bar"));
525+
526+
when(this.vaultOperations.read(this.requestedSecret.getPath())).thenReturn(first, second);
527+
when(this.vaultOperations.doWithSession(any(RestOperationsCallback.class)))
528+
.thenReturn(Lease.of("after_restart", Duration.ofSeconds(1), true));
529+
530+
RequestedSecret secret = RequestedSecret.rotating("my-secret");
531+
532+
this.secretLeaseContainer.setExpiryThreshold(Duration.ofSeconds(1));
533+
this.secretLeaseContainer.addRequestedSecret(secret);
534+
this.secretLeaseContainer.start();
535+
536+
Map<RequestedSecret, SecretLeaseContainer.LeaseRenewalScheduler> renewals = (Map<RequestedSecret, SecretLeaseContainer.LeaseRenewalScheduler>) ReflectionTestUtils.getField(this.secretLeaseContainer, "renewals");
537+
538+
SecretLeaseContainer.LeaseRenewalScheduler scheduler = renewals.get(secret);
539+
Lease lease = scheduler.getLease();
540+
ArgumentCaptor<Runnable> captor = ArgumentCaptor.forClass(Runnable.class);
541+
this.secretLeaseContainer.restartSecrets();
542+
543+
scheduler.associateLease(lease);
544+
545+
verify(this.taskScheduler, times(2)).schedule(captor.capture(), any(Trigger.class));
546+
captor.getAllValues().get(1).run(); // new lease run
547+
captor.getAllValues().get(0).run(); // old lease run
548+
549+
// one from restartSecrets, the second from the scheduler detecting expiry
550+
verify(this.leaseListenerAdapter, times(2)).onLeaseEvent(any(SecretLeaseRotatedEvent.class));
551+
verify(this.leaseListenerAdapter).onLeaseEvent(any(SecretLeaseExpiredEvent.class));
552+
verify(this.leaseListenerAdapter, never()).onLeaseEvent(any(SecretLeaseErrorEvent.class));
553+
}
554+
482555
@Test
483556
void scheduleRenewalShouldApplyExpiryThreshold() {
484557

0 commit comments

Comments
 (0)