Adjust timeout for health check + bump spring (#78)

entur · Feb 29, 2024 · e29b4d1 · e29b4d1
1 parent 7e9768c
commit e29b4d1
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 13 deletions.
diff --git a/examples/jwt-client-spring-boot-example/pom.xml b/examples/jwt-client-spring-boot-example/pom.xml
@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>org.springframework.boot</groupId>
 		<artifactId>spring-boot-starter-parent</artifactId>
-		<version>3.2.2</version>
+		<version>3.2.3</version>
 		<relativePath /> <!-- lookup parent from repository -->
 	</parent>
 	<groupId>org.entur.example</groupId>

diff --git a/examples/jwt-resource-server-spring-boot-example/pom.xml b/examples/jwt-resource-server-spring-boot-example/pom.xml
@@ -4,7 +4,7 @@
 	<parent>
 		<groupId>org.springframework.boot</groupId>
 		<artifactId>spring-boot-starter-parent</artifactId>
-		<version>3.2.2</version>
+		<version>3.2.3</version>
 		<relativePath /> <!-- lookup parent from repository -->
 	</parent>
 	<groupId>org.entur.example</groupId>

diff --git a/...ver/spring/jwt-spring-common/src/main/java/org/entur/jwt/spring/JwtAutoConfiguration.java b/...ver/spring/jwt-spring-common/src/main/java/org/entur/jwt/spring/JwtAutoConfiguration.java
@@ -3,6 +3,7 @@
 import org.entur.jwt.spring.actuate.ListJwksHealthIndicator;
 import org.entur.jwt.spring.properties.JwtProperties;
 import org.entur.jwt.spring.properties.SecurityProperties;
+import org.entur.jwt.spring.properties.jwk.JwkProperties;
 import org.entur.jwt.spring.properties.jwk.JwtTenantProperties;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -33,8 +34,16 @@ public class JwtAutoConfiguration {
 
     @Bean(destroyMethod = "close", value = "jwks")
     @ConditionalOnEnabledHealthIndicator("jwks")
-    public ListJwksHealthIndicator jwksHealthIndicator() {
-        return new ListJwksHealthIndicator(1000L, Executors.newCachedThreadPool());
+    public ListJwksHealthIndicator jwksHealthIndicator(SecurityProperties properties) {
+        JwtProperties jwtProperties = properties.getJwt();
+        JwkProperties jwk = jwtProperties.getJwk();
+
+        // TODO should also the number of active provides be taken into account? Don't want the
+        // health check http response to time out
+
+        long maxDelay = (jwk.getConnectTimeout() + jwk.getReadTimeout()) * 1000;
+
+        return new ListJwksHealthIndicator(maxDelay, Executors.newCachedThreadPool());
     }
 
     @Bean(destroyMethod = "close")

diff --git a/...jwt-spring-common/src/main/java/org/entur/jwt/spring/actuate/ListJwksHealthIndicator.java b/...jwt-spring-common/src/main/java/org/entur/jwt/spring/actuate/ListJwksHealthIndicator.java
@@ -7,9 +7,12 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 public class ListJwksHealthIndicator extends AbstractJwksHealthIndicator implements Closeable {
 
@@ -30,7 +33,7 @@ public void addHealthIndicators(DefaultJwksHealthIndicator healthIndicator) {
     }
 
     @Override
-    public void close() throws IOException {
+    public void close() {
         this.executorService.shutdown();
     }
 
@@ -61,13 +64,21 @@ protected JwksHealth getJwksHealth() {
             return new JwksHealth(time, false);
         }
 
-        // refresh multiple sources, wrap in completion service to them all at once
+        // refresh multiple sources, wrap in completion service to visit them all in parallel
         ExecutorCompletionService completionService = new ExecutorCompletionService(executorService);
 
         List<Future<Boolean>> workerList = new ArrayList<>(unhealthy.size());
 
-        for (DefaultJwksHealthIndicator defaultJwksHealthIndicator : unhealthy) {
-            Future<Boolean> future = completionService.submit(() -> defaultJwksHealthIndicator.refreshJwksHealth(time));
+        for (DefaultJwksHealthIndicator unhealthyJwksHealthIndicator : unhealthy) {
+            Future<Boolean> future = completionService.submit(() -> {
+                try {
+                    return unhealthyJwksHealthIndicator.refreshJwksHealth(time);
+                } catch(Exception e) {
+                    LOGGER.warn("Problem getting JWKs health", e);
+
+                    return false;
+                }
+            });
 
             workerList.add(future);
         }
@@ -76,6 +87,8 @@ protected JwksHealth getJwksHealth() {
             try {
                 Thread.sleep(maxDelay);
 
+                LOGGER.warn("Timeout collecting " + workerList.size() + " JWKs health after" + maxDelay + "ms");
+
                 for (Future<Boolean> worker : workerList) {
                     worker.cancel(true);
                 }
@@ -98,8 +111,14 @@ protected JwksHealth getJwksHealth() {
                     if (status != null && status) {
                         continue;
                     }
+                } catch(CancellationException e) {
+                    // ignore but return false
                 } catch (InterruptedException e) {
+                    // ignore but return false
                     Thread.currentThread().interrupt();
+                } catch (ExecutionException e) {
+                    // this should not happen as the jobs are wrapped in try - catch
+                    LOGGER.warn("Problem getting health info", e);
                 } catch (Exception e) {
                     // ignore
                     LOGGER.info("Problem getting health info", e);

diff --git a/...-webflux/src/test/java/org/entur/jwt/spring/rest/GreetingControllerAuthenticatedTest.java b/...-webflux/src/test/java/org/entur/jwt/spring/rest/GreetingControllerAuthenticatedTest.java
@@ -73,7 +73,7 @@ public void testSecurityHeaders(@AccessToken(audience = "mock.my.audience") Stri
             .getResponseHeaders();
 
         assertThat(responseHeaders.get("X-Content-Type-Options")).contains("nosniff");
-        assertThat(responseHeaders.get("X-XSS-Protection")).contains("1 ; mode=block");
+        assertThat(responseHeaders.get("X-XSS-Protection")).contains("1; mode=block");
         assertThat(responseHeaders.get("Cache-Control")).contains("no-cache, no-store, max-age=0, must-revalidate");
         assertThat(responseHeaders.get("Pragma")).contains("no-cache");
         assertThat(responseHeaders.get("Expires")).contains("0");

diff --git a/pom.xml b/pom.xml
@@ -59,15 +59,15 @@
 		<junit.version>5.10.2</junit.version>
 		<mockito.version>5.10.0</mockito.version>
 		<hamcrest.version>2.2</hamcrest.version>
-		<slf4j.version>2.0.9</slf4j.version>
+		<slf4j.version>2.0.12</slf4j.version>
 		<jjwt.version>0.12.5</jjwt.version>
 		<nimbus-jose-jwt.version>9.37.3</nimbus-jose-jwt.version>
 		<jackson-databind.version>2.16.1</jackson-databind.version>
 		<jadler.version>1.3.1</jadler.version>
-		<spring-boot.version>3.2.2</spring-boot.version>
+		<spring-boot.version>3.2.3</spring-boot.version>
 		<httpclient.version>5.2.3</httpclient.version>
 		<!-- added due to security issue -->
-		<spring-security.version>6.2.1</spring-security.version>
+		<spring-security.version>6.2.2</spring-security.version>
 		<spring.version>6.1.4</spring.version>
 		<rest-assured.version>5.4.0</rest-assured.version>
 		<okhttp.version>4.12.0</okhttp.version>
@@ -78,7 +78,7 @@
 		<lognet.version>5.1.5</lognet.version>
 		<!-- https://repo1.maven.org/maven2/org/springframework/boot/spring-boot-dependencies  -->
 		<!-- transient dependencies added due to security patching -->
-		<tomcat.version>10.1.18</tomcat.version>
+		<tomcat.version>10.1.19</tomcat.version>
 		<netty.version>4.1.107.Final</netty.version>
 
 		<json-smart.version>2.5.0</json-smart.version>