Skip to content

Commit 36e4741

Browse files
committed
refactor(workers): Use the SecretService for resolving secret values
Use the `SecretService` to resolve secret values instead of creating a separate instance of `SecretStorage` in `WorkerContextImpl`. Signed-off-by: Martin Nonnenmacher <[email protected]>
1 parent b993753 commit 36e4741

File tree

2 files changed

+20
-20
lines changed

2 files changed

+20
-20
lines changed

workers/common/src/main/kotlin/common/context/WorkerContextImpl.kt

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ import org.eclipse.apoapsis.ortserver.model.ResolvablePluginConfig
4444
import org.eclipse.apoapsis.ortserver.model.Secret
4545
import org.eclipse.apoapsis.ortserver.model.repositories.OrtRunRepository
4646
import org.eclipse.apoapsis.ortserver.model.repositories.RepositoryRepository
47-
import org.eclipse.apoapsis.ortserver.secrets.Path
48-
import org.eclipse.apoapsis.ortserver.secrets.SecretStorage
4947
import org.eclipse.apoapsis.ortserver.workers.common.auth.AuthenticationInfo
5048
import org.eclipse.apoapsis.ortserver.workers.common.auth.AuthenticationListener
5149
import org.eclipse.apoapsis.ortserver.workers.common.auth.CredentialResolverFun
@@ -81,11 +79,8 @@ internal class WorkerContextImpl(
8179
/** The service for accessing secrets. */
8280
private val secretService: SecretService
8381
) : WorkerContext {
84-
/** The object for accessing secrets. */
85-
private val secretStorage by lazy { SecretStorage.createStorage(configManager) }
86-
8782
/** A cache for the secrets that have already been loaded. */
88-
private val secretsCache = ConcurrentHashMap<String, Deferred<String>>()
83+
private val secretsCache = ConcurrentHashMap<Secret, Deferred<String>>()
8984

9085
/** A cache for the configuration secrets that have already been loaded. */
9186
private val configSecretsCache = ConcurrentHashMap<String, Deferred<String>>()
@@ -130,10 +125,10 @@ internal class WorkerContextImpl(
130125
private val currentContext by lazy { ortRun.resolvedJobConfigContext?.let(::Context) }
131126

132127
override suspend fun resolveSecret(secret: Secret): String =
133-
singleTransform(secret, secretsCache, this::resolveSecret, ::extractSecretKey)
128+
singleTransform(secret, secretsCache, ::resolveSecretValue) { it }
134129

135130
override suspend fun resolveSecrets(vararg secrets: Secret): Map<Secret, String> =
136-
parallelTransform(secrets.toList(), secretsCache, this::resolveSecret, ::extractSecretKey)
131+
parallelTransform(secrets.toList(), secretsCache, ::resolveSecretValue) { it }
137132

138133
override suspend fun resolvePluginConfigSecrets(
139134
config: Map<String, ResolvablePluginConfig>?
@@ -263,10 +258,10 @@ internal class WorkerContextImpl(
263258
}
264259

265260
/**
266-
* Resolve the secret with the given [path] using the [SecretStorage] owned by this instance.
261+
* Resolve the value of the provided [secret] using the [secretService].
267262
*/
268-
private fun resolveSecret(path: String): String =
269-
secretStorage.getSecret(Path(path)).value
263+
private fun resolveSecretValue(secret: Secret): String =
264+
secretService.getSecretValue(secret)?.value ?: error("Could not resolve secret at path '${secret.path}'")
270265

271266
/**
272267
* Resolve the given [secret] from the configuration manager.
@@ -287,11 +282,6 @@ internal class WorkerContextImpl(
287282
}
288283
}
289284

290-
/**
291-
* A key extraction function for [Secret]s. As key for the given [secret] its path is used.
292-
*/
293-
private fun extractSecretKey(secret: Secret): String = secret.path
294-
295285
/**
296286
* Return a key extraction function for configuration files that are downloaded to the given [directory] and optionally
297287
* renamed to the given [targetName]. The resulting function ensures that all relevant components are reflected in

workers/common/src/test/kotlin/common/context/WorkerContextTest.kt

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import io.mockk.slot
4848
import io.mockk.unmockkAll
4949
import io.mockk.verify
5050

51+
import org.eclipse.apoapsis.ortserver.components.secrets.SecretService
5152
import org.eclipse.apoapsis.ortserver.config.ConfigFileProviderFactoryForTesting
5253
import org.eclipse.apoapsis.ortserver.config.ConfigManager
5354
import org.eclipse.apoapsis.ortserver.config.ConfigSecretProviderFactoryForTesting
@@ -63,6 +64,7 @@ import org.eclipse.apoapsis.ortserver.model.Secret
6364
import org.eclipse.apoapsis.ortserver.model.SecretSource
6465
import org.eclipse.apoapsis.ortserver.model.repositories.OrtRunRepository
6566
import org.eclipse.apoapsis.ortserver.model.repositories.RepositoryRepository
67+
import org.eclipse.apoapsis.ortserver.model.repositories.SecretRepository
6668
import org.eclipse.apoapsis.ortserver.secrets.Path as SecretPath
6769
import org.eclipse.apoapsis.ortserver.secrets.SecretStorage
6870
import org.eclipse.apoapsis.ortserver.secrets.SecretValue
@@ -572,9 +574,6 @@ class WorkerContextTest : WordSpec({
572574
"be aware of later changes of authentication data" {
573575
val context = helper.context()
574576

575-
// Make sure the secrets provider is initialized.
576-
context.resolveSecret(createSecret(SecretsProviderFactoryForTesting.PASSWORD_PATH.path))
577-
578577
val resolverFun = context.credentialResolverFun
579578

580579
val secretsProvider = SecretsProviderFactoryForTesting.instance()
@@ -650,8 +649,19 @@ private class ContextFactoryTestHelper {
650649
/** Mock for the [RepositoryRepository]. */
651650
val repositoryRepository: RepositoryRepository = mockk()
652651

652+
/** Mock for the [SecretRepository]. */
653+
val secretRepository: SecretRepository = mockk()
654+
655+
/** The [SecretService] used by the test factory. */
656+
val secretService = SecretService(
657+
mockk(),
658+
secretRepository,
659+
SecretStorage(SecretsProviderFactoryForTesting().createProvider())
660+
)
661+
653662
/** The factory to be tested. */
654-
val factory: WorkerContextFactory = WorkerContextFactory(config, ortRunRepository, repositoryRepository, mockk())
663+
val factory: WorkerContextFactory =
664+
WorkerContextFactory(config, ortRunRepository, repositoryRepository, secretService)
655665

656666
/**
657667
* Prepare the mock [OrtRunRepository] to be queried for the test run ID. Return a mock run that is also returned

0 commit comments

Comments
 (0)