From 85e093d19ade6007949371db34c1aed129602135 Mon Sep 17 00:00:00 2001 From: Barry Klawans Date: Thu, 13 Jun 2019 10:42:31 -0700 Subject: [PATCH 1/2] Adding the ability to not treat 4xx response codes as errors that should be retried and converted into exceptions --- .../com/bettercloud/vault/VaultConfig.java | 25 +++++++++++++++++++ .../com/bettercloud/vault/api/Logical.java | 6 ++--- .../bettercloud/vault/VaultConfigTests.java | 8 ++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/bettercloud/vault/VaultConfig.java b/src/main/java/com/bettercloud/vault/VaultConfig.java index 9ba58ef2..fd89e6a9 100644 --- a/src/main/java/com/bettercloud/vault/VaultConfig.java +++ b/src/main/java/com/bettercloud/vault/VaultConfig.java @@ -40,6 +40,7 @@ public class VaultConfig implements Serializable { private int retryIntervalMilliseconds; private Integer globalEngineVersion; private String nameSpace; + private Boolean treatInvalidRequestsAsErrors = true; private EnvironmentLoader environmentLoader; /** @@ -207,6 +208,26 @@ public VaultConfig readTimeout(final Integer readTimeout) { return this; } + /** + *

Specifies whether 4xx class status codes should be treated as errors or invalid calls.

+ * + *

If 4xx status codes are treated as errors, any call that returns them will retry (if specified) + * and will result in an exception being thrown. If disabled, a 4xx return code does not retry, does + * not generate an exception, and instead returns the error in the response body

+ * + *

If you set this to false you need to check the response code before using the data + * in a response

+ * + *

The default behavior is to treat 4xx status codes as errors.

+ * + * @param treatInvalidRequestsAsErrors Should 4xx class status codes be treated as errors. + * @return This object, with treatInvalidRequestsAsErrors populated, ready for additional builder-pattern method calls or else finalization with the build() method + */ + public VaultConfig treatInvalidRequestsAsErrors(final Boolean treatInvalidRequestsAsErrors) { + this.treatInvalidRequestsAsErrors = treatInvalidRequestsAsErrors; + return this; + } + /** *

Sets the maximum number of times that an API operation will retry upon failure.

* @@ -314,6 +335,10 @@ public Integer getReadTimeout() { return readTimeout; } + public Boolean isTreatInvalidRequestsAsErrors() { + return treatInvalidRequestsAsErrors; + } + public int getMaxRetries() { return maxRetries; } diff --git a/src/main/java/com/bettercloud/vault/api/Logical.java b/src/main/java/com/bettercloud/vault/api/Logical.java index b6a379ac..2c0d8e0a 100644 --- a/src/main/java/com/bettercloud/vault/api/Logical.java +++ b/src/main/java/com/bettercloud/vault/api/Logical.java @@ -92,7 +92,7 @@ private LogicalResponse read(final String path, Boolean shouldRetry, final logic .get(); // Validate response - if (restResponse.getStatus() != 200) { + if (restResponse.getStatus() != 200 && !(!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), StandardCharsets.UTF_8), restResponse.getStatus()); @@ -161,7 +161,7 @@ public LogicalResponse read(final String path, Boolean shouldRetry, final Intege .get(); // Validate response - if (restResponse.getStatus() != 200) { + if (restResponse.getStatus() != 200 && !(!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), StandardCharsets.UTF_8), restResponse.getStatus()); @@ -261,7 +261,7 @@ private LogicalResponse write(final String path, final Map nameV // HTTP Status should be either 200 (with content - e.g. PKI write) or 204 (no content) final int restStatus = restResponse.getStatus(); - if (restStatus == 200 || restStatus == 204) { + if (restStatus == 200 || restStatus == 204 || (!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { return new LogicalResponse(restResponse, retryCount, operation); } else { throw new VaultException("Expecting HTTP status 204 or 200, but instead receiving " + restStatus diff --git a/src/test/java/com/bettercloud/vault/VaultConfigTests.java b/src/test/java/com/bettercloud/vault/VaultConfigTests.java index 90d437e9..c6d0cbf1 100644 --- a/src/test/java/com/bettercloud/vault/VaultConfigTests.java +++ b/src/test/java/com/bettercloud/vault/VaultConfigTests.java @@ -256,4 +256,12 @@ public void testConfigBuilder_WithNamespace() throws VaultException { Assert.assertEquals(vaultConfig.getNameSpace(), "namespace"); } + @Test + public void testConfigBuiler_WithInvalidRequestAsNonError() throws VaultException { + VaultConfig vaultConfig = new VaultConfig().address("address").token("token").treatInvalidRequestsAsErrors(false).build(); + assertEquals("address", vaultConfig.getAddress()); + assertEquals("token", vaultConfig.getToken()); + assertEquals(Boolean.FALSE, vaultConfig.isTreatInvalidRequestsAsErrors()); + + } } From 144917aabeb94d39e3f9d56a8e6035bd96da97bb Mon Sep 17 00:00:00 2001 From: Barry Klawans Date: Mon, 19 Aug 2019 11:25:18 -0700 Subject: [PATCH 2/2] Made not treating 400 class status codes as errors the default behavior --- .../com/bettercloud/vault/VaultConfig.java | 25 ------------------- .../com/bettercloud/vault/api/Logical.java | 10 ++++---- .../bettercloud/vault/VaultConfigTests.java | 9 ------- .../com/bettercloud/vault/VaultTests.java | 24 ++++++++++++++++++ 4 files changed, 29 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/bettercloud/vault/VaultConfig.java b/src/main/java/com/bettercloud/vault/VaultConfig.java index fd89e6a9..9ba58ef2 100644 --- a/src/main/java/com/bettercloud/vault/VaultConfig.java +++ b/src/main/java/com/bettercloud/vault/VaultConfig.java @@ -40,7 +40,6 @@ public class VaultConfig implements Serializable { private int retryIntervalMilliseconds; private Integer globalEngineVersion; private String nameSpace; - private Boolean treatInvalidRequestsAsErrors = true; private EnvironmentLoader environmentLoader; /** @@ -208,26 +207,6 @@ public VaultConfig readTimeout(final Integer readTimeout) { return this; } - /** - *

Specifies whether 4xx class status codes should be treated as errors or invalid calls.

- * - *

If 4xx status codes are treated as errors, any call that returns them will retry (if specified) - * and will result in an exception being thrown. If disabled, a 4xx return code does not retry, does - * not generate an exception, and instead returns the error in the response body

- * - *

If you set this to false you need to check the response code before using the data - * in a response

- * - *

The default behavior is to treat 4xx status codes as errors.

- * - * @param treatInvalidRequestsAsErrors Should 4xx class status codes be treated as errors. - * @return This object, with treatInvalidRequestsAsErrors populated, ready for additional builder-pattern method calls or else finalization with the build() method - */ - public VaultConfig treatInvalidRequestsAsErrors(final Boolean treatInvalidRequestsAsErrors) { - this.treatInvalidRequestsAsErrors = treatInvalidRequestsAsErrors; - return this; - } - /** *

Sets the maximum number of times that an API operation will retry upon failure.

* @@ -335,10 +314,6 @@ public Integer getReadTimeout() { return readTimeout; } - public Boolean isTreatInvalidRequestsAsErrors() { - return treatInvalidRequestsAsErrors; - } - public int getMaxRetries() { return maxRetries; } diff --git a/src/main/java/com/bettercloud/vault/api/Logical.java b/src/main/java/com/bettercloud/vault/api/Logical.java index 2c0d8e0a..3f8d545e 100644 --- a/src/main/java/com/bettercloud/vault/api/Logical.java +++ b/src/main/java/com/bettercloud/vault/api/Logical.java @@ -91,8 +91,8 @@ private LogicalResponse read(final String path, Boolean shouldRetry, final logic .sslContext(config.getSslConfig().getSslContext()) .get(); - // Validate response - if (restResponse.getStatus() != 200 && !(!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { + // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response + if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), StandardCharsets.UTF_8), restResponse.getStatus()); @@ -160,8 +160,8 @@ public LogicalResponse read(final String path, Boolean shouldRetry, final Intege .sslContext(config.getSslConfig().getSslContext()) .get(); - // Validate response - if (restResponse.getStatus() != 200 && !(!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { + // Validate response - don't treat 4xx class errors as exceptions, we want to return an error as the response + if (restResponse.getStatus() != 200 && !(restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus() + "\nResponse body: " + new String(restResponse.getBody(), StandardCharsets.UTF_8), restResponse.getStatus()); @@ -261,7 +261,7 @@ private LogicalResponse write(final String path, final Map nameV // HTTP Status should be either 200 (with content - e.g. PKI write) or 204 (no content) final int restStatus = restResponse.getStatus(); - if (restStatus == 200 || restStatus == 204 || (!config.isTreatInvalidRequestsAsErrors() && restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { + if (restStatus == 200 || restStatus == 204 || (restResponse.getStatus() >= 400 && restResponse.getStatus() < 500)) { return new LogicalResponse(restResponse, retryCount, operation); } else { throw new VaultException("Expecting HTTP status 204 or 200, but instead receiving " + restStatus diff --git a/src/test/java/com/bettercloud/vault/VaultConfigTests.java b/src/test/java/com/bettercloud/vault/VaultConfigTests.java index c6d0cbf1..230445e2 100644 --- a/src/test/java/com/bettercloud/vault/VaultConfigTests.java +++ b/src/test/java/com/bettercloud/vault/VaultConfigTests.java @@ -255,13 +255,4 @@ public void testConfigBuilder_WithNamespace() throws VaultException { VaultConfig vaultConfig = new VaultConfig().nameSpace("namespace").address("address").build(); Assert.assertEquals(vaultConfig.getNameSpace(), "namespace"); } - - @Test - public void testConfigBuiler_WithInvalidRequestAsNonError() throws VaultException { - VaultConfig vaultConfig = new VaultConfig().address("address").token("token").treatInvalidRequestsAsErrors(false).build(); - assertEquals("address", vaultConfig.getAddress()); - assertEquals("token", vaultConfig.getToken()); - assertEquals(Boolean.FALSE, vaultConfig.isTreatInvalidRequestsAsErrors()); - - } } diff --git a/src/test/java/com/bettercloud/vault/VaultTests.java b/src/test/java/com/bettercloud/vault/VaultTests.java index 5d6eee2c..502fe86b 100644 --- a/src/test/java/com/bettercloud/vault/VaultTests.java +++ b/src/test/java/com/bettercloud/vault/VaultTests.java @@ -1,11 +1,17 @@ package com.bettercloud.vault; +import com.bettercloud.vault.response.LogicalResponse; +import com.bettercloud.vault.vault.VaultTestUtils; +import com.bettercloud.vault.vault.mock.MockVault; +import org.eclipse.jetty.server.Server; import org.junit.Assert; import org.junit.Test; import java.util.HashMap; import java.util.Map; +import static junit.framework.TestCase.assertEquals; + /** * Unit tests for the various Vault constructors. @@ -88,4 +94,22 @@ public void kvEngineMapIsHonored() throws VaultException { Assert.assertEquals(String.valueOf(1), vault.logical().getEngineVersionForSecretPath("kv-v1").toString()); Assert.assertEquals(String.valueOf(2), vault.logical().getEngineVersionForSecretPath("notInMap").toString()); } + + @Test + public void testConfigBuiler_WithInvalidRequestAsNonError() throws Exception { + final MockVault mockVault = new MockVault(403, "{\"errors\":[\"preflight capability check returned 403, please ensure client's policies grant access to path \"path/that/does/not/exist/\"]}"); + final Server server = VaultTestUtils.initHttpMockVault(mockVault); + server.start(); + + final VaultConfig vaultConfig = new VaultConfig() + .address("http://127.0.0.1:8999") + .token("mock_token") + .build(); + final Vault vault = new Vault(vaultConfig); + + LogicalResponse response = vault.logical().read("path/that/does/not/exist/"); + VaultTestUtils.shutdownMockVault(server); + Assert.assertEquals(403, response.getRestResponse().getStatus()); + Assert.assertEquals(0, response.getRetries()); + } } \ No newline at end of file