From 25dfaf450efb093149a9e695555011b744c941fb Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Fri, 10 Jan 2025 11:18:26 +0100 Subject: [PATCH 1/4] fix: ORS_CONFIG_LOCATION doesn't work with *.yaml files --- .../ors/api/ORSEnvironmentPostProcessor.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/ORSEnvironmentPostProcessor.java b/ors-api/src/main/java/org/heigit/ors/api/ORSEnvironmentPostProcessor.java index af22815b00..2a2e4b4327 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/ORSEnvironmentPostProcessor.java +++ b/ors-api/src/main/java/org/heigit/ors/api/ORSEnvironmentPostProcessor.java @@ -12,6 +12,7 @@ import org.springframework.core.io.FileSystemResource; import java.io.IOException; +import java.io.File; import java.util.ArrayList; import java.util.List; import java.util.Map.Entry; @@ -42,8 +43,18 @@ public void postProcessEnvironment(ConfigurableEnvironment environment, SpringAp log.info("Configuration file set by program argument."); } if (configLocations.isEmpty() && !StringUtility.isNullOrEmpty(System.getenv(ORS_CONFIG_LOCATION_ENV))) { - configLocations.add(System.getenv(ORS_CONFIG_LOCATION_ENV)); - log.info("Configuration file set by environment variable."); + String configPath = System.getenv(ORS_CONFIG_LOCATION_ENV); + File configFile = new File(configPath); + // Check if the file exists and log the appropriate message + if (configFile.exists()) { + log.info("ORS config file found at: " + configFile.getAbsolutePath()); + configLocations.add(configPath); + log.info("Configuration file set by environment variable."); + } else { + // Add it anyway to provoke error and stop ORS + configLocations.add(configPath); + log.error("ORS config file not found at: " + configPath); + } } if (configLocations.isEmpty()) { String home = System.getProperty("user.home"); From 6d27b4bb79f6be32e0b9d1a00dcb1bb06a12e363 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Mon, 27 Jan 2025 13:11:26 +0100 Subject: [PATCH 2/4] feat: add test WAR_CONTAINER still fails here --- .../ConfigEnvironmentTest.java | 52 +++++++++++++++---- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java b/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java index 62d24cef53..1727de6539 100644 --- a/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java +++ b/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java @@ -1,5 +1,6 @@ package integrationtests; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; @@ -7,6 +8,7 @@ import org.junit.jupiter.api.parallel.ExecutionMode; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.testcontainers.containers.ContainerLaunchException; import org.testcontainers.containers.GenericContainer; import org.testcontainers.junit.jupiter.Testcontainers; import org.testcontainers.junit.jupiter.TestcontainersExtension; @@ -45,7 +47,7 @@ class ConfigEnvironmentTests { @MethodSource("utils.ContainerInitializer#ContainerTestImageBareImageStream") @ParameterizedTest(name = "{0}") @Execution(ExecutionMode.CONCURRENT) - void testMissingConfigButRequiredParamsAsEnvUpperAndLower(ContainerInitializer.ContainerTestImageBare targetImage) { + void testMissingConfigButRequiredParamsAsEnvUpperAndLower(ContainerTestImageBare targetImage) { GenericContainer container = initContainer(targetImage, false, "testMissingConfigButRequiredParamsAsEnvUpperAndLower"); container.waitingFor(healthyWaitStrategyWithLogMessage(List.of( "Configuration file lookup by default locations.", @@ -73,8 +75,8 @@ void testMissingConfigButRequiredParamsAsEnvUpperAndLower(ContainerInitializer.C @MethodSource("utils.ContainerInitializer#ContainerTestImageBareImageStream") @ParameterizedTest(name = "{0}") @Execution(ExecutionMode.CONCURRENT) - void testMissingConfigButRequiredParamsAsArg(ContainerInitializer.ContainerTestImageBare targetImage) { - if (targetImage.equals(ContainerInitializer.ContainerTestImageBare.WAR_CONTAINER_BARE)) { + void testMissingConfigButRequiredParamsAsArg(ContainerTestImageBare targetImage) { + if (targetImage.equals(ContainerTestImageBare.WAR_CONTAINER_BARE)) { // This test is not applicable to the WAR container as it does not support command line arguments. return; } @@ -86,10 +88,10 @@ void testMissingConfigButRequiredParamsAsArg(ContainerInitializer.ContainerTestI "Configuration lookup finished." ).toArray(new String[0]), Duration.ofSeconds(60))); ArrayList command = targetImage.getCommand("250M"); - if (targetImage.equals(ContainerInitializer.ContainerTestImageBare.JAR_CONTAINER_BARE)) { + if (targetImage.equals(ContainerTestImageBare.JAR_CONTAINER_BARE)) { command.add("--ors.engine.profiles.driving-hgv.enabled=true"); command.add("--ors.engine.profile_default.build.source_file=/home/ors/openrouteservice/files/heidelberg.test.pbf"); - } else if (targetImage.equals(ContainerInitializer.ContainerTestImageBare.MAVEN_CONTAINER_BARE)) { + } else if (targetImage.equals(ContainerTestImageBare.MAVEN_CONTAINER_BARE)) { command.add("-Dspring-boot.run.arguments=--ors.engine.profile_default.build.source_file=/home/ors/openrouteservice/files/heidelberg.test.pbf --ors.engine.profiles.driving-hgv.enabled=true"); } container.setCommand(command.toArray(new String[0])); @@ -105,7 +107,7 @@ void testMissingConfigButRequiredParamsAsArg(ContainerInitializer.ContainerTestI @MethodSource("utils.ContainerInitializer#ContainerTestImageDefaultsImageStream") @ParameterizedTest(name = "{0}") @Execution(ExecutionMode.CONCURRENT) - void testPropertyOverridesDefaultConfig(ContainerInitializer.ContainerTestImageDefaults targetImage) { + void testPropertyOverridesDefaultConfig(ContainerTestImageDefaults targetImage) { GenericContainer container = initContainer(targetImage, false, "testPropertyOverridesDefaultConfig"); container.addEnv("ors.engine.profiles.driving-car.enabled", "false"); @@ -126,7 +128,7 @@ void testPropertyOverridesDefaultConfig(ContainerInitializer.ContainerTestImageD @MethodSource("utils.ContainerInitializer#ContainerTestImageBareImageStream") @ParameterizedTest(name = "{0}") @Execution(ExecutionMode.CONCURRENT) - void testActivateAllBareGraphsWithEnv(ContainerInitializer.ContainerTestImageBare targetImage) { + void testActivateAllBareGraphsWithEnv(ContainerTestImageBare targetImage) { GenericContainer container = initContainer(targetImage, false, "testActivateAllBareGraphsWithEnv"); container.addEnv("ors.engine.profile_default.enabled", "true"); container.addEnv("ors.engine.profiles.public-transport.enabled", "false"); @@ -158,9 +160,9 @@ void testActivateAllBareGraphsWithEnv(ContainerInitializer.ContainerTestImageBar @MethodSource("utils.ContainerInitializer#ContainerTestImageDefaultsImageStream") @ParameterizedTest(name = "{0}") @Execution(ExecutionMode.CONCURRENT) - void testDefaultProfileActivated(ContainerInitializer.ContainerTestImageDefaults targetImage) { + void testDefaultProfileActivated(ContainerTestImageDefaults targetImage) { GenericContainer container = initContainer(targetImage, false, "testDefaultProfileActivated"); - if (targetImage.equals(ContainerInitializer.ContainerTestImageDefaults.WAR_CONTAINER)) { + if (targetImage.equals(ContainerTestImageDefaults.WAR_CONTAINER)) { container.setWaitStrategy(orsCorrectConfigLoadedWaitStrategy("/home/ors/openrouteservice/ors-config.yml")); } else { container.waitingFor(orsCorrectConfigLoadedWaitStrategy("./ors-config.yml")); @@ -178,7 +180,7 @@ void testDefaultProfileActivated(ContainerInitializer.ContainerTestImageDefaults @MethodSource("utils.ContainerInitializer#ContainerTestImageDefaultsImageStream") @ParameterizedTest(name = "{0}") @Execution(ExecutionMode.CONCURRENT) - void testActivateEachProfileWithEnvAndOverwriteDefaultConfig(ContainerInitializer.ContainerTestImageDefaults targetImage) { + void testActivateEachProfileWithEnvAndOverwriteDefaultConfig(ContainerTestImageDefaults targetImage) { GenericContainer container = initContainer(targetImage, false, "testActivateEachProfileWithEnvAndOverwriteDefaultConfig"); // Prepare the environment @@ -192,5 +194,35 @@ void testActivateEachProfileWithEnvAndOverwriteDefaultConfig(ContainerInitialize OrsApiHelper.assertProfilesLoaded(container, allProfiles.stream().collect(HashMap::new, (m, v) -> m.put(v, true), HashMap::putAll)); container.stop(); } + + /** + * arg-overrides-default-prop.sh + */ + @MethodSource("utils.ContainerInitializer#ContainerTestImageDefaultsImageStream") + @ParameterizedTest(name = "{0}") + @Execution(ExecutionMode.CONCURRENT) + void testNonExistentConfigFail(ContainerTestImageDefaults targetImage) { + GenericContainer container = initContainer(targetImage, false, "testNonExistentConfigFail"); + + // Prepare the environment + container.addEnv("ORS_CONFIG_LOCATION", "nonexistent/ors-config.yaml"); + container.addEnv("ors.config.location", ""); + + try { + container.start(); + + String logs = container.getLogs(); + Assertions.assertTrue(logs.contains("ORS config file not found at: nonexistent/ors-config.yaml")); + + } catch (ContainerLaunchException e) { + + } catch (Exception e) { + Assertions.fail("Container startup failed with exception: " + e.getMessage()); + } finally { + if (container.isRunning()) { + container.stop(); + } + } + } } } \ No newline at end of file From c3988887bb421ce649ab05ade9e04fe44d57abf0 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Mon, 27 Jan 2025 13:13:27 +0100 Subject: [PATCH 3/4] docs: add to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e93df4d82..5703dc226d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ RELEASING: - Add missing 'build' in documentation for profile properties ([#1947](https://github.com/GIScience/openrouteservice/issues//1947)) - Allow vehicles on ferries without explicit access tags ([#1954](https://github.com/GIScience/openrouteservice/pull/1954)) +- ORS_CONFIG_LOCATION doesn't log with wrong paths ([#1816](https://github.com/GIScience/openrouteservice/issues/1816)) ### Security From bf4ac2eb52c8fb19390fdd6153356e348f2b9884 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 28 Jan 2025 09:55:24 +0100 Subject: [PATCH 4/4] docs: add comment outlining empty catch statement --- .../src/test/java/integrationtests/ConfigEnvironmentTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java b/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java index 1727de6539..aae8088333 100644 --- a/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java +++ b/ors-test-scenarios/src/test/java/integrationtests/ConfigEnvironmentTest.java @@ -215,7 +215,7 @@ void testNonExistentConfigFail(ContainerTestImageDefaults targetImage) { Assertions.assertTrue(logs.contains("ORS config file not found at: nonexistent/ors-config.yaml")); } catch (ContainerLaunchException e) { - + // Pass when throwing this exception since the container is supposed to fail when the config path is wrong. } catch (Exception e) { Assertions.fail("Container startup failed with exception: " + e.getMessage()); } finally {