diff --git a/src/main/java/io/github/jopenlibs/vault/api/Logical.java b/src/main/java/io/github/jopenlibs/vault/api/Logical.java index a8a8977e..9a4bd129 100644 --- a/src/main/java/io/github/jopenlibs/vault/api/Logical.java +++ b/src/main/java/io/github/jopenlibs/vault/api/Logical.java @@ -98,9 +98,10 @@ private LogicalResponse read(final String path, final logicalOperations operatio .sslContext(config.getSslConfig().getSslContext()) .get(); - // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response + // Validate response - don't treat 4xx class errors as exceptions (except 412), we want to return an error as the response + // 412 (Precondition Failed) should be retried if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 - && restResponse.getStatus() < 500)) { + && restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) { throw new VaultException( "Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), @@ -157,9 +158,10 @@ public LogicalResponse read(final String path, Boolean shouldRetry, final Intege .sslContext(config.getSslConfig().getSslContext()) .get(); - // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response + // Validate response - don't treat 4xx class errors as exceptions (except 412), we want to return an error as the response + // 412 (Precondition Failed) should be retried if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 - && restResponse.getStatus() < 500)) { + && restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) { throw new VaultException( "Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), @@ -297,9 +299,10 @@ private LogicalResponse write(final String path, final Map nameV .post(); // HTTP Status should be either 200 (with content - e.g. PKI write) or 204 (no content) + // 412 (Precondition Failed) should be retried, so exclude it from the 4xx pass-through final int restStatus = restResponse.getStatus(); if (restStatus == 200 || restStatus == 204 || (restResponse.getStatus() >= 400 - && restResponse.getStatus() < 500)) { + && restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) { return new LogicalResponse(restResponse, attempt, operation); } else { throw new VaultException( diff --git a/src/test/java/io/github/jopenlibs/vault/RetryTests.java b/src/test/java/io/github/jopenlibs/vault/RetryTests.java index d1f88d40..c160cd23 100644 --- a/src/test/java/io/github/jopenlibs/vault/RetryTests.java +++ b/src/test/java/io/github/jopenlibs/vault/RetryTests.java @@ -53,4 +53,76 @@ public void testRetries_Write() throws Exception { VaultTestUtils.shutdownMockVault(server); } + @Test + public void testRetries_Read_412() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(3, 412, 200, + "{\"lease_id\":\"12345\",\"renewable\":false,\"lease_duration\":10000,\"data\":{\"value\":\"mock\"}}"); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").engineVersion(1).build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical().read("secret/hello"); + assertEquals(3, response.getRetries()); + assertEquals("mock", response.getData().get("value")); + + VaultTestUtils.shutdownMockVault(server); + } + + @Test + public void testRetries_Write_412() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(3, 412, 204, null); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical() + .write("secret/hello", new HashMap() {{ + put("value", "world"); + }}); + assertEquals(3, response.getRetries()); + + VaultTestUtils.shutdownMockVault(server); + } + + @Test + public void testNoRetries_Read_404() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(1, 404, 404, + "{\"errors\":[\"Not found\"]}"); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").engineVersion(1).build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical().read("secret/hello"); + assertEquals(0, response.getRetries()); + assertEquals(404, response.getRestResponse().getStatus()); + + VaultTestUtils.shutdownMockVault(server); + } + + @Test + public void testNoRetries_Write_400() throws Exception { + final RetriesMockVault retriesMockVault = new RetriesMockVault(1, 400, 400, + "{\"errors\":[\"Bad request\"]}"); + final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999") + .token("mock_token").build(); + final Vault vault = Vault.create(vaultConfig); + final LogicalResponse response = vault.withRetries(5, 100).logical() + .write("secret/hello", new HashMap() {{ + put("value", "world"); + }}); + assertEquals(0, response.getRetries()); + assertEquals(400, response.getRestResponse().getStatus()); + + VaultTestUtils.shutdownMockVault(server); + } + } diff --git a/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java b/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java index e33fa72f..52596681 100644 --- a/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java +++ b/src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java @@ -15,7 +15,7 @@ * *
    *
  1. - * RetriesMockVault responds with HTTP 500 status codes to a designated number of requests (which + * RetriesMockVault responds with a specified HTTP status code to a designated number of requests (which * can be zero). This can be used to test retry logic. *
    2. *
  2. @@ -46,11 +46,18 @@ public class RetriesMockVault extends MockVault { private final int mockStatus; private final String mockResponse; + private final int failureStatus; private int failureCount; public RetriesMockVault(final int failureCount, final int mockStatus, final String mockResponse) { + this(failureCount, 500, mockStatus, mockResponse); + } + + public RetriesMockVault(final int failureCount, final int failureStatus, + final int mockStatus, final String mockResponse) { this.failureCount = failureCount; + this.failureStatus = failureStatus; this.mockStatus = mockStatus; this.mockResponse = mockResponse; } @@ -66,8 +73,8 @@ public void handle( baseRequest.setHandled(true); if (failureCount > 0) { failureCount = failureCount - 1; - response.setStatus(500); - System.out.println("RetriesMockVault is sending an HTTP 500 code, to cause a retry..."); + response.setStatus(failureStatus); + System.out.println("RetriesMockVault is sending an HTTP " + failureStatus + " code, to trigger retry logic..."); } else { System.out.println("RetriesMockVault is sending an HTTP " + mockStatus + " code, with expected success payload...");