Skip to content

Commit

Permalink
feat(advisor)!: Migrate the advisor to the new plugin API
Browse files Browse the repository at this point in the history
Migrate the Advisor API and all plugins to the new plugin API. This
allows to simplify the code in several places, as the plugin factories
and service loader classes do not have to be created by hand anymore.

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>
  • Loading branch information
mnonnenmacher committed Aug 29, 2024
1 parent 71983f1 commit 848e666
Show file tree
Hide file tree
Showing 27 changed files with 134 additions and 114 deletions.
1 change: 1 addition & 0 deletions advisor/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ plugins {

dependencies {
api(projects.model)
api(projects.plugins.api)
api(projects.utils.commonUtils)

implementation(projects.utils.ortUtils)
Expand Down
7 changes: 4 additions & 3 deletions advisor/src/main/kotlin/AdviceProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,21 @@ package org.ossreviewtoolkit.advisor
import org.ossreviewtoolkit.model.AdvisorDetails
import org.ossreviewtoolkit.model.AdvisorResult
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.plugins.api.Plugin

/**
* An abstract class that represents a service that can retrieve any kind of advice information
* for a list of given [Package]s. Examples of such information can be security vulnerabilities, known defects,
* or code analysis results.
*/
abstract class AdviceProvider(val providerName: String) {
interface AdviceProvider : Plugin {
/**
* For a given set of [Package]s, retrieve findings and return a map that associates packages with [AdvisorResult]s.
*/
abstract suspend fun retrievePackageFindings(packages: Set<Package>): Map<Package, AdvisorResult>
suspend fun retrievePackageFindings(packages: Set<Package>): Map<Package, AdvisorResult>

/**
* An object with detail information about this [AdviceProvider].
*/
abstract val details: AdvisorDetails
val details: AdvisorDetails
}
14 changes: 3 additions & 11 deletions advisor/src/main/kotlin/AdviceProviderFactory.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,16 @@ package org.ossreviewtoolkit.advisor

import java.util.ServiceLoader

import org.ossreviewtoolkit.utils.common.Plugin
import org.ossreviewtoolkit.utils.common.TypedConfigurablePluginFactory
import org.ossreviewtoolkit.plugins.api.PluginFactory

/**
* A common abstract class for use with [ServiceLoader] that all [AdviceProviderFactory] classes need to implement.
*/
abstract class AdviceProviderFactory<CONFIG>(override val type: String) :
TypedConfigurablePluginFactory<CONFIG, AdviceProvider> {
interface AdviceProviderFactory : PluginFactory<AdviceProvider> {
companion object {
/**
* All [advice provider factories][AdviceProviderFactory] available in the classpath, associated by their names.
*/
val ALL by lazy { Plugin.getAll<AdviceProviderFactory<*>>() }
val ALL by lazy { PluginFactory.getAll<AdviceProviderFactory, AdviceProvider>() }
}

/**
* Return the provider's type here to allow Clikt to display something meaningful when listing the advisors which
* are enabled by default via their factories.
*/
override fun toString() = type
}
9 changes: 5 additions & 4 deletions advisor/src/main/kotlin/Advisor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.OrtResult
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.config.AdvisorConfiguration
import org.ossreviewtoolkit.plugins.api.PluginConfig
import org.ossreviewtoolkit.utils.ort.Environment

/**
* The class to manage [AdviceProvider]s. It invokes the configured providers and adds their findings to the current
* [OrtResult].
*/
class Advisor(
private val providerFactories: List<AdviceProviderFactory<*>>,
private val providerFactories: List<AdviceProviderFactory>,
private val config: AdvisorConfiguration
) {
/**
Expand Down Expand Up @@ -75,8 +76,8 @@ class Advisor(
logger.info { "There are no packages to give advice for." }
} else {
val providers = providerFactories.map {
val providerConfig = config.config?.get(it.type)
it.create(providerConfig?.options.orEmpty(), providerConfig?.secrets.orEmpty())
val providerConfig = config.config?.get(it.descriptor.className)
it.create(PluginConfig(providerConfig?.options.orEmpty(), providerConfig?.secrets.orEmpty()))
}

providers.map { provider ->
Expand All @@ -85,7 +86,7 @@ class Advisor(

logger.info {
"Found ${providerResults.values.flatMap { it.vulnerabilities }.distinct().size} distinct " +
"vulnerabilities via ${provider.providerName}. "
"vulnerabilities via ${provider.descriptor.name}. "
}

providerResults.keys.takeIf { it.isNotEmpty() }?.let { pkgs ->
Expand Down
8 changes: 5 additions & 3 deletions advisor/src/test/kotlin/AdvisorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import org.ossreviewtoolkit.model.OrtResult
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.Project
import org.ossreviewtoolkit.model.config.AdvisorConfiguration
import org.ossreviewtoolkit.plugins.api.PluginConfig
import org.ossreviewtoolkit.plugins.api.PluginDescriptor

class AdvisorTest : WordSpec({
"retrieveFindings" should {
Expand Down Expand Up @@ -117,8 +119,8 @@ private fun createAdvisor(providers: List<AdviceProvider>): Advisor {
val advisorConfig = AdvisorConfiguration()

val factories = providers.map { provider ->
val factory = mockk<AdviceProviderFactory<*>>()
every { factory.create(emptyMap(), emptyMap()) } returns provider
val factory = mockk<AdviceProviderFactory>()
every { factory.create(PluginConfig(emptyMap(), emptyMap())) } returns provider
factory
}

Expand Down Expand Up @@ -146,7 +148,7 @@ private fun createPackage(index: Int): Package =

private fun mockkAdviceProvider(): AdviceProvider =
mockk<AdviceProvider>().apply {
every { providerName } returns "provider"
every { descriptor } returns PluginDescriptor("provider", "Provider", "", emptyList())
}

private fun mockkAdvisorResult(): AdvisorResult =
Expand Down
2 changes: 1 addition & 1 deletion integrations/completions/ort-completion.fish
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ complete -c ort -n "__fish_seen_subcommand_from advise" -l output-dir -s o -r -F
complete -c ort -n "__fish_seen_subcommand_from advise" -l output-formats -s f -r -fa "JSON YAML" -d 'The list of output formats to be used for the ORT result file(s).'
complete -c ort -n "__fish_seen_subcommand_from advise" -l label -s l -r -d 'Set a label in the ORT result, overwriting any existing label of the same name. Can be used multiple times. For example: --label distribution=external'
complete -c ort -n "__fish_seen_subcommand_from advise" -l resolutions-file -r -F -d 'A file containing issue and rule violation resolutions.'
complete -c ort -n "__fish_seen_subcommand_from advise" -l advisors -s a -r -d 'The comma-separated advisors to use, any of [NexusIQ, OssIndex, OSV, VulnerableCode].'
complete -c ort -n "__fish_seen_subcommand_from advise" -l advisors -s a -r -d 'The comma-separated advisors to use, any of [NexusIq, OssIndex, Osv, VulnerableCode].'
complete -c ort -n "__fish_seen_subcommand_from advise" -l skip-excluded -d 'Do not check excluded projects or packages.'
complete -c ort -n "__fish_seen_subcommand_from advise" -s h -l help -d 'Show this message and exit'

Expand Down
6 changes: 6 additions & 0 deletions plugins/advisors/nexus-iq/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
plugins {
// Apply precompiled plugins.
id("ort-library-conventions")

// Apply third-party plugins.
alias(libs.plugins.ksp)
}

dependencies {
Expand All @@ -29,4 +32,7 @@ dependencies {
implementation(projects.clients.nexusIqClient)
implementation(projects.utils.commonUtils)
implementation(projects.utils.ortUtils)

ksp(projects.advisor)
ksp(projects.plugins.api)
}
29 changes: 10 additions & 19 deletions plugins/advisors/nexus-iq/src/main/kotlin/NexusIq.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ import org.ossreviewtoolkit.model.utils.PurlType
import org.ossreviewtoolkit.model.utils.getPurlType
import org.ossreviewtoolkit.model.vulnerabilities.Vulnerability
import org.ossreviewtoolkit.model.vulnerabilities.VulnerabilityReference
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.plugins.api.OrtPlugin
import org.ossreviewtoolkit.plugins.api.PluginDescriptor
import org.ossreviewtoolkit.utils.common.collectMessages
import org.ossreviewtoolkit.utils.common.enumSetOf
import org.ossreviewtoolkit.utils.ort.OkHttpClientHelper
Expand Down Expand Up @@ -77,23 +78,13 @@ private val READ_TIMEOUT = Duration.ofSeconds(60)
*
* If not both `username` and `password` are provided, authentication is disabled.
*/
class NexusIq(name: String, private val config: NexusIqConfiguration) : AdviceProvider(name) {
class Factory : AdviceProviderFactory<NexusIqConfiguration>("NexusIQ") {
override fun create(config: NexusIqConfiguration) = NexusIq(type, config)

override fun parseConfig(options: Options, secrets: Options): NexusIqConfiguration {
val serverUrl = options.getValue("serverUrl")

return NexusIqConfiguration(
serverUrl = serverUrl,
browseUrl = options["browseUrl"] ?: serverUrl,
username = secrets["username"],
password = secrets["password"]
)
}
}

override val details: AdvisorDetails = AdvisorDetails(providerName, enumSetOf(AdvisorCapability.VULNERABILITIES))
@OrtPlugin(
name = "Nexus IQ",
description = "An advisor that uses Sonatype's Nexus IQ Server to determine vulnerabilities in dependencies.",
factory = AdviceProviderFactory::class
)
class NexusIq(override val descriptor: PluginDescriptor, private val config: NexusIqConfiguration) : AdviceProvider {
override val details = AdvisorDetails(descriptor.className, enumSetOf(AdvisorCapability.VULNERABILITIES))

private val service by lazy {
NexusIqService.create(
Expand Down Expand Up @@ -141,7 +132,7 @@ class NexusIq(name: String, private val config: NexusIqConfiguration) : AdvicePr
component.packageUrl to ComponentDetails(component, SecurityData(emptyList()))
}

issues += Issue(source = providerName, message = it.collectMessages())
issues += Issue(source = descriptor.name, message = it.collectMessages())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ data class NexusIqConfiguration(
val serverUrl: String,

/**
* A URL to use as a base for browsing vulnerability details. Defaults to the server URL.
* A URL to use as a base for browsing vulnerability details. If not set, the [serverUrl] is used.
*/
val browseUrl: String = serverUrl,
val browseUrl: String?,

/**
* The username to use for authentication. If not both [username] and [password] are provided, authentication is
* disabled.
*/
val username: String? = null,
val username: String?,

/**
* The password to use for authentication. If not both [username] and [password] are provided, authentication is
* disabled.
*/
val password: String? = null
val password: String?
)

This file was deleted.

6 changes: 6 additions & 0 deletions plugins/advisors/oss-index/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
plugins {
// Apply precompiled plugins.
id("ort-library-conventions")

// Apply third-party plugins.
alias(libs.plugins.ksp)
}

dependencies {
Expand All @@ -30,5 +33,8 @@ dependencies {
implementation(projects.utils.commonUtils)
implementation(projects.utils.ortUtils)

ksp(projects.advisor)
ksp(projects.plugins.api)

testImplementation(libs.wiremock)
}
29 changes: 12 additions & 17 deletions plugins/advisors/oss-index/src/main/kotlin/OssIndex.kt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.config.PluginConfiguration
import org.ossreviewtoolkit.model.vulnerabilities.Vulnerability
import org.ossreviewtoolkit.model.vulnerabilities.VulnerabilityReference
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.plugins.api.OrtPlugin
import org.ossreviewtoolkit.plugins.api.PluginDescriptor
import org.ossreviewtoolkit.utils.common.collectMessages
import org.ossreviewtoolkit.utils.common.enumSetOf
import org.ossreviewtoolkit.utils.ort.OkHttpClientHelper
Expand All @@ -65,25 +66,19 @@ private const val BULK_REQUEST_SIZE = 128
/**
* A wrapper for Sonatype's [OSS Index](https://ossindex.sonatype.org/) security vulnerability data.
*/
class OssIndex(name: String, config: OssIndexConfiguration) : AdviceProvider(name) {
class Factory : AdviceProviderFactory<OssIndexConfiguration>("OssIndex") {
override fun create(config: OssIndexConfiguration) = OssIndex(type, config)

override fun parseConfig(options: Options, secrets: Options) =
OssIndexConfiguration(
serverUrl = options["serverUrl"],
username = secrets["username"],
password = secrets["password"]
)
}

override val details = AdvisorDetails(providerName, enumSetOf(AdvisorCapability.VULNERABILITIES))
@OrtPlugin(
name = "OSS Index",
description = "An advisor that uses Sonatype's OSS Index to determine vulnerabilities in dependencies.",
factory = AdviceProviderFactory::class
)
class OssIndex(override val descriptor: PluginDescriptor, config: OssIndexConfiguration) : AdviceProvider {
override val details = AdvisorDetails(descriptor.className, enumSetOf(AdvisorCapability.VULNERABILITIES))

private val service by lazy {
OssIndexService.create(
config.serverUrl,
config.username,
config.password,
config.username?.value,
config.password?.value,
OkHttpClientHelper.buildClient()
)
}
Expand Down Expand Up @@ -118,7 +113,7 @@ class OssIndex(name: String, config: OssIndexConfiguration) : AdviceProvider(nam
ComponentReport(purl, reference = "", vulnerabilities = emptyList())
}

issues += Issue(source = providerName, message = it.collectMessages())
issues += Issue(source = descriptor.name, message = it.collectMessages())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,29 @@

package org.ossreviewtoolkit.plugins.advisors.ossindex

import org.ossreviewtoolkit.clients.ossindex.OssIndexService
import org.ossreviewtoolkit.plugins.api.OrtPluginOption
import org.ossreviewtoolkit.plugins.api.Secret

/**
* The configuration for the OSS Index provider.
*/
data class OssIndexConfiguration(
/**
* The base URL of the OSS Index REST API. If undefined, default base URL for the REST API of the public OSS Index
* service.
* The base URL of the OSS Index REST API.
*/
val serverUrl: String? = null,
@OrtPluginOption(defaultValue = OssIndexService.DEFAULT_BASE_URL)
val serverUrl: String,

/**
* The username to use for authentication. If not both [username] and [password] are provided, authentication is
* disabled.
*/
val username: String? = null,
val username: Secret?,

/**
* The password to use for authentication. If not both [username] and [password] are provided, authentication is
* disabled.
*/
val password: String? = null
val password: Secret?
)

This file was deleted.

17 changes: 13 additions & 4 deletions plugins/advisors/oss-index/src/test/kotlin/OssIndexTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ class OssIndexTest : WordSpec({
"OssIndex" should {
"return vulnerability information" {
server.stubComponentsRequest("response_components.json")
val ossIndex = OssIndex(ADVISOR_NAME, OssIndexConfiguration("http://localhost:${server.port()}"))
val ossIndex = OssIndex(
OssIndexFactory().descriptor,
OssIndexConfiguration("http://localhost:${server.port()}", null, null)
)
val packages = COMPONENTS_REQUEST_IDS.mapTo(mutableSetOf()) {
Package.EMPTY.copy(id = it, purl = it.toPurl())
}
Expand Down Expand Up @@ -114,7 +117,10 @@ class OssIndexTest : WordSpec({
aResponse().withStatus(500)
)
)
val ossIndex = OssIndex(ADVISOR_NAME, OssIndexConfiguration("http://localhost:${server.port()}"))
val ossIndex = OssIndex(
OssIndexFactory().descriptor,
OssIndexConfiguration("http://localhost:${server.port()}", null, null)
)
val packages = COMPONENTS_REQUEST_IDS.mapTo(mutableSetOf()) {
Package.EMPTY.copy(id = it, purl = it.toPurl())
}
Expand All @@ -130,14 +136,17 @@ class OssIndexTest : WordSpec({
}

"provide correct details" {
val ossIndex = OssIndex(ADVISOR_NAME, OssIndexConfiguration("http://localhost:${server.port()}"))
val ossIndex = OssIndex(
OssIndexFactory().descriptor,
OssIndexConfiguration("http://localhost:${server.port()}", null, null)
)

ossIndex.details shouldBe AdvisorDetails(ADVISOR_NAME, enumSetOf(AdvisorCapability.VULNERABILITIES))
}
}
})

private const val ADVISOR_NAME = "OssIndexTest"
private const val ADVISOR_NAME = "OssIndex"

private const val TEST_FILES_ROOT = "src/test/assets"

Expand Down
Loading

0 comments on commit 848e666

Please sign in to comment.