change unprotected type to Set (instead of List)

cloudfoundry · May 26, 2023 · 4abcaad · 4abcaad
1 parent 6c3ffef
commit 4abcaad
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 31 deletions.
diff --git a/...r/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java b/...r/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java
@@ -1,8 +1,8 @@
 package org.cloudfoundry.autoscaler.scheduler.conf;
 
 import jakarta.annotation.PostConstruct;
-import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import lombok.AllArgsConstructor;
 import lombok.Data;
 import lombok.NoArgsConstructor;

diff --git a/...r/src/main/java/org/cloudfoundry/autoscaler/scheduler/rest/BasicAuthenticationFilter.java b/...r/src/main/java/org/cloudfoundry/autoscaler/scheduler/rest/BasicAuthenticationFilter.java
@@ -11,6 +11,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import lombok.extern.slf4j.Slf4j;
@@ -47,15 +48,16 @@ public void doFilter(
       throws IOException, ServletException {
     HttpServletRequest httpRequest = (HttpServletRequest) servletRequest;
     HttpServletResponse httpResponse = (HttpServletResponse) servletResponse;
-    List<String> unprotectedEndpointsConfig = healthServerConfiguration.getUnprotectedEndpoints();
+    Set<String> unprotectedEndpointsConfig = healthServerConfiguration.getUnprotectedEndpoints();
 
     if (!httpRequest.getRequestURI().contains("/health")) {
       log.debug("Not a health request: " + httpRequest.getRequestURI());
       chain.doFilter(servletRequest, servletResponse);
       return;
     }
 
-    final allEndpointsRequireAuthorization = ObjectUtils.isEmpty(unprotectedEndpointsConfig);
+    final boolean allEndpointsRequireAuthorization =
+        ObjectUtils.isEmpty(unprotectedEndpointsConfig);
     if (allEndpointsRequireAuthorization) {
       isUserAuthenticatedOrSendError(chain, httpRequest, httpResponse);
     } else if (!ObjectUtils.isEmpty(unprotectedEndpointsConfig)) {
@@ -133,7 +135,7 @@ private void isUserAuthenticatedOrSendError(
     }
   }
 
-  private Map<String, Boolean> checkValidEndpoints(List<String> unprotectedEndpointsConfig) {
+  private Map<String, Boolean> checkValidEndpoints(Set<String> unprotectedEndpointsConfig) {
 
     Map<String, Boolean> invalidEndpointsMap = new HashMap<>();
     for (String unprotectedEndpoint : unprotectedEndpointsConfig) {
@@ -144,7 +146,7 @@ private Map<String, Boolean> checkValidEndpoints(List<String> unprotectedEndpoin
     return invalidEndpointsMap;
   }
 
-  private static Map<String, Boolean> getMapFromList(List<String> unprotectedEndpointsConfig) {
+  private static Map<String, Boolean> getMapFromList(Set<String> unprotectedEndpointsConfig) {
     return unprotectedEndpointsConfig.stream()
         .collect(Collectors.toMap(endpoint -> endpoint, endpoint -> true, (a, b) -> b));
   }

diff --git a/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/rest/MultiPortFilter.java b/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/rest/MultiPortFilter.java
@@ -30,12 +30,11 @@ public void doFilter(
     HttpServletRequest httpRequest = (HttpServletRequest) servletRequest;
     HttpServletResponse httpResponse = (HttpServletResponse) servletResponse;
 
-    // FIXME : refactor this duplicate block
     if (isMainRequest(httpRequest)) {
-      log.debug("Main server request received on port " + healthServerConfiguration.getPort());
+      log.debug("Main server request received on port " + httpRequest.getLocalPort());
       filterChain.doFilter(servletRequest, servletResponse);
     } else if (isHealthRequest(httpRequest)) {
-      log.debug("Main server request received on port " + healthServerConfiguration.getPort());
+      log.debug("Health server request received on port " + httpRequest.getLocalPort());
       filterChain.doFilter(servletRequest, servletResponse);
     } else {
       httpResponse.sendError(HttpServletResponse.SC_NOT_FOUND, "Health endpoints do not exist");

diff --git a/...c/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java b/...c/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java
@@ -4,12 +4,12 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
-import java.util.Arrays;
-import java.util.List;
+import java.util.Set;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.junit.runner.RunWith;
+import org.mockito.internal.util.collections.Sets;
 import org.springframework.test.context.junit4.SpringRunner;
 
 @RunWith(SpringRunner.class)
@@ -18,29 +18,28 @@ class HealthServerConfigurationTest {
   void givenUnprotectedEndpointListAndUsernameOrPasswordIsNull() {
     assertThrows(
         IllegalArgumentException.class,
-        () ->
-            new HealthServerConfiguration(null, null, 8081, Arrays.asList("test_endpoint")).init());
+        () -> new HealthServerConfiguration(null, null, 8081, Sets.newSet("test_endpoint")).init());
   }
 
   @Test
   void givenUnprotectedEndpointListAndUsernameOrPasswordIsEmpty() {
     assertThrows(
         IllegalArgumentException.class,
-        () -> new HealthServerConfiguration("", "", 8081, Arrays.asList("test_endpoint")).init());
+        () -> new HealthServerConfiguration("", "", 8081, Sets.newSet("test_endpoint")).init());
   }
 
   @Test
   void givenEmptyUnprotectedEndpointListAndUsernameOrPasswordIsNull() {
     assertThrows(
         IllegalArgumentException.class,
-        () -> new HealthServerConfiguration(null, null, 8081, List.of()).init());
+        () -> new HealthServerConfiguration(null, null, 8081, Set.of()).init());
   }
 
   @Test
   void givenEmptyUnprotectedEndpointListAndUsernameOrPasswordIsEmpty() {
     assertThrows(
         IllegalArgumentException.class,
-        () -> new HealthServerConfiguration("", "", 8081, List.of()).init());
+        () -> new HealthServerConfiguration("", "", 8081, Set.of()).init());
   }
 
   @Test
@@ -52,7 +51,7 @@ void givenUnprotectedEndpointListIsNullThenBasicAuthRequired() {
   @Test
   void givenEmptyUnprotectedEndpointListWithUsernameOrPassword() {
     assertDoesNotThrow(
-        () -> new HealthServerConfiguration("some-user", "some-test", 8081, List.of()).init());
+        () -> new HealthServerConfiguration("some-user", "some-test", 8081, Set.of()).init());
   }
 
   @ParameterizedTest
@@ -61,14 +60,13 @@ public void givenInvalidPortThrowsException(String healthPort) {
 
     assertThrows(
         IllegalArgumentException.class,
-        () ->
-            new HealthServerConfiguration("", "", Integer.parseInt(healthPort), List.of()).init());
+        () -> new HealthServerConfiguration("", "", Integer.parseInt(healthPort), Set.of()).init());
   }
 
   @Test
   void givenValidReturnsPort() {
     HealthServerConfiguration healthServerConfiguration =
-        new HealthServerConfiguration("s", "s", 888, List.of());
+        new HealthServerConfiguration("s", "s", 888, Set.of());
     healthServerConfiguration.init();
     assertEquals(healthServerConfiguration.getPort(), 888);
   }
@@ -77,6 +75,6 @@ void givenValidReturnsPort() {
   void givenEmptyPortThrowsException() {
     assertThrows(
         IllegalArgumentException.class,
-        () -> new HealthServerConfiguration("", "", null, List.of()).init());
+        () -> new HealthServerConfiguration("", "", null, Set.of()).init());
   }
 }
diff --git a/...c/test/java/org/cloudfoundry/autoscaler/scheduler/rest/BasicAuthenticationFilterTest.java b/...c/test/java/org/cloudfoundry/autoscaler/scheduler/rest/BasicAuthenticationFilterTest.java
@@ -6,7 +6,7 @@
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
 import java.io.IOException;
-import java.util.List;
+import java.util.Set;
 import org.apache.commons.codec.binary.Base64;
 import org.cloudfoundry.autoscaler.scheduler.conf.HealthServerConfiguration;
 import org.junit.Before;
@@ -37,7 +37,7 @@ public void setUp() {
   public void allowRequestIfPort8081andURIContainHealthWithoutUnprotectedEndpoints()
       throws IOException, ServletException {
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of());
+        new HealthServerConfiguration(username, password, 8081, Set.of());
 
     req.setRequestURI("some/health/uri");
     String auth = username + ":" + password;
@@ -59,7 +59,7 @@ public void denyHealthRequestWithoutUnprotectedEndpointsAndInvalidCredentials()
     String auth = username + ":" + password;
     req.addHeader("Authorization", "Basic " + Base64.encodeBase64String(auth.getBytes()));
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(null, null, 8081, List.of());
+        new HealthServerConfiguration(null, null, 8081, Set.of());
     BasicAuthenticationFilter userPwNullFilter = new BasicAuthenticationFilter(healthServerConfig);
     userPwNullFilter.doFilter(req, res, filterChainMock);
 
@@ -68,7 +68,7 @@ public void denyHealthRequestWithoutUnprotectedEndpointsAndInvalidCredentials()
     assertEquals(res.getHeader("WWW-Authenticate"), "Basic");
 
     res = new MockHttpServletResponse();
-    healthServerConfig = new HealthServerConfiguration(username, password, 8081, List.of());
+    healthServerConfig = new HealthServerConfiguration(username, password, 8081, Set.of());
     BasicAuthenticationFilter wrongCredsFilter = new BasicAuthenticationFilter(healthServerConfig);
     req.removeHeader("Authorization");
     String wrongCreds = "wrong-user:pw";
@@ -80,7 +80,7 @@ public void denyHealthRequestWithoutUnprotectedEndpointsAndInvalidCredentials()
     assertEquals(res.getHeader("WWW-Authenticate"), "Basic");
 
     res = new MockHttpServletResponse();
-    healthServerConfig = new HealthServerConfiguration(username, password, 8081, List.of());
+    healthServerConfig = new HealthServerConfiguration(username, password, 8081, Set.of());
     BasicAuthenticationFilter noAuthHeaderFilter =
         new BasicAuthenticationFilter(healthServerConfig);
     req.removeHeader("Authorization");
@@ -97,7 +97,7 @@ public void denyHealthRequestIfAuthHeaderIsInvalid() throws IOException, Servlet
     req.setRequestURI("some/health/uri");
 
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of());
+        new HealthServerConfiguration(username, password, 8081, Set.of());
     BasicAuthenticationFilter malformedHeaderFilter =
         new BasicAuthenticationFilter(healthServerConfig);
     req.removeHeader("Authorization");
@@ -114,7 +114,7 @@ public void allowRequestIfPort8081andURIContainHealthWithUnprotectedEndpoints()
       throws IOException, ServletException {
 
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of("/health/liveness"));
+        new HealthServerConfiguration(username, password, 8081, Set.of("/health/liveness"));
     req.setRequestURI("/health/liveness");
     String auth = username + ":" + password;
     req.addHeader("Authorization", "Basic " + Base64.encodeBase64String(auth.getBytes()));
@@ -134,7 +134,7 @@ public void denyHealthRequestWithWrongUnprotectedEndpoints()
     req.addHeader("Authorization", "Basic " + Base64.encodeBase64String(auth.getBytes()));
 
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of("/health/wrong-endpoint"));
+        new HealthServerConfiguration(username, password, 8081, Set.of("/health/wrong-endpoint"));
     BasicAuthenticationFilter filter = new BasicAuthenticationFilter(healthServerConfig);
     filter.doFilter(req, res, filterChainMock);
 
@@ -155,7 +155,7 @@ public void denyHealthRequestsWithNoUnprotectedEndpointsConfigThenBasicAuthRequi
     req.setRequestURI(requestURI);
 
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of());
+        new HealthServerConfiguration(username, password, 8081, Set.of());
     BasicAuthenticationFilter basicAuthenticationFilter =
         new BasicAuthenticationFilter(healthServerConfig);
 
@@ -170,7 +170,7 @@ public void denyHealthRequestsWithNoUnprotectedEndpointsConfigThenBasicAuthRequi
   public void denyHealthRequestIfBasicAuthRequired() throws IOException, ServletException {
 
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of("/health/prometheus"));
+        new HealthServerConfiguration(username, password, 8081, Set.of("/health/prometheus"));
     req.setRequestURI("/health/liveness");
 
     BasicAuthenticationFilter noBasicAuthFilter = new BasicAuthenticationFilter(healthServerConfig);
@@ -187,7 +187,7 @@ public void basicAuthFilterNotAppliedIfNoHealthRequest() throws IOException, Ser
     req.setRequestURI("/routeTo8080");
 
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(username, password, 8081, List.of("/health/wrong-endpoint"));
+        new HealthServerConfiguration(username, password, 8081, Set.of("/health/wrong-endpoint"));
     BasicAuthenticationFilter filter = new BasicAuthenticationFilter(healthServerConfig);
     filter.doFilter(req, res, filterChainMock);