Skip to content

Commit

Permalink
Actually test directory name generation
Browse files Browse the repository at this point in the history
  • Loading branch information
@ties
ties committed Dec 8, 2023
1 parent b4cdd37 commit 3679fc9
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 28 deletions.
5 changes: 3 additions & 2 deletions src/main/java/net/ripe/rpki/rsyncit/config/AppConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.springframework.context.annotation.Bean;
import org.springframework.stereotype.Component;

import java.nio.file.Path;
import java.time.Duration;
import java.util.Map;
import java.util.function.Function;
Expand All @@ -21,7 +22,7 @@ public class AppConfig implements InfoContributor {

private final String rrdpUrl;
private final String rrdpReplaceHostWith;
private final String rsyncPath;
private final Path rsyncPath;
private final String cron;
private final Duration requestTimeout;
private final ApplicationInfo info;
Expand All @@ -30,7 +31,7 @@ public class AppConfig implements InfoContributor {

public AppConfig(@Value("${rrdpUrl}") String rrdpUrl,
@Value("${rrdpReplaceHost:}") String rrdpReplaceHostWith,
@Value("${rsyncPath}") String rsyncPath,
@Value("${rsyncPath}") Path rsyncPath,
// Run every 10 minutes
@Value("${cron:0 0/10 * * * ?}") String cron,
// 3 minutes by default
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/net/ripe/rpki/rsyncit/config/Config.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

import lombok.With;

import java.nio.file.Path;
import java.time.Duration;
import java.util.function.Function;

@With
public record Config(
String rrdpUrl,
Function<String, String> substituteHost,
String rsyncPath,
Path rsyncPath,
String cron,
Duration requestTimeout,
long targetDirectoryRetentionPeriodMs,
Expand Down
20 changes: 11 additions & 9 deletions src/main/java/net/ripe/rpki/rsyncit/rsync/RsyncWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import net.ripe.rpki.rsyncit.rrdp.RpkiObject;
import org.apache.tomcat.util.http.fileupload.FileUtils;

import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URI;
Expand Down Expand Up @@ -55,10 +54,9 @@ public RsyncWriter(Config config) {
public Path writeObjects(List<RpkiObject> objects) {
try {
final Instant now = Instant.now();
var baseDirectory = Paths.get(config.rsyncPath());
final Path targetDirectory = writeObjectToNewDirectory(objects, now);
atomicallyReplacePublishedSymlink(Paths.get(config.rsyncPath()), targetDirectory);
cleanupOldTargetDirectories(now, baseDirectory);
atomicallyReplacePublishedSymlink(config.rsyncPath(), targetDirectory);
cleanupOldTargetDirectories(now, config.rsyncPath());
return targetDirectory;
} catch (Exception e) {
throw new RuntimeException(e);
Expand All @@ -73,9 +71,7 @@ private Path writeObjectToNewDirectory(List<RpkiObject> objects, Instant now) th
final Map<String, List<RpkiObject>> groupedByHost =
objects.stream().collect(Collectors.groupingBy(o -> o.url().getHost()));

final String formattedNow = DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(now);

final Path temporaryDirectory = Files.createTempDirectory(Paths.get(config.rsyncPath()), "tmp-" + formattedNow + "-");
final Path temporaryDirectory = Files.createTempDirectory(config.rsyncPath(), "rsync-writer-tmp");
try {
groupedByHost.forEach((hostName, os) -> {
// create a directory per hostname (in realistic cases there will be just one)
Expand Down Expand Up @@ -131,8 +127,8 @@ private Path writeObjectToNewDirectory(List<RpkiObject> objects, Instant now) th
hostName);
});

// Calculate target directory after writing phase, to be sure variable is not used beforehand.
final Path targetDirectory = Paths.get(config.rsyncPath()).resolve("published-" + formattedNow);
// Calculate target directory after writing phase, to be sure it is not used beforehand.
final Path targetDirectory = generatePublicationDirectoryPath(config.rsyncPath(), now);

// Directory write is fully complete, rename temporary to target directory name
Files.setLastModifiedTime(temporaryDirectory, FileTime.from(now));
Expand All @@ -148,6 +144,12 @@ private Path writeObjectToNewDirectory(List<RpkiObject> objects, Instant now) th
}
}

static Path generatePublicationDirectoryPath(Path baseDir, Instant now) {
var timeSegment = DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(now);

return baseDir.resolve("published-" + timeSegment);
}

static List<RpkiObject> filterOutBadUrls(Path hostBasedPath, Collection<RpkiObject> objects) {
final String normalizedHostPath = hostBasedPath.normalize().toString();
return objects.stream().flatMap(object -> {
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/net/ripe/rpki/TestDefaults.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import net.ripe.rpki.rsyncit.config.Config;
import org.springframework.web.reactive.function.client.WebClient;

import java.nio.file.Paths;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.function.Function;
Expand All @@ -11,7 +12,7 @@ public class TestDefaults {
public static Config defaultConfig() {
return new Config("https://rrdp.ripe.net/notification.xml",
Function.identity(),
"/tmp/rsync",
Paths.get("/tmp/rsync"),
"0 0/10 * * * ?",
Duration.of(1, ChronoUnit.MINUTES),
3600_000, 10);
Expand Down
45 changes: 30 additions & 15 deletions src/test/java/net/ripe/rpki/rsyncit/rsync/RsyncWriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.time.Instant;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.Arrays;
import java.util.Collections;
import java.util.Random;
Expand All @@ -30,10 +31,24 @@
class RsyncWriterTest {

@Test
public void testDateFormat() {
public void testPublicationDirectoryNaming(@TempDir Path tempDir) {
Instant now = Instant.now();
final String formattedNow = DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(now);
assertNotNull(formattedNow);
Function<Instant, String> format = then -> DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(then);

var d1 = RsyncWriter.generatePublicationDirectoryPath(tempDir, now);
var d2 = RsyncWriter.generatePublicationDirectoryPath(tempDir, now.plus(25, ChronoUnit.HOURS));

assertThat(d1.startsWith(tempDir));
assertThat(d1).isNotEqualTo(d2);
// sibling dirs
assertThat(d1.getParent()).isEqualTo(d2.getParent());

assertThat(d1.toString())
.contains("published-")
.contains(format.apply(now));

assertThat(d2.toString())
.contains(format.apply(now.plus(25, ChronoUnit.HOURS)));
}

@Test
Expand All @@ -59,13 +74,13 @@ public void testWriteMultipleObjects(@TempDir Path tmpPath) throws Exception {
var o5 = new RpkiObject(URI.create("rsync://bla.net/path2/nested1/nested2/e.cer"), someBytes(), Instant.now());
var o6 = new RpkiObject(URI.create("rsync://different.net/path2/nested1/nested2/e.cer"), someBytes(), Instant.now());
rsyncWriter.writeObjects(Arrays.asList(o1, o2, o3, o4, o5, o6));
final String root = rsyncWriter.getConfig().rsyncPath();
checkFile(Paths.get(root, "published", "bla.net", "path1", "a.cer"), o1.bytes());
checkFile(Paths.get(root, "published", "bla.net", "path1", "b.cer"), o2.bytes());
checkFile(Paths.get(root, "published", "bla.net", "path1", "nested", "c.cer"), o3.bytes());
checkFile(Paths.get(root, "published", "bla.net", "path2", "d.cer"), o4.bytes());
checkFile(Paths.get(root, "published", "bla.net", "path2", "nested1", "nested2", "e.cer"), o5.bytes());
checkFile(Paths.get(root, "published", "different.net", "path2", "nested1", "nested2", "e.cer"), o6.bytes());
var root = rsyncWriter.getConfig().rsyncPath();
checkFile(root.resolve("published/bla.net/path1/a.cer"), o1.bytes());
checkFile(root.resolve("published/bla.net/path1/b.cer"), o2.bytes());
checkFile(root.resolve("published/bla.net/path1/nested/c.cer"), o3.bytes());
checkFile(root.resolve("published/bla.net/path2/d.cer"), o4.bytes());
checkFile(root.resolve("published/bla.net/path2/nested1/nested2/e.cer"), o5.bytes());
checkFile(root.resolve("published/different.net/path2/nested1/nested2/e.cer"), o6.bytes());
});
}

Expand All @@ -76,10 +91,10 @@ public void testIgnoreBadUrls(@TempDir Path tmpPath) throws Exception {
var o2 = new RpkiObject(URI.create("rsync://bla.net/path1/../../PATH_INJECTION.txt"), someBytes(), Instant.now());
var o3 = new RpkiObject(URI.create("rsync://bla.net/path1/path2/../NOT_REALLY_PATH_INJECTION.txt"), someBytes(), Instant.now());
rsyncWriter.writeObjects(Arrays.asList(o1, o2, o3));
final String root = rsyncWriter.getConfig().rsyncPath();
checkFile(Paths.get(root, "published", "bla.net", "path1", "a.cer"), o1.bytes());
assertThat(Paths.get(root, "published", "PATH_INJECTION.txt").toFile().exists()).isFalse();
checkFile(Paths.get(root, "published", "bla.net", "path1", "NOT_REALLY_PATH_INJECTION.txt"), o3.bytes());
var root = rsyncWriter.getConfig().rsyncPath();
checkFile(root.resolve("published/bla.net/path1/a.cer"), o1.bytes());
assertThat(root.resolve( "published/PATH_INJECTION.txt").toFile().exists()).isFalse();
checkFile(root.resolve( "published/bla.net/path1/NOT_REALLY_PATH_INJECTION.txt"), o3.bytes());
});
}

Expand Down Expand Up @@ -145,7 +160,7 @@ private static void checkFile(Path path, byte[] bytes) throws IOException {
}

static void withRsyncWriter(Path tmpPath, Function<Config, Config> transformConfig, ThrowingConsumer<RsyncWriter> actualTest) throws Exception {
final Config config = defaultConfig().withRsyncPath(tmpPath.toString());
final Config config = defaultConfig().withRsyncPath(tmpPath);
actualTest.accept(new RsyncWriter(transformConfig.apply(config)));
}

Expand Down

0 comments on commit 3679fc9

Please sign in to comment.