Patch Shell class in hdfs to not execute #119189
Conversation
Shell utility in hdfs tries to execute a local script statically to determine whether setsid is available. With the security manager this doesn't work, but hdfs catches the SecurityException and assumes false. With entitlements this doesn't work since hdfs does not know about our NotEntitledException. This commit reworks the patching of hdfs-client-api to use asm. It then adds patching of hdfs' Shell class to replace the method that tries to execute.
rjernst
added
:Delivery/Build
elasticsearchmachine
added
v9.0.0
Team:Core/Infra
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
Pinging @elastic/es-delivery (Team:Delivery) |
|
|
||
| def outputDir = layout.buildDirectory.dir("patched-classes") | ||
|
|
||
| def patchTask = tasks.register("patchClasses", JavaExec) |
There was a problem hiding this comment.
This can be simplified to def patchTask = tasks.register("pachClasses, JavaExec) { // config }
| def patchTask = tasks.register("patchClasses", JavaExec) | ||
| patchTask.configure { | ||
| dependsOn configurations.thejar, sourceSets.patcher.getCompileJavaTaskName() | ||
| outputs.dir(outputDir) |
There was a problem hiding this comment.
We define outputs but no inputs here. A dependsOn doesn't imply an input, but an input does imply a dependency.
Sounds like configurations.thejar and sourcesets.patcher.output should be inputs to this task.
| outputs.dir(outputDir) | ||
| classpath = sourceSets.patcher.runtimeClasspath | ||
| mainClass = 'org.elasticsearch.hdfs.patch.HdfsClassPatcher' | ||
| doFirst { |
There was a problem hiding this comment.
We should be able to call args() with a Provider to avoid having to do this in a task action.
There was a problem hiding this comment.
As discussed, I've kept this as doFirst for now.
| dependsOn(configurations.thejar) | ||
| duplicatesStrategy = DuplicatesStrategy.EXCLUDE | ||
|
|
||
| from(outputDir) // patch directory first so any files patched are excluded as duplicates |
There was a problem hiding this comment.
I'm not completely certain this ordering is deterministic. We should instead explicitly exclude the stuff we are going to patch from the unpacked jar given we know which files these are.
| duplicatesStrategy = DuplicatesStrategy.EXCLUDE | ||
|
|
||
| from(outputDir) // patch directory first so any files patched are excluded as duplicates | ||
| from(project.zipTree(configurations.thejar.singleFile)) |
There was a problem hiding this comment.
This needs to be in a closure otherwise this is going to trigger resolution of this configuration eagerly.
|
Thanks @mark-vieira, I believe I've addressed everything. |
| dependsOn(patchTask) | ||
| dependsOn(configurations.thejar) | ||
|
|
||
| from(outputDir) |
There was a problem hiding this comment.
I think this can actually just become from(patchTask) which will automatically use the task's outputs and you can remove the dependsOn above.
rjernst
added
auto-backport
💚 Backport successful
|
Shell utility in hdfs tries to execute a local script statically to determine whether setsid is available. With the security manager this doesn't work, but hdfs catches the SecurityException and assumes false. With entitlements this doesn't work since hdfs does not know about our NotEntitledException. This commit reworks the patching of hdfs-client-api to use asm. It then adds patching of hdfs' Shell class to replace the method that tries to execute.
Shell utility in hdfs tries to execute a local script statically to determine whether setsid is available. With the security manager this doesn't work, but hdfs catches the SecurityException and assumes false. With entitlements this doesn't work since hdfs does not know about our NotEntitledException. This commit reworks the patching of hdfs-client-api to use asm. It then adds patching of hdfs' Shell class to replace the method that tries to execute.
Shell utility in hdfs tries to execute a local script statically to determine whether setsid is available. With the security manager this doesn't work, but hdfs catches the SecurityException and assumes false. With entitlements hdfs doesn't know about our NotEntitledException, so the exception propagates.
This commit reworks the patching of hdfs-client-api to use asm. It adds patching of hdfs' Shell class to replace the method that tries to execute a process.