Reverting the no-cleanup for validation container

src/NuGet.Services.Validation.Orchestrator/Configuration/ValidationConfiguration.cs

@@ -49,10 +49,5 @@ public class ValidationConfiguration
         /// The threshold until a validation set is no longer processed.
         /// </summary>
         public TimeSpan TimeoutValidationSetAfter { get; set; }
-
-        /// <summary>
-        /// The duration for which SAS tokens are generated for package URLs passed down to validators.
-        /// </summary>
-        public TimeSpan NupkgUrlValidityPeriod { get; set; } = TimeSpan.FromDays(7);
     }
 }

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

@@ -141,10 +141,10 @@ await ScheduleCheckIfNotTimedOut(
                         scheduleNextCheck,
                         tooLongNotificationAllowed: false);
                 }
-
-                // TODO: implement delayed cleanup that would allow internal services
-                // to access original packages for some time after package become available:
-                // https://github.com/NuGet/Engineering/issues/2506
+                else
+                {
+                    await _packageFileService.DeletePackageForValidationSetAsync(validationSet);
+                }
             }
             else
             {

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

@@ -154,11 +154,11 @@ private async Task<bool> ProcessNotStartedValidations(PackageValidationSet valid
                 using (_logger.BeginScope("Not started {ValidationType} Key {ValidationId}", packageValidation.Type, packageValidation.Key))
                 {
                     _logger.LogInformation("Processing not started validation {ValidationType} for {PackageId} {PackageVersion}, validation set {ValidationSetId}, {ValidationId}",
-                        packageValidation.Type,
-                        validationSet.PackageId,
-                        validationSet.PackageNormalizedVersion,
-                        validationSet.ValidationTrackingId,
-                        packageValidation.Key);
+                    packageValidation.Type,
+                    validationSet.PackageId,
+                    validationSet.PackageNormalizedVersion,
+                    validationSet.ValidationTrackingId,
+                    packageValidation.Key);
                     var validationConfiguration = GetValidationConfiguration(packageValidation.Type);
                     if (validationConfiguration == null)
                     {
@@ -283,7 +283,7 @@ private async Task<IValidationRequest> CreateValidationRequest(
         {
             var nupkgUrl = await _packageFileService.GetPackageForValidationSetReadUriAsync(
                 packageValidationSet,
-                DateTimeOffset.UtcNow.Add(_validationConfiguration.NupkgUrlValidityPeriod));
+                DateTimeOffset.UtcNow.Add(_validationConfiguration.TimeoutValidationSetAfter));
 
             var validationRequest = new ValidationRequest(
                 validationId: packageValidation.Key,

tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationOutcomeProcessorFacts.cs

@@ -29,6 +29,10 @@ public async Task ProcessesFailedValidationAccordingToFailureBehavior(Validation
             PackageStateProcessorMock.Verify(
                 x => x.SetStatusAsync(PackageValidatingEntity, ValidationSet, expectedPackageStatus),
                 Times.Once);
+            PackageFileServiceMock.Verify(
+                x => x.DeletePackageForValidationSetAsync(ValidationSet),
+                Times.Once);
+
             Assert.Equal(ValidationSetStatus.Completed, ValidationSet.ValidationSetStatus);
         }
 
@@ -302,7 +306,7 @@ public async Task MakesPackageAvailableAndSendsEmailUponSuccess()
 
             PackageFileServiceMock.Verify(
                 x => x.DeletePackageForValidationSetAsync(ValidationSet),
-                Times.Never);
+                Times.Once);
 
             MessageServiceMock
                 .Verify(ms => ms.SendPublishedMessageAsync(Package), Times.Once());
@@ -333,6 +337,7 @@ public async Task HasProperOperationOrderWhenTransitioningToAvailable(PackageSta
                     nameof(IValidationStorageService.UpdateValidationSetAsync),
                     nameof(IMessageService<Package>.SendPublishedMessageAsync),
                     nameof(ITelemetryService.TrackTotalValidationDuration),
+                    nameof(IValidationFileService.DeletePackageForValidationSetAsync),
                 },
                 operations.ToArray());
         }
@@ -355,6 +360,7 @@ public async Task HasProperOperationOrderWhenAlreadyAvailable()
                     nameof(IStatusProcessor<Package>.SetStatusAsync),
                     nameof(IValidationStorageService.UpdateValidationSetAsync),
                     nameof(ITelemetryService.TrackTotalValidationDuration),
+                    nameof(IValidationFileService.DeletePackageForValidationSetAsync),
                 },
                 operations.ToArray());
         }
@@ -451,19 +457,9 @@ public async Task MarksPackageStatusBasedOnValidatorResults(
                     Times.Never);
             }
 
-            if (validation != ValidationStatus.Failed)
-            {
-                PackageFileServiceMock.Verify(
-                    x => x.DeletePackageForValidationSetAsync(ValidationSet),
-                    Times.Never);
-            }
-            else
-            {
-                PackageFileServiceMock.Verify(
-                    x => x.DeletePackageForValidationSetAsync(ValidationSet),
-                    Times.Once);
-            }
-
+            PackageFileServiceMock.Verify(
+                x => x.DeletePackageForValidationSetAsync(ValidationSet),
+                Times.Once);
             TelemetryServiceMock
                 .Verify(ts => ts.TrackTotalValidationDuration(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<Guid>(), It.IsAny<TimeSpan>(), It.IsAny<bool>()), Times.Once());
             Assert.InRange(duration, before - ValidationSet.Created, after - ValidationSet.Created);

tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationSetProcessorFacts.cs

@@ -308,11 +308,11 @@ public async Task UsesProperNupkgUrl()
                 .ReturnsAsync(ValidationResult.Incomplete);
 
             var processor = CreateProcessor();
-            var expectedEndOfAccessLower = DateTimeOffset.UtcNow.Add(Configuration.NupkgUrlValidityPeriod);
+            var expectedEndOfAccessLower = DateTimeOffset.UtcNow.Add(Configuration.TimeoutValidationSetAfter);
 
             await processor.ProcessValidationsAsync(ValidationSet);
 
-            var expectedEndOfAccessUpper = DateTimeOffset.UtcNow.Add(Configuration.NupkgUrlValidityPeriod);
+            var expectedEndOfAccessUpper = DateTimeOffset.UtcNow.Add(Configuration.TimeoutValidationSetAfter);
 
             validator
                 .Verify(v => v.GetResultAsync(It.IsAny<IValidationRequest>()), Times.AtLeastOnce());
@@ -402,7 +402,6 @@ protected ValidationSetProcessorFactsBase(
                 {
                 },
                 TimeoutValidationSetAfter = TimeSpan.FromDays(5),
-                NupkgUrlValidityPeriod = TimeSpan.FromDays(9),
             };
             ConfigurationAccessorMock
                 .SetupGet(ca => ca.Value)