Enable unpack and repack of .deb containers by NikolaMilosavljevic · Pull Request #15345 · dotnet/arcade

NikolaMilosavljevic · 2024-12-17T22:45:23Z

This work enables unpack and repack of .deb containers. I've tested this with all current .NET DEB packages.

I've updated test.deb to include a signable file - this makes the DEB test complete.

ellahathaway · 2024-12-17T23:11:02Z

Do you have a binlog from an arcade-validation (or some equivalent repo) build with these changes? Wondering if you've done e2e testing and also curious to see what the SignTool output looks like.

src/Microsoft.DotNet.SignTool/src/ZipData.cs

jkoritzinsky · 2024-12-17T23:30:18Z

src/Microsoft.DotNet.SignTool/src/ZipData.cs

 using System.Data;
 using System.Diagnostics;
+using Microsoft.DotNet.Build.Tasks.Installers;
+using Microsoft.DotNet.Build.Tasks.Installers.src;


Can we change the namespaces in the Installers package to remove the .src nested namespace?

Sure, will do that.

Fixed with 9fe1ae4

NikolaMilosavljevic · 2024-12-17T23:42:33Z

Do you have a binlog from an arcade-validation (or some equivalent repo) build with these changes? Wondering if you've done e2e testing and also curious to see what the SignTool output looks like.

Working on this validation right now.

NikolaMilosavljevic · 2024-12-18T02:12:44Z

Binlog from local validation in arcade-validation repo:
msbuild.zip

Debian signing log entries:

File /src/git/arcade-validation/artifacts/packages/Debug/NonShipping/test.deb is not signed.
Extracting file 'data.tar.gz' from '/src/git/arcade-validation/artifacts/packages/Debug/NonShipping/test.deb' to '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz'.
Tracking file '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz' isNested=True
Extracting file 'hello' from '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz' to '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/./usr/local/bin/hello'.
Tracking file '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/./usr/local/bin/hello' isNested=True
Ignoring non-signable file: /src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/./usr/local/bin/hello
Caching file hello 036E707FF7B4A823E1182FE6DF81E90A93AE44EA17A8B2F0044A11CDE169342E
Caching file data.tar.gz 57989E84E7AFBDE7A3396AC35B47F4F3553B0EF68ADC6652C70507814FA45BA2
Caching file test.deb EC14B84B6074AF3A402E8CA7AF2381373705D121EDEAC5310425FD659E23EDC2

This is with current test.deb package in arcade-validation repo. I will recheck with newer version of the package that I've prepared for this PR.

NikolaMilosavljevic · 2024-12-18T02:20:14Z

Here's the binlog from signing test that includes the updated test.deb (with signable binary, mscorlib.dll):
msbuild.zip

Log shows additional entries for this binary, i.e.:

Extracting file 'mscorlib.dll' from '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz' to '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/39/./usr/local/bin/mscorlib.dll'.
Tracking file '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/39/./usr/local/bin/mscorlib.dll' isNested=True

src/Microsoft.DotNet.SignTool/src/ZipData.cs

mmitche

Can you add another test or two that validates that the repacked deb has the expected content?

Correct checksums/metadata
Correct files/layout

Otherwise looks great aside from some questions/nits.

Co-authored-by: Matt Mitchell <[email protected]>

Co-authored-by: Jeremy Koritzinsky <[email protected]>

NikolaMilosavljevic · 2024-12-18T17:44:24Z

Can you add another test or two that validates that the repacked deb has the expected content?

Correct checksums/metadata

Correct files/layout

Otherwise looks great aside from some questions/nits.

Certainly. Will do.

ellahathaway · 2024-12-18T18:19:29Z

Binlog from local validation in arcade-validation repo: msbuild.zip

Debian signing log entries:

File /src/git/arcade-validation/artifacts/packages/Debug/NonShipping/test.deb is not signed.
Extracting file 'data.tar.gz' from '/src/git/arcade-validation/artifacts/packages/Debug/NonShipping/test.deb' to '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz'.
Tracking file '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz' isNested=True
Extracting file 'hello' from '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/data.tar.gz' to '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/./usr/local/bin/hello'.
Tracking file '/src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/./usr/local/bin/hello' isNested=True
Ignoring non-signable file: /src/git/arcade-validation/artifacts/tmp/Debug/ContainerSigning/38/./usr/local/bin/hello
Caching file hello 036E707FF7B4A823E1182FE6DF81E90A93AE44EA17A8B2F0044A11CDE169342E
Caching file data.tar.gz 57989E84E7AFBDE7A3396AC35B47F4F3553B0EF68ADC6652C70507814FA45BA2
Caching file test.deb EC14B84B6074AF3A402E8CA7AF2381373705D121EDEAC5310425FD659E23EDC2

