Skip to content

Commit

Permalink
[MBUILDCACHE-86] bugfix / enhancements restoration of outputs on disk (
Browse files Browse the repository at this point in the history
…#104)

* [MBUILDCACHE-86] bugfix/enhancement around restoration on disk

- Bugfix / "todo" : files in a base directory containing an underscore were wrongly restored to disk (not at the same location).
  -> To do so, the path is not guessed anymore from the classifier. I introduced a "filePath" property in the "attachedArtifact" section of the buildinfo.xml file.
  -> Because the buildInfo structure change, I changed the cache implementation version from "v1" to "v1.1". I assume it was one of the purpose of this value : we don't have to deal with structure migration. Any previous cache entry is defacto invalidated.
- Forbid the possibility to extract/restore data in a directory outside the project (like extracting ../../../.ssh for example)
  -> I guess the extraction part is not a vulnerability since someone with commit permissions can guess other ways to extract data. But the possibility of restoring at any place on the disk looks pretty dangerous to me if a remote cache server is compromised.
- Gives the possibility to restore artefacts on disk, with a dedicated property : maven.build.cache.restoreOnDiskArtefacts (default to true, open for discussion)
- Introduce "globs" to filter extra attached outputs by filenames.

* [MBUILDCACHE-86] Adapt restoration logic to MBUILDCACHE-80 developments
+ TI

Fix  javadoc for jdk8
  • Loading branch information
kbuntrock authored May 1, 2024
1 parent 8e3dad7 commit dcc89ad
Show file tree
Hide file tree
Showing 17 changed files with 472 additions and 126 deletions.
221 changes: 132 additions & 89 deletions src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java

Large diffs are not rendered by default.

34 changes: 28 additions & 6 deletions src/main/java/org/apache/maven/buildcache/CacheUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystems;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.util.Arrays;
Expand All @@ -37,6 +40,7 @@
import java.util.zip.ZipOutputStream;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.mutable.MutableBoolean;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.handler.ArtifactHandler;
import org.apache.maven.buildcache.xml.build.Scm;
Expand Down Expand Up @@ -160,21 +164,39 @@ public static boolean isArchive(File file) {
return StringUtils.endsWithAny(fileName, ".jar", ".zip", ".war", ".ear");
}

public static void zip(Path dir, Path zip) throws IOException {
/**
* Put every matching files of a directory in a zip.
* @param dir directory to zip
* @param zip zip to populate
* @param glob glob to apply to filenames
* @return true if at least one file has been included in the zip.
* @throws IOException
*/
public static boolean zip(final Path dir, final Path zip, final String glob) throws IOException {
final MutableBoolean hasFiles = new MutableBoolean();
try (ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(zip))) {

PathMatcher matcher =
"*".equals(glob) ? null : FileSystems.getDefault().getPathMatcher("glob:" + glob);
Files.walkFileTree(dir, new SimpleFileVisitor<Path>() {

@Override
public FileVisitResult visitFile(Path path, BasicFileAttributes basicFileAttributes)
throws IOException {
final ZipEntry zipEntry = new ZipEntry(dir.relativize(path).toString());
zipOutputStream.putNextEntry(zipEntry);
Files.copy(path, zipOutputStream);
zipOutputStream.closeEntry();

if (matcher == null || matcher.matches(path.getFileName())) {
final ZipEntry zipEntry =
new ZipEntry(dir.relativize(path).toString());
zipOutputStream.putNextEntry(zipEntry);
Files.copy(path, zipOutputStream);
hasFiles.setTrue();
zipOutputStream.closeEntry();
}
return FileVisitResult.CONTINUE;
}
});
}
return hasFiles.booleanValue();
}

public static void unzip(Path zip, Path out) throws IOException {
Expand All @@ -190,7 +212,7 @@ public static void unzip(Path zip, Path out) throws IOException {
} else {
Path parent = file.getParent();
Files.createDirectories(parent);
Files.copy(zis, file);
Files.copy(zis, file, StandardCopyOption.REPLACE_EXISTING);
}
Files.setLastModifiedTime(file, FileTime.fromMillis(entry.getTime()));
entry = zis.getNextEntry();
Expand Down
49 changes: 49 additions & 0 deletions src/main/java/org/apache/maven/buildcache/artifact/OutputType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.buildcache.artifact;

public enum OutputType {
// generated project artifact
ARTIFACT(""),
// generated source (regular and test)
GENERATED_SOURCE("mvn-cache-ext-generated-source-"),
// any unclassified output added by configuration
EXTRA_OUTPUT("mvn-cache-ext-extra-output-");

private String classifierPrefix;

OutputType(String getClassifierPrefix) {
this.classifierPrefix = getClassifierPrefix;
}

public String getClassifierPrefix() {
return classifierPrefix;
}

public static OutputType fromClassifier(String classifier) {
if (classifier != null) {
if (classifier.startsWith(GENERATED_SOURCE.classifierPrefix)) {
return GENERATED_SOURCE;
} else if (classifier.startsWith(EXTRA_OUTPUT.classifierPrefix)) {
return EXTRA_OUTPUT;
}
}
return ARTIFACT;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.RunnableFuture;
import java.util.function.Function;

import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.DefaultArtifact;
Expand All @@ -46,8 +47,15 @@ public class RestoredArtifact extends DefaultArtifact {

private volatile Future<File> fileFuture;

private Function<File, File> restoreToDiskConsumer;

public RestoredArtifact(
Artifact parent, Future<File> fileFuture, String type, String classifier, ArtifactHandler handler) {
Artifact parent,
Future<File> fileFuture,
String type,
String classifier,
ArtifactHandler handler,
Function<File, File> restoreToDiskConsumer) {
super(
parent.getGroupId(),
parent.getArtifactId(),
Expand All @@ -58,6 +66,7 @@ public RestoredArtifact(
handler,
parent.isOptional());
this.fileFuture = requireNonNull(fileFuture, "fileFuture == null");
this.restoreToDiskConsumer = restoreToDiskConsumer;
}

/**
Expand Down Expand Up @@ -89,7 +98,8 @@ public File getFile() {
}

try {
return fileFuture.get();
File file = fileFuture.get();
return restoreToDiskConsumer.apply(file);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new InvalidArtifactRTException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,17 @@
import static org.apache.maven.buildcache.xml.CacheConfigImpl.CACHE_ENABLED_PROPERTY_NAME;
import static org.apache.maven.buildcache.xml.CacheConfigImpl.CACHE_SKIP;
import static org.apache.maven.buildcache.xml.CacheConfigImpl.RESTORE_GENERATED_SOURCES_PROPERTY_NAME;
import static org.apache.maven.buildcache.xml.CacheConfigImpl.RESTORE_ON_DISK_ARTIFACTS_PROPERTY_NAME;

/**
* MavenProjectInput
*/
public class MavenProjectInput {

/**
* Version of hashing algorithm implementation. It is recommended to change to simplify remote cache maintenance
* Version of cache implementation. It is recommended to change to simplify remote cache maintenance
*/
public static final String CACHE_IMPLEMENTATION_VERSION = "v1";
public static final String CACHE_IMPLEMENTATION_VERSION = "v1.1";

/**
* property name to pass glob value. The glob to be used to list directory files in plugins scanning
Expand Down Expand Up @@ -728,6 +729,18 @@ public static boolean isRestoreGeneratedSources(MavenProject project) {
project.getProperties().getProperty(RESTORE_GENERATED_SOURCES_PROPERTY_NAME, "true"));
}

/**
* Allow skipping artifacts restoration on a per-project level via a property (which defaults to true)
* e.g. {@code <maven.build.cache.restoreOnDiskArtifacts>false<maven.build.cache.restoreOnDiskArtifacts/>}.
*
* @param project
* @return
*/
public static boolean isRestoreOnDiskArtifacts(MavenProject project) {
return Boolean.parseBoolean(
project.getProperties().getProperty(RESTORE_ON_DISK_ARTIFACTS_PROPERTY_NAME, "true"));
}

/**
* Allow disabling caching entirely on a per-project level via a property - both artifact lookup and upload
* Defaults to false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import org.apache.maven.buildcache.PluginScanConfig;
import org.apache.maven.buildcache.hash.HashFactory;
import org.apache.maven.buildcache.xml.config.DirName;
import org.apache.maven.buildcache.xml.config.Exclude;
import org.apache.maven.buildcache.xml.config.Include;
import org.apache.maven.buildcache.xml.config.MultiModule;
Expand Down Expand Up @@ -103,7 +104,7 @@ public interface CacheConfig {

String getLocalRepositoryLocation();

List<String> getAttachedOutputs();
List<DirName> getAttachedOutputs();

boolean adjustMetaInfVersion();

Expand Down Expand Up @@ -133,6 +134,11 @@ public interface CacheConfig {
*/
boolean isRestoreGeneratedSources();

/**
* Flag to restore (default) or not generated artifacts
*/
boolean isRestoreOnDiskArtifacts();

String getAlwaysRunPlugins();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.apache.maven.buildcache.xml.config.CacheConfig;
import org.apache.maven.buildcache.xml.config.Configuration;
import org.apache.maven.buildcache.xml.config.CoordinatesBase;
import org.apache.maven.buildcache.xml.config.DirName;
import org.apache.maven.buildcache.xml.config.Exclude;
import org.apache.maven.buildcache.xml.config.Executables;
import org.apache.maven.buildcache.xml.config.ExecutionConfigurationScan;
Expand Down Expand Up @@ -90,6 +91,7 @@ public class CacheConfigImpl implements org.apache.maven.buildcache.xml.CacheCon
public static final String FAIL_FAST_PROPERTY_NAME = "maven.build.cache.failFast";
public static final String BASELINE_BUILD_URL_PROPERTY_NAME = "maven.build.cache.baselineUrl";
public static final String LAZY_RESTORE_PROPERTY_NAME = "maven.build.cache.lazyRestore";
public static final String RESTORE_ON_DISK_ARTIFACTS_PROPERTY_NAME = "maven.build.cache.restoreOnDiskArtifacts";
public static final String RESTORE_GENERATED_SOURCES_PROPERTY_NAME = "maven.build.cache.restoreGeneratedSources";
public static final String ALWAYS_RUN_PLUGINS = "maven.build.cache.alwaysRunPlugins";
public static final String MANDATORY_CLEAN = "maven.build.cache.mandatoryClean";
Expand Down Expand Up @@ -504,6 +506,11 @@ public boolean isRestoreGeneratedSources() {
return getProperty(RESTORE_GENERATED_SOURCES_PROPERTY_NAME, true);
}

@Override
public boolean isRestoreOnDiskArtifacts() {
return getProperty(RESTORE_ON_DISK_ARTIFACTS_PROPERTY_NAME, true);
}

@Override
public String getAlwaysRunPlugins() {
return getProperty(ALWAYS_RUN_PLUGINS, null);
Expand Down Expand Up @@ -550,7 +557,7 @@ public String getLocalRepositoryLocation() {
}

@Override
public List<String> getAttachedOutputs() {
public List<DirName> getAttachedOutputs() {
checkInitializedState();
final AttachedOutputs attachedOutputs = getConfiguration().getAttachedOutputs();
return attachedOutputs == null ? Collections.emptyList() : attachedOutputs.getDirNames();
Expand Down
4 changes: 4 additions & 0 deletions src/main/mdo/build-cache-build.mdo
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ under the License.
<name>fileSize</name>
<type>long</type>
</field>
<field>
<name>filePath</name>
<type>String</type>
</field>
<!--/xs:sequence-->
</fields>
<!--/xs:complexType-->
Expand Down
32 changes: 30 additions & 2 deletions src/main/mdo/build-cache-config.mdo
Original file line number Diff line number Diff line change
Expand Up @@ -374,14 +374,42 @@ under the License.
-->
<class>
<name>AttachedOutputs</name>
<description>Section relative to outputs which are not artifacts but need to be saved/restored.</description>
<fields>
<field>
<name>dirNames</name>
<association>
<type>String</type>
<type>DirName</type>
<multiplicity>*</multiplicity>
</association>
<description>Directory name in build output directory to attach to cached artifacts</description>
<description>Path to a directory containing files which needs to be saved/restored (relative to the build directory).</description>
</field>
</fields>
</class>
<class>
<name>DirName</name>
<description><![CDATA[Path to a directory containing files which needs to be saved/restored (relative to the build directory).
<br>
Examples :
<ul>
<li><code>&lt;dirName&gt;classes&lt;/dirName&gt;</code> : files in ${project.basedir}/target/classes</li>
<li><code>&lt;dirName glob="jacoco.xml"&gt;&lt;/dirName&gt;</code> : jacoco report files in ${project.basedir}/target</li>
<li><code>&lt;dirName&gt;../src/main/javagen&lt;/dirName&gt;</code> : files in ${project.basedir}/src/main/javagen (in this exemple, javagen is a folder saved in git but erased on clean) </li>
</ul>
<br><br>

]]></description>
<fields>
<field xml.content="true">
<name>value</name>
<type>String</type>
<description>Directory name in build output directory to attach to cached artifacts.</description>
</field>
<field xml.attribute="true">
<name>glob</name>
<type>String</type>
<defaultValue>*</defaultValue>
<description>Entries are filtered by matching this glob.</description>
</field>
</fields>
</class>
Expand Down
Loading

0 comments on commit dcc89ad

Please sign in to comment.