Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate Advisor plugins to configurable plugin API #7690

Merged
merged 3 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion advisor/src/funTest/kotlin/OsvFunTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import io.kotest.matchers.shouldBe
import java.time.Instant

import org.ossreviewtoolkit.advisor.advisors.Osv
import org.ossreviewtoolkit.advisor.advisors.OsvConfiguration
import org.ossreviewtoolkit.model.AdvisorResult
import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.config.OsvConfiguration
import org.ossreviewtoolkit.model.readValue
import org.ossreviewtoolkit.model.utils.toPurl
import org.ossreviewtoolkit.utils.test.getAssetFile
Expand Down
43 changes: 5 additions & 38 deletions advisor/src/main/kotlin/AdviceProviderFactory.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,48 +21,15 @@ package org.ossreviewtoolkit.advisor

import java.util.ServiceLoader

import org.ossreviewtoolkit.model.config.AdvisorConfiguration
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.Plugin
import org.ossreviewtoolkit.utils.ort.ORT_CONFIG_FILENAME
import org.ossreviewtoolkit.utils.ort.ortConfigDirectory
import org.ossreviewtoolkit.utils.common.TypedConfigurablePluginFactory

/**
* A common interface for use with [ServiceLoader] that all [AbstractAdviceProviderFactory] classes need to
* implement.
* A common abstract class for use with [ServiceLoader] that all [AdviceProviderFactory] classes need to implement.
*/
interface AdviceProviderFactory : Plugin {
abstract class AdviceProviderFactory<CONFIG>(override val type: String) :
TypedConfigurablePluginFactory<CONFIG, AdviceProvider> {
/**
* Create an [AdviceProvider] using the specified [config].
*/
fun create(config: AdvisorConfiguration): AdviceProvider
}

/**
* A generic factory class for an [AdviceProvider].
*/
abstract class AbstractAdviceProviderFactory<out T : AdviceProvider>(
override val type: String
) : AdviceProviderFactory {
abstract override fun create(config: AdvisorConfiguration): T

/**
* For providers that require configuration, return the typed configuration dedicated to provider [T] or throw if it
* does not exist.
*/
protected fun <T : Any> AdvisorConfiguration.forProvider(select: AdvisorConfiguration.() -> T?): T =
requireNotNull(select()) {
"No configuration for '$type' found in '${ortConfigDirectory.resolve(ORT_CONFIG_FILENAME)}'."
}

/**
* Return a map with options for the [AdviceProvider] managed by this factory or an empty map if no options are
* available.
*/
protected fun AdvisorConfiguration.providerOptions(): Options = options.orEmpty()[type].orEmpty()

/**
* Return the provider's name here to allow Clikt to display something meaningful when listing the advisors which
* 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
Expand Down
9 changes: 6 additions & 3 deletions advisor/src/main/kotlin/Advisor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ import org.ossreviewtoolkit.utils.ort.Environment
* [OrtResult].
*/
class Advisor(
private val providerFactories: List<AdviceProviderFactory>,
private val providerFactories: List<AdviceProviderFactory<*>>,
private val config: AdvisorConfiguration
) {
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 { Plugin.getAll<AdviceProviderFactory<*>>() }
}

/**
Expand Down Expand Up @@ -83,7 +83,10 @@ class Advisor(
if (packages.isEmpty()) {
logger.info { "There are no packages to give advice for." }
} else {
val providers = providerFactories.map { it.create(config) }
val providers = providerFactories.map {
val providerConfig = config.config?.get(it.type)
it.create(providerConfig?.options.orEmpty(), providerConfig?.secrets.orEmpty())
}

providers.map { provider ->
async {
Expand Down
37 changes: 32 additions & 5 deletions advisor/src/main/kotlin/advisors/GitHubDefects.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@

import org.apache.logging.log4j.kotlin.logger

import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
import org.ossreviewtoolkit.advisor.AdviceProvider
import org.ossreviewtoolkit.advisor.AdviceProviderFactory
import org.ossreviewtoolkit.clients.github.DateTime
import org.ossreviewtoolkit.clients.github.GitHubService
import org.ossreviewtoolkit.clients.github.Paging
Expand All @@ -50,9 +50,9 @@
import org.ossreviewtoolkit.model.Issue
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.Severity
import org.ossreviewtoolkit.model.config.AdvisorConfiguration
import org.ossreviewtoolkit.model.config.GitHubDefectsConfiguration
import org.ossreviewtoolkit.model.config.PluginConfiguration
import org.ossreviewtoolkit.model.createAndLogIssue
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.collectMessages
import org.ossreviewtoolkit.utils.common.enumSetOf
import org.ossreviewtoolkit.utils.ort.filterVersionNames
Expand All @@ -79,6 +79,23 @@
*
* For these reasons, this advisor is more a reference implementation for ORT's defects model and not necessarily
* suitable for production usage.
*
* This [AdviceProvider] offers the following configuration options:
*
* #### [Options][PluginConfiguration.options]
*
* * **`endpointUrl`:** The URL of the GraphQL endpoint to be accessed by the service. If undefined, default is the
* endpoint of the official GitHub GraphQL API.
* * **`labelFilter`:** A list with labels to be used for filtering GitHub issues. See
* [GitHubDefectsConfiguration.labelFilter] for details.
* * **`maxNumberOfIssuesPerRepository`:** The maximum number of defects that are retrieved from a single repository.
* See [GitHubDefectsConfiguration.maxNumberOfIssuesPerRepository] for details.
* * **`parallelRequests`:** Determines the number of requests to the GitHub GraphQL API that are executed in parallel.
* See [GitHubDefectsConfiguration.parallelRequests] for details.
*
* #### [Secrets][PluginConfiguration.secrets]
*
* * **`token`:** The access token to authenticate against the GitHub GraphQL endpoint.
*/
class GitHubDefects(name: String, config: GitHubDefectsConfiguration) : AdviceProvider(name) {
companion object {
Expand All @@ -89,8 +106,18 @@
const val DEFAULT_PARALLEL_REQUESTS = 4
}

class Factory : AbstractAdviceProviderFactory<GitHubDefects>("GitHubDefects") {
override fun create(config: AdvisorConfiguration) = GitHubDefects(type, config.forProvider { gitHubDefects })
class Factory : AdviceProviderFactory<GitHubDefectsConfiguration>("GitHubDefects") {
override fun create(config: GitHubDefectsConfiguration) = GitHubDefects(type, config)

override fun parseConfig(options: Options, secrets: Options) =
GitHubDefectsConfiguration(
token = secrets["token"],
endpointUrl = options["endpointUrl"],

Check warning on line 115 in advisor/src/main/kotlin/advisors/GitHubDefects.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/GitHubDefects.kt#L113-L115

Added lines #L113 - L115 were not covered by tests
labelFilter = options["labelFilter"]?.split(",")?.map { it.trim() }
?: listOf("!duplicate", "!enhancement", "!invalid", "!question", "*"),

Check warning on line 117 in advisor/src/main/kotlin/advisors/GitHubDefects.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/GitHubDefects.kt#L117

Added line #L117 was not covered by tests
Copy link
Member

@sschuberth sschuberth Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now duplicates the default value of GitHubDefectsConfiguration.labelFilter, which IMO is not so nice. In my old draft I tried to solve this by extracting default values to constants (not fully implemented, also see the last commit in that PR).

This raises the general question where default values should be applied. I see basically tree options:

  1. in parseConfig(),
  2. in the constructor of the output config class,
  3. in the consumer of the config class.

My current preference would be 1., and I'd propose a clean-up commit that removes the default parameters from GitHubDefectsConfiguration's constructor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be fine with doing that clean up commit outside of this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if it's resolved within reasonable time, before we forget about it.

maxNumberOfIssuesPerRepository = options["maxNumberOfIssuesPerRepository"]?.toInt(),
parallelRequests = options["parallelRequests"]?.toInt()
)

Check warning on line 120 in advisor/src/main/kotlin/advisors/GitHubDefects.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/GitHubDefects.kt#L120

Added line #L120 was not covered by tests
}

/**
Expand Down
73 changes: 73 additions & 0 deletions advisor/src/main/kotlin/advisors/GitHubDefectsConfiguration.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright (C) 2021 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>)
sschuberth marked this conversation as resolved.
Show resolved Hide resolved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/

package org.ossreviewtoolkit.advisor.advisors

/**
* The configuration for the GitHub Defects advisor.
*/
data class GitHubDefectsConfiguration(
/**
* The access token to authenticate against the GitHub GraphQL endpoint.
*/
val token: String? = null,

/**
* The URL of the GraphQL endpoint to be accessed by the service. If undefined, default is the endpoint of the
* official GitHub GraphQL API.
*/
val endpointUrl: String? = null,

/**
* A list with labels to be used for filtering GitHub issues. With GitHub's data model for issues, it is not
* possible to determine whether a specific issue is actually a defect or something else, e.g. a feature request.
* Via this property, it is possible to limit the issues retrieved by the GitHub defects advisor by filtering for
* specific label values. The filtering works as follows:
* - Each string in this list refers to a label to be matched. The strings are processed in order.
* - If for an issue a label with the name of the current string is found, the issue is included into the result
* set.
* - If the current string starts with one of the characters '-' or '!', it defines an exclusion. So, if an issue
* contains a label named like the current string with the first character removed, this issue is not added to
* the result set, and filtering stops here. (The ordered processing resolves conflicting filters, as the first
* match wins.)
* - Label name matches are case-insensitive.
* - Wildcards are supported; a "*" matches arbitrary characters.
* - If the end of the list is reached and no match was found, the issue is not added to the result set. In order
* to have all issues included for which no specific exclusion was found, a wildcard match "*" can be added at
* the end.
* Per default, some of GitHub's default labels are excluded that typically indicate that an issue is not a defect
* (see https://docs.github.com/en/issues/using-labels-and-milestones-to-track-work/managing-labels#about-default-labels)
*/
val labelFilter: List<String> = listOf("!duplicate", "!enhancement", "!invalid", "!question", "*"),

/**
* The maximum number of defects that are retrieved from a single repository. If a repository contains more
* issues, only this number is returned (the newest ones). Popular libraries hosted on GitHub can really have a
* large number of issues; therefore, it makes sense to restrict the result set produced by this advisor.
*/
val maxNumberOfIssuesPerRepository: Int? = null,

/**
* Determines the number of requests to the GitHub GraphQL API that are executed in parallel. Rather than querying
* each repository one after the other, fetching the data of multiple repositories concurrently can reduce the
* execution times for this advisor implementation. If unspecified, a default value for parallel executions as
* defined in the _GitHubDefects_ class is used.
*/
val parallelRequests: Int? = null
)
35 changes: 30 additions & 5 deletions advisor/src/main/kotlin/advisors/NexusIq.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@

import org.apache.logging.log4j.kotlin.logger

import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
import org.ossreviewtoolkit.advisor.AdviceProvider
import org.ossreviewtoolkit.advisor.AdviceProviderFactory
import org.ossreviewtoolkit.clients.nexusiq.NexusIqService
import org.ossreviewtoolkit.clients.nexusiq.NexusIqService.Component
import org.ossreviewtoolkit.clients.nexusiq.NexusIqService.ComponentDetails
Expand All @@ -41,11 +41,11 @@
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.Vulnerability
import org.ossreviewtoolkit.model.VulnerabilityReference
import org.ossreviewtoolkit.model.config.AdvisorConfiguration
import org.ossreviewtoolkit.model.config.NexusIqConfiguration
import org.ossreviewtoolkit.model.config.PluginConfiguration
import org.ossreviewtoolkit.model.utils.PurlType
import org.ossreviewtoolkit.model.utils.getPurlType
import org.ossreviewtoolkit.model.utils.toPurl
import org.ossreviewtoolkit.utils.common.Options
import org.ossreviewtoolkit.utils.common.collectMessages
import org.ossreviewtoolkit.utils.common.enumSetOf
import org.ossreviewtoolkit.utils.ort.OkHttpClientHelper
Expand All @@ -63,10 +63,35 @@

/**
* A wrapper for [Nexus IQ Server](https://help.sonatype.com/iqserver) security vulnerability data.
*
* This [AdviceProvider] offers the following configuration options:
*
* #### [Options][PluginConfiguration.options]
*
* * **`serverUrl`:** The URL to use for REST API requests against the server.
* * **`browseUrl`:** The URL to use as a base for browsing vulnerability details. Defaults to the server URL.
*
* #### [Secrets][PluginConfiguration.secrets]
*
* * **`username`:** The username to use for authentication.
* * **`password`:** The password to use for authentication.
*
* If not both `username` and `password` are provided, authentication is disabled.
*/
class NexusIq(name: String, private val config: NexusIqConfiguration) : AdviceProvider(name) {
class Factory : AbstractAdviceProviderFactory<NexusIq>("NexusIQ") {
override fun create(config: AdvisorConfiguration) = NexusIq(type, config.forProvider { nexusIq })
class Factory : AdviceProviderFactory<NexusIqConfiguration>("NexusIQ") {
override fun create(config: NexusIqConfiguration) = NexusIq(type, config)

Check warning on line 83 in advisor/src/main/kotlin/advisors/NexusIq.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIq.kt#L82-L83

Added lines #L82 - L83 were not covered by tests

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

Check warning on line 86 in advisor/src/main/kotlin/advisors/NexusIq.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIq.kt#L86

Added line #L86 was not covered by tests

return NexusIqConfiguration(
serverUrl = serverUrl,

Check warning on line 89 in advisor/src/main/kotlin/advisors/NexusIq.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIq.kt#L88-L89

Added lines #L88 - L89 were not covered by tests
browseUrl = options["browseUrl"] ?: serverUrl,
username = secrets["username"],
password = secrets["password"]

Check warning on line 92 in advisor/src/main/kotlin/advisors/NexusIq.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIq.kt#L91-L92

Added lines #L91 - L92 were not covered by tests
)
}
}

override val details: AdvisorDetails = AdvisorDetails(providerName, enumSetOf(AdvisorCapability.VULNERABILITIES))
Expand Down
47 changes: 47 additions & 0 deletions advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright (C) 2020 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/

package org.ossreviewtoolkit.advisor.advisors

/**
* The configuration for Nexus IQ as a security vulnerability provider.
*/
data class NexusIqConfiguration(

Check warning on line 25 in advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt#L25

Added line #L25 was not covered by tests
/**
* The URL to use for REST API requests against the server.
*/
val serverUrl: String,

Check warning on line 29 in advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt#L29

Added line #L29 was not covered by tests

/**
* A URL to use as a base for browsing vulnerability details. Defaults to the server URL.
*/
val browseUrl: String = serverUrl,

Check warning on line 34 in advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt#L34

Added line #L34 was not covered by tests

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

Check warning on line 40 in advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt#L40

Added line #L40 was not covered by tests

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

Check warning on line 47 in advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt

View check run for this annotation

Codecov / codecov/patch

advisor/src/main/kotlin/advisors/NexusIqConfiguration.kt#L46-L47

Added lines #L46 - L47 were not covered by tests
Loading
Loading