[JENKINS-49635] Defining new VirtualFile methods to better support ex…

…ternal artifact storage.
jenkinsci · Feb 19, 2018 · 3f01a77 · 3f01a77
1 parent 844a13f
commit 3f01a77
Show file tree

Hide file tree

Showing 3 changed files with 286 additions and 2 deletions.
diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java
@@ -29,6 +29,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.Serializable;
+import java.net.URL;
 import java.text.Collator;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -51,6 +52,7 @@
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
@@ -295,6 +297,14 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
             return;
         }
 
+        URL external = baseFile.toExternalURL();
+        if (external != null) {
+            // or this URL could be emitted directly from dir.jelly
+            // though we would prefer to delay toExternalURL calls unless and until needed
+            rsp.sendRedirect2(external.toExternalForm());
+            return;
+        }
+
         long lastModified = baseFile.lastModified();
         long length = baseFile.length();
 

diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java
@@ -28,26 +28,28 @@
 import hudson.model.DirectoryBrowserSupport;
 import hudson.remoting.Callable;
 import hudson.remoting.Channel;
+import hudson.remoting.RemoteInputStream;
 import hudson.remoting.VirtualChannel;
 import hudson.util.DirScanner;
 import hudson.util.FileVisitor;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Serializable;
 import java.net.URI;
+import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.LinkOption;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
-
 import jenkins.MasterToSlaveFileCallable;
+import jenkins.model.ArtifactManager;
 
 /**
  * Abstraction over {@link File}, {@link FilePath}, or other items such as network resources or ZIP entries.
@@ -62,6 +64,26 @@
  * {@link VirtualFile} makes no assumption about where the actual files are, or whether there really exists
  * {@link File}s somewhere. This makes VirtualFile more abstract.
  *
+ * <h2>Opening files from other machines</h2>
+ *
+ * While {@link VirtualFile} is marked {@link Serializable},
+ * it is <em>not</em> safe in general to transfer over a Remoting channel.
+ * (For example, an implementation from {@link #forFilePath} could be sent on the <em>same</em> channel,
+ * but an implementation from {@link #forFile} will not.)
+ * Thus callers should assume that methods such as {@link #open} will work
+ * only on the node on which the object was created.
+ *
+ * <p>Since some implementations may in fact use external file storage,
+ * callers may request optional APIs to access those services more efficiently.
+ * Otherwise, for example, a plugin copying a file
+ * previously saved by {@link ArtifactManager} to an external storage servuce
+ * which tunneled a stream from {@link #open} using {@link RemoteInputStream}
+ * would wind up transferring the file from the service to the Jenkins master and then on to an agent.
+ * Similarly, if {@link DirectoryBrowserSupport} rendered a link to an in-Jenkins URL,
+ * a large file could be transferred from the service to the Jenkins master and then on to the browser.
+ * To avoid this overhead, callers may check whether an implementation
+ * supports {@link #asRemotable} and/or {@link #toExternalURL}.
+ *
  * @see DirectoryBrowserSupport
  * @see FilePath
  * @since 1.532
@@ -80,6 +102,7 @@ public abstract class VirtualFile implements Comparable<VirtualFile>, Serializab
     /**
      * Gets a URI.
      * Should at least uniquely identify this virtual file within its root, but not necessarily globally.
+     * <p>When {@link #toExternalURL} is implemented, it is natural but not required to use that same value here.
      * @return a URI (need not be absolute)
      */
     public abstract URI toURI();
@@ -208,6 +231,43 @@ public <V> V run(Callable<V,IOException> callable) throws IOException {
         return callable.call();
     }
 
