From 7aff4d1adaa8250e6246f80d6056af1543492cd0 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 | 4 +-- .../conf/HealthServerConfiguration.java | 14 ++++++++- .../conf/HealthServerConfigurationTest.java | 30 +++++++++++++++++-- 4 files changed, 44 insertions(+), 8 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..cb4c8643cb 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 @@ -27,13 +27,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..12c3e5da1f 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,7 @@ import jakarta.annotation.PostConstruct; import java.util.List; +import java.util.Optional; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; @@ -17,11 +18,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 +36,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..e1d5d15a59 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,11 +1,12 @@ package org.cloudfoundry.autoscaler.scheduler.conf; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.*; import java.util.Arrays; import java.util.List; 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.springframework.test.context.junit4.SpringRunner; @@ -51,4 +52,29 @@ 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()); + } }