From 1bda4f2737452c16efb5a1a05c930bea9c9dc68e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 20 Dec 2024 17:01:12 -0800 Subject: [PATCH 01/10] Patch Shell class in hdfs to not 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. --- plugins/repository-hdfs/build.gradle | 2 +- .../hadoop-client-api/build.gradle | 39 ++++++++++++-- .../hadoop/util/ShutdownHookManager.java | 48 ----------------- .../hdfs/patch/HdfsClassPatcher.java | 53 +++++++++++++++++++ .../hdfs/patch/MethodReplacement.java | 36 +++++++++++++ .../hdfs/patch/ShellPatcher.java | 34 ++++++++++++ .../patch/ShutdownHookManagerPatcher.java | 49 +++++++++++++++++ 7 files changed, 208 insertions(+), 53 deletions(-) delete mode 100644 plugins/repository-hdfs/hadoop-client-api/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java create mode 100644 plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java create mode 100644 plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java create mode 100644 plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShellPatcher.java create mode 100644 plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java diff --git a/plugins/repository-hdfs/build.gradle b/plugins/repository-hdfs/build.gradle index 4da7c24de80f1..dea1e1bdd273f 100644 --- a/plugins/repository-hdfs/build.gradle +++ b/plugins/repository-hdfs/build.gradle @@ -28,7 +28,7 @@ configurations { } dependencies { - api project(path: 'hadoop-client-api', configuration: 'shadow') + api project(path: 'hadoop-client-api', configuration: 'default') if (isEclipse) { /* * Eclipse can't pick up the shadow dependency so we point it at *something* diff --git a/plugins/repository-hdfs/hadoop-client-api/build.gradle b/plugins/repository-hdfs/hadoop-client-api/build.gradle index 24e4213780fe2..d5b15e06c6559 100644 --- a/plugins/repository-hdfs/hadoop-client-api/build.gradle +++ b/plugins/repository-hdfs/hadoop-client-api/build.gradle @@ -1,12 +1,43 @@ apply plugin: 'elasticsearch.build' -apply plugin: 'com.gradleup.shadow' + +sourceSets { + patcher +} + +configurations { + thejar { + canBeResolved = true + } +} dependencies { - implementation "org.apache.hadoop:hadoop-client-api:${project.parent.versions.hadoop}" + thejar("org.apache.hadoop:hadoop-client-api:${project.parent.versions.hadoop}") { + transitive = false + } + + patcherImplementation 'org.ow2.asm:asm:9.7.1' + patcherImplementation 'org.ow2.asm:asm-tree:9.7.1' +} + +def outputDir = layout.buildDirectory.dir("patched-classes") + +def patchTask = tasks.register("patchClasses", JavaExec) +patchTask.configure { + dependsOn configurations.thejar + classpath = sourceSets.patcher.runtimeClasspath + mainClass = 'org.elasticsearch.hdfs.patch.HdfsClassPatcher' + doFirst { + args configurations.thejar.singleFile, outputDir.get().getAsFile().toString() + } } -tasks.named('shadowJar').configure { - exclude 'org/apache/hadoop/util/ShutdownHookManager$*.class' +tasks.named('jar').configure { + dependsOn(patchTask) + dependsOn(configurations.thejar) + duplicatesStrategy = DuplicatesStrategy.EXCLUDE + + from(outputDir) // patch directory first so any files patched are excluded as duplicates + from(project.zipTree(configurations.thejar.singleFile)) } ['jarHell', 'thirdPartyAudit', 'forbiddenApisMain', 'splitPackagesAudit'].each { diff --git a/plugins/repository-hdfs/hadoop-client-api/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java b/plugins/repository-hdfs/hadoop-client-api/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java deleted file mode 100644 index c3d15dc06e7c1..0000000000000 --- a/plugins/repository-hdfs/hadoop-client-api/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -package org.apache.hadoop.util; - -import java.util.concurrent.TimeUnit; - -/** - * A replacement for the ShutdownHookManager from hadoop. - * - * This class does not actually add a shutdown hook. Hadoop's shutdown hook - * manager does not fail gracefully when it lacks security manager permissions - * to add shutdown hooks. This implements the same api as the hadoop class, but - * with no-ops. - */ -public class ShutdownHookManager { - private static final ShutdownHookManager MGR = new ShutdownHookManager(); - - public static ShutdownHookManager get() { - return MGR; - } - - private ShutdownHookManager() {} - - public void addShutdownHook(Runnable shutdownHook, int priority) {} - - public void addShutdownHook(Runnable shutdownHook, int priority, long timeout, TimeUnit unit) {} - - public boolean removeShutdownHook(Runnable shutdownHook) { - return false; - } - - public boolean hasShutdownHook(Runnable shutdownHook) { - return false; - } - - public boolean isShutdownInProgress() { - return false; - } - - public void clearShutdownHooks() {} -} diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java new file mode 100644 index 0000000000000..1806ce4a93ce2 --- /dev/null +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.hdfs.patch; + +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Map; +import java.util.function.Function; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; + +public class HdfsClassPatcher { + static final Map> patchers = Map.of( + "org/apache/hadoop/util/ShutdownHookManager.class", ShutdownHookManagerPatcher::new, + "org/apache/hadoop/util/Shell.class", ShellPatcher::new + ); + + public static void main(String[] args) throws Exception { + String jarPath = args[0]; + Path outputDir = Paths.get(args[1]); + + try (JarFile jarFile = new JarFile(new File(jarPath))) { + for (var patcher : patchers.entrySet()) { + JarEntry jarEntry = jarFile.getJarEntry(patcher.getKey()); + if (jarEntry == null) { + throw new IllegalArgumentException("path [" + patcher.getKey() + "] not found in [" + jarPath + "]"); + } + byte[] classToPatch = jarFile.getInputStream(jarEntry).readAllBytes(); + + ClassReader classReader = new ClassReader(classToPatch); + ClassWriter classWriter = new ClassWriter(classReader, 0); + classReader.accept(patcher.getValue().apply(classWriter), 0); + + Path outputFile = outputDir.resolve(patcher.getKey()); + Files.createDirectories(outputFile.getParent()); + Files.write(outputFile, classWriter.toByteArray()); + } + } + } +} diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java new file mode 100644 index 0000000000000..09c27c41a60bf --- /dev/null +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.hdfs.patch; + +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +public class MethodReplacement extends MethodVisitor { + private final MethodVisitor delegate; + private final Runnable bodyWriter; + + MethodReplacement(MethodVisitor delegate, Runnable bodyWriter) { + super(Opcodes.ASM9); + this.delegate = delegate; + this.bodyWriter = bodyWriter; + } + + @Override + public void visitCode() { + delegate.visitCode(); + bodyWriter.run(); + delegate.visitEnd(); + } + + @Override + public void visitMaxs(int maxStack, int maxLocals) { + delegate.visitMaxs(maxStack, maxLocals); + } +} diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShellPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShellPatcher.java new file mode 100644 index 0000000000000..397b63e434ba2 --- /dev/null +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShellPatcher.java @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.hdfs.patch; + +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +class ShellPatcher extends ClassVisitor { + + ShellPatcher(ClassWriter classWriter) { + super(Opcodes.ASM9, classWriter); + } + + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + if (name.equals("isSetsidSupported")) { + return new MethodReplacement(mv, () -> { + mv.visitInsn(Opcodes.ICONST_0); + mv.visitInsn(Opcodes.IRETURN); + }); + } + return mv; + } +} diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java new file mode 100644 index 0000000000000..24fcc98395376 --- /dev/null +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.hdfs.patch; + +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +import java.util.Set; + +class ShutdownHookManagerPatcher extends ClassVisitor { + private static final Set voidMethods = Set.of( + "addShutdownHook", + "clearShutdownHooks" + ); + private static final Set booleanMethods = Set.of( + "removeShutdownHook", + "hasShutdownHook", + "isShutdownInProgress" + ); + + ShutdownHookManagerPatcher(ClassWriter classWriter) { + super(Opcodes.ASM9, classWriter); + } + + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + if (voidMethods.contains(name)) { + return new MethodReplacement(mv, () -> { + mv.visitInsn(Opcodes.RETURN); + }); + } else if (booleanMethods.contains(name)) { + return new MethodReplacement(mv, () -> { + mv.visitInsn(Opcodes.ICONST_0); + mv.visitInsn(Opcodes.IRETURN); + }); + } + return mv; + } +} From ab3c783ac2ae617a6a072d980b27614747174ecc Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Sat, 21 Dec 2024 01:11:39 +0000 Subject: [PATCH 02/10] [CI] Auto commit changes from spotless --- .../hdfs/patch/HdfsClassPatcher.java | 6 ++++-- .../hdfs/patch/ShutdownHookManagerPatcher.java | 15 +++------------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java index 1806ce4a93ce2..6636b39445964 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/HdfsClassPatcher.java @@ -24,8 +24,10 @@ public class HdfsClassPatcher { static final Map> patchers = Map.of( - "org/apache/hadoop/util/ShutdownHookManager.class", ShutdownHookManagerPatcher::new, - "org/apache/hadoop/util/Shell.class", ShellPatcher::new + "org/apache/hadoop/util/ShutdownHookManager.class", + ShutdownHookManagerPatcher::new, + "org/apache/hadoop/util/Shell.class", + ShellPatcher::new ); public static void main(String[] args) throws Exception { diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java index 24fcc98395376..609ecefda4eef 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -17,15 +17,8 @@ import java.util.Set; class ShutdownHookManagerPatcher extends ClassVisitor { - private static final Set voidMethods = Set.of( - "addShutdownHook", - "clearShutdownHooks" - ); - private static final Set booleanMethods = Set.of( - "removeShutdownHook", - "hasShutdownHook", - "isShutdownInProgress" - ); + private static final Set voidMethods = Set.of("addShutdownHook", "clearShutdownHooks"); + private static final Set booleanMethods = Set.of("removeShutdownHook", "hasShutdownHook", "isShutdownInProgress"); ShutdownHookManagerPatcher(ClassWriter classWriter) { super(Opcodes.ASM9, classWriter); @@ -35,9 +28,7 @@ class ShutdownHookManagerPatcher extends ClassVisitor { public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); if (voidMethods.contains(name)) { - return new MethodReplacement(mv, () -> { - mv.visitInsn(Opcodes.RETURN); - }); + return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.RETURN); }); } else if (booleanMethods.contains(name)) { return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.ICONST_0); From e39c305e84bbe3c601c5029be144d2c7c2b43f6c Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 21 Dec 2024 06:27:45 -0800 Subject: [PATCH 03/10] replace static block --- .../org/elasticsearch/hdfs/patch/MethodReplacement.java | 4 ++-- .../hdfs/patch/ShutdownHookManagerPatcher.java | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java index 09c27c41a60bf..9ad85c004a8bc 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java @@ -24,9 +24,9 @@ public class MethodReplacement extends MethodVisitor { @Override public void visitCode() { - delegate.visitCode(); + //delegate.visitCode(); bodyWriter.run(); - delegate.visitEnd(); + //delegate.visitEnd(); } @Override diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java index 609ecefda4eef..c99f481c58bf7 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -34,6 +34,14 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str mv.visitInsn(Opcodes.ICONST_0); mv.visitInsn(Opcodes.IRETURN); }); + } else if (name.equals("")) { + return new MethodReplacement(mv, () -> { + mv.visitFieldInsn(Opcodes.GETSTATIC, "java/util/concurrent/TimeUnit", "SECONDS", "Ljava/util/concurrent/TimeUnit;"); + mv.visitFieldInsn(Opcodes.PUTSTATIC, "org/apache/hadoop/util/ShutdownHookManager", "TIME_UNIT_DEFAULT", "Ljava/util/concurrent/TimeUnit;"); + mv.visitInsn(Opcodes.ACONST_NULL); + mv.visitFieldInsn(Opcodes.PUTSTATIC, "org/apache/hadoop/util/ShutdownHookManager", "EXECUTOR", "Ljava/util/concurrent/ExecutorService;"); + mv.visitInsn(Opcodes.RETURN); + }); } return mv; } From 9dcb1273bc88a30bce6cd7b00fc7b0acd3c0a0ea Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 21 Dec 2024 06:28:10 -0800 Subject: [PATCH 04/10] spotless --- .../hdfs/patch/MethodReplacement.java | 4 ++-- .../hdfs/patch/ShutdownHookManagerPatcher.java | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java index 9ad85c004a8bc..e07a32cc294a5 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/MethodReplacement.java @@ -24,9 +24,9 @@ public class MethodReplacement extends MethodVisitor { @Override public void visitCode() { - //delegate.visitCode(); + // delegate.visitCode(); bodyWriter.run(); - //delegate.visitEnd(); + // delegate.visitEnd(); } @Override diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java index c99f481c58bf7..dd308a30c453c 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -37,9 +37,19 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str } else if (name.equals("")) { return new MethodReplacement(mv, () -> { mv.visitFieldInsn(Opcodes.GETSTATIC, "java/util/concurrent/TimeUnit", "SECONDS", "Ljava/util/concurrent/TimeUnit;"); - mv.visitFieldInsn(Opcodes.PUTSTATIC, "org/apache/hadoop/util/ShutdownHookManager", "TIME_UNIT_DEFAULT", "Ljava/util/concurrent/TimeUnit;"); + mv.visitFieldInsn( + Opcodes.PUTSTATIC, + "org/apache/hadoop/util/ShutdownHookManager", + "TIME_UNIT_DEFAULT", + "Ljava/util/concurrent/TimeUnit;" + ); mv.visitInsn(Opcodes.ACONST_NULL); - mv.visitFieldInsn(Opcodes.PUTSTATIC, "org/apache/hadoop/util/ShutdownHookManager", "EXECUTOR", "Ljava/util/concurrent/ExecutorService;"); + mv.visitFieldInsn( + Opcodes.PUTSTATIC, + "org/apache/hadoop/util/ShutdownHookManager", + "EXECUTOR", + "Ljava/util/concurrent/ExecutorService;" + ); mv.visitInsn(Opcodes.RETURN); }); } From 31e77e7e1ad725c24e69ce1cce60ba2bfefce394 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 21 Dec 2024 06:38:47 -0800 Subject: [PATCH 05/10] cleanup --- .../patch/ShutdownHookManagerPatcher.java | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java index dd308a30c453c..abb6a42f4d8ad 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -13,12 +13,16 @@ import org.objectweb.asm.ClassWriter; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; class ShutdownHookManagerPatcher extends ClassVisitor { - private static final Set voidMethods = Set.of("addShutdownHook", "clearShutdownHooks"); - private static final Set booleanMethods = Set.of("removeShutdownHook", "hasShutdownHook", "isShutdownInProgress"); + private static final String CLASSNAME = "org/apache/hadoop/util/ShutdownHookManager"; + private static final Set VOID_METHODS = Set.of("addShutdownHook", "clearShutdownHooks"); + private static final Set BOOLEAN_METHODS = Set.of("removeShutdownHook", "hasShutdownHook", "isShutdownInProgress"); ShutdownHookManagerPatcher(ClassWriter classWriter) { super(Opcodes.ASM9, classWriter); @@ -27,29 +31,21 @@ class ShutdownHookManagerPatcher extends ClassVisitor { @Override public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); - if (voidMethods.contains(name)) { + if (VOID_METHODS.contains(name)) { return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.RETURN); }); - } else if (booleanMethods.contains(name)) { + } else if (BOOLEAN_METHODS.contains(name)) { return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.ICONST_0); mv.visitInsn(Opcodes.IRETURN); }); } else if (name.equals("")) { return new MethodReplacement(mv, () -> { - mv.visitFieldInsn(Opcodes.GETSTATIC, "java/util/concurrent/TimeUnit", "SECONDS", "Ljava/util/concurrent/TimeUnit;"); - mv.visitFieldInsn( - Opcodes.PUTSTATIC, - "org/apache/hadoop/util/ShutdownHookManager", - "TIME_UNIT_DEFAULT", - "Ljava/util/concurrent/TimeUnit;" - ); + var timeUnitType = Type.getType(TimeUnit.class); + var executorServiceType = Type.getType(ExecutorService.class); + mv.visitFieldInsn(Opcodes.GETSTATIC, timeUnitType.getInternalName(), "SECONDS", timeUnitType.getDescriptor()); + mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "TIME_UNIT_DEFAULT", timeUnitType.getDescriptor()); mv.visitInsn(Opcodes.ACONST_NULL); - mv.visitFieldInsn( - Opcodes.PUTSTATIC, - "org/apache/hadoop/util/ShutdownHookManager", - "EXECUTOR", - "Ljava/util/concurrent/ExecutorService;" - ); + mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "EXECUTOR", executorServiceType.getDescriptor()); mv.visitInsn(Opcodes.RETURN); }); } From 05b18045dcb8197db7ba9f574f8e11888df8a4e1 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 21 Dec 2024 06:40:00 -0800 Subject: [PATCH 06/10] add comments --- .../elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java index abb6a42f4d8ad..f5370b977c8c9 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -32,14 +32,17 @@ class ShutdownHookManagerPatcher extends ClassVisitor { public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); if (VOID_METHODS.contains(name)) { + // make void methods noops return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.RETURN); }); } else if (BOOLEAN_METHODS.contains(name)) { + // make boolean methods always return false return new MethodReplacement(mv, () -> { mv.visitInsn(Opcodes.ICONST_0); mv.visitInsn(Opcodes.IRETURN); }); } else if (name.equals("")) { return new MethodReplacement(mv, () -> { + // just initialize the two statics, don't actually get runtime to add shutdown hook var timeUnitType = Type.getType(TimeUnit.class); var executorServiceType = Type.getType(ExecutorService.class); mv.visitFieldInsn(Opcodes.GETSTATIC, timeUnitType.getInternalName(), "SECONDS", timeUnitType.getDescriptor()); From fdbaab6dafde765109a9f7e2411c7d5d72f865cf Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 21 Dec 2024 06:45:43 -0800 Subject: [PATCH 07/10] cleanup gradle --- .../repository-hdfs/hadoop-client-api/build.gradle | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/build.gradle b/plugins/repository-hdfs/hadoop-client-api/build.gradle index d5b15e06c6559..c20d391e6bedd 100644 --- a/plugins/repository-hdfs/hadoop-client-api/build.gradle +++ b/plugins/repository-hdfs/hadoop-client-api/build.gradle @@ -1,4 +1,4 @@ -apply plugin: 'elasticsearch.build' +apply plugin: 'elasticsearch.java' sourceSets { patcher @@ -23,7 +23,8 @@ def outputDir = layout.buildDirectory.dir("patched-classes") def patchTask = tasks.register("patchClasses", JavaExec) patchTask.configure { - dependsOn configurations.thejar + dependsOn configurations.thejar, sourceSets.patcher.getCompileJavaTaskName() + outputs.dir(outputDir) classpath = sourceSets.patcher.runtimeClasspath mainClass = 'org.elasticsearch.hdfs.patch.HdfsClassPatcher' doFirst { @@ -39,9 +40,3 @@ tasks.named('jar').configure { from(outputDir) // patch directory first so any files patched are excluded as duplicates from(project.zipTree(configurations.thejar.singleFile)) } - -['jarHell', 'thirdPartyAudit', 'forbiddenApisMain', 'splitPackagesAudit'].each { - tasks.named(it).configure { - enabled = false - } -} From eb89cc41beab842a96f9351f28fcc9f40615b9e9 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 21 Dec 2024 08:24:00 -0800 Subject: [PATCH 08/10] init MGR too --- .../hdfs/patch/ShutdownHookManagerPatcher.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java index f5370b977c8c9..1235b5af9002f 100644 --- a/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java +++ b/plugins/repository-hdfs/hadoop-client-api/src/patcher/java/org/elasticsearch/hdfs/patch/ShutdownHookManagerPatcher.java @@ -42,13 +42,22 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str }); } else if (name.equals("")) { return new MethodReplacement(mv, () -> { - // just initialize the two statics, don't actually get runtime to add shutdown hook + // just initialize the statics, don't actually get runtime to add shutdown hook + + var classType = Type.getObjectType(CLASSNAME); + mv.visitTypeInsn(Opcodes.NEW, CLASSNAME); + mv.visitInsn(Opcodes.DUP); + mv.visitMethodInsn(Opcodes.INVOKESPECIAL, CLASSNAME, "", "()V", false); + mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "MGR", classType.getDescriptor()); + var timeUnitType = Type.getType(TimeUnit.class); - var executorServiceType = Type.getType(ExecutorService.class); mv.visitFieldInsn(Opcodes.GETSTATIC, timeUnitType.getInternalName(), "SECONDS", timeUnitType.getDescriptor()); mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "TIME_UNIT_DEFAULT", timeUnitType.getDescriptor()); + + var executorServiceType = Type.getType(ExecutorService.class); mv.visitInsn(Opcodes.ACONST_NULL); mv.visitFieldInsn(Opcodes.PUTSTATIC, CLASSNAME, "EXECUTOR", executorServiceType.getDescriptor()); + mv.visitInsn(Opcodes.RETURN); }); } From 7a110087ffd2209254ff42052e32b027ef82fe90 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 23 Dec 2024 11:10:21 -0800 Subject: [PATCH 09/10] address feedback --- .../src/main/groovy/elasticsearch.ide.gradle | 2 +- .../hadoop-client-api/build.gradle | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/build-tools-internal/src/main/groovy/elasticsearch.ide.gradle b/build-tools-internal/src/main/groovy/elasticsearch.ide.gradle index eb3a529498fa7..60ae4d58f343e 100644 --- a/build-tools-internal/src/main/groovy/elasticsearch.ide.gradle +++ b/build-tools-internal/src/main/groovy/elasticsearch.ide.gradle @@ -166,7 +166,7 @@ if (providers.systemProperty('idea.active').getOrNull() == 'true') { tasks.register('buildDependencyArtifacts') { group = 'ide' description = 'Builds artifacts needed as dependency for IDE modules' - dependsOn([':plugins:repository-hdfs:hadoop-client-api:shadowJar', + dependsOn([':plugins:repository-hdfs:hadoop-client-api:jar', ':x-pack:plugin:esql:compute:ann:jar', ':x-pack:plugin:esql:compute:gen:jar', ':server:generateModulesList', diff --git a/plugins/repository-hdfs/hadoop-client-api/build.gradle b/plugins/repository-hdfs/hadoop-client-api/build.gradle index c20d391e6bedd..c156e44d2f06f 100644 --- a/plugins/repository-hdfs/hadoop-client-api/build.gradle +++ b/plugins/repository-hdfs/hadoop-client-api/build.gradle @@ -21,22 +21,27 @@ dependencies { def outputDir = layout.buildDirectory.dir("patched-classes") -def patchTask = tasks.register("patchClasses", JavaExec) -patchTask.configure { - dependsOn configurations.thejar, sourceSets.patcher.getCompileJavaTaskName() +def patchTask = tasks.register("patchClasses", JavaExec) { + inputs.files(configurations.thejar).withPathSensitivity(PathSensitivity.RELATIVE) + inputs.files(sourceSets.patcher.output).withPathSensitivity(PathSensitivity.RELATIVE) outputs.dir(outputDir) classpath = sourceSets.patcher.runtimeClasspath mainClass = 'org.elasticsearch.hdfs.patch.HdfsClassPatcher' doFirst { - args configurations.thejar.singleFile, outputDir.get().getAsFile().toString() + args(configurations.thejar.singleFile, outputDir.get().asFile) } } tasks.named('jar').configure { dependsOn(patchTask) dependsOn(configurations.thejar) - duplicatesStrategy = DuplicatesStrategy.EXCLUDE - from(outputDir) // patch directory first so any files patched are excluded as duplicates - from(project.zipTree(configurations.thejar.singleFile)) + from(outputDir) + from({ project.zipTree(configurations.thejar.singleFile) }) { + eachFile { + if (outputDir.get().file(it.relativePath.pathString).asFile.exists()) { + it.exclude() + } + } + } } From f6c9f6efd1a56ac1c40bfdceccfde9383d7b03fa Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 23 Dec 2024 11:50:25 -0800 Subject: [PATCH 10/10] more feedback --- plugins/repository-hdfs/hadoop-client-api/build.gradle | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/repository-hdfs/hadoop-client-api/build.gradle b/plugins/repository-hdfs/hadoop-client-api/build.gradle index c156e44d2f06f..5e87b81292501 100644 --- a/plugins/repository-hdfs/hadoop-client-api/build.gradle +++ b/plugins/repository-hdfs/hadoop-client-api/build.gradle @@ -33,10 +33,9 @@ def patchTask = tasks.register("patchClasses", JavaExec) { } tasks.named('jar').configure { - dependsOn(patchTask) dependsOn(configurations.thejar) - from(outputDir) + from(patchTask) from({ project.zipTree(configurations.thejar.singleFile) }) { eachFile { if (outputDir.get().file(it.relativePath.pathString).asFile.exists()) {