From 12aec5fa3d837c8fd208fd6142b4ac97309f41f8 Mon Sep 17 00:00:00 2001 From: Arsalan Khan Date: Tue, 23 May 2023 17:38:37 +0200 Subject: [PATCH] Review -> Scheduler -> validate health port start health server on defined ports (Port 0 is not allowed) --- src/autoscaler/integration/components_test.go | 4 +- .../conf/EmbeddedTomcatConfiguration.java | 5 +-- .../conf/HealthServerConfiguration.java | 15 ++++++- .../conf/HealthServerConfigurationTest.java | 40 +++++++++++++++++-- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/autoscaler/integration/components_test.go b/src/autoscaler/integration/components_test.go index 03169c6900..29fd282560 100644 --- a/src/autoscaler/integration/components_test.go +++ b/src/autoscaler/integration/components_test.go @@ -12,6 +12,7 @@ import ( opConfig "code.cloudfoundry.org/app-autoscaler/src/autoscaler/operator/config" "code.cloudfoundry.org/app-autoscaler/src/autoscaler/routes" seConfig "code.cloudfoundry.org/app-autoscaler/src/autoscaler/scalingengine/config" + "github.com/go-sql-driver/mysql" "fmt" "net/url" @@ -21,7 +22,6 @@ import ( "strings" "time" - "github.com/go-sql-driver/mysql" . "github.com/onsi/gomega" "github.com/tedsuo/ifrit/ginkgomon_v2" "gopkg.in/yaml.v3" @@ -355,7 +355,7 @@ server.ssl.ciphers=TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_CBC_SHA2 server.port=%d # Health Server -scheduler.healthserver.port=0 +scheduler.healthserver.port=7000 scheduler.healthserver.username=test-user scheduler.healthserver.password=test-password client.httpClientTimeout=%d diff --git a/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/EmbeddedTomcatConfiguration.java b/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/EmbeddedTomcatConfiguration.java index 21c3fea605..1b5cb3f418 100644 --- a/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/EmbeddedTomcatConfiguration.java +++ b/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/EmbeddedTomcatConfiguration.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; import org.apache.catalina.connector.Connector; import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory; import org.springframework.context.annotation.Bean; @@ -27,13 +28,11 @@ public TomcatServletWebServerFactory servletContainer() { } private Connector[] additionalConnector() { - if (healthServerConfig.getPort() == 0) { - return new Connector[0]; - } List result = new ArrayList<>(); Connector connector = new Connector("org.apache.coyote.http11.Http11NioProtocol"); connector.setScheme("http"); connector.setPort(healthServerConfig.getPort()); + result.add(connector); return result.toArray(new Connector[] {}); } diff --git a/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java b/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java index a366351b21..883693363d 100644 --- a/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java +++ b/src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java @@ -2,6 +2,8 @@ import jakarta.annotation.PostConstruct; import java.util.List; +import java.util.Optional; + import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; @@ -17,11 +19,14 @@ public class HealthServerConfiguration { private String username; private String password; - private int port; + private Integer port; private List unprotectedEndpoints; @PostConstruct public void init() { + + validatePort(); + boolean basicAuthEnabled = (unprotectedEndpoints != null || ObjectUtils.isEmpty(unprotectedEndpoints)); if (basicAuthEnabled @@ -32,4 +37,12 @@ public void init() { throw new IllegalArgumentException("Heath Server Basic Auth Username or password is not set"); } } + + private void validatePort() { + Optional healthPortOptional = Optional.ofNullable(this.port); + if (!healthPortOptional.isPresent() || healthPortOptional.get() == 0) { + throw new IllegalArgumentException( + "Health Configuration: health server port not defined or port=0"); + } + } } diff --git a/src/scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java b/src/scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java index 32791f8fb1..a1660e1997 100644 --- a/src/scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java +++ b/src/scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java @@ -1,13 +1,23 @@ package org.cloudfoundry.autoscaler.scheduler.conf; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertThrows; - +import java.io.IOException; import java.util.Arrays; import java.util.List; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import org.cloudfoundry.autoscaler.scheduler.rest.BasicAuthenticationFilter; 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.Mockito; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.client.HttpClientErrorException; + +import static org.junit.jupiter.api.Assertions.*; @RunWith(SpringRunner.class) class HealthServerConfigurationTest { @@ -51,4 +61,28 @@ void givenEmptyUnprotectedEndpointListWithUsernameOrPassword() { assertDoesNotThrow( () -> new HealthServerConfiguration("some-user", "some-test", 8081, List.of()).init()); } + + @ParameterizedTest + @ValueSource(strings = {"null", "0", ""}) + public void givenInvalidPortThrowsException(String healthPort) { + + assertThrows( + IllegalArgumentException.class, + () -> new HealthServerConfiguration("", "", Integer.parseInt(healthPort), List.of()).init()); + } + + @Test + void givenValidReturnsPort() { + HealthServerConfiguration healthServerConfiguration = + new HealthServerConfiguration("s", "s", 888, List.of()); + healthServerConfiguration.init(); + assertEquals(healthServerConfiguration.getPort(), 888); + } + + @Test + void givenEmptyPortThrowsException() { + assertThrows( + IllegalArgumentException.class, + () -> new HealthServerConfiguration("", "", null, List.of()).init()); + } }