From fa2f9f5263298ba3215ddb34875c81968bc19d43 Mon Sep 17 00:00:00 2001 From: Attila Kanto Date: Mon, 9 Dec 2024 09:09:21 +0100 Subject: [PATCH] CB-28055 It looks like that dependabot has done a partial update of tomcat, and after fixing it in a former commit, it turned out that the org.apache.tomcat.embed:tomcat-embed-el is a transitive dependency of spring boot, so we shall update it as well. It turned out that Tomcat has also introduced a bug and for a few version it did not send back the content length for HTTP HEAD, therefore Tomcat version had to be upgraded, othervise the image catalog content validation would just fail: https://github.com/spring-projects/spring-boot/issues/43272 --- .../validation/HttpContentSizeValidator.java | 21 +++++++++++++++---- .../HttpContentSizeValidatorTest.java | 9 +++++--- dependencies.gradle | 4 ++-- mock-infrastructure/build.gradle | 1 + .../MockImageCatalogController.java | 11 +++++++++- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/core-api/src/main/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidator.java b/core-api/src/main/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidator.java index b3cd6062af4..91f18223c69 100644 --- a/core-api/src/main/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidator.java +++ b/core-api/src/main/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidator.java @@ -50,10 +50,18 @@ public boolean isValid(String value, ConstraintValidatorContext context) { return false; } int maxSizeInBytes = contentSizeProvider.getMaxSizeInBytes(); - boolean valid = contentLength.getValue() > 0 && contentLength.getValue() <= maxSizeInBytes; - if (!valid) { - String message = String.format("The content of the given URL must be less than {%s}", FileUtils.byteCountToDisplaySize(maxSizeInBytes)); - ValidatorUtil.addConstraintViolation(context, message); + boolean valid = true; + + if (contentLength.getValue() == null || contentLength.getValue() < 0) { + logAndAddViolation(context, "The content length of the image catalog URL is not returned by the server!"); + valid = false; + } else if (contentLength.getValue() == 0) { + logAndAddViolation(context, "The content length of the image catalog URL is 0!"); + valid = false; + } else if (contentLength.getValue() > maxSizeInBytes) { + String message = String.format("The content of the image catalog URL must be less than {%s}", FileUtils.byteCountToDisplaySize(maxSizeInBytes)); + logAndAddViolation(context, message); + valid = false; } return valid; @@ -63,4 +71,9 @@ public boolean isValid(String value, ConstraintValidatorContext context) { } return false; } + + private void logAndAddViolation(ConstraintValidatorContext context, String message) { + LOGGER.info(message); + ValidatorUtil.addConstraintViolation(context, message); + } } diff --git a/core-api/src/test/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidatorTest.java b/core-api/src/test/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidatorTest.java index b9a0fa6b82a..d808ff7d9a6 100644 --- a/core-api/src/test/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidatorTest.java +++ b/core-api/src/test/java/com/sequenceiq/cloudbreak/validation/HttpContentSizeValidatorTest.java @@ -107,7 +107,8 @@ void testUrlFailsWithMinusOne() { assertFalse(underTest.isValid("http://unknown.error.com", constraintValidatorContext)); - verify(constraintValidatorContext, times(1)).buildConstraintViolationWithTemplate(anyString()); + verify(constraintValidatorContext, times(1)). + buildConstraintViolationWithTemplate(eq("The content length of the image catalog URL is not returned by the server!")); verify(constraintViolationBuilder, times(1)).addConstraintViolation(); } @@ -122,7 +123,8 @@ void testUrlFailsWithZero() { assertFalse(underTest.isValid("http://empty.content.com", constraintValidatorContext)); - verify(constraintValidatorContext, times(1)).buildConstraintViolationWithTemplate(anyString()); + verify(constraintValidatorContext, times(1)) + .buildConstraintViolationWithTemplate(eq("The content length of the image catalog URL is 0!")); verify(constraintViolationBuilder, times(1)).addConstraintViolation(); } @@ -137,7 +139,8 @@ void testUrlFailsWithMoreThanMax() { assertFalse(underTest.isValid("http://big.content.com", constraintValidatorContext)); - verify(constraintValidatorContext, times(1)).buildConstraintViolationWithTemplate(anyString()); + verify(constraintValidatorContext, times(1)) + .buildConstraintViolationWithTemplate(eq("The content of the image catalog URL must be less than {6 MB}")); verify(constraintViolationBuilder, times(1)).addConstraintViolation(); } diff --git a/dependencies.gradle b/dependencies.gradle index ed9c15f5b7b..e50cef4408a 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -9,7 +9,7 @@ ext { // Spring caffeineVersion = '3.1.8' quartzVersion = '2.5.0' - springBootVersion = '3.3.4' + springBootVersion = '3.3.6' springDataJpaFrameworkVersion = '3.2.3' springFrameworkVersion = '6.1.14' springRetryVersion = '2.0.9' @@ -37,7 +37,7 @@ ext { nettyVersion = '4.1.115.Final' reactorNettyHttp = '1.2.0' okhttpVersion = '4.12.0' - tomcatVersion = '10.1.33' + tomcatVersion = '10.1.34' // Cloud SDKs awsSdkVersion = '2.28.3' diff --git a/mock-infrastructure/build.gradle b/mock-infrastructure/build.gradle index 3707b64b5e7..1c32856107c 100644 --- a/mock-infrastructure/build.gradle +++ b/mock-infrastructure/build.gradle @@ -80,6 +80,7 @@ dependencies { testImplementation group: 'org.mockito', name: 'mockito-core', version: mockitoVersion testImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: springBootVersion testImplementation group: 'org.assertj', name: 'assertj-core', version: assertjVersion + implementation group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: tomcatVersion implementation project (':cloud-api') implementation project (':common') diff --git a/mock-infrastructure/src/main/java/com/sequenceiq/mock/controller/MockImageCatalogController.java b/mock-infrastructure/src/main/java/com/sequenceiq/mock/controller/MockImageCatalogController.java index 7c7618e7ab3..0b449162a71 100644 --- a/mock-infrastructure/src/main/java/com/sequenceiq/mock/controller/MockImageCatalogController.java +++ b/mock-infrastructure/src/main/java/com/sequenceiq/mock/controller/MockImageCatalogController.java @@ -2,6 +2,8 @@ import jakarta.inject.Inject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseBody; @@ -12,6 +14,8 @@ @RestController public class MockImageCatalogController { + private static final Logger LOGGER = LoggerFactory.getLogger(MockImageCatalogController.class); + @Inject private ImageCatalogMockService imageCatalogMockService; @@ -24,6 +28,11 @@ public String auth(@RequestParam("catalog-name") String name, @RequestParam(name = "default-image-uuid", required = false) String defaultImageUuid, @RequestParam(name = "non-default-image-uuid", required = false) String nonDefaultImageUuid, @RequestParam("mock-server-address") String mockServerAddress) { - return imageCatalogMockService.getImageCatalogByName(name, cbVersion, runtime, cm, defaultImageUuid, nonDefaultImageUuid, mockServerAddress); + LOGGER.info("Request to generate image catalog with name: {}, cb version: {}, runtime: {}, cm: {}, default image uuid: {}, " + + "non default image uuid: {}, mock server address: {}", name, cbVersion, runtime, cm, defaultImageUuid, nonDefaultImageUuid, mockServerAddress); + String generatedImageCatalog = imageCatalogMockService.getImageCatalogByName(name, cbVersion, runtime, cm, defaultImageUuid, + nonDefaultImageUuid, mockServerAddress); + LOGGER.info("Generated image catalog: {}", generatedImageCatalog); + return generatedImageCatalog; } }