Add cross-version DataProtection E2E tests#66438
Add cross-version DataProtection E2E tests#66438DeagleGross wants to merge 2 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds cross-version end-to-end tests for DataProtection by loading a pinned, released NuGet package in an isolated AssemblyLoadContext and verifying ManagedAuthenticatedEncryptor payload compatibility between the released package and the current source.
Changes:
- Introduce a new NuGet integration test project that downloads/extracts a pinned DataProtection NuGet package and creates encryptor instances via reflection.
- Add cross-version compatibility tests covering NuGet↔current-source decrypt/encrypt and a current netcore↔current netfx (net462) cross-TFM check.
- Expose DataProtection internals to the new test assembly and wire the project into solution files.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.NuGetIntegrationTests/NuGetEncryptorFactory.cs | Implements NuGet package download/extract + reflection-based encryptor creation in isolated ALCs. |
| src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.NuGetIntegrationTests/CrossVersionCompatibility_ManagedAuthenticatedEncryptor_Tests.cs | Adds compatibility test coverage for ManagedAuthenticatedEncryptor across versions/TFMs. |
| src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.NuGetIntegrationTests/Microsoft.AspNetCore.DataProtection.NuGetIntegrationTests.csproj | New test project definition for the NuGet integration tests. |
| src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj | Adds InternalsVisibleTo for the new integration test assembly. |
| src/DataProtection/DataProtection.slnf | Includes the new test project in the DataProtection solution filter. |
| AspNetCore.slnx | Includes the new test project in the main solution. |
| private static NuGetEncryptorWrapper CreateEncryptorFromDirectory(string directory, string dllPath, string alcName) | ||
| { | ||
| var alc = new DirectoryAssemblyLoadContext(directory, alcName); | ||
| var assembly = alc.LoadFromAssemblyPath(dllPath); | ||
|
|
||
| var secretType = assembly.GetType(typeof(Secret).FullName!) | ||
| ?? throw new InvalidOperationException($"Cannot find {nameof(Secret)} in assembly ({alcName})"); | ||
| var encryptorType = assembly.GetType(typeof(ManagedAuthenticatedEncryptor).FullName!) | ||
| ?? throw new InvalidOperationException($"Cannot find {nameof(ManagedAuthenticatedEncryptor)} in assembly ({alcName})"); | ||
|
|
||
| var encryptorCtor = encryptorType.GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance) | ||
| .FirstOrDefault(c => c.GetParameters().Length == 5) | ||
| ?? throw new InvalidOperationException($"Cannot find {nameof(ManagedAuthenticatedEncryptor)} 5-param constructor ({alcName})"); | ||
|
|
||
| var genRandomImplType = assembly.GetType(typeof(ManagedGenRandomImpl).FullName!) | ||
| ?? throw new InvalidOperationException($"Cannot find {nameof(ManagedGenRandomImpl)} in assembly ({alcName})"); | ||
| var genRandomInstance = genRandomImplType.GetField("Instance", BindingFlags.Public | BindingFlags.Static)?.GetValue(null) | ||
| ?? throw new InvalidOperationException($"Cannot find {nameof(ManagedGenRandomImpl)}.Instance ({alcName})"); | ||
|
|
||
| var secretCtor = secretType.GetConstructor([typeof(byte[])]) | ||
| ?? throw new InvalidOperationException($"Cannot find Secret(byte[]) constructor ({alcName})"); | ||
| var secret = secretCtor.Invoke([new byte[512 / 8]]); | ||
|
|
||
| Func<SymmetricAlgorithm> symFactory = Aes.Create; | ||
| Func<KeyedHashAlgorithm> hmacFactory = () => new HMACSHA256(); | ||
|
|
||
| var encryptor = encryptorCtor.Invoke([secret, symFactory, 256 / 8, hmacFactory, genRandomInstance]); | ||
| return new NuGetEncryptorWrapper(encryptor, encryptorType); | ||
| } |
There was a problem hiding this comment.
Each call to CreateEncryptorFromDirectory creates a collectible AssemblyLoadContext, but the returned NuGetEncryptorWrapper doesn’t retain the ALC reference or provide a way to unload it. Over multiple tests this can accumulate loaded assemblies and memory. Consider making NuGetEncryptorWrapper IDisposable, storing the AssemblyLoadContext inside it, and calling Unload() (and dropping references to Type/MethodInfo) on disposal so tests can clean up per encryptor instance.
| var nugetEncryptor = _factory.CreateEncryptor(targetFramework); | ||
| using var currentEncryptor = CreateCurrentEncryptor(); | ||
| var plaintext = new ArraySegment<byte>("cross-version-test-payload"u8.ToArray()); | ||
| var aad = new ArraySegment<byte>("cross-version-aad"u8.ToArray()); | ||
|
|
||
| var ciphertext = nugetEncryptor.Encrypt(plaintext, aad); | ||
| var decrypted = currentEncryptor.Decrypt(new ArraySegment<byte>(ciphertext), aad); | ||
|
|
There was a problem hiding this comment.
These tests create NuGetEncryptorWrapper instances (which internally create collectible AssemblyLoadContexts) but never dispose/unload them. If the wrapper is updated to support unloading, update the tests to use using var (or dispose in finally) for nugetEncryptor / netFxEncryptor so test runs don’t accumulate loaded assemblies and temp-file locks.
| private async Task<string> DownloadAndExtractPackage(string packageId) | ||
| { | ||
| var extractDir = Path.Combine(_tempDir, packageId); | ||
| var nupkgPath = Path.Combine(_tempDir, $"{packageId}.{_packageVersion}.nupkg"); | ||
|
|
||
| using var http = new HttpClient(); | ||
| var url = $"{NuGetBaseUrl}/{packageId.ToLowerInvariant()}/{_packageVersion}/{packageId.ToLowerInvariant()}.{_packageVersion}.nupkg"; | ||
| var bytes = await http.GetByteArrayAsync(url); | ||
| await File.WriteAllBytesAsync(nupkgPath, bytes); | ||
| ZipFile.ExtractToDirectory(nupkgPath, extractDir); | ||
|
|
There was a problem hiding this comment.
DownloadAndExtractPackage reaches out directly to api.nuget.org during test execution. This makes the test suite dependent on external network availability and NuGet service uptime, which can cause flaky CI runs and makes offline/dev runs harder. Consider sourcing the pinned package from the repo’s restore artifacts / global packages folder (so restore handles download/caching) or adding a resilient cache + retry/timeout strategy with a clear skip when the package can’t be obtained.
| var netFxDir = Path.Combine(artifactsBinDir, "Microsoft.AspNetCore.DataProtection", "Debug", "net462"); | ||
| var dllPath = Path.Combine(netFxDir, $"{DataProtectionPackageId}.dll"); | ||
|
|
||
| if (!File.Exists(dllPath)) | ||
| { | ||
| throw new FileNotFoundException( | ||
| $"Source-built net462 DLL not found at {dllPath}. " + | ||
| $"Ensure the DataProtection project is built for net462."); | ||
| } | ||
|
|
||
| return CreateEncryptorFromDirectory(netFxDir, dllPath, "SourceBuilt-net462"); | ||
| } | ||
|
|
There was a problem hiding this comment.
CreateSourceBuiltNetFxEncryptor hard-codes the build configuration as "Debug" when computing artifacts/bin output paths. This will break in Release test runs (and any non-Debug configuration) because the net462 DLL will be searched in the wrong directory. Consider deriving the configuration from the current test assembly output path (e.g., the parent folder of the TFM directory) and using that value instead of a constant, or probing both Debug/Release before failing.
| var netFxDir = Path.Combine(artifactsBinDir, "Microsoft.AspNetCore.DataProtection", "Debug", "net462"); | |
| var dllPath = Path.Combine(netFxDir, $"{DataProtectionPackageId}.dll"); | |
| if (!File.Exists(dllPath)) | |
| { | |
| throw new FileNotFoundException( | |
| $"Source-built net462 DLL not found at {dllPath}. " + | |
| $"Ensure the DataProtection project is built for net462."); | |
| } | |
| return CreateEncryptorFromDirectory(netFxDir, dllPath, "SourceBuilt-net462"); | |
| } | |
| foreach (var netFxDir in GetSourceBuiltNetFxCandidateDirectories(testAssemblyDir, artifactsBinDir)) | |
| { | |
| var dllPath = Path.Combine(netFxDir, $"{DataProtectionPackageId}.dll"); | |
| if (File.Exists(dllPath)) | |
| { | |
| return CreateEncryptorFromDirectory(netFxDir, dllPath, "SourceBuilt-net462"); | |
| } | |
| } | |
| throw new FileNotFoundException( | |
| $"Source-built net462 DLL not found. Probed: {string.Join(", ", GetSourceBuiltNetFxCandidateDirectories(testAssemblyDir, artifactsBinDir))}. " + | |
| $"Ensure the DataProtection project is built for net462."); | |
| } | |
| private static IEnumerable<string> GetSourceBuiltNetFxCandidateDirectories(string testAssemblyDir, string artifactsBinDir) | |
| { | |
| var configurations = new List<string>(); | |
| var configuration = new DirectoryInfo(testAssemblyDir).Parent?.Name; | |
| if (!string.IsNullOrEmpty(configuration)) | |
| { | |
| configurations.Add(configuration); | |
| } | |
| foreach (var fallbackConfiguration in new[] { "Debug", "Release" }) | |
| { | |
| if (!configurations.Contains(fallbackConfiguration, StringComparer.OrdinalIgnoreCase)) | |
| { | |
| configurations.Add(fallbackConfiguration); | |
| } | |
| } | |
| return configurations.Select(configurationName => Path.Combine(artifactsBinDir, "Microsoft.AspNetCore.DataProtection", configurationName, "net462")); | |
| } |
| private string PickBestModernTfm() | ||
| { | ||
| return _tfmDirs.Keys | ||
| .Where(t => t.StartsWith("net", StringComparison.Ordinal) | ||
| && !t.StartsWith("netstandard", StringComparison.Ordinal) | ||
| && !t.StartsWith("net4", StringComparison.Ordinal)) | ||
| .OrderByDescending(t => t) | ||
| .FirstOrDefault() | ||
| ?? throw new InvalidOperationException("No suitable TFM found in NuGet package"); | ||
| } |
There was a problem hiding this comment.
PickBestModernTfm orders TFMs using simple string ordering (OrderByDescending(t => t)). This produces incorrect results once net10.0+ is present (e.g., "net9.0" sorts after "net10.0" lexicographically). Parse the numeric portion (and optional patch) and sort by version components, or use a TFM parser to pick the highest compatible netX target.
| { | ||
| public const string DefaultPackageVersion = "9.0.15"; | ||
|
|
||
| private const string NuGetBaseUrl = "https://api.nuget.org/v3-flatcontainer"; |
There was a problem hiding this comment.
There might be some policies in official pipelines that disallows access to nuget.org. Is the package available on dotnet-public or dotnet-tools and we can use from there instead?
| // Source output: artifacts/bin/Microsoft.AspNetCore.DataProtection/Debug/net462/ | ||
| var testAssemblyDir = Path.GetDirectoryName(typeof(NuGetEncryptorFactory).Assembly.Location)!; | ||
| var artifactsBinDir = Path.GetFullPath(Path.Combine(testAssemblyDir, "..", "..", "..")); | ||
| var netFxDir = Path.Combine(artifactsBinDir, "Microsoft.AspNetCore.DataProtection", "Debug", "net462"); |
There was a problem hiding this comment.
Should we have it as?
const string Configuration =
#if DEBUG
"Debug";
#else
"Release";
#endif
var netFxDir = Path.Combine(artifactsBinDir, "Microsoft.AspNetCore.DataProtection", Configuration, "net462");
DataProtection is quite unique in the sense that both package of X version and X+1 version have to be compatible and be able to perform encrypt/decrypt between different versions.
For example, encrypting data via DataProtection of 10.0.5 and decrypting via 10.0.6 should give the same result as encrypting via 10.0.6 and decrypting via 10.0.5. And also should give the same result as encrypting/decrypting in the same package (but we have such units tests already).
I am proposing to introduce cross-version testing via downloading a good "known" (pinned) version of NuGet package that performs encryption and decryption correctly, and making a cross-version comparison between current-branch code and the pinned NuGet package.
As a starter I added tests for
ManagedAuthenticatedEncryptor, but we can add for other encryptors as well.Related #66335