Use repository specific work dirs (#1510)

With this change, work dirs are created in the
directory of the repository and no longer in the
global scm work dir directory. This is relevant due
to two facts:

1. Repositories may contain confidential data and therefore
   reside in special directories (that may be mounted on
   special drives). It may be considered a breach when these
   directories are cloned or otherwise copied to global
   temporary drives.
2. Big repositories may overload global temp spaces. It may be
   easier to create special drives with more space for such
   big repositories.
This commit is contained in:
René Pfeuffer
2021-01-28 12:53:39 +01:00
committed by GitHub
parent 57c9484d41
commit bd3671b428
23 changed files with 167 additions and 43 deletions

View File

@@ -81,9 +81,9 @@ public class ModifyCommandBuilder {
private final ModifyCommandRequest request = new ModifyCommandRequest();
ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider, @Nullable EMail eMail) {
ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider, String repositoryId, @Nullable EMail eMail) {
this.command = command;
this.workdir = workdirProvider.createNewWorkdir();
this.workdir = workdirProvider.createNewWorkdir(repositoryId);
this.eMail = eMail;
}

View File

@@ -444,7 +444,7 @@ public final class RepositoryService implements Closeable {
LOG.debug("create modify command for repository {}",
repository.getNamespaceAndName());
return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider, eMail);
return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider, repository.getId(), eMail);
}
/**

View File

@@ -46,7 +46,7 @@ public class NoneCachingWorkingCopyPool implements WorkingCopyPool {
@Override
public <R, W> WorkingCopy<R, W> getWorkingCopy(SimpleWorkingCopyFactory<R, W, ?>.WorkingCopyContext context) {
return context.initialize(workdirProvider.createNewWorkdir());
return context.initialize(workdirProvider.createNewWorkdir(context.getScmRepository().getId()));
}
@Override

View File

@@ -96,7 +96,7 @@ public class SimpleCachingWorkingCopyPool implements WorkingCopyPool {
private <R, W> WorkingCopy<R, W> createNewWorkingCopy(SimpleWorkingCopyFactory<R, W, ?>.WorkingCopyContext workingCopyContext) {
Stopwatch stopwatch = Stopwatch.createStarted();
File newWorkdir = workdirProvider.createNewWorkdir();
File newWorkdir = workdirProvider.createNewWorkdir(workingCopyContext.getScmRepository().getId());
WorkingCopy<R, W> parentAndClone = workingCopyContext.initialize(newWorkdir);
LOG.debug("initialized new workdir for {} in path {} in {}", workingCopyContext.getScmRepository(), newWorkdir, stopwatch.stop());
return parentAndClone;

View File

@@ -31,6 +31,10 @@ public class WorkdirCreationException extends ExceptionWithContext {
public static final String CODE = "3tS0mjSoo1";
public WorkdirCreationException(String path) {
super(ContextEntry.ContextBuilder.entity("Path", path).build(), "Could not create directory " + path);
}
public WorkdirCreationException(String path, Exception cause) {
super(ContextEntry.ContextBuilder.entity("Path", path).build(), "Could not create directory " + path, cause);
}

View File

@@ -24,30 +24,55 @@
package sonia.scm.repository.work;
import sonia.scm.repository.RepositoryLocationResolver;
import javax.inject.Inject;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
public class WorkdirProvider {
private final File rootDirectory;
private final RepositoryLocationResolver repositoryLocationResolver;
private final boolean useRepositorySpecificDir;
public WorkdirProvider() {
this(new File(System.getProperty("scm.workdir" , System.getProperty("java.io.tmpdir")), "scm-work"));
@Inject
public WorkdirProvider(RepositoryLocationResolver repositoryLocationResolver) {
this(new File(System.getProperty("scm.workdir" , System.getProperty("java.io.tmpdir")), "scm-work"), repositoryLocationResolver, System.getProperty("scm.workdir") == null);
}
public WorkdirProvider(File rootDirectory) {
public WorkdirProvider(File rootDirectory, RepositoryLocationResolver repositoryLocationResolver, boolean useRepositorySpecificDir) {
this.rootDirectory = rootDirectory;
this.repositoryLocationResolver = repositoryLocationResolver;
this.useRepositorySpecificDir = useRepositorySpecificDir;
if (!rootDirectory.exists() && !rootDirectory.mkdirs()) {
throw new IllegalStateException("could not create pool directory " + rootDirectory);
}
}
public File createNewWorkdir() {
return createWorkDir(this.rootDirectory);
}
public File createNewWorkdir(String repositoryId) {
if (useRepositorySpecificDir) {
return createWorkDir(repositoryLocationResolver.forClass(Path.class).getLocation(repositoryId).resolve("work").toFile());
} else {
return createNewWorkdir();
}
}
private File createWorkDir(File baseDirectory) {
// recreate base directory when it may be deleted (see https://github.com/scm-manager/scm-manager/issues/1493 for example)
if (!baseDirectory.exists() && !baseDirectory.mkdirs()) {
throw new WorkdirCreationException(baseDirectory.toString());
}
try {
return Files.createTempDirectory(rootDirectory.toPath(),"workdir").toFile();
return Files.createTempDirectory(baseDirectory.toPath(),"work-").toFile();
} catch (IOException e) {
throw new WorkdirCreationException(rootDirectory.toString(), e);
throw new WorkdirCreationException(baseDirectory.toString(), e);
}
}
}

View File

@@ -83,8 +83,8 @@ class ModifyCommandBuilderTest {
@BeforeEach
void initWorkdir(@TempDir Path temp) throws IOException {
workdir = Files.createDirectory(temp.resolve("workdir"));
lenient().when(workdirProvider.createNewWorkdir()).thenReturn(workdir.toFile());
commandBuilder = new ModifyCommandBuilder(command, workdirProvider, new EMail(SCM_CONFIGURATION));
lenient().when(workdirProvider.createNewWorkdir("1")).thenReturn(workdir.toFile());
commandBuilder = new ModifyCommandBuilder(command, workdirProvider, "1", new EMail(SCM_CONFIGURATION));
}
@BeforeEach

View File

@@ -38,6 +38,7 @@ import java.nio.file.Path;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
@@ -68,7 +69,7 @@ class SimpleCachingWorkingCopyPoolTest {
@Test
void shouldCreateNewWorkdirForTheFirstRequest(@TempDir Path temp) {
when(workingCopyContext.getScmRepository()).thenReturn(REPOSITORY);
when(workdirProvider.createNewWorkdir()).thenReturn(temp.toFile());
when(workdirProvider.createNewWorkdir(anyString())).thenReturn(temp.toFile());
WorkingCopy<?, ?> workdir = simpleCachingWorkingCopyPool.getWorkingCopy(workingCopyContext);
@@ -78,7 +79,7 @@ class SimpleCachingWorkingCopyPoolTest {
@Test
void shouldCreateWorkdirOnlyOnceForTheSameRepository(@TempDir Path temp) throws SimpleWorkingCopyFactory.ReclaimFailedException {
when(workingCopyContext.getScmRepository()).thenReturn(REPOSITORY);
when(workdirProvider.createNewWorkdir()).thenReturn(temp.toFile());
when(workdirProvider.createNewWorkdir(anyString())).thenReturn(temp.toFile());
WorkingCopy<?, ?> firstWorkdir = simpleCachingWorkingCopyPool.getWorkingCopy(workingCopyContext);
simpleCachingWorkingCopyPool.contextClosed(workingCopyContext, firstWorkdir.getDirectory());
@@ -96,7 +97,7 @@ class SimpleCachingWorkingCopyPoolTest {
firstDirectory.mkdirs();
File secondDirectory = temp.resolve("second").toFile();
secondDirectory.mkdirs();
when(workdirProvider.createNewWorkdir()).thenReturn(
when(workdirProvider.createNewWorkdir(anyString())).thenReturn(
firstDirectory,
secondDirectory);

View File

@@ -29,16 +29,20 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryProvider;
import sonia.scm.util.IOUtil;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class SimpleWorkingCopyFactoryTest {
@@ -51,6 +55,8 @@ public class SimpleWorkingCopyFactoryTest {
public TemporaryFolder temporaryFolder = new TemporaryFolder();
private SimpleWorkingCopyFactory<Closeable, Closeable, Context> simpleWorkingCopyFactory;
private final RepositoryLocationResolver repositoryLocationResolver = mock(RepositoryLocationResolver.class);
private String initialBranchForLastCloneCall;
private boolean workdirIsCached = false;
@@ -58,11 +64,11 @@ public class SimpleWorkingCopyFactoryTest {
@Before
public void initFactory() throws IOException {
WorkdirProvider workdirProvider = new WorkdirProvider(temporaryFolder.newFolder());
WorkdirProvider workdirProvider = new WorkdirProvider(temporaryFolder.newFolder(), repositoryLocationResolver, false);
WorkingCopyPool configurableTestWorkingCopyPool = new WorkingCopyPool() {
@Override
public <R, W> WorkingCopy<R, W> getWorkingCopy(SimpleWorkingCopyFactory<R, W, ?>.WorkingCopyContext context) {
workdir = workdirProvider.createNewWorkdir();
workdir = workdirProvider.createNewWorkdir(context.getScmRepository().getId());
return context.initialize(workdir);
}
@@ -99,6 +105,10 @@ public class SimpleWorkingCopyFactoryTest {
workingCopy.close();
}
};
RepositoryLocationResolver.RepositoryLocationResolverInstance instanceMock = mock(RepositoryLocationResolver.RepositoryLocationResolverInstance.class);
when(repositoryLocationResolver.forClass(Path.class)).thenReturn(instanceMock);
when(instanceMock.getLocation(anyString())).thenAnswer(invocation -> temporaryFolder.newFolder(invocation.getArgument(0, String.class)).toPath());
}
@Test

View File

@@ -0,0 +1,93 @@
/*
* MIT License
*
* Copyright (c) 2020-present Cloudogu GmbH and Contributors
*
* 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 sonia.scm.repository.work;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryLocationResolver.RepositoryLocationResolverInstance;
import java.io.File;
import java.nio.file.Path;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
class WorkdirProviderTest {
@Mock
private RepositoryLocationResolver repositoryLocationResolver;
@Mock
private RepositoryLocationResolverInstance<Path> repositoryLocationResolverInstance;
@BeforeEach
void initResolver(@TempDir Path temp) {
lenient().when(repositoryLocationResolver.forClass(Path.class)).thenReturn(repositoryLocationResolverInstance);
}
@Test
void shouldUseGlobalTempDirectory(@TempDir Path temp) {
WorkdirProvider provider = new WorkdirProvider(temp.toFile(), repositoryLocationResolver, true);
File newWorkdir = provider.createNewWorkdir();
assertThat(newWorkdir).exists();
assertThat(newWorkdir).hasParent(temp.toFile());
verify(repositoryLocationResolverInstance, never()).getLocation(anyString());
}
@Test
void shouldUseRepositorySpecificDirectory(@TempDir Path temp) {
when(repositoryLocationResolverInstance.getLocation("42")).thenReturn(temp.resolve("repo-dir"));
WorkdirProvider provider = new WorkdirProvider(temp.toFile(), repositoryLocationResolver, true);
File newWorkdir = provider.createNewWorkdir("42");
assertThat(newWorkdir).exists();
assertThat(newWorkdir.getParentFile()).hasName("work");
assertThat(newWorkdir.getParentFile().getParentFile()).hasName("repo-dir");
}
@Test
void shouldUseGlobalDirectoryIfExplicitlySet(@TempDir Path temp) {
WorkdirProvider provider = new WorkdirProvider(temp.toFile(), repositoryLocationResolver, false);
File newWorkdir = provider.createNewWorkdir("42");
assertThat(newWorkdir).exists();
assertThat(newWorkdir).hasParent(temp.toFile());
verify(repositoryLocationResolverInstance, never()).getLocation(anyString());
}
}