Skip to content

Commit 606a70a

Browse files
vbabenkoruvbabenkoru
authored
Fix: Retry HTTP 412 errors to support eventual consistency on standbys (#68)
Because tests cannot executed on this PR, I accept it anyway after a review
1 parent c0074c7 commit 606a70a

File tree

3 files changed

+90
-8
lines changed

3 files changed

+90
-8
lines changed

‎src/main/java/io/github/jopenlibs/vault/api/Logical.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,10 @@ private LogicalResponse read(final String path, final logicalOperations operatio
9898
.sslContext(config.getSslConfig().getSslContext())
9999
.get();
100100

101-
// Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response
101+
// Validate response - don't treat 4xx class errors as exceptions (except 412), we want to return an error as the response
102+
// 412 (Precondition Failed) should be retried
102103
if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400
103-
&& restResponse.getStatus() < 500)) {
104+
&& restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) {
104105
throw new VaultException(
105106
"Vault responded with HTTP status code: " + restResponse.getStatus()
106107
+ "\nResponse body: " + new String(restResponse.getBody(),
@@ -157,9 +158,10 @@ public LogicalResponse read(final String path, Boolean shouldRetry, final Intege
157158
.sslContext(config.getSslConfig().getSslContext())
158159
.get();
159160

160-
// Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response
161+
// Validate response - don't treat 4xx class errors as exceptions (except 412), we want to return an error as the response
162+
// 412 (Precondition Failed) should be retried
161163
if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400
162-
&& restResponse.getStatus() < 500)) {
164+
&& restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) {
163165
throw new VaultException(
164166
"Vault responded with HTTP status code: " + restResponse.getStatus()
165167
+ "\nResponse body: " + new String(restResponse.getBody(),
@@ -297,9 +299,10 @@ private LogicalResponse write(final String path, final Map<String, Object> nameV
297299
.post();
298300

299301
// HTTP Status should be either 200 (with content - e.g. PKI write) or 204 (no content)
302+
// 412 (Precondition Failed) should be retried, so exclude it from the 4xx pass-through
300303
final int restStatus = restResponse.getStatus();
301304
if (restStatus == 200 || restStatus == 204 || (restResponse.getStatus() >= 400
302-
&& restResponse.getStatus() < 500)) {
305+
&& restResponse.getStatus() < 500 && restResponse.getStatus() != 412)) {
303306
return new LogicalResponse(restResponse, attempt, operation);
304307
} else {
305308
throw new VaultException(

‎src/test/java/io/github/jopenlibs/vault/RetryTests.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,76 @@ public void testRetries_Write() throws Exception {
5353
VaultTestUtils.shutdownMockVault(server);
5454
}
5555

56+
@Test
57+
public void testRetries_Read_412() throws Exception {
58+
final RetriesMockVault retriesMockVault = new RetriesMockVault(3, 412, 200,
59+
"{\"lease_id\":\"12345\",\"renewable\":false,\"lease_duration\":10000,\"data\":{\"value\":\"mock\"}}");
60+
final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault);
61+
server.start();
62+
63+
final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999")
64+
.token("mock_token").engineVersion(1).build();
65+
final Vault vault = Vault.create(vaultConfig);
66+
final LogicalResponse response = vault.withRetries(5, 100).logical().read("secret/hello");
67+
assertEquals(3, response.getRetries());
68+
assertEquals("mock", response.getData().get("value"));
69+
70+
VaultTestUtils.shutdownMockVault(server);
71+
}
72+
73+
@Test
74+
public void testRetries_Write_412() throws Exception {
75+
final RetriesMockVault retriesMockVault = new RetriesMockVault(3, 412, 204, null);
76+
final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault);
77+
server.start();
78+
79+
final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999")
80+
.token("mock_token").build();
81+
final Vault vault = Vault.create(vaultConfig);
82+
final LogicalResponse response = vault.withRetries(5, 100).logical()
83+
.write("secret/hello", new HashMap<String, Object>() {{
84+
put("value", "world");
85+
}});
86+
assertEquals(3, response.getRetries());
87+
88+
VaultTestUtils.shutdownMockVault(server);
89+
}
90+
91+
@Test
92+
public void testNoRetries_Read_404() throws Exception {
93+
final RetriesMockVault retriesMockVault = new RetriesMockVault(1, 404, 404,
94+
"{\"errors\":[\"Not found\"]}");
95+
final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault);
96+
server.start();
97+
98+
final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999")
99+
.token("mock_token").engineVersion(1).build();
100+
final Vault vault = Vault.create(vaultConfig);
101+
final LogicalResponse response = vault.withRetries(5, 100).logical().read("secret/hello");
102+
assertEquals(0, response.getRetries());
103+
assertEquals(404, response.getRestResponse().getStatus());
104+
105+
VaultTestUtils.shutdownMockVault(server);
106+
}
107+
108+
@Test
109+
public void testNoRetries_Write_400() throws Exception {
110+
final RetriesMockVault retriesMockVault = new RetriesMockVault(1, 400, 400,
111+
"{\"errors\":[\"Bad request\"]}");
112+
final Server server = VaultTestUtils.initHttpMockVault(retriesMockVault);
113+
server.start();
114+
115+
final VaultConfig vaultConfig = new VaultConfig().address("http://127.0.0.1:8999")
116+
.token("mock_token").build();
117+
final Vault vault = Vault.create(vaultConfig);
118+
final LogicalResponse response = vault.withRetries(5, 100).logical()
119+
.write("secret/hello", new HashMap<String, Object>() {{
120+
put("value", "world");
121+
}});
122+
assertEquals(0, response.getRetries());
123+
assertEquals(400, response.getRestResponse().getStatus());
124+
125+
VaultTestUtils.shutdownMockVault(server);
126+
}
127+
56128
}

‎src/test/java/io/github/jopenlibs/vault/vault/mock/RetriesMockVault.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*
1616
* <ol>
1717
* <li>
18-
* <code>RetriesMockVault</code> responds with HTTP 500 status codes to a designated number of requests (which
18+
* <code>RetriesMockVault</code> responds with a specified HTTP status code to a designated number of requests (which
1919
* can be zero). This can be used to test retry logic.
2020
* </li>
2121
* <li>
@@ -46,11 +46,18 @@ public class RetriesMockVault extends MockVault {
4646

4747
private final int mockStatus;
4848
private final String mockResponse;
49+
private final int failureStatus;
4950
private int failureCount;
5051

5152
public RetriesMockVault(final int failureCount, final int mockStatus,
5253
final String mockResponse) {
54+
this(failureCount, 500, mockStatus, mockResponse);
55+
}
56+
57+
public RetriesMockVault(final int failureCount, final int failureStatus,
58+
final int mockStatus, final String mockResponse) {
5359
this.failureCount = failureCount;
60+
this.failureStatus = failureStatus;
5461
this.mockStatus = mockStatus;
5562
this.mockResponse = mockResponse;
5663
}
@@ -66,8 +73,8 @@ public void handle(
6673
baseRequest.setHandled(true);
6774
if (failureCount > 0) {
6875
failureCount = failureCount - 1;
69-
response.setStatus(500);
70-
System.out.println("RetriesMockVault is sending an HTTP 500 code, to cause a retry...");
76+
response.setStatus(failureStatus);
77+
System.out.println("RetriesMockVault is sending an HTTP " + failureStatus + " code, to trigger retry logic...");
7178
} else {
7279
System.out.println("RetriesMockVault is sending an HTTP " + mockStatus
7380
+ " code, with expected success payload...");

0 commit comments

Comments
 (0)