Repository: kroxylicious/kroxylicious
Pull request #817 (2023-12-12 01:12:04)

Set requestTimeout == connectTimeout on vault KMS
### Type of change

- Enhancement / new feature

### Description

Why:
This will cause the KMS to complete the future exceptionally if too much time elapses between connection and the client receiving headers. Note that no timeout will occur if there is a subsequent delay, like if the server sent headers but hangs and doesn't send any response body. Handling that case is a future enhancement as it would involve setting up our own timeout future and scheduling that somewhere.

Contributes to #795 but only implements the simple bit, setting request timeout

### Checklist

- [ ] Write tests
- [ ] Make sure all tests pass
- [ ] Review performance test results. Ensure that any degradations to performance numbers are understood and justified.
- [ ] Make sure all Sonar-Lint warnings are addressed or are justifiably ignored.
- [ ] Update documentation
- [ ] Reference relevant issue(s) and close them after merging
- [ ] For user facing changes, update CHANGELOG.md (remember to include changes affecting the API of the test artefacts too).


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=CODE_SMELL)

[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list)


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&types=CODE_SMELL)

[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[2 New issues](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&inNewCodePeriod=true)  
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[2 New issues](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&inNewCodePeriod=true)  
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[1 New issue](https://sonarcloud.io/project/issues?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=kroxylicious_kroxylicious&pullRequest=817&resolved=false&inNewCodePeriod=true)  
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=kroxylicious_kroxylicious&pullRequest=817&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=kroxylicious_kroxylicious&pullRequest=817)



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7cae182c..bbe5e16e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@ Please enumerate **all user-facing** changes using format `<githib issue/pr numb
 
 ## 0.4.0
 
+* [#817](https://github.com/kroxylicious/kroxylicious/pull/817): Encryption Filter: Set hardcoded request timeout on Vault requests
 * [#798](https://github.com/kroxylicious/kroxylicious/pull/798): Encryption Filter: Refactor Serialization to new Parcel Scheme
 * [#809](https://github.com/kroxylicious/kroxylicious/pull/809): Bump Kroxylicious Junit Ext from 0.7.0 to 0.8.0
 * [#803](https://github.com/kroxylicious/kroxylicious/pull/803): Bump kafka.version from 3.6.0 to 3.6.1 #803
diff --git a/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKms.java b/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKms.java
index 4cca0041..3d7df46c 100644
--- a/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKms.java
+++ b/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKms.java
@@ -55,17 +55,24 @@ public class VaultKms implements Kms<String, VaultEdek> {
 
     private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
     private static final String AES_KEY_ALGO = "AES";
+    private final Duration timeout;
+    private final HttpClient vaultClient;
 
-    private final HttpClient vaultClient = HttpClient.newBuilder()
-            .followRedirects(HttpClient.Redirect.NORMAL)
-            .connectTimeout(Duration.ofSeconds(20))
-            .build();
     private final URI vaultUrl;
     private final String vaultToken;
 
-    VaultKms(URI vaultUrl, String vaultToken) {
+    VaultKms(URI vaultUrl, String vaultToken, Duration timeout) {
         this.vaultUrl = vaultUrl;
         this.vaultToken = vaultToken;
+        this.timeout = timeout;
+        vaultClient = createClient();
+    }
+
+    private HttpClient createClient() {
+        return HttpClient.newBuilder()
+                .followRedirects(HttpClient.Redirect.NORMAL)
+                .connectTimeout(timeout)
+                .build();
     }
 
     /**
@@ -158,6 +165,7 @@ public class VaultKms implements Kms<String, VaultEdek> {
 
     private HttpRequest.Builder createVaultRequest() {
         return HttpRequest.newBuilder()
+                .timeout(timeout)
                 .header("X-Vault-Token", vaultToken)
                 .header("Accept", "application/json");
     }
diff --git a/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsService.java b/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsService.java
index 0f0b42f8..0d0033b7 100644
--- a/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsService.java
+++ b/kroxylicious-kms-provider-hashicorp-vault/src/main/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsService.java
@@ -7,6 +7,7 @@
 package io.kroxylicious.kms.provider.hashicorp.vault;
 
 import java.net.URI;
+import java.time.Duration;
 import java.util.Objects;
 
 import io.kroxylicious.kms.service.KmsService;
@@ -35,7 +36,7 @@ public class VaultKmsService implements KmsService<VaultKmsService.Config, Strin
     @NonNull
     @Override
     public VaultKms buildKms(Config options) {
-        return new VaultKms(options.vaultUrl(), options.vaultToken());
+        return new VaultKms(options.vaultUrl(), options.vaultToken(), Duration.ofSeconds(20));
     }
 
 }


Test patch:
diff --git a/kroxylicious-kms-provider-hashicorp-vault/src/test/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsTest.java b/kroxylicious-kms-provider-hashicorp-vault/src/test/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsTest.java
new file mode 100644
index 00000000..543e2b09
--- /dev/null
+++ b/kroxylicious-kms-provider-hashicorp-vault/src/test/java/io/kroxylicious/kms/provider/hashicorp/vault/VaultKmsTest.java
@@ -0,0 +1,94 @@
+/*
+ * Copyright Kroxylicious Authors.
+ *
+ * Licensed under the Apache Software License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+
+package io.kroxylicious.kms.provider.hashicorp.vault;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.URI;
+import java.net.http.HttpConnectTimeoutException;
+import java.net.http.HttpTimeoutException;
+import java.time.Duration;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+import com.sun.net.httpserver.HttpServer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class VaultKmsTest {
+
+    // address in TEST-NET-1, reserved for use in example specifications and other documents
+    private static final URI NON_ROUTABLE = URI.create("http://192.0.2.1/");
+    private static final byte ARBITRARY_BODY_BYTE = 5;
+
+    @Test
+    void testConnectionTimeout() {
+        VaultKms kms = new VaultKms(NON_ROUTABLE, "token", Duration.ofMillis(500));
+        CompletableFuture<String> alias = kms.resolveAlias("alias");
+        assertThat(alias).failsWithin(Duration.ofSeconds(2))
+                .withThrowableOfType(ExecutionException.class)
+                .withCauseInstanceOf(HttpConnectTimeoutException.class);
+    }
+
+    @Test
+    void testRequestTimeoutWaitingForHeaders() {
+        HttpServer httpServer = delayResponse(Duration.ofSeconds(1), Duration.ZERO);
+        URI address = URI.create("http://" + httpServer.getAddress().getHostName() + ":" + httpServer.getAddress().getPort() + "/");
+        VaultKms kms = new VaultKms(address, "token", Duration.ofMillis(500));
+        CompletableFuture<String> alias = kms.resolveAlias("alias");
+        assertThat(alias).failsWithin(Duration.ofSeconds(2))
+                .withThrowableOfType(ExecutionException.class)
+                .withCauseInstanceOf(HttpTimeoutException.class);
+        httpServer.stop(0);
+    }
+
+    @Disabled("JDK http client request timeout cancelled after headers received currently, todo add our own timeout future")
+    @Test
+    void testRequestTimeoutWaitingForBody() {
+        HttpServer httpServer = delayResponse(Duration.ZERO, Duration.ofSeconds(1));
+        URI address = URI.create("http://" + httpServer.getAddress().getHostName() + ":" + httpServer.getAddress().getPort() + "/");
+        VaultKms kms = new VaultKms(address, "token", Duration.ofMillis(500));
+        CompletableFuture<String> alias = kms.resolveAlias("alias");
+        assertThat(alias).failsWithin(Duration.ofSeconds(2))
+                .withThrowableOfType(ExecutionException.class)
+                .withCauseInstanceOf(HttpTimeoutException.class);
+        httpServer.stop(0);
+    }
+
+    public static HttpServer delayResponse(Duration delayHeaders, Duration delayResponseAfterHeaders) {
+        try {
+            HttpServer server = HttpServer.create(new InetSocketAddress(InetAddress.getLocalHost(), 0), 0);
+            server.createContext("/", exchange -> {
+                sleep(delayHeaders);
+                exchange.sendResponseHeaders(200, 1);
+                sleep(delayResponseAfterHeaders);
+                exchange.getResponseBody().write(ARBITRARY_BODY_BYTE);
+                exchange.close();
+            });
+            server.setExecutor(null); // creates a default executor
+            server.start();
+            return server;
+        }
+        catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private static void sleep(Duration delayHeaders) {
+        try {
+            Thread.sleep(delayHeaders.toMillis());
+        }
+        catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new RuntimeException(e);
+        }
+    }
+}