Skip to content

Commit

Permalink
[JBEAP-27367] Ignore non-readable folders in the installation
Browse files Browse the repository at this point in the history
  • Loading branch information
spyrkob committed Jul 22, 2024
1 parent b54f5ca commit 63e1c82
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.File;
import java.io.IOException;
import java.nio.file.AccessDeniedException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -64,7 +65,6 @@
import static org.jboss.galleon.diff.FsDiff.REMOVED;
import static org.jboss.galleon.diff.FsDiff.formatMessage;
import static org.wildfly.prospero.metadata.ProsperoMetadataUtils.CURRENT_VERSION_FILE;
import static org.wildfly.prospero.metadata.ProsperoMetadataUtils.INSTALLER_CHANNELS_FILE_NAME;
import static org.wildfly.prospero.metadata.ProsperoMetadataUtils.MANIFEST_FILE_NAME;
import static org.wildfly.prospero.metadata.ProsperoMetadataUtils.METADATA_DIR;

Expand Down Expand Up @@ -176,7 +176,7 @@ public List<FileConflict> applyUpdate(Type operation) throws ProvisioningExcepti
final FsDiff diffs = findChanges();
ApplyStageBackup backup = null;
try {
backup = new ApplyStageBackup(installationDir);
backup = new ApplyStageBackup(installationDir, updateDir);
backup.recordAll();

ProsperoLogger.ROOT_LOGGER.debug("Update backup generated in " + installationDir.resolve(ApplyStageBackup.BACKUP_FOLDER));
Expand Down Expand Up @@ -708,6 +708,9 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) {
if (dir.equals(skipInstallationGalleon) || dir.equals(skipInstallationInstallation) || dir.equals(installationDir.resolve(ApplyStageBackup.BACKUP_FOLDER))) {
return FileVisitResult.SKIP_SUBTREE;
}
if (!Files.isReadable(dir)) {
return FileVisitResult.SKIP_SUBTREE;
}
if (!dir.equals(installationDir)) {
Path relative = installationDir.relativize(dir);
Path target = updateDir.resolve(relative);
Expand Down Expand Up @@ -738,6 +741,21 @@ public FileVisitResult postVisitDirectory(Path dir, IOException e) {
}
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
if (exc instanceof AccessDeniedException) {
Path relative = installationDir.relativize(file);
Path target = updateDir.resolve(relative);
// TODO: check it's not in the Galleon hashes
if (Files.exists(target)) {
throw exc;
}
return FileVisitResult.SKIP_SUBTREE;
} else {
throw exc;
}
}
});
return Collections.unmodifiableList(conflicts);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -23,15 +22,17 @@ class ApplyStageBackup implements AutoCloseable {
protected static final String BACKUP_FOLDER = ".update.old";
private final Path backupRoot;
private final Path serverRoot;
private final Path candidateRoot;

/**
* create a record for server at {@code serverRoot}. The recorded files will be stored in {@tempRoot}
*
* @param serverRoot - root folder of the server that will be updated
*/
public ApplyStageBackup(Path serverRoot) throws IOException {
ApplyStageBackup(Path serverRoot, Path candidateRoot) throws IOException {

this.serverRoot = serverRoot;
this.candidateRoot = candidateRoot;
this.backupRoot = serverRoot.resolve(BACKUP_FOLDER);

if (ProsperoLogger.ROOT_LOGGER.isDebugEnabled()) {
Expand Down Expand Up @@ -68,43 +69,51 @@ public ApplyStageBackup(Path serverRoot) throws IOException {
*
* @throws IOException - if unable to backup the files
*/
public void recordAll() {
try {
Files.walkFileTree(serverRoot, new SimpleFileVisitor<>() {
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
if (dir.equals(backupRoot)) {
return FileVisitResult.SKIP_SUBTREE;
} else {
final Path relative = serverRoot.relativize(dir);
Files.createDirectories(backupRoot.resolve(relative));
return FileVisitResult.CONTINUE;
}
public void recordAll() throws IOException {
Files.walkFileTree(serverRoot, new SimpleFileVisitor<>() {

@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
// if the file in the installation is not readable AND it does not exist in the candidate
// we assume it is user controlled file and we ignore it
return ignoreUserManagedFiles(file, exc);
}

@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {

if (dir.equals(backupRoot)) {
return FileVisitResult.SKIP_SUBTREE;
} else {
final Path relative = serverRoot.relativize(dir);
Files.createDirectories(backupRoot.resolve(relative));
return FileVisitResult.CONTINUE;
}
}

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
final Path relative = serverRoot.relativize(file);
if (relative.startsWith(Path.of(".installation", ".git"))) {
// when using hardlinks, we need to remove the file and copy the new one in it's place otherwise both would be changed.
// the git folder is manipulated by jgit and we can't control how it operates on the files
// therefore we need to copy the files upfront rather than hardlinking them
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (isUserProtectedFile(file)) {
return FileVisitResult.SKIP_SUBTREE;
}
final Path relative = serverRoot.relativize(file);
if (relative.startsWith(Path.of(".installation", ".git"))) {
// when using hardlinks, we need to remove the file and copy the new one in it's place otherwise both would be changed.
// the git folder is manipulated by jgit and we can't control how it operates on the files
// therefore we need to copy the files upfront rather than hardlinking them
Files.copy(file, backupRoot.resolve(relative));
} else {
// we try to use hardlinks instead of copy to save disk space
// fallback on copy if Filesystem doesn't support hardlinks
try {
Files.createLink(backupRoot.resolve(relative), file);
} catch (UnsupportedOperationException e) {
Files.copy(file, backupRoot.resolve(relative));
} else {
// we try to use hardlinks instead of copy to save disk space
// fallback on copy if Filesystem doesn't support hardlinks
try {
Files.createLink(backupRoot.resolve(relative), file);
} catch (UnsupportedEncodingException e) {
Files.copy(file, backupRoot.resolve(relative));
}
}
return FileVisitResult.CONTINUE;
}
});
} catch (IOException e) {
throw new RuntimeException(e);
}
return FileVisitResult.CONTINUE;
}
});
}

/**
Expand Down Expand Up @@ -138,8 +147,17 @@ public void restore() throws IOException {

private SimpleFileVisitor<Path> deleteNewFiles() {
return new SimpleFileVisitor<>() {

@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
return ignoreUserManagedFiles(file, exc);
}

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (isUserProtectedFile(file)) {
return FileVisitResult.SKIP_SUBTREE;
}
final Path relativePath = serverRoot.relativize(file);
if (!Files.exists(backupRoot.resolve(relativePath))) {
if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
Expand Down Expand Up @@ -175,6 +193,21 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th
};
}

private FileVisitResult ignoreUserManagedFiles(Path file, IOException exc) throws IOException {
// if the file in the installation is not readable AND it does not exist in the candidate
// we assume it is user controlled file and we ignore it
if (isUserProtectedFile(file)) {
return FileVisitResult.SKIP_SUBTREE;
}
throw exc;
}

private boolean isUserProtectedFile(Path file) {
final Path relative = serverRoot.relativize(file);
final Path candidatePath = candidateRoot.resolve(relative);
return !Files.isReadable(file) && !Files.exists(candidatePath);
}

private SimpleFileVisitor<Path> restoreModifiedFiles() {
return new SimpleFileVisitor<>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,44 @@ public void testFindConflictsInSystemPaths() throws Exception {
);
}

@Test
public void ignoreAddedNonReadableFolders() throws Exception {
final DirState expectedState = dirBuilder
.addFile("prod1/p1.txt", "p1 1.0.1")
.addFile("prod1/p2.txt", "p2 1.0.1")
.addFile("test/test.txt", "test")
.build();

// build test packages
creator.newFeaturePack(FeaturePackLocation.fromString(FPL_100).getFPID())
.addSystemPaths("prod1")
.newPackage("p1", true)
.writeContent("prod1/p1.txt", "p1 1.0.0") // modified by the user
.writeContent("prod1/p2.txt", "p2 1.0.0") // removed by user and restored in update
.getFeaturePack();
creator.newFeaturePack(FeaturePackLocation.fromString(FPL_101).getFPID())
.addSystemPaths("prod1")
.newPackage("p1", true)
.writeContent("prod1/p1.txt", "p1 1.0.1")
.writeContent("prod1/p2.txt", "p2 1.0.1")
.getFeaturePack();
creator.install();

// install base and update. perform apply-update
install(installationPath, FPL_100);
Files.createDirectories(installationPath.resolve("test"));
Files.writeString(installationPath.resolve("test").resolve("test.txt"), "test");
installationPath.resolve("test").toFile().setReadable(false);

prepareUpdate(updatePath, installationPath, FPL_101);
new ApplyCandidateAction(installationPath, updatePath)
.applyUpdate(ApplyCandidateAction.Type.UPDATE);

// verify
installationPath.resolve("test").toFile().setReadable(true);
expectedState.assertState(installationPath);
}

private void createSimpleFeaturePacks() throws ProvisioningException {
creator.newFeaturePack(FeaturePackLocation.fromString(FPL_100).getFPID())
.newPackage("p1", true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
import org.apache.commons.io.FileUtils;
import org.assertj.core.api.Assertions;
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import java.io.IOException;
import java.nio.file.AccessDeniedException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
Expand All @@ -23,13 +25,15 @@ public class ApplyStageBackupTest {

private ApplyStageBackup backup;
private Path server;
private Path candidate;
private Path backupFolder;

@Before
public void setUp() throws Exception {
server = temp.newFolder().toPath();
candidate = temp.newFolder().toPath();
backupFolder = server.resolve(ApplyStageBackup.BACKUP_FOLDER);
backup = new ApplyStageBackup(server);
backup = new ApplyStageBackup(server, candidate);
}

@After
Expand Down Expand Up @@ -237,7 +241,7 @@ public void dontRemoveUnchangedFiles() throws Exception {
public void cleanBackupFolderAfterInit() throws Exception {
final Path existingFile = backupFolder.resolve("pre-existing.file");
Files.writeString(existingFile, "test");
backup = new ApplyStageBackup(server);
backup = new ApplyStageBackup(server, candidate);

assertThat(existingFile)
.doesNotExist();
Expand All @@ -249,7 +253,7 @@ public void createNonExistingBackupFolder() throws Exception {
if (Files.exists(backupFolder)) {
FileUtils.forceDelete(backupFolder.toFile());
}
backup = new ApplyStageBackup(server);
backup = new ApplyStageBackup(server, candidate);

assertThat(backupFolder)
.exists();
Expand All @@ -262,12 +266,61 @@ public void throwsExceptionIfBackupFolderIsFile() throws Exception {
FileUtils.forceDelete(backupFolder.toFile());
}
Files.writeString(backupFolder, "foo");
Assertions.assertThatThrownBy(()->new ApplyStageBackup(server))
Assertions.assertThatThrownBy(()->new ApplyStageBackup(server, candidate))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("Unable to create backup");

}

@Test
public void ignoreNonReadableUserFiles() throws Exception {
final Path testFile = server.resolve("test.txt");
final Path testDir = server.resolve("test");
Files.createDirectories(testDir);
writeFile(testFile);

try {
Assume.assumeTrue("Skipping test because OS doesn't support setting folders un-readable", testFile.toFile().setReadable(false));
Assume.assumeTrue("Skipping test because OS doesn't support setting folders un-readable", testDir.toFile().setReadable(false));

backup.recordAll();
backup.restore();

assertThat(testFile)
.exists();
assertThat(testDir)
.exists();
} finally {
testFile.toFile().setReadable(true);
testDir.toFile().setReadable(true);
}
}

@Test
public void throwExceptionOnNonReadableServerFiles() throws Exception {
final Path testFile = server.resolve("test");
final Path candidateFile = candidate.resolve("test");
Files.createDirectories(testFile);
Files.createDirectories(candidateFile);

try {
Assume.assumeTrue("Skipping test because OS doesn't support setting folders un-readable", testFile.toFile().setReadable(false));

Assertions.assertThatThrownBy(()->backup.recordAll())
.isInstanceOf(AccessDeniedException.class)
.hasMessageContaining(testFile.toString());

Assertions.assertThatThrownBy(()->backup.restore())
.isInstanceOf(AccessDeniedException.class)
.hasMessageContaining(testFile.toString());

assertThat(testFile)
.exists();
} finally {
testFile.toFile().setReadable(true);
}
}

private Path createFile(String path) throws IOException {
final Path testFile = server.resolve(path);
if (!Files.exists(testFile.getParent())) {
Expand Down

0 comments on commit 63e1c82

Please sign in to comment.