From 99fa36160803014e71ab2464cae0c989a80e49e7 Mon Sep 17 00:00:00 2001 From: Lana Parezanin Date: Thu, 11 Jul 2024 11:20:13 -0700 Subject: [PATCH 1/6] Made initial changes --- src/NuGet.Jobs.Common/JobRunner.cs | 8 ++++---- src/NuGet.Jobs.GitHubIndexer/Job.cs | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Jobs.Common/JobRunner.cs b/src/NuGet.Jobs.Common/JobRunner.cs index 36b833a7b2..d0d397149b 100644 --- a/src/NuGet.Jobs.Common/JobRunner.cs +++ b/src/NuGet.Jobs.Common/JobRunner.cs @@ -85,7 +85,7 @@ private static async Task Run(JobBase job, string[] commandLineArgs, bool? commandLineArgs); // Setup logging - _applicationInsightsConfiguration = ConfigureApplicationInsights(job, jobArgsDictionary); + _applicationInsightsConfiguration = ConfigureApplicationInsights(job, jobArgsDictionary); // Configure our logging again with Application Insights initialized. loggerFactory = ConfigureLogging(job, _applicationInsightsConfiguration.TelemetryConfiguration); @@ -279,9 +279,9 @@ private static async Task JobLoop( _logger.LogInformation("Job run took {RunDuration}", PrettyPrintTime(stopWatch.ElapsedMilliseconds)); _applicationInsightsConfiguration.DiagnosticsTelemetryModule?.SetHeartbeatProperty( - HeartbeatProperty_JobLoopExitCode, - exitCode.ToString(), - isHealthy: exitCode == 0); + HeartbeatProperty_JobLoopExitCode, + exitCode.ToString(), + isHealthy: exitCode == 0); } if (!runContinuously) diff --git a/src/NuGet.Jobs.GitHubIndexer/Job.cs b/src/NuGet.Jobs.GitHubIndexer/Job.cs index efb24c52e3..470ec3bef1 100644 --- a/src/NuGet.Jobs.GitHubIndexer/Job.cs +++ b/src/NuGet.Jobs.GitHubIndexer/Job.cs @@ -44,6 +44,7 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi services.AddTransient(); services.AddTransient(provider => { var config = provider.GetRequiredService>(); + //return new CloudBlobClientWrapper.UsingMsi(config.Value.StorageConnectionString, config.Value.StorageReadAccessGeoRedundant); return new CloudBlobClientWrapper(config.Value.StorageConnectionString, config.Value.StorageReadAccessGeoRedundant); }); From 1d1903d5e3b71e23b1dc2d475d89fc95d2043019 Mon Sep 17 00:00:00 2001 From: Andrei Grigorev Date: Thu, 11 Jul 2024 14:56:34 -0700 Subject: [PATCH 2/6] CloudBlobLeaseService using new SDK. --- .../Job.cs | 5 +- .../Leases/CloudBlobLeaseService.cs | 71 ++++++++----------- .../Validation.Common.Job.csproj | 1 - .../CloudBlobLeaseServiceIntegrationTests.cs | 9 +-- 4 files changed, 35 insertions(+), 51 deletions(-) diff --git a/src/NuGet.Services.Validation.Orchestrator/Job.cs b/src/NuGet.Services.Validation.Orchestrator/Job.cs index 861bf44a0a..e343de91db 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Job.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Job.cs @@ -10,12 +10,12 @@ using System.Threading.Tasks; using Autofac; using Autofac.Core; +using Azure.Storage.Blobs; using Microsoft.ApplicationInsights; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.WindowsAzure.Storage; using NuGet.Jobs; using NuGet.Jobs.Configuration; using NuGet.Jobs.Validation; @@ -387,8 +387,7 @@ private static void ConfigureLeaseService(ContainerBuilder builder) .Register(c => { var config = c.Resolve>().Value; - var storageAccount = CloudStorageAccount.Parse(config.ConnectionString); - var blobClient = storageAccount.CreateCloudBlobClient(); + var blobClient = new BlobServiceClient(config.ConnectionString); return new CloudBlobLeaseService(blobClient, config.ContainerName, config.StoragePath); }) .As(); diff --git a/src/Validation.Common.Job/Leases/CloudBlobLeaseService.cs b/src/Validation.Common.Job/Leases/CloudBlobLeaseService.cs index 4d5636470e..249ec0d048 100644 --- a/src/Validation.Common.Job/Leases/CloudBlobLeaseService.cs +++ b/src/Validation.Common.Job/Leases/CloudBlobLeaseService.cs @@ -2,11 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO; using System.Net; using System.Threading; using System.Threading.Tasks; -using Microsoft.WindowsAzure.Storage; -using Microsoft.WindowsAzure.Storage.Blob; +using Azure; +using Azure.Storage.Blobs; +using Azure.Storage.Blobs.Specialized; namespace NuGet.Jobs.Validation.Leases { @@ -20,11 +22,11 @@ public class CloudBlobLeaseService : ILeaseService private static readonly TimeSpan MinLeaseTime = TimeSpan.FromSeconds(15); private static readonly TimeSpan MaxLeaseTime = TimeSpan.FromSeconds(60); - private readonly CloudBlobClient _cloudBlobClient; + private readonly BlobServiceClient _cloudBlobClient; private readonly string _containerName; private readonly string _basePath; - public CloudBlobLeaseService(CloudBlobClient cloudBlobClient, string containerName, string basePath) + public CloudBlobLeaseService(BlobServiceClient cloudBlobClient, string containerName, string basePath) { _cloudBlobClient = cloudBlobClient ?? throw new ArgumentNullException(nameof(cloudBlobClient)); _containerName = containerName ?? throw new ArgumentNullException(nameof(containerName)); @@ -36,34 +38,32 @@ public async Task TryAcquireAsync(string resourceName, TimeSpan lea var blob = GetBlob(resourceName); try { - return await TryAcquireAsync(blob, leaseTime, cancellationToken); + return await TryAcquireAsync(blob.GetBlobLeaseClient(), leaseTime, cancellationToken); } - catch (StorageException ex) when (ex.RequestInformation?.HttpStatusCode == (int)HttpStatusCode.NotFound) + catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.NotFound) { // The lease file does not exist. Try to create it and lease it. return await TryCreateAndAcquireAsync(blob, leaseTime, cancellationToken); } } - private CloudBlockBlob GetBlob(string resourceName) + private BlockBlobClient GetBlob(string resourceName) { - var container = _cloudBlobClient.GetContainerReference(_containerName); - return container.GetBlockBlobReference($"{_basePath}{resourceName}"); + var container = _cloudBlobClient.GetBlobContainerClient(_containerName); + return container.GetBlockBlobClient($"{_basePath}{resourceName}"); } public async Task ReleaseAsync(string resourceName, string leaseId, CancellationToken cancellationToken) { var blob = GetBlob(resourceName); + var blobLeaseClient = blob.GetBlobLeaseClient(leaseId); + try { - await blob.ReleaseLeaseAsync( - AccessCondition.GenerateLeaseCondition(leaseId), - options: null, - operationContext: null, - cancellationToken); + await blobLeaseClient.ReleaseAsync(cancellationToken: cancellationToken); return true; } - catch (StorageException ex) when (ex.RequestInformation?.HttpStatusCode == (int)HttpStatusCode.Conflict) + catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.Conflict) { return false; } @@ -79,48 +79,41 @@ public async Task RenewAsync(string resourceName, string leaseId, C } var blob = GetBlob(resourceName); + var blobLeaseClient = blob.GetBlobLeaseClient(leaseId); try { - await blob.RenewLeaseAsync( - AccessCondition.GenerateLeaseCondition(leaseId), - options: null, - operationContext: null, - cancellationToken); + await blobLeaseClient.RenewAsync(cancellationToken: cancellationToken); return LeaseResult.Success(leaseId); } - catch (StorageException ex) when (ex.RequestInformation?.HttpStatusCode == (int)HttpStatusCode.Conflict) + catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.Conflict) { return LeaseResult.Failure(); } } - private async Task TryCreateAndAcquireAsync(CloudBlockBlob blob, TimeSpan leaseTime, CancellationToken cancellationToken) + private async Task TryCreateAndAcquireAsync(BlockBlobClient blob, TimeSpan leaseTime, CancellationToken cancellationToken) { try { // Use an empty blob for the lease blob. The contents are not important. Only the lease state (managed // by Azure Blob Storage) is important. - await blob.UploadFromByteArrayAsync( - Array.Empty(), - index: 0, - count: 0, - accessCondition: null, - options: null, - operationContext: null, - cancellationToken: cancellationToken); + using (var stream = new MemoryStream()) + { + await blob.UploadAsync(stream); + } - return await TryAcquireAsync(blob, leaseTime, cancellationToken); + return await TryAcquireAsync(blob.GetBlobLeaseClient(), leaseTime, cancellationToken); } - catch (StorageException ex) when (ex.RequestInformation?.HttpStatusCode == (int)HttpStatusCode.PreconditionFailed) + catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.PreconditionFailed) { // The file has already created and leased by someone else. return LeaseResult.Failure(); } } - private async Task TryAcquireAsync(CloudBlockBlob blob, TimeSpan leaseTime, CancellationToken cancellationToken) + private async Task TryAcquireAsync(BlobLeaseClient blobLeaseClient, TimeSpan leaseTime, CancellationToken cancellationToken) { if (leaseTime < MinLeaseTime || leaseTime > MaxLeaseTime) { @@ -131,17 +124,13 @@ private async Task TryAcquireAsync(CloudBlockBlob blob, TimeSpan le try { - var leaseId = await blob.AcquireLeaseAsync( - leaseTime: leaseTime, - proposedLeaseId: null, - accessCondition: null, - options: null, - operationContext: null, + var leaseId = await blobLeaseClient.AcquireAsync( + duration: leaseTime, cancellationToken: cancellationToken); - return LeaseResult.Success(leaseId); + return LeaseResult.Success(leaseId.Value.LeaseId); } - catch (StorageException ex) when (ex.RequestInformation?.HttpStatusCode == (int)HttpStatusCode.Conflict) + catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.Conflict) { // The lease has already been acquired by someone else. return LeaseResult.Failure(); diff --git a/src/Validation.Common.Job/Validation.Common.Job.csproj b/src/Validation.Common.Job/Validation.Common.Job.csproj index 57e5bf7584..1e6d10f069 100644 --- a/src/Validation.Common.Job/Validation.Common.Job.csproj +++ b/src/Validation.Common.Job/Validation.Common.Job.csproj @@ -27,7 +27,6 @@ - diff --git a/tests/Validation.Common.Job.Tests/Leases/CloudBlobLeaseServiceIntegrationTests.cs b/tests/Validation.Common.Job.Tests/Leases/CloudBlobLeaseServiceIntegrationTests.cs index 31abc4b26a..0e746b9be0 100644 --- a/tests/Validation.Common.Job.Tests/Leases/CloudBlobLeaseServiceIntegrationTests.cs +++ b/tests/Validation.Common.Job.Tests/Leases/CloudBlobLeaseServiceIntegrationTests.cs @@ -4,8 +4,7 @@ using System; using System.Threading; using System.Threading.Tasks; -using Microsoft.WindowsAzure.Storage; -using Microsoft.WindowsAzure.Storage.Blob; +using Azure.Storage.Blobs; using NuGet.Jobs.Validation.Leases; using Xunit; using Xunit.Abstractions; @@ -20,8 +19,7 @@ public CloudBlobLeaseServiceIntegrationTests(BlobStorageFixture fixture, ITestOu Fixture = fixture ?? throw new ArgumentNullException(nameof(fixture)); Output = output ?? throw new ArgumentNullException(nameof(output)); - CloudStorageAccount = CloudStorageAccount.Parse(Fixture.ConnectionString); - CloudBlobClient = CloudStorageAccount.CreateCloudBlobClient(); + CloudBlobClient = new BlobServiceClient(Fixture.ConnectionString); LeaseTime = TimeSpan.FromSeconds(60); Token = CancellationToken.None; @@ -30,8 +28,7 @@ public CloudBlobLeaseServiceIntegrationTests(BlobStorageFixture fixture, ITestOu public BlobStorageFixture Fixture { get; } public ITestOutputHelper Output { get; } - public CloudStorageAccount CloudStorageAccount { get; } - public CloudBlobClient CloudBlobClient { get; } + public BlobServiceClient CloudBlobClient { get; } public TimeSpan LeaseTime { get; } public CancellationToken Token { get; } public CloudBlobLeaseService Target { get; } From 6c090d6e84ee9b378057cbcc8ad2a73c91935872 Mon Sep 17 00:00:00 2001 From: Andrei Grigorev Date: Thu, 11 Jul 2024 16:43:38 -0700 Subject: [PATCH 3/6] Question mark --- src/NuGet.Services.Validation.Orchestrator/Job.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Services.Validation.Orchestrator/Job.cs b/src/NuGet.Services.Validation.Orchestrator/Job.cs index e343de91db..9b1d538549 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Job.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Job.cs @@ -387,7 +387,7 @@ private static void ConfigureLeaseService(ContainerBuilder builder) .Register(c => { var config = c.Resolve>().Value; - var blobClient = new BlobServiceClient(config.ConnectionString); + var blobClient = new BlobServiceClient(config.ConnectionString.Replace("SharedAccessSignature=?", "SharedAccessSignature=")); return new CloudBlobLeaseService(blobClient, config.ContainerName, config.StoragePath); }) .As(); From dd9f06d2e6e9b924b3558a596eea139279d249bb Mon Sep 17 00:00:00 2001 From: Andrei Grigorev Date: Fri, 12 Jul 2024 18:44:09 -0700 Subject: [PATCH 4/6] Common storage creation methods. --- .../DependencyInjectionExtensions.cs | 9 +--- src/NuGet.Jobs.Common/JsonConfigurationJob.cs | 8 +--- .../StorageAccountExtensions.cs | 44 +++++++++++++++++++ src/NuGet.Jobs.GitHubIndexer/Job.cs | 7 ++- .../DependencyInjectionExtensions.cs | 19 +++----- .../Job.cs | 19 +++----- .../ValidationJobBase.cs | 11 ++--- src/Validation.Symbols/Job.cs | 11 +++-- 8 files changed, 69 insertions(+), 59 deletions(-) create mode 100644 src/NuGet.Jobs.Common/StorageAccountExtensions.cs diff --git a/src/NuGet.Jobs.Catalog2Registration/DependencyInjectionExtensions.cs b/src/NuGet.Jobs.Catalog2Registration/DependencyInjectionExtensions.cs index 1ea503da9b..9146f4145d 100644 --- a/src/NuGet.Jobs.Catalog2Registration/DependencyInjectionExtensions.cs +++ b/src/NuGet.Jobs.Catalog2Registration/DependencyInjectionExtensions.cs @@ -28,13 +28,8 @@ public static ContainerBuilder AddCatalog2Registration(this ContainerBuilder con RegisterCursorStorage(containerBuilder); containerBuilder - .Register(c => - { - var options = c.Resolve>(); - return new CloudBlobClientWrapper( - options.Value.StorageConnectionString, - requestTimeout: DefaultBlobRequestOptions.ServerTimeout); - }); + .RegisterStorageAccount(c => c.StorageConnectionString, requestTimeout: DefaultBlobRequestOptions.ServerTimeout) + .As(); containerBuilder.Register(c => new Catalog2RegistrationCommand( c.Resolve(), diff --git a/src/NuGet.Jobs.Common/JsonConfigurationJob.cs b/src/NuGet.Jobs.Common/JsonConfigurationJob.cs index ecfb296ad0..ee21af1c1e 100644 --- a/src/NuGet.Jobs.Common/JsonConfigurationJob.cs +++ b/src/NuGet.Jobs.Common/JsonConfigurationJob.cs @@ -197,13 +197,7 @@ private void AddScopedSqlConnectionFactory(IServiceCollection public static void ConfigureFeatureFlagAutofacServices(ContainerBuilder containerBuilder) { containerBuilder - .Register(c => - { - var options = c.Resolve>(); - return new CloudBlobClientWrapper( - options.Value.ConnectionString, - requestTimeout: TimeSpan.FromMinutes(2)); - }) + .RegisterStorageAccount(c => c.ConnectionString, requestTimeout: TimeSpan.FromMinutes(2)) .Keyed(FeatureFlagBindingKey); containerBuilder diff --git a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs new file mode 100644 index 0000000000..c6a550e82b --- /dev/null +++ b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs @@ -0,0 +1,44 @@ +// 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 Autofac; +using Autofac.Builder; +using Microsoft.Extensions.Options; +using NuGetGallery; + +namespace NuGet.Jobs +{ + public static class StorageAccountHelper + { + public static CloudBlobClientWrapper CreateCloudBlobClient( + string storageConnectionString, + bool readAccessGeoRedundant = false, + TimeSpan? requestTimeout = null) + { + return new CloudBlobClientWrapper( + storageConnectionString, + readAccessGeoRedundant, + requestTimeout); + } + + public static IRegistrationBuilder RegisterStorageAccount( + this ContainerBuilder builder, + Func getConnectionString, + Func getReadAccessGeoRedundant = null, + TimeSpan? requestTimeout = null) + where TConfiguration : class, new() + { + return builder.Register(c => + { + var options = c.Resolve>(); + string storageConnectionString = getConnectionString(options.Value); + bool readAccessGeoRedundant = getReadAccessGeoRedundant?.Invoke(options.Value) ?? false; + return CreateCloudBlobClient( + storageConnectionString, + readAccessGeoRedundant, + requestTimeout); + }); + } + } +} diff --git a/src/NuGet.Jobs.GitHubIndexer/Job.cs b/src/NuGet.Jobs.GitHubIndexer/Job.cs index efb24c52e3..48f05b42bc 100644 --- a/src/NuGet.Jobs.GitHubIndexer/Job.cs +++ b/src/NuGet.Jobs.GitHubIndexer/Job.cs @@ -42,16 +42,15 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi services.AddTransient(); services.AddTransient(); services.AddTransient(); - services.AddTransient(provider => { - var config = provider.GetRequiredService>(); - return new CloudBlobClientWrapper(config.Value.StorageConnectionString, config.Value.StorageReadAccessGeoRedundant); - }); services.Configure(configurationRoot.GetSection(GitHubIndexerConfigurationSectionName)); } protected override void ConfigureAutofacServices(ContainerBuilder containerBuilder, IConfigurationRoot configurationRoot) { + containerBuilder + .RegisterStorageAccount(c => c.StorageConnectionString, c => c.StorageReadAccessGeoRedundant) + .As(); } } } \ No newline at end of file diff --git a/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs b/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs index 621e87bc2b..473b167d05 100644 --- a/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs +++ b/src/NuGet.Services.AzureSearch/DependencyInjectionExtensions.cs @@ -19,6 +19,7 @@ using Microsoft.Extensions.Options; using Microsoft.Rest; using Microsoft.WindowsAzure.Storage; +using NuGet.Jobs; using NuGet.Protocol; using NuGet.Services.AzureSearch.Auxiliary2AzureSearch; using NuGet.Services.AzureSearch.AuxiliaryFiles; @@ -109,13 +110,7 @@ private static void RegisterIndexServices(ContainerBuilder containerBuilder, str private static void RegisterAzureSearchStorageServices(ContainerBuilder containerBuilder, string key) { containerBuilder - .Register(c => - { - var options = c.Resolve>(); - return new CloudBlobClientWrapper( - options.Value.StorageConnectionString, - requestTimeout: DefaultBlobRequestOptions.ServerTimeout); - }) + .RegisterStorageAccount(c => c.StorageConnectionString, requestTimeout: DefaultBlobRequestOptions.ServerTimeout) .Keyed(key); containerBuilder @@ -217,13 +212,9 @@ private static void RegisterAzureSearchStorageServices(ContainerBuilder containe private static void RegisterAuxiliaryDataStorageServices(ContainerBuilder containerBuilder, string key) { containerBuilder - .Register(c => - { - var options = c.Resolve>(); - return new CloudBlobClientWrapper( - options.Value.AuxiliaryDataStorageConnectionString, - requestTimeout: DefaultBlobRequestOptions.ServerTimeout); - }) + .RegisterStorageAccount( + c => c.AuxiliaryDataStorageConnectionString, + requestTimeout: DefaultBlobRequestOptions.ServerTimeout) .Keyed(key); containerBuilder diff --git a/src/NuGet.Services.Validation.Orchestrator/Job.cs b/src/NuGet.Services.Validation.Orchestrator/Job.cs index 9b1d538549..76e35bc7da 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Job.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Job.cs @@ -180,13 +180,6 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi services.AddTransient, PackageValidationMessageDataSerializationAdapter>(); services.AddTransient, PackageCriteriaEvaluator>(); services.AddTransient(); - services.AddTransient(c => - { - var configurationAccessor = c.GetRequiredService>(); - return new CloudBlobClientWrapper( - configurationAccessor.Value.ValidationStorageConnectionString, - readAccessGeoRedundant: false); - }); services.AddTransient(); services.AddTransient(); services.AddTransient(); @@ -236,6 +229,10 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi protected override void ConfigureAutofacServices(ContainerBuilder containerBuilder, IConfigurationRoot configurationRoot) { + containerBuilder + .RegisterStorageAccount(c => c.ValidationStorageConnectionString) + .As(); + containerBuilder .Register(c => { @@ -452,13 +449,7 @@ private static void ConfigureSymbolScanValidator(ContainerBuilder builder) private static void ConfigureFlatContainer(ContainerBuilder builder) { builder - .Register(c => - { - var configurationAccessor = c.Resolve>(); - return new CloudBlobClientWrapper( - configurationAccessor.Value.ConnectionString, - readAccessGeoRedundant: false); - }) + .RegisterStorageAccount(c => c.ConnectionString) .Keyed(FlatContainerBindingKey); builder diff --git a/src/Validation.Common.Job/ValidationJobBase.cs b/src/Validation.Common.Job/ValidationJobBase.cs index 799c08abff..71bbc29277 100644 --- a/src/Validation.Common.Job/ValidationJobBase.cs +++ b/src/Validation.Common.Job/ValidationJobBase.cs @@ -50,13 +50,6 @@ protected override void ConfigureDefaultJobServices(IServiceCollection services, services.AddTransient(); services.AddTransient(); - services.AddTransient(c => - { - var configurationAccessor = c.GetRequiredService>(); - return new CloudBlobClientWrapper( - configurationAccessor.Value.ConnectionString, - readAccessGeoRedundant: false); - }); services.AddTransient(); services.AddTransient(); @@ -97,6 +90,10 @@ protected override void ConfigureDefaultAutofacServices(ContainerBuilder contain ConfigureFeatureFlagAutofacServices(containerBuilder); + containerBuilder + .RegisterStorageAccount(c => c.ConnectionString) + .As(); + containerBuilder .Register(c => { diff --git a/src/Validation.Symbols/Job.cs b/src/Validation.Symbols/Job.cs index 4a03efa42c..40662000e5 100644 --- a/src/Validation.Symbols/Job.cs +++ b/src/Validation.Symbols/Job.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using NuGet.Jobs; using NuGet.Jobs.Validation; using NuGet.Jobs.Validation.Storage; using NuGet.Jobs.Validation.Symbols.Core; @@ -32,16 +33,14 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi { var configurationAccessor = c.GetRequiredService>(); var packageStorageService = new CloudBlobCoreFileStorageService( - new CloudBlobClientWrapper( - configurationAccessor.Value.PackageConnectionString, - readAccessGeoRedundant: false), + StorageAccountHelper.CreateCloudBlobClient( + configurationAccessor.Value.PackageConnectionString), c.GetRequiredService(), c.GetRequiredService()); var packageValidationStorageService = new CloudBlobCoreFileStorageService( - new CloudBlobClientWrapper( - configurationAccessor.Value.ValidationPackageConnectionString, - readAccessGeoRedundant: false), + StorageAccountHelper.CreateCloudBlobClient( + configurationAccessor.Value.ValidationPackageConnectionString), c.GetRequiredService(), c.GetRequiredService()); From 857e9ee9ef4f58e34feaf6264bd3c1ba9df087db Mon Sep 17 00:00:00 2001 From: Andrei Grigorev Date: Fri, 12 Jul 2024 20:51:12 -0700 Subject: [PATCH 5/6] MSI support. --- src/NuGet.Jobs.Common/JsonConfigurationJob.cs | 1 + .../StorageAccountExtensions.cs | 86 ++++++++++++++++++- .../StorageMsiConfiguration.cs | 11 +++ src/Validation.Symbols/Job.cs | 6 +- 4 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 src/NuGet.Jobs.Common/StorageMsiConfiguration.cs diff --git a/src/NuGet.Jobs.Common/JsonConfigurationJob.cs b/src/NuGet.Jobs.Common/JsonConfigurationJob.cs index ee21af1c1e..0c46143f5b 100644 --- a/src/NuGet.Jobs.Common/JsonConfigurationJob.cs +++ b/src/NuGet.Jobs.Common/JsonConfigurationJob.cs @@ -167,6 +167,7 @@ protected virtual void ConfigureDefaultJobServices(IServiceCollection services, services.Configure(configurationRoot.GetSection(ValidationDbConfigurationSectionName)); services.Configure(configurationRoot.GetSection(ServiceBusConfigurationSectionName)); services.Configure(configurationRoot.GetSection(ValidationStorageConfigurationSectionName)); + services.ConfigureStorageMsi(configurationRoot); services.AddSingleton(new TelemetryClient(ApplicationInsightsConfiguration.TelemetryConfiguration)); services.AddTransient(); diff --git a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs index c6a550e82b..c7d9328fc2 100644 --- a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs +++ b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs @@ -4,6 +4,8 @@ using System; using Autofac; using Autofac.Builder; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using NuGetGallery; @@ -11,12 +13,52 @@ namespace NuGet.Jobs { public static class StorageAccountHelper { + private const string StorageUseManagedIdentityProperyName = "Storage_UseManagedIdentity"; + private const string StorageManagedIdentityClientIdProperyName = "Storage_ManagedIdentityClientId"; + + public static IServiceCollection ConfigureStorageMsi(this IServiceCollection serviceCollection, IConfiguration configuration) + { + if (serviceCollection == null) + { + throw new ArgumentNullException(nameof(serviceCollection)); + } + if (configuration == null) + { + throw new ArgumentNullException(nameof(configuration)); + } + + string useManagedIdentityStr = configuration[StorageUseManagedIdentityProperyName]; + string managedIdentityClientId = configuration[StorageManagedIdentityClientIdProperyName]; + bool useManagedIdentity = false; + if (!string.IsNullOrWhiteSpace(useManagedIdentityStr)) + { + useManagedIdentity = bool.Parse(useManagedIdentityStr); + } + return serviceCollection.Configure(storageConfiguration => + { + storageConfiguration.UseManagedIdentity = useManagedIdentity; + storageConfiguration.ManagedIdentityClientId = managedIdentityClientId; + }); + } + public static CloudBlobClientWrapper CreateCloudBlobClient( + this IServiceProvider serviceProvider, string storageConnectionString, bool readAccessGeoRedundant = false, TimeSpan? requestTimeout = null) { - return new CloudBlobClientWrapper( + if (serviceProvider == null) + { + throw new ArgumentNullException(nameof(serviceProvider)); + } + if (string.IsNullOrWhiteSpace(storageConnectionString)) + { + throw new ArgumentException($"{nameof(storageConnectionString)} cannot be null or empty.", nameof(storageConnectionString)); + } + + var msiConfiguration = serviceProvider.GetRequiredService>().Value; + return CreateCloudBlobClient( + msiConfiguration, storageConnectionString, readAccessGeoRedundant, requestTimeout); @@ -29,16 +71,58 @@ public static IRegistrationBuilder { var options = c.Resolve>(); string storageConnectionString = getConnectionString(options.Value); bool readAccessGeoRedundant = getReadAccessGeoRedundant?.Invoke(options.Value) ?? false; + var msiConfiguration = c.Resolve>().Value; return CreateCloudBlobClient( + msiConfiguration, storageConnectionString, readAccessGeoRedundant, requestTimeout); }); } + + private static CloudBlobClientWrapper CreateCloudBlobClient( + StorageMsiConfiguration msiConfiguration, + string storageConnectionString, + bool readAccessGeoRedundant = false, + TimeSpan? requestTimeout = null) + { + if (msiConfiguration.UseManagedIdentity) + { + if (string.IsNullOrWhiteSpace(msiConfiguration.ManagedIdentityClientId)) + { + return CloudBlobClientWrapper.UsingDefaultAzureCredential( + storageConnectionString, + readAccessGeoRedundant: readAccessGeoRedundant, + requestTimeout: requestTimeout); + } + else + { + return CloudBlobClientWrapper.UsingMsi( + storageConnectionString, + msiConfiguration.ManagedIdentityClientId, + readAccessGeoRedundant, + requestTimeout); + } + } + + return new CloudBlobClientWrapper( + storageConnectionString, + readAccessGeoRedundant, + requestTimeout); + } } } diff --git a/src/NuGet.Jobs.Common/StorageMsiConfiguration.cs b/src/NuGet.Jobs.Common/StorageMsiConfiguration.cs new file mode 100644 index 0000000000..edf36826e1 --- /dev/null +++ b/src/NuGet.Jobs.Common/StorageMsiConfiguration.cs @@ -0,0 +1,11 @@ +// 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.Jobs +{ + public class StorageMsiConfiguration + { + public bool UseManagedIdentity { get; set; } + public string ManagedIdentityClientId { get; set; } + } +} diff --git a/src/Validation.Symbols/Job.cs b/src/Validation.Symbols/Job.cs index 40662000e5..ae9cc6976f 100644 --- a/src/Validation.Symbols/Job.cs +++ b/src/Validation.Symbols/Job.cs @@ -33,14 +33,12 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi { var configurationAccessor = c.GetRequiredService>(); var packageStorageService = new CloudBlobCoreFileStorageService( - StorageAccountHelper.CreateCloudBlobClient( - configurationAccessor.Value.PackageConnectionString), + c.CreateCloudBlobClient(configurationAccessor.Value.PackageConnectionString), c.GetRequiredService(), c.GetRequiredService()); var packageValidationStorageService = new CloudBlobCoreFileStorageService( - StorageAccountHelper.CreateCloudBlobClient( - configurationAccessor.Value.ValidationPackageConnectionString), + c.CreateCloudBlobClient(configurationAccessor.Value.ValidationPackageConnectionString), c.GetRequiredService(), c.GetRequiredService()); From b41c73da7f202f736f1e6e2de84e4db4c7f559d5 Mon Sep 17 00:00:00 2001 From: Andrei Grigorev Date: Mon, 15 Jul 2024 14:16:28 -0700 Subject: [PATCH 6/6] Typo fix --- src/NuGet.Jobs.Common/StorageAccountExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs index c7d9328fc2..624c4694af 100644 --- a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs +++ b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs @@ -13,8 +13,8 @@ namespace NuGet.Jobs { public static class StorageAccountHelper { - private const string StorageUseManagedIdentityProperyName = "Storage_UseManagedIdentity"; - private const string StorageManagedIdentityClientIdProperyName = "Storage_ManagedIdentityClientId"; + private const string StorageUseManagedIdentityPropertyName = "Storage_UseManagedIdentity"; + private const string StorageManagedIdentityClientIdPropertyName = "Storage_ManagedIdentityClientId"; public static IServiceCollection ConfigureStorageMsi(this IServiceCollection serviceCollection, IConfiguration configuration) { @@ -27,8 +27,8 @@ public static IServiceCollection ConfigureStorageMsi(this IServiceCollection ser throw new ArgumentNullException(nameof(configuration)); } - string useManagedIdentityStr = configuration[StorageUseManagedIdentityProperyName]; - string managedIdentityClientId = configuration[StorageManagedIdentityClientIdProperyName]; + string useManagedIdentityStr = configuration[StorageUseManagedIdentityPropertyName]; + string managedIdentityClientId = configuration[StorageManagedIdentityClientIdPropertyName]; bool useManagedIdentity = false; if (!string.IsNullOrWhiteSpace(useManagedIdentityStr)) {