+    /**
+     * Optionally produces a variant of this handle which may be safely passed over a Remoting {@link Channel}.
+     * This would allow remote nodes such as agents to make calls such as {@link #open}
+     * and be assured of the most efficient possible access.
+     * Otherwise, all calls must be made on the node originally producing this object,
+     * and the caller must arrange for transport of the result.
+     * <p>Note that the result of {@link #forFilePath} does <em>not</em> implement this method,
+     * since a {@link FilePath} may only be transferred over the channel on which it was created.
+     * It cannot, for example, be used to represent a workspace file from one agent on another agent.
+     * @return this object or a variant which may be passed over a {@link Channel}, or null if there is no such support
+     * @since FIXME
+     * @see #toExternalURL
+     */
+    public @CheckForNull VirtualFile asRemotable() {
+        return null;
+    }
+
+    /**
+     * Optionally obtains a URL which may be used to retrieve file contents from any process on any node.
+     * For example, given cloud storage this might produce a permalink to the file.
+     * <p>This is only meaningful for {@link #isFile}:
+     * no ZIP etc. archiving protocol is defined to allow bulk access to directory trees.
+     * <p>Any necessary authentication must be encoded somehow into the URL itself;
+     * do not include any tokens or other authentication which might allow access to unrelated files
+     * (for example {@link ArtifactManager} builds from a different job).
+     * <p>Generally this will be harder to implement than {@link #asRemotable},
+     * which would have the opportunity to perform arbitrary preparation for {@link #open}
+     * such as negotiating session authentication.
+     * @return an externally usable URL like {@code https://gist.githubusercontent.com/ACCT/GISTID/raw/COMMITHASH/FILE}, or null if there is no such support
+     * @since FIXME
+     * @see #toURI
+     * @see #asRemotable
+     */
+    public @CheckForNull URL toExternalURL() throws IOException {
+        return null;
+    }
+
     /**
      * Creates a virtual file wrapper for a local file.
      * @param f a disk file (need not exist)

diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
@@ -23,6 +23,7 @@
  */
 package hudson.model;
 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -51,8 +52,26 @@
 import com.gargoylesoftware.htmlunit.Page;
 import com.gargoylesoftware.htmlunit.UnexpectedPage;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.ExtensionList;
+import hudson.Util;
+import java.io.ByteArrayInputStream;
+import java.io.FileNotFoundException;
 import java.io.OutputStream;
+import java.net.URI;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import jenkins.model.ArtifactManager;
+import jenkins.model.ArtifactManagerConfiguration;
+import jenkins.model.ArtifactManagerFactory;
+import jenkins.model.ArtifactManagerFactoryDescriptor;
+import jenkins.model.Jenkins;
+import jenkins.util.VirtualFile;
 import org.apache.commons.io.IOUtils;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -212,4 +231,199 @@ private File download(UnexpectedPage page) throws IOException {
 
         return file;
     }
