Use GroovySourceFileAllowlist to adapt to SECURITY-359 changes#256
Use GroovySourceFileAllowlist to adapt to SECURITY-359 changes#256dwnusbaum merged 6 commits intojenkinsci:masterfrom
Conversation
dwnusbaum
force-pushed
the
post-SECURITY-359
branch
from
b0ead48 to
5d68fc4
Compare
|
Hmm, around 70 tests get skipped in CI (presumably |
| String script = "node {\n" + | ||
| " sh 'mkdir buildWithFROMArgs'\n" + | ||
| " writeFile file: 'Dockerfile', text: '" + dockerFile + "'\n" + | ||
| " withEnv(['DOCKER_BUILDKIT=0']) {\n" + |
There was a problem hiding this comment.
When using BuildKit (which IIUC is currently the default), the built image does not list the FROM image as its parent in docker inspect metadata, and the FROM image is not even present in docker image ls -a, so the test doesn't work at all. dockerFingerprintFrom doesn't run by default as of #180, so I could also see an argument for just deleting the whole test file at this point.
| agent { | ||
| docker { | ||
| image "maven:3-alpine" | ||
| image "maven:3-jdk-8-slim" |
There was a problem hiding this comment.
3-alpine hasn't been updated for around 5 years, and for whatever reason mvn hangs in the container running from the pulled image (but not in the container running from the built image). I suspect it has something to do with QEMU emulation since I am using an M1 Mac, but I really have no idea. I switched to 3-jdk-8-slim, which is actively maintained and has native arm64 builds, and the test passes locally now.
|
I am requesting reviews but leaving this in draft state so that it is not accidentally merged with the incremental dependencies. |
|
Interesting log messages from one of the test failures: |
|
Same test failures as the second-to-last build, will try one more time to try to help understand if they are flakey or failing consistently (they do not fail locally). |
| */ | ||
| @Extension | ||
| public static class ChangelogConditionalScriptAllowlist extends GroovySourceFileAllowlist { | ||
| private final String scriptUrl = AbstractDockerAgent.class.getResource("AbstractDockerPipelineScript.groovy").toString(); |
There was a problem hiding this comment.
Do I get it right, that if AbstractDockerPipelineScript is allowed, then all classes that extend it are also allowed?
There was a problem hiding this comment.
No, they have to be allowed individually. However, the concrete classes are all used by subclasses of WithScriptDescribable, which are handled automatically by jenkinsci/pipeline-model-definition-plugin#529. See specifically WithScriptAllowlist: jenkinsci/pipeline-model-definition-plugin#529 (comment).
The reason we need this entry is that without it, the concrete classes are allowed by WithScriptAllowist, but they cannot be instantiated because their superclass AbstractDockerPipelineScript cannot be found.
| */ | ||
| @Extension | ||
| public static class ChangelogConditionalScriptAllowlist extends GroovySourceFileAllowlist { | ||
| private final String scriptUrl = AbstractDockerAgent.class.getResource("AbstractDockerPipelineScript.groovy").toString(); |
3 participants
See jenkinsci/workflow-cps-plugin#538 and jenkinsci/pipeline-model-definition-plugin#529. This would not be needed except for the fact that
DockerPipelineFromDockerfileScript.groovyandDockerPipelineScript.groovyinherit fromAbstractDockerPipelineScript.groovy, which does not correspond to any concrete subclass ofWithScriptDescribable.