Commit f560e86f authored by Scott Frederick's avatar Scott Frederick

Write buildpack directories to builder layer

When a custom buildpack is provided for image building, the contents
of the buildpack directory, tgz file, or image are copied as tar
entries to a new layer in the ephemeral builder image. Prior to this
commit, only file entries from the buildpack source were copied as
builder layer tar entries; intermediate directory entries from the
source were not copied. This results in directories being created in
the builder container using default permissions. This worked on most
Linux-like OSs where the default permissions allow others-read
access. On some OSs like Arch Linux where the default directory
permissions do not allow others-read, this prevented the lifecycle
processes from reading the buildpack files.

This commit explicitly creates all intermediate directory tar entries
in the builder image layer to ensure that the buildpack directories
and files can be read by the lifecycle processes.

Fixes gh-26658
parent e2cba40d
......@@ -24,7 +24,6 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFileAttributeView;
import org.springframework.boot.buildpack.platform.docker.type.Layer;
import org.springframework.boot.buildpack.platform.io.Content;
......@@ -82,9 +81,18 @@ final class DirectoryBuildpack implements Buildpack {
private void addLayerContent(Layout layout) throws IOException {
String id = this.coordinates.getSanitizedId();
Path cnbPath = Paths.get("/cnb/buildpacks/", id, this.coordinates.getVersion());
writeBasePathEntries(layout, cnbPath);
Files.walkFileTree(this.path, new LayoutFileVisitor(this.path, cnbPath, layout));
}
private void writeBasePathEntries(Layout layout, Path basePath) throws IOException {
int pathCount = basePath.getNameCount();
for (int pathIndex = 1; pathIndex < pathCount + 1; pathIndex++) {
String name = "/" + basePath.subpath(0, pathIndex) + "/";
layout.directory(name, Owner.ROOT);
}
}
/**
* A {@link BuildpackResolver} compatible method to resolve directory buildpacks.
* @param context the resolver context
......@@ -116,16 +124,30 @@ final class DirectoryBuildpack implements Buildpack {
this.layout = layout;
}
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
if (!dir.equals(this.basePath)) {
this.layout.directory(relocate(dir), Owner.ROOT, getMode(dir));
}
return FileVisitResult.CONTINUE;
}
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
PosixFileAttributeView attributeView = Files.getFileAttributeView(file, PosixFileAttributeView.class);
Assert.state(attributeView != null,
"Buildpack content in a directory is not supported on this operating system");
int mode = FilePermissions.posixPermissionsToUmask(attributeView.readAttributes().permissions());
this.layout.file(relocate(file), Owner.ROOT, mode, Content.of(file.toFile()));
this.layout.file(relocate(file), Owner.ROOT, getMode(file), Content.of(file.toFile()));
return FileVisitResult.CONTINUE;
}
private int getMode(Path path) throws IOException {
try {
return FilePermissions.umaskForPath(path);
}
catch (IllegalStateException ex) {
throw new IllegalStateException(
"Buildpack content in a directory is not supported on this operating system");
}
}
private String relocate(Path path) {
Path node = path.subpath(this.basePath.getNameCount(), path.getNameCount());
return Paths.get(this.layerPath.toString(), node.toString()).toString();
......
......@@ -127,11 +127,9 @@ final class ImageBuildpack implements Buildpack {
tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
TarArchiveEntry entry = tarIn.getNextTarEntry();
while (entry != null) {
if (entry.isFile()) {
tarOut.putArchiveEntry(entry);
StreamUtils.copy(tarIn, tarOut);
tarOut.closeArchiveEntry();
}
tarOut.putArchiveEntry(entry);
StreamUtils.copy(tarIn, tarOut);
tarOut.closeArchiveEntry();
entry = tarIn.getNextTarEntry();
}
tarOut.finish();
......
......@@ -89,6 +89,7 @@ final class TarGzipBuildpack implements Buildpack {
try (TarArchiveInputStream tar = new TarArchiveInputStream(
new GzipCompressorInputStream(Files.newInputStream(this.path)));
TarArchiveOutputStream output = new TarArchiveOutputStream(outputStream)) {
writeBasePathEntries(output, basePath);
TarArchiveEntry entry = tar.getNextTarEntry();
while (entry != null) {
entry.setName(basePath + "/" + entry.getName());
......@@ -101,6 +102,16 @@ final class TarGzipBuildpack implements Buildpack {
}
}
private void writeBasePathEntries(TarArchiveOutputStream output, Path basePath) throws IOException {
int pathCount = basePath.getNameCount();
for (int pathIndex = 1; pathIndex < pathCount + 1; pathIndex++) {
String name = "/" + basePath.subpath(0, pathIndex) + "/";
TarArchiveEntry entry = new TarArchiveEntry(name);
output.putArchiveEntry(entry);
output.closeArchiveEntry();
}
}
/**
* A {@link BuildpackResolver} compatible method to resolve tar-gzip buildpacks.
* @param context the resolver context
......
......@@ -16,6 +16,10 @@
package org.springframework.boot.buildpack.platform.io;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Collection;
......@@ -32,6 +36,21 @@ public final class FilePermissions {
private FilePermissions() {
}
/**
* Return the integer representation of the file permissions for a path, where the
* integer value conforms to the
* <a href="https://en.wikipedia.org/wiki/Umask">umask</a> octal notation.
* @param path the file path
* @return the integer representation
* @throws IOException if path permissions cannot be read
*/
public static int umaskForPath(Path path) throws IOException {
Assert.notNull(path, "Path must not be null");
PosixFileAttributeView attributeView = Files.getFileAttributeView(path, PosixFileAttributeView.class);
Assert.state(attributeView != null, "Unsupported file type for retrieving Posix attributes");
return posixPermissionsToUmask(attributeView.readAttributes().permissions());
}
/**
* Return the integer representation of a set of Posix file permissions, where the
* integer value conforms to the
......
......@@ -33,7 +33,18 @@ public interface Layout {
* @param owner the owner of the directory
* @throws IOException on IO error
*/
void directory(String name, Owner owner) throws IOException;
default void directory(String name, Owner owner) throws IOException {
directory(name, owner, 0755);
}
/**
* Add a directory to the content.
* @param name the full name of the directory to add
* @param owner the owner of the directory
* @param mode the permissions for the file
* @throws IOException on IO error
*/
void directory(String name, Owner owner, int mode) throws IOException;
/**
* Write a file to the content.
......
......@@ -44,8 +44,8 @@ class TarLayoutWriter implements Layout, Closeable {
}
@Override
public void directory(String name, Owner owner) throws IOException {
this.outputStream.putArchiveEntry(createDirectoryEntry(name, owner));
public void directory(String name, Owner owner, int mode) throws IOException {
this.outputStream.putArchiveEntry(createDirectoryEntry(name, owner, mode));
this.outputStream.closeArchiveEntry();
}
......@@ -56,8 +56,8 @@ class TarLayoutWriter implements Layout, Closeable {
this.outputStream.closeArchiveEntry();
}
private TarArchiveEntry createDirectoryEntry(String name, Owner owner) {
return createEntry(name, owner, TarConstants.LF_DIR, 0755, 0);
private TarArchiveEntry createDirectoryEntry(String name, Owner owner, int mode) {
return createEntry(name, owner, TarConstants.LF_DIR, mode, 0);
}
private TarArchiveEntry createFileEntry(String name, Owner owner, int mode, int size) {
......
......@@ -133,8 +133,11 @@ class DirectoryBuildpackTests {
entries.add(entry);
entry = tar.getNextTarEntry();
}
assertThat(entries).extracting("name", "mode").containsExactlyInAnyOrder(
assertThat(entries).extracting("name", "mode").containsExactlyInAnyOrder(tuple("/cnb/", 0755),
tuple("/cnb/buildpacks/", 0755), tuple("/cnb/buildpacks/example_buildpack1/", 0755),
tuple("/cnb/buildpacks/example_buildpack1/0.0.1/", 0755),
tuple("/cnb/buildpacks/example_buildpack1/0.0.1/buildpack.toml", 0644),
tuple("/cnb/buildpacks/example_buildpack1/0.0.1/bin/", 0755),
tuple("/cnb/buildpacks/example_buildpack1/0.0.1/bin/detect", 0744),
tuple("/cnb/buildpacks/example_buildpack1/0.0.1/bin/build", 0744));
}
......
......@@ -38,6 +38,7 @@ import org.springframework.boot.buildpack.platform.json.AbstractJsonTests;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.tuple;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.willAnswer;
......@@ -126,6 +127,10 @@ class ImageBuildpackTests extends AbstractJsonTests {
TarArchive archive = (out) -> {
try (TarArchiveOutputStream tarOut = new TarArchiveOutputStream(out)) {
tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
writeTarEntry(tarOut, "/cnb/");
writeTarEntry(tarOut, "/cnb/buildpacks/");
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/");
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/");
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml");
writeTarEntry(tarOut, "/cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath);
tarOut.finish();
......@@ -154,16 +159,22 @@ class ImageBuildpackTests extends AbstractJsonTests {
});
assertThat(layers).hasSize(1);
byte[] content = layers.get(0).toByteArray();
List<String> names = new ArrayList<>();
List<TarArchiveEntry> entries = new ArrayList<>();
try (TarArchiveInputStream tar = new TarArchiveInputStream(new ByteArrayInputStream(content))) {
TarArchiveEntry entry = tar.getNextTarEntry();
while (entry != null) {
names.add(entry.getName());
entries.add(entry);
entry = tar.getNextTarEntry();
}
}
assertThat(names).containsExactlyInAnyOrder("cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml",
"cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath);
assertThat(entries).extracting("name", "mode").containsExactlyInAnyOrder(
tuple("cnb/", TarArchiveEntry.DEFAULT_DIR_MODE),
tuple("cnb/buildpacks/", TarArchiveEntry.DEFAULT_DIR_MODE),
tuple("cnb/buildpacks/example_buildpack/", TarArchiveEntry.DEFAULT_DIR_MODE),
tuple("cnb/buildpacks/example_buildpack/0.0.1/", TarArchiveEntry.DEFAULT_DIR_MODE),
tuple("cnb/buildpacks/example_buildpack/0.0.1/buildpack.toml", TarArchiveEntry.DEFAULT_FILE_MODE),
tuple("cnb/buildpacks/example_buildpack/0.0.1/" + this.longFilePath,
TarArchiveEntry.DEFAULT_FILE_MODE));
}
}
......@@ -87,12 +87,19 @@ class TestTarGzip {
String buildScript = "#!/usr/bin/env bash\n" + "echo \"---> build\"\n";
try (TarArchiveOutputStream tar = new TarArchiveOutputStream(Files.newOutputStream(archive))) {
writeEntry(tar, "buildpack.toml", buildpackToml.toString());
writeEntry(tar, "bin/");
writeEntry(tar, "bin/detect", detectScript);
writeEntry(tar, "bin/build", buildScript);
tar.finish();
}
}
private void writeEntry(TarArchiveOutputStream tar, String entryName) throws IOException {
TarArchiveEntry entry = new TarArchiveEntry(entryName);
tar.putArchiveEntry(entry);
tar.closeArchiveEntry();
}
private void writeEntry(TarArchiveOutputStream tar, String entryName, String content) throws IOException {
TarArchiveEntry entry = new TarArchiveEntry(entryName);
entry.setSize(content.length());
......@@ -111,8 +118,13 @@ class TestTarGzip {
assertThat(layers).hasSize(1);
byte[] content = layers.get(0).toByteArray();
try (TarArchiveInputStream tar = new TarArchiveInputStream(new ByteArrayInputStream(content))) {
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/");
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/buildpacks/");
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/buildpacks/example_buildpack1/");
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/buildpacks/example_buildpack1/0.0.1/");
assertThat(tar.getNextEntry().getName())
.isEqualTo("cnb/buildpacks/example_buildpack1/0.0.1/buildpack.toml");
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/buildpacks/example_buildpack1/0.0.1/bin/");
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/buildpacks/example_buildpack1/0.0.1/bin/detect");
assertThat(tar.getNextEntry().getName()).isEqualTo("cnb/buildpacks/example_buildpack1/0.0.1/bin/build");
assertThat(tar.getNextEntry()).isNull();
......
......@@ -16,14 +16,21 @@
package org.springframework.boot.buildpack.platform.io;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.Collections;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIOException;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
/**
......@@ -33,6 +40,28 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
*/
class FilePermissionsTests {
@TempDir
Path tempDir;
@Test
void umaskForPath() throws IOException {
FileAttribute<Set<PosixFilePermission>> fileAttribute = PosixFilePermissions
.asFileAttribute(PosixFilePermissions.fromString("rw-r-----"));
Path tempFile = Files.createTempFile(this.tempDir, "umask", null, fileAttribute);
assertThat(FilePermissions.umaskForPath(tempFile)).isEqualTo(0640);
}
@Test
void umaskForPathWithNonExistentFile() throws IOException {
assertThatIOException()
.isThrownBy(() -> FilePermissions.umaskForPath(Paths.get(this.tempDir.toString(), "does-not-exist")));
}
@Test
void umaskForPathWithNullPath() throws IOException {
assertThatIllegalArgumentException().isThrownBy(() -> FilePermissions.umaskForPath(null));
}
@Test
void posixPermissionsToUmask() {
Set<PosixFilePermission> permissions = PosixFilePermissions.fromString("rwxrw-r--");
......
......@@ -42,6 +42,7 @@ import org.junit.jupiter.api.condition.OS;
import org.springframework.boot.buildpack.platform.docker.DockerApi;
import org.springframework.boot.buildpack.platform.docker.type.ImageName;
import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
import org.springframework.boot.buildpack.platform.io.FilePermissions;
import org.springframework.boot.gradle.junit.GradleCompatibility;
import org.springframework.boot.gradle.testkit.GradleBuild;
import org.springframework.boot.testsupport.testcontainers.DisabledIfDockerUnavailable;
......@@ -312,8 +313,14 @@ class BootBuildImageIntegrationTests {
}
private void writeBuildpackContent() throws IOException {
FileAttribute<Set<PosixFilePermission>> dirAttribute = PosixFilePermissions
.asFileAttribute(PosixFilePermissions.fromString("rwxr-xr-x"));
FileAttribute<Set<PosixFilePermission>> execFileAttribute = PosixFilePermissions
.asFileAttribute(PosixFilePermissions.fromString("rwxrwxrwx"));
File buildpackDir = new File(this.gradleBuild.getProjectDir(), "buildpack/hello-world");
buildpackDir.mkdirs();
Files.createDirectories(buildpackDir.toPath(), dirAttribute);
File binDir = new File(buildpackDir, "bin");
Files.createDirectories(binDir.toPath(), dirAttribute);
File descriptor = new File(buildpackDir, "buildpack.toml");
try (PrintWriter writer = new PrintWriter(new FileWriter(descriptor))) {
writer.println("api = \"0.2\"");
......@@ -325,17 +332,13 @@ class BootBuildImageIntegrationTests {
writer.println("[[stacks]]\n");
writer.println("id = \"io.buildpacks.stacks.bionic\"");
}
File binDir = new File(buildpackDir, "bin");
binDir.mkdirs();
FileAttribute<Set<PosixFilePermission>> attribute = PosixFilePermissions
.asFileAttribute(PosixFilePermissions.fromString("rwxrwxrwx"));
File detect = Files.createFile(Paths.get(binDir.getAbsolutePath(), "detect"), attribute).toFile();
File detect = Files.createFile(Paths.get(binDir.getAbsolutePath(), "detect"), execFileAttribute).toFile();
try (PrintWriter writer = new PrintWriter(new FileWriter(detect))) {
writer.println("#!/usr/bin/env bash");
writer.println("set -eo pipefail");
writer.println("exit 0");
}
File build = Files.createFile(Paths.get(binDir.getAbsolutePath(), "build"), attribute).toFile();
File build = Files.createFile(Paths.get(binDir.getAbsolutePath(), "build"), execFileAttribute).toFile();
try (PrintWriter writer = new PrintWriter(new FileWriter(build))) {
writer.println("#!/usr/bin/env bash");
writer.println("set -eo pipefail");
......@@ -349,16 +352,33 @@ class BootBuildImageIntegrationTests {
Path tarGzipPath = Paths.get(this.gradleBuild.getProjectDir().getAbsolutePath(), "hello-world.tgz");
try (TarArchiveOutputStream tar = new TarArchiveOutputStream(
new GzipCompressorOutputStream(Files.newOutputStream(Files.createFile(tarGzipPath))))) {
writeFileToTar(tar, new File(this.gradleBuild.getProjectDir(), "buildpack/hello-world/buildpack.toml"),
"buildpack.toml", 0644);
writeFileToTar(tar, new File(this.gradleBuild.getProjectDir(), "buildpack/hello-world/bin/detect"),
"bin/detect", 0777);
writeFileToTar(tar, new File(this.gradleBuild.getProjectDir(), "buildpack/hello-world/bin/build"),
"bin/build", 0777);
File buildpackDir = new File(this.gradleBuild.getProjectDir(), "buildpack/hello-world");
writeDirectoryToTar(tar, buildpackDir, buildpackDir.getAbsolutePath());
}
}
private void writeDirectoryToTar(TarArchiveOutputStream tar, File dir, String baseDirPath) throws IOException {
for (File file : dir.listFiles()) {
String name = file.getAbsolutePath().replace(baseDirPath, "");
int mode = FilePermissions.umaskForPath(file.toPath());
if (file.isDirectory()) {
writeTarEntry(tar, name + "/", mode);
writeDirectoryToTar(tar, file, baseDirPath);
}
else {
writeTarEntry(tar, file, name, mode);
}
}
}
private void writeFileToTar(TarArchiveOutputStream tar, File file, String name, int mode) throws IOException {
private void writeTarEntry(TarArchiveOutputStream tar, String name, int mode) throws IOException {
TarArchiveEntry entry = new TarArchiveEntry(name);
entry.setMode(mode);
tar.putArchiveEntry(entry);
tar.closeArchiveEntry();
}
private void writeTarEntry(TarArchiveOutputStream tar, File file, String name, int mode) throws IOException {
TarArchiveEntry entry = new TarArchiveEntry(file, name);
entry.setMode(mode);
tar.putArchiveEntry(entry);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment