diff --git a/spring-vault-core/src/main/java/org/springframework/vault/core/lease/SecretLeaseContainer.java b/spring-vault-core/src/main/java/org/springframework/vault/core/lease/SecretLeaseContainer.java index b5dfa5b8..8302f7e6 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/core/lease/SecretLeaseContainer.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/core/lease/SecretLeaseContainer.java @@ -311,7 +311,13 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements lease = Lease.none(); } - potentiallyScheduleLeaseRenewal(requestedSecret, lease, renewalScheduler); + if (renewalScheduler.isLeaseRenewable(lease, requestedSecret)) { + scheduleLeaseRenewal(requestedSecret, lease, renewalScheduler); + } + else if (renewalScheduler.isLeaseRotateOnly(lease, requestedSecret)) { + scheduleLeaseRotation(requestedSecret, lease, renewalScheduler); + } + onSecretsObtained(requestedSecret, lease, secrets.getData()); } } @@ -408,26 +414,10 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements } } - void potentiallyScheduleLeaseRenewal(final RequestedSecret requestedSecret, - final Lease lease, final LeaseRenewalScheduler leaseRenewal) { + void scheduleLeaseRenewal(final RequestedSecret requestedSecret, final Lease lease, + final LeaseRenewalScheduler leaseRenewal) { - if (!leaseRenewal.isLeaseRenewable(lease, requestedSecret)) { - return; - } - - if (log.isDebugEnabled()) { - - if (lease.hasLeaseId()) { - log.debug(String.format("Secret %s with Lease %s qualified for renewal", - requestedSecret.getPath(), lease.getLeaseId())); - } - else { - log.debug(String.format( - "Secret %s with cache hint is qualified for renewal", - requestedSecret.getPath())); - } - - } + logRenewalCandidate(requestedSecret, lease, "renewal"); leaseRenewal.scheduleRenewal(requestedSecret, new RenewLease() { @@ -438,8 +428,7 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements if (!Lease.none().equals(newLease)) { - potentiallyScheduleLeaseRenewal(requestedSecret, newLease, - leaseRenewal); + scheduleLeaseRenewal(requestedSecret, newLease, leaseRenewal); onAfterLeaseRenewed(requestedSecret, newLease); } @@ -449,6 +438,39 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements }, lease, getMinRenewalSeconds(), getExpiryThresholdSeconds()); } + void scheduleLeaseRotation(final RequestedSecret requestedSecret, final Lease lease, + final LeaseRenewalScheduler leaseRenewal) { + + logRenewalCandidate(requestedSecret, lease, "rotation"); + + leaseRenewal.scheduleRenewal(requestedSecret, new RenewLease() { + + @Override + public Lease renewLease(Lease lease) { + + onLeaseExpired(requestedSecret, lease); + + return Lease.none(); + } + }, lease, getMinRenewalSeconds(), getExpiryThresholdSeconds()); + } + + private static void logRenewalCandidate(RequestedSecret requestedSecret, Lease lease, + String action) { + + if (log.isDebugEnabled()) { + + if (lease.hasLeaseId()) { + log.debug(String.format("Secret %s with Lease %s qualified for %s", + requestedSecret.getPath(), lease.getLeaseId(), action)); + } + else { + log.debug(String.format("Secret %s with cache hint is qualified for %s", + requestedSecret.getPath(), action)); + } + } + } + // ------------------------------------------------------------------------- // Implementation hooks and helper methods // ------------------------------------------------------------------------- @@ -674,6 +696,10 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements } try { + + // Renew lease may call scheduleRenewal(…) with a different lease + // Id to alter set up its own renewal schedule. If it's the old + // lease, then renewLease() outcome controls the current LeaseId. currentLeaseRef .compareAndSet(lease, renewLease.renewLease(lease)); } @@ -747,6 +773,16 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements public Lease getLease() { return currentLeaseRef.get(); } + + private boolean isLeaseRotateOnly(Lease lease, RequestedSecret requestedSecret) { + + if (lease == null) { + return false; + } + + return lease.hasLeaseId() && !lease.isRenewable() + && requestedSecret.getMode() == Mode.ROTATE; + } } /** diff --git a/spring-vault-core/src/test/java/org/springframework/vault/core/lease/SecretLeaseContainerUnitTests.java b/spring-vault-core/src/test/java/org/springframework/vault/core/lease/SecretLeaseContainerUnitTests.java index 89bdfdb8..0279d2b5 100644 --- a/spring-vault-core/src/test/java/org/springframework/vault/core/lease/SecretLeaseContainerUnitTests.java +++ b/spring-vault-core/src/test/java/org/springframework/vault/core/lease/SecretLeaseContainerUnitTests.java @@ -15,6 +15,7 @@ */ package org.springframework.vault.core.lease; +import java.util.ArrayList; import java.util.Collections; import java.util.Date; import java.util.HashMap; @@ -215,6 +216,43 @@ public class SecretLeaseContainerUnitTests { verify(taskScheduler, times(2)).schedule(captor.capture(), any(Trigger.class)); } + @Test + public void shouldRotateNonRenewableLease() { + + final List events = new ArrayList(); + when(taskScheduler.schedule(any(Runnable.class), any(Trigger.class))).thenReturn( + scheduledFuture); + + when(vaultOperations.read(requestedSecret.getPath())).thenReturn( + createSecrets("key", "value", false), + createSecrets("key", "value2", false)); + + secretLeaseContainer.addRequestedSecret(RequestedSecret.rotating(requestedSecret + .getPath())); + secretLeaseContainer.addLeaseListener(new LeaseListenerAdapter() { + @Override + public void onLeaseEvent(SecretLeaseEvent leaseEvent) { + events.add(leaseEvent); + } + }); + + secretLeaseContainer.start(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Runnable.class); + verify(taskScheduler).schedule(captor.capture(), any(Trigger.class)); + + captor.getValue().run(); + verify(taskScheduler, times(2)).schedule(captor.capture(), any(Trigger.class)); + + assertThat(events).hasSize(3); + assertThat(events.get(0)).isInstanceOf(SecretLeaseCreatedEvent.class); + assertThat(events.get(1)).isInstanceOf(SecretLeaseExpiredEvent.class); + assertThat(events.get(2)).isInstanceOf(SecretLeaseCreatedEvent.class); + + SecretLeaseCreatedEvent rotated = (SecretLeaseCreatedEvent) events.get(2); + assertThat(rotated.getSecrets()).containsEntry("key", "value2"); + } + @Test public void shouldRotateGenericSecret() { @@ -521,13 +559,17 @@ public class SecretLeaseContainerUnitTests { } private VaultResponse createSecrets() { + return createSecrets("key", "value", true); + } + + private VaultResponse createSecrets(String key, String value, boolean renewable) { VaultResponse secrets = new VaultResponse(); secrets.setLeaseId("lease"); - secrets.setRenewable(true); + secrets.setRenewable(renewable); secrets.setLeaseDuration(100); - secrets.setData(Collections.singletonMap("key", (Object) "value")); + secrets.setData(Collections.singletonMap(key, (Object) value)); return secrets; }