Skip to content

Commit 0823f60

Browse files
mnonnenmachersschuberth
authored andcommitted
feat(config-worker): Correctly handle restricted package managers
If a plugin is set to `RESTRICTED`, it is only available in organizations where a plugin template is configured for the plugin. Respect that when determining the default list of enabled package managers in the config worker. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@doubleopen.io>
1 parent 7979379 commit 0823f60

6 files changed

Lines changed: 95 additions & 21 deletions

File tree

components/plugin-manager/backend/src/main/kotlin/PluginTemplateService.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ class PluginTemplateService(
263263
* [organizationId]. This is either a template that is assigned to the organization, a global template if no
264264
* template is assigned to the organization, or `null` if no template exists for the organization.
265265
*/
266-
internal fun getTemplateForOrganization(
266+
fun getTemplateForOrganization(
267267
pluginType: PluginType,
268268
pluginId: String,
269269
organizationId: OrganizationId
@@ -571,7 +571,7 @@ class PluginTemplateService(
571571
}
572572
}
573573

574-
internal sealed interface TemplateError {
574+
sealed interface TemplateError {
575575
val message: String
576576

577577
data class InvalidPlugin(override val message: String) : TemplateError

workers/config/src/main/kotlin/ConfigComponent.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,16 @@
2020
package org.eclipse.apoapsis.ortserver.workers.config
2121

2222
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginService
23+
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginTemplateEventStore
24+
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginTemplateService
2325
import org.eclipse.apoapsis.ortserver.dao.databaseModule
26+
import org.eclipse.apoapsis.ortserver.dao.repositories.organization.DaoOrganizationRepository
27+
import org.eclipse.apoapsis.ortserver.dao.repositories.repository.DaoRepositoryRepository
2428
import org.eclipse.apoapsis.ortserver.model.orchestrator.ConfigRequest
2529
import org.eclipse.apoapsis.ortserver.model.orchestrator.ConfigWorkerError
2630
import org.eclipse.apoapsis.ortserver.model.orchestrator.ConfigWorkerResult
31+
import org.eclipse.apoapsis.ortserver.model.repositories.OrganizationRepository
32+
import org.eclipse.apoapsis.ortserver.model.repositories.RepositoryRepository
2733
import org.eclipse.apoapsis.ortserver.transport.ConfigEndpoint
2834
import org.eclipse.apoapsis.ortserver.transport.EndpointComponent
2935
import org.eclipse.apoapsis.ortserver.transport.EndpointHandler
@@ -37,6 +43,7 @@ import org.eclipse.apoapsis.ortserver.workers.common.context.workerContextModule
3743
import org.koin.core.component.inject
3844
import org.koin.core.module.Module
3945
import org.koin.core.module.dsl.singleOf
46+
import org.koin.dsl.bind
4047
import org.koin.dsl.module
4148

4249
/**
@@ -77,5 +84,10 @@ class ConfigComponent : EndpointComponent<ConfigRequest>(ConfigEndpoint) {
7784
private fun configModule(): Module = module {
7885
singleOf(::ConfigWorker)
7986
singleOf(::PluginService)
87+
88+
singleOf(::DaoOrganizationRepository).bind<OrganizationRepository>()
89+
singleOf(::DaoRepositoryRepository).bind<RepositoryRepository>()
90+
singleOf(::PluginTemplateEventStore)
91+
singleOf(::PluginTemplateService)
8092
}
8193
}

workers/config/src/main/kotlin/ConfigWorker.kt

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
package org.eclipse.apoapsis.ortserver.workers.config
2121

2222
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginService
23+
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginTemplateService
2324
import org.eclipse.apoapsis.ortserver.config.Context
2425
import org.eclipse.apoapsis.ortserver.config.Path
2526
import org.eclipse.apoapsis.ortserver.dao.dbQuery
2627
import org.eclipse.apoapsis.ortserver.model.JobConfigurations
28+
import org.eclipse.apoapsis.ortserver.model.OrganizationId
2729
import org.eclipse.apoapsis.ortserver.model.OrtRun
2830
import org.eclipse.apoapsis.ortserver.model.repositories.OrtRunRepository
2931
import org.eclipse.apoapsis.ortserver.model.util.OptionalValue
@@ -54,7 +56,10 @@ class ConfigWorker(
5456
private val adminConfigService: AdminConfigService,
5557

5658
/** The service for accessing plugin information. */
57-
private val pluginService: PluginService
59+
private val pluginService: PluginService,
60+
61+
/** The service for accessing plugin templates. */
62+
private val pluginTemplateService: PluginTemplateService
5863
) {
5964
companion object {
6065
/** Constant for the path to the script that validates and transforms parameters. */
@@ -88,7 +93,7 @@ class ConfigWorker(
8893

8994
// Resolve the default package managers before the validation script runs so the script can see and
9095
// potentially override them.
91-
val baseConfigs = resolveDefaultPackageManagers(context.ortRun.jobConfigs)
96+
val baseConfigs = context.resolveDefaultPackageManagers(context.ortRun.jobConfigs)
9297

9398
// TODO: Currently the path to the validation script is hard-coded. It may make sense to have it
9499
// configurable.
@@ -182,10 +187,14 @@ class ConfigWorker(
182187
* configuration if [AnalyzerJobConfiguration.enabledPackageManagers][org.eclipse.apoapsis.ortserver.model
183188
* .AnalyzerJobConfiguration.enabledPackageManagers] is null or empty.
184189
*/
185-
private fun resolveDefaultPackageManagers(jobConfigs: JobConfigurations): JobConfigurations {
190+
private fun WorkerContext.resolveDefaultPackageManagers(jobConfigs: JobConfigurations): JobConfigurations {
186191
if (!jobConfigs.analyzer.enabledPackageManagers.isNullOrEmpty()) return jobConfigs
187192

188-
val defaults = getDefaultPackageManagers(pluginService)
193+
val defaults = getDefaultPackageManagers(
194+
pluginService,
195+
pluginTemplateService,
196+
OrganizationId(ortRun.organizationId)
197+
)
189198

190199
logger.info("Determined default package managers: $defaults")
191200

workers/config/src/main/kotlin/Utils.kt

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,44 @@
1919

2020
package org.eclipse.apoapsis.ortserver.workers.config
2121

22+
import com.github.michaelbull.result.getOrElse
23+
2224
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginAvailability
2325
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginService
26+
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginTemplateService
2427
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginType
28+
import org.eclipse.apoapsis.ortserver.model.OrganizationId
2529

2630
import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
2731

2832
/**
2933
* Returns the IDs of the package managers that are enabled by default. This includes package managers which are enabled
3034
* by default in ORT and which are enabled in the [pluginService].
3135
*/
32-
fun getDefaultPackageManagers(pluginService: PluginService): List<String> {
36+
fun getDefaultPackageManagers(
37+
pluginService: PluginService,
38+
pluginTemplateService: PluginTemplateService,
39+
organizationId: OrganizationId
40+
): List<String> {
3341
val ortDefaultPackageManagers = AnalyzerConfiguration().enabledPackageManagers
3442

3543
return pluginService.getPlugins()
3644
.filter {
37-
it.type == PluginType.PACKAGE_MANAGER &&
38-
// TODO: Validate if restricted plugins are available in the current organization.
39-
it.availability in listOf(PluginAvailability.ENABLED, PluginAvailability.RESTRICTED) &&
40-
it.id in ortDefaultPackageManagers
45+
val isPluginEnabled by lazy {
46+
when (it.availability) {
47+
PluginAvailability.ENABLED -> true
48+
49+
PluginAvailability.DISABLED -> false
50+
51+
PluginAvailability.RESTRICTED -> {
52+
// Add restricted package managers only if a template exists for the organization.
53+
pluginTemplateService.getTemplateForOrganization(it.type, it.id, organizationId)
54+
.getOrElse { null } != null
55+
}
56+
}
57+
}
58+
59+
it.type == PluginType.PACKAGE_MANAGER && it.id in ortDefaultPackageManagers && isPluginEnabled
4160
}
4261
.map { it.id }
4362
}

workers/config/src/test/kotlin/ConfigWorkerTest.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class ConfigWorkerTest : StringSpec({
8080
}
8181

8282
mockkTransaction {
83-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
83+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
8484
worker.testRun() shouldBe RunResult.Success
8585

8686
verify {
@@ -110,7 +110,7 @@ class ConfigWorkerTest : StringSpec({
110110
}
111111

112112
mockkTransaction {
113-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
113+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
114114
worker.testRun() shouldBe RunResult.Success
115115

116116
verify {
@@ -139,7 +139,7 @@ class ConfigWorkerTest : StringSpec({
139139
}
140140

141141
mockkTransaction {
142-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
142+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
143143
when (val result = worker.testRun()) {
144144
is RunResult.Failed -> result.error should beInstanceOf<IllegalArgumentException>()
145145
else -> AssertionErrorBuilder.fail("Unexpected result: $result")
@@ -164,7 +164,7 @@ class ConfigWorkerTest : StringSpec({
164164
val configManager = mockConfigManager()
165165
every { configManager.getFileAsString(any(), any()) } throws configException
166166

167-
val worker = ConfigWorker(mockk(), mockk(), contextFactory, mockk(), mockPluginService())
167+
val worker = ConfigWorker(mockk(), mockk(), contextFactory, mockk(), mockPluginService(), mockk())
168168
when (val result = worker.testRun()) {
169169
is RunResult.Failed -> result.error shouldBe configException
170170
else -> AssertionErrorBuilder.fail("Unexpected result: $result")
@@ -184,7 +184,7 @@ class ConfigWorkerTest : StringSpec({
184184
}
185185

186186
mockkTransaction {
187-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
187+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
188188
worker.testRun() shouldBe RunResult.Success
189189

190190
val slotContext = mutableListOf<WorkerContext>()
@@ -213,7 +213,7 @@ class ConfigWorkerTest : StringSpec({
213213
}
214214

215215
mockkTransaction {
216-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
216+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
217217
worker.testRun() shouldBe RunResult.Success
218218

219219
val expectedJobConfigs = JobConfigurations(
@@ -245,7 +245,7 @@ class ConfigWorkerTest : StringSpec({
245245
}
246246

247247
mockkTransaction {
248-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
248+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
249249
worker.testRun() shouldBe RunResult.Success
250250

251251
val slotContext = mutableListOf<WorkerContext>()
@@ -273,7 +273,7 @@ class ConfigWorkerTest : StringSpec({
273273
}
274274

275275
mockkTransaction {
276-
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService())
276+
val worker = ConfigWorker(mockk(), ortRunRepository, contextFactory, mockk(), mockPluginService(), mockk())
277277
worker.testRun() shouldBe RunResult.Success
278278

279279
val slotContext = mutableListOf<WorkerContext>()

workers/config/src/test/kotlin/UtilsTest.kt

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.eclipse.apoapsis.ortserver.workers.config
2121

22+
import com.github.michaelbull.result.Ok
23+
2224
import io.kotest.core.spec.style.WordSpec
2325
import io.kotest.matchers.collections.containExactlyInAnyOrder
2426
import io.kotest.matchers.should
@@ -29,11 +31,14 @@ import io.mockk.mockk
2931
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginAvailability
3032
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginDescriptor
3133
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginService
34+
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginTemplate
35+
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginTemplateService
3236
import org.eclipse.apoapsis.ortserver.components.pluginmanager.PluginType
37+
import org.eclipse.apoapsis.ortserver.model.OrganizationId
3338

3439
class UtilsTest : WordSpec({
3540
"getDefaultPackageManagers()" should {
36-
"return all enabled and restricted package managers that are ORT defaults" {
41+
"return all enabled and restricted package managers available for an organization that are ORT defaults" {
3742
val pluginService = mockk<PluginService> {
3843
every { getPlugins() } returns listOf(
3944
PluginDescriptor(
@@ -57,6 +62,13 @@ class UtilsTest : WordSpec({
5762
"description",
5863
availability = PluginAvailability.ENABLED
5964
),
65+
PluginDescriptor(
66+
"Yarn2",
67+
PluginType.PACKAGE_MANAGER,
68+
"Yarn2",
69+
"description",
70+
availability = PluginAvailability.RESTRICTED
71+
),
6072
PluginDescriptor(
6173
"OSV",
6274
PluginType.ADVISOR,
@@ -67,7 +79,29 @@ class UtilsTest : WordSpec({
6779
)
6880
}
6981

70-
val defaultPackageManagers = getDefaultPackageManagers(pluginService)
82+
val pluginTemplateService = mockk<PluginTemplateService> {
83+
every { getTemplateForOrganization(any(), any(), any()) } answers {
84+
// Return a template for NPM but not for Yarn2 to validate that only restricted plugins with a
85+
// template are added to the defaults.
86+
if (secondArg<String>() == "NPM") {
87+
Ok(
88+
PluginTemplate(
89+
name = "PNPM",
90+
pluginType = PluginType.PACKAGE_MANAGER,
91+
pluginId = "template content",
92+
options = emptyList(),
93+
isGlobal = false,
94+
organizationIds = listOf(1)
95+
)
96+
)
97+
} else {
98+
Ok(null)
99+
}
100+
}
101+
}
102+
103+
val defaultPackageManagers =
104+
getDefaultPackageManagers(pluginService, pluginTemplateService, OrganizationId(1))
71105

72106
defaultPackageManagers should containExactlyInAnyOrder("NPM", "PNPM")
73107
}

0 commit comments

Comments
 (0)