Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ project.ext.dependencyStrings = [
JACKSON_DATABIND: 'com.fasterxml.jackson.core:jackson-databind',
JACKSON_DATAFORMAT_YAML: 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml',
JACKSON_DATATYPE_JSR310: 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310',
ASM: 'org.ow2.asm:asm:9.5',
ASM: 'org.ow2.asm:asm:9.8',
PICOCLI: 'info.picocli:picocli:4.7.4',

MAVEN_API: 'org.apache.maven:maven-plugin-api:3.9.3',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void testTarballStructure() throws IOException {
try (TarArchiveInputStream input =
new TarArchiveInputStream(Files.newInputStream(imageTar.toPath()))) {
TarArchiveEntry imageEntry;
while ((imageEntry = input.getNextTarEntry()) != null) {
while ((imageEntry = input.getNextEntry()) != null) {
actual.add(imageEntry.getName());
}
}
Expand Down Expand Up @@ -218,14 +218,14 @@ private void layerEntriesDo(BiConsumer<String, TarArchiveEntry> layerConsumer)
try (TarArchiveInputStream input =
new TarArchiveInputStream(Files.newInputStream(imageTar.toPath()))) {
TarArchiveEntry imageEntry;
while ((imageEntry = input.getNextTarEntry()) != null) {
while ((imageEntry = input.getNextEntry()) != null) {
String imageEntryName = imageEntry.getName();
// assume all .tar.gz files are layers
if (imageEntry.isFile() && imageEntryName.endsWith(".tar.gz")) {
@SuppressWarnings("resource") // must not close sub-streams
TarArchiveInputStream layer = new TarArchiveInputStream(new GZIPInputStream(input));
TarArchiveEntry layerEntry;
while ((layerEntry = layer.getNextTarEntry()) != null) {
while ((layerEntry = layer.getNextEntry()) != null) {
layerConsumer.accept(imageEntryName, layerEntry);
}
}
Expand All @@ -238,7 +238,7 @@ private static String extractFromTarFileAsString(File tarFile, String filename)
try (TarArchiveInputStream input =
new TarArchiveInputStream(Files.newInputStream(tarFile.toPath()))) {
TarArchiveEntry imageEntry;
while ((imageEntry = input.getNextTarEntry()) != null) {
while ((imageEntry = input.getNextEntry()) != null) {
if (filename.equals(imageEntry.getName())) {
return CharStreams.toString(new InputStreamReader(input, StandardCharsets.UTF_8));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ public static void extract(Path source, Path destination, boolean enableReproduc
List<TarArchiveEntry> entries = new ArrayList<>();
try (InputStream in = new BufferedInputStream(Files.newInputStream(source));
TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream(in)) {
for (TarArchiveEntry entry = tarArchiveInputStream.getNextTarEntry();
for (TarArchiveEntry entry = tarArchiveInputStream.getNextEntry();
entry != null;
entry = tarArchiveInputStream.getNextTarEntry()) {
entry = tarArchiveInputStream.getNextEntry()) {
Comment on lines +74 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with other parts of the codebase and for better readability, this for loop can be converted to a while loop. The while ((entry = ...getNextEntry()) != null) pattern is more idiomatic for iterating through archive entries.

      TarArchiveEntry entry;
      while ((entry = tarArchiveInputStream.getNextEntry()) != null) {

Comment on lines +74 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better readability and consistency with other parts of the codebase (e.g., ReproducibleImageTest.java), consider refactoring this for loop into a while loop. The while ((entry = ...)) {} pattern is more idiomatic in Java for this type of iteration.

      TarArchiveEntry entry;
      while ((entry = tarArchiveInputStream.getNextEntry()) != null) {

Comment on lines +74 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this for loop is correct, it can be simplified into a more idiomatic while loop. This would also make it consistent with other parts of the codebase where TarArchiveInputStream is used (for example, in ReproducibleImageTest.java).

Consider refactoring this loop to:

      TarArchiveEntry entry;
      while ((entry = tarArchiveInputStream.getNextEntry()) != null) {
        // ... loop body
      }

entries.add(entry);
Path entryPath = destination.resolve(entry.getName());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,31 +105,31 @@ public void testWriteTo_docker()
try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream(in)) {

// Verifies layer with fileA was added.
TarArchiveEntry headerFileALayer = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileALayer = tarArchiveInputStream.getNextEntry();
Assert.assertEquals(fakeDigestA.getHash() + ".tar.gz", headerFileALayer.getName());
String fileAString =
CharStreams.toString(
new InputStreamReader(tarArchiveInputStream, StandardCharsets.UTF_8));
Assert.assertEquals(Blobs.writeToString(Blobs.from(fileA)), fileAString);

// Verifies layer with fileB was added.
TarArchiveEntry headerFileBLayer = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileBLayer = tarArchiveInputStream.getNextEntry();
Assert.assertEquals(fakeDigestB.getHash() + ".tar.gz", headerFileBLayer.getName());
String fileBString =
CharStreams.toString(
new InputStreamReader(tarArchiveInputStream, StandardCharsets.UTF_8));
Assert.assertEquals(Blobs.writeToString(Blobs.from(fileB)), fileBString);

// Verifies container configuration was added.
TarArchiveEntry headerContainerConfiguration = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerContainerConfiguration = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("config.json", headerContainerConfiguration.getName());
String containerConfigJson =
CharStreams.toString(
new InputStreamReader(tarArchiveInputStream, StandardCharsets.UTF_8));
JsonTemplateMapper.readJson(containerConfigJson, ContainerConfigurationTemplate.class);

// Verifies manifest was added.
TarArchiveEntry headerManifest = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerManifest = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("manifest.json", headerManifest.getName());
String manifestJson =
CharStreams.toString(
Expand Down Expand Up @@ -159,45 +159,45 @@ public void testWriteTo_oci()
try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream(in)) {

// Verifies layer with fileA was added.
TarArchiveEntry headerFileALayer = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileALayer = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("blobs/sha256/" + fakeDigestA.getHash(), headerFileALayer.getName());
String fileAString =
CharStreams.toString(
new InputStreamReader(tarArchiveInputStream, StandardCharsets.UTF_8));
Assert.assertEquals(Blobs.writeToString(Blobs.from(fileA)), fileAString);

// Verifies layer with fileB was added.
TarArchiveEntry headerFileBLayer = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileBLayer = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("blobs/sha256/" + fakeDigestB.getHash(), headerFileBLayer.getName());
String fileBString =
CharStreams.toString(
new InputStreamReader(tarArchiveInputStream, StandardCharsets.UTF_8));
Assert.assertEquals(Blobs.writeToString(Blobs.from(fileB)), fileBString);

// Verifies container configuration was added.
TarArchiveEntry headerContainerConfiguration = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerContainerConfiguration = tarArchiveInputStream.getNextEntry();
Assert.assertEquals(
"blobs/sha256/011212cff4d5d6b18c7d3a00a7a2701514a1fdd3ec0d250a03756f84f3d955d4",
headerContainerConfiguration.getName());
JsonTemplateMapper.readJson(tarArchiveInputStream, ContainerConfigurationTemplate.class);

// Verifies manifest was added.
TarArchiveEntry headerManifest = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerManifest = tarArchiveInputStream.getNextEntry();
Assert.assertEquals(
"blobs/sha256/1543d061159a8d6877087938bfd62681cdeff873e1fa3e1fcf12dec358c112a4",
headerManifest.getName());
JsonTemplateMapper.readJson(tarArchiveInputStream, OciManifestTemplate.class);

// Verifies oci-layout was added.
TarArchiveEntry headerOciLayout = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerOciLayout = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("oci-layout", headerOciLayout.getName());
String ociLayoutJson =
CharStreams.toString(
new InputStreamReader(tarArchiveInputStream, StandardCharsets.UTF_8));
Assert.assertEquals("{\"imageLayoutVersion\": \"1.0.0\"}", ociLayoutJson);

// Verifies index.json was added.
TarArchiveEntry headerIndex = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerIndex = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("index.json", headerIndex.getName());
OciIndexTemplate index =
JsonTemplateMapper.readJson(tarArchiveInputStream, OciIndexTemplate.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class ReproducibleLayerBuilderTest {
private static void verifyNextTarArchiveEntry(
TarArchiveInputStream tarArchiveInputStream, String expectedExtractionPath, Path expectedFile)
throws IOException {
TarArchiveEntry header = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry header = tarArchiveInputStream.getNextEntry();
assertThat(header.getName()).isEqualTo(expectedExtractionPath);

byte[] expectedBytes = Files.readAllBytes(expectedFile);
Expand All @@ -78,7 +78,7 @@ private static void verifyNextTarArchiveEntry(
private static void verifyNextTarArchiveEntryIsDirectory(
TarArchiveInputStream tarArchiveInputStream, String expectedExtractionPath)
throws IOException {
TarArchiveEntry extractionPathEntry = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry extractionPathEntry = tarArchiveInputStream.getNextEntry();
assertThat(extractionPathEntry.getName()).isEqualTo(expectedExtractionPath);
assertThat(extractionPathEntry.isDirectory()).isTrue();
assertThat(extractionPathEntry.getMode()).isEqualTo(TarArchiveEntry.DEFAULT_DIR_MODE);
Expand Down Expand Up @@ -234,7 +234,7 @@ public void testBuild_parentDirBehavior() throws IOException {

try (TarArchiveInputStream in = new TarArchiveInputStream(Files.newInputStream(tarFile))) {
// root (default folder permissions)
TarArchiveEntry root = in.getNextTarEntry();
TarArchiveEntry root = in.getNextEntry();
assertThat(root.getMode()).isEqualTo(040755);
assertThat(root.getModTime().toInstant()).isEqualTo(Instant.ofEpochSecond(1));
assertThat(root.getLongUserId()).isEqualTo(0);
Expand All @@ -243,7 +243,7 @@ public void testBuild_parentDirBehavior() throws IOException {
assertThat(root.getGroupName()).isEmpty();

// parentAAA (custom permissions, custom timestamp)
TarArchiveEntry rootParentA = in.getNextTarEntry();
TarArchiveEntry rootParentA = in.getNextEntry();
assertThat(rootParentA.getMode()).isEqualTo(040111);
assertThat(rootParentA.getModTime().toInstant()).isEqualTo(Instant.ofEpochSecond(10));
assertThat(rootParentA.getLongUserId()).isEqualTo(0);
Expand All @@ -252,10 +252,10 @@ public void testBuild_parentDirBehavior() throws IOException {
assertThat(rootParentA.getGroupName()).isEmpty();

// skip over fileA
in.getNextTarEntry();
in.getNextEntry();

// parentBBB (default permissions - ignored custom permissions, since fileB added first)
TarArchiveEntry rootParentB = in.getNextTarEntry();
TarArchiveEntry rootParentB = in.getNextEntry();
// TODO (#1650): we want 040444 here.
assertThat(rootParentB.getMode()).isEqualTo(040755);
// TODO (#1650): we want Instant.ofEpochSecond(40) here.
Expand All @@ -266,10 +266,10 @@ public void testBuild_parentDirBehavior() throws IOException {
assertThat(rootParentB.getGroupName()).isEmpty();

// skip over fileB
in.getNextTarEntry();
in.getNextEntry();

// parentCCC (default permissions - no entry provided)
TarArchiveEntry rootParentC = in.getNextTarEntry();
TarArchiveEntry rootParentC = in.getNextEntry();
assertThat(rootParentC.getMode()).isEqualTo(040755);
assertThat(rootParentC.getModTime().toInstant()).isEqualTo(Instant.ofEpochSecond(1));
assertThat(rootParentC.getLongUserId()).isEqualTo(0);
Expand Down Expand Up @@ -358,13 +358,13 @@ public void testBuild_permissions() throws IOException {

try (TarArchiveInputStream in = new TarArchiveInputStream(Files.newInputStream(tarFile))) {
// Root folder (default folder permissions)
assertThat(in.getNextTarEntry().getMode()).isEqualTo(040755);
assertThat(in.getNextEntry().getMode()).isEqualTo(040755);
// fileA (default file permissions)
assertThat(in.getNextTarEntry().getMode()).isEqualTo(0100644);
assertThat(in.getNextEntry().getMode()).isEqualTo(0100644);
// fileB (custom file permissions)
assertThat(in.getNextTarEntry().getMode()).isEqualTo(0100123);
assertThat(in.getNextEntry().getMode()).isEqualTo(0100123);
// folder (custom folder permissions)
assertThat(in.getNextTarEntry().getMode()).isEqualTo(040456);
assertThat(in.getNextEntry().getMode()).isEqualTo(040456);
}
}

Expand Down Expand Up @@ -433,55 +433,55 @@ public void testBuild_ownership() throws IOException {
}

try (TarArchiveInputStream in = new TarArchiveInputStream(Files.newInputStream(tarFile))) {
TarArchiveEntry entry1 = in.getNextTarEntry();
TarArchiveEntry entry1 = in.getNextEntry();
assertThat(entry1.getLongUserId()).isEqualTo(0);
assertThat(entry1.getLongGroupId()).isEqualTo(0);
assertThat(entry1.getUserName()).isEmpty();
assertThat(entry1.getGroupName()).isEmpty();

TarArchiveEntry entry2 = in.getNextTarEntry();
TarArchiveEntry entry2 = in.getNextEntry();
assertThat(entry2.getLongUserId()).isEqualTo(0);
assertThat(entry2.getLongGroupId()).isEqualTo(0);
assertThat(entry2.getUserName()).isEmpty();
assertThat(entry2.getGroupName()).isEmpty();

TarArchiveEntry entry3 = in.getNextTarEntry();
TarArchiveEntry entry3 = in.getNextEntry();
assertThat(entry3.getLongUserId()).isEqualTo(0);
assertThat(entry3.getLongGroupId()).isEqualTo(0);
assertThat(entry3.getUserName()).isEmpty();
assertThat(entry3.getGroupName()).isEmpty();

TarArchiveEntry entry4 = in.getNextTarEntry();
TarArchiveEntry entry4 = in.getNextEntry();
assertThat(entry4.getLongUserId()).isEqualTo(333);
assertThat(entry4.getLongGroupId()).isEqualTo(0);
assertThat(entry4.getUserName()).isEmpty();
assertThat(entry4.getGroupName()).isEmpty();

TarArchiveEntry entry5 = in.getNextTarEntry();
TarArchiveEntry entry5 = in.getNextEntry();
assertThat(entry5.getLongUserId()).isEqualTo(0);
assertThat(entry5.getLongGroupId()).isEqualTo(555);
assertThat(entry5.getUserName()).isEmpty();
assertThat(entry5.getGroupName()).isEmpty();

TarArchiveEntry entry6 = in.getNextTarEntry();
TarArchiveEntry entry6 = in.getNextEntry();
assertThat(entry6.getLongUserId()).isEqualTo(333);
assertThat(entry6.getLongGroupId()).isEqualTo(555);
assertThat(entry6.getUserName()).isEmpty();
assertThat(entry6.getGroupName()).isEmpty();

TarArchiveEntry entry7 = in.getNextTarEntry();
TarArchiveEntry entry7 = in.getNextEntry();
assertThat(entry7.getLongUserId()).isEqualTo(0);
assertThat(entry7.getLongGroupId()).isEqualTo(0);
assertThat(entry7.getUserName()).isEqualTo("user");
assertThat(entry7.getGroupName()).isEmpty();

TarArchiveEntry entry8 = in.getNextTarEntry();
TarArchiveEntry entry8 = in.getNextEntry();
assertThat(entry8.getLongUserId()).isEqualTo(0);
assertThat(entry8.getLongGroupId()).isEqualTo(0);
assertThat(entry8.getUserName()).isEmpty();
assertThat(entry8.getGroupName()).isEqualTo("group");

TarArchiveEntry entry9 = in.getNextTarEntry();
TarArchiveEntry entry9 = in.getNextEntry();
assertThat(entry9.getLongUserId()).isEqualTo(0);
assertThat(entry9.getLongGroupId()).isEqualTo(0);
assertThat(entry9.getUserName()).isEqualTo("user");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,22 @@ public void testToBlob_multiByte() throws IOException {
TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream(tarByteInputStream);

// Verify multi-byte characters are written/read correctly
TarArchiveEntry headerFile = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFile = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("test", headerFile.getName());
Assert.assertEquals(
"日本語", new String(ByteStreams.toByteArray(tarArchiveInputStream), StandardCharsets.UTF_8));

headerFile = tarArchiveInputStream.getNextTarEntry();
headerFile = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("crepecake", headerFile.getName());
Assert.assertEquals(
"asdf", new String(ByteStreams.toByteArray(tarArchiveInputStream), StandardCharsets.UTF_8));

headerFile = tarArchiveInputStream.getNextTarEntry();
headerFile = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("jib", headerFile.getName());
Assert.assertEquals(
"jib", new String(ByteStreams.toByteArray(tarArchiveInputStream), StandardCharsets.UTF_8));

Assert.assertNull(tarArchiveInputStream.getNextTarEntry());
Assert.assertNull(tarArchiveInputStream.getNextEntry());
}

@Test
Expand All @@ -151,8 +151,8 @@ public void testToBlob_modificationTime() throws IOException {

TarArchiveInputStream tarInStream =
new TarArchiveInputStream(new ByteArrayInputStream(outStream.toByteArray()));
TarArchiveEntry entry1 = tarInStream.getNextTarEntry();
TarArchiveEntry entry2 = tarInStream.getNextTarEntry();
TarArchiveEntry entry1 = tarInStream.getNextEntry();
TarArchiveEntry entry2 = tarInStream.getNextEntry();

assertThat(entry1.getName()).isEqualTo("foo");
assertThat(entry1.getModTime().toInstant()).isEqualTo(Instant.ofEpochSecond(1234));
Expand Down Expand Up @@ -231,29 +231,29 @@ private void verifyBlobWithoutCompression() throws IOException {
*/
private void verifyTarArchive(TarArchiveInputStream tarArchiveInputStream) throws IOException {
// Verifies fileA was archived correctly.
TarArchiveEntry headerFileA = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileA = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("some/path/to/resourceFileA", headerFileA.getName());
byte[] fileAString = ByteStreams.toByteArray(tarArchiveInputStream);
Assert.assertArrayEquals(fileAContents, fileAString);

// Verifies fileB was archived correctly.
TarArchiveEntry headerFileB = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileB = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("crepecake", headerFileB.getName());
byte[] fileBString = ByteStreams.toByteArray(tarArchiveInputStream);
Assert.assertArrayEquals(fileBContents, fileBString);

// Verifies directoryA was archived correctly.
TarArchiveEntry headerDirectoryA = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerDirectoryA = tarArchiveInputStream.getNextEntry();
Assert.assertEquals("some/path/to/", headerDirectoryA.getName());

// Verifies the long file was archived correctly.
TarArchiveEntry headerFileALong = tarArchiveInputStream.getNextTarEntry();
TarArchiveEntry headerFileALong = tarArchiveInputStream.getNextEntry();
Assert.assertEquals(
"some/really/long/path/that/exceeds/100/characters/abcdefghijklmnopqrstuvwxyz0123456789012345678901234567890",
headerFileALong.getName());
byte[] fileALongString = ByteStreams.toByteArray(tarArchiveInputStream);
Assert.assertArrayEquals(fileAContents, fileALongString);

Assert.assertNull(tarArchiveInputStream.getNextTarEntry());
Assert.assertNull(tarArchiveInputStream.getNextEntry());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,13 @@ private List<List<String>> getLayers(Path tar) throws IOException {

try (TarArchiveInputStream image = new TarArchiveInputStream(Files.newInputStream(tar))) {
TarArchiveEntry entry;
while ((entry = image.getNextTarEntry()) != null) {
while ((entry = image.getNextEntry()) != null) {
if (entry.getName().endsWith(".tar.gz")) {
@SuppressWarnings("resource") // must not close sub-streams
TarArchiveInputStream layer = new TarArchiveInputStream(new GZIPInputStream(image));
TarArchiveEntry layerEntry;
List<String> layerFiles = new ArrayList<>();
while ((layerEntry = layer.getNextTarEntry()) != null) {
while ((layerEntry = layer.getNextEntry()) != null) {
layerFiles.add(layerEntry.getName());
}
layers.add(0, layerFiles);
Expand Down
Loading