This is with current test.deb package in arcade-validation repo. I will recheck with newer version of the package that I've prepared for this PR.

Binlog looks great. Thanks for validating that.

NikolaMilosavljevic · 2024-12-19T21:27:01Z

Can you add another test or two that validates that the repacked deb has the expected content?

Correct checksums/metadata

Correct files/layout

Otherwise looks great aside from some questions/nits.

Fixed with 3172fe5

src/Microsoft.DotNet.SignTool/src/ZipData.cs

NikolaMilosavljevic · 2024-12-20T16:28:00Z

One test is still failing: MissingCertificateNameButExtensionIsIgnored for extension: *.deb -

arcade/src/Microsoft.DotNet.SignTool.Tests/SignToolTests.cs

Line 2297 in 06ecc3c

public void MissingCertificateNameButExtensionIsIgnored(string extension)

At first, this failed due to a missing deb entry in needContent Dictionary - causing the creation of dummy DEB package, which doesn't work with real unpacking code.

It is now failing due to a presence of the the signable file, since we're using the same test package for all DEB tests. New DEB package has a signable file, to be able to better test the DEB unpack/repack code.

If I put back the old test.deb file (without a signable file), this test succeeds.

I can add another test DEB package, to be used by this test, but is that the right option?

ellahathaway · 2024-12-20T17:08:14Z

One test is still failing: MissingCertificateNameButExtensionIsIgnored for extension: *.deb -

arcade/src/Microsoft.DotNet.SignTool.Tests/SignToolTests.cs

Line 2297 in 06ecc3c

public void MissingCertificateNameButExtensionIsIgnored(string extension)

At first, this failed due to a missing deb entry in needContent Dictionary - causing the creation of dummy DEB package, which doesn't work with real unpacking code.

It is now failing due to a presence of the the signable file, since we're using the same test package for all DEB tests. New DEB package has a signable file, to be able to better test the DEB unpack/repack code.

If I put back the old test.deb file (without a signable file), this test succeeds.

I can add another test DEB package, to be used by this test, but is that the right option?

Can we have two test deb packages? One called SignableDeb.deb and the other called UnsignableDeb.deb? I don't see harm in having two test packages.

NikolaMilosavljevic · 2024-12-20T18:29:39Z

One test is still failing: MissingCertificateNameButExtensionIsIgnored for extension: *.deb -

arcade/src/Microsoft.DotNet.SignTool.Tests/SignToolTests.cs

Line 2297 in 06ecc3c

public void MissingCertificateNameButExtensionIsIgnored(string extension)

At first, this failed due to a missing deb entry in needContent Dictionary - causing the creation of dummy DEB package, which doesn't work with real unpacking code.
It is now failing due to a presence of the the signable file, since we're using the same test package for all DEB tests. New DEB package has a signable file, to be able to better test the DEB unpack/repack code.
If I put back the old test.deb file (without a signable file), this test succeeds.
I can add another test DEB package, to be used by this test, but is that the right option?

Can we have two test deb packages? One called SignableDeb.deb and the other called UnsignableDeb.deb? I don't see harm in having two test packages.

Yeah, that is an option. But, we first need to understand why this is failing and if it's a real issue. We don't want to add a test package for the sake of passing a test-case. @mmitche is going to investigate this.

NikolaMilosavljevic · 2024-12-20T23:46:17Z

Hmm, not sure why NuGet.Packaging was added - will remove.

NikolaMilosavljevic · 2025-01-02T16:27:26Z

Two commits were not reviewed so this is still blocked. Checks are now passing.

Enable unpack and repack of .deb containers

1e34aac

NikolaMilosavljevic requested review from ellahathaway, jkoritzinsky and mmitche December 17, 2024 22:45

jkoritzinsky reviewed Dec 17, 2024

View reviewed changes

Merge branch 'main' into deb.repack

f0653f3