Skip to content

Commit

Permalink
return counts from RestoreTask for PackageReference projects
Browse files Browse the repository at this point in the history
  • Loading branch information
zivkan committed Sep 14, 2024
1 parent fef6562 commit 1ed0647
Show file tree
Hide file tree
Showing 14 changed files with 190 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ 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
List<RestoreSummary> restoreSummaries = await BuildTasksUtility.RestoreAsync(
dependencyGraphSpec: dependencyGraphSpec,
interactive,
recursive: IsOptionTrue(nameof(RestoreTaskEx.Recursive), options),
Expand All @@ -152,6 +153,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,4 +1,5 @@
#nullable enable
NuGet.Commands.RestoreSummary.AuditRan.get -> bool
NuGet.Commands.UpdateSourceArgs.AllowInsecureConnections.set -> void
NuGet.Commands.UpdateSourceArgs.AllowInsecureConnections.get -> bool
NuGet.Commands.AddSourceArgs.AllowInsecureConnections.set -> void
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#nullable enable
NuGet.Commands.RestoreSummary.AuditRan.get -> bool
NuGet.Commands.UpdateSourceArgs.AllowInsecureConnections.set -> void
NuGet.Commands.UpdateSourceArgs.AllowInsecureConnections.get -> bool
NuGet.Commands.AddSourceArgs.AllowInsecureConnections.set -> void
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#nullable enable
NuGet.Commands.RestoreSummary.AuditRan.get -> bool
NuGet.Commands.UpdateSourceArgs.AllowInsecureConnections.set -> void
NuGet.Commands.UpdateSourceArgs.AllowInsecureConnections.get -> bool
NuGet.Commands.AddSourceArgs.AllowInsecureConnections.set -> void
Expand Down
17 changes: 14 additions & 3 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 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 audit.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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ 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 either is NuGetAudit is disabled, but also if
/// no sources provided NuGetAudit with a vulnerability database.</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 +66,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 @@ -51,6 +51,9 @@ internal class AuditUtility
internal int DistinctAdvisoriesSuppressedCount { get; private set; }
internal int TotalWarningsSuppressedCount { get; private set; }

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

public AuditUtility(
ProjectModel.RestoreAuditProperties? restoreAuditProperties,
string projectFullPath,
Expand Down Expand Up @@ -83,6 +86,8 @@ public async Task CheckPackageVulnerabilitiesAsync(CancellationToken cancellatio
// Performance: Early exit if restore graph does not contain any packages.
if (!HasPackages())
{
// No packages means we've validated there are none with known vulnerabilities.
AuditRan = true;
return;
}

Expand All @@ -94,10 +99,13 @@ 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))
{
AuditRan = false;
return;
}

CheckPackageVulnerabilities(allVulnerabilityData);
AuditRan = true;
return;

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,106 @@ public async Task DotnetRestore_WithMultiTargettingProject_WhenTargetFrameworkIs
allTargets.Should().Contain(condition);
}

[Fact]
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 1ed0647

Please sign in to comment.