diff --git a/workers/analyzer/src/main/kotlin/analyzer/AnalyzerWorker.kt b/workers/analyzer/src/main/kotlin/analyzer/AnalyzerWorker.kt index 402e758a3..5e7718584 100644 --- a/workers/analyzer/src/main/kotlin/analyzer/AnalyzerWorker.kt +++ b/workers/analyzer/src/main/kotlin/analyzer/AnalyzerWorker.kt @@ -80,8 +80,12 @@ internal class AnalyzerWorker( ortRun.path.orEmpty() ) - val resolvedEnvConfig = envConfigFromJob?.let { environmentService.setUpEnvironment(context, it) } - ?: environmentService.setUpEnvironment(context, sourcesDir, repositoryService) + val resolvedEnvConfig = environmentService.setUpEnvironment( + context, + sourcesDir, + envConfigFromJob, + repositoryService + ) val ortResult = runner.run(context, sourcesDir, job.configuration, resolvedEnvConfig) ortRunService.storeRepositoryInformation(ortRun.id, ortResult.repository) diff --git a/workers/analyzer/src/test/kotlin/AnalyzerEndpointTest.kt b/workers/analyzer/src/test/kotlin/AnalyzerEndpointTest.kt index e5bab316f..87d1665c6 100644 --- a/workers/analyzer/src/test/kotlin/AnalyzerEndpointTest.kt +++ b/workers/analyzer/src/test/kotlin/AnalyzerEndpointTest.kt @@ -365,7 +365,7 @@ class AnalyzerEndpointTest : KoinTest, StringSpec() { val environmentService by inject() withSystemProperties(properties, mode = OverrideMode.SetOrOverride) { - environmentService.setUpEnvironment(context, repositoryFolder, null) + environmentService.setUpEnvironment(context, repositoryFolder, null, null) } block(homeFolder) diff --git a/workers/analyzer/src/test/kotlin/AnalyzerWorkerTest.kt b/workers/analyzer/src/test/kotlin/AnalyzerWorkerTest.kt index 1c1d7b488..44473ecda 100644 --- a/workers/analyzer/src/test/kotlin/AnalyzerWorkerTest.kt +++ b/workers/analyzer/src/test/kotlin/AnalyzerWorkerTest.kt @@ -145,7 +145,7 @@ class AnalyzerWorkerTest : StringSpec({ every { findInfrastructureServiceForRepository(context) } returns infrastructureService coEvery { generateNetRcFile(context, listOf(infrastructureService)) } just runs coEvery { - setUpEnvironment(context, projectDir, infrastructureService) + setUpEnvironment(context, projectDir, null, infrastructureService) } returns ResolvedEnvironmentConfig() } @@ -171,7 +171,7 @@ class AnalyzerWorkerTest : StringSpec({ coVerifyOrder { envService.generateNetRcFile(context, listOf(infrastructureService)) downloader.downloadRepository(repository.url, ortRun.revision) - envService.setUpEnvironment(context, projectDir, infrastructureService) + envService.setUpEnvironment(context, projectDir, null, infrastructureService) } } } @@ -203,7 +203,7 @@ class AnalyzerWorkerTest : StringSpec({ val envService = mockk { every { findInfrastructureServiceForRepository(context) } returns null - coEvery { setUpEnvironment(context, projectDir, null) } returns ResolvedEnvironmentConfig() + coEvery { setUpEnvironment(context, projectDir, null, null) } returns ResolvedEnvironmentConfig() } val worker = AnalyzerWorker( @@ -229,7 +229,7 @@ class AnalyzerWorkerTest : StringSpec({ } coVerify { - envService.setUpEnvironment(context, projectDir, null) + envService.setUpEnvironment(context, projectDir, null, null) } } } @@ -264,7 +264,7 @@ class AnalyzerWorkerTest : StringSpec({ val envService = mockk { every { findInfrastructureServiceForRepository(context) } returns null every { findInfrastructureServiceForRepository(context, envConfig) } returns null - coEvery { setUpEnvironment(context, envConfig) } returns ResolvedEnvironmentConfig() + coEvery { setUpEnvironment(context, projectDir, envConfig, null) } returns ResolvedEnvironmentConfig() } val worker = AnalyzerWorker( @@ -286,7 +286,7 @@ class AnalyzerWorkerTest : StringSpec({ } coVerify { - envService.setUpEnvironment(context, envConfig) + envService.setUpEnvironment(context, projectDir, envConfig, null) } } } @@ -319,7 +319,7 @@ class AnalyzerWorkerTest : StringSpec({ val envService = mockk { every { findInfrastructureServiceForRepository(context) } returns null every { findInfrastructureServiceForRepository(context, envConfig) } returns null - coEvery { setUpEnvironment(context, envConfig) } returns resolvedEnvConfig + coEvery { setUpEnvironment(context, projectDir, envConfig, null) } returns resolvedEnvConfig } val testException = IllegalStateException("AnalyzerRunner test exception") @@ -367,7 +367,8 @@ class AnalyzerWorkerTest : StringSpec({ val resolvedEnvConfig = mockk() val envService = mockk { every { findInfrastructureServiceForRepository(context) } returns null - coEvery { setUpEnvironment(context, projectDir, null) } returns resolvedEnvConfig + every { findInfrastructureServiceForRepository(context, any()) } returns null + coEvery { setUpEnvironment(context, projectDir, null, null) } returns resolvedEnvConfig } val testException = IllegalStateException("AnalyzerRunner test exception") @@ -464,7 +465,7 @@ class AnalyzerWorkerTest : StringSpec({ val envService = mockk { every { findInfrastructureServiceForRepository(context) } returns null - coEvery { setUpEnvironment(context, projectDir, null) } returns ResolvedEnvironmentConfig() + coEvery { setUpEnvironment(context, projectDir, null, null) } returns ResolvedEnvironmentConfig() } val runnerMock = spyk(AnalyzerRunner(ConfigFactory.empty())) { diff --git a/workers/common/src/main/kotlin/common/env/EnvironmentService.kt b/workers/common/src/main/kotlin/common/env/EnvironmentService.kt index 12dc54e19..60cf3d445 100644 --- a/workers/common/src/main/kotlin/common/env/EnvironmentService.kt +++ b/workers/common/src/main/kotlin/common/env/EnvironmentService.kt @@ -35,6 +35,10 @@ import org.eclipse.apoapsis.ortserver.workers.common.env.config.EnvironmentConfi import org.eclipse.apoapsis.ortserver.workers.common.env.config.ResolvedEnvironmentConfig import org.eclipse.apoapsis.ortserver.workers.common.env.definition.EnvironmentServiceDefinition +import org.slf4j.LoggerFactory + +private val logger = LoggerFactory.getLogger(EnvironmentService::class.java) + /** * A service class providing functionality for setting up the build environment when running a worker. * @@ -87,31 +91,21 @@ class EnvironmentService( /** * Set up the analysis environment for the current repository defined by the given [context] that has been * checked out to the given [repositoryFolder]. The credentials of this repository - if any - are defined by the - * given [repositoryService]. + * given [repositoryService]. If an optional [config] is provided, it will be merged with the parsed configuration. + * In case of overlapping entries, the provided [config] will take priority over the parsed configuration. */ suspend fun setUpEnvironment( context: WorkerContext, repositoryFolder: File, + config: EnvironmentConfig?, repositoryService: InfrastructureService? ): ResolvedEnvironmentConfig { - val resolvedConfig = configLoader.resolve(configLoader.parse(repositoryFolder), context.hierarchy) + val mergedConfig = configLoader.parse(repositoryFolder).merge(config) + val resolvedConfig = configLoader.resolve(mergedConfig, context.hierarchy) return setUpEnvironmentForConfig(context, resolvedConfig, repositoryService) } - /** - * Set up the analysis environment for the current repository defined by the given [context] using the provided - * [config]. This function can be used if the environment configuration was passed when the run was triggered. - */ - suspend fun setUpEnvironment( - context: WorkerContext, - config: EnvironmentConfig - ): ResolvedEnvironmentConfig { - val resolvedConfig = configLoader.resolve(config, context.hierarchy) - - return setUpEnvironmentForConfig(context, resolvedConfig, null) - } - /** * Set up the analysis environment based on the given resolved [config]. Use the given [context]. If the repository * has credentials, as defined by the given optional [repositoryService], take them into account as well. @@ -185,3 +179,34 @@ class EnvironmentService( } } } + +/** + * Merge this [EnvironmentConfig] with another [EnvironmentConfig]. The merging process ensures that: + * - Overlapping infrastructure services are overridden by the ones from the [other] config. + * - Environment definitions are combined, with values from both configs being flattened. + * - Environment variables with the same name are overridden by the ones from the [other] config. + */ +internal fun EnvironmentConfig.merge(other: EnvironmentConfig?): EnvironmentConfig { + if (other == null) return this + + val (overridden, unreferenced) = infrastructureServices + .partition { service -> other.infrastructureServices.any { it.name == service.name } } + + if (overridden.isNotEmpty()) { + logger.info( + "The following infrastructure services have been overridden: ${overridden.joinToString { it.name }}." + ) + } + + val mergedInfrastructureService = unreferenced + other.infrastructureServices + val mergedEnvironmentDefinitions = + (environmentDefinitions.asSequence() + other.environmentDefinitions.asSequence()) + .groupBy({ it.key }, { it.value }) + .mapValues { (_, values) -> values.flatten() } + val mergedEnvironmentVariables = (environmentVariables + other.environmentVariables) + .associateBy { it.name } + .values + .toList() + + return EnvironmentConfig(mergedInfrastructureService, mergedEnvironmentDefinitions, mergedEnvironmentVariables) +} diff --git a/workers/common/src/test/kotlin/common/env/EnvironmentServiceTest.kt b/workers/common/src/test/kotlin/common/env/EnvironmentServiceTest.kt index 8497c8041..7802121cd 100644 --- a/workers/common/src/test/kotlin/common/env/EnvironmentServiceTest.kt +++ b/workers/common/src/test/kotlin/common/env/EnvironmentServiceTest.kt @@ -32,16 +32,20 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.mockkStatic import io.mockk.runs import io.mockk.slot +import io.mockk.unmockkAll import java.io.File import java.util.EnumSet import org.eclipse.apoapsis.ortserver.model.CredentialsType import org.eclipse.apoapsis.ortserver.model.EnvironmentConfig +import org.eclipse.apoapsis.ortserver.model.EnvironmentVariableDeclaration import org.eclipse.apoapsis.ortserver.model.Hierarchy import org.eclipse.apoapsis.ortserver.model.InfrastructureService +import org.eclipse.apoapsis.ortserver.model.InfrastructureServiceDeclaration import org.eclipse.apoapsis.ortserver.model.Organization import org.eclipse.apoapsis.ortserver.model.OrtRun import org.eclipse.apoapsis.ortserver.model.Product @@ -56,6 +60,10 @@ import org.eclipse.apoapsis.ortserver.workers.common.env.config.ResolvedEnvironm import org.eclipse.apoapsis.ortserver.workers.common.env.definition.EnvironmentServiceDefinition class EnvironmentServiceTest : WordSpec({ + afterEach { + unmockkAll() + } + "findInfrastructureServiceForRepository" should { "return null if no infrastructure services are defined" { val repository = mockk { @@ -185,7 +193,7 @@ class EnvironmentServiceTest : WordSpec({ val environmentService = EnvironmentService(serviceRepository, listOf(generator1, generator2), configLoader) - val configResult = environmentService.setUpEnvironment(context, repositoryFolder, null) + val configResult = environmentService.setUpEnvironment(context, repositoryFolder, null, null) configResult shouldBe config @@ -206,7 +214,7 @@ class EnvironmentServiceTest : WordSpec({ val assignedServices = serviceRepository.expectServiceAssignments() val environmentService = EnvironmentService(serviceRepository, emptyList(), configLoader) - val configResult = environmentService.setUpEnvironment(context, repositoryFolder, null) + val configResult = environmentService.setUpEnvironment(context, repositoryFolder, null, null) configResult shouldBe config @@ -225,7 +233,7 @@ class EnvironmentServiceTest : WordSpec({ val assignedServices = serviceRepository.expectServiceAssignments() val environmentService = EnvironmentService(serviceRepository, emptyList(), configLoader) - environmentService.setUpEnvironment(context, repositoryFolder, repositoryService) + environmentService.setUpEnvironment(context, repositoryFolder, null, repositoryService) assignedServices shouldContainExactlyInAnyOrder listOf(repositoryService, otherService) } @@ -242,7 +250,7 @@ class EnvironmentServiceTest : WordSpec({ val assignedServices = serviceRepository.expectServiceAssignments() val environmentService = EnvironmentService(serviceRepository, emptyList(), configLoader) - environmentService.setUpEnvironment(context, repositoryFolder, null) + environmentService.setUpEnvironment(context, repositoryFolder, null, null) assignedServices shouldContainExactlyInAnyOrder services } @@ -269,7 +277,7 @@ class EnvironmentServiceTest : WordSpec({ val assignedServices = serviceRepository.expectServiceAssignments() val environmentService = EnvironmentService(serviceRepository, emptyList(), configLoader) - environmentService.setUpEnvironment(context, repositoryFolder, null) + environmentService.setUpEnvironment(context, repositoryFolder, null, null) val expectedAssignedService = service.copy( credentialsTypes = EnumSet.of(CredentialsType.GIT_CREDENTIALS_FILE) @@ -290,7 +298,7 @@ class EnvironmentServiceTest : WordSpec({ val assignedServices = serviceRepository.expectServiceAssignments() val environmentService = EnvironmentService(serviceRepository, emptyList(), configLoader) - environmentService.setUpEnvironment(context, repositoryFolder, repositoryService) + environmentService.setUpEnvironment(context, repositoryFolder, null, repositoryService) assignedServices shouldContainExactlyInAnyOrder services } @@ -307,7 +315,7 @@ class EnvironmentServiceTest : WordSpec({ val generator1 = mockGenerator() val generator2 = mockGenerator() - val envConfig = mockk() + val envConfig = mockk(relaxed = true) val resolvedConfig = ResolvedEnvironmentConfig(emptyList(), definitions) val configLoader = mockConfigLoader(envConfig, resolvedConfig) @@ -316,7 +324,7 @@ class EnvironmentServiceTest : WordSpec({ val environmentService = EnvironmentService(serviceRepository, listOf(generator1, generator2), configLoader) - val configResult = environmentService.setUpEnvironment(context, envConfig) + val configResult = environmentService.setUpEnvironment(context, repositoryFolder, envConfig, null) configResult shouldBe resolvedConfig @@ -331,7 +339,7 @@ class EnvironmentServiceTest : WordSpec({ val context = mockContext() val generator = mockGenerator() - val envConfig = mockk() + val envConfig = mockk(relaxed = true) val resolvedConfig = ResolvedEnvironmentConfig(listOf(service), emptyList()) val configLoader = mockConfigLoader(envConfig, resolvedConfig) @@ -339,7 +347,7 @@ class EnvironmentServiceTest : WordSpec({ serviceRepository.expectServiceAssignments() val environmentService = EnvironmentService(serviceRepository, listOf(generator), configLoader) - environmentService.setUpEnvironment(context, envConfig) + environmentService.setUpEnvironment(context, repositoryFolder, envConfig, null) val (_, definitions) = generator.verify(context, null) definitions shouldHaveSize 1 @@ -386,6 +394,92 @@ class EnvironmentServiceTest : WordSpec({ args.second.map { it.service } shouldContainExactlyInAnyOrder services } } + + "merge" should { + "return the first configuration if the second one is null" { + val config = EnvironmentConfig( + infrastructureServices = listOf(createInfrastructureServiceDeclaration("service")), + environmentDefinitions = mapOf("key" to listOf(mapOf("key" to "value"))), + environmentVariables = listOf(EnvironmentVariableDeclaration("env")) + ) + + val result = config.merge(null) + + result shouldBe config + } + + "combine non-overlapping services" { + val service1 = createInfrastructureServiceDeclaration("service1") + val service2 = createInfrastructureServiceDeclaration("service2") + + val config1 = EnvironmentConfig(infrastructureServices = listOf(service1)) + val config2 = EnvironmentConfig(infrastructureServices = listOf(service2)) + + val result = config1.merge(config2) + + result.infrastructureServices shouldContainExactlyInAnyOrder listOf(service1, service2) + } + + "override overlapping services" { + val service1 = createInfrastructureServiceDeclaration("service") + val service2 = createInfrastructureServiceDeclaration("service") + + val config1 = EnvironmentConfig(infrastructureServices = listOf(service1)) + val config2 = EnvironmentConfig(infrastructureServices = listOf(service2)) + + val result = config1.merge(config2) + + result.infrastructureServices shouldContainExactlyInAnyOrder listOf(service2) + } + + "combine non-overlapping environment definitions" { + val definitions1 = mapOf("key1" to listOf(mapOf("key1" to "value1"))) + val definitions2 = mapOf("key2" to listOf(mapOf("key2" to "value2"))) + + val config1 = EnvironmentConfig(environmentDefinitions = definitions1) + val config2 = EnvironmentConfig(environmentDefinitions = definitions2) + + val result = config1.merge(config2) + + result.environmentDefinitions shouldBe definitions1 + definitions2 + } + + "override overlapping environment definitions" { + val definitions1 = listOf(mapOf("key" to "value1")) + val definitions2 = listOf(mapOf("key" to "value2")) + + val config1 = EnvironmentConfig(environmentDefinitions = mapOf("key" to definitions1)) + val config2 = EnvironmentConfig(environmentDefinitions = mapOf("key" to definitions2)) + + val result = config1.merge(config2) + + result.environmentDefinitions shouldBe mapOf("key" to definitions1 + definitions2) + } + + "combine non-overlapping environment variables" { + val variable1 = EnvironmentVariableDeclaration("env1") + val variable2 = EnvironmentVariableDeclaration("env2") + + val config1 = EnvironmentConfig(environmentVariables = listOf(variable1)) + val config2 = EnvironmentConfig(environmentVariables = listOf(variable2)) + + val result = config1.merge(config2) + + result.environmentVariables shouldContainExactlyInAnyOrder listOf(variable1, variable2) + } + + "override overlapping environment variables" { + val variable1 = EnvironmentVariableDeclaration("env") + val variable2 = EnvironmentVariableDeclaration("env") + + val config1 = EnvironmentConfig(environmentVariables = listOf(variable1)) + val config2 = EnvironmentConfig(environmentVariables = listOf(variable2)) + + val result = config1.merge(config2) + + result.environmentVariables shouldContainExactlyInAnyOrder listOf(variable2) + } + } }) private const val ORGANIZATION_ID = 20230607115501L @@ -407,6 +501,18 @@ private val currentOrtRun = mockk { /** A file representing the checkout folder of the current repository. */ private val repositoryFolder = File("repositoryCheckoutLocation") +/** + * Create an InfrastructureServiceDeclaration with the given [name]. + + */ +fun createInfrastructureServiceDeclaration(name: String): InfrastructureServiceDeclaration = + InfrastructureServiceDeclaration( + name = name, + url = "https://example.org/$name", + usernameSecret = "usernameSecret", + passwordSecret = "passwordSecret", + ) + /** * Create a mock [WorkerContext] object that is prepared to return the [Hierarchy] of the test repository. */ @@ -428,11 +534,17 @@ private fun mockGenerator(): EnvironmentConfigGenerator { - every { parse(repositoryFolder) } returns mockk() +private fun mockConfigLoader(config: ResolvedEnvironmentConfig): EnvironmentConfigLoader { + val envConfig = mockk(relaxed = true) + + mockkStatic(EnvironmentConfig::merge) + every { envConfig.merge(any()) } returns envConfig + + return mockk { + every { parse(repositoryFolder) } returns envConfig every { resolve(any(), repositoryHierarchy) } returns config } +} /** * Create a mock [EnvironmentConfigLoader] that is prepared to resolve the given [envConfig] and to return the @@ -441,10 +553,17 @@ private fun mockConfigLoader(config: ResolvedEnvironmentConfig): EnvironmentConf private fun mockConfigLoader( envConfig: EnvironmentConfig, resultConfig: ResolvedEnvironmentConfig -): EnvironmentConfigLoader = - mockk { +): EnvironmentConfigLoader { + val mockConfig = mockk(relaxed = true) + + mockkStatic(EnvironmentConfig::merge) + every { mockConfig.merge(envConfig) } returns envConfig + + return mockk { + every { parse(repositoryFolder) } returns mockConfig every { resolve(envConfig, repositoryHierarchy) } returns resultConfig } +} /** * Verify that this [EnvironmentConfigGenerator] has been invoked correctly with the given [context]. Optionally,