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

@@ -69,7 +69,7 @@ import org.springframework.web.client.HttpStatusCodeException;
  * SecretLeaseContainer container = new SecretLeaseContainer(vaultOperations,
  * 		taskScheduler);
  *
- * final RequestedSecret requestedSecret = container
+ * RequestedSecret requestedSecret = container
  * 		.requestRotatingSecret("mysql/creds/my-role");
  * container.addLeaseListener(new LeaseListenerAdapter() {
  * 	@Override
@@ -373,7 +373,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.getRequiredData());
 		}
 	}
@@ -470,34 +476,18 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements
 		}
 	}
 
-	void potentiallyScheduleLeaseRenewal(final RequestedSecret requestedSecret,
-			final Lease lease, final LeaseRenewalScheduler leaseRenewal) {
+	void scheduleLeaseRenewal(RequestedSecret requestedSecret, Lease lease,
+			LeaseRenewalScheduler leaseRenewal) {
 
-		if (!leaseRenewal.isLeaseRenewable(lease, requestedSecret)) {
-			return;
-		}
+		logRenewalCandidate(requestedSecret, lease, "renewal");
 
-		if (log.isDebugEnabled()) {
+		leaseRenewal.scheduleRenewal(requestedSecret, leaseToRenew -> {
 
-			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()));
-			}
-
-		}
-
-		leaseRenewal.scheduleRenewal(requestedSecret, lease1 -> {
-
-			Lease newLease = doRenewLease(requestedSecret, lease1);
+			Lease newLease = doRenewLease(requestedSecret, leaseToRenew);
 
 			if (!Lease.none().equals(newLease)) {
 
-				potentiallyScheduleLeaseRenewal(requestedSecret, newLease, leaseRenewal);
+				scheduleLeaseRenewal(requestedSecret, newLease, leaseRenewal);
 
 				onAfterLeaseRenewed(requestedSecret, newLease);
 			}
@@ -507,6 +497,35 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements
 
 	}
 
+	void scheduleLeaseRotation(RequestedSecret requestedSecret, Lease lease,
+			LeaseRenewalScheduler leaseRenewal) {
+
+		logRenewalCandidate(requestedSecret, lease, "rotation");
+
+		leaseRenewal.scheduleRenewal(requestedSecret, leaseToRotate -> {
+
+			onLeaseExpired(requestedSecret, leaseToRotate);
+
+			return Lease.none(); // rotation creates a new lease.
+			}, lease, getMinRenewal(), getExpiryThreshold());
+	}
+
+	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
 	// -------------------------------------------------------------------------
@@ -539,7 +558,7 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements
 	 * @param lease the lease.
 	 * @return the new lease or {@literal null} if expired/secret cannot be rotated.
 	 */
-	protected Lease doRenewLease(final RequestedSecret requestedSecret, final Lease lease) {
+	protected Lease doRenewLease(RequestedSecret requestedSecret, Lease lease) {
 
 		try {
 			Lease renewed = lease.hasLeaseId() ? renew(lease) : lease;
@@ -572,7 +591,7 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements
 	}
 
 	@SuppressWarnings("unchecked")
-	private Lease renew(final Lease lease) {
+	private Lease renew(Lease lease) {
 
 		ResponseEntity<Map<String, Object>> entity = operations
 				.doWithSession(restOperations -> (ResponseEntity) restOperations
@@ -615,7 +634,7 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements
 	 * @param lease must not be {@literal null}.
 	 */
 	@SuppressWarnings("unchecked")
-	protected void doRevokeLease(RequestedSecret requestedSecret, final Lease lease) {
+	protected void doRevokeLease(RequestedSecret requestedSecret, Lease lease) {
 
 		try {
 
@@ -673,9 +692,8 @@ public class SecretLeaseContainer extends SecretLeaseEventPublisher implements
 		 * prevent too many renewals in a very short timeframe.
 		 * @param expiryThreshold duration to renew before {@link Lease}.
 		 */
-		void scheduleRenewal(final RequestedSecret requestedSecret,
-				final RenewLease renewLease, final Lease lease,
-				final Duration minRenewal, final Duration expiryThreshold) {
+		void scheduleRenewal(RequestedSecret requestedSecret, RenewLease renewLease,
+				Lease lease, Duration minRenewal, Duration expiryThreshold) {
 
 			if (log.isDebugEnabled()) {
 				if (lease.hasLeaseId()) {
@@ -723,6 +741,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));
 					}
@@ -797,6 +819,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

@@ -16,6 +16,7 @@
 package org.springframework.vault.core.lease;
 
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
@@ -228,6 +229,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() {
 
@@ -534,13 +572,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;
 	}