diff --git a/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj b/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj index 02f71a448..4506bf410 100644 --- a/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj +++ b/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj @@ -105,13 +105,13 @@ all - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 4.3.3 diff --git a/src/NuGet.Services.Revalidate/NuGet.Services.Revalidate.csproj b/src/NuGet.Services.Revalidate/NuGet.Services.Revalidate.csproj index 5e223e60f..ea66a805b 100644 --- a/src/NuGet.Services.Revalidate/NuGet.Services.Revalidate.csproj +++ b/src/NuGet.Services.Revalidate/NuGet.Services.Revalidate.csproj @@ -123,7 +123,7 @@ all - 2.39.0 + 2.40.0 diff --git a/src/NuGet.Services.Validation.Orchestrator/Configuration/FlatContainerConfiguration.cs b/src/NuGet.Services.Validation.Orchestrator/Configuration/FlatContainerConfiguration.cs new file mode 100644 index 000000000..09de09bf1 --- /dev/null +++ b/src/NuGet.Services.Validation.Orchestrator/Configuration/FlatContainerConfiguration.cs @@ -0,0 +1,10 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace NuGet.Services.Validation.Orchestrator +{ + public class FlatContainerConfiguration + { + public string ConnectionString { get; set; } + } +} diff --git a/src/NuGet.Services.Validation.Orchestrator/EntityStatusProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/EntityStatusProcessor.cs new file mode 100644 index 000000000..cbaa5c325 --- /dev/null +++ b/src/NuGet.Services.Validation.Orchestrator/EntityStatusProcessor.cs @@ -0,0 +1,315 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using NuGet.Services.Entities; +using NuGet.Services.Validation.Orchestrator.Telemetry; +using NuGetGallery; +using NuGetGallery.Packaging; + +namespace NuGet.Services.Validation.Orchestrator +{ + public abstract class EntityStatusProcessor : IStatusProcessor where T : class, IEntity + { + protected readonly IEntityService _galleryPackageService; + protected readonly IValidationFileService _packageFileService; + protected readonly IValidatorProvider _validatorProvider; + protected readonly ITelemetryService _telemetryService; + protected readonly ILogger> _logger; + + public EntityStatusProcessor( + IEntityService galleryPackageService, + IValidationFileService packageFileService, + IValidatorProvider validatorProvider, + ITelemetryService telemetryService, + ILogger> logger) + { + _galleryPackageService = galleryPackageService ?? throw new ArgumentNullException(nameof(galleryPackageService)); + _packageFileService = packageFileService ?? throw new ArgumentNullException(nameof(packageFileService)); + _validatorProvider = validatorProvider ?? throw new ArgumentNullException(nameof(validatorProvider)); + _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + public Task SetStatusAsync( + IValidatingEntity validatingEntity, + PackageValidationSet validationSet, + PackageStatus status) + { + if (validatingEntity == null) + { + throw new ArgumentNullException(nameof(validatingEntity)); + } + + if (validationSet == null) + { + throw new ArgumentNullException(nameof(validationSet)); + } + + if (validatingEntity.Status == PackageStatus.Deleted) + { + throw new ArgumentException( + $"A package in the {nameof(PackageStatus.Deleted)} state cannot be processed.", + nameof(validatingEntity)); + } + + if (validatingEntity.Status == PackageStatus.Available && + status == PackageStatus.FailedValidation) + { + throw new ArgumentException( + $"A package cannot transition from {nameof(PackageStatus.Available)} to {nameof(PackageStatus.FailedValidation)}.", + nameof(status)); + } + + switch (status) + { + case PackageStatus.Available: + return MakePackageAvailableAsync(validatingEntity, validationSet); + case PackageStatus.FailedValidation: + return MakePackageFailedValidationAsync(validatingEntity, validationSet); + default: + throw new ArgumentException( + $"A package can only transition to the {nameof(PackageStatus.Available)} or " + + $"{nameof(PackageStatus.FailedValidation)} states.", nameof(status)); + } + } + + protected virtual async Task MakePackageFailedValidationAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + { + var fromStatus = validatingEntity.Status; + + await _galleryPackageService.UpdateStatusAsync(validatingEntity.EntityRecord, PackageStatus.FailedValidation, commitChanges: true); + + if (fromStatus != PackageStatus.FailedValidation) + { + _telemetryService.TrackPackageStatusChange(fromStatus, PackageStatus.FailedValidation); + } + } + + protected virtual async Task MakePackageAvailableAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + { + // 1) Operate on blob storage. + var copied = await UpdatePublicPackageAsync(validationSet); + + // 2) Update the package's blob metadata in the public blob storage container. + var metadata = await _packageFileService.UpdatePackageBlobMetadataAsync(validationSet); + + // 3) Update the package's blob properties in the public blob storage container. + await _packageFileService.UpdatePackageBlobPropertiesAsync(validationSet); + + // 3.5) Allow descendants to do their own things before we update the database + await OnBeforeUpdateDatabaseToMakePackageAvailable(validatingEntity, validationSet); + + // 4) Operate on the database. + var fromStatus = await MarkPackageAsAvailableAsync(validationSet, validatingEntity, metadata, copied); + + // 5) Emit telemetry and clean up. + if (fromStatus != PackageStatus.Available) + { + _telemetryService.TrackPackageStatusChange(fromStatus, PackageStatus.Available); + + _logger.LogInformation("Deleting from the source for package {PackageId} {PackageVersion}, validation set {ValidationSetId}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId); + + await _packageFileService.DeleteValidationPackageFileAsync(validationSet); + } + + // 5) Verify the package still exists (we've had bugs here before). + if (validatingEntity.Status == PackageStatus.Available + && !await _packageFileService.DoesPackageFileExistAsync(validationSet)) + { + var validationPackageAvailable = await _packageFileService.DoesValidationPackageFileExistAsync(validationSet); + + _logger.LogWarning("Package {PackageId} {PackageVersion} is marked as available, but does not exist " + + "in public container. Does package exist in validation container: {ExistsInValidation}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationPackageAvailable); + + // Report missing package, don't try to fix up anything. This shouldn't happen and needs an investigation. + _telemetryService.TrackMissingNupkgForAvailablePackage( + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId.ToString()); + } + } + + /// + /// Allows descendants to do additional operations before database is updated to mark package as available. + /// + /// Entity being marked as available. + /// Validation set that was completed. + protected virtual Task OnBeforeUpdateDatabaseToMakePackageAvailable(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + { + return Task.CompletedTask; + } + + private async Task MarkPackageAsAvailableAsync( + PackageValidationSet validationSet, + IValidatingEntity validatingEntity, + PackageStreamMetadata streamMetadata, + bool copied) + { + // Use whatever package made it into the packages container. This is what customers will consume so the DB + // record must match. + + // We don't immediately commit here. Later, we will commit these changes as well as the new package + // status as part of the same transaction. + await _galleryPackageService.UpdateMetadataAsync( + validatingEntity.EntityRecord, + streamMetadata, + commitChanges: false); + + _logger.LogInformation("Marking package {PackageId} {PackageVersion}, validation set {ValidationSetId} as {PackageStatus} in DB", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId, + PackageStatus.Available); + + var fromStatus = validatingEntity.Status; + + try + { + // Make the package available and commit any other pending changes (e.g. updated hash). + await _galleryPackageService.UpdateStatusAsync(validatingEntity.EntityRecord, PackageStatus.Available, commitChanges: true); + } + catch (Exception e) + { + _logger.LogError( + Error.UpdatingPackageDbStatusFailed, + e, + "Failed to update package status in Gallery Db. Package {PackageId} {PackageVersion}, validation set {ValidationSetId}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId); + + // If this execution was not the one to copy the package, then don't delete the package on failure. + // This prevents a missing passing in the (unlikely) case where two actors attempt the DB update, one + // succeeds and one fails. We don't want an available package record with nothing in the packages + // container! + if (copied && fromStatus != PackageStatus.Available) + { + await _packageFileService.DeletePackageFileAsync(validationSet); + await OnCleanupAfterDatabaseUpdateFailure(validatingEntity, validationSet); + } + + throw; + } + + return fromStatus; + } + + /// + /// Allows descendants to do additional cleanup on failure to update DB when marking package as available. + /// Only called if package was copied to public container before trying to update DB. + /// + /// Entity being marked as available. + /// Validation set that was completed. + protected virtual Task OnCleanupAfterDatabaseUpdateFailure( + IValidatingEntity validatingEntity, + PackageValidationSet validationSet) + { + return Task.CompletedTask; + } + + private async Task UpdatePublicPackageAsync(PackageValidationSet validationSet) + { + _logger.LogInformation("Copying .nupkg to public storage for package {PackageId} {PackageVersion}, validation set {ValidationSetId}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId); + + // If the validation set contains any processors, we must use the copy of the package that is specific to + // this validation set. We can't use the original validation package because it does not have any of the + // changes that the processors made. If the validation set package does not exist for some reason and there + // are processors in the validation set, this indicates a bug and an exception will be thrown by the copy + // operation below. This will cause the validation queue message to eventually dead-letter at which point + // the on-call person should investigate. + bool copied; + if (validationSet.PackageValidations.Any(x => _validatorProvider.IsProcessor(x.Type)) || + await _packageFileService.DoesValidationSetPackageExistAsync(validationSet)) + { + IAccessCondition destAccessCondition; + + // The package etag will be null if this validation set is expecting the package to not yet exist in + // the packages container. + if (validationSet.PackageETag == null) + { + // This will fail with HTTP 409 if the package already exists. This means that another validation + // set has completed and moved the package into the Available state first, with different package + // content. + destAccessCondition = AccessConditionWrapper.GenerateIfNotExistsCondition(); + + _logger.LogInformation( + "Attempting to copy validation set {ValidationSetId} package {PackageId} {PackageVersion} to" + + " the packages container, assuming that the package does not already exist.", + validationSet.ValidationTrackingId, + validationSet.PackageId, + validationSet.PackageNormalizedVersion); + } + else + { + // This will fail with HTTP 412 if the package has been modified by another validation set. This + // would only happen if this validation set and another validation set are operating on a package + // already in the Available state. + destAccessCondition = AccessConditionWrapper.GenerateIfMatchCondition(validationSet.PackageETag); + + _logger.LogInformation( + "Attempting to copy validation set {ValidationSetId} package {PackageId} {PackageVersion} to" + + " the packages container, assuming that the package has etag {PackageETag}.", + validationSet.ValidationTrackingId, + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.PackageETag); + } + + // Failures here should result in an unhandled exception. This means that this validation set has + // modified the package but is unable to copy the modified package into the packages container because + // another validation set completed first. + await _packageFileService.CopyValidationSetPackageToPackageFileAsync( + validationSet, + destAccessCondition); + + copied = true; + } + else + { + _logger.LogInformation( + "The package specific to the validation set does not exist. Falling back to the validation " + + "container for package {PackageId} {PackageVersion}, validation set {ValidationSetId}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId); + + try + { + await _packageFileService.CopyValidationPackageToPackageFileAsync(validationSet); + + copied = true; + } + catch (InvalidOperationException) + { + // The package already exists in the packages container. This can happen if the DB commit below fails + // and this flow is retried or another validation set for the package completed first. Either way, we + // will later attempt to use the hash from the package in the packages container (the destination). + // In other words, we don't care which copy wins when copying from the validation package because + // we know the package has not been modified. + _logger.LogInformation( + "Package already exists in packages container for {PackageId} {PackageVersion}, validation set {ValidationSetId}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId); + + copied = false; + } + } + + return copied; + } + } +} diff --git a/src/NuGet.Services.Validation.Orchestrator/Job.cs b/src/NuGet.Services.Validation.Orchestrator/Job.cs index 3d4659d6b..de0c7152d 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Job.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Job.cs @@ -61,9 +61,9 @@ public class Job : JobBase private const string GalleryDbConfigurationSectionName = "GalleryDb"; private const string ValidationDbConfigurationSectionName = "ValidationDb"; private const string ServiceBusConfigurationSectionName = "ServiceBus"; - private const string SmtpConfigurationSectionName = "Smtp"; private const string EmailConfigurationSectionName = "Email"; private const string PackageDownloadTimeoutName = "PackageDownloadTimeout"; + private const string FlatContainerConfigurationSectionName = "FlatContainer"; private const string EmailBindingKey = EmailConfigurationSectionName; private const string PackageVerificationTopicClientBindingKey = "PackageVerificationTopicClient"; @@ -74,6 +74,7 @@ public class Job : JobBase private const string ScanBindingKey = "Scan"; private const string ValidationStorageBindingKey = "ValidationStorage"; private const string OrchestratorBindingKey = "Orchestrator"; + private const string CoreLicenseFileServiceBindingKey = "CoreLicenseFileService"; private const string SymbolsValidatorSectionName = "SymbolsValidator"; private const string SymbolsValidationBindingKey = SymbolsValidatorSectionName; @@ -193,6 +194,7 @@ private void ConfigureJobServices(IServiceCollection services, IConfigurationRoo services.Configure(configurationRoot.GetSection(ScanAndSignSectionName)); services.Configure(configurationRoot.GetSection(SymbolScanOnlySectionName)); services.Configure(configurationRoot.GetSection(ScanAndSignSectionName)); + services.Configure(configurationRoot.GetSection(FlatContainerConfigurationSectionName)); services.Configure(configurationRoot.GetSection(SymbolsValidatorSectionName)); services.Configure(configurationRoot.GetSection(SymbolsIngesterSectionName)); @@ -243,7 +245,7 @@ private void ConfigureJobServices(IServiceCollection services, IConfigurationRoo }); services.AddTransient(); services.AddTransient(); - services.AddTransient, EntityStatusProcessor>(); + services.AddTransient, PackageStatusProcessor>(); services.AddTransient, ValidationSetProvider>(); services.AddTransient(); services.AddTransient, SignatureValidationMessageSerializer>(); @@ -349,6 +351,7 @@ private static IServiceProvider CreateProvider(IServiceCollection services, ICon ConfigurePackageCertificatesValidator(containerBuilder); ConfigureScanAndSignProcessor(containerBuilder); ConfigureScanValidator(containerBuilder); + ConfigureFlatContainer(containerBuilder); break; case ValidatingType.SymbolPackage: ConfigureSymbolScanValidator(containerBuilder); @@ -507,6 +510,33 @@ private static void ConfigureScanValidator(ContainerBuilder builder) .AsSelf(); } + private static void ConfigureFlatContainer(ContainerBuilder builder) + { + builder + .Register(c => + { + var configurationAccessor = c.Resolve>(); + return new CloudBlobClientWrapper( + configurationAccessor.Value.ConnectionString, + readAccessGeoRedundant: false); + }) + .Keyed(CoreLicenseFileServiceBindingKey); + + builder + .RegisterType() + .WithKeyedParameter(typeof(ICloudBlobClient), CoreLicenseFileServiceBindingKey) + .Keyed(CoreLicenseFileServiceBindingKey); + + builder + .RegisterType() + .As(); + + builder + .RegisterType() + .WithKeyedParameter(typeof(ICoreFileStorageService), CoreLicenseFileServiceBindingKey) + .As(); + } + private static void ConfigureOrchestratorMessageHandler(IServiceCollection services, IConfigurationRoot configurationRoot) { var validatingType = configurationRoot diff --git a/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj b/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj index fba087900..c550fb080 100644 --- a/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj +++ b/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj @@ -45,12 +45,16 @@ + + + + @@ -92,7 +96,6 @@ - @@ -160,7 +163,7 @@ all - 2.39.0 + 2.40.0 diff --git a/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs index d4454b9fe..218f238cb 100644 --- a/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs +++ b/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs @@ -2,287 +2,62 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using NuGet.Services.Entities; using NuGet.Services.Validation.Orchestrator.Telemetry; using NuGetGallery; -using NuGetGallery.Packaging; namespace NuGet.Services.Validation.Orchestrator { - public class EntityStatusProcessor : IStatusProcessor where T : class, IEntity + public class PackageStatusProcessor : EntityStatusProcessor { - protected readonly IEntityService _galleryPackageService; - protected readonly IValidationFileService _packageFileService; - protected readonly IValidatorProvider _validatorProvider; - protected readonly ITelemetryService _telemetryService; - protected readonly ILogger> _logger; + private readonly ICoreLicenseFileService _coreLicenseFileService; - public EntityStatusProcessor( - IEntityService galleryPackageService, + public PackageStatusProcessor( + IEntityService galleryPackageService, IValidationFileService packageFileService, IValidatorProvider validatorProvider, ITelemetryService telemetryService, - ILogger> logger) + ILogger> logger, + ICoreLicenseFileService coreLicenseFileService) + : base(galleryPackageService, packageFileService, validatorProvider, telemetryService, logger) { - _galleryPackageService = galleryPackageService ?? throw new ArgumentNullException(nameof(galleryPackageService)); - _packageFileService = packageFileService ?? throw new ArgumentNullException(nameof(packageFileService)); - _validatorProvider = validatorProvider ?? throw new ArgumentNullException(nameof(validatorProvider)); - _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService)); - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _coreLicenseFileService = coreLicenseFileService ?? throw new ArgumentNullException(nameof(coreLicenseFileService)); } - public Task SetStatusAsync( - IValidatingEntity validatingEntity, - PackageValidationSet validationSet, - PackageStatus status) + protected override async Task OnBeforeUpdateDatabaseToMakePackageAvailable( + IValidatingEntity validatingEntity, + PackageValidationSet validationSet) { - if (validatingEntity == null) + if (validatingEntity.EntityRecord.EmbeddedLicenseType != EmbeddedLicenseFileType.Absent) { - throw new ArgumentNullException(nameof(validatingEntity)); - } - - if (validationSet == null) - { - throw new ArgumentNullException(nameof(validationSet)); - } - - if (validatingEntity.Status == PackageStatus.Deleted) - { - throw new ArgumentException( - $"A package in the {nameof(PackageStatus.Deleted)} state cannot be processed.", - nameof(validatingEntity)); - } - - if (validatingEntity.Status == PackageStatus.Available && - status == PackageStatus.FailedValidation) - { - throw new ArgumentException( - $"A package cannot transition from {nameof(PackageStatus.Available)} to {nameof(PackageStatus.FailedValidation)}.", - nameof(status)); - } - - switch (status) - { - case PackageStatus.Available: - return MakePackageAvailableAsync(validatingEntity, validationSet); - case PackageStatus.FailedValidation: - return MakePackageFailedValidationAsync(validatingEntity, validationSet); - default: - throw new ArgumentException( - $"A package can only transition to the {nameof(PackageStatus.Available)} or " + - $"{nameof(PackageStatus.FailedValidation)} states.", nameof(status)); - } - } - - protected virtual async Task MakePackageFailedValidationAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) - { - var fromStatus = validatingEntity.Status; - - await _galleryPackageService.UpdateStatusAsync(validatingEntity.EntityRecord, PackageStatus.FailedValidation, commitChanges: true); - - if (fromStatus != PackageStatus.FailedValidation) - { - _telemetryService.TrackPackageStatusChange(fromStatus, PackageStatus.FailedValidation); - } - } - - protected virtual async Task MakePackageAvailableAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) - { - // 1) Operate on blob storage. - var copied = await UpdatePublicPackageAsync(validationSet); - - // 2) Update the package's blob metadata in the public blob storage container. - var metadata = await _packageFileService.UpdatePackageBlobMetadataAsync(validationSet); - - // 3) Update the package's blob properties in the public blob storage container. - await _packageFileService.UpdatePackageBlobPropertiesAsync(validationSet); - - // 4) Operate on the database. - var fromStatus = await MarkPackageAsAvailableAsync(validationSet, validatingEntity, metadata, copied); - - // 5) Emit telemetry and clean up. - if (fromStatus != PackageStatus.Available) - { - _telemetryService.TrackPackageStatusChange(fromStatus, PackageStatus.Available); - - _logger.LogInformation("Deleting from the source for package {PackageId} {PackageVersion}, validation set {ValidationSetId}", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId); - - await _packageFileService.DeleteValidationPackageFileAsync(validationSet); - } - - // 5) Verify the package still exists (we've had bugs here before). - if (validatingEntity.Status == PackageStatus.Available - && !await _packageFileService.DoesPackageFileExistAsync(validationSet)) - { - var validationPackageAvailable = await _packageFileService.DoesValidationPackageFileExistAsync(validationSet); - - _logger.LogWarning("Package {PackageId} {PackageVersion} is marked as available, but does not exist " + - "in public container. Does package exist in validation container: {ExistsInValidation}", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationPackageAvailable); - - // Report missing package, don't try to fix up anything. This shouldn't happen and needs an investigation. - _telemetryService.TrackMissingNupkgForAvailablePackage( - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId.ToString()); - } - } - - private async Task MarkPackageAsAvailableAsync( - PackageValidationSet validationSet, - IValidatingEntity validatingEntity, - PackageStreamMetadata streamMetadata, - bool copied) - { - // Use whatever package made it into the packages container. This is what customers will consume so the DB - // record must match. - - // We don't immediately commit here. Later, we will commit these changes as well as the new package - // status as part of the same transaction. - await _galleryPackageService.UpdateMetadataAsync( - validatingEntity.EntityRecord, - streamMetadata, - commitChanges: false); - - _logger.LogInformation("Marking package {PackageId} {PackageVersion}, validation set {ValidationSetId} as {PackageStatus} in DB", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId, - PackageStatus.Available); - - var fromStatus = validatingEntity.Status; - - try - { - // Make the package available and commit any other pending changes (e.g. updated hash). - await _galleryPackageService.UpdateStatusAsync(validatingEntity.EntityRecord, PackageStatus.Available, commitChanges: true); - } - catch (Exception e) - { - _logger.LogError( - Error.UpdatingPackageDbStatusFailed, - e, - "Failed to update package status in Gallery Db. Package {PackageId} {PackageVersion}, validation set {ValidationSetId}", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId); - - // If this execution was not the one to copy the package, then don't delete the package on failure. - // This prevents a missing passing in the (unlikely) case where two actors attempt the DB update, one - // succeeds and one fails. We don't want an available package record with nothing in the packages - // container! - if (copied && fromStatus != PackageStatus.Available) + using (_telemetryService.TrackDurationToExtractLicenseFile(validationSet.PackageId, validationSet.PackageNormalizedVersion, validationSet.ValidationTrackingId.ToString())) + using (var packageStream = await _packageFileService.DownloadPackageFileToDiskAsync(validationSet)) { - await _packageFileService.DeletePackageFileAsync(validationSet); + _logger.LogInformation("Extracting the license file of type {EmbeddedLicenseFileType} for the package {PackageId} {PackageVersion}", + validatingEntity.EntityRecord.EmbeddedLicenseType, + validationSet.PackageId, + validationSet.PackageNormalizedVersion); + await _coreLicenseFileService.ExtractAndSaveLicenseFileAsync(validatingEntity.EntityRecord, packageStream); + _logger.LogInformation("Successfully extracted the license file."); } - - throw; } - - return fromStatus; } - private async Task UpdatePublicPackageAsync(PackageValidationSet validationSet) + protected override async Task OnCleanupAfterDatabaseUpdateFailure( + IValidatingEntity validatingEntity, + PackageValidationSet validationSet) { - _logger.LogInformation("Copying .nupkg to public storage for package {PackageId} {PackageVersion}, validation set {ValidationSetId}", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId); - - // If the validation set contains any processors, we must use the copy of the package that is specific to - // this validation set. We can't use the original validation package because it does not have any of the - // changes that the processors made. If the validation set package does not exist for some reason and there - // are processors in the validation set, this indicates a bug and an exception will be thrown by the copy - // operation below. This will cause the validation queue message to eventually dead-letter at which point - // the on-call person should investigate. - bool copied; - if (validationSet.PackageValidations.Any(x => _validatorProvider.IsProcessor(x.Type)) || - await _packageFileService.DoesValidationSetPackageExistAsync(validationSet)) - { - IAccessCondition destAccessCondition; - - // The package etag will be null if this validation set is expecting the package to not yet exist in - // the packages container. - if (validationSet.PackageETag == null) - { - // This will fail with HTTP 409 if the package already exists. This means that another validation - // set has completed and moved the package into the Available state first, with different package - // content. - destAccessCondition = AccessConditionWrapper.GenerateIfNotExistsCondition(); - - _logger.LogInformation( - "Attempting to copy validation set {ValidationSetId} package {PackageId} {PackageVersion} to" + - " the packages container, assuming that the package does not already exist.", - validationSet.ValidationTrackingId, - validationSet.PackageId, - validationSet.PackageNormalizedVersion); - } - else - { - // This will fail with HTTP 412 if the package has been modified by another validation set. This - // would only happen if this validation set and another validation set are operating on a package - // already in the Available state. - destAccessCondition = AccessConditionWrapper.GenerateIfMatchCondition(validationSet.PackageETag); - - _logger.LogInformation( - "Attempting to copy validation set {ValidationSetId} package {PackageId} {PackageVersion} to" + - " the packages container, assuming that the package has etag {PackageETag}.", - validationSet.ValidationTrackingId, - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.PackageETag); - } - - // Failures here should result in an unhandled exception. This means that this validation set has - // modified the package but is unable to copy the modified package into the packages container because - // another validation set completed first. - await _packageFileService.CopyValidationSetPackageToPackageFileAsync( - validationSet, - destAccessCondition); - - copied = true; - } - else + if (validatingEntity.EntityRecord.EmbeddedLicenseType != EmbeddedLicenseFileType.Absent) { - _logger.LogInformation( - "The package specific to the validation set does not exist. Falling back to the validation " + - "container for package {PackageId} {PackageVersion}, validation set {ValidationSetId}", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId); - - try + using (_telemetryService.TrackDurationToDeleteLicenseFile(validationSet.PackageId, validationSet.PackageNormalizedVersion, validationSet.ValidationTrackingId.ToString())) { - await _packageFileService.CopyValidationPackageToPackageFileAsync(validationSet); - - copied = true; - } - catch (InvalidOperationException) - { - // The package already exists in the packages container. This can happen if the DB commit below fails - // and this flow is retried or another validation set for the package completed first. Either way, we - // will later attempt to use the hash from the package in the packages container (the destination). - // In other words, we don't care which copy wins when copying from the validation package because - // we know the package has not been modified. - _logger.LogInformation( - "Package already exists in packages container for {PackageId} {PackageVersion}, validation set {ValidationSetId}", - validationSet.PackageId, - validationSet.PackageNormalizedVersion, - validationSet.ValidationTrackingId); - - copied = false; + _logger.LogInformation("Cleaning up the license file for the package {PackageId} {PackageVersion}", validationSet.PackageId, validationSet.PackageNormalizedVersion); + await _coreLicenseFileService.DeleteLicenseFileAsync(validationSet.PackageId, validationSet.PackageNormalizedVersion); + _logger.LogInformation("Deleted the license file for the package {PackageId} {PackageVersion}", validationSet.PackageId, validationSet.PackageNormalizedVersion); } } - - return copied; } } } diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/OrchestratorContentFileMetadataService.cs b/src/NuGet.Services.Validation.Orchestrator/Services/OrchestratorContentFileMetadataService.cs new file mode 100644 index 000000000..44c91cd4f --- /dev/null +++ b/src/NuGet.Services.Validation.Orchestrator/Services/OrchestratorContentFileMetadataService.cs @@ -0,0 +1,18 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using NuGetGallery; + +namespace NuGet.Services.Validation.Orchestrator +{ + class OrchestratorContentFileMetadataService : IContentFileMetadataService + { + public OrchestratorContentFileMetadataService() + { + } + + public string PackageContentFolderName => CoreConstants.Folders.FlatContainerFolderName; + + public string PackageContentPathTemplate => CoreConstants.PackageContentFileSavePathTemplate; + } +} diff --git a/src/NuGet.Services.Validation.Orchestrator/Telemetry/ITelemetryService.cs b/src/NuGet.Services.Validation.Orchestrator/Telemetry/ITelemetryService.cs index 1b556a4fe..e64d25db4 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Telemetry/ITelemetryService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Telemetry/ITelemetryService.cs @@ -131,5 +131,22 @@ public interface ITelemetryService /// The validator name the message was sent to. /// The validationId. void TrackSymbolsMessageEnqueued(string validatorName, Guid validationId); + + /// + /// A metric to track duration of the license file extraction. + /// + /// Package ID from which license file is extracted. + /// Normalized version of the package from which license file is extracted. + /// Validation tracking ID for which extraction is done. + /// + IDisposable TrackDurationToExtractLicenseFile(string packageId, string normalizedVersion, string validationTrackingId); + + /// + /// A metric to track duration of the license file deletion from flat container. + /// + /// Package ID for which licenes file is deleted. + /// Normalized version of the package for which license file is deleted. + /// Validation tracking ID for which delete operation is done. + IDisposable TrackDurationToDeleteLicenseFile(string packageId, string normalizedVersion, string validationTrackingId); } } \ No newline at end of file diff --git a/src/NuGet.Services.Validation.Orchestrator/Telemetry/TelemetryService.cs b/src/NuGet.Services.Validation.Orchestrator/Telemetry/TelemetryService.cs index 473e48acf..191ba4926 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Telemetry/TelemetryService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Telemetry/TelemetryService.cs @@ -35,9 +35,10 @@ public class TelemetryService : ITelemetryService, ISubscriptionProcessorTelemet private const string MessageHandlerDurationSeconds = OrchestratorPrefix + "MessageHandlerDurationSeconds"; private const string MessageLockLost = OrchestratorPrefix + "MessageLockLost"; private const string SymbolsMessageEnqueued = OrchestratorPrefix + "SymbolsMessageEnqueued"; + private const string ExtractLicenseFileDuration = OrchestratorPrefix + "ExtractLicenseFileDuration"; + private const string LicenseFileDeleted = OrchestratorPrefix + "LicenseFileDeleted"; private const string DurationToStartPackageSigningValidatorSeconds = PackageSigningPrefix + "DurationToStartSeconds"; - private const string DurationToStartPackageCertificatesValidatorSeconds = PackageCertificatesPrefix + "DurationToStartSeconds"; private const string FromStatus = "FromStatus"; @@ -298,5 +299,21 @@ public void TrackSymbolsMessageEnqueued(string validatorName, Guid validationId) { ValidatorType, validatorName }, { ValidationId, validationId.ToString()} }); + + public IDisposable TrackDurationToExtractLicenseFile(string packageId, string normalizedVersion, string validationTrackingId) + => _telemetryClient.TrackDuration(ExtractLicenseFileDuration, + new Dictionary { + { PackageId, packageId }, + { NormalizedVersion, normalizedVersion }, + { ValidationTrackingId, validationTrackingId }, + }); + + public IDisposable TrackDurationToDeleteLicenseFile(string packageId, string normalizedVersion, string validationTrackingId) + => _telemetryClient.TrackDuration(LicenseFileDeleted, + new Dictionary { + { PackageId, packageId }, + { NormalizedVersion, normalizedVersion }, + { ValidationTrackingId, validationTrackingId }, + }); } } diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs index 4eb94032a..4dfe6f81b 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs +++ b/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs @@ -99,7 +99,7 @@ public async Task ProcessValidationOutcomeAsync(PackageValidationSet validationS var fromStatus = validatingEntity.Status; - // Always set the package status to available so that processors can have a change to fix packages + // Always set the package status to available so that processors can have a chance to fix packages // that are already available. Processors should no-op when their work is already done, so the // modification of an already available package should be rare. The most common case for this is if // the processor has never been run on a package that was published before the processor was diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs index 52a917b94..22b274d07 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs @@ -47,7 +47,7 @@ public static string BuildFileName(PackageValidationSet validationSet, string pa { string id = validationSet.PackageId; string version = validationSet.PackageNormalizedVersion; - return BuildFileName(id, version, pathTemplate, extension); + return FileNameHelper.BuildFileName(id, version, pathTemplate, extension); } public Task GetPackageReadUriAsync(PackageValidationSet validationSet) diff --git a/src/NuGet.Services.Validation.Orchestrator/settings.json b/src/NuGet.Services.Validation.Orchestrator/settings.json index 1b24eeeab..d4dd614e6 100644 --- a/src/NuGet.Services.Validation.Orchestrator/settings.json +++ b/src/NuGet.Services.Validation.Orchestrator/settings.json @@ -97,6 +97,9 @@ "AnnouncementsUrl": "https://github.com/NuGet/Announcements/issues", "TwitterUrl": "https://twitter.com/nuget" }, + "FlatContainer": { + "ConnectionString": "" + }, "PackageDownloadTimeout": "00:10:00", "KeyVault_VaultName": "", "KeyVault_ClientId": "", diff --git a/src/PackageHash/PackageHash.csproj b/src/PackageHash/PackageHash.csproj index ea57797da..171e8e0cc 100644 --- a/src/PackageHash/PackageHash.csproj +++ b/src/PackageHash/PackageHash.csproj @@ -81,7 +81,7 @@ 1.1.2 - 2.39.0 + 2.40.0 diff --git a/src/PackageLagMonitor/Monitoring.PackageLag.csproj b/src/PackageLagMonitor/Monitoring.PackageLag.csproj index db2b507e9..7db850ccb 100644 --- a/src/PackageLagMonitor/Monitoring.PackageLag.csproj +++ b/src/PackageLagMonitor/Monitoring.PackageLag.csproj @@ -112,7 +112,7 @@ 0.5.0-CI-20180510-012541 - 2.39.0 + 2.40.0 4.3.3 diff --git a/src/StatusAggregator/StatusAggregator.csproj b/src/StatusAggregator/StatusAggregator.csproj index 71ec7892a..d63eaeef5 100644 --- a/src/StatusAggregator/StatusAggregator.csproj +++ b/src/StatusAggregator/StatusAggregator.csproj @@ -157,13 +157,13 @@ 1.1.1 - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 9.2.0 diff --git a/src/Validation.Common.Job/Validation.Common.Job.csproj b/src/Validation.Common.Job/Validation.Common.Job.csproj index b6cca797b..a2e11d3e6 100644 --- a/src/Validation.Common.Job/Validation.Common.Job.csproj +++ b/src/Validation.Common.Job/Validation.Common.Job.csproj @@ -103,19 +103,19 @@ 5.0.0-preview1.5665 - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 - 2.39.0 + 2.40.0 - 4.4.5-master-2224741 + 4.4.5-dev-2258789 2.5.0 diff --git a/src/Validation.ScanAndSign.Core/Validation.ScanAndSign.Core.csproj b/src/Validation.ScanAndSign.Core/Validation.ScanAndSign.Core.csproj index d7701388e..4ab76c755 100644 --- a/src/Validation.ScanAndSign.Core/Validation.ScanAndSign.Core.csproj +++ b/src/Validation.ScanAndSign.Core/Validation.ScanAndSign.Core.csproj @@ -64,7 +64,7 @@ all - 2.39.0 + 2.40.0 0.3.0 diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs index 4a4efc89a..7ce58da30 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs @@ -162,6 +162,35 @@ public async Task SetsPackageStreamMetadataIfChanged() Times.Once); } + [Theory] + [InlineData(EmbeddedLicenseFileType.Absent, false)] + [InlineData(EmbeddedLicenseFileType.PlainText, true)] + [InlineData(EmbeddedLicenseFileType.Markdown, true)] + public async Task SavesPackageLicenseFileWhenPresent(EmbeddedLicenseFileType licenseFileType, bool expectedSave) + { + var content = "Hello, world."; + var stream = new MemoryStream(Encoding.ASCII.GetBytes(content)); + Package.EmbeddedLicenseType = licenseFileType; + PackageFileServiceMock + .Setup(x => x.DownloadPackageFileToDiskAsync(ValidationSet)) + .ReturnsAsync(stream); + + await Target.SetStatusAsync(PackageValidatingEntity, ValidationSet, PackageStatus.Available); + + if (expectedSave) + { + CoreLicenseFileServiceMock + .Verify(clfs => clfs.ExtractAndSaveLicenseFileAsync(PackageValidatingEntity.EntityRecord, stream), Times.Once); + CoreLicenseFileServiceMock + .Verify(clfs => clfs.ExtractAndSaveLicenseFileAsync(It.IsAny(), It.IsAny()), Times.Once); + } + else + { + CoreLicenseFileServiceMock + .Verify(clfs => clfs.ExtractAndSaveLicenseFileAsync(It.IsAny(), It.IsAny()), Times.Never); + } + } + [Fact] public async Task AllowsPackageAlreadyInPublicContainerWhenValidationSetPackageDoesNotExist() { @@ -300,6 +329,42 @@ public async Task DeletesPackageFromPublicStorageOnDbUpdateFailureIfCopiedAndOri Times.Never); } + [Theory] + [InlineData(EmbeddedLicenseFileType.Absent, PackageStatus.Validating, false)] + [InlineData(EmbeddedLicenseFileType.Absent, PackageStatus.Available, false)] + [InlineData(EmbeddedLicenseFileType.PlainText, PackageStatus.Validating, true)] + [InlineData(EmbeddedLicenseFileType.PlainText, PackageStatus.Available, false)] + [InlineData(EmbeddedLicenseFileType.Markdown, PackageStatus.Validating, true)] + [InlineData(EmbeddedLicenseFileType.Markdown, PackageStatus.Available, false)] + public async Task DeletesLicenseFromPublicStorageOnDbUpdateFailure(EmbeddedLicenseFileType licenseFileType, PackageStatus originalStatus, bool expectedDelete) + { + Package.PackageStatusKey = originalStatus; + Package.EmbeddedLicenseType = licenseFileType; + + var expected = new Exception("Everything failed"); + PackageServiceMock + .Setup(ps => ps.UpdateStatusAsync(Package, PackageStatus.Available, true)) + .Throws(expected); + + var actual = await Assert.ThrowsAsync( + () => Target.SetStatusAsync(PackageValidatingEntity, ValidationSet, PackageStatus.Available)); + + Assert.Same(expected, actual); + + if (expectedDelete) + { + CoreLicenseFileServiceMock + .Verify(clfs => clfs.DeleteLicenseFileAsync(Package.Id, Package.NormalizedVersion), Times.Once); + CoreLicenseFileServiceMock + .Verify(clfs => clfs.DeleteLicenseFileAsync(It.IsAny(), It.IsAny()), Times.Once); + } + else + { + CoreLicenseFileServiceMock + .Verify(clfs => clfs.DeleteLicenseFileAsync(It.IsAny(), It.IsAny()), Times.Never); + } + } + [Fact] public async Task DoesNotDeletePackageFromPublicStorageOnDbUpdateFailureIfCopiedAndOriginallyAvailable() { @@ -526,6 +591,7 @@ public BaseFacts() ValidatorProviderMock = new Mock(); TelemetryServiceMock = new Mock(); LoggerMock = new Mock>>(); + CoreLicenseFileServiceMock = new Mock(); var streamMetadata = new PackageStreamMetadata() { @@ -538,12 +604,13 @@ public BaseFacts() .Setup(x => x.UpdatePackageBlobMetadataAsync(It.IsAny())) .ReturnsAsync(streamMetadata); - Target = new EntityStatusProcessor( + Target = new PackageStatusProcessor( PackageServiceMock.Object, PackageFileServiceMock.Object, ValidatorProviderMock.Object, TelemetryServiceMock.Object, - LoggerMock.Object); + LoggerMock.Object, + CoreLicenseFileServiceMock.Object); PackageValidatingEntity = new PackageValidatingEntity(Package); } @@ -555,6 +622,7 @@ public BaseFacts() public Mock ValidatorProviderMock { get; } public Mock TelemetryServiceMock { get; } public Mock>> LoggerMock { get; } + public Mock CoreLicenseFileServiceMock { get; } public EntityStatusProcessor Target { get; } public PackageValidatingEntity PackageValidatingEntity { get; } }