From 62562ac013b27b0bdba25384210b146c9af49c97 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 2 Mar 2023 21:18:01 -0500 Subject: [PATCH 01/43] Sketch of saving libraries as JAR files rather than unpacked --- pom.xml | 5 ++ .../cps/global/UserDefinedGlobalVariable.java | 1 + .../plugins/workflow/libs/LibraryAdder.java | 89 +++++++++---------- .../libs/LibraryCachingConfiguration.java | 2 +- .../plugins/workflow/libs/LibraryRecord.java | 2 +- .../workflow/libs/LibraryRetriever.java | 71 ++++++++++++++- .../plugins/workflow/libs/LibraryStep.java | 14 +-- .../plugins/workflow/libs/SCMRetriever.java | 6 +- .../workflow/libs/SCMSourceRetriever.java | 74 ++++----------- .../workflow/cps/global/GrapeTest.java | 3 - .../workflow/libs/LibraryRetrieverTest.java | 74 +++++++++++++++ .../workflow/libs/SCMSourceRetrieverTest.java | 4 - 12 files changed, 220 insertions(+), 125 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java diff --git a/pom.xml b/pom.xml index 0a310c58..d898fd3c 100644 --- a/pom.xml +++ b/pom.xml @@ -75,6 +75,11 @@ import pom + + org.jenkins-ci.plugins.workflow + workflow-cps + 999999-SNAPSHOT + diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java index 7ec37c28..dfa0f8e9 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java @@ -22,6 +22,7 @@ */ // not @Extension because these are instantiated programmatically public class UserDefinedGlobalVariable extends GlobalVariable { + // TODO switch to URL private final File help; private final String name; diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 9ff64777..8f65b261 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -50,6 +50,8 @@ import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.jar.JarFile; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.commons.io.IOUtils; import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution; @@ -97,6 +99,7 @@ if (action != null) { // Resuming a build, so just look up what we loaded before. for (LibraryRecord record : action.getLibraries()) { + // TODO call LibraryRetriever.dir2Jar as needed FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName()); for (String root : new String[] {"src", "vars"}) { FilePath dir = libDir.child(root); @@ -147,9 +150,7 @@ // Now actually try to retrieve the libraries. for (LibraryRecord record : librariesAdded.values()) { listener.getLogger().println("Loading library " + record.name + "@" + record.version); - for (URL u : retrieve(record, retrievers.get(record.name), listener, build, execution)) { - additions.add(new Addition(u, record.trusted)); - } + additions.add(new Addition(retrieve(record, retrievers.get(record.name), listener, build, execution), record.trusted)); } return additions; } @@ -169,14 +170,14 @@ private enum CacheStatus { EXPIRED; } - private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheDir) + private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheJar) throws IOException, InterruptedException { if (cachingConfiguration.isRefreshEnabled()) { final long cachingMilliseconds = cachingConfiguration.getRefreshTimeMilliseconds(); - if(versionCacheDir.exists()) { - if ((versionCacheDir.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) { + if(versionCacheJar.exists()) { + if ((versionCacheJar.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) { return CacheStatus.VALID; } else { return CacheStatus.EXPIRED; @@ -185,7 +186,7 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c return CacheStatus.DOES_NOT_EXIST; } } else { - if (versionCacheDir.exists()) { + if (versionCacheJar.exists()) { return CacheStatus.VALID; } else { return CacheStatus.DOES_NOT_EXIST; @@ -194,16 +195,16 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c } /** Retrieve library files. */ - static List retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run run, @NonNull CpsFlowExecution execution) throws Exception { + static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run run, @NonNull CpsFlowExecution execution) throws Exception { String name = record.name; String version = record.version; boolean changelog = record.changelog; LibraryCachingConfiguration cachingConfiguration = record.cachingConfiguration; - FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName()); + FilePath libJar = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName() + ".jar"); Boolean shouldCache = cachingConfiguration != null; - final FilePath versionCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName()); + final FilePath versionCacheJar = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName() + ".jar"); ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getDirectoryName()); - final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE); + final FilePath lastReadFile = versionCacheJar.sibling(record.getDirectoryName() + "." + LibraryCachingConfiguration.LAST_READ_FILE); if(shouldCache && cachingConfiguration.isExcluded(version)) { listener.getLogger().println("Library " + name + "@" + version + " is excluded from caching."); @@ -213,13 +214,13 @@ static List retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev if(shouldCache) { retrieveLock.readLock().lockInterruptibly(); try { - CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheDir); + CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheJar); if (cacheStatus == CacheStatus.DOES_NOT_EXIST || cacheStatus == CacheStatus.EXPIRED) { retrieveLock.readLock().unlock(); retrieveLock.writeLock().lockInterruptibly(); try { boolean retrieve = false; - switch (getCacheStatus(cachingConfiguration, versionCacheDir)) { + switch (getCacheStatus(cachingConfiguration, versionCacheJar)) { case VALID: listener.getLogger().println("Library " + name + "@" + version + " is cached. Copying from home."); break; @@ -229,9 +230,9 @@ static List retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev case EXPIRED: long cachingMinutes = cachingConfiguration.getRefreshTimeMinutes(); listener.getLogger().println("Library " + name + "@" + version + " is due for a refresh after " + cachingMinutes + " minutes, clearing."); - if (versionCacheDir.exists()) { - versionCacheDir.deleteRecursive(); - versionCacheDir.withSuffix("-name.txt").delete(); + if (versionCacheJar.exists()) { + versionCacheJar.delete(); + versionCacheJar.sibling(record.getDirectoryName() + "-name.txt").delete(); } retrieve = true; break; @@ -239,8 +240,7 @@ static List retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev if (retrieve) { listener.getLogger().println("Caching library " + name + "@" + version); - versionCacheDir.mkdirs(); - retriever.retrieve(name, version, changelog, versionCacheDir, run, listener); + retriever.retrieveJar(name, version, changelog, versionCacheJar, run, listener); } retrieveLock.readLock().lock(); } finally { @@ -251,50 +251,44 @@ static List retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev } lastReadFile.touch(System.currentTimeMillis()); - versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8"); - versionCacheDir.copyRecursiveTo(libDir); + versionCacheJar.sibling(record.getDirectoryName() + "-name.txt").write(name, "UTF-8"); + versionCacheJar.copyTo(libJar); } finally { retrieveLock.readLock().unlock(); } } else { - retriever.retrieve(name, version, changelog, libDir, run, listener); + retriever.retrieveJar(name, version, changelog, libJar, run, listener); } // Write the user-provided name to a file as a debugging aid. - libDir.withSuffix("-name.txt").write(name, "UTF-8"); + libJar.sibling(record.getDirectoryName() + "-name.txt").write(name, "UTF-8"); // Replace any classes requested for replay: if (!record.trusted) { for (String clazz : ReplayAction.replacementsIn(execution)) { - for (String root : new String[] {"src", "vars"}) { - String rel = root + "/" + clazz.replace('.', '/') + ".groovy"; - FilePath f = libDir.child(rel); - if (f.exists()) { - String replacement = ReplayAction.replace(execution, clazz); - if (replacement != null) { - listener.getLogger().println("Replacing contents of " + rel); - f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler - } + String rel = clazz.replace('.', '/') + ".groovy"; + /* TODO need to unpack & repack I guess + FilePath f = libDir.child(rel); + if (f.exists()) { + String replacement = ReplayAction.replace(execution, clazz); + if (replacement != null) { + listener.getLogger().println("Replacing contents of " + rel); + f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler } } + */ } } - List urls = new ArrayList<>(); - FilePath srcDir = libDir.child("src"); - if (srcDir.isDirectory()) { - urls.add(srcDir.toURI().toURL()); - } - FilePath varsDir = libDir.child("vars"); - if (varsDir.isDirectory()) { - urls.add(varsDir.toURI().toURL()); - for (FilePath var : varsDir.list("*.groovy")) { - record.variables.add(var.getBaseName()); - } - } - if (urls.isEmpty()) { - throw new AbortException("Library " + name + " expected to contain at least one of src or vars directories"); + try (JarFile jf = new JarFile(libJar.getRemote())) { + jf.stream().forEach(entry -> { + Matcher m = ROOT_GROOVY_SOURCE.matcher(entry.getName()); + if (m.matches()) { + record.variables.add(m.group(1)); + } + }); } - return urls; + return libJar.toURI().toURL(); } + private static final Pattern ROOT_GROOVY_SOURCE = Pattern.compile("([^/]+)[.]groovy"); /** * Loads resources for {@link ResourceStep}. @@ -309,6 +303,7 @@ static List retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev Run run = (Run) executable; LibrariesAction action = run.getAction(LibrariesAction.class); if (action != null) { + // TODO handle *.jar FilePath libs = new FilePath(run.getRootDir()).child("libs"); for (LibraryRecord library : action.getLibraries()) { FilePath libResources = libs.child(library.getDirectoryName() + "/resources/"); @@ -347,6 +342,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding) List vars = new ArrayList<>(); for (LibraryRecord library : action.getLibraries()) { for (String variable : library.variables) { + // TODO pass URL of *.jar!/$variable.txt vars.add(new UserDefinedGlobalVariable(variable, new File(run.getRootDir(), "libs/" + library.getDirectoryName() + "/vars/" + variable + ".txt"))); } } @@ -367,6 +363,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding) Run run = (Run) executable; LibrariesAction action = run.getAction(LibrariesAction.class); if (action != null) { + // TODO handle *.jar FilePath libs = new FilePath(run.getRootDir()).child("libs"); for (LibraryRecord library : action.getLibraries()) { if (library.trusted) { diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java index afc3dfad..2158c512 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java @@ -31,7 +31,7 @@ public final class LibraryCachingConfiguration extends AbstractDescribableImpl implements ExtensionPoint { + /** + * Obtains library sources. + * @param name the {@link LibraryConfiguration#getName} + * @param version the version of the library, such as from {@link LibraryConfiguration#getDefaultVersion} or an override + * @param changelog whether to include changesets in the library in jobs using it from {@link LibraryConfiguration#getIncludeInChangesets} + * @param target a JAR file in which to stash sources; should contain {@code **}{@code /*.groovy} (sources at top level will be considered vars), and optionally also {@code resources/} + * @param run a build which will use the library + * @param listener a way to report progress + * @throws Exception if there is any problem (use {@link AbortException} for user errors) + */ + public void retrieveJar(@NonNull String name, @NonNull String version, boolean changelog, @NonNull FilePath target, @NonNull Run run, @NonNull TaskListener listener) throws Exception { + if (Util.isOverridden(LibraryRetriever.class, getClass(), "retrieve", String.class, String.class, boolean.class, FilePath.class, Run.class, TaskListener.class)) { + FilePath tmp = target.withSuffix(".checkout"); + try { + retrieve(name, version, changelog, tmp, run, listener); + dir2Jar(tmp, target); + } finally { + tmp.deleteRecursive(); + } + } else { + throw new AbstractMethodError("Implement retrieveJar"); + } + } + + /** + * Translates a historical directory with {@code src/} and/or {@code vars/} and/or {@code resources/} subdirectories + * into a JAR file with Groovy in classpath orientation and {@code resources/} as a ZIP folder. + */ + static void dir2Jar(@NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException { + // TODO do this more efficiently by packing JAR directly + FilePath tmp2 = jar.withSuffix(".tmp2"); + tmp2.mkdirs(); + try { + FilePath src = dir.child("src"); + if (src.isDirectory()) { + src.moveAllChildrenTo(tmp2); + } + FilePath vars = dir.child("vars"); + if (vars.isDirectory()) { + vars.moveAllChildrenTo(tmp2); + } + FilePath resources = dir.child("resources"); + if (resources.isDirectory()) { + resources.renameTo(tmp2.child("resources")); + } + try (OutputStream os = jar.write()) { + tmp2.archive(ArchiverFactory.ZIP, os, "**"); + } + } finally { + tmp2.deleteRecursive(); + } + } + /** * Obtains library sources. * @param name the {@link LibraryConfiguration#getName} @@ -51,7 +107,14 @@ public abstract class LibraryRetriever extends AbstractDescribableImpl run, @NonNull TaskListener listener) throws Exception; + @Deprecated + public void retrieve(@NonNull String name, @NonNull String version, boolean changelog, @NonNull FilePath target, @NonNull Run run, @NonNull TaskListener listener) throws Exception { + if (Util.isOverridden(LibraryRetriever.class, getClass(), "retrieve", String.class, String.class, FilePath.class, Run.class, TaskListener.class)) { + retrieve(name, version, target, run, listener); + } else { + throw new AbstractMethodError("Implement retrieveJar"); + } + } /** * Obtains library sources. @@ -62,8 +125,10 @@ public abstract class LibraryRetriever extends AbstractDescribableImpl run, @NonNull TaskListener listener) throws Exception; + @Deprecated + public void retrieve(@NonNull String name, @NonNull String version, @NonNull FilePath target, @NonNull Run run, @NonNull TaskListener listener) throws Exception { + throw new AbstractMethodError("Implement retrieveJar"); + } @Deprecated public FormValidation validateVersion(@NonNull String name, @NonNull String version) { diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java index 69f709aa..6283e6c3 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java @@ -229,9 +229,7 @@ else if (retriever instanceof SCMRetriever) { listener.getLogger().println("Loading library " + record.name + "@" + record.version); CpsFlowExecution exec = (CpsFlowExecution) getContext().get(FlowExecution.class); GroovyClassLoader loader = (trusted ? exec.getTrustedShell() : exec.getShell()).getClassLoader(); - for (URL u : LibraryAdder.retrieve(record, retriever, listener, run, (CpsFlowExecution) getContext().get(FlowExecution.class))) { - loader.addURL(u); - } + loader.addURL(LibraryAdder.retrieve(record, retriever, listener, run, (CpsFlowExecution) getContext().get(FlowExecution.class))); run.save(); // persist changes to LibrariesAction.libraries*.variables return new LoadedClasses(name, record.getDirectoryName(), trusted, changelog, run); } @@ -257,7 +255,7 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S private final @NonNull String srcUrl; LoadedClasses(String library, String libraryDirectoryName, boolean trusted, Boolean changelog, Run run) { - this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + "/src").toURI().toString()); + this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").toURI().toString()); } LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String srcUrl) { @@ -358,7 +356,9 @@ private Class loadClass(String name) { if (codeSource == null) { throw new IllegalAccessException(name + " had no defined code source"); } - String actual = canonicalize(codeSource.getLocation().toString()); + String loc = codeSource.getLocation().toString(); + LOGGER.info(() -> "TODO got " + loc); + String actual = canonicalize(loc); String srcUrlC = canonicalize(srcUrl); // do not do this in constructor: path might not actually exist if (!actual.startsWith(srcUrlC)) { throw new IllegalAccessException(name + " was defined in " + actual + " which was not inside " + srcUrlC); @@ -375,6 +375,10 @@ private Class loadClass(String name) { } private static String canonicalize(String uri) { + if (uri.startsWith("jar:") && uri.contains("!/")) { + // See warning in CpsGroovyShell.parseClass. + uri = uri.substring(4, uri.indexOf("!/")); + } if (uri.startsWith("file:/")) { try { return Paths.get(new URI(uri)).toRealPath().toUri().toString(); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java index da01ef44..071264bf 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java @@ -88,14 +88,10 @@ public void setLibraryPath(String libraryPath) { this.libraryPath = libraryPath; } - @Override public void retrieve(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { + @Override public void retrieveJar(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { SCMSourceRetriever.doRetrieve(name, changelog, scm, libraryPath, target, run, listener); } - @Override public void retrieve(String name, String version, FilePath target, Run run, TaskListener listener) throws Exception { - SCMSourceRetriever.doRetrieve(name, true, scm, libraryPath, target, run, listener); - } - @Override public FormValidation validateVersion(String name, String version, Item context) { if (!Items.XSTREAM2.toXML(scm).contains("${library." + name + ".version}")) { return FormValidation.warningWithMarkup("When using " + getDescriptor().getDisplayName() + ", you will need to include ${library." + Util.escape(name) + ".version} in the SCM configuration somewhere."); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index 8b42c8f0..a77f0b22 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -142,7 +142,7 @@ public String getLibraryPath() { this.libraryPath = libraryPath; } - @Override public void retrieve(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { + @Override public void retrieveJar(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { SCMRevision revision = retrySCMOperation(listener, () -> scm.fetch(version, listener, run.getParent())); if (revision == null) { throw new AbortException("No version " + version + " found for library " + name); @@ -157,10 +157,6 @@ public String getLibraryPath() { } } - @Override public void retrieve(String name, String version, FilePath target, Run run, TaskListener listener) throws Exception { - retrieve(name, version, true, target, run, listener); - } - private static T retrySCMOperation(TaskListener listener, Callable task) throws Exception{ T ret = null; for (int retryCount = Jenkins.get().getScmCheckoutRetryCount(); retryCount >= 0; retryCount--) { @@ -232,7 +228,7 @@ static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String } // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? // Copy sources with relevant files from the checkout: - lease.path.child(libraryPath).copyRecursiveTo("src/**/*.groovy,vars/*.groovy,vars/*.txt,resources/", excludes, target); + LibraryRetriever.dir2Jar(lease.path.child(libraryPath), target); } } @@ -245,65 +241,29 @@ private static String getFilePathSuffix() { * Similar to {@link #doRetrieve} but used in {@link #clone} mode. */ private static void doClone(@NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener) throws Exception { + // TODO merge into doRetrieve SCMStep delegate = new GenericSCMStep(scm); delegate.setPoll(false); delegate.setChangelog(false); - if (libraryPath == null) { - retrySCMOperation(listener, () -> { - delegate.checkout(run, target, listener, Jenkins.get().createLauncher(listener)); - return null; - }); - } else { - if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { - throw new AbortException("Library path may not contain '..'"); - } - FilePath root = target.child("root"); + FilePath tmp = target.withSuffix(".tmp"); + try { retrySCMOperation(listener, () -> { - delegate.checkout(run, root, listener, Jenkins.get().createLauncher(listener)); + delegate.checkout(run, tmp, listener, Jenkins.get().createLauncher(listener)); return null; }); - FilePath subdir = root.child(libraryPath); - if (!subdir.isDirectory()) { - throw new AbortException("Did not find " + libraryPath + " in checkout"); - } - for (String content : List.of("src", "vars", "resources")) { - FilePath contentDir = subdir.child(content); - if (contentDir.isDirectory()) { - listener.getLogger().println("Moving " + content + " to top level"); - contentDir.renameTo(target.child(content)); + FilePath root; + if (libraryPath == null) { + root = tmp; + } else { + if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { + throw new AbortException("Library path may not contain '..'"); } + root = tmp.child(libraryPath); } - // root itself will be deleted below - } - Set deleted = new TreeSet<>(); - if (!INCLUDE_SRC_TEST_IN_LIBRARIES) { - FilePath srcTest = target.child("src/test"); - if (srcTest.isDirectory()) { - listener.getLogger().println("Excluding src/test/ from checkout of " + scm.getKey() + " so that library test code cannot be accessed by Pipelines."); - listener.getLogger().println("To remove this log message, move the test code outside of src/. To restore the previous behavior that allowed access to files in src/test/, pass -D" + SCMSourceRetriever.class.getName() + ".INCLUDE_SRC_TEST_IN_LIBRARIES=true to the java command used to start Jenkins."); - srcTest.deleteRecursive(); - deleted.add("src/test"); - } - } - for (FilePath child : target.list()) { - String name = child.getName(); - switch (name) { - case "src": - // TODO delete everything that is not *.groovy - break; - case "vars": - // TODO delete everything that is not *.groovy or *.txt, incl. subdirs - break; - case "resources": - // OK, leave it all - break; - default: - deleted.add(name); - child.deleteRecursive(); - } - } - if (!deleted.isEmpty()) { - listener.getLogger().println("Deleted " + deleted.stream().collect(Collectors.joining(", "))); + // TODO handle INCLUDE_SRC_TEST_IN_LIBRARIES + LibraryRetriever.dir2Jar(root, target); + } finally { + tmp.deleteRecursive(); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/cps/global/GrapeTest.java b/src/test/java/org/jenkinsci/plugins/workflow/cps/global/GrapeTest.java index f84d8422..5fd678ef 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/cps/global/GrapeTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/cps/global/GrapeTest.java @@ -197,9 +197,6 @@ private static final class LocalRetriever extends LibraryRetriever { @Override public void retrieve(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { new FilePath(lib).copyRecursiveTo(target); } - @Override public void retrieve(String name, String version, FilePath target, Run run, TaskListener listener) throws Exception { - retrieve(name, version, false, target, run, listener); - } } @Test public void outsideLibrary() throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java new file mode 100644 index 00000000..10560d1e --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -0,0 +1,74 @@ +/* + * The MIT License + * + * Copyright 2023 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.workflow.libs; + +import hudson.FilePath; +import java.util.Set; +import java.util.TreeSet; +import java.util.jar.JarFile; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class LibraryRetrieverTest { + + @Rule public TemporaryFolder tmp = new TemporaryFolder(); + + @Test public void justVars() throws Exception { + assertDir2Jar(Set.of("vars/xxx.groovy", "vars/yyy.groovy"), Set.of("xxx.groovy", "yyy.groovy")); + } + + @Test public void justSrc() throws Exception { + assertDir2Jar(Set.of("src/p1/xxx.groovy", "src/p2/yyy.groovy"), Set.of("p1/xxx.groovy", "p2/yyy.groovy")); + } + + @Test public void theWorks() throws Exception { + assertDir2Jar(Set.of("src/p1/xxx.groovy", "vars/yyy.groovy", "resources/a.txt", "resources/b/c.txt"), Set.of("p1/xxx.groovy", "yyy.groovy", "resources/a.txt", "resources/b/c.txt")); + } + + // TODO assert that other files are not copied + + private void assertDir2Jar(Set inputs, Set outputs) throws Exception { + FilePath dir = new FilePath(tmp.newFolder()); + for (String input : inputs) { + dir.child(input).write("xxx", null); + } + FilePath jar = new FilePath(tmp.newFile("x.jar")); + LibraryRetriever.dir2Jar(dir, jar); + Set actualOutputs = new TreeSet<>(); + try (JarFile jf = new JarFile(jar.getRemote())) { + jf.stream().forEach(e -> { + String name = e.getName(); + if (!name.endsWith("/")) { + actualOutputs.add(name); + } + }); + } + assertThat(actualOutputs, is(outputs)); + } + +} diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index 91806f00..63004431 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -375,7 +375,6 @@ public static class BasicSCMSource extends SCMSource { sampleRepo.init(); sampleRepo.write("vars/myecho.groovy", "def call() {echo 'something special'}"); sampleRepo.write("README.md", "Summary"); - sampleRepo.git("rm", "file"); sampleRepo.git("add", "."); sampleRepo.git("commit", "--message=init"); GitSCMSource src = new GitSCMSource(sampleRepo.toString()); @@ -390,7 +389,6 @@ public static class BasicSCMSource extends SCMSource { WorkflowRun b = r.buildAndAssertSuccess(p); assertFalse(r.jenkins.getWorkspaceFor(p).withSuffix("@libs").isDirectory()); r.assertLogContains("something special", b); - r.assertLogContains("Deleted .git, README.md", b); r.assertLogContains("Using shallow clone with depth 1", b); } @@ -409,8 +407,6 @@ public static class BasicSCMSource extends SCMSource { p.setDefinition(new CpsFlowDefinition("@Library('root_sub_path@master') import myecho; myecho()", true)); WorkflowRun b = r.buildAndAssertSuccess(p); r.assertLogContains("something special", b); - r.assertLogContains("Moving vars to top level", b); - r.assertLogContains("Deleted root", b); } @Test public void cloneModeLibraryPathSecurity() throws Exception { From c02ed474c1217a15b5cf1a87ad3335c8341ed48f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 06:49:25 -0500 Subject: [PATCH 02/43] Incremental deployment of https://github.com/jenkinsci/workflow-cps-plugin/pull/668 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d898fd3c..35914100 100644 --- a/pom.xml +++ b/pom.xml @@ -78,7 +78,7 @@ org.jenkins-ci.plugins.workflow workflow-cps - 999999-SNAPSHOT + 3625.v4390b_f51b_50f From e5e95d0d2f92f5091f1646915f82a88e7e2a5590 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 12:06:01 -0500 Subject: [PATCH 03/43] Avoiding an extraneous empty dir --- .../plugins/workflow/libs/SCMSourceRetriever.java | 1 + .../plugins/workflow/libs/LibraryRetrieverTest.java | 7 +++++-- .../plugins/workflow/libs/SCMSourceRetrieverTest.java | 3 +++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index a77f0b22..1fd2dd7d 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -264,6 +264,7 @@ private static void doClone(@NonNull SCM scm, String libraryPath, FilePath targe LibraryRetriever.dir2Jar(root, target); } finally { tmp.deleteRecursive(); + WorkspaceList.tempDir(tmp).deleteRecursive(); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index 10560d1e..8074011b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -29,6 +29,7 @@ import java.util.TreeSet; import java.util.jar.JarFile; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; import org.junit.Rule; import org.junit.Test; @@ -53,11 +54,12 @@ public class LibraryRetrieverTest { // TODO assert that other files are not copied private void assertDir2Jar(Set inputs, Set outputs) throws Exception { - FilePath dir = new FilePath(tmp.newFolder()); + FilePath work = new FilePath(tmp.newFolder()); + FilePath dir = work.child("dir"); for (String input : inputs) { dir.child(input).write("xxx", null); } - FilePath jar = new FilePath(tmp.newFile("x.jar")); + FilePath jar = work.child("x.jar"); LibraryRetriever.dir2Jar(dir, jar); Set actualOutputs = new TreeSet<>(); try (JarFile jf = new JarFile(jar.getRemote())) { @@ -69,6 +71,7 @@ private void assertDir2Jar(Set inputs, Set outputs) throws Excep }); } assertThat(actualOutputs, is(outputs)); + assertThat(work.list(), containsInAnyOrder(dir, jar)); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index 63004431..31ac3311 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -79,6 +79,7 @@ import org.jvnet.hudson.test.WithoutJenkins; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.nullValue; @@ -390,6 +391,8 @@ public static class BasicSCMSource extends SCMSource { assertFalse(r.jenkins.getWorkspaceFor(p).withSuffix("@libs").isDirectory()); r.assertLogContains("something special", b); r.assertLogContains("Using shallow clone with depth 1", b); + // Fails to reproduce presence of *.jar.tmp@tmp; probably specific to use of GIT_ASKPASS: + assertThat(new File(b.getRootDir(), "libs").list(), arrayContainingInAnyOrder(matchesPattern("[0-9a-f]{64}[.]jar"), matchesPattern("[0-9a-f]{64}-name[.]txt"))); } @Test public void cloneModeLibraryPath() throws Exception { From 8598f5e4e032a64c48ae41df12b19936dee7871d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 12:08:19 -0500 Subject: [PATCH 04/43] Nicer name for temp checkout dir --- .../org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index 1fd2dd7d..543e7eff 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -245,7 +245,7 @@ private static void doClone(@NonNull SCM scm, String libraryPath, FilePath targe SCMStep delegate = new GenericSCMStep(scm); delegate.setPoll(false); delegate.setChangelog(false); - FilePath tmp = target.withSuffix(".tmp"); + FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); try { retrySCMOperation(listener, () -> { delegate.checkout(run, tmp, listener, Jenkins.get().createLauncher(listener)); From 10155eab5be1e7f5193c2e3712bd0a02c2a8c5a8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 12:46:18 -0500 Subject: [PATCH 05/43] Simpler handling of `LoadedClasses.srcUrl`; no apparent need for `canonicalize` any more --- .../plugins/workflow/libs/LibraryStep.java | 25 +++---------------- .../workflow/libs/LibraryStepTest.java | 2 ++ 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java index 6283e6c3..3f2662e3 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java @@ -251,11 +251,11 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S private final @NonNull String prefix; /** {@link Class#getName} minus package prefix */ private final @CheckForNull String clazz; - /** {@code file:/…/libs/NAME/src/} */ + /** {@code jar:file:/…/libs/$hash.jar!/} */ private final @NonNull String srcUrl; LoadedClasses(String library, String libraryDirectoryName, boolean trusted, Boolean changelog, Run run) { - this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").toURI().toString()); + this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ "jar:" + new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").toURI() + "!/"); } LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String srcUrl) { @@ -357,11 +357,8 @@ private Class loadClass(String name) { throw new IllegalAccessException(name + " had no defined code source"); } String loc = codeSource.getLocation().toString(); - LOGGER.info(() -> "TODO got " + loc); - String actual = canonicalize(loc); - String srcUrlC = canonicalize(srcUrl); // do not do this in constructor: path might not actually exist - if (!actual.startsWith(srcUrlC)) { - throw new IllegalAccessException(name + " was defined in " + actual + " which was not inside " + srcUrlC); + if (!loc.startsWith(srcUrl)) { + throw new IllegalAccessException(name + " was defined in " + loc + " which was not inside " + srcUrl); } if (!Modifier.isPublic(c.getModifiers())) { // unlikely since Groovy makes classes implicitly public throw new IllegalAccessException(c + " is not public"); @@ -374,20 +371,6 @@ private Class loadClass(String name) { } } - private static String canonicalize(String uri) { - if (uri.startsWith("jar:") && uri.contains("!/")) { - // See warning in CpsGroovyShell.parseClass. - uri = uri.substring(4, uri.indexOf("!/")); - } - if (uri.startsWith("file:/")) { - try { - return Paths.get(new URI(uri)).toRealPath().toUri().toString(); - } catch (IOException | URISyntaxException x) { - LOGGER.log(Level.WARNING, "could not canonicalize " + uri, x); - } - } - return uri; - } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java index 67f757c9..4eaa04f4 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java @@ -129,6 +129,8 @@ public class LibraryStepTest { r.assertLogContains("using constant vs. constant", b); } + // TODO test LoadedClasses.srcUrl after reload build started prior to dir2Jar (@OldData) + @Test public void missingProperty() throws Exception { sampleRepo.init(); sampleRepo.write("src/some/pkg/MyClass.groovy", "package some.pkg; class MyClass { }"); From 0a2ed44ffea64a61a3d9f50e628d24c9c8c87b60 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 12:55:01 -0500 Subject: [PATCH 06/43] e5e95d0d2f92f5091f1646915f82a88e7e2a5590 & 8598f5e4e032a64c48ae41df12b19936dee7871d applied also to `LibraryRetriever` compat code --- .../org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index dec92b9f..81246317 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -35,6 +35,7 @@ import hudson.util.FormValidation; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.slaves.WorkspaceList; import hudson.util.io.ArchiverFactory; import java.io.IOException; import java.io.OutputStream; @@ -56,12 +57,13 @@ public abstract class LibraryRetriever extends AbstractDescribableImpl run, @NonNull TaskListener listener) throws Exception { if (Util.isOverridden(LibraryRetriever.class, getClass(), "retrieve", String.class, String.class, boolean.class, FilePath.class, Run.class, TaskListener.class)) { - FilePath tmp = target.withSuffix(".checkout"); + FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); try { retrieve(name, version, changelog, tmp, run, listener); dir2Jar(tmp, target); } finally { tmp.deleteRecursive(); + WorkspaceList.tempDir(tmp).deleteRecursive(); } } else { throw new AbstractMethodError("Implement retrieveJar"); From fec1f4e082552358c3bf0cb0a09d19df0b2d8f55 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 12:59:39 -0500 Subject: [PATCH 07/43] Support for resuming builds --- .../jenkinsci/plugins/workflow/libs/LibraryAdder.java | 10 +++------- .../plugins/workflow/libs/LibraryAdderTest.java | 2 ++ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 8f65b261..ea20b9a2 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -99,13 +99,9 @@ if (action != null) { // Resuming a build, so just look up what we loaded before. for (LibraryRecord record : action.getLibraries()) { - // TODO call LibraryRetriever.dir2Jar as needed - FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName()); - for (String root : new String[] {"src", "vars"}) { - FilePath dir = libDir.child(root); - if (dir.isDirectory()) { - additions.add(new Addition(dir.toURI().toURL(), record.trusted)); - } + FilePath libJar = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName() + ".jar"); + if (libJar.exists()) { + additions.add(new Addition(libJar.toURI().toURL(), record.trusted)); } String unparsed = librariesUnparsed.get(record.name); if (unparsed != null) { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java index 2cd590b8..0e11b6f2 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java @@ -516,6 +516,8 @@ public void parallelBuildsDontInterfereWithExpiredCache() throws Throwable { // r.assertLogContains("Library library@master is cached. Copying from home.", f2.get()); } + // TODO test migration in LibraryAdder.add after reload build started prior to dir2Jar (@OldData) + @Issue("JENKINS-68544") @WithoutJenkins @Test public void className() { From 3cd31022f0203874c4a509c3720f222c73a5f10d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 13:18:03 -0500 Subject: [PATCH 08/43] Fixing `ResourceStep` --- .../plugins/workflow/libs/LibraryAdder.java | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index ea20b9a2..549380b9 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -24,7 +24,6 @@ package org.jenkinsci.plugins.workflow.libs; -import hudson.AbortException; import hudson.Extension; import hudson.ExtensionList; import hudson.FilePath; @@ -50,6 +49,7 @@ import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -299,15 +299,20 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret Run run = (Run) executable; LibrariesAction action = run.getAction(LibrariesAction.class); if (action != null) { - // TODO handle *.jar FilePath libs = new FilePath(run.getRootDir()).child("libs"); for (LibraryRecord library : action.getLibraries()) { - FilePath libResources = libs.child(library.getDirectoryName() + "/resources/"); - FilePath f = libResources.child(name); - if (!new File(f.getRemote()).getCanonicalFile().toPath().startsWith(new File(libResources.getRemote()).getCanonicalPath())) { - throw new AbortException(name + " references a file that is not contained within the library: " + library.name); - } else if (f.exists()) { - resources.put(library.name, readResource(f, encoding)); + FilePath libJar = libs.child(library.getDirectoryName() + ".jar"); + try (JarFile jf = new JarFile(libJar.getRemote())) { + JarEntry je = jf.getJarEntry("resources/" + name); + if (je != null) { + try (InputStream in = jf.getInputStream(je)) { + if ("Base64".equals(encoding)) { + resources.put(library.name, Base64.getEncoder().encodeToString(IOUtils.toByteArray(in))); + } else { + resources.put(library.name, IOUtils.toString(in, encoding)); // The platform default is used if encoding is null. + } + } + } } } } @@ -315,16 +320,6 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret return resources; } - private static String readResource(FilePath file, @CheckForNull String encoding) throws IOException, InterruptedException { - try (InputStream in = file.read()) { - if ("Base64".equals(encoding)) { - return Base64.getEncoder().encodeToString(IOUtils.toByteArray(in)); - } else { - return IOUtils.toString(in, encoding); // The platform default is used if encoding is null. - } - } - } - @Extension public static class GlobalVars extends GlobalVariableSet { @Override public Collection forRun(Run run) { From e2b627614e8e9ac041ba1d7c84bb6644da2d1c94 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 3 Mar 2023 13:18:46 -0500 Subject: [PATCH 09/43] Clearer name for `GLOBAL_LIBRARIES_DIR` --- .../plugins/workflow/libs/LibraryCachingConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java index 2158c512..62182c9f 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java @@ -31,7 +31,7 @@ public final class LibraryCachingConfiguration extends AbstractDescribableImpl Date: Fri, 3 Mar 2023 14:00:18 -0500 Subject: [PATCH 10/43] Switching `*-name.txt` to a manifest attribute; and fixes related to cache maintenance --- .../plugins/workflow/libs/LibraryAdder.java | 4 --- .../workflow/libs/LibraryCachingCleanup.java | 32 ++++++------------ .../libs/LibraryCachingConfiguration.java | 18 ++++------ .../workflow/libs/LibraryRetriever.java | 33 ++++++++++++++----- .../workflow/libs/SCMSourceRetriever.java | 8 ++--- .../libs/LibraryCachingCleanupTest.java | 23 +++---------- .../libs/LibraryCachingConfigurationTest.java | 2 -- .../workflow/libs/LibraryRetrieverTest.java | 5 +-- .../workflow/libs/SCMSourceRetrieverTest.java | 4 +-- 9 files changed, 54 insertions(+), 75 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 549380b9..6318471b 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -228,7 +228,6 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret listener.getLogger().println("Library " + name + "@" + version + " is due for a refresh after " + cachingMinutes + " minutes, clearing."); if (versionCacheJar.exists()) { versionCacheJar.delete(); - versionCacheJar.sibling(record.getDirectoryName() + "-name.txt").delete(); } retrieve = true; break; @@ -247,7 +246,6 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret } lastReadFile.touch(System.currentTimeMillis()); - versionCacheJar.sibling(record.getDirectoryName() + "-name.txt").write(name, "UTF-8"); versionCacheJar.copyTo(libJar); } finally { retrieveLock.readLock().unlock(); @@ -255,8 +253,6 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret } else { retriever.retrieveJar(name, version, changelog, libJar, run, listener); } - // Write the user-provided name to a file as a debugging aid. - libJar.sibling(record.getDirectoryName() + "-name.txt").write(name, "UTF-8"); // Replace any classes requested for replay: if (!record.trusted) { diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java index 591dcc0a..71eb1ed0 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java @@ -9,6 +9,7 @@ import java.io.IOException; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantReadWriteLock; +import jenkins.model.Jenkins; import jenkins.util.SystemProperties; @@ -27,41 +28,28 @@ public LibraryCachingCleanup() { @Override protected void execute(TaskListener listener) throws IOException, InterruptedException { FilePath globalCacheDir = LibraryCachingConfiguration.getGlobalLibrariesCacheDir(); - for (FilePath library : globalCacheDir.list()) { - if (!removeIfExpiredCacheDirectory(library)) { - // Prior to the SECURITY-2586 fix, library caches had a two-level directory structure. - // These caches will never be used again, so we delete any that we find. - for (FilePath version: library.list()) { - if (version.child(LibraryCachingConfiguration.LAST_READ_FILE).exists()) { - library.deleteRecursive(); - break; - } - } - } + for (FilePath libJar : globalCacheDir.list()) { + removeIfExpiredCacheJar(libJar); } + // Old cache directory; format has changed, so just delete it: + Jenkins.get().getRootPath().child("global-libraries-cache").deleteRecursive(); } /** - * Delete the specified cache directory if it is outdated. - * @return true if specified directory is a cache directory, regardless of whether it was outdated. Used to detect - * whether the cache was created before or after the fix for SECURITY-2586. + * Delete the specified cache JAR if it is outdated. */ - private boolean removeIfExpiredCacheDirectory(FilePath library) throws IOException, InterruptedException { - final FilePath lastReadFile = new FilePath(library, LibraryCachingConfiguration.LAST_READ_FILE); + private void removeIfExpiredCacheJar(FilePath libJar) throws IOException, InterruptedException { + final FilePath lastReadFile = libJar.sibling(libJar.getBaseName() + "." + LibraryCachingConfiguration.LAST_READ_FILE); if (lastReadFile.exists()) { - ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(library.getName()); + ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libJar.getBaseName()); retrieveLock.writeLock().lockInterruptibly(); try { if (System.currentTimeMillis() - lastReadFile.lastModified() > TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS)) { - - library.deleteRecursive(); - library.withSuffix("-name.txt").delete(); + libJar.delete(); } } finally { retrieveLock.writeLock().unlock(); } - return true; } - return false; } } diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java index 62182c9f..8cf77fa8 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java @@ -10,17 +10,15 @@ import org.kohsuke.stapler.QueryParameter; import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.jar.JarFile; import java.util.logging.Level; import java.util.logging.Logger; -import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; public final class LibraryCachingConfiguration extends AbstractDescribableImpl { @@ -94,26 +92,22 @@ public FormValidation doClearCache(@QueryParameter String name, @QueryParameter try { if (LibraryCachingConfiguration.getGlobalLibrariesCacheDir().exists()) { - for (FilePath libraryNamePath : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().list("*-name.txt")) { + for (FilePath libraryCachePath : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().list("*.jar")) { // Libraries configured in distinct locations may have the same name. Since only admins are allowed here, this is not a huge issue, but it is probably unexpected. String cacheName; - try (InputStream stream = libraryNamePath.read()) { - cacheName = IOUtils.toString(stream, StandardCharsets.UTF_8); + try (JarFile jf = new JarFile(libraryCachePath.getRemote())) { + cacheName = jf.getManifest().getMainAttributes().getValue(LibraryRetriever.ATTR_LIBRARY_NAME); } if (cacheName.equals(name)) { - FilePath libraryCachePath = LibraryCachingConfiguration.getGlobalLibrariesCacheDir() - .child(libraryNamePath.getName().replace("-name.txt", "")); if (forceDelete) { LOGGER.log(Level.FINER, "Force deleting cache for {0}", name); - libraryCachePath.deleteRecursive(); - libraryNamePath.delete(); + libraryCachePath.delete(); } else { LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name); ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libraryCachePath.getName()); if (retrieveLock.writeLock().tryLock(10, TimeUnit.SECONDS)) { try { - libraryCachePath.deleteRecursive(); - libraryNamePath.delete(); + libraryCachePath.delete(); } finally { retrieveLock.writeLock().unlock(); } diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index 81246317..9e52b418 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -39,12 +39,20 @@ import hudson.util.io.ArchiverFactory; import java.io.IOException; import java.io.OutputStream; +import java.util.jar.Attributes; +import java.util.jar.JarFile; +import java.util.jar.Manifest; /** * A way in which a library can be physically obtained for use in a build. */ public abstract class LibraryRetriever extends AbstractDescribableImpl implements ExtensionPoint { + /** + * JAR manifest attribute giving original library name. + */ + static final String ATTR_LIBRARY_NAME = "Jenkins-Library-Name"; + /** * Obtains library sources. * @param name the {@link LibraryConfiguration#getName} @@ -60,7 +68,7 @@ public void retrieveJar(@NonNull String name, @NonNull String version, boolean c FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); try { retrieve(name, version, changelog, tmp, run, listener); - dir2Jar(tmp, target); + dir2Jar(name, tmp, target); } finally { tmp.deleteRecursive(); WorkspaceList.tempDir(tmp).deleteRecursive(); @@ -74,28 +82,35 @@ public void retrieveJar(@NonNull String name, @NonNull String version, boolean c * Translates a historical directory with {@code src/} and/or {@code vars/} and/or {@code resources/} subdirectories * into a JAR file with Groovy in classpath orientation and {@code resources/} as a ZIP folder. */ - static void dir2Jar(@NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException { + static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException { // TODO do this more efficiently by packing JAR directly - FilePath tmp2 = jar.withSuffix(".tmp2"); - tmp2.mkdirs(); + FilePath tmp = jar.sibling(jar.getBaseName() + "-repack"); + tmp.mkdirs(); try { FilePath src = dir.child("src"); if (src.isDirectory()) { - src.moveAllChildrenTo(tmp2); + src.moveAllChildrenTo(tmp); } FilePath vars = dir.child("vars"); if (vars.isDirectory()) { - vars.moveAllChildrenTo(tmp2); + vars.moveAllChildrenTo(tmp); } FilePath resources = dir.child("resources"); if (resources.isDirectory()) { - resources.renameTo(tmp2.child("resources")); + resources.renameTo(tmp.child("resources")); + } + try (OutputStream os = tmp.child(JarFile.MANIFEST_NAME).write()) { + Manifest m = new Manifest(); + m.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + // Informational debugging aid, since the hex JAR basename will be meaningless: + m.getMainAttributes().putValue(ATTR_LIBRARY_NAME, name); + m.write(os); } try (OutputStream os = jar.write()) { - tmp2.archive(ArchiverFactory.ZIP, os, "**"); + tmp.archive(ArchiverFactory.ZIP, os, "**"); } } finally { - tmp2.deleteRecursive(); + tmp.deleteRecursive(); } } diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index 543e7eff..764c648f 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -151,7 +151,7 @@ public String getLibraryPath() { if (changelog) { listener.getLogger().println("WARNING: ignoring request to compute changelog in clone mode"); } - doClone(scm.build(revision.getHead(), revision), libraryPath, target, run, listener); + doClone(name, scm.build(revision.getHead(), revision), libraryPath, target, run, listener); } else { doRetrieve(name, changelog, scm.build(revision.getHead(), revision), libraryPath, target, run, listener); } @@ -228,7 +228,7 @@ static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String } // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? // Copy sources with relevant files from the checkout: - LibraryRetriever.dir2Jar(lease.path.child(libraryPath), target); + LibraryRetriever.dir2Jar(name, lease.path.child(libraryPath), target); } } @@ -240,7 +240,7 @@ private static String getFilePathSuffix() { /** * Similar to {@link #doRetrieve} but used in {@link #clone} mode. */ - private static void doClone(@NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener) throws Exception { + private static void doClone(@NonNull String name, @NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener) throws Exception { // TODO merge into doRetrieve SCMStep delegate = new GenericSCMStep(scm); delegate.setPoll(false); @@ -261,7 +261,7 @@ private static void doClone(@NonNull SCM scm, String libraryPath, FilePath targe root = tmp.child(libraryPath); } // TODO handle INCLUDE_SRC_TEST_IN_LIBRARIES - LibraryRetriever.dir2Jar(root, target); + LibraryRetriever.dir2Jar(name, root, target); } finally { tmp.deleteRecursive(); WorkspaceList.tempDir(tmp).deleteRecursive(); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java index ff30c86e..4ca0bf18 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java @@ -39,7 +39,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.not; -import static org.hamcrest.io.FileMatchers.anExistingDirectory; import static org.hamcrest.io.FileMatchers.anExistingFile; public class LibraryCachingCleanupTest { @@ -67,28 +66,16 @@ public void smokes() throws Throwable { WorkflowRun b = r.buildAndAssertSuccess(p); LibrariesAction action = b.getAction(LibrariesAction.class); LibraryRecord record = action.getLibraries().get(0); - FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName()); - assertThat(new File(cache.getRemote()), anExistingDirectory()); + FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName() + ".jar"); + assertThat(new File(cache.getRemote()), anExistingFile()); // Run LibraryCachingCleanup and show that cache is not deleted. ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr()); - assertThat(new File(cache.getRemote()), anExistingDirectory()); - assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile()); + assertThat(new File(cache.getRemote()), anExistingFile()); // Run LibraryCachingCleanup after modifying LAST_READ_FILE to be an old date and and show that cache is deleted. long oldMillis = ZonedDateTime.now().minusDays(LibraryCachingCleanup.EXPIRE_AFTER_READ_DAYS + 1).toInstant().toEpochMilli(); - cache.child(LibraryCachingConfiguration.LAST_READ_FILE).touch(oldMillis); + cache.sibling(cache.getBaseName() + "." + LibraryCachingConfiguration.LAST_READ_FILE).touch(oldMillis); ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr()); - assertThat(new File(cache.getRemote()), not(anExistingDirectory())); - assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingDirectory())); - } - - @Test - public void preSecurity2586() throws Throwable { - FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child("name").child("version"); - cache.mkdirs(); - cache.child(LibraryCachingConfiguration.LAST_READ_FILE).touch(System.currentTimeMillis()); - ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr()); - assertThat(new File(cache.getRemote()), not(anExistingDirectory())); - assertThat(new File(cache.getParent().getRemote()), not(anExistingDirectory())); + assertThat(new File(cache.getRemote()), not(anExistingFile())); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java index b2a0a55f..898806ab 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java @@ -175,11 +175,9 @@ public void clearCache() throws Exception { LibraryRecord record = action.getLibraries().get(0); FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName()); assertThat(new File(cache.getRemote()), anExistingDirectory()); - assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile()); // Clear the cache. TODO: Would be more realistic to set up security and use WebClient. ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", false); assertThat(new File(cache.getRemote()), not(anExistingDirectory())); - assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile())); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index 8074011b..0c176233 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -60,12 +60,13 @@ private void assertDir2Jar(Set inputs, Set outputs) throws Excep dir.child(input).write("xxx", null); } FilePath jar = work.child("x.jar"); - LibraryRetriever.dir2Jar(dir, jar); + LibraryRetriever.dir2Jar("mylib", dir, jar); Set actualOutputs = new TreeSet<>(); try (JarFile jf = new JarFile(jar.getRemote())) { + assertThat(jf.getManifest().getMainAttributes().getValue(LibraryRetriever.ATTR_LIBRARY_NAME), is("mylib")); jf.stream().forEach(e -> { String name = e.getName(); - if (!name.endsWith("/")) { + if (!name.endsWith("/") && !name.startsWith("META-INF/")) { actualOutputs.add(name); } }); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index 31ac3311..2cd4cfbd 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -79,7 +79,7 @@ import org.jvnet.hudson.test.WithoutJenkins; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.nullValue; @@ -392,7 +392,7 @@ public static class BasicSCMSource extends SCMSource { r.assertLogContains("something special", b); r.assertLogContains("Using shallow clone with depth 1", b); // Fails to reproduce presence of *.jar.tmp@tmp; probably specific to use of GIT_ASKPASS: - assertThat(new File(b.getRootDir(), "libs").list(), arrayContainingInAnyOrder(matchesPattern("[0-9a-f]{64}[.]jar"), matchesPattern("[0-9a-f]{64}-name[.]txt"))); + assertThat(new File(b.getRootDir(), "libs").list(), arrayContaining(matchesPattern("[0-9a-f]{64}[.]jar"))); } @Test public void cloneModeLibraryPath() throws Exception { From a14ed451f02e6b3188664a3d29a857212041aec3 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Sat, 4 Mar 2023 08:56:00 -0500 Subject: [PATCH 11/43] Giving up on https://github.com/jenkinsci/workflow-cps-plugin/pull/668 and tracking code source more conservatively --- pom.xml | 5 -- .../plugins/workflow/libs/LibraryStep.java | 47 +++++++++++-------- .../workflow/libs/LibraryStepTest.java | 3 ++ 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/pom.xml b/pom.xml index 35914100..0a310c58 100644 --- a/pom.xml +++ b/pom.xml @@ -75,11 +75,6 @@ import pom - - org.jenkins-ci.plugins.workflow - workflow-cps - 3625.v4390b_f51b_50f - diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java index 3f2662e3..88f707b5 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java @@ -45,21 +45,20 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; -import java.nio.file.Paths; -import java.security.CodeSource; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Set; import java.util.TreeSet; -import java.util.logging.Level; import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import groovy.lang.MissingPropertyException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.inject.Inject; import jenkins.model.Jenkins; import jenkins.scm.impl.SingleSCMSource; @@ -251,20 +250,20 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S private final @NonNull String prefix; /** {@link Class#getName} minus package prefix */ private final @CheckForNull String clazz; - /** {@code jar:file:/…/libs/$hash.jar!/} */ - private final @NonNull String srcUrl; + /** {@code /…/libs/$hash.jar}, or null if resuming a pre-dir2Jar build */ + private final @Nullable String jar; LoadedClasses(String library, String libraryDirectoryName, boolean trusted, Boolean changelog, Run run) { - this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ "jar:" + new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").toURI() + "!/"); + this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").getAbsolutePath()); } - LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String srcUrl) { + LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String jar) { this.library = library; this.trusted = trusted; this.changelog = changelog; this.prefix = prefix; this.clazz = clazz; - this.srcUrl = srcUrl; + this.jar = jar; } @Override public Object getProperty(String property) { @@ -288,12 +287,12 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S String fullClazz = clazz != null ? clazz + '$' + property : property; loadClass(prefix + fullClazz); // OK, class really exists, stash it and await methods - return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, srcUrl); + return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, jar); } else if (clazz != null) { throw new MissingPropertyException(property, loadClass(prefix + clazz)); } else { // Still selecting package components. - return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, srcUrl); + return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, jar); } } @@ -339,6 +338,8 @@ private static boolean isSandboxed() { // TODO putProperty for static field set + private static final Pattern JAR_URL = Pattern.compile("jar:(file:/.+[.]jar)!/.+"); + private Class loadClass(String name) { CpsFlowExecution exec = CpsThread.current().getExecution(); GroovyClassLoader loader = (trusted ? exec.getTrustedShell() : exec.getShell()).getClassLoader(); @@ -351,14 +352,22 @@ private Class loadClass(String name) { if (definingLoader != loader) { throw new IllegalAccessException("cannot access " + c + " via library handle: " + definingLoader + " is not " + loader); } - // Note that this goes through GroovyCodeSource.(File, String), which unlike (say) URLClassLoader set the “location” to the actual file, *not* the root. - CodeSource codeSource = c.getProtectionDomain().getCodeSource(); - if (codeSource == null) { - throw new IllegalAccessException(name + " had no defined code source"); - } - String loc = codeSource.getLocation().toString(); - if (!loc.startsWith(srcUrl)) { - throw new IllegalAccessException(name + " was defined in " + loc + " which was not inside " + srcUrl); + if (jar != null) { + URL res = loader.getResource(name.replaceFirst("[$][^.]+$", "").replace('.', '/') + ".groovy"); + if (res == null) { + throw new IllegalAccessException("Unknown where " + name + " (" + c.getProtectionDomain().getCodeSource().getLocation() + ") was loaded from"); + } + Matcher m = JAR_URL.matcher(res.toString()); + if (!m.matches()) { + throw new IllegalAccessException("Unexpected URL " + res); + } + File actual = new File(URI.create(m.group(1))); + if (!actual.equals(new File(jar))) { + throw new IllegalAccessException(name + " was defined in " + actual + " rather than the expected " + jar); + } + LOGGER.fine(() -> "loaded " + name + " from " + res + " ~ " + actual + " as expected"); + } else { + LOGGER.fine(() -> "loaded " + name + " but resuming from an old build which did not properly record JAR location"); } if (!Modifier.isPublic(c.getModifiers())) { // unlikely since Groovy makes classes implicitly public throw new IllegalAccessException(c + " is not public"); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java index 4eaa04f4..95b58f5f 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java @@ -36,6 +36,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.logging.Level; import jenkins.plugins.git.GitSCMSource; import jenkins.plugins.git.GitSampleRepoRule; @@ -56,6 +57,7 @@ import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.LoggerRule; @Issue("JENKINS-39450") public class LibraryStepTest { @@ -64,6 +66,7 @@ public class LibraryStepTest { @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); @Rule public GitSampleRepoRule sampleRepo2 = new GitSampleRepoRule(); + @Rule public LoggerRule logging = new LoggerRule().record(LibraryStep.class, Level.FINE); @Test public void configRoundtrip() throws Exception { StepConfigTester stepTester = new StepConfigTester(r); From 1783fb0e9e63e34baa96f09bc2c4b0f0145fc5c0 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 07:28:50 -0500 Subject: [PATCH 12/43] May as well pick up https://github.com/jenkinsci/workflow-cps-plugin/pull/667 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0a310c58..974cf9af 100644 --- a/pom.xml +++ b/pom.xml @@ -71,7 +71,7 @@ io.jenkins.tools.bom bom-2.361.x - 1750.v0071fa_4c4a_e3 + 1886.va_11c9f461054 import pom From 3062a9795eb263c781b4cc79c88ba8f9d444f6d6 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 09:30:21 -0500 Subject: [PATCH 13/43] Comment obsolete as of a14ed45 --- .../org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java index 95b58f5f..5925cd0b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java @@ -132,8 +132,6 @@ public class LibraryStepTest { r.assertLogContains("using constant vs. constant", b); } - // TODO test LoadedClasses.srcUrl after reload build started prior to dir2Jar (@OldData) - @Test public void missingProperty() throws Exception { sampleRepo.init(); sampleRepo.write("src/some/pkg/MyClass.groovy", "package some.pkg; class MyClass { }"); From 859dad14fa0893fae45aecbd5532ed178c9f0d87 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 09:55:01 -0500 Subject: [PATCH 14/43] Properly migrating running builds crossing the update line --- .../plugins/workflow/libs/LibraryAdder.java | 8 ++ .../workflow/libs/LibraryAdderTest.java | 28 ++++ .../jobs/p/builds/1/build.xml | 130 ++++++++++++++++++ .../1/changelog13282840290199774426.xml | 0 ...f7702b01d5d4b3167434a1b110c3dfbf8-name.txt | 1 + .../vars/foo.groovy | 1 + .../jobs/p/builds/1/log | 28 ++++ .../jobs/p/builds/1/log-index | 1 + .../jobs/p/builds/1/program.dat | Bin 0 -> 3049 bytes .../jobs/p/builds/1/workflow/2.xml | 12 ++ .../jobs/p/builds/1/workflow/3.xml | 26 ++++ .../jobs/p/builds/legacyIds | 0 .../jobs/p/builds/permalinks | 1 + .../jobs/p/config.xml | 13 ++ .../jobs/p/nextBuildNumber | 1 + ...lugins.workflow.flow.FlowExecutionList.xml | 7 + 16 files changed, 257 insertions(+) create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/changelog13282840290199774426.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8-name.txt create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8/vars/foo.groovy create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/program.dat create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/2.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/legacyIds create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/config.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 6318471b..f67334f1 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -102,6 +102,14 @@ FilePath libJar = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName() + ".jar"); if (libJar.exists()) { additions.add(new Addition(libJar.toURI().toURL(), record.trusted)); + } else { + FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName()); + if (libDir.isDirectory()) { + execution.getOwner().getListener().getLogger().println("Migrating " + libDir + " to " + libJar); + LibraryRetriever.dir2Jar(record.getName(), libDir, libJar); + libDir.deleteRecursive(); + additions.add(new Addition(libJar.toURI().toURL(), record.trusted)); + } } String unparsed = librariesUnparsed.get(record.name); if (unparsed != null) { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java index 0e11b6f2..5b71977c 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java @@ -475,6 +475,34 @@ public void correctLibraryDirectoryUsedWhenResumingOldBuild() throws Exception { r.assertLogContains("called Foo", b); } + @LocalData + @Test + public void correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild() throws Exception { + // LocalData captured as of 1091aea7fa252acae11389588addf603a505e195: + /* + sampleRepo.init(); + sampleRepo.write("vars/foo.groovy", "def call() { echo('called Foo') }"); + sampleRepo.git("add", "vars"); + sampleRepo.git("commit", "--message=init"); + GlobalLibraries.get().setLibraries(Collections.singletonList( + new LibraryConfiguration("lib", + new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))); + WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "@Library('lib@master') _\n" + + "sleep 180\n" + + "foo()", true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + r.waitForMessage("Sleeping for 3 min", b); + b.save(); + Thread.sleep(Long.MAX_VALUE); + */ + WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class); + WorkflowRun b = p.getBuildByNumber(1); + r.assertBuildStatus(Result.SUCCESS, r.waitForCompletion(b)); + r.assertLogContains("called Foo", b); + } + @Issue("JENKINS-66898") @Test public void parallelBuildsDontInterfereWithExpiredCache() throws Throwable { diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml new file mode 100644 index 00000000..2e9bc7ba --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml @@ -0,0 +1,130 @@ + + + + + + + lib + master + + foo + + true + true + 897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8 + + + + + + + master + + + 3ed68a85765cb39641044af8dc4ad436fafe3fdd + + + + master + + + + + 1 + + + + + + …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 + + + + + + git …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 + + + + + + 1 + 1678113342893 + 1678113342911 + 0 + UTF-8 + false + + SUCCESS + + + MAX_SURVIVABILITY + + + flowNode + 36231997 + + + classLoad + 193985514 + + + run + 152858131 + + + parse + 500490530 + + + saveProgram + 38950881 + + + true + 3 + 1:3 + 2 + false + false + + false + + + + 2 + + + origin + +refs/heads/*:refs/remotes/origin/* + …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 + + + + + master + + + false + + + + + false + + + + + + + + + + …/pipeline-groovy-lib-plugin/target/tmp/j h17061459720094902767/workspace/p@libs/999c1ee22db1f1f266cfcd6f657a4e9044467f77fcf8c8e4a377d3099a1d3a75 + …/pipeline-groovy-lib-plugin/target/tmp/j h17061459720094902767/jobs/p/builds/1/changelog13282840290199774426.xml + + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/changelog13282840290199774426.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/changelog13282840290199774426.xml new file mode 100644 index 00000000..e69de29b diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8-name.txt b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8-name.txt new file mode 100644 index 00000000..7951405f --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8-name.txt @@ -0,0 +1 @@ +lib \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8/vars/foo.groovy b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8/vars/foo.groovy new file mode 100644 index 00000000..8f75b987 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/897d64f4c3184a6d97a566b1786f0f0f7702b01d5d4b3167434a1b110c3dfbf8/vars/foo.groovy @@ -0,0 +1 @@ +def call() { echo('called Foo') } \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log new file mode 100644 index 00000000..423b9c8d --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log @@ -0,0 +1,28 @@ +Started +Loading library lib@master +Attempting to resolve master from remote references... + > git --version # timeout=10 + > git --version # 'git version 2.34.1' + > git ls-remote -h -- …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 # timeout=10 +Found match: refs/heads/master revision 3ed68a85765cb39641044af8dc4ad436fafe3fdd +The recommended git tool is: NONE +No credentials specified +Cloning the remote Git repository +Cloning with configured refspecs honoured and without tags +Cloning repository …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 + > git init …/pipeline-groovy-lib-plugin/target/tmp/j h17061459720094902767/workspace/p@libs/999c1ee22db1f1f266cfcd6f657a4e9044467f77fcf8c8e4a377d3099a1d3a75 # timeout=10 +Fetching upstream changes from …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 + > git --version # timeout=10 + > git --version # 'git version 2.34.1' + > git fetch --no-tags --force --progress -- …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 +refs/heads/*:refs/remotes/origin/* # timeout=10 + > git config remote.origin.url …/pipeline-groovy-lib-plugin/target/tmp/junit14177916507003849782/junit11215768211970974458 # timeout=10 + > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10 +Avoid second fetch +Checking out Revision 3ed68a85765cb39641044af8dc4ad436fafe3fdd (master) + > git config core.sparsecheckout # timeout=10 + > git checkout -f 3ed68a85765cb39641044af8dc4ad436fafe3fdd # timeout=10 +Commit message: "init" +First time build. Skipping changelog. +ha:////4DehqyOFtNH235D4++PGtYgv/4iflo72uInNzFKYe/qgAAAAoh+LCAAAAAAAAP9tjTEOwjAQBM8BClpKHuFItIiK1krDC0x8GCfWnbEdkooX8TX+gCESFVvtrLSa5wtWKcKBo5UdUu8otU4GP9jS5Mixv3geZcdn2TIl9igbHBs2eJyx4YwwR1SwULBGaj0nRzbDRnX6rmuvydanHMu2V1A5c4MHCFXMWcf8hSnC9jqYxPTz/BXAFEIGsfuclm8zQVqFvQAAAA==[Pipeline] Start of Pipeline +ha:////4KmcPvrxnGvS+11mq0cJwM0MifzD3S6+ZqB6eVBoHivtAAAAoh+LCAAAAAAAAP9tjTEOAiEURD9rLGwtPQSbaGmsbAmNJ0AWEZb8zwLrbuWJvJp3kLiJlZNMMm+a93rDOic4UbLcG+wdZu14DKOti0+U+lugiXu6ck2YKRguzSSpM+cFJRUDS1gDKwEbgzpQdmgLbIVXD9UGhba9lFS/o4DGdQM8gYlqLiqVL8wJdvexy4Q/z18BzLEA29ce4gfya1RxvAAAAA==[Pipeline] sleep +Sleeping for 3 min 0 sec diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index new file mode 100644 index 00000000..8a1427c8 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index @@ -0,0 +1 @@ +2388 3 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/program.dat b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/program.dat new file mode 100644 index 0000000000000000000000000000000000000000..e2bd5c9b13f761dc0de6ed673c3589f31959ee78 GIT binary patch literal 3049 zcma)8&u<$=6yCKrcHESZa##*UC~eb#NLe6Ysf*D-NYW(Grd3>8DXIjM-HE;RWM`S# zO&pFKIPwQ@C_+LM2??n}fRI3>;)Zew2YyKiIM80GdguX4MZ|lvcGuAQO5`aaYtDHVHiA%jffMnh1}kSx$i^}kYmOHwXEIW! z>ylA_Mv5dd(gzTXKGN@Tp%TfIZIJs}lX*!Ti0~9m6!V7-Vg(|mhYV6wo(!TGcNJp| z1xwpHO7@+ntJDD_?#xnEKS86u;?I}Av0vK)_#D~81@(_nFBWptASEZyVuf&m>&F}>+v9LFu%q4nT`2#Lww^FdLaYU^O#a6 zrO)KBQMuKkx%W0rlk};WMG62%d*CruJ&uvXgA7bksMFBr46|P%(_H{m5=BDBP84_z z4s=zUBNo}2lItheLMAnOD?@hKC|i}At*A{Mbq*cBjJv>PkGW6vmO<_(4?yZ8&d~L& z<{(i{L!Jm@){M_kNI#?=-}OORk_Z-79YhPCxd0SHijfpGNmt;-0O%-LVP_~$AQi;I zj6zBl7ESkeP~}*cykJutgYK9G_)LsB_ix*9Z-O|rTE;E-&D5#{;^iJNtH5hXG$(;l z5GF|HmoFau_~3!}FCY>rq^)Cvghx8+Fk_qzjhIN(Y@~&MyrX#lId+aEp@(^%q?}KO zs~H>!f(wswNOOrC-W}y$>jD`7TV4%fc3gN6PXWI&39CMzMR6Ug6ojjyq2JvnS)9ml z!uO$Ey0GzjgcU~Z9@*V_RV30wFBFMn7~r82gls-pSz;3W6;@bW7oZXwHst{Lk&eJ8 z^v)eH^JLIx9tR1AwMkBOto_RYXY+Ig)4nJ5k$$t?O?Q*zWfFq3&gY3P#EX#ZM%5D$ zyR!^A_fJsT4J^WF$8a^mDJh!kcNc@vP|Zndonr5yFWnn zE!K?2pLYWZ-4aJv->~X2NovD!%jH+aGf4~K((^~ z+5&ndjAa7nZ}uQH?D9kra31a$yABudyY$aXaJx7Snz=iTAkNASaFBO@><$72Rxu6Z zyFp+QuciaUCi!I{4EHyy^2ZLc-K!E6vCvP)4d}1EDX-@vj)|Q6tLbi=W%r8vsRfucs zLMW)7!0rG+LwM4lie5Ug(__5H`zOL-qrs$4u{2nLSWCiR2j!RW6UQ<3#opT&(FL{) zjdu~=4JkmFV(C3p$T_?g^r!Q54lV~J;&!9ex(iLQWtwoEq*g#PZ*CoGe)qvo7+8rs o*$oHcP|nVL;Nq~b=)ObDr-{=WOj`hHQp~qLKR5GL*;-iq57g*&l>h($ literal 0 HcmV?d00001 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/2.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/2.xml new file mode 100644 index 00000000..725bc7eb --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/2.xml @@ -0,0 +1,12 @@ + + + + + 2 + + + + 1678113343524 + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml new file mode 100644 index 00000000..3c34d0fe --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml @@ -0,0 +1,26 @@ + + + + + 2 + + 3 + org.jenkinsci.plugins.workflow.steps.SleepStep + + + + + + time + 180 + + + + true + + + 1678113343620 + + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/legacyIds b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/legacyIds new file mode 100644 index 00000000..e69de29b diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks new file mode 100644 index 00000000..48ab9e85 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks @@ -0,0 +1 @@ +lastSuccessfulBuild -1 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/config.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/config.xml new file mode 100644 index 00000000..07f78a6f --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/config.xml @@ -0,0 +1,13 @@ + + + false + + + + true + + + false + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber new file mode 100644 index 00000000..0cfbf088 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber @@ -0,0 +1 @@ +2 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml new file mode 100644 index 00000000..33e6c526 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest/correctLibraryDirectoryUsedWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml @@ -0,0 +1,7 @@ + + + + p + 1 + + \ No newline at end of file From 6ba9d4342a09bc3f98a40aca8868d01174b2f714 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 11:23:50 -0500 Subject: [PATCH 15/43] Restoring SECURITY-2479 defense (SECURITY-2476 is no longer vulnerable by design) --- .../workflow/libs/LibraryRetriever.java | 18 ++++++++++++ .../workflow/libs/LibraryRetrieverTest.java | 28 +++++++++++++++++++ .../workflow/libs/ResourceStepTest.java | 4 +-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index 9e52b418..ccd15ab2 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -39,6 +39,7 @@ import hudson.util.io.ArchiverFactory; import java.io.IOException; import java.io.OutputStream; +import java.nio.file.Path; import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -99,6 +100,7 @@ static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePa if (resources.isDirectory()) { resources.renameTo(tmp.child("resources")); } + lookForBadSymlinks(tmp, tmp); try (OutputStream os = tmp.child(JarFile.MANIFEST_NAME).write()) { Manifest m = new Manifest(); m.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); @@ -114,6 +116,22 @@ static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePa } } + private static void lookForBadSymlinks(FilePath root, FilePath dir) throws IOException, InterruptedException { + for (FilePath child : dir.list()) { + if (child.isDirectory()) { + lookForBadSymlinks(root, child); + } else { + String link = child.readLink(); + if (link != null) { + Path target = Path.of(dir.getRemote(), link).toRealPath(); + if (!target.startsWith(Path.of(root.getRemote()))) { + throw new SecurityException(child + " → " + target + " is not inside " + root); + } + } + } + } + } + /** * Obtains library sources. * @param name the {@link LibraryConfiguration#getName} diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index 0c176233..bf454ea1 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -25,12 +25,17 @@ package org.jenkinsci.plugins.workflow.libs; import hudson.FilePath; +import hudson.model.TaskListener; +import hudson.util.StreamTaskListener; +import java.nio.charset.StandardCharsets; import java.util.Set; import java.util.TreeSet; import java.util.jar.JarFile; +import org.apache.commons.io.IOUtils; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThrows; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -51,6 +56,29 @@ public class LibraryRetrieverTest { assertDir2Jar(Set.of("src/p1/xxx.groovy", "vars/yyy.groovy", "resources/a.txt", "resources/b/c.txt"), Set.of("p1/xxx.groovy", "yyy.groovy", "resources/a.txt", "resources/b/c.txt")); } + @Test public void safeSymlinks() throws Exception { + FilePath work = new FilePath(tmp.newFolder()); + FilePath dir = work.child("dir"); + dir.child("resources/a.txt").write("content", null); + dir.child("resources/b.txt").symlinkTo("a.txt", TaskListener.NULL); + FilePath jar = work.child("x.jar"); + LibraryRetriever.dir2Jar("mylib", dir, jar); + try (JarFile jf = new JarFile(jar.getRemote())) { + assertThat(IOUtils.toString(jf.getInputStream(jf.getEntry("resources/a.txt")), StandardCharsets.UTF_8), is("content")); + assertThat(IOUtils.toString(jf.getInputStream(jf.getEntry("resources/b.txt")), StandardCharsets.UTF_8), is("content")); + } + } + + @Test public void unsafeSymlinks() throws Exception { + FilePath work = new FilePath(tmp.newFolder()); + FilePath dir = work.child("dir"); + dir.child("resources").mkdirs(); + work.child("secret.txt").write("s3cr3t", null); + dir.child("resources/hack.txt").symlinkTo("../../secret.txt", StreamTaskListener.fromStderr()); + FilePath jar = work.child("x.jar"); + assertThrows(SecurityException.class, () -> LibraryRetriever.dir2Jar("mylib", dir, jar)); + } + // TODO assert that other files are not copied private void assertDir2Jar(Set inputs, Set outputs) throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java index c1faefca..5d0023cd 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java @@ -204,7 +204,7 @@ public class ResourceStepTest { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("@Library('symlink-stuff@master') import Stuff; echo(Stuff.contents(this))", true)); - r.assertLogContains("master.key references a file that is not contained within the library: symlink-stuff", r.buildAndAssertStatus(Result.FAILURE, p)); + r.assertLogContains("master.key is not inside", r.buildAndAssertStatus(Result.FAILURE, p)); } @Issue("SECURITY-2476") @@ -222,7 +222,7 @@ public class ResourceStepTest { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("@Library('libres-stuff@master') import Stuff; echo(Stuff.contents(this))", true)); - r.assertLogContains("../../../../../../../secrets/master.key references a file that is not contained within the library: libres-stuff", r.buildAndAssertStatus(Result.FAILURE, p)); + r.assertLogContains("No such library resource ../../../../../../../secrets/master.key could be found.", r.buildAndAssertStatus(Result.FAILURE, p)); } @Test public void findResourcesAttemptsToLoadFromAllIncludedLibraries() throws Exception { From fa82612d4d4884e911b57f17e59c23670cc0aa26 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 13:21:27 -0500 Subject: [PATCH 16/43] `SCMSourceRetrieverTest.lease` highlighted that `dir2Jar` was improperly deleting its source; anyway it needed to be refactored to be simpler and more efficient by directly archiving the desired files --- .../workflow/libs/LibraryRetriever.java | 34 ++++++++----------- .../workflow/libs/LibraryRetrieverTest.java | 9 +++-- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index ccd15ab2..585ebbbe 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -36,7 +36,10 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import hudson.slaves.WorkspaceList; +import hudson.util.DirScanner; +import hudson.util.FileVisitor; import hudson.util.io.ArchiverFactory; +import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.nio.file.Path; @@ -84,24 +87,10 @@ public void retrieveJar(@NonNull String name, @NonNull String version, boolean c * into a JAR file with Groovy in classpath orientation and {@code resources/} as a ZIP folder. */ static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException { - // TODO do this more efficiently by packing JAR directly - FilePath tmp = jar.sibling(jar.getBaseName() + "-repack"); - tmp.mkdirs(); + lookForBadSymlinks(dir, dir); + FilePath mf = jar.withSuffix(".mf"); try { - FilePath src = dir.child("src"); - if (src.isDirectory()) { - src.moveAllChildrenTo(tmp); - } - FilePath vars = dir.child("vars"); - if (vars.isDirectory()) { - vars.moveAllChildrenTo(tmp); - } - FilePath resources = dir.child("resources"); - if (resources.isDirectory()) { - resources.renameTo(tmp.child("resources")); - } - lookForBadSymlinks(tmp, tmp); - try (OutputStream os = tmp.child(JarFile.MANIFEST_NAME).write()) { + try (OutputStream os = mf.write()) { Manifest m = new Manifest(); m.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); // Informational debugging aid, since the hex JAR basename will be meaningless: @@ -109,10 +98,17 @@ static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePa m.write(os); } try (OutputStream os = jar.write()) { - tmp.archive(ArchiverFactory.ZIP, os, "**"); + dir.archive(ArchiverFactory.ZIP, os, new DirScanner() { + @Override public void scan(File dir, FileVisitor visitor) throws IOException { + scanSingle(new File(mf.getRemote()), JarFile.MANIFEST_NAME, visitor); + new DirScanner.Glob("**/*.groovy", null).scan(new File(dir, "src"), visitor); + new DirScanner.Glob("*.groovy,*.txt", null).scan(new File(dir, "vars"), visitor); + new DirScanner.Glob("resources/", null).scan(dir, visitor); + } + }); } } finally { - tmp.deleteRecursive(); + mf.delete(); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index bf454ea1..2e4ade51 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -33,6 +33,7 @@ import java.util.jar.JarFile; import org.apache.commons.io.IOUtils; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThrows; @@ -56,6 +57,10 @@ public class LibraryRetrieverTest { assertDir2Jar(Set.of("src/p1/xxx.groovy", "vars/yyy.groovy", "resources/a.txt", "resources/b/c.txt"), Set.of("p1/xxx.groovy", "yyy.groovy", "resources/a.txt", "resources/b/c.txt")); } + @Test public void otherFiles() throws Exception { + assertDir2Jar(Set.of("vars/v.groovy", "vars/v.txt", "vars/README.md", "README.md", "docs/usage.png", "src/p1/C.groovy", "src/p1/README.md"), Set.of("v.groovy", "v.txt", "p1/C.groovy")); + } + @Test public void safeSymlinks() throws Exception { FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); @@ -79,8 +84,6 @@ public class LibraryRetrieverTest { assertThrows(SecurityException.class, () -> LibraryRetriever.dir2Jar("mylib", dir, jar)); } - // TODO assert that other files are not copied - private void assertDir2Jar(Set inputs, Set outputs) throws Exception { FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); @@ -88,7 +91,9 @@ private void assertDir2Jar(Set inputs, Set outputs) throws Excep dir.child(input).write("xxx", null); } FilePath jar = work.child("x.jar"); + var before = dir.list("**"); LibraryRetriever.dir2Jar("mylib", dir, jar); + assertThat(dir.list("**"), arrayContainingInAnyOrder(before)); Set actualOutputs = new TreeSet<>(); try (JarFile jf = new JarFile(jar.getRemote())) { assertThat(jf.getManifest().getMainAttributes().getValue(LibraryRetriever.ATTR_LIBRARY_NAME), is("mylib")); From 75f2caeb8952146fcf622733a8289758cf2f813f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 13:22:46 -0500 Subject: [PATCH 17/43] Incremental build of https://github.com/jenkinsci/workflow-cps-plugin/pull/672 --- pom.xml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 974cf9af..d43b7878 100644 --- a/pom.xml +++ b/pom.xml @@ -71,10 +71,15 @@ io.jenkins.tools.bom bom-2.361.x - 1886.va_11c9f461054 + 1750.v0071fa_4c4a_e3 import pom + + org.jenkins-ci.plugins.workflow + workflow-cps + 3625.vb_50f7cb_e66b_7 + From 5bbb1edd9a7c5c535991b1f5c8ebfe576dfe16f9 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 13:50:39 -0500 Subject: [PATCH 18/43] Restoring support for `INCLUDE_SRC_TEST_IN_LIBRARIES`, as well as warning about missing sources --- .../plugins/workflow/libs/LibraryAdder.java | 6 ++--- .../workflow/libs/LibraryRetriever.java | 27 ++++++++++++++++--- .../workflow/libs/SCMSourceRetriever.java | 10 ++----- .../workflow/libs/LibraryAdderTest.java | 2 +- .../workflow/libs/LibraryRetrieverTest.java | 8 +++--- 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index f67334f1..201bcea6 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -94,6 +94,7 @@ libraryChangelogs.put(parsed[0], changelogs.get(library)); librariesUnparsed.put(parsed[0], library); } + TaskListener listener = execution.getOwner().getListener(); List additions = new ArrayList<>(); LibrariesAction action = build.getAction(LibrariesAction.class); if (action != null) { @@ -105,8 +106,8 @@ } else { FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName()); if (libDir.isDirectory()) { - execution.getOwner().getListener().getLogger().println("Migrating " + libDir + " to " + libJar); - LibraryRetriever.dir2Jar(record.getName(), libDir, libJar); + listener.getLogger().println("Migrating " + libDir + " to " + libJar); + LibraryRetriever.dir2Jar(record.getName(), libDir, libJar, listener); libDir.deleteRecursive(); additions.add(new Addition(libJar.toURI().toURL(), record.trusted)); } @@ -121,7 +122,6 @@ // Now we will see which libraries we want to load for this job. Map librariesAdded = new LinkedHashMap<>(); Map retrievers = new HashMap<>(); - TaskListener listener = execution.getOwner().getListener(); for (LibraryResolver kind : ExtensionList.lookup(LibraryResolver.class)) { boolean kindTrusted = kind.isTrusted(); for (LibraryConfiguration cfg : kind.forJob(build.getParent(), libraryVersions)) { diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index 585ebbbe..7dd13b54 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -43,6 +43,7 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -72,7 +73,7 @@ public void retrieveJar(@NonNull String name, @NonNull String version, boolean c FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); try { retrieve(name, version, changelog, tmp, run, listener); - dir2Jar(name, tmp, target); + dir2Jar(name, tmp, target, listener); } finally { tmp.deleteRecursive(); WorkspaceList.tempDir(tmp).deleteRecursive(); @@ -86,7 +87,7 @@ public void retrieveJar(@NonNull String name, @NonNull String version, boolean c * Translates a historical directory with {@code src/} and/or {@code vars/} and/or {@code resources/} subdirectories * into a JAR file with Groovy in classpath orientation and {@code resources/} as a ZIP folder. */ - static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException { + static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePath jar, @NonNull TaskListener listener) throws IOException, InterruptedException { lookForBadSymlinks(dir, dir); FilePath mf = jar.withSuffix(".mf"); try { @@ -101,8 +102,26 @@ static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePa dir.archive(ArchiverFactory.ZIP, os, new DirScanner() { @Override public void scan(File dir, FileVisitor visitor) throws IOException { scanSingle(new File(mf.getRemote()), JarFile.MANIFEST_NAME, visitor); - new DirScanner.Glob("**/*.groovy", null).scan(new File(dir, "src"), visitor); - new DirScanner.Glob("*.groovy,*.txt", null).scan(new File(dir, "vars"), visitor); + String excludes; + if (!SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES && new File(dir, "src/test").isDirectory()) { + excludes = "test/"; + listener.getLogger().println("Excluding src/test/ so that library test code cannot be accessed by Pipelines."); + listener.getLogger().println("To remove this log message, move the test code outside of src/. To restore the previous behavior that allowed access to files in src/test/, pass -D" + SCMSourceRetriever.class.getName() + ".INCLUDE_SRC_TEST_IN_LIBRARIES=true to the java command used to start Jenkins."); + } else { + excludes = null; + } + AtomicBoolean found = new AtomicBoolean(); + FileVisitor verifyingVisitor = visitor.with(pathname -> { + if (pathname.getName().endsWith(".groovy")) { + found.set(true); + } + return true; + }); + new DirScanner.Glob("**/*.groovy", excludes).scan(new File(dir, "src"), verifyingVisitor); + new DirScanner.Glob("*.groovy,*.txt", null).scan(new File(dir, "vars"), verifyingVisitor); + if (!found.get()) { + throw new AbortException("Library " + name + " expected to contain at least one of src or vars directories"); + } new DirScanner.Glob("resources/", null).scan(dir, visitor); } }); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index 764c648f..d6888b05 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -221,14 +221,9 @@ static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String } else if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { throw new AbortException("Library path may not contain '..'"); } - String excludes = INCLUDE_SRC_TEST_IN_LIBRARIES ? null : "src/test/"; - if (lease.path.child(libraryPath).child("src/test").exists()) { - listener.getLogger().println("Excluding src/test/ from checkout of " + scm.getKey() + " so that library test code cannot be accessed by Pipelines."); - listener.getLogger().println("To remove this log message, move the test code outside of src/. To restore the previous behavior that allowed access to files in src/test/, pass -D" + SCMSourceRetriever.class.getName() + ".INCLUDE_SRC_TEST_IN_LIBRARIES=true to the java command used to start Jenkins."); - } // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? // Copy sources with relevant files from the checkout: - LibraryRetriever.dir2Jar(name, lease.path.child(libraryPath), target); + LibraryRetriever.dir2Jar(name, lease.path.child(libraryPath), target, listener); } } @@ -260,8 +255,7 @@ private static void doClone(@NonNull String name, @NonNull SCM scm, String libra } root = tmp.child(libraryPath); } - // TODO handle INCLUDE_SRC_TEST_IN_LIBRARIES - LibraryRetriever.dir2Jar(name, root, target); + LibraryRetriever.dir2Jar(name, root, target, listener); } finally { tmp.deleteRecursive(); WorkspaceList.tempDir(tmp).deleteRecursive(); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java index 5b71977c..a7972094 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java @@ -374,7 +374,7 @@ public class LibraryAdderTest { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("@Library('lib@master') import test.Foo", true)); WorkflowRun b = r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0)); - r.assertLogContains("Excluding src/test/ from checkout", b); + r.assertLogContains("Excluding src/test/", b); r.assertLogContains("expected to contain at least one of src or vars directories", b); } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index 2e4ade51..7c9be8d5 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -65,9 +65,9 @@ public class LibraryRetrieverTest { FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); dir.child("resources/a.txt").write("content", null); - dir.child("resources/b.txt").symlinkTo("a.txt", TaskListener.NULL); + dir.child("resources/b.txt").symlinkTo("a.txt", StreamTaskListener.fromStderr()); FilePath jar = work.child("x.jar"); - LibraryRetriever.dir2Jar("mylib", dir, jar); + LibraryRetriever.dir2Jar("mylib", dir, jar, StreamTaskListener.fromStderr()); try (JarFile jf = new JarFile(jar.getRemote())) { assertThat(IOUtils.toString(jf.getInputStream(jf.getEntry("resources/a.txt")), StandardCharsets.UTF_8), is("content")); assertThat(IOUtils.toString(jf.getInputStream(jf.getEntry("resources/b.txt")), StandardCharsets.UTF_8), is("content")); @@ -81,7 +81,7 @@ public class LibraryRetrieverTest { work.child("secret.txt").write("s3cr3t", null); dir.child("resources/hack.txt").symlinkTo("../../secret.txt", StreamTaskListener.fromStderr()); FilePath jar = work.child("x.jar"); - assertThrows(SecurityException.class, () -> LibraryRetriever.dir2Jar("mylib", dir, jar)); + assertThrows(SecurityException.class, () -> LibraryRetriever.dir2Jar("mylib", dir, jar, StreamTaskListener.fromStderr())); } private void assertDir2Jar(Set inputs, Set outputs) throws Exception { @@ -92,7 +92,7 @@ private void assertDir2Jar(Set inputs, Set outputs) throws Excep } FilePath jar = work.child("x.jar"); var before = dir.list("**"); - LibraryRetriever.dir2Jar("mylib", dir, jar); + LibraryRetriever.dir2Jar("mylib", dir, jar, StreamTaskListener.fromStderr()); assertThat(dir.list("**"), arrayContainingInAnyOrder(before)); Set actualOutputs = new TreeSet<>(); try (JarFile jf = new JarFile(jar.getRemote())) { From 0a7ccb440e822f62c72975b47b144ceba1ff07ac Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:12:38 -0500 Subject: [PATCH 19/43] Adapting `cloneModeExcludeSrcTest` to revised message --- .../jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index 2cd4cfbd..64d98f39 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -447,7 +447,7 @@ public static class BasicSCMSource extends SCMSource { WorkflowRun b = r.buildAndAssertSuccess(p); assertFalse(r.jenkins.getWorkspaceFor(p).withSuffix("@libs").isDirectory()); r.assertLogContains("something special", b); - r.assertLogContains("Excluding src/test/ from checkout", b); + r.assertLogContains("Excluding src/test/", b); } @Test public void cloneModeIncludeSrcTest() throws Exception { From 20729d2546f0edec1865bfef7226e1291a34fae5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:24:46 -0500 Subject: [PATCH 20/43] Fixing `vars/*.txt` --- .../cps/global/UserDefinedGlobalVariable.java | 40 ++++++++++++------- .../plugins/workflow/libs/LibraryAdder.java | 4 +- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java index dfa0f8e9..35b1e978 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java @@ -1,20 +1,21 @@ package org.jenkinsci.plugins.workflow.cps.global; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; import groovy.lang.Binding; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.URI; import java.nio.charset.StandardCharsets; -import org.apache.commons.io.FileUtils; +import jenkins.model.Jenkins; +import org.apache.commons.io.IOUtils; import org.codehaus.groovy.control.MultipleCompilationErrorsException; import org.jenkinsci.plugins.workflow.cps.CpsCompilationErrorsException; import org.jenkinsci.plugins.workflow.cps.CpsScript; import org.jenkinsci.plugins.workflow.cps.CpsThread; import org.jenkinsci.plugins.workflow.cps.GlobalVariable; -import edu.umd.cs.findbugs.annotations.CheckForNull; -import edu.umd.cs.findbugs.annotations.NonNull; -import java.io.File; -import java.io.IOException; -import jenkins.model.Jenkins; - /** * Global variable backed by user-supplied script. * @@ -22,11 +23,18 @@ */ // not @Extension because these are instantiated programmatically public class UserDefinedGlobalVariable extends GlobalVariable { - // TODO switch to URL - private final File help; + private final URI help; private final String name; + /** + * @deprecated use {@link #UserDefinedGlobalVariable(String, URI)} + */ + @Deprecated public UserDefinedGlobalVariable(String name, File help) { + this(name, help.toURI()); + } + + public UserDefinedGlobalVariable(String name, URI help) { this.name = name; this.help = help; } @@ -70,12 +78,14 @@ public Object getValue(@NonNull CpsScript script) throws Exception { * Loads help from user-defined file, if available. */ public @CheckForNull String getHelpHtml() throws IOException { - if (!help.exists()) return null; - - return Jenkins.get().getMarkupFormatter().translate( - FileUtils.readFileToString(help, StandardCharsets.UTF_8). - // Util.escape translates \n but not \r, and we do not know what platform the library will be checked out on: - replace("\r\n", "\n")); + try { + return Jenkins.get().getMarkupFormatter().translate( + IOUtils.toString(help, StandardCharsets.UTF_8). + // Util.escape translates \n but not \r, and we do not know what platform the library will be checked out on: + replace("\r\n", "\n")); + } catch (FileNotFoundException x) { + return null; + } } @Override diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 201bcea6..82b12ca0 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -49,6 +49,7 @@ import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import java.net.URI; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.regex.Matcher; @@ -337,8 +338,7 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret List vars = new ArrayList<>(); for (LibraryRecord library : action.getLibraries()) { for (String variable : library.variables) { - // TODO pass URL of *.jar!/$variable.txt - vars.add(new UserDefinedGlobalVariable(variable, new File(run.getRootDir(), "libs/" + library.getDirectoryName() + "/vars/" + variable + ".txt"))); + vars.add(new UserDefinedGlobalVariable(variable, URI.create("jar:" + new File(run.getRootDir(), "libs/" + library.getDirectoryName() + ".jar").toURI() + "!/" + variable + ".txt"))); } } return vars; From 29a8848bd6ad6520e40d264d7d040e1304d57872 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:26:44 -0500 Subject: [PATCH 21/43] `safeSymlinks` broken by requirement to have at least one source file --- .../jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index 7c9be8d5..50b69947 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -64,6 +64,7 @@ public class LibraryRetrieverTest { @Test public void safeSymlinks() throws Exception { FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); + dir.child("vars/x.groovy").write("content", null); dir.child("resources/a.txt").write("content", null); dir.child("resources/b.txt").symlinkTo("a.txt", StreamTaskListener.fromStderr()); FilePath jar = work.child("x.jar"); @@ -77,6 +78,7 @@ public class LibraryRetrieverTest { @Test public void unsafeSymlinks() throws Exception { FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); + dir.child("vars/x.groovy").write("content", null); dir.child("resources").mkdirs(); work.child("secret.txt").write("s3cr3t", null); dir.child("resources/hack.txt").symlinkTo("../../secret.txt", StreamTaskListener.fromStderr()); From af8414f3931900cb8fa86546c9e1dcc1880dc721 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:41:29 -0500 Subject: [PATCH 22/43] Fixing `LoadedLibraries`, though not yet actual replay --- .../plugins/workflow/libs/LibraryAdder.java | 27 ++++++++++--------- .../workflow/libs/LibraryAdderTest.java | 10 ------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 82b12ca0..7c259cd5 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -50,6 +50,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.regex.Matcher; @@ -358,20 +359,24 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret Run run = (Run) executable; LibrariesAction action = run.getAction(LibrariesAction.class); if (action != null) { - // TODO handle *.jar FilePath libs = new FilePath(run.getRootDir()).child("libs"); for (LibraryRecord library : action.getLibraries()) { if (library.trusted) { continue; // TODO JENKINS-41157 allow replay of trusted libraries if you have ADMINISTER } - for (String rootName : new String[] {"src", "vars"}) { - FilePath root = libs.child(library.getDirectoryName() + "/" + rootName); - if (!root.isDirectory()) { - continue; - } - for (FilePath groovy : root.list("**/*.groovy")) { - String clazz = className(groovy.getRemote(), root.getRemote()); - scripts.put(clazz, groovy.readToString()); // TODO no idea what encoding the Groovy compiler uses + FilePath jar = libs.child(library.getDirectoryName() + ".jar"); + if (!jar.exists()) { + continue; + } + try (JarFile jf = new JarFile(jar.getRemote())) { + for (JarEntry je : (Iterable) jf.stream()::iterator) { + if (je.getName().endsWith(".groovy")) { + String text; + try (InputStream is = jf.getInputStream(je)) { + text = IOUtils.toString(is, StandardCharsets.UTF_8); // TODO no idea what encoding the Groovy compiler uses + } + scripts.put(je.getName().replaceFirst("[.]groovy$", "").replace('/', '.'), text); + } } } } @@ -383,10 +388,6 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret return scripts; } - static String className(String groovy, String root) { - return groovy.replaceFirst("^" + Pattern.quote(root) + "[/\\\\](.+)[.]groovy", "$1").replace('/', '.').replace('\\', '.'); - } - } @Extension public static class Copier extends FlowCopier.ByRun { diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java index a7972094..3169dcc0 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java @@ -46,7 +46,6 @@ import jenkins.scm.impl.subversion.SubversionSCMSource; import jenkins.scm.impl.subversion.SubversionSampleRepoRule; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.GlobalVariable; @@ -64,7 +63,6 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; -import org.jvnet.hudson.test.WithoutJenkins; import org.jvnet.hudson.test.recipes.LocalData; public class LibraryAdderTest { @@ -546,12 +544,4 @@ public void parallelBuildsDontInterfereWithExpiredCache() throws Throwable { // TODO test migration in LibraryAdder.add after reload build started prior to dir2Jar (@OldData) - @Issue("JENKINS-68544") - @WithoutJenkins - @Test public void className() { - assertThat(LibraryAdder.LoadedLibraries.className("/path/to/lib/src/some/pkg/Type.groovy", "/path/to/lib/src"), is("some.pkg.Type")); - assertThat(LibraryAdder.LoadedLibraries.className("C:\\path\\to\\lib\\src\\some\\pkg\\Type.groovy", "C:\\path\\to\\lib\\src"), is("some.pkg.Type")); - assertThat(LibraryAdder.LoadedLibraries.className("C:\\path\\to\\Extra\\lib\\src\\some\\pkg\\Type.groovy", "C:\\path\\to\\Extra\\lib\\src"), is("some.pkg.Type")); - } - } From 894fbbbdd9525b719767d5199ffd0dc37b3d18ae Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:42:39 -0500 Subject: [PATCH 23/43] https://github.com/jenkinsci/workflow-cps-plugin/pull/672 released --- pom.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d43b7878..d8912a14 100644 --- a/pom.xml +++ b/pom.xml @@ -75,10 +75,11 @@ import pom + org.jenkins-ci.plugins.workflow workflow-cps - 3625.vb_50f7cb_e66b_7 + 3626.v4eb_a_7d8b_2fa_4 From 38e609ae0b06d822961193f016023711d037fcd2 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:51:02 -0500 Subject: [PATCH 24/43] Removing comment satisfied by 859dad14fa0893fae45aecbd5532ed178c9f0d87 --- .../org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java index 3169dcc0..4b35484f 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java @@ -542,6 +542,4 @@ public void parallelBuildsDontInterfereWithExpiredCache() throws Throwable { // r.assertLogContains("Library library@master is cached. Copying from home.", f2.get()); } - // TODO test migration in LibraryAdder.add after reload build started prior to dir2Jar (@OldData) - } From 2c181cc7148ad75fcfcc5ab5053c024040a883b8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:51:26 -0500 Subject: [PATCH 25/43] Fixing replay --- .../plugins/workflow/libs/LibraryAdder.java | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java index 7c259cd5..6a3c0b85 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java @@ -49,8 +49,10 @@ import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import java.io.OutputStream; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.regex.Matcher; @@ -266,18 +268,29 @@ static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever ret // Replace any classes requested for replay: if (!record.trusted) { - for (String clazz : ReplayAction.replacementsIn(execution)) { - String rel = clazz.replace('.', '/') + ".groovy"; - /* TODO need to unpack & repack I guess - FilePath f = libDir.child(rel); - if (f.exists()) { - String replacement = ReplayAction.replace(execution, clazz); - if (replacement != null) { - listener.getLogger().println("Replacing contents of " + rel); - f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler + Set clazzes = ReplayAction.replacementsIn(execution); + if (!clazzes.isEmpty()) { + FilePath tmp = libJar.withSuffix(".tmp"); + try { + libJar.unzip(tmp); + for (String clazz : clazzes) { + String rel = clazz.replace('.', '/') + ".groovy"; + FilePath f = tmp.child(rel); + if (f.exists()) { + String replacement = ReplayAction.replace(execution, clazz); + if (replacement != null) { + listener.getLogger().println("Replacing contents of " + rel); + f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler + } + } + } + libJar.delete(); + try (OutputStream os = libJar.write()) { + tmp.zip(os, "**"); } + } finally { + tmp.deleteRecursive(); } - */ } } try (JarFile jf = new JarFile(libJar.getRemote())) { From f5b08660d2c1f464a6910140cf71d73bfa42b4d2 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 14:58:01 -0500 Subject: [PATCH 26/43] Fixing `LibraryCachingConfigurationTest.clearCache` --- .../workflow/libs/LibraryCachingConfigurationTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java index 898806ab..67c47068 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java @@ -173,11 +173,11 @@ public void clearCache() throws Exception { WorkflowRun b = r.buildAndAssertSuccess(p); LibrariesAction action = b.getAction(LibrariesAction.class); LibraryRecord record = action.getLibraries().get(0); - FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName()); - assertThat(new File(cache.getRemote()), anExistingDirectory()); + FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName() + ".jar"); + assertThat(new File(cache.getRemote()), anExistingFile()); // Clear the cache. TODO: Would be more realistic to set up security and use WebClient. ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", false); - assertThat(new File(cache.getRemote()), not(anExistingDirectory())); + assertThat(new File(cache.getRemote()), not(anExistingFile())); } } From 80395ee31c7dc37ce4bb9acb6bd47e02f4d51193 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 15:00:19 -0500 Subject: [PATCH 27/43] `LibraryAdderTest.parallelBuildsDontInterfereWithExpiredCache` similarly needed to be fixed --- .../org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java index 4b35484f..495a9399 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryAdderTest.java @@ -528,7 +528,7 @@ public void parallelBuildsDontInterfereWithExpiredCache() throws Throwable { WorkflowRun b1 = r.buildAndAssertSuccess(p1); LibrariesAction action = b1.getAction(LibrariesAction.class); LibraryRecord record = action.getLibraries().get(0); - FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName()); + FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName() + ".jar"); //Expire the cache long oldMillis = ZonedDateTime.now().minusMinutes(35).toInstant().toEpochMilli(); cache.touch(oldMillis); From c2c711e5791a692131cc92ef2fd2554a4a99ff70 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 15:03:51 -0500 Subject: [PATCH 28/43] Similarly `ResourceStepTest.cachingRefresh` --- .../jenkinsci/plugins/workflow/libs/ResourceStepTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java index 5d0023cd..c91be0b7 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java @@ -271,9 +271,9 @@ public void clearCache(String name) throws Exception { public void modifyCacheTimestamp(String name, String version, long timestamp) throws Exception { String cacheDirName = LibraryRecord.directoryNameFor(name, version, String.valueOf(true), GlobalLibraries.ForJob.class.getName()); - FilePath cacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), cacheDirName); - if (cacheDir.exists()) { - cacheDir.touch(timestamp); + FilePath cacheJar = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), cacheDirName + ".jar"); + if (cacheJar.exists()) { + cacheJar.touch(timestamp); } } From 81ca5a6d609e2b9b0d8584ed99b05f6bd421d29d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 15:33:34 -0500 Subject: [PATCH 29/43] `symlinksInLibraryResourcesAreNotAllowedToEscapeWorkspaceContext` attack needed to be adjusted given that `lookForBadSymlinks` is now running from the workspace when packing the lib rather than when running the `libraryResource` step, so the required relative path is different --- .../org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java index c91be0b7..769ccf1b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/ResourceStepTest.java @@ -195,7 +195,7 @@ public class ResourceStepTest { Path resourcesDir = Paths.get(sampleRepo.getRoot().getPath(), "resources"); Files.createDirectories(resourcesDir); Path symlinkPath = Paths.get(resourcesDir.toString(), "master.key"); - Files.createSymbolicLink(symlinkPath, Paths.get("../../../../../../../secrets/master.key")); + Files.createSymbolicLink(symlinkPath, Paths.get("../../../../secrets/master.key")); sampleRepo.git("add", "src", "resources"); sampleRepo.git("commit", "--message=init"); From d1f3acafc179477de7bc322c2af853a8cb1ad170 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 15:42:14 -0500 Subject: [PATCH 30/43] SpotBugs --- .../plugins/workflow/libs/LibraryCachingCleanup.java | 2 +- .../jenkinsci/plugins/workflow/libs/LibraryRetriever.java | 8 +++++++- .../plugins/workflow/libs/SCMSourceRetriever.java | 8 +++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java index 71eb1ed0..064bacb9 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java @@ -40,7 +40,7 @@ public LibraryCachingCleanup() { */ private void removeIfExpiredCacheJar(FilePath libJar) throws IOException, InterruptedException { final FilePath lastReadFile = libJar.sibling(libJar.getBaseName() + "." + LibraryCachingConfiguration.LAST_READ_FILE); - if (lastReadFile.exists()) { + if (lastReadFile != null && lastReadFile.exists()) { ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libJar.getBaseName()); retrieveLock.writeLock().lockInterruptibly(); try { diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index 7dd13b54..9ce95634 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -71,12 +71,18 @@ public abstract class LibraryRetriever extends AbstractDescribableImpl run, @NonNull TaskListener listener) throws Exception { if (Util.isOverridden(LibraryRetriever.class, getClass(), "retrieve", String.class, String.class, boolean.class, FilePath.class, Run.class, TaskListener.class)) { FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); + if (tmp == null) { + throw new IOException(); + } try { retrieve(name, version, changelog, tmp, run, listener); dir2Jar(name, tmp, target, listener); } finally { tmp.deleteRecursive(); - WorkspaceList.tempDir(tmp).deleteRecursive(); + FilePath tmp2 = WorkspaceList.tempDir(tmp); + if (tmp2 != null) { + tmp2.deleteRecursive(); + } } } else { throw new AbstractMethodError("Implement retrieveJar"); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index d6888b05..8780aa8e 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -241,6 +241,9 @@ private static void doClone(@NonNull String name, @NonNull SCM scm, String libra delegate.setPoll(false); delegate.setChangelog(false); FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); + if (tmp == null) { + throw new IOException(); + } try { retrySCMOperation(listener, () -> { delegate.checkout(run, tmp, listener, Jenkins.get().createLauncher(listener)); @@ -258,7 +261,10 @@ private static void doClone(@NonNull String name, @NonNull SCM scm, String libra LibraryRetriever.dir2Jar(name, root, target, listener); } finally { tmp.deleteRecursive(); - WorkspaceList.tempDir(tmp).deleteRecursive(); + FilePath tmp2 = WorkspaceList.tempDir(tmp); + if (tmp2 != null) { + tmp2.deleteRecursive(); + } } } From 840cd6004501d8d9e52317288ebbbb86fddbd7a4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 16:15:51 -0500 Subject: [PATCH 31/43] `LibraryCachingCleanup` was unreliable as it sometimes deleted the last-read file rather than a JAR --- .../plugins/workflow/libs/LibraryCachingCleanup.java | 11 ++++++++--- .../workflow/libs/LibraryCachingCleanupTest.java | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java index 064bacb9..7c2d7d56 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java @@ -28,8 +28,8 @@ public LibraryCachingCleanup() { @Override protected void execute(TaskListener listener) throws IOException, InterruptedException { FilePath globalCacheDir = LibraryCachingConfiguration.getGlobalLibrariesCacheDir(); - for (FilePath libJar : globalCacheDir.list()) { - removeIfExpiredCacheJar(libJar); + for (FilePath libJar : globalCacheDir.list("*.jar")) { + removeIfExpiredCacheJar(libJar, listener); } // Old cache directory; format has changed, so just delete it: Jenkins.get().getRootPath().child("global-libraries-cache").deleteRecursive(); @@ -38,18 +38,23 @@ public LibraryCachingCleanup() { /** * Delete the specified cache JAR if it is outdated. */ - private void removeIfExpiredCacheJar(FilePath libJar) throws IOException, InterruptedException { + private void removeIfExpiredCacheJar(FilePath libJar, TaskListener listener) throws IOException, InterruptedException { final FilePath lastReadFile = libJar.sibling(libJar.getBaseName() + "." + LibraryCachingConfiguration.LAST_READ_FILE); if (lastReadFile != null && lastReadFile.exists()) { ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libJar.getBaseName()); retrieveLock.writeLock().lockInterruptibly(); try { if (System.currentTimeMillis() - lastReadFile.lastModified() > TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS)) { + listener.getLogger().println("Deleting " + libJar); libJar.delete(); + } else { + listener.getLogger().println(lastReadFile + " is sufficiently recent"); } } finally { retrieveLock.writeLock().unlock(); } + } else { + listener.getLogger().println("No such file " + lastReadFile); } } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java index 4ca0bf18..a212588c 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java @@ -27,7 +27,7 @@ import hudson.FilePath; import hudson.util.StreamTaskListener; import java.io.File; -import java.time.ZonedDateTime; +import java.util.concurrent.TimeUnit; import jenkins.plugins.git.GitSCMSource; import jenkins.plugins.git.GitSampleRepoRule; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; @@ -72,7 +72,7 @@ public void smokes() throws Throwable { ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr()); assertThat(new File(cache.getRemote()), anExistingFile()); // Run LibraryCachingCleanup after modifying LAST_READ_FILE to be an old date and and show that cache is deleted. - long oldMillis = ZonedDateTime.now().minusDays(LibraryCachingCleanup.EXPIRE_AFTER_READ_DAYS + 1).toInstant().toEpochMilli(); + long oldMillis = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(LibraryCachingCleanup.EXPIRE_AFTER_READ_DAYS + 1); cache.sibling(cache.getBaseName() + "." + LibraryCachingConfiguration.LAST_READ_FILE).touch(oldMillis); ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr()); assertThat(new File(cache.getRemote()), not(anExistingFile())); From 0aa22f4d02d728ff52d5244b8648a674594279e4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 16:34:45 -0500 Subject: [PATCH 32/43] Merged `doClone` back into `doRetrieve` for clarity --- .../plugins/workflow/libs/SCMRetriever.java | 2 +- .../workflow/libs/SCMSourceRetriever.java | 124 +++++++----------- 2 files changed, 51 insertions(+), 75 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java index 071264bf..5e8df48d 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java @@ -89,7 +89,7 @@ public void setLibraryPath(String libraryPath) { } @Override public void retrieveJar(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { - SCMSourceRetriever.doRetrieve(name, changelog, scm, libraryPath, target, run, listener); + SCMSourceRetriever.doRetrieve(name, changelog, scm, libraryPath, target, run, listener, false); // TODO support clone } @Override public FormValidation validateVersion(String name, String version, Item context) { diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index 8780aa8e..252d618a 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -60,14 +60,11 @@ import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.CheckForNull; -import java.util.Set; -import java.util.TreeSet; import jenkins.model.Jenkins; import jenkins.scm.api.SCMRevision; import jenkins.scm.api.SCMSource; import jenkins.scm.api.SCMSourceDescriptor; import java.util.regex.Pattern; -import java.util.stream.Collectors; import org.jenkinsci.Symbol; import org.jenkinsci.plugins.structs.describable.CustomDescribableModel; import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable; @@ -143,18 +140,18 @@ public String getLibraryPath() { } @Override public void retrieveJar(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { + if (libraryPath != null && PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { + throw new AbortException("Library path may not contain '..'"); + } SCMRevision revision = retrySCMOperation(listener, () -> scm.fetch(version, listener, run.getParent())); if (revision == null) { throw new AbortException("No version " + version + " found for library " + name); } - if (clone) { - if (changelog) { - listener.getLogger().println("WARNING: ignoring request to compute changelog in clone mode"); - } - doClone(name, scm.build(revision.getHead(), revision), libraryPath, target, run, listener); - } else { - doRetrieve(name, changelog, scm.build(revision.getHead(), revision), libraryPath, target, run, listener); + if (clone && changelog) { + listener.getLogger().println("WARNING: ignoring request to compute changelog in clone mode"); + changelog = false; } + doRetrieve(name, changelog, scm.build(revision.getHead(), revision), libraryPath, target, run, listener, clone); } private static T retrySCMOperation(TaskListener listener, Callable task) throws Exception{ @@ -188,42 +185,57 @@ private static T retrySCMOperation(TaskListener listener, Callable task) return ret; } - static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener) throws Exception { + static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener, boolean clone) throws Exception { // Adapted from CpsScmFlowDefinition: SCMStep delegate = new GenericSCMStep(scm); delegate.setPoll(false); // TODO we have no API for determining if a given SCMHead is branch-like or tag-like; would we want to turn on polling if the former? delegate.setChangelog(changelog); - FilePath dir; Node node = Jenkins.get(); - if (run.getParent() instanceof TopLevelItem) { - FilePath baseWorkspace = node.getWorkspaceFor((TopLevelItem) run.getParent()); - if (baseWorkspace == null) { + if (clone) { + FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); + if (tmp == null) { + throw new IOException(); + } + try { + retrySCMOperation(listener, () -> { + delegate.checkout(run, tmp, listener, node.createLauncher(listener)); + return null; + }); + LibraryRetriever.dir2Jar(name, libraryPath != null ? tmp.child(libraryPath) : tmp, target, listener); + } finally { + tmp.deleteRecursive(); + FilePath tmp2 = WorkspaceList.tempDir(tmp); + if (tmp2 != null) { + tmp2.deleteRecursive(); + } + } + } else { + FilePath dir; + if (run.getParent() instanceof TopLevelItem) { + FilePath baseWorkspace = node.getWorkspaceFor((TopLevelItem) run.getParent()); + if (baseWorkspace == null) { + throw new IOException(node.getDisplayName() + " may be offline"); + } + String checkoutDirName = LibraryRecord.directoryNameFor(scm.getKey()); + dir = baseWorkspace.withSuffix(getFilePathSuffix() + "libs").child(checkoutDirName); + } else { // should not happen, but just in case: + throw new AbortException("Cannot check out in non-top-level build"); + } + Computer computer = node.toComputer(); + if (computer == null) { throw new IOException(node.getDisplayName() + " may be offline"); } - String checkoutDirName = LibraryRecord.directoryNameFor(scm.getKey()); - dir = baseWorkspace.withSuffix(getFilePathSuffix() + "libs").child(checkoutDirName); - } else { // should not happen, but just in case: - throw new AbortException("Cannot check out in non-top-level build"); - } - Computer computer = node.toComputer(); - if (computer == null) { - throw new IOException(node.getDisplayName() + " may be offline"); - } - try (WorkspaceList.Lease lease = computer.getWorkspaceList().allocate(dir)) { - // Write the SCM key to a file as a debugging aid. - lease.path.withSuffix("-scm-key.txt").write(scm.getKey(), "UTF-8"); - retrySCMOperation(listener, () -> { - delegate.checkout(run, lease.path, listener, node.createLauncher(listener)); - return null; - }); - if (libraryPath == null) { - libraryPath = "."; - } else if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { - throw new AbortException("Library path may not contain '..'"); + try (WorkspaceList.Lease lease = computer.getWorkspaceList().allocate(dir)) { + // Write the SCM key to a file as a debugging aid. + lease.path.withSuffix("-scm-key.txt").write(scm.getKey(), "UTF-8"); + retrySCMOperation(listener, () -> { + delegate.checkout(run, lease.path, listener, node.createLauncher(listener)); + return null; + }); + // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? + // Copy sources with relevant files from the checkout: + LibraryRetriever.dir2Jar(name, libraryPath != null ? lease.path.child(libraryPath) : lease.path, target, listener); } - // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? - // Copy sources with relevant files from the checkout: - LibraryRetriever.dir2Jar(name, lease.path.child(libraryPath), target, listener); } } @@ -232,42 +244,6 @@ private static String getFilePathSuffix() { return System.getProperty(WorkspaceList.class.getName(), "@"); } - /** - * Similar to {@link #doRetrieve} but used in {@link #clone} mode. - */ - private static void doClone(@NonNull String name, @NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener) throws Exception { - // TODO merge into doRetrieve - SCMStep delegate = new GenericSCMStep(scm); - delegate.setPoll(false); - delegate.setChangelog(false); - FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); - if (tmp == null) { - throw new IOException(); - } - try { - retrySCMOperation(listener, () -> { - delegate.checkout(run, tmp, listener, Jenkins.get().createLauncher(listener)); - return null; - }); - FilePath root; - if (libraryPath == null) { - root = tmp; - } else { - if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { - throw new AbortException("Library path may not contain '..'"); - } - root = tmp.child(libraryPath); - } - LibraryRetriever.dir2Jar(name, root, target, listener); - } finally { - tmp.deleteRecursive(); - FilePath tmp2 = WorkspaceList.tempDir(tmp); - if (tmp2 != null) { - tmp2.deleteRecursive(); - } - } - } - @Override public FormValidation validateVersion(String name, String version, Item context) { StringWriter w = new StringWriter(); try { From 743db17cc8b96b23167d24e59a6d8fd9a01f724a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 17:19:23 -0500 Subject: [PATCH 33/43] Introduced `SCMBasedRetriever`, permitting `SCMRetriever` to support `clone` --- .../workflow/libs/LibraryRetriever.java | 6 +- .../workflow/libs/SCMBasedRetriever.java | 281 ++++++++++++++++++ .../plugins/workflow/libs/SCMRetriever.java | 38 +-- .../workflow/libs/SCMSourceRetriever.java | 244 +-------------- .../help-clone.html | 0 .../help-libraryPath.html | 0 .../libs/SCMBasedRetriever/options.jelly | 34 +++ .../libs/SCMBasedRetriever/options.properties | 23 ++ .../workflow/libs/SCMRetriever/config.jelly | 6 +- .../libs/SCMSourceRetriever/config.jelly | 9 +- .../libs/SCMSourceRetriever/config.properties | 1 - .../SCMSourceRetriever/help-libraryPath.html | 5 - .../workflow/libs/SCMSourceRetrieverTest.java | 8 +- 13 files changed, 357 insertions(+), 298 deletions(-) create mode 100644 src/main/java/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever.java rename src/main/resources/org/jenkinsci/plugins/workflow/libs/{SCMSourceRetriever => SCMBasedRetriever}/help-clone.html (100%) rename src/main/resources/org/jenkinsci/plugins/workflow/libs/{SCMRetriever => SCMBasedRetriever}/help-libraryPath.html (100%) create mode 100644 src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.jelly create mode 100644 src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.properties delete mode 100644 src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/help-libraryPath.html diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java index 9ce95634..3b601002 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java @@ -35,6 +35,7 @@ import hudson.util.FormValidation; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.slaves.WorkspaceList; import hudson.util.DirScanner; import hudson.util.FileVisitor; @@ -58,6 +59,9 @@ public abstract class LibraryRetriever extends AbstractDescribableImplUsed to prevent {@link #libraryPath} from being used for directory traversal. + */ + static final Pattern PROHIBITED_DOUBLE_DOT = Pattern.compile("(^|.*[\\\\/])\\.\\.($|[\\\\/].*)"); + + private boolean clone; + + /** + * The path to the library inside of the SCM. + * + * {@code null} is the default and means that the library is in the root of the repository. Otherwise, the value is + * considered to be a relative path inside of the repository and always ends in a forward slash + * + * @see #setLibraryPath + */ + private @CheckForNull String libraryPath; + + public boolean isClone() { + return clone; + } + + @DataBoundSetter public void setClone(boolean clone) { + this.clone = clone; + } + + public String getLibraryPath() { + return libraryPath; + } + + @DataBoundSetter public void setLibraryPath(String libraryPath) { + libraryPath = Util.fixEmptyAndTrim(libraryPath); + if (libraryPath != null && !libraryPath.endsWith("/")) { + libraryPath += '/'; + } + this.libraryPath = libraryPath; + } + + protected final void doRetrieve(String name, boolean changelog, @NonNull SCM scm, FilePath target, Run run, TaskListener listener) throws Exception { + if (libraryPath != null && PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { + throw new AbortException("Library path may not contain '..'"); + } + if (clone && changelog) { + listener.getLogger().println("WARNING: ignoring request to compute changelog in clone mode"); + changelog = false; + } + // Adapted from CpsScmFlowDefinition: + SCMStep delegate = new GenericSCMStep(scm); + delegate.setPoll(false); // TODO we have no API for determining if a given SCMHead is branch-like or tag-like; would we want to turn on polling if the former? + delegate.setChangelog(changelog); + Node node = Jenkins.get(); + if (clone) { + FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); + if (tmp == null) { + throw new IOException(); + } + try { + retrySCMOperation(listener, () -> { + delegate.checkout(run, tmp, listener, node.createLauncher(listener)); + return null; + }); + LibraryRetriever.dir2Jar(name, libraryPath != null ? tmp.child(libraryPath) : tmp, target, listener); + } finally { + tmp.deleteRecursive(); + FilePath tmp2 = WorkspaceList.tempDir(tmp); + if (tmp2 != null) { + tmp2.deleteRecursive(); + } + } + } else { + FilePath dir; + if (run.getParent() instanceof TopLevelItem) { + FilePath baseWorkspace = node.getWorkspaceFor((TopLevelItem) run.getParent()); + if (baseWorkspace == null) { + throw new IOException(node.getDisplayName() + " may be offline"); + } + String checkoutDirName = LibraryRecord.directoryNameFor(scm.getKey()); + dir = baseWorkspace.withSuffix(getFilePathSuffix() + "libs").child(checkoutDirName); + } else { // should not happen, but just in case: + throw new AbortException("Cannot check out in non-top-level build"); + } + Computer computer = node.toComputer(); + if (computer == null) { + throw new IOException(node.getDisplayName() + " may be offline"); + } + try (WorkspaceList.Lease lease = computer.getWorkspaceList().allocate(dir)) { + // Write the SCM key to a file as a debugging aid. + lease.path.withSuffix("-scm-key.txt").write(scm.getKey(), "UTF-8"); + retrySCMOperation(listener, () -> { + delegate.checkout(run, lease.path, listener, node.createLauncher(listener)); + return null; + }); + // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? + // Copy sources with relevant files from the checkout: + LibraryRetriever.dir2Jar(name, libraryPath != null ? lease.path.child(libraryPath) : lease.path, target, listener); + } + } + } + + protected static T retrySCMOperation(TaskListener listener, Callable task) throws Exception{ + T ret = null; + for (int retryCount = Jenkins.get().getScmCheckoutRetryCount(); retryCount >= 0; retryCount--) { + try { + ret = task.call(); + break; + } + catch (AbortException e) { + // abort exception might have a null message. + // If so, just skip echoing it. + if (e.getMessage() != null) { + listener.error(e.getMessage()); + } + } + catch (InterruptedIOException e) { + throw e; + } + catch (Exception e) { + // checkout error not yet reported + Functions.printStackTrace(e, listener.error("Checkout failed")); + } + + if (retryCount == 0) // all attempts failed + throw new AbortException("Maximum checkout retry attempts reached, aborting"); + + listener.getLogger().println("Retrying after 10 seconds"); + Thread.sleep(10000); + } + return ret; + } + + // TODO there is WorkspaceList.tempDir but no API to make other variants + protected static String getFilePathSuffix() { + return System.getProperty(WorkspaceList.class.getName(), "@"); + } + + protected abstract static class SCMBasedRetrieverDescriptor extends LibraryRetrieverDescriptor { + + @POST + public FormValidation doCheckLibraryPath(@QueryParameter String libraryPath) { + libraryPath = Util.fixEmptyAndTrim(libraryPath); + if (libraryPath == null) { + return FormValidation.ok(); + } else if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { + return FormValidation.error(Messages.SCMSourceRetriever_library_path_no_double_dot()); + } + return FormValidation.ok(); + } + + } + + @Restricted(DoNotUse.class) + @Extension + public static class WorkspaceListener extends ItemListener { + + @Override + public void onDeleted(Item item) { + deleteLibsDir(item, item.getFullName()); + } + + @Override + public void onLocationChanged(Item item, String oldFullName, String newFullName) { + deleteLibsDir(item, oldFullName); + } + + private static void deleteLibsDir(Item item, String itemFullName) { + if (item instanceof Job + && item.getClass() + .getName() + .equals("org.jenkinsci.plugins.workflow.job.WorkflowJob")) { + synchronized (item) { + String base = + expandVariablesForDirectory( + Jenkins.get().getRawWorkspaceDir(), + itemFullName, + item.getRootDir().getPath()); + FilePath dir = + new FilePath(new File(base)).withSuffix(getFilePathSuffix() + "libs"); + try { + if (dir.isDirectory()) { + LOGGER.log( + Level.INFO, + () -> "Deleting obsolete library workspace " + dir); + dir.deleteRecursive(); + } + } catch (IOException | InterruptedException e) { + LOGGER.log( + Level.WARNING, + e, + () -> "Could not delete obsolete library workspace " + dir); + } + } + } + } + + private static String expandVariablesForDirectory( + String base, String itemFullName, String itemRootDir) { + // If the item is moved, it is too late to look up its original workspace location by + // the time we get the notification. See: + // https://github.com/jenkinsci/jenkins/blob/f03183ab09ce5fb8f9f4cc9ccee42a3c3e6b2d3e/core/src/main/java/jenkins/model/Jenkins.java#L2567-L2576 + Map properties = new HashMap<>(); + properties.put("JENKINS_HOME", Jenkins.get().getRootDir().getPath()); + properties.put("ITEM_ROOTDIR", itemRootDir); + properties.put("ITEM_FULLNAME", itemFullName); // legacy, deprecated + properties.put( + "ITEM_FULL_NAME", itemFullName.replace(':', '$')); // safe, see JENKINS-12251 + return Util.replaceMacro(base, Collections.unmodifiableMap(properties)); + } + } + +} diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java index 5e8df48d..8a9f1a19 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMRetriever.java @@ -39,34 +39,20 @@ import hudson.util.FormValidation; import java.util.ArrayList; import java.util.List; -import edu.umd.cs.findbugs.annotations.CheckForNull; import jenkins.model.Jenkins; import org.jenkinsci.Symbol; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.DoNotUse; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.DataBoundConstructor; -import org.kohsuke.stapler.DataBoundSetter; -import org.kohsuke.stapler.QueryParameter; -import org.kohsuke.stapler.verb.POST; /** * Uses legacy {@link SCM} to check out sources based on variable interpolation. */ -public class SCMRetriever extends LibraryRetriever { +public class SCMRetriever extends SCMBasedRetriever { private final SCM scm; - /** - * The path to the library inside of the SCM. - * - * {@code null} is the default and means that the library is in the root of the repository. Otherwise, the value is - * considered to be a relative path inside of the repository and always ends in a forward slash - * - * @see #setLibraryPath - */ - private @CheckForNull String libraryPath; - @DataBoundConstructor public SCMRetriever(SCM scm) { this.scm = scm; } @@ -75,21 +61,8 @@ public SCM getScm() { return scm; } - public String getLibraryPath() { - return libraryPath; - } - - @DataBoundSetter - public void setLibraryPath(String libraryPath) { - libraryPath = Util.fixEmptyAndTrim(libraryPath); - if (libraryPath != null && !libraryPath.endsWith("/")) { - libraryPath += '/'; - } - this.libraryPath = libraryPath; - } - @Override public void retrieveJar(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { - SCMSourceRetriever.doRetrieve(name, changelog, scm, libraryPath, target, run, listener, false); // TODO support clone + doRetrieve(name, changelog, scm, target, run, listener); } @Override public FormValidation validateVersion(String name, String version, Item context) { @@ -101,12 +74,7 @@ public void setLibraryPath(String libraryPath) { } @Symbol("legacySCM") - @Extension(ordinal=-100) public static class DescriptorImpl extends LibraryRetrieverDescriptor { - - @POST - public FormValidation doCheckLibraryPath(@QueryParameter String libraryPath) { - return SCMSourceRetriever.DescriptorImpl.checkLibraryPath(libraryPath); - } + @Extension(ordinal=-100) public static class DescriptorImpl extends SCMBasedRetrieverDescriptor { @Override public String getDisplayName() { return "Legacy SCM"; diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java index 252d618a..fe0a10c3 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java @@ -24,90 +24,42 @@ package org.jenkinsci.plugins.workflow.libs; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.Extension; import hudson.ExtensionList; import hudson.FilePath; -import hudson.Functions; import hudson.Util; -import hudson.model.Computer; import hudson.model.Descriptor; import hudson.model.DescriptorVisibilityFilter; import hudson.model.Item; -import hudson.model.Job; -import hudson.model.Node; import hudson.model.Run; import hudson.model.TaskListener; -import hudson.model.TopLevelItem; -import hudson.model.listeners.ItemListener; -import hudson.scm.SCM; -import hudson.slaves.WorkspaceList; import hudson.util.FormValidation; import hudson.util.StreamTaskListener; -import java.io.File; -import java.io.IOException; -import java.io.InterruptedIOException; import java.io.StringWriter; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.Callable; -import java.util.logging.Level; -import java.util.logging.Logger; -import edu.umd.cs.findbugs.annotations.NonNull; -import edu.umd.cs.findbugs.annotations.CheckForNull; -import jenkins.model.Jenkins; import jenkins.scm.api.SCMRevision; import jenkins.scm.api.SCMSource; import jenkins.scm.api.SCMSourceDescriptor; -import java.util.regex.Pattern; import org.jenkinsci.Symbol; import org.jenkinsci.plugins.structs.describable.CustomDescribableModel; import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable; -import org.jenkinsci.plugins.workflow.steps.scm.GenericSCMStep; -import org.jenkinsci.plugins.workflow.steps.scm.SCMStep; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.DoNotUse; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.DataBoundConstructor; -import org.kohsuke.stapler.DataBoundSetter; -import org.kohsuke.stapler.QueryParameter; -import org.kohsuke.stapler.verb.POST; /** * Uses {@link SCMSource#fetch(String, TaskListener)} to retrieve a specific revision. */ -public class SCMSourceRetriever extends LibraryRetriever { - - private static final Logger LOGGER = Logger.getLogger(SCMSourceRetriever.class.getName()); - - @SuppressFBWarnings(value="MS_SHOULD_BE_FINAL", justification="Non-final for write access via the Script Console") - public static boolean INCLUDE_SRC_TEST_IN_LIBRARIES = Boolean.getBoolean(SCMSourceRetriever.class.getName() + ".INCLUDE_SRC_TEST_IN_LIBRARIES"); - /** - * Matches ".." in positions where it would be treated as the parent directory. - * - *