+
+    @Issue("JENKINS-49635")
+    @Test
+    public void externalURLDownload() throws Exception {
+        ArtifactManagerConfiguration.get().getArtifactManagerFactories().add(new ExternalArtifactManagerFactory());
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setScm(new SingleFileSCM("f", "Hello world!"));
+        p.getPublishersList().add(new ArtifactArchiver("f"));
+        j.buildAndAssertSuccess(p);
+        HtmlPage page = j.createWebClient().goTo("job/" + p.getName() + "/lastSuccessfulBuild/artifact/");
+        try {
+            Page download = page.getAnchorByText("f").click();
+            assertEquals("Hello world!", download.getWebResponse().getContentAsString());
+        } catch (FailingHttpStatusCodeException x) {
+            IOUtils.copy(x.getResponse().getContentAsStream(), System.err);
+            throw x;
+        }
+    }
+    @TestExtension("externalURLDownload")
+    public static final class ContentAddressableStore implements UnprotectedRootAction {
+        final List<byte[]> files = new ArrayList<>();
+        @Override
+        public String getUrlName() {
+            return "files";
+        }
+        @Override
+        public String getIconFileName() {
+            return null;
+        }
+        @Override
+        public String getDisplayName() {
+            return null;
+        }
+        public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws Exception {
+            String hash = req.getRestOfPath().substring(1);
+            for (byte[] file : files) {
+                if (Util.getDigestOf(new ByteArrayInputStream(file)).equals(hash)) {
+                    rsp.setContentType("application/octet-stream");
+                    rsp.getOutputStream().write(file);
+                    return;
+                }
+            }
+            rsp.sendError(404);
+        }
+    }
+    public static final class ExternalArtifactManagerFactory extends ArtifactManagerFactory {
+        @Override
+        public ArtifactManager managerFor(Run<?, ?> build) {
+            return new ExternalArtifactManager();
+        }
+        @TestExtension("externalURLDownload")
+        public static final class DescriptorImpl extends ArtifactManagerFactoryDescriptor {}
+    }
+    private static final class ExternalArtifactManager extends ArtifactManager {
+        String hash;
+        @Override
+        public void archive(FilePath workspace, Launcher launcher, BuildListener listener, Map<String, String> artifacts) throws IOException, InterruptedException {
+            assertEquals(1, artifacts.size());
+            Map.Entry<String, String> entry = artifacts.entrySet().iterator().next();
+            assertEquals("f", entry.getKey());
+            try (InputStream is = workspace.child(entry.getValue()).read()) {
+                byte[] data = IOUtils.toByteArray(is);
+                ExtensionList.lookupSingleton(ContentAddressableStore.class).files.add(data);
+                hash = Util.getDigestOf(new ByteArrayInputStream(data));
+            }
+        }
+        @Override
+        public VirtualFile root() {
+            final VirtualFile file = new VirtualFile() {
+                @Override
+                public String getName() {
+                    return "f";
+                }
+                @Override
+                public URI toURI() {
+                    return URI.create("root:f");
+                }
+                @Override
+                public VirtualFile getParent() {
+                    return root();
+                }
+                @Override
+                public boolean isDirectory() throws IOException {
+                    return false;
+                }
+                @Override
+                public boolean isFile() throws IOException {
+                    return true;
+                }
+                @Override
+                public boolean exists() throws IOException {
+                    return true;
+                }
+                @Override
+                public VirtualFile[] list() throws IOException {
+                    return new VirtualFile[0];
+                }
+                @Override
+                public String[] list(String glob) throws IOException {
+                    return new String[0];
+                }
+                @Override
+                public VirtualFile child(String name) {
+                    throw new UnsupportedOperationException();
+                }
+                @Override
+                public long length() throws IOException {
+                    return 0;
+                }
+                @Override
+                public long lastModified() throws IOException {
+                    return 0;
+                }
+                @Override
+                public boolean canRead() throws IOException {
+                    return true;
+                }
+                @Override
+                public InputStream open() throws IOException {
+                    throw new FileNotFoundException("expect to be opened via URL only");
+                }
+                @Override
+                public URL toExternalURL() throws IOException {
+                    return new URL(Jenkins.get().getRootUrl() + "files/" + hash);
+                }
+            };
+            return new VirtualFile() {
+                @Override
+                public String getName() {
+                    return "";
+                }
+                @Override
+                public URI toURI() {
+                    return URI.create("root:");
+                }
+                @Override
+                public VirtualFile getParent() {
+                    return this;
+                }
+                @Override
+                public boolean isDirectory() throws IOException {
+                    return true;
+                }
+                @Override
+                public boolean isFile() throws IOException {
+                    return false;
+                }
+                @Override
+                public boolean exists() throws IOException {
+                    return true;
+                }
+                @Override
+                public VirtualFile[] list() throws IOException {
+                    return new VirtualFile[] {file};
+                }
+                @Override
+                public String[] list(String glob) throws IOException {
+                    throw new UnsupportedOperationException();
+                }
+                @Override
+                public VirtualFile child(String name) {
+                    if (name.equals("f")) {
+                        return file;
+                    } else if (name.isEmpty()) {
+                        return this;
+                    } else {
+                        throw new UnsupportedOperationException("trying to call child on " + name);
+                    }
+                }
+                @Override
+                public long length() throws IOException {
+                    return 0;
+                }
+                @Override
+                public long lastModified() throws IOException {
+                    return 0;
+                }
+                @Override
+                public boolean canRead() throws IOException {
+                    return true;
+                }
+                @Override
+                public InputStream open() throws IOException {
+                    throw new FileNotFoundException();
+                }
+            };
+        }
+        @Override
+        public void onLoad(Run<?, ?> build) {}
+        @Override
+        public boolean delete() throws IOException, InterruptedException {
+            return false;
+        }
+    }
+
 }