Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Tar writing to include directory records #323

Merged
merged 2 commits into from
Feb 7, 2023
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
29 changes: 28 additions & 1 deletion Microsoft.NET.Build.Containers/Layer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Formats.Tar;
using System.IO.Compression;
using System.Security.Cryptography;
using System.Text;

namespace Microsoft.NET.Build.Containers;

Expand Down Expand Up @@ -39,19 +40,23 @@ public static Layer FromFiles(IEnumerable<(string path, string containerPath)> f
Span<byte> hash = stackalloc byte[SHA256.HashSizeInBytes];
Span<byte> uncompressedHash = stackalloc byte[SHA256.HashSizeInBytes];

var directoryEntries = new HashSet<string>();

string tempTarballPath = ContentStore.GetTempFile();
using (FileStream fs = File.Create(tempTarballPath))
{
using (HashDigestGZipStream gz = new(fs, leaveOpen: true))
{
using (TarWriter writer = new(gz, TarEntryFormat.Gnu, leaveOpen: true))
using (TarWriter writer = new(gz, TarEntryFormat.Pax, leaveOpen: true))
{
foreach (var item in fileList)
{
// Docker treats a COPY instruction that copies to a path like `/app` by
// including `app/` as a directory, with no leading slash. Emulate that here.
string containerPath = item.containerPath.TrimStart(PathSeparators);

EnsureDirectoryEntries(writer, directoryEntries, containerPath.Split(PathSeparators));

writer.WriteEntry(item.path, containerPath);
}
} // Dispose of the TarWriter before getting the hash so the final data get written to the tar stream
Expand Down Expand Up @@ -95,6 +100,28 @@ public static Layer FromFiles(IEnumerable<(string path, string containerPath)> f

private readonly static char[] PathSeparators = new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar };

/// <summary>
/// Ensures that all directory entries for the given segments are created within the tar.
/// </summary>
/// <param name="tar">The tar into which to add the directory entries.</param>
/// <param name="directoryEntries">The lookup of all known directory entries. </param>
/// <param name="filePathSegments">The segments of the file within the tar for which to create the folders</param>
private static void EnsureDirectoryEntries(TarWriter tar, ISet<string> directoryEntries, IReadOnlyList<string> filePathSegments)
Copy link
Member

Choose a reason for hiding this comment

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

nit: can be better as local function, then passing directoryEntries is not needed.
With current implementation directoryEntries is being modified, which is quite implicit.

{
var pathBuilder = new StringBuilder();
for (var i = 0; i < filePathSegments.Count - 1; i++)
{
pathBuilder.Append($"{filePathSegments[i]}/");

var fullPath = pathBuilder.ToString();
if (!directoryEntries.Contains(fullPath))
{
tar.WriteEntry(new PaxTarEntry(TarEntryType.Directory, fullPath));
directoryEntries.Add(fullPath);
}
}
}

/// <summary>
/// A stream capable of computing the hash digest of raw uncompressed data while also compressing it.
/// </summary>
Expand Down
6 changes: 3 additions & 3 deletions Microsoft.NET.Build.Containers/LocalDocker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static async Task Load(Image x, string name, string tag, string baseName)

public static async Task WriteImageToStream(Image x, string name, string tag, Stream imageStream)
{
TarWriter writer = new(imageStream, TarEntryFormat.Gnu, leaveOpen: true);
using TarWriter writer = new(imageStream, TarEntryFormat.Pax, leaveOpen: true);


// Feed each layer tarball into the stream
Expand Down Expand Up @@ -67,7 +67,7 @@ public static async Task WriteImageToStream(Image x, string name, string tag, St

using (MemoryStream configStream = new MemoryStream(Encoding.UTF8.GetBytes(x.config.ToJsonString())))
{
GnuTarEntry configEntry = new(TarEntryType.RegularFile, configTarballPath)
PaxTarEntry configEntry = new(TarEntryType.RegularFile, configTarballPath)
{
DataStream = configStream
};
Expand All @@ -90,7 +90,7 @@ public static async Task WriteImageToStream(Image x, string name, string tag, St

using (MemoryStream manifestStream = new MemoryStream(Encoding.UTF8.GetBytes(manifestNode.ToJsonString())))
{
GnuTarEntry manifestEntry = new(TarEntryType.RegularFile, "manifest.json")
PaxTarEntry manifestEntry = new(TarEntryType.RegularFile, "manifest.json")
{
DataStream = manifestStream
};
Expand Down
34 changes: 31 additions & 3 deletions Test.Microsoft.NET.Build.Containers.Filesystem/LayerEndToEnd.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Formats.Tar;
using Microsoft.NET.Build.Containers;
using System.IO.Compression;
using System.Security.Cryptography;
Expand All @@ -22,10 +23,14 @@ public void SingleFileInFolder()
Console.WriteLine(l.Descriptor);

//Assert.AreEqual("application/vnd.oci.image.layer.v1.tar", l.Descriptor.MediaType); // TODO: configurability
Assert.IsTrue(l.Descriptor.Size is >= 135 and <= 200, $"'l.Descriptor.Size' should be between 135 and 200, but is {l.Descriptor.Size}"); // TODO: determinism!
Assert.IsTrue(l.Descriptor.Size is >= 135 and <= 500, $"'l.Descriptor.Size' should be between 135 and 500, but is {l.Descriptor.Size}"); // TODO: determinism!
//Assert.AreEqual("sha256:26140bc75f2fcb3bf5da7d3b531d995c93d192837e37df0eb5ca46e2db953124", l.Descriptor.Digest); // TODO: determinism!

VerifyDescriptorInfo(l);

var allEntries = LoadAllTarEntries(l.BackingFile);
Assert.IsTrue(allEntries.TryGetValue("app/", out var appEntryType) && appEntryType == TarEntryType.Directory, "Missing app directory entry");
Assert.IsTrue(allEntries.TryGetValue("app/TestFile.txt", out var fileEntryType) && fileEntryType == TarEntryType.RegularFile, "Missing TestFile.txt file entry");
}

[TestMethod]
Expand All @@ -51,10 +56,16 @@ public void TwoFilesInTwoFolders()
Console.WriteLine(l.Descriptor);

//Assert.AreEqual("application/vnd.oci.image.layer.v1.tar", l.Descriptor.MediaType); // TODO: configurability
Assert.IsTrue(l.Descriptor.Size is >= 150 and <= 200, $"'l.Descriptor.Size' should be between 150 and 200, but is {l.Descriptor.Size}"); // TODO: determinism!
Assert.IsTrue(l.Descriptor.Size is >= 150 and <= 500, $"'l.Descriptor.Size' should be between 150 and 500, but is {l.Descriptor.Size}"); // TODO: determinism!
//Assert.AreEqual("sha256:26140bc75f2fcb3bf5da7d3b531d995c93d192837e37df0eb5ca46e2db953124", l.Descriptor.Digest); // TODO: determinism!

VerifyDescriptorInfo(l);

var allEntries = LoadAllTarEntries(l.BackingFile);
Assert.IsTrue(allEntries.TryGetValue("app/", out var appEntryType) && appEntryType == TarEntryType.Directory, "Missing app directory entry");
Assert.IsTrue(allEntries.TryGetValue("app/TestFile.txt", out var fileEntryType) && fileEntryType == TarEntryType.RegularFile, "Missing TestFile.txt file entry");
Assert.IsTrue(allEntries.TryGetValue("app/subfolder/", out var subfolderType) && subfolderType == TarEntryType.Directory, "Missing subfolder directory entry");
Assert.IsTrue(allEntries.TryGetValue("app/subfolder/TestFile.txt", out var subfolderFileEntryType) && subfolderFileEntryType == TarEntryType.RegularFile, "Missing subfolder/TestFile.txt file entry");
}

private static void VerifyDescriptorInfo(Layer l)
Expand Down Expand Up @@ -102,4 +113,21 @@ public void TestCleanup()
ContentStore.ArtifactRoot = priorArtifactRoot;
}
}
}


private static IDictionary<string, TarEntryType> LoadAllTarEntries(string file)
{
using var gzip = new GZipStream(File.OpenRead(file), CompressionMode.Decompress);
using var tar = new TarReader(gzip);

var entries = new Dictionary<string, TarEntryType>();

TarEntry? entry;
while ((entry = tar.GetNextEntry()) != null)
{
entries[entry.Name] = entry.EntryType;
}

return entries;
}
}