Skip to content

Commit a0a3294

Browse files
committed
refactor(workers): Simplify secret resolution
Use the later introduced function `SecretService.listForHierarchy` to simplify the secret resolution in `EnvironmentConfigLoader`. Signed-off-by: Martin Nonnenmacher <[email protected]>
1 parent 23cfc6c commit a0a3294

File tree

3 files changed

+12
-37
lines changed

3 files changed

+12
-37
lines changed

workers/analyzer/src/test/kotlin/AnalyzerEndpointTest.kt

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,11 @@ import org.eclipse.apoapsis.ortserver.model.OrtRun
5959
import org.eclipse.apoapsis.ortserver.model.OrtRunStatus
6060
import org.eclipse.apoapsis.ortserver.model.Product
6161
import org.eclipse.apoapsis.ortserver.model.Repository
62-
import org.eclipse.apoapsis.ortserver.model.RepositoryId
6362
import org.eclipse.apoapsis.ortserver.model.RepositoryType
6463
import org.eclipse.apoapsis.ortserver.model.Secret
6564
import org.eclipse.apoapsis.ortserver.model.orchestrator.AnalyzerRequest
6665
import org.eclipse.apoapsis.ortserver.model.orchestrator.AnalyzerWorkerError
6766
import org.eclipse.apoapsis.ortserver.model.orchestrator.AnalyzerWorkerResult
68-
import org.eclipse.apoapsis.ortserver.model.util.ListQueryParameters
69-
import org.eclipse.apoapsis.ortserver.model.util.ListQueryResult
7067
import org.eclipse.apoapsis.ortserver.secrets.SecretsProviderFactoryForTesting
7168
import org.eclipse.apoapsis.ortserver.services.config.AdminConfig
7269
import org.eclipse.apoapsis.ortserver.services.config.AdminConfigService
@@ -314,8 +311,7 @@ class AnalyzerEndpointTest : KoinTest, StringSpec() {
314311
val usernameSecret = Secret(20230627040646L, "p1", "repositoryUsername", null, null, null, repository.id)
315312
val passwordSecret = Secret(20230627070543L, "p2", "repositoryPassword", null, null, null, repository.id)
316313
declareMock<SecretService> {
317-
coEvery { listForId(RepositoryId(repository.id)) } returns
318-
ListQueryResult(listOf(usernameSecret, passwordSecret), ListQueryParameters.DEFAULT, 2)
314+
coEvery { listForHierarchy(testHierarchy) } returns listOf(usernameSecret, passwordSecret)
319315
}
320316

321317
declareMock<InfrastructureServiceService> {

workers/common/src/main/kotlin/common/env/config/EnvironmentConfigLoader.kt

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import org.eclipse.apoapsis.ortserver.model.InfrastructureService
3333
import org.eclipse.apoapsis.ortserver.model.InfrastructureServiceDeclaration
3434
import org.eclipse.apoapsis.ortserver.model.OrganizationId
3535
import org.eclipse.apoapsis.ortserver.model.ProductId
36-
import org.eclipse.apoapsis.ortserver.model.RepositoryId
3736
import org.eclipse.apoapsis.ortserver.model.Secret
3837
import org.eclipse.apoapsis.ortserver.utils.logging.runBlocking
3938
import org.eclipse.apoapsis.ortserver.workers.common.env.definition.EnvironmentServiceDefinition
@@ -225,21 +224,13 @@ class EnvironmentConfigLoader(
225224
allSecretsNames += service.passwordSecret
226225
}
227226

228-
val resolvedSecrets = mutableMapOf<String, Secret>()
229-
230-
fun fetchSecrets(fetcher: suspend () -> List<Secret>) {
231-
if (allSecretsNames.isNotEmpty()) {
232-
val secrets = runBlocking { fetcher() }
233-
234-
val secretsMap = secrets.associateBy(Secret::name)
235-
resolvedSecrets += secretsMap
236-
allSecretsNames -= secretsMap.keys
237-
}
227+
val resolvedSecrets = if (allSecretsNames.isNotEmpty()) {
228+
runBlocking { secretService.listForHierarchy(hierarchy) }.associateBy(Secret::name)
229+
} else {
230+
emptyMap()
238231
}
239232

240-
fetchSecrets { secretService.listForId(RepositoryId(hierarchy.repository.id)).data }
241-
fetchSecrets { secretService.listForId(ProductId(hierarchy.product.id)).data }
242-
fetchSecrets { secretService.listForId(OrganizationId(hierarchy.organization.id)).data }
233+
allSecretsNames -= resolvedSecrets.keys
243234

244235
if (allSecretsNames.isNotEmpty()) {
245236
val message = "Invalid secret names. The following names cannot be resolved: $allSecretsNames"

workers/common/src/test/kotlin/common/env/config/EnvironmentConfigLoaderTest.kt

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import io.kotest.matchers.string.shouldContain
3434

3535
import io.mockk.coEvery
3636
import io.mockk.coVerify
37-
import io.mockk.every
3837
import io.mockk.mockk
3938

4039
import java.io.File
@@ -53,7 +52,6 @@ import org.eclipse.apoapsis.ortserver.model.OrganizationId
5352
import org.eclipse.apoapsis.ortserver.model.Product
5453
import org.eclipse.apoapsis.ortserver.model.ProductId
5554
import org.eclipse.apoapsis.ortserver.model.Repository
56-
import org.eclipse.apoapsis.ortserver.model.RepositoryId
5755
import org.eclipse.apoapsis.ortserver.model.RepositoryType
5856
import org.eclipse.apoapsis.ortserver.model.Secret
5957
import org.eclipse.apoapsis.ortserver.model.util.ListQueryParameters
@@ -182,7 +180,7 @@ class EnvironmentConfigLoaderTest : StringSpec({
182180

183181
// There should be only one call for the repository hierarchy.
184182
coVerify(exactly = 1) {
185-
helper.secretService.listForId(any(), any())
183+
helper.secretService.listForHierarchy(any())
186184
}
187185
}
188186

@@ -544,21 +542,11 @@ private class TestHelper(
544542
*/
545543
private fun initSecretRepository() {
546544
coEvery {
547-
secretService.listForId(RepositoryId(repository.id))
548-
} returns mockk<ListQueryResult<Secret>> {
549-
every { data } returns secrets.filter { it.repositoryId != null }
550-
}
551-
552-
coEvery {
553-
secretService.listForId(ProductId(product.id))
554-
} returns mockk<ListQueryResult<Secret>> {
555-
every { data } returns secrets.filter { it.productId != null }
556-
}
557-
558-
coEvery {
559-
secretService.listForId(OrganizationId(organization.id))
560-
} returns mockk<ListQueryResult<Secret>> {
561-
every { data } returns secrets.filter { it.organizationId != null }
545+
secretService.listForHierarchy(Hierarchy(repository, product, organization))
546+
} returns buildList {
547+
addAll(secrets.filter { it.repositoryId == repository.id })
548+
addAll(secrets.filter { it.productId == product.id && none { secret -> secret.name == it.name } })
549+
addAll(secrets.filter { it.organizationId == organization.id && none { secret -> secret.name == it.name } })
562550
}
563551
}
564552

0 commit comments

Comments
 (0)