Skip to content

Commit

Permalink
return counts from RestoreTask for PackageReference projects (#6049)
Browse files Browse the repository at this point in the history
  • Loading branch information
zivkan authored Oct 1, 2024
1 parent a793d11 commit db35f73
Show file tree
Hide file tree
Showing 14 changed files with 195 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ static bool HasProjectToRestore(DependencyGraphSpec dgSpec, bool restorePackages

try
{
bool result = await BuildTasksUtility.RestoreAsync(
// todo: need to return Restore task output properties, like in NuGet.targets
// https://github.com/NuGet/Home/issues/13828
List<RestoreSummary> restoreSummaries = await BuildTasksUtility.RestoreAsync(
dependencyGraphSpec: dependencyGraphSpec,
interactive,
recursive: IsOptionTrue(nameof(RestoreTaskEx.Recursive), options),
Expand All @@ -152,6 +154,7 @@ static bool HasProjectToRestore(DependencyGraphSpec dgSpec, bool restorePackages
cleanupAssetsForUnsupportedProjects: IsOptionTrue(nameof(RestoreTaskEx.CleanupAssetsForUnsupportedProjects), options),
log: MSBuildLogger,
cancellationToken: CancellationToken.None);
bool result = restoreSummaries.All(rs => rs.Success);

LogFilesToEmbedInBinlog(dependencyGraphSpec, options);

Expand Down
6 changes: 3 additions & 3 deletions src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public static void AddPropertyIfExists(IDictionary<string, string> properties, s
ProjectStyle.ProjectJson
};

public static Task<bool> RestoreAsync(
public static Task<List<RestoreSummary>> RestoreAsync(
DependencyGraphSpec dependencyGraphSpec,
bool interactive,
bool recursive,
Expand All @@ -131,7 +131,7 @@ public static Task<bool> RestoreAsync(
return RestoreAsync(dependencyGraphSpec, interactive, recursive, noCache, ignoreFailedSources, disableParallel, force, forceEvaluate, hideWarningsAndErrors, restorePC, cleanupAssetsForUnsupportedProjects: false, log, cancellationToken);
}

public static async Task<bool> RestoreAsync(
public static async Task<List<RestoreSummary>> RestoreAsync(
DependencyGraphSpec dependencyGraphSpec,
bool interactive,
bool recursive,
Expand Down Expand Up @@ -289,7 +289,7 @@ public static async Task<bool> RestoreAsync(
{
RestoreSummary.Log(log, restoreSummaries);
}
return restoreSummaries.All(x => x.Success);
return restoreSummaries;
}
finally
{
Expand Down
3 changes: 3 additions & 0 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ Copyright (c) .NET Foundation. All rights reserved.
RestorePackagesConfig="$(RestorePackagesConfig)"
EmbedFilesInBinlog="$(RestoreEmbedFilesInBinlog)">
<Output TaskParameter="EmbedInBinlog" ItemName="EmbedInBinlog" />
<Output TaskParameter="ProjectsRestored" PropertyName="RestoreProjectCount" />
<Output TaskParameter="ProjectsAlreadyUpToDate" PropertyName="RestoreSkippedCount" />
<Output TaskParameter="ProjectsAudited" PropertyName="RestoreProjectsAuditedCount" />
</RestoreTask>
</Target>

Expand Down
40 changes: 39 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks/RestoreTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,30 @@ public class RestoreTask : Microsoft.Build.Utilities.Task, ICancelableTask, IDis
[Output]
public ITaskItem[] EmbedInBinlog { get; set; }

/// <summary>
/// Gets or sets the number of projects that were considered for this restore operation.
/// </summary>
/// <remarks>
/// Projects that no-op (were already up to date) are included.
/// </remarks>
[Output]
public int ProjectsRestored { get; set; }

/// <summary>
/// Gets or sets the number of projects that were already up to date.
/// </summary>
[Output]
public int ProjectsAlreadyUpToDate { get; set; }

/// <summary>
/// Gets or sets the number of projects that NuGetAudit scanned.
/// </summary>
/// <remarks>
/// NuGetAudit does not run on no-op restores, or when no vulnerability database can be downloaded.
/// </remarks>
[Output]
public int ProjectsAudited { get; set; }

/// <summary>
/// Gets or sets a value indicating whether to embed files produced by restore in the MSBuild binary logger.
/// 0 = Nothing
Expand Down Expand Up @@ -148,7 +172,7 @@ private async Task<bool> ExecuteAsync(Common.ILogger log)
log.LogInformation(Strings.Log_RestoreNoCacheInformation);
}

return await BuildTasksUtility.RestoreAsync(
var restoreSummaries = await BuildTasksUtility.RestoreAsync(
dependencyGraphSpec: dgFile,
interactive: Interactive,
recursive: RestoreRecursive,
Expand All @@ -161,6 +185,20 @@ private async Task<bool> ExecuteAsync(Common.ILogger log)
restorePC: RestorePackagesConfig,
log: log,
cancellationToken: _cts.Token);

int upToDate = 0;
int audited = 0;
foreach (var summary in restoreSummaries)
{
if (summary.NoOpRestore) { upToDate++; }
if (summary.AuditRan) { audited++; }
}

ProjectsRestored = restoreSummaries.Count;
ProjectsAlreadyUpToDate = upToDate;
ProjectsAudited = audited;

return restoreSummaries.All(s => s.Success);
}

public void Cancel()
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
#nullable enable
NuGet.Commands.RestoreSummary.AuditRan.get -> bool
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
#nullable enable
NuGet.Commands.RestoreSummary.AuditRan.get -> bool
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
#nullable enable
NuGet.Commands.RestoreSummary.AuditRan.get -> bool
19 changes: 15 additions & 4 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,10 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803,
_request.Project.FilePath,
_logger);
telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled";
bool auditRan = false;
if (auditEnabled)
{
await PerformAuditAsync(graphs, telemetry, token);
auditRan = await PerformAuditAsync(graphs, telemetry, token);
}

telemetry.StartIntervalMeasure();
Expand Down Expand Up @@ -511,11 +512,19 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803,
dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request),
dependencyGraphSpec: _request.DependencyGraphSpec,
_request.ProjectStyle,
restoreTime.Elapsed);
restoreTime.Elapsed)
{
AuditRan = auditRan
};
}
}

private async Task PerformAuditAsync(IEnumerable<RestoreTargetGraph> graphs, TelemetryActivity telemetry, CancellationToken token)
/// <summary>Run NuGetAudit on the project's resolved restore graphs, and log messages and telemetry with the results.</summary>
/// <param name="graphs">The resolved package graphs, one for each project target framework.</param>
/// <param name="telemetry">The <see cref="TelemetryActivity"/> to log NuGetAudit telemetry to.</param>
/// <param name="token">A <see cref="CancellationToken"/> to cancel obtaining a vulnerability database. Once the database is downloaded, audit is quick to complete.</param>
/// <returns>False if no vulnerability database could be found (so packages were not scanned for vulnerabilities), true otherwise.</returns>
private async Task<bool> PerformAuditAsync(IEnumerable<RestoreTargetGraph> graphs, TelemetryActivity telemetry, CancellationToken token)
{
telemetry.StartIntervalMeasure();
var audit = new AuditUtility(
Expand All @@ -524,7 +533,7 @@ private async Task PerformAuditAsync(IEnumerable<RestoreTargetGraph> graphs, Tel
graphs,
_request.DependencyProviders.VulnerabilityInfoProviders,
_logger);
await audit.CheckPackageVulnerabilitiesAsync(token);
bool auditRan = await audit.CheckPackageVulnerabilitiesAsync(token);

telemetry.TelemetryEvent[AuditLevel] = (int)audit.MinSeverity;
telemetry.TelemetryEvent[AuditMode] = AuditUtility.GetString(audit.AuditMode);
Expand Down Expand Up @@ -554,6 +563,8 @@ private async Task PerformAuditAsync(IEnumerable<RestoreTargetGraph> graphs, Tel
if (audit.GenerateOutputDurationSeconds.HasValue) { telemetry.TelemetryEvent[AuditDurationOutput] = audit.GenerateOutputDurationSeconds.Value; }
telemetry.EndIntervalMeasure(AuditDurationTotal);

return auditRan;

void AddPackagesList(TelemetryActivity telemetry, string eventName, List<string> packages)
{
List<TelemetryEvent> result = new List<TelemetryEvent>(packages.Count);
Expand Down
3 changes: 3 additions & 0 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ public class RestoreResult : IRestoreResult
/// </summary>
internal PackagesLockFile _newPackagesLockFile { get; }

/// <inheritdoc cref="RestoreSummary.AuditRan"/>
internal bool AuditRan { get; init; }


private readonly string _dependencyGraphSpecFilePath;

Expand Down
13 changes: 13 additions & 0 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ public class RestoreSummary

public int InstallCount { get; }

/// <summary>
/// A boolean that specifies if NuGetAudit verified packages for known vulnerabilities
/// </summary>
/// <remarks>This could be false if NuGetAudit is disabled, if
/// <list type="bullet">
/// <item>NuGetAudit is disabled</item>
/// <item>Project is already up to date (no-op restore)</item>
/// <item>No sources provided a vulnerability database</item>
/// </list>
/// </remarks>
public bool AuditRan { get; }

/// <summary>
/// All the warnings and errors that were produced as a result of the restore.
/// </summary>
Expand Down Expand Up @@ -59,6 +71,7 @@ public RestoreSummary(
.AsReadOnly();
InstallCount = result.GetAllInstalled().Count;
Errors = errors.ToArray();
AuditRan = result.AuditRan;
}

public RestoreSummary(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ public AuditUtility(
}
}

public async Task CheckPackageVulnerabilitiesAsync(CancellationToken cancellationToken)
public async Task<bool> CheckPackageVulnerabilitiesAsync(CancellationToken cancellationToken)
{
// Performance: Early exit if restore graph does not contain any packages.
if (!HasPackages())
{
return;
// No packages means we've validated there are none with known vulnerabilities.
return true;
}

Stopwatch stopwatch = Stopwatch.StartNew();
Expand All @@ -94,10 +95,11 @@ public async Task CheckPackageVulnerabilitiesAsync(CancellationToken cancellatio
// Performance: Early exit if there's no vulnerability data to check packages against.
if (allVulnerabilityData is null || !AnyVulnerabilityDataFound(allVulnerabilityData))
{
return;
return false;
}

CheckPackageVulnerabilities(allVulnerabilityData);
return true;

bool HasPackages()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
using NuGet.Packaging;
using NuGet.Packaging.Signing;
using NuGet.ProjectModel;
using NuGet.Protocol;
using NuGet.Test.Utility;
using NuGet.Versioning;
using Test.Utility;
using Test.Utility.Signing;
using Xunit;
using Xunit.Abstractions;
Expand Down Expand Up @@ -2693,6 +2696,107 @@ public async Task DotnetRestore_WithMultiTargettingProject_WhenTargetFrameworkIs
allTargets.Should().Contain(condition);
}

// https://github.com/NuGet/Home/issues/13829
[PlatformFact(SkipPlatform = Platform.Linux)]
public async Task DotnetRestore_WithServerProvidingAuditData_RestoreTaskReturnsCountProperties()
{
// Arrange
using var pathContext = new SimpleTestPathContext();
using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true);

mockServer.Vulnerabilities.Add(
"Insecure.Package",
new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> {
(new Uri("https://contoso.test/advisories/12346"), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 2.0.0)"))
});
pathContext.Settings.RemoveSource("source");
pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri, allowInsecureConnectionsValue: "true");

var packageA1 = new SimpleTestPackageContext()
{
Id = "packageA",
Version = "1.0.0"
};

await SimpleTestPackageUtility.CreatePackagesAsync(
pathContext.PackageSource,
packageA1);

var targetFramework = Constants.DefaultTargetFramework.GetShortFolderName();
string csprojContents = GetProjectFileForRestoreTaskOutputTests(targetFramework);
var csprojPath = Path.Combine(pathContext.SolutionRoot, "test.csproj");
File.WriteAllText(csprojPath, csprojContents);

mockServer.Start();

// Act & Assert

// First restore is fresh, so should not be skipped (no-op), and should run audit
_dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=0 -p:ExpectedRestoreProjectsAuditedCount=1");

// Second restore is no-op, so should be skipped, and audit not run
_dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=1 -p:ExpectedRestoreProjectsAuditedCount=0");

mockServer.Stop();
}

[Fact]
public async Task DotnetRestore_WithServerWithoutAuditData_RestoreTaskReturnsCountProperties()
{
// Arrange
using var pathContext = new SimpleTestPathContext();

var packageA1 = new SimpleTestPackageContext()
{
Id = "packageA",
Version = "1.0.0"
};

await SimpleTestPackageUtility.CreatePackagesAsync(
pathContext.PackageSource,
packageA1);

var targetFramework = Constants.DefaultTargetFramework.GetShortFolderName();
string csprojContents = GetProjectFileForRestoreTaskOutputTests(targetFramework);
var csprojPath = Path.Combine(pathContext.SolutionRoot, "test.csproj");
File.WriteAllText(csprojPath, csprojContents);

// Act & Assert

// First restore is fresh, so should not be skipped (no-op), and should run audit
_dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=0 -p:ExpectedRestoreProjectsAuditedCount=0");

// Second restore is no-op, so should be skipped, and audit not run
_dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=1 -p:ExpectedRestoreProjectsAuditedCount=0");
}

private string GetProjectFileForRestoreTaskOutputTests(string targetFramework)
{
string csprojContents = @$"<Project Sdk=""Microsoft.NET.Sdk"">
<PropertyGroup>
<TargetFramework>{targetFramework}</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include=""packageA"" Version=""1.0.0"" />
</ItemGroup>
<Target Name=""AssertRestoreTaskOutputProperties""
AfterTargets=""Restore"">
<Error
Condition="" '$(RestoreProjectCount)' != '$(ExpectedRestoreProjectCount)' ""
Text=""RestoreProjectCount did not have the expected value. Expected: $(ExpectedRestoreProjectCount) Found: $(RestoreProjectCount)"" />
<Error
Condition="" '$(RestoreSkippedCount)' != '$(ExpectedRestoreSkippedCount)' ""
Text=""RestoreSkippedCount did not have the expected value. Expected: $(ExpectedRestoreSkippedCount) Found: $(RestoreSkippedCount)"" />
<Error
Condition="" '$(RestoreProjectsAuditedCount)' != '$(ExpectedRestoreProjectsAuditedCount)' ""
Text=""RestoreProjectsAuditedCount did not have the expected value. Expected: $(ExpectedRestoreProjectsAuditedCount) Found: $(RestoreProjectsAuditedCount)"" />
</Target>
</Project>";
return csprojContents;
}

private void AssertRelatedProperty(IList<LockFileItem> items, string path, string related)
{
var item = items.Single(i => i.Path.Equals(path));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private Action<HttpListenerResponse> ServerHandlerV3(HttpListenerRequest request
return new Action<HttpListenerResponse>(response =>
{
response.ContentType = "application/json";
var vulnerabilityJson = FeedUtilities.CreateVulnerabilitiesJson(Uri + "/vulnerability/vulnerability.json");
var vulnerabilityJson = FeedUtilities.CreateVulnerabilitiesJson(Uri + "vulnerability/vulnerability.json");
SetResponseContent(response, vulnerabilityJson.ToString());
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ private static XDocument GetEmptyConfig()

var config = GetOrAddSection(doc, "config");
var packageSources = GetOrAddSection(doc, "packageSources");
var auditSources = GetOrAddSection(doc, "auditSources");
var disabledSources = GetOrAddSection(doc, "disabledPackageSources");
var fallbackFolders = GetOrAddSection(doc, "fallbackPackageFolders");
var packageSourceMapping = GetOrAddSection(doc, "packageSourceMapping");

packageSources.Add(new XElement(XName.Get("clear")));
auditSources.Add(new XElement(XName.Get("clear")));
disabledSources.Add(new XElement(XName.Get("clear")));
packageSourceMapping.Add(new XElement(XName.Get("clear")));

Expand Down

0 comments on commit db35f73

Please sign in to comment.