Skip to content

Commit

Permalink
Introduce checkAccessModifier#ignoreFailures
Browse files Browse the repository at this point in the history
This was requested to ease into the new functionality.
checkAccessModifier will run by default, but not fail the build. It will
never be considered up-to-date when ignoreFailures is true.

Fixes #176.
  • Loading branch information
sghill committed Dec 31, 2020
1 parent 3c77717 commit 6254b41
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 5 deletions.
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,22 @@ Listening for transport dt_socket at address: 6000
Any additional dependencies for the `server` task's classpath can be added to the `jenkinsServerRuntimeOnly`
configuration. This can be useful for alternative logging implementations.


### Checking for Restricted APIs

Starting with v0.41.0, we now [check for using `@Restricted`][restricted] types, methods, and fields.

Initially this functionality will warn by default (see [#176][176]), but will eventually fail the build. To opt-into failing
the build now, add this configuration to build.gradle:

```gradle
tasks.named('checkAccessModifier').configure {
ignoreFailures.set(false)
}
```

`checkAccessModifier` will only be cached on success if `ignoreFailures` is `false`.

## Disabling SHA256 and SHA512 checksums when releasing a plugin

This section applies to the warning:
Expand Down Expand Up @@ -278,3 +294,5 @@ Here are some real world examples of Jenkins plugins using the Gradle JPI plugin
* [Doktor Plugin](https://github.com/jenkinsci/doktor-plugin)

[javaexecspec]: https://docs.gradle.org/current/javadoc/org/gradle/process/JavaExecSpec.html
[restricted]: https://tiny.cc/jenkins-restricted
[176]: https://github.com/jenkinsci/gradle-jpi-plugin/issues/176
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ open class AccessModifierPlugin : Plugin<Project> {
accessModifierProperties.set(propertyProvider)
compileClasspath.from(target.configurations.getByName("compileClasspath"))
compilationDirs.from(dirs)
ignoreFailures.convention(true)
outputDirectory.set(project.layout.buildDirectory.dir("access-modifier"))
outputs.upToDateWhen { !ignoreFailures.get() }
}
tasks.named("check").configure {
dependsOn(checkAccessModifier)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ abstract class CheckAccess : WorkAction<CheckAccessParameters> {
checker.check(parameters.dirToCheck.asFile.get())
parameters.outputFile.asFile.get().writeText(listener.errorMessage())
if (listener.hasErrors()) {
LOGGER.error(listener.errorMessage())
throw RestrictedApiException()
if (parameters.ignoreFailures.get()) {
LOGGER.warn(listener.errorMessage())
} else {
LOGGER.error(listener.errorMessage())
throw RestrictedApiException()
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import org.gradle.api.DefaultTask
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.provider.MapProperty
import org.gradle.api.provider.Property
import org.gradle.api.tasks.Classpath
import org.gradle.api.tasks.CompileClasspath
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.TaskAction
import org.gradle.kotlin.dsl.mapProperty
import org.gradle.kotlin.dsl.property
import org.gradle.kotlin.dsl.submit
import org.gradle.workers.WorkerExecutor
import javax.inject.Inject
Expand Down Expand Up @@ -38,6 +40,9 @@ open class CheckAccessModifierTask @Inject constructor(private val workerExecuto
@InputFiles
val compilationDirs: ConfigurableFileCollection = project.objects.fileCollection()

@Input
val ignoreFailures: Property<Boolean> = project.objects.property()

@OutputDirectory
val outputDirectory: DirectoryProperty = project.objects.directoryProperty()

Expand All @@ -50,6 +55,7 @@ open class CheckAccessModifierTask @Inject constructor(private val workerExecuto
q.submit(CheckAccess::class) {
classpathToScan.from(compilationDirs, compileClasspath)
dirToCheck.set(compilationDir)
ignoreFailures.set(this@CheckAccessModifierTask.ignoreFailures)
propertiesForAccessModifier.set(accessModifierProperties)
outputFile.set(outputDirectory.file("${compilationDir.name}-${compilationDir.parentFile.name}.txt"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.provider.MapProperty
import org.gradle.api.provider.Property
import org.gradle.workers.WorkParameters

interface CheckAccessParameters : WorkParameters {
val propertiesForAccessModifier: MapProperty<String, Any>
val classpathToScan: ConfigurableFileCollection
val dirToCheck: DirectoryProperty
val ignoreFailures: Property<Boolean>
val outputFile: RegularFileProperty
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ class CheckAccessModifierCachingIntegrationSpec extends IntegrationSpec {
jenkinsVersion = '$jenkinsVersion'
}
tasks.named('checkAccessModifier').configure {
ignoreFailures.set($ignoreFailures)
}
dependencies {
<% for (dep in dependencies) { %>
${dep.configuration} '${dep.coordinate}'
Expand All @@ -51,6 +55,7 @@ class CheckAccessModifierCachingIntegrationSpec extends IntegrationSpec {
def made = template.make([
'jenkinsVersion': TestSupport.RECENT_JENKINS_VERSION,
'dependencies' : [],
'ignoreFailures': false,
])
build.withWriter { made.writeTo(it) }
srcMainJava = new File(projectDir.root, 'src/main/java').toPath()
Expand All @@ -76,6 +81,7 @@ class CheckAccessModifierCachingIntegrationSpec extends IntegrationSpec {
def made = template.make([
'jenkinsVersion': TestSupport.RECENT_JENKINS_VERSION,
'dependencies' : before.collect { ['configuration': configuration, 'coordinate': it] },
'ignoreFailures': false,
])
build.withWriter { made.writeTo(it) }

Expand All @@ -93,6 +99,7 @@ class CheckAccessModifierCachingIntegrationSpec extends IntegrationSpec {
def remade = this.template.make([
'jenkinsVersion': TestSupport.RECENT_JENKINS_VERSION,
'dependencies' : after.collect { ['configuration': configuration, 'coordinate': it] },
'ignoreFailures': false,
])
build.withWriter { remade.writeTo(it) }
def rerunResult = gradleRunner()
Expand Down Expand Up @@ -135,6 +142,7 @@ class CheckAccessModifierCachingIntegrationSpec extends IntegrationSpec {
'dependencies' : [
['configuration': 'implementation', 'coordinate': 'org.jenkins-ci.plugins:mercurial:2.10'],
],
'ignoreFailures': false,
])
build.withWriter { made.writeTo(it) }

Expand All @@ -155,6 +163,44 @@ class CheckAccessModifierCachingIntegrationSpec extends IntegrationSpec {
rerunResult.task(taskPath).outcome == TaskOutcome.FAILED
}

def 'should not cache success if ignoreFailures'() {
given:
JavaFile.builder('org.example.restricted', ok.toBuilder()
.addMethod(MethodSpec.methodBuilder('callDoNotUse')
.addStatement('$1T o = new $1T()', ClassName.get('hudson.plugins.mercurial', 'MercurialChangeSet'))
.addStatement('o.setMsg($S)', 'some message')
.build())
.build())
.build()
.writeTo(srcMainJava)
def made = template.make([
'jenkinsVersion': TestSupport.RECENT_JENKINS_VERSION,
'dependencies' : [
['configuration': 'implementation', 'coordinate': 'org.jenkins-ci.plugins:mercurial:2.10'],
],
'ignoreFailures': true,
])
build.withWriter { made.writeTo(it) }

when:
def result = gradleRunner()
.withArguments(CheckAccessModifierTask.NAME)
.build()

then:
result.task(taskPath).outcome == TaskOutcome.SUCCESS
result.output.contains('hudson/plugins/mercurial/MercurialChangeSet.setMsg(Ljava/lang/String;)V must not be used')

when:
def rerunResult = gradleRunner()
.withArguments(CheckAccessModifierTask.NAME)
.build()

then:
rerunResult.task(taskPath).outcome == TaskOutcome.SUCCESS
rerunResult.output.contains('hudson/plugins/mercurial/MercurialChangeSet.setMsg(Ljava/lang/String;)V must not be used')
}

def 'should rerun when source added'() {
given:
okFile.writeTo(srcMainJava)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.jenkinsci.gradle.plugins.accmod

import com.squareup.javapoet.ClassName
import com.squareup.javapoet.JavaFile
import com.squareup.javapoet.MethodSpec
import com.squareup.javapoet.TypeSpec
import org.gradle.testkit.runner.TaskOutcome
import org.jenkinsci.gradle.plugins.jpi.IntegrationSpec
import org.jenkinsci.gradle.plugins.jpi.TestDataGenerator
import org.jenkinsci.gradle.plugins.jpi.TestSupport

import javax.lang.model.element.Modifier
import java.nio.file.Path

class CheckAccessModifierIgnoreFailuresIntegrationSpec extends IntegrationSpec {
private final String projectName = TestDataGenerator.generateName()
private File build
private Path srcMainJava

def setup() {
File settings = projectDir.newFile('settings.gradle')
settings << """rootProject.name = \"$projectName\""""
build = projectDir.newFile('build.gradle')
build << """\
plugins {
id 'org.jenkins-ci.jpi'
}
jenkinsPlugin {
jenkinsVersion.set('${TestSupport.RECENT_JENKINS_VERSION}')
}
dependencies {
implementation 'org.jenkins-ci.plugins:mercurial:2.10'
}
""".stripIndent()
srcMainJava = new File(projectDir.root, 'src/main/java').toPath()
JavaFile.builder('org.example', TypeSpec.classBuilder('Consumer')
.addModifiers(Modifier.PUBLIC)
.addMethod(MethodSpec.methodBuilder('callDoNotUse')
.addStatement('$1T o = new $1T()', ClassName.get('hudson.plugins.mercurial', 'MercurialChangeSet'))
.addStatement('o.setMsg($S)', 'some message')
.build())
.addMethod(MethodSpec.methodBuilder('callNoExternalUse')
.addStatement('$1T o = new $1T()', ClassName.get('hudson.plugins.mercurial', 'MercurialStatus'))
.addStatement('o.doNotifyCommit($S, $S, $S)', '', '', '')
.addException(ClassName.get('javax.servlet', 'ServletException'))
.addException(IOException)
.build())
.build())
.build()
.writeToPath(srcMainJava)
}

def 'should not fail when using @Restricted method by default'() {
when:
def result = gradleRunner()
.withArguments(CheckAccessModifierTask.NAME, '-s')
.build()

then:
result.task(':' + CheckAccessModifierTask.NAME).outcome == TaskOutcome.SUCCESS
result.output.contains('hudson/plugins/mercurial/MercurialChangeSet.setMsg(Ljava/lang/String;)V must not be used')
result.output.contains('hudson/plugins/mercurial/MercurialStatus.doNotifyCommit(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Lorg/kohsuke/stapler/HttpResponse; must not be used')
}

def 'should allow overriding by configuring task property'() {
given:
build << '''
tasks.named('checkAccessModifier').configure {
ignoreFailures.set(false)
}
'''.stripIndent()

when:
def result = gradleRunner()
.withArguments(CheckAccessModifierTask.NAME)
.buildAndFail()

then:
result.task(':' + CheckAccessModifierTask.NAME).outcome == TaskOutcome.FAILED
result.output.contains('hudson/plugins/mercurial/MercurialChangeSet.setMsg(Ljava/lang/String;)V must not be used')
result.output.contains('hudson/plugins/mercurial/MercurialStatus.doNotifyCommit(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Lorg/kohsuke/stapler/HttpResponse; must not be used')
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import groovy.transform.CompileStatic
import org.gradle.testkit.runner.TaskOutcome
import org.jenkinsci.gradle.plugins.jpi.IntegrationSpec
import org.jenkinsci.gradle.plugins.jpi.TestDataGenerator
import org.jenkinsci.gradle.plugins.jpi.TestSupport
import org.kohsuke.accmod.Restricted
import org.kohsuke.accmod.restrictions.Beta
import org.kohsuke.accmod.restrictions.DoNotUse
Expand Down Expand Up @@ -42,14 +43,17 @@ class CheckAccessModifierIntegrationSpec extends IntegrationSpec {
File settings = projectDir.newFile('settings.gradle')
settings << """rootProject.name = \"$projectName\""""
build = projectDir.newFile('build.gradle')
build << '''\
build << """\
plugins {
id 'org.jenkins-ci.jpi'
}
jenkinsPlugin {
coreVersion = '2.204.6'
jenkinsVersion.set('${TestSupport.RECENT_JENKINS_VERSION}')
}
'''.stripIndent()
tasks.named('checkAccessModifier').configure {
ignoreFailures.set(false)
}
""".stripIndent()
srcMainJava = new File(projectDir.root, 'src/main/java').toPath()
srcMainGroovy = new File(projectDir.root, 'src/main/groovy').toPath()
ohNo = TypeSpec.classBuilder('OhNo')
Expand Down

0 comments on commit 6254b41

Please sign in to comment.