diff --git a/spring-boot-admin-server/pom.xml b/spring-boot-admin-server/pom.xml index 806c79c8e29..98083582d96 100644 --- a/spring-boot-admin-server/pom.xml +++ b/spring-boot-admin-server/pom.xml @@ -156,5 +156,9 @@ testcontainers-junit-jupiter test + + org.springframework.boot + spring-boot-starter-validation + diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/values/Registration.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/values/Registration.java index 3f450346bb9..4b1d31ca618 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/values/Registration.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/domain/values/Registration.java @@ -23,11 +23,12 @@ import java.util.LinkedHashMap; import java.util.Map; +import jakarta.validation.constraints.AssertTrue; +import jakarta.validation.constraints.NotBlank; import lombok.ToString; import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** @@ -41,6 +42,7 @@ public final class Registration implements Serializable { private static final Logger log = LoggerFactory.getLogger(Registration.class); + @NotBlank private final String name; /** @@ -55,6 +57,7 @@ public final class Registration implements Serializable { * Admin to determine the instance status. Example: * https://example.com/actuator/health */ + @NotBlank private final String healthUrl; /** @@ -71,13 +74,6 @@ public final class Registration implements Serializable { @lombok.Builder(builderClassName = "Builder", toBuilder = true) private Registration(String name, @Nullable String managementUrl, String healthUrl, @Nullable String serviceUrl, String source, @lombok.Singular("metadata") Map metadata) { - Assert.hasText(name, "'name' must not be empty."); - Assert.hasText(healthUrl, "'healthUrl' must not be empty."); - Assert.isTrue(checkUrl(healthUrl), "'healthUrl' is not valid: " + healthUrl); - Assert.isTrue(!StringUtils.hasText(managementUrl) || checkUrl(managementUrl), - "'managementUrl' is not valid: " + managementUrl); - Assert.isTrue(!StringUtils.hasText(serviceUrl) || checkUrl(serviceUrl), - "'serviceUrl' is not valid: " + serviceUrl); this.name = name; this.managementUrl = managementUrl; @@ -128,6 +124,21 @@ public Map getMetadata() { return Collections.unmodifiableMap(this.metadata); } + @AssertTrue(message = "'healthUrl' must be an absolute URL.") + public boolean isHealthUrlAbsolute() { + return StringUtils.hasText(this.healthUrl) && checkUrl(this.healthUrl); + } + + @AssertTrue(message = "'managementUrl' must be an absolute URL when present.") + public boolean isManagementUrlAbsoluteWhenPresent() { + return !StringUtils.hasText(this.managementUrl) || checkUrl(this.managementUrl); + } + + @AssertTrue(message = "'serviceUrl' must be an absolute URL when present.") + public boolean isServiceUrlAbsoluteWhenPresent() { + return !StringUtils.hasText(this.serviceUrl) || checkUrl(this.serviceUrl); + } + /** * Checks the syntax of the given URL. * @param url the URL. diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/web/InstancesController.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/web/InstancesController.java index 109efc0d7a2..2553b146859 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/web/InstancesController.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/web/InstancesController.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.Map; +import jakarta.validation.Valid; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.MediaType; @@ -74,7 +75,7 @@ public InstancesController(InstanceRegistry registry, InstanceEventStore eventSt * @return the registered instance id; */ @PostMapping(path = "/instances", consumes = MediaType.APPLICATION_JSON_VALUE) - public Mono>> register(@RequestBody Registration registration, + public Mono>> register(@Valid @RequestBody Registration registration, UriComponentsBuilder builder) { Registration withSource = Registration.copyOf(registration).source("http-api").build(); LOGGER.debug("Register instance {}", withSource); diff --git a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/web/InstancesControllerIntegrationTest.java b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/web/InstancesControllerIntegrationTest.java index 664c03d3b90..b67ba4350b4 100644 --- a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/web/InstancesControllerIntegrationTest.java +++ b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/web/InstancesControllerIntegrationTest.java @@ -17,6 +17,7 @@ package de.codecentric.boot.admin.server.web; import java.time.Duration; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; @@ -33,6 +34,7 @@ import org.springframework.context.ConfigurableApplicationContext; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.MediaType; +import org.springframework.test.web.reactive.server.EntityExchangeResult; import org.springframework.test.web.reactive.server.WebTestClient; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -95,6 +97,105 @@ void should_return_not_found_when_get_unknown_instance() { this.client.get().uri("/instances/unknown").exchange().expectStatus().isNotFound(); } + @Test + void should_reject_invalid_json() { + this.client.post() + .uri("/instances") + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue("{") + .exchange() + .expectStatus() + .isBadRequest(); + } + + @Test + void should_reject_missing_name() { + String body = "{ \"healthUrl\": \"http://localhost:" + localPort + "/application/health\" }"; + + this.client.post() + .uri("/instances") + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue(body) + .exchange() + .expectStatus() + .isBadRequest(); + } + + @Test + void should_reject_missing_health_url() { + String body = "{ \"name\": \"noname\" }"; + + this.client.post() + .uri("/instances") + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue(body) + .exchange() + .expectStatus() + .isBadRequest(); + } + + @Test + void should_create_distinct_ids_for_different_health_urls() { + String id1 = register().block(); + + String other = "{ \"name\": \"other\", \"healthUrl\": \"http://localhost:" + localPort + "/other/health\" }"; + String id2 = registerWithBody(other); + + assertThat(id2).isNotEqualTo(id1); + + assertInstanceById(id1); + assertInstanceById(id2); + } + + @Test + void should_remove_instance_after_delete() { + String id = register().block(); + + this.client.delete().uri(getLocation(id)).exchange().expectStatus().isNoContent(); + + this.client.get().uri(getLocation(id)).exchange().expectStatus().isNotFound(); + + this.client.get() + .uri("/instances") + .exchange() + .expectStatus() + .isOk() + .expectHeader() + .contentType(MediaType.APPLICATION_JSON) + .expectBody(String.class) + .consumeWith((response) -> { + DocumentContext json = JsonPath.parse(response.getResponseBody()); + List ids = json.read("$[?(@.id == '" + id + "')].id"); + assertThat(ids).isEmpty(); + }); + + } + + @Test + void should_not_create_duplicate_instance_on_reregister() { + String id = register().block(Duration.ofSeconds(5)); + + registerSecondTime(id); + + this.client.get() + .uri("/instances") + .exchange() + .expectStatus() + .isOk() + .expectHeader() + .contentType(MediaType.APPLICATION_JSON) + .expectBody(String.class) + .consumeWith((response) -> { + DocumentContext json = JsonPath.parse(response.getResponseBody()); + List ids = json.read("$[?(@.id == '" + id + "')].id"); + assertThat(ids).hasSize(1); + }); + + } + @Test void should_return_empty_list() { this.client.get() @@ -280,6 +381,23 @@ private Mono register() { //@formatter:on } + private String registerWithBody(String body) { + //@formatter:off + EntityExchangeResult> result = client.post() + .uri("/instances") + .accept(MediaType.APPLICATION_JSON).contentType(MediaType.APPLICATION_JSON) + .bodyValue(body) + .exchange() + .expectStatus().isCreated() + .expectHeader().contentType(MediaType.APPLICATION_JSON) + .expectHeader().valueMatches("location", "http://localhost:" + localPort + "/instances/[0-9a-f]+") + .expectBody(responseType) + .returnResult(); + //@formatter:on + assertThat(result.getResponseBody()).containsKeys("id"); + return result.getResponseBody().get("id").toString(); + } + private String getLocation(String id) { return "http://localhost:" + localPort + "/instances/" + id;