Used to prevent {@link #libraryPath} from being used for directory traversal. - */ - static final Pattern PROHIBITED_DOUBLE_DOT = Pattern.compile("(^|.*[\\\\/])\\.\\.($|[\\\\/].*)"); +public class SCMSourceRetriever extends SCMBasedRetriever { private final SCMSource scm; - private boolean clone; - - /** - * The path to the library inside of the SCM. - * - * {@code null} is the default and means that the library is in the root of the repository. Otherwise, the value is - * considered to be a relative path inside of the repository and always ends in a forward slash - * - * @see #setLibraryPath - */ - private @CheckForNull String libraryPath; - @DataBoundConstructor public SCMSourceRetriever(SCMSource scm) { this.scm = scm; } @@ -119,129 +71,12 @@ public SCMSource getScm() { return scm; } - public boolean isClone() { - return clone; - } - - @DataBoundSetter public void setClone(boolean clone) { - this.clone = clone; - } - - public String getLibraryPath() { - return libraryPath; - } - - @DataBoundSetter public void setLibraryPath(String libraryPath) { - libraryPath = Util.fixEmptyAndTrim(libraryPath); - if (libraryPath != null && !libraryPath.endsWith("/")) { - libraryPath += '/'; - } - this.libraryPath = libraryPath; - } - @Override public void retrieveJar(String name, String version, boolean changelog, FilePath target, Run run, TaskListener listener) throws Exception { - if (libraryPath != null && PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { - throw new AbortException("Library path may not contain '..'"); - } SCMRevision revision = retrySCMOperation(listener, () -> scm.fetch(version, listener, run.getParent())); if (revision == null) { throw new AbortException("No version " + version + " found for library " + name); } - if (clone && changelog) { - listener.getLogger().println("WARNING: ignoring request to compute changelog in clone mode"); - changelog = false; - } - doRetrieve(name, changelog, scm.build(revision.getHead(), revision), libraryPath, target, run, listener, clone); - } - - private static T retrySCMOperation(TaskListener listener, Callable task) throws Exception{ - T ret = null; - for (int retryCount = Jenkins.get().getScmCheckoutRetryCount(); retryCount >= 0; retryCount--) { - try { - ret = task.call(); - break; - } - catch (AbortException e) { - // abort exception might have a null message. - // If so, just skip echoing it. - if (e.getMessage() != null) { - listener.error(e.getMessage()); - } - } - catch (InterruptedIOException e) { - throw e; - } - catch (Exception e) { - // checkout error not yet reported - Functions.printStackTrace(e, listener.error("Checkout failed")); - } - - if (retryCount == 0) // all attempts failed - throw new AbortException("Maximum checkout retry attempts reached, aborting"); - - listener.getLogger().println("Retrying after 10 seconds"); - Thread.sleep(10000); - } - return ret; - } - - static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String libraryPath, FilePath target, Run run, TaskListener listener, boolean clone) throws Exception { - // Adapted from CpsScmFlowDefinition: - SCMStep delegate = new GenericSCMStep(scm); - delegate.setPoll(false); // TODO we have no API for determining if a given SCMHead is branch-like or tag-like; would we want to turn on polling if the former? - delegate.setChangelog(changelog); - Node node = Jenkins.get(); - if (clone) { - FilePath tmp = target.sibling(target.getBaseName() + "-checkout"); - if (tmp == null) { - throw new IOException(); - } - try { - retrySCMOperation(listener, () -> { - delegate.checkout(run, tmp, listener, node.createLauncher(listener)); - return null; - }); - LibraryRetriever.dir2Jar(name, libraryPath != null ? tmp.child(libraryPath) : tmp, target, listener); - } finally { - tmp.deleteRecursive(); - FilePath tmp2 = WorkspaceList.tempDir(tmp); - if (tmp2 != null) { - tmp2.deleteRecursive(); - } - } - } else { - FilePath dir; - if (run.getParent() instanceof TopLevelItem) { - FilePath baseWorkspace = node.getWorkspaceFor((TopLevelItem) run.getParent()); - if (baseWorkspace == null) { - throw new IOException(node.getDisplayName() + " may be offline"); - } - String checkoutDirName = LibraryRecord.directoryNameFor(scm.getKey()); - dir = baseWorkspace.withSuffix(getFilePathSuffix() + "libs").child(checkoutDirName); - } else { // should not happen, but just in case: - throw new AbortException("Cannot check out in non-top-level build"); - } - Computer computer = node.toComputer(); - if (computer == null) { - throw new IOException(node.getDisplayName() + " may be offline"); - } - try (WorkspaceList.Lease lease = computer.getWorkspaceList().allocate(dir)) { - // Write the SCM key to a file as a debugging aid. - lease.path.withSuffix("-scm-key.txt").write(scm.getKey(), "UTF-8"); - retrySCMOperation(listener, () -> { - delegate.checkout(run, lease.path, listener, node.createLauncher(listener)); - return null; - }); - // Cannot add WorkspaceActionImpl to private CpsFlowExecution.flowStartNodeActions; do we care? - // Copy sources with relevant files from the checkout: - LibraryRetriever.dir2Jar(name, libraryPath != null ? lease.path.child(libraryPath) : lease.path, target, listener); - } - } - } - - // TODO there is WorkspaceList.tempDir but no API to make other variants - private static String getFilePathSuffix() { - return System.getProperty(WorkspaceList.class.getName(), "@"); + doRetrieve(name, changelog, scm.build(revision.getHead(), revision), target, run, listener); } @Override public FormValidation validateVersion(String name, String version, Item context) { @@ -262,22 +97,7 @@ private static String getFilePathSuffix() { } @Symbol("modernSCM") - @Extension public static class DescriptorImpl extends LibraryRetrieverDescriptor implements CustomDescribableModel { - - static FormValidation checkLibraryPath(@QueryParameter String libraryPath) { - libraryPath = Util.fixEmptyAndTrim(libraryPath); - if (libraryPath == null) { - return FormValidation.ok(); - } else if (PROHIBITED_DOUBLE_DOT.matcher(libraryPath).matches()) { - return FormValidation.error(Messages.SCMSourceRetriever_library_path_no_double_dot()); - } - return FormValidation.ok(); - } - - @POST - public FormValidation doCheckLibraryPath(@QueryParameter String libraryPath) { - return checkLibraryPath(libraryPath); - } + @Extension public static class DescriptorImpl extends SCMBasedRetrieverDescriptor implements CustomDescribableModel { @Override public String getDisplayName() { return "Modern SCM"; @@ -326,62 +146,4 @@ public Collection getSCMDescriptors() { } - @Restricted(DoNotUse.class) - @Extension - public static class WorkspaceListener extends ItemListener { - - @Override - public void onDeleted(Item item) { - deleteLibsDir(item, item.getFullName()); - } - - @Override - public void onLocationChanged(Item item, String oldFullName, String newFullName) { - deleteLibsDir(item, oldFullName); - } - - private static void deleteLibsDir(Item item, String itemFullName) { - if (item instanceof Job - && item.getClass() - .getName() - .equals("org.jenkinsci.plugins.workflow.job.WorkflowJob")) { - synchronized (item) { - String base = - expandVariablesForDirectory( - Jenkins.get().getRawWorkspaceDir(), - itemFullName, - item.getRootDir().getPath()); - FilePath dir = - new FilePath(new File(base)).withSuffix(getFilePathSuffix() + "libs"); - try { - if (dir.isDirectory()) { - LOGGER.log( - Level.INFO, - () -> "Deleting obsolete library workspace " + dir); - dir.deleteRecursive(); - } - } catch (IOException | InterruptedException e) { - LOGGER.log( - Level.WARNING, - e, - () -> "Could not delete obsolete library workspace " + dir); - } - } - } - } - - private static String expandVariablesForDirectory( - String base, String itemFullName, String itemRootDir) { - // If the item is moved, it is too late to look up its original workspace location by - // the time we get the notification. See: - // https://github.com/jenkinsci/jenkins/blob/f03183ab09ce5fb8f9f4cc9ccee42a3c3e6b2d3e/core/src/main/java/jenkins/model/Jenkins.java#L2567-L2576 - Map properties = new HashMap<>(); - properties.put("JENKINS_HOME", Jenkins.get().getRootDir().getPath()); - properties.put("ITEM_ROOTDIR", itemRootDir); - properties.put("ITEM_FULLNAME", itemFullName); // legacy, deprecated - properties.put( - "ITEM_FULL_NAME", itemFullName.replace(':', '$')); // safe, see JENKINS-12251 - return Util.replaceMacro(base, Collections.unmodifiableMap(properties)); - } - } } diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/help-clone.html b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html similarity index 100% rename from src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/help-clone.html rename to src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/help-libraryPath.html b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-libraryPath.html similarity index 100% rename from src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/help-libraryPath.html rename to src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-libraryPath.html diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.jelly b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.jelly new file mode 100644 index 00000000..97b6739e --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.jelly @@ -0,0 +1,34 @@ + + + + + + + + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.properties b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.properties new file mode 100644 index 00000000..5afdfe40 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/options.properties @@ -0,0 +1,23 @@ +# The MIT License +# +# Copyright 2020 CloudBees, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +libraryPath=Library Path (optional) diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly index d7489eb1..6f89b7ae 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly @@ -24,10 +24,8 @@ THE SOFTWARE. --> - + ${%blurb} - - - + diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly index 331ea146..6f89b7ae 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly @@ -24,13 +24,8 @@ THE SOFTWARE. --> - + ${%blurb} - - - - - - + diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.properties b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.properties index 85d2d1d8..16109f36 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.properties +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.properties @@ -20,7 +20,6 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. -libraryPath=Library Path (optional) blurb=\ Loads a library from an SCM plugin using newer interfaces optimized for this purpose. \ The recommended option when available. diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/help-libraryPath.html b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/help-libraryPath.html deleted file mode 100644 index acbba272..00000000 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/help-libraryPath.html +++ /dev/null @@ -1,5 +0,0 @@ -

- A relative path from the root of the SCM to the root of the library. - Leave this field blank if the root of the library is the root of the SCM. - Note that ".." is not permitted as a path component to avoid security issues. -
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index 64d98f39..d0c6e7a3 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -83,7 +83,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.nullValue; -import static org.jenkinsci.plugins.workflow.libs.SCMSourceRetriever.PROHIBITED_DOUBLE_DOT; +import static org.jenkinsci.plugins.workflow.libs.SCMBasedRetriever.PROHIBITED_DOUBLE_DOT; import static org.junit.Assume.assumeFalse; import org.jvnet.hudson.test.FlagRule; @@ -93,7 +93,7 @@ public class SCMSourceRetrieverTest { @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); @Rule public SubversionSampleRepoRule sampleRepoSvn = new SubversionSampleRepoRule(); - @Rule public FlagRule includeSrcTest = new FlagRule<>(() -> SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES, v -> SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES = v); + @Rule public FlagRule includeSrcTest = new FlagRule<>(() -> LibraryRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES, v -> LibraryRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES = v); @Issue("JENKINS-40408") @Test public void lease() throws Exception { @@ -443,7 +443,7 @@ public static class BasicSCMSource extends SCMSource { GlobalLibraries.get().setLibraries(Collections.singletonList(lc)); WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("@Library('echoing@master') import myecho; myecho()", true)); - SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES = false; + LibraryRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES = false; WorkflowRun b = r.buildAndAssertSuccess(p); assertFalse(r.jenkins.getWorkspaceFor(p).withSuffix("@libs").isDirectory()); r.assertLogContains("something special", b); @@ -464,7 +464,7 @@ public static class BasicSCMSource extends SCMSource { GlobalLibraries.get().setLibraries(Collections.singletonList(lc)); WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("@Library('echoing@master') import myecho; myecho()", true)); - SCMSourceRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES = true; + LibraryRetriever.INCLUDE_SRC_TEST_IN_LIBRARIES = true; WorkflowRun b = r.buildAndAssertSuccess(p); assertFalse(r.jenkins.getWorkspaceFor(p).withSuffix("@libs").isDirectory()); r.assertLogContains("got something special", b); From 444bde7967e0fa5ad9242bb1c2e08a073b49f21e Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 19:00:10 -0500 Subject: [PATCH 34/43] Also need `class="${descriptor.clazz}"` --- .../jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly | 2 +- .../plugins/workflow/libs/SCMSourceRetriever/config.jelly | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly index 6f89b7ae..d1b4cb02 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMRetriever/config.jelly @@ -27,5 +27,5 @@ THE SOFTWARE. ${%blurb} - + diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly index 6f89b7ae..d1b4cb02 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever/config.jelly @@ -27,5 +27,5 @@ THE SOFTWARE. ${%blurb} - + From 992eeaa014ba1627d7bc93eafb6b14c0ee50b677 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Mar 2023 19:03:26 -0500 Subject: [PATCH 35/43] Skip new symlink unit tests on Windows --- .../plugins/workflow/libs/LibraryRetrieverTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java index 50b69947..6958ec6b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java @@ -25,7 +25,7 @@ package org.jenkinsci.plugins.workflow.libs; import hudson.FilePath; -import hudson.model.TaskListener; +import hudson.Functions; import hudson.util.StreamTaskListener; import java.nio.charset.StandardCharsets; import java.util.Set; @@ -37,6 +37,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThrows; +import static org.junit.Assume.assumeFalse; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -62,6 +63,7 @@ public class LibraryRetrieverTest { } @Test public void safeSymlinks() throws Exception { + assumeFalse(Functions.isWindows()); FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); dir.child("vars/x.groovy").write("content", null); @@ -76,6 +78,7 @@ public class LibraryRetrieverTest { } @Test public void unsafeSymlinks() throws Exception { + assumeFalse(Functions.isWindows()); FilePath work = new FilePath(tmp.newFolder()); FilePath dir = work.child("dir"); dir.child("vars/x.groovy").write("content", null); From b7bd7741e05e751584afd78b1dc6f47dd06d4129 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 7 Mar 2023 06:49:08 -0500 Subject: [PATCH 36/43] Pick up https://github.com/jenkinsci/workflow-cps-plugin/pull/673 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d8912a14..b65a8786 100644 --- a/pom.xml +++ b/pom.xml @@ -79,7 +79,7 @@ org.jenkins-ci.plugins.workflow workflow-cps - 3626.v4eb_a_7d8b_2fa_4 + 3630.vf8b_1e63e3d03 From 10623ddba466c9a5c0f059d6a98cff9bcbb09deb Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 8 Mar 2023 13:44:02 -0500 Subject: [PATCH 37/43] Also recommend `CloneOption.noTags` --- .../plugins/workflow/libs/SCMBasedRetriever/help-clone.html | 4 +++- .../plugins/workflow/libs/SCMSourceRetrieverTest.java | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html index 2f375430..939f3e6b 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html @@ -1,6 +1,8 @@
If checked, every build performs a fresh clone of the SCM rather than locking and updating a common copy. No changelog will be computed. - For Git, you are advised to select Advanced clone behaviors » Shallow clone to make the clone much faster. + For Git, you are advised to add Advanced clone behaviors + and then check Shallow clone and uncheck Fetch tags + to make the clone much faster. You may still enable Cache fetched versions on controller for quick retrieval if you prefer.
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index d0c6e7a3..899b2f69 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -379,7 +379,7 @@ public static class BasicSCMSource extends SCMSource { sampleRepo.git("add", "."); sampleRepo.git("commit", "--message=init"); GitSCMSource src = new GitSCMSource(sampleRepo.toString()); - src.setTraits(List.of(new CloneOptionTrait(new CloneOption(true, null, null)))); + src.setTraits(List.of(new CloneOptionTrait(new CloneOption(true, true, null, null)))); SCMSourceRetriever scm = new SCMSourceRetriever(src); LibraryConfiguration lc = new LibraryConfiguration("echoing", scm); lc.setIncludeInChangesets(false); @@ -391,6 +391,7 @@ public static class BasicSCMSource extends SCMSource { assertFalse(r.jenkins.getWorkspaceFor(p).withSuffix("@libs").isDirectory()); r.assertLogContains("something special", b); r.assertLogContains("Using shallow clone with depth 1", b); + r.assertLogContains("Avoid fetching tags", b); // Fails to reproduce presence of *.jar.tmp@tmp; probably specific to use of GIT_ASKPASS: assertThat(new File(b.getRootDir(), "libs").list(), arrayContaining(matchesPattern("[0-9a-f]{64}[.]jar"))); } From 698ae6887eae199e4b5da9b478f5039265562350 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 8 Mar 2023 14:09:14 -0500 Subject: [PATCH 38/43] `CloneOption.honorRefspec` can also be useful --- .../plugins/workflow/libs/SCMBasedRetriever/help-clone.html | 2 +- .../plugins/workflow/libs/SCMSourceRetrieverTest.java | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html index 939f3e6b..c0d502c7 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html +++ b/src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html @@ -2,7 +2,7 @@ If checked, every build performs a fresh clone of the SCM rather than locking and updating a common copy. No changelog will be computed. For Git, you are advised to add Advanced clone behaviors - and then check Shallow clone and uncheck Fetch tags + and then check Shallow clone and Honor refspec on initial clone and uncheck Fetch tags to make the clone much faster. You may still enable Cache fetched versions on controller for quick retrieval if you prefer. diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java index 899b2f69..1cd90fb6 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetrieverTest.java @@ -62,6 +62,7 @@ import static hudson.ExtensionList.lookupSingleton; import hudson.plugins.git.extensions.impl.CloneOption; import jenkins.plugins.git.traits.CloneOptionTrait; +import jenkins.plugins.git.traits.RefSpecsSCMSourceTrait; import jenkins.scm.api.trait.SCMSourceTrait; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -379,7 +380,9 @@ public static class BasicSCMSource extends SCMSource { sampleRepo.git("add", "."); sampleRepo.git("commit", "--message=init"); GitSCMSource src = new GitSCMSource(sampleRepo.toString()); - src.setTraits(List.of(new CloneOptionTrait(new CloneOption(true, true, null, null)))); + CloneOption cloneOption = new CloneOption(true, true, null, null); + cloneOption.setHonorRefspec(true); + src.setTraits(List.of(new CloneOptionTrait(cloneOption), new RefSpecsSCMSourceTrait("+refs/heads/master:refs/remotes/origin/master"))); SCMSourceRetriever scm = new SCMSourceRetriever(src); LibraryConfiguration lc = new LibraryConfiguration("echoing", scm); lc.setIncludeInChangesets(false); @@ -392,6 +395,7 @@ public static class BasicSCMSource extends SCMSource { r.assertLogContains("something special", b); r.assertLogContains("Using shallow clone with depth 1", b); r.assertLogContains("Avoid fetching tags", b); + r.assertLogNotContains("+refs/heads/*:refs/remotes/origin/*", b); // Fails to reproduce presence of *.jar.tmp@tmp; probably specific to use of GIT_ASKPASS: assertThat(new File(b.getRootDir(), "libs").list(), arrayContaining(matchesPattern("[0-9a-f]{64}[.]jar"))); } From 77aea3e0310b5b24b03aa6209e24f520a7fbb9f2 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 9 Mar 2023 11:58:32 -0500 Subject: [PATCH 39/43] Migrate `LoadedClasses.srcUrl`, and switching persisted field to `LibraryRecord.directoryName` to avoid use of `$JENKINS_HOME` in `program.dat` --- .../plugins/workflow/libs/LibraryStep.java | 66 +++++---- .../workflow/libs/LibraryStepTest.java | 24 ++++ .../jobs/p/builds/1/build.xml | 126 ++++++++++++++++++ .../builds/1/changelog5512666869627859761.xml | 0 ...8810cf8b02b166382c33fab989458805b-name.txt | 1 + .../src/pkg/C.groovy | 1 + .../jobs/p/builds/1/log | 29 ++++ .../jobs/p/builds/1/log-index | 3 + .../jobs/p/builds/1/program.dat | Bin 0 -> 4196 bytes .../jobs/p/builds/1/workflow/2.xml | 12 ++ .../jobs/p/builds/1/workflow/3.xml | 26 ++++ .../jobs/p/builds/1/workflow/4.xml | 26 ++++ .../jobs/p/builds/legacyIds | 0 .../jobs/p/builds/permalinks | 1 + .../jobs/p/config.xml | 11 ++ .../jobs/p/nextBuildNumber | 1 + ...lugins.workflow.flow.FlowExecutionList.xml | 7 + 17 files changed, 308 insertions(+), 26 deletions(-) create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/changelog5512666869627859761.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b-name.txt create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b/src/pkg/C.groovy create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/program.dat create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/2.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/4.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/legacyIds create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/config.xml create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber create mode 100644 src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java index 88f707b5..7647ee7f 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java @@ -39,12 +39,10 @@ import hudson.model.TaskListener; import hudson.scm.SCM; import hudson.security.AccessControlled; -import java.io.File; import java.io.IOException; import java.io.Serializable; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.net.URI; import java.net.URL; import java.util.ArrayList; import java.util.Collection; @@ -250,20 +248,40 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S private final @NonNull String prefix; /** {@link Class#getName} minus package prefix */ private final @CheckForNull String clazz; - /** {@code /…/libs/$hash.jar}, or null if resuming a pre-dir2Jar build */ - private final @Nullable String jar; + /** {@code file:/…/libs/NAME/src/} */ + @Deprecated + private @NonNull String srcUrl; + /** {@link LibraryRecord#getDirectoryName}, or null if resuming a pre-dir2Jar build */ + private final @Nullable String directoryName; LoadedClasses(String library, String libraryDirectoryName, boolean trusted, Boolean changelog, Run run) { - this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").getAbsolutePath()); + this(library, trusted, changelog, "", null, libraryDirectoryName); } - LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String jar) { + LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String directoryName) { this.library = library; this.trusted = trusted; this.changelog = changelog; this.prefix = prefix; this.clazz = clazz; - this.jar = jar; + this.directoryName = directoryName; + } + + private static final Pattern SRC_URL = Pattern.compile("file:/.+/([0-9a-f]{64})/src/"); + + private Object readResolve() throws IllegalAccessException { + if (srcUrl != null) { + Matcher m = SRC_URL.matcher(srcUrl); + if (!m.matches()) { + // Perhaps predating hash-based naming (ace0de3, Feb 2022): + throw new IllegalAccessException("Unexpected form of library source URL: " + srcUrl); + } + String inferredDirectoryName = m.group(1); + LOGGER.fine(() -> "deserializing to " + inferredDirectoryName); + return new LoadedClasses(library, trusted, changelog, prefix, clazz, inferredDirectoryName); + } else { + return this; + } } @Override public Object getProperty(String property) { @@ -287,12 +305,12 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S String fullClazz = clazz != null ? clazz + '$' + property : property; loadClass(prefix + fullClazz); // OK, class really exists, stash it and await methods - return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, jar); + return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, directoryName); } else if (clazz != null) { throw new MissingPropertyException(property, loadClass(prefix + clazz)); } else { // Still selecting package components. - return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, jar); + return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, directoryName); } } @@ -338,7 +356,7 @@ private static boolean isSandboxed() { // TODO putProperty for static field set - private static final Pattern JAR_URL = Pattern.compile("jar:(file:/.+[.]jar)!/.+"); + private static final Pattern JAR_URL = Pattern.compile("jar:file:/.+/([0-9a-f]{64})[.]jar!/.+"); private Class loadClass(String name) { CpsFlowExecution exec = CpsThread.current().getExecution(); @@ -352,23 +370,19 @@ private Class loadClass(String name) { if (definingLoader != loader) { throw new IllegalAccessException("cannot access " + c + " via library handle: " + definingLoader + " is not " + loader); } - if (jar != null) { - URL res = loader.getResource(name.replaceFirst("[$][^.]+$", "").replace('.', '/') + ".groovy"); - if (res == null) { - throw new IllegalAccessException("Unknown where " + name + " (" + c.getProtectionDomain().getCodeSource().getLocation() + ") was loaded from"); - } - Matcher m = JAR_URL.matcher(res.toString()); - if (!m.matches()) { - throw new IllegalAccessException("Unexpected URL " + res); - } - File actual = new File(URI.create(m.group(1))); - if (!actual.equals(new File(jar))) { - throw new IllegalAccessException(name + " was defined in " + actual + " rather than the expected " + jar); - } - LOGGER.fine(() -> "loaded " + name + " from " + res + " ~ " + actual + " as expected"); - } else { - LOGGER.fine(() -> "loaded " + name + " but resuming from an old build which did not properly record JAR location"); + URL res = loader.getResource(name.replaceFirst("[$][^.]+$", "").replace('.', '/') + ".groovy"); + if (res == null) { + throw new IllegalAccessException("Unknown where " + name + " (" + c.getProtectionDomain().getCodeSource().getLocation() + ") was loaded from"); + } + Matcher m = JAR_URL.matcher(res.toString()); + if (!m.matches()) { + throw new IllegalAccessException("Unexpected URL " + res); + } + String actual = m.group(1); + if (!actual.equals(directoryName)) { + throw new IllegalAccessException(name + " was defined in " + res + " rather than the expected " + directoryName); } + LOGGER.fine(() -> "loaded " + name + " from " + res + " ~ " + actual + " as expected"); if (!Modifier.isPublic(c.getModifiers())) { // unlikely since Groovy makes classes implicitly public throw new IllegalAccessException(c + " is not public"); } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java index 5925cd0b..3eb12763 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java @@ -58,6 +58,7 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.LoggerRule; +import org.jvnet.hudson.test.recipes.LocalData; @Issue("JENKINS-39450") public class LibraryStepTest { @@ -345,4 +346,27 @@ public class LibraryStepTest { r.assertLogContains("/lib/java", b); r.assertLogContains("/pipeline/java", b); } + + @LocalData + @Test public void classesWhenResumingPreDir2JarBuild() throws Exception { + // LocalData captured as of 1091aea7fa252acae11389588addf603a505e195: + /* + sampleRepo.init(); + sampleRepo.write("src/pkg/C.groovy", "package pkg; class C {static void m() {23}}"); + sampleRepo.git("add", "src"); + sampleRepo.git("commit", "--message=init"); + GlobalLibraries.get().setLibraries(Collections.singletonList(new LibraryConfiguration("stuff", new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))); + WorkflowJob p = r.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition("def lib = library('stuff@master'); sleep 180; echo(/got ${lib.pkg.C.m()}/)", true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + r.waitForMessage("Sleeping for 3 min", b); + b.save(); + Thread.sleep(Long.MAX_VALUE); + */ + WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class); + WorkflowRun b = p.getBuildByNumber(1); + r.assertBuildStatus(Result.SUCCESS, r.waitForCompletion(b)); + r.assertLogContains("got 23", b); + } + } diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml new file mode 100644 index 00000000..d0441445 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/build.xml @@ -0,0 +1,126 @@ + + + + + + + stuff + master + + true + true + eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b + + + + + + + master + + + 98c9e41033d994442c0105ba3011fe39a57a69b1 + + + + master + + + + + 1 + + + + + + …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 + + + + + + git …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 + + + + + + 1 + 1678379139742 + 1678379139771 + 0 + UTF-8 + false + + SUCCESS + + + MAX_SURVIVABILITY + + + flowNode + 82579725 + + + classLoad + 406679430 + + + run + 370461094 + + + parse + 387067399 + + + saveProgram + 121174695 + + + true + 4 + 1:4 + 2 + false + false + + false + + + + 2 + + + origin + +refs/heads/*:refs/remotes/origin/* + …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 + + + + + master + + + false + + + + + false + + + + + + + + + + …/pipeline-groovy-lib-plugin/target/tmp/j h744515433002142412/workspace/p@libs/f42b7770bb31e81024ba7b8451a6e63e6a1ce3eac92cd7c91e73094d7d85b2b9 + …/pipeline-groovy-lib-plugin/target/tmp/j h744515433002142412/jobs/p/builds/1/changelog5512666869627859761.xml + + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/changelog5512666869627859761.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/changelog5512666869627859761.xml new file mode 100644 index 00000000..e69de29b diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b-name.txt b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b-name.txt new file mode 100644 index 00000000..59c5d2b4 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b-name.txt @@ -0,0 +1 @@ +stuff \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b/src/pkg/C.groovy b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b/src/pkg/C.groovy new file mode 100644 index 00000000..6ac9168c --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/libs/eb3ddf43cdf7079385a1295409e0c698810cf8b02b166382c33fab989458805b/src/pkg/C.groovy @@ -0,0 +1 @@ +package pkg; class C {static void m() {23}} \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log new file mode 100644 index 00000000..97a7e935 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log @@ -0,0 +1,29 @@ +Started +ha:////4ESdDHApaBCDSybNFTtB8QH7JYx/td1ExPJr3dgp1IVpAAAAoh+LCAAAAAAAAP9tjTEOwjAQBM8BClpKHuFItIiK1krDC0x8GCfWnbEdkooX8TX+gCESFVvtrLSa5wtWKcKBo5UdUu8otU4GP9jS5Mixv3geZcdn2TIl9igbHBs2eJyx4YwwR1SwULBGaj0nRzbDRnX6rmuvydanHMu2V1A5c4MHCFXMWcf8hSnC9jqYxPTz/BXAFEIGsfuclm8zQVqFvQAAAA==[Pipeline] Start of Pipeline +ha:////4GmDEP+pf+jq6je4q08twRF2g64HEz1ZmA5RyCc5wqQeAAAAoh+LCAAAAAAAAP9tjTEOAiEURD9rLGwtPQSbaGmsbAmNJ0AWEZb8zwLrbuWJvJp3kLiJlZNMMm+a93rDOic4UbLcG+wdZu14DKOti0+U+lugiXu6ck2YKRguzSSpM+cFJRUDS1gDKwEbgzpQdmgLbIVXD9UGhba9lFS/o4DGdQM8gYlqLiqVL8wJdvexy4Q/z18BzLEA29ce4gfya1RxvAAAAA==[Pipeline] library +Loading library stuff@master +Attempting to resolve master from remote references... + > git --version # timeout=10 + > git --version # 'git version 2.34.1' + > git ls-remote -h -- …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 # timeout=10 +Found match: refs/heads/master revision 98c9e41033d994442c0105ba3011fe39a57a69b1 +The recommended git tool is: NONE +No credentials specified +Cloning the remote Git repository +Cloning with configured refspecs honoured and without tags +Cloning repository …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 + > git init …/pipeline-groovy-lib-plugin/target/tmp/j h744515433002142412/workspace/p@libs/f42b7770bb31e81024ba7b8451a6e63e6a1ce3eac92cd7c91e73094d7d85b2b9 # timeout=10 +Fetching upstream changes from …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 + > git --version # timeout=10 + > git --version # 'git version 2.34.1' + > git fetch --no-tags --force --progress -- …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 +refs/heads/*:refs/remotes/origin/* # timeout=10 + > git config remote.origin.url …/pipeline-groovy-lib-plugin/target/tmp/junit12658522753310550560/junit9582443866860979657 # timeout=10 + > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10 +Avoid second fetch +Checking out Revision 98c9e41033d994442c0105ba3011fe39a57a69b1 (master) + > git config core.sparsecheckout # timeout=10 + > git checkout -f 98c9e41033d994442c0105ba3011fe39a57a69b1 # timeout=10 +Commit message: "init" +First time build. Skipping changelog. +ha:////4JPM8UkGCuomMvb51Uo5DyzahUcbxePS+FWXyxySXcOwAAAAoh+LCAAAAAAAAP9tjTEOAiEURD9rLGwtPQSbGDtjZUtoPAGyiLDkfxZYdytP5NW8g8RNrJxkknnTvNcb1jnBiZLl3mDvMGvHYxhtXXyi1N8CTdzTlWvCTMFwaSZJnTkvKKkYWMIaWAnYGNSBskNbYCu8eqg2KLTtpaT6HQU0rhvgCUxUc1GpfGFOsLuPXSb8ef4KYI4F2L72ED81/RU+vAAAAA==[Pipeline] sleep +Sleeping for 3 min 0 sec diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index new file mode 100644 index 00000000..45645b44 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/log-index @@ -0,0 +1,3 @@ +622 3 +2385 +2685 4 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/program.dat b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/program.dat new file mode 100644 index 0000000000000000000000000000000000000000..8a44b773194e763a71f8f1cfd27f4ef202339b59 GIT binary patch literal 4196 zcma)9U2Ggz6`t80uk9oaPNB;~5ONzQAkxgP*SpEyZgatLer)QdQMNx)1QN3|cX!90 zof+=TIO|Bf@W3PDm6sq2;zt#vZzjMBGzI)ES;upIfdHL4D4Ncdy3Cl3X`E(##=92K2T|cs2GxU-cM&`vp zE;qg4qG^YbITuDxx23S0Gcrg*Eq#EXjqows3!+4dD6jF4iLS7d*bV%1RuTzkQsagj z#MS|gk4Cn1!HhhP6D|CJkn&+uJdU^%BP zI}YR_jgQmPs2zCD0WJUeFQk5y#_%~o;|l%L>-q)I+WTCOKwTNW%=Uiw@c#F%(u{Hb z=*~E&OKAxnqN`Q;VLRxU;G8%OAtJLSgJ5M<1*tBiYkBS^tKo^wEA#N_lmF+s==#gT z=`k)}J^Q5z@9H{P;kzx{7E$z+bcCGLYU@3kd+yTI`A8ItFaqGA9q>3=UnI}*5Cfg( z+LrHlf}%gpPi+GbC1DuI*bH5J*#q6E&ykXBo|LOkUG#-i=)DN#D_X^wWBfscKJ)~co>v?s$!Wq4{8)73XDx&up{I8vNJ|o8V6-7xd}drI zhL46aXi2Ms7rUXOq$8fSyab`(21OJhIe$TCZ$XuZw}}gFiqqH~^MD`lF~dH{YwSaa zQ>mq7;5S{V62vPzz^sE;Bk0b7lItf#=TATX>`xzg`1_X$iRaR-lZS+-hU#!&@t|v5 zC!+dtn)o9_m%E6uc{=fJiu0W1d8ho90US8t1-9iO%;Wsv)+n1-=lE`{_3Cac76Tjp zzh0jN6nY*Ix;i)w>6zh!g40Iu6pM5*x3Elws2DqgxxV2Z^-FibX^Q7*J7h zeQ`GFG=zk`Tt~$104m8-i~;aIdKCxhMXv@6eLM zk5k?mqjnqg@$ny>{#31idxfFkQ7TolC)tHGX#;UqN&4K-^?jLAcBbYnFW!IUcQ3Df z>N|%BBJUctwaqnz8*$r>)Q-xqb@tKyXR+JtYQ??(Z#?U{4e&a?&1S@`$Xs+A(vqun z)P|`=^daG>+=}SV^ZO33&HwUEc$(wmHY#dMctML)3=v$qguy5fXfcMlP>QD8rBNj9 zXQW3z$1*|9L5s|vQw2R~HjnccNivTWx*MKC=!Swcdn9c)M}U1KZPbO>k}VMz;!apt zI#?>UkCn@n=}LKKrdTXZmrLd8Qei1*M1`=>NLi8PIW`7EIF?oVTe%{TaGg1W9z1GjL2$=No=*!)?BFGQE5vzZQ42q z?CFrN)l${#o^#J!fMr5H_sthxd2IiW_7Pv6#Z0sj8mC@5ySws+O;u z&`0s&(?gwC{u;G)wtga0y8}zZE1(MMoE60zgPB7q4+JVM4t*1D%MYZOP2vE}3?*?= zWA9Roqgc9jOvgg1<9IW+LJe3&87<6+`9!fOz9^n?M zyDcJD((*P^(b?Nj*9pD0Zq(jqYqR*oF?nlHRM}9-cy(4C-Zmn_DRXD*-Kf2B@dy;z zhHBVb6fhiNlp%coGs60ptVUHm5tX_25jYjM_KEtO*FRQm#K^EW;Eq9?B(K$O z8?}F^)V;#4^@l%$g>AYUwtEy?{w6V@A}V1Z4Llnzw>0*KQTw|x@RCux!}U3h-R!^l ziF(t4dfq1qrPF>)68eNu`v8jmH+ma4T0uOC!EyW<4?$Yg0PMgKZCq9Q$9S~l*zE}O zOdUe?TU3FjuRPIO|JvUkB;&ZnsP{<5IRAL>=(cqBx=n9Xw-2-K2@$H+c&dNv0~S5$JtGUF zQ-~Aa5!!46p~W?i0Po^a{D1ZE4`01?@H(A;0an@(*gYV$5KrpVsGVLGAS@8w;nkPm zs4?M^kzy&Z95)()L;Zc6KItZaT=PA5@4)9p6Pt7i? + + + + 2 + + + + 1678379140348 + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml new file mode 100644 index 00000000..9cf66a5b --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/3.xml @@ -0,0 +1,26 @@ + + + + + 2 + + 3 + org.jenkinsci.plugins.workflow.libs.LibraryStep + + + + + + identifier + stuff@master + + + + true + + + 1678379140501 + + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/4.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/4.xml new file mode 100644 index 00000000..7cf17cd2 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/1/workflow/4.xml @@ -0,0 +1,26 @@ + + + + + 3 + + 4 + org.jenkinsci.plugins.workflow.steps.SleepStep + + + + + + time + 180 + + + + true + + + 1678379141028 + + + + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/legacyIds b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/legacyIds new file mode 100644 index 00000000..e69de29b diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks new file mode 100644 index 00000000..48ab9e85 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/builds/permalinks @@ -0,0 +1 @@ +lastSuccessfulBuild -1 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/config.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/config.xml new file mode 100644 index 00000000..69201d5e --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/config.xml @@ -0,0 +1,11 @@ + + + false + + + + true + + + false + \ No newline at end of file diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber new file mode 100644 index 00000000..0cfbf088 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/jobs/p/nextBuildNumber @@ -0,0 +1 @@ +2 diff --git a/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml new file mode 100644 index 00000000..33e6c526 --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/workflow/libs/LibraryStepTest/classesWhenResumingPreDir2JarBuild/org.jenkinsci.plugins.workflow.flow.FlowExecutionList.xml @@ -0,0 +1,7 @@ + + + + p + 1 + + \ No newline at end of file From 2fb8ed94969c87c72be3bb7a68614d588a261dea Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 9 Mar 2023 14:09:44 -0500 Subject: [PATCH 40/43] SpotBugs --- .../java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java index 7647ee7f..19a2e6e7 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java @@ -248,9 +248,9 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S private final @NonNull String prefix; /** {@link Class#getName} minus package prefix */ private final @CheckForNull String clazz; - /** {@code file:/…/libs/NAME/src/} */ + /** {@code file:/…/libs/NAME/src/}, migrated to {@link #directoryName} */ @Deprecated - private @NonNull String srcUrl; + private @CheckForNull String srcUrl; /** {@link LibraryRecord#getDirectoryName}, or null if resuming a pre-dir2Jar build */ private final @Nullable String directoryName; From 6acba02a09a80faa39e0d94b166cedf0757e23a7 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 9 Mar 2023 18:31:57 -0500 Subject: [PATCH 41/43] https://github.com/jenkinsci/workflow-cps-plugin/pull/673 released --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b65a8786..c7358a1e 100644 --- a/pom.xml +++ b/pom.xml @@ -79,7 +79,7 @@ org.jenkins-ci.plugins.workflow workflow-cps - 3630.vf8b_1e63e3d03 + 3637.v63b_c17e0ed5b_ From 23125c9a177b622915b40602933aa3ce6eff0355 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 13 Mar 2023 14:18:38 -0400 Subject: [PATCH 42/43] Also delete `lastReadFile` when clearing cache Co-authored-by: Devin Nusbaum --- .../plugins/workflow/libs/LibraryCachingCleanup.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java index 7c2d7d56..8946f394 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java @@ -46,7 +46,11 @@ private void removeIfExpiredCacheJar(FilePath libJar, TaskListener listener) thr try { if (System.currentTimeMillis() - lastReadFile.lastModified() > TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS)) { listener.getLogger().println("Deleting " + libJar); - libJar.delete(); + try { + libJar.delete(); + } finally { + lastReadFile.delete(); + } } else { listener.getLogger().println(lastReadFile + " is sufficiently recent"); } From 157d6f0c49c645335a39866c2fdfa3412b27d2ef Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 24 Mar 2023 09:35:49 -0400 Subject: [PATCH 43/43] Removing some minor differences to `SCMSourceRetriever.clone` --- .../jenkinsci/plugins/workflow/libs/SCMBasedRetriever.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever.java b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever.java index 334f0017..bf870409 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever.java @@ -138,7 +138,7 @@ protected final void doRetrieve(String name, boolean changelog, @NonNull SCM scm tmp2.deleteRecursive(); } } - } else { + } else { // !clone FilePath dir; if (run.getParent() instanceof TopLevelItem) { FilePath baseWorkspace = node.getWorkspaceFor((TopLevelItem) run.getParent()); @@ -200,7 +200,7 @@ protected static T retrySCMOperation(TaskListener listener, Callable task } // TODO there is WorkspaceList.tempDir but no API to make other variants - protected static String getFilePathSuffix() { + private static String getFilePathSuffix() { return System.getProperty(WorkspaceList.class.getName(), "@"); }