Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit b5c7d3f

Browse files
loic-sharmaloic-sharma
authored
Add Validation Issues to Orchestrator (#277)
Updates the Orchestrator and its validators to use the new APIs that were introduced in ServerCommon's v2.6 dependencies. In addition, the Orchestrator now persists `IValidationIssues` to the database.
1 parent 4e13ec6 commit b5c7d3f

28 files changed

Lines changed: 368 additions & 143 deletions

‎src/NuGet.Services.Validation.Orchestrator/EmailConfiguration.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,15 @@ public class EmailConfiguration
3434
/// Url for email settings, so user can opt out of receiving email notifications.
3535
/// </summary>
3636
public string EmailSettingsUrl { get; set; }
37+
38+
/// <summary>
39+
/// Url for the announcements github page
40+
/// </summary>
41+
public string AnnouncementsUrl { get; set; }
42+
43+
/// <summary>
44+
/// NuGet Twitter url
45+
/// </summary>
46+
public string TwitterUrl { get; set; }
3747
}
3848
}

‎src/NuGet.Services.Validation.Orchestrator/IMessageService.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ public interface IMessageService
99
{
1010
void SendPackagePublishedMessage(Package package);
1111
void SendPackageValidationFailedMessage(Package package);
12+
void SendPackageSignedValidationFailedMessage(Package package);
1213
}
1314
}

‎src/NuGet.Services.Validation.Orchestrator/IValidationStorageService.cs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,29 @@ public interface IValidationStorageService
2626
Task<PackageValidationSet> GetValidationSetAsync(Guid validationTrackingId);
2727

2828
/// <summary>
29-
/// Updates the passed <see cref="PackageValidation"/> with specified validation status,
30-
/// updates the <see cref="PackageValidation.ValidationStatusTimestamp"/>
31-
/// and <see cref="PackageValidation.Started"/> properties to current timestamp, persists changes in the storage.
29+
/// Updates the passed <see cref="PackageValidation"/> with the validation result's status,
30+
/// updates the <see cref="PackageValidation.ValidationStatusTimestamp"/> to current timestamp,
31+
/// and persists changes in the storage. The result's status cannot be <see cref="ValidationStatus.NotStarted"/>
3232
/// </summary>
3333
/// <param name="packageValidation">Validation object to update, must be an object from the <see cref="PackageValidationSet.PackageValidations"/> collection
3434
/// from an <see cref="PackageValidationSet"/> previously returned by either <see cref="CreateValidationSetAsync(PackageValidationSet)"/>
3535
/// or <see cref="GetValidationSetAsync(Guid)"/> calls.</param>
36-
/// <param name="startedStatus">Validation status to set. Cannot be <see cref="ValidationStatus.NotStarted"/></param>
36+
/// <param name="validationResult">Validation result. Its status cannot be <see cref="ValidationStatus.NotStarted"/></param>
3737
/// <returns>Task object tracking the async operation status.</returns>
3838
/// <exception cref="ArgumentOutOfRangeException">If <paramref name="startedStatus"/> is <see cref="ValidationStatus.NotStarted"/></exception>
39-
Task MarkValidationStartedAsync(PackageValidation packageValidation, ValidationStatus startedStatus);
39+
Task MarkValidationStartedAsync(PackageValidation packageValidation, IValidationResult validationResult);
4040

4141
/// <summary>
42-
/// Updates the passed <see cref="PackageValidation"/> object with specified validation status,
43-
/// updates the <see cref="PackageValidation.ValidationStatusTimestamp"/>
44-
/// property to current timestamp, persists changes in the storage.
42+
/// Updates the passed <see cref="PackageValidation"/> object with the result's validation status,
43+
/// updates the <see cref="PackageValidation.ValidationStatusTimestamp"/> property to the current
44+
/// timestamp, adds the result's <see cref="PackageValidationIssue"/>s to the validation, and then persists
45+
/// changes in the storage.
4546
/// </summary>
4647
/// <param name="packageValidation">Validation object to update, must be an object from the <see cref="PackageValidationSet.PackageValidations"/> collection
4748
/// from an <see cref="PackageValidationSet"/> previously returned by either <see cref="CreateValidationSetAsync(PackageValidationSet)"/>
4849
/// or <see cref="GetValidationSetAsync(Guid)"/> calls.</param>
49-
/// <param name="validationStatus">Validation status to set.</param>
50+
/// <param name="validationResult">The result of the validation.</param>
5051
/// <returns>Task object tracking the async operation status.</returns>
51-
Task UpdateValidationStatusAsync(PackageValidation packageValidation, ValidationStatus validationStatus);
52+
Task UpdateValidationStatusAsync(PackageValidation packageValidation, IValidationResult validationResult);
5253
}
5354
}

‎src/NuGet.Services.Validation.Orchestrator/MessageService.cs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,15 @@ public void SendPackageValidationFailedMessage(Package package)
6060

6161
var galleryPackageUrl = string.Format(_emailConfiguration.PackageUrlTemplate, package.PackageRegistration.Id, package.NormalizedVersion);
6262
var packageSupportUrl = string.Format(_emailConfiguration.PackageSupportTemplate, package.PackageRegistration.Id, package.NormalizedVersion);
63-
_coreMessageService.SendPackageValidationFailedNotice(package, galleryPackageUrl, packageSupportUrl, _emailConfiguration.EmailSettingsUrl);
63+
_coreMessageService.SendPackageValidationFailedNotice(package, galleryPackageUrl, packageSupportUrl);
64+
}
65+
66+
public void SendPackageSignedValidationFailedMessage(Package package)
67+
{
68+
package = package ?? throw new ArgumentNullException(nameof(package));
69+
70+
var galleryPackageUrl = string.Format(_emailConfiguration.PackageUrlTemplate, package.PackageRegistration.Id, package.NormalizedVersion);
71+
_coreMessageService.SendSignedPackageNotAllowedNotice(package, galleryPackageUrl, _emailConfiguration.AnnouncementsUrl, _emailConfiguration.TwitterUrl);
6472
}
6573
}
6674
}

‎src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,28 +134,31 @@
134134
<Version>1.1.2</Version>
135135
</PackageReference>
136136
<PackageReference Include="NuGet.Services.Configuration">
137-
<Version>2.5.0</Version>
137+
<Version>2.7.0</Version>
138138
</PackageReference>
139139
<PackageReference Include="NuGet.Services.Contracts">
140-
<Version>2.5.0</Version>
140+
<Version>2.7.0</Version>
141141
</PackageReference>
142142
<PackageReference Include="NuGet.Services.KeyVault">
143-
<Version>2.5.0</Version>
143+
<Version>2.7.0</Version>
144144
</PackageReference>
145145
<PackageReference Include="NuGet.Services.Logging">
146-
<Version>2.5.0</Version>
146+
<Version>2.7.0</Version>
147147
</PackageReference>
148148
<PackageReference Include="NuGet.Services.ServiceBus">
149-
<Version>2.5.0</Version>
149+
<Version>2.7.0</Version>
150150
</PackageReference>
151151
<PackageReference Include="NuGet.Services.Validation">
152-
<Version>2.5.0</Version>
152+
<Version>2.7.0</Version>
153+
</PackageReference>
154+
<PackageReference Include="NuGet.Services.Validation.Issues">
155+
<Version>2.7.0</Version>
153156
</PackageReference>
154157
<PackageReference Include="NuGet.Versioning">
155158
<Version>4.3.0</Version>
156159
</PackageReference>
157160
<PackageReference Include="NuGetGallery.Core">
158-
<Version>4.4.4-dev-18852</Version>
161+
<Version>4.4.4-dev-19677</Version>
159162
</PackageReference>
160163
<PackageReference Include="Serilog">
161164
<Version>2.5.0</Version>

‎src/NuGet.Services.Validation.Orchestrator/PackageCertificates/PackageCertificatesValidator.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,14 @@ public PackageCertificatesValidator(
5050
}
5151
}
5252

53-
public async Task<ValidationStatus> GetStatusAsync(IValidationRequest request)
53+
public async Task<IValidationResult> GetResultAsync(IValidationRequest request)
54+
{
55+
var status = await GetStatusAsync(request);
56+
57+
return new ValidationResult(status);
58+
}
59+
60+
private async Task<ValidationStatus> GetStatusAsync(IValidationRequest request)
5461
{
5562
// Look up this validator's state in the database.
5663
var status = await _validatorStateService.GetStatusAsync(request);
@@ -107,7 +114,12 @@ public async Task<ValidationStatus> GetStatusAsync(IValidationRequest request)
107114
return await _validatorStateService.TryUpdateValidationStatusAsync(request, status, ValidationStatus.Succeeded);
108115
}
109116

110-
public async Task<ValidationStatus> StartValidationAsync(IValidationRequest request)
117+
public async Task<IValidationResult> StartValidationAsync(IValidationRequest request)
118+
{
119+
return new ValidationResult(await StartValidationInternalAsync(request));
120+
}
121+
122+
private async Task<ValidationStatus> StartValidationInternalAsync(IValidationRequest request)
111123
{
112124
var status = await _validatorStateService.GetStatusAsync(request);
113125

‎src/NuGet.Services.Validation.Orchestrator/PackageSigning/PackageSigningValidator.cs‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Threading.Tasks;
66
using Microsoft.Extensions.Logging;
77
using NuGet.Jobs.Validation.PackageSigning.Storage;
8+
using NuGet.Services.Validation.Issues;
89

910
namespace NuGet.Services.Validation.PackageSigning
1011
{
@@ -24,14 +25,29 @@ public PackageSigningValidator(
2425
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
2526
}
2627

27-
public async Task<ValidationStatus> GetStatusAsync(IValidationRequest request)
28+
public async Task<IValidationResult> GetResultAsync(IValidationRequest request)
2829
{
2930
var validatorStatus = await _validatorStateService.GetStatusAsync(request);
3031

31-
return validatorStatus.State;
32+
if (validatorStatus.State == ValidationStatus.Failed)
33+
{
34+
// If the validation has failed, assume it is because signed packages are blocked.
35+
return ValidationResult.FailedWithIssues(new PackageIsSigned(request.PackageId, request.PackageVersion));
36+
}
37+
else
38+
{
39+
return new ValidationResult(validatorStatus.State);
40+
}
41+
}
42+
43+
public async Task<IValidationResult> StartValidationAsync(IValidationRequest request)
44+
{
45+
var status = await StartValidationInternalAsync(request);
46+
47+
return new ValidationResult(status);
3248
}
3349

34-
public async Task<ValidationStatus> StartValidationAsync(IValidationRequest request)
50+
public async Task<ValidationStatus> StartValidationInternalAsync(IValidationRequest request)
3551
{
3652
// Check that this is the first validation for this specific request.
3753
var validatorStatus = await _validatorStateService.GetStatusAsync(request);

‎src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,22 @@ public async Task ProcessValidationOutcomeAsync(PackageValidationSet validationS
5656
if (package.PackageStatusKey != PackageStatus.Available)
5757
{
5858
await _galleryPackageService.UpdatePackageStatusAsync(package, PackageStatus.FailedValidation);
59-
_messageService.SendPackageValidationFailedMessage(package);
59+
60+
var issuesExistAndAllPackageSigned = validationSet
61+
.PackageValidations
62+
.SelectMany(pv => pv.PackageValidationIssues)
63+
.Select(pvi => pvi.IssueCode == ValidationIssueCode.PackageIsSigned)
64+
.DefaultIfEmpty(false)
65+
.All(v => v);
66+
67+
if (issuesExistAndAllPackageSigned)
68+
{
69+
_messageService.SendPackageSignedValidationFailedMessage(package);
70+
}
71+
else
72+
{
73+
_messageService.SendPackageValidationFailedMessage(package);
74+
}
6075
}
6176
else
6277
{

‎src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -89,35 +89,35 @@ private async Task<bool> ProcessIncompleteValidations(PackageValidationSet valid
8989

9090
var validator = _validatorProvider.GetValidator(packageValidation.Type);
9191
var validationRequest = await CreateValidationRequest(packageValidation.PackageValidationSet, packageValidation, package, validationConfiguration);
92-
var validationStatus = await validator.GetStatusAsync(validationRequest);
92+
var validationResult = await validator.GetResultAsync(validationRequest);
9393
_logger.LogInformation("New status for validation {ValidationType} for {PackageId} {PackageVersion} is {ValidationStatus}, validation set {ValidationSetId}, {ValidationId}",
9494
packageValidation.Type,
9595
package.PackageRegistration.Id,
9696
package.NormalizedVersion,
97-
validationStatus,
97+
validationResult.Status,
9898
validationSet.ValidationTrackingId,
9999
packageValidation.Key);
100100

101-
switch (validationStatus)
101+
switch (validationResult.Status)
102102
{
103103
case ValidationStatus.Incomplete:
104104
await ProcessIncompleteValidation(packageValidation, validationConfiguration);
105105
break;
106106

107107
case ValidationStatus.Failed:
108-
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, validationStatus);
108+
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, validationResult);
109109
break;
110110

111111
case ValidationStatus.Succeeded:
112-
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, validationStatus);
112+
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, validationResult);
113113
// need another iteration to try running new validations
114114
tryMoreValidations = true;
115115
break;
116116

117117
default:
118118
throw new InvalidOperationException($"Unexpected validation state: " +
119119
$"DB: {ValidationStatus.Incomplete} ({(int)ValidationStatus.Incomplete}), " +
120-
$"Actual: {validationStatus} {(int)validationStatus}");
120+
$"Actual: {validationResult.Status} {(int)validationResult.Status}");
121121
}
122122
}
123123
}
@@ -159,9 +159,9 @@ private async Task<bool> ProcessNotStartedValidations(PackageValidationSet valid
159159

160160
var validator = _validatorProvider.GetValidator(packageValidation.Type);
161161
var validationRequest = await CreateValidationRequest(packageValidation.PackageValidationSet, packageValidation, package, validationConfiguration);
162-
var validationStatus = await validator.GetStatusAsync(validationRequest);
162+
var validationResult = await validator.GetResultAsync(validationRequest);
163163

164-
if (validationStatus == ValidationStatus.NotStarted)
164+
if (validationResult.Status == ValidationStatus.NotStarted)
165165
{
166166
_logger.LogInformation("Requesting validation {ValidationType} for {PackageId} {PackageVersion}, validation set {ValidationSetId}, {ValidationId}, {NupkgUrl}",
167167
packageValidation.Type,
@@ -170,17 +170,17 @@ private async Task<bool> ProcessNotStartedValidations(PackageValidationSet valid
170170
validationSet.ValidationTrackingId,
171171
packageValidation.Key,
172172
validationRequest.NupkgUrl);
173-
validationStatus = await validator.StartValidationAsync(validationRequest);
173+
validationResult = await validator.StartValidationAsync(validationRequest);
174174
_logger.LogInformation("Got validationStatus = {ValidationStatus} for validation {ValidationType} for {PackageId} {PackageVersion}, validation set {ValidationSetId}, {ValidationId}",
175-
validationStatus,
175+
validationResult.Status,
176176
packageValidation.Type,
177177
package.PackageRegistration.Id,
178178
package.NormalizedVersion,
179179
validationSet.ValidationTrackingId,
180180
packageValidation.Key);
181181
}
182182

183-
if (validationStatus == ValidationStatus.NotStarted)
183+
if (validationResult.Status == ValidationStatus.NotStarted)
184184
{
185185
_logger.LogWarning("Unexpected NotStarted state after start attempt for validation {ValidationName}, package: {PackageId} {PackageVersion}",
186186
packageValidation.Type,
@@ -189,10 +189,10 @@ private async Task<bool> ProcessNotStartedValidations(PackageValidationSet valid
189189
}
190190
else
191191
{
192-
await _validationStorageService.MarkValidationStartedAsync(packageValidation, validationStatus);
192+
await _validationStorageService.MarkValidationStartedAsync(packageValidation, validationResult);
193193
}
194194

195-
tryMoreValidations = tryMoreValidations || validationStatus == ValidationStatus.Succeeded;
195+
tryMoreValidations = tryMoreValidations || validationResult.Status == ValidationStatus.Succeeded;
196196
}
197197
}
198198

@@ -212,7 +212,7 @@ private async Task OnUnknownValidation(PackageValidation packageValidation)
212212
packageValidation.PackageValidationSet.PackageId,
213213
packageValidation.PackageValidationSet.PackageNormalizedVersion);
214214

215-
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, ValidationStatus.Failed);
215+
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, ValidationResult.Failed);
216216
}
217217

218218
private async Task ProcessIncompleteValidation(PackageValidation packageValidation, ValidationConfigurationItem validationConfiguration)
@@ -226,7 +226,7 @@ private async Task ProcessIncompleteValidation(PackageValidation packageValidati
226226
packageValidation.PackageValidationSet.PackageId,
227227
packageValidation.PackageValidationSet.PackageNormalizedVersion,
228228
validationConfiguration.FailAfter);
229-
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, ValidationStatus.Failed);
229+
await _validationStorageService.UpdateValidationStatusAsync(packageValidation, ValidationResult.Failed);
230230
return;
231231
}
232232
}

0 commit comments

Comments
 (0)