Rotate non-renewable leases.

We now support rotation of non-renewable leases in the sense that we expire the lease first and then obtain secrets from Vault again. Previously, non-renewable secrets were not rotated at all as rotation was tied to the renewal only and without renewal there was no rotation.

See gh-215.

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;
+		}
 	}
 
 	/**

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<SecretLeaseEvent> events = new ArrayList<SecretLeaseEvent>();
+		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<Runnable> 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;
 	}