Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing blob storage SDK leakage into the code base #9981

Draft
wants to merge 20 commits into
base: dev
Choose a base branch
from
Draft
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<PropertyGroup>
<NuGetClientPackageVersion>6.9.1</NuGetClientPackageVersion>
<ServerCommonPackageVersion>2.120.0</ServerCommonPackageVersion>
<NuGetJobsPackageVersion>4.3.0-main-9408418</NuGetJobsPackageVersion>
<NuGetJobsPackageVersion>4.3.0-agr-azst-9643015</NuGetJobsPackageVersion>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have to be updated with a separate PR as NuGet.Jobs release depends on this PR.

Copy link
Contributor

@drewgillies drewgillies May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems circular. Should we get away from this dependency? (sometime in the future)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes this circular dependency makes hard to reason with runtime issue caused by dependency injection. Not sure exactly what was the actual dll consumed.

Copy link
Contributor Author

@agr agr May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have jobs in Gallery repo that use base classes from NuGet.Jobs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured it would be something like that--I'll create a task for moving them, although that would also involve nugetizing some additional gallery dependencies to support them, I expect. DI, for one may make this a pipe dream.

</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers">
Expand Down
38 changes: 12 additions & 26 deletions src/NuGetGallery.Core/Auditing/CloudAuditingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
using System.Globalization;
using System.IO;
using System.Threading.Tasks;
using Microsoft.WindowsAzure.Storage;
using Microsoft.WindowsAzure.Storage.Blob;
using Microsoft.WindowsAzure.Storage.Blob.Protocol;
using Newtonsoft.Json;
using NuGetGallery.Auditing.Obfuscation;

Expand Down Expand Up @@ -58,23 +55,16 @@ protected override async Task SaveAuditRecordAsync(string auditData, string reso
{
await WriteBlob(auditData, fullPath, blob);
}
catch (StorageException ex)
catch (CloudBlobContainerNotFoundException)
{
if (ex.RequestInformation?.ExtendedErrorInformation?.ErrorCode == BlobErrorCodeStrings.ContainerNotFound)
{
retry = true;
}
else
{
throw;
}
retry = true;
}

if (retry)
{
// Create the container and try again,
// this time we let exceptions bubble out
await container.CreateIfNotExistAsync(permissions: null);
await container.CreateIfNotExistAsync(enablePublicAccess: false);
await WriteBlob(auditData, fullPath, blob);
}
}
Expand All @@ -89,29 +79,25 @@ private static async Task WriteBlob(string auditData, string fullPath, ISimpleCl
{
try
{
using (var stream = await blob.OpenWriteAsync(AccessCondition.GenerateIfNoneMatchCondition("*")))
using (var stream = await blob.OpenWriteAsync(AccessConditionWrapper.GenerateIfNoneMatchCondition("*")))
using (var writer = new StreamWriter(stream))
{
await writer.WriteAsync(auditData);
}
}
catch (StorageException ex)
catch (CloudBlobConflictException ex)
{
if (ex.RequestInformation != null && ex.RequestInformation.HttpStatusCode == 409)
{
// Blob already existed!
throw new InvalidOperationException(String.Format(
CultureInfo.CurrentCulture,
CoreStrings.CloudAuditingService_DuplicateAuditRecord,
fullPath), ex);
}
throw;
// Blob already existed!
throw new InvalidOperationException(String.Format(
CultureInfo.CurrentCulture,
CoreStrings.CloudAuditingService_DuplicateAuditRecord,
fullPath), ex.InnerException);
}
}

public Task<bool> IsAvailableAsync(BlobRequestOptions options, OperationContext operationContext)
public Task<bool> IsAvailableAsync(CloudBlobLocationMode? locationMode)
{
return _auditContainerFactory().ExistsAsync(options, operationContext);
return _auditContainerFactory().ExistsAsync(locationMode);
}

public override string RenderAuditEntry(AuditEntry entry)
Expand Down
21 changes: 0 additions & 21 deletions src/NuGetGallery.Core/Extensions/StorageExceptionExtensions.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.WindowsAzure.Storage;
using Newtonsoft.Json;
using NuGet.Services.Entities;
using NuGet.Services.FeatureFlags;
Expand Down Expand Up @@ -117,7 +116,7 @@ private async Task<ContentSaveResult> TrySaveInternalAsync(FeatureFlags flags, s
return ContentSaveResult.Ok;
}
}
catch (StorageException e) when (e.IsPreconditionFailedException())
catch (CloudBlobPreconditionFailedException)
{
return ContentSaveResult.Conflict;
}
Expand Down
4 changes: 1 addition & 3 deletions src/NuGetGallery.Core/ICloudStorageStatusDependency.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Threading.Tasks;
using Microsoft.WindowsAzure.Storage;
using Microsoft.WindowsAzure.Storage.Blob;

namespace NuGetGallery
{
Expand All @@ -13,6 +11,6 @@ namespace NuGetGallery
/// </summary>
public interface ICloudStorageStatusDependency
{
Task<bool> IsAvailableAsync(BlobRequestOptions options, OperationContext operationContext);
Task<bool> IsAvailableAsync(CloudBlobLocationMode? locationMode);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.ComTypes;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.WindowsAzure.Storage;
using Newtonsoft.Json;
using NuGetGallery.Shared;

Expand Down Expand Up @@ -123,7 +121,7 @@ private async Task<ContentSaveResult> TrySaveInternalAsync(LoginDiscontinuation
return ContentSaveResult.Ok;
}
}
catch (StorageException e) when (e.IsPreconditionFailedException())
catch (CloudBlobPreconditionFailedException)
{
return ContentSaveResult.Conflict;
}
Expand Down
16 changes: 16 additions & 0 deletions src/NuGetGallery.Core/Services/BlobListContinuationToken.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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 Microsoft.WindowsAzure.Storage.Blob;

namespace NuGetGallery
{
internal class BlobListContinuationToken : IBlobListContinuationToken
{
public BlobListContinuationToken(BlobContinuationToken continuationToken)
{
ContinuationToken = continuationToken;
}
public BlobContinuationToken ContinuationToken { get; }
}
}
4 changes: 2 additions & 2 deletions src/NuGetGallery.Core/Services/BlobResultSegmentWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public BlobResultSegmentWrapper(BlobResultSegment segment)
// creation of block blobs so it's good enough for now. If another caller created a non-block blob, this
// cast will fail at runtime.
Results = segment.Results.Cast<CloudBlockBlob>().Select(x => new CloudBlobWrapper(x)).ToList();
ContinuationToken = segment.ContinuationToken;
ContinuationToken = new BlobListContinuationToken(segment.ContinuationToken);
}

public IReadOnlyList<ISimpleCloudBlob> Results { get; }
public BlobContinuationToken ContinuationToken { get; }
public IBlobListContinuationToken ContinuationToken { get; }
}
}
15 changes: 15 additions & 0 deletions src/NuGetGallery.Core/Services/CloudBlobConflictException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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;

namespace NuGetGallery
{
public class CloudBlobConflictException : CloudBlobStorageException
{
public CloudBlobConflictException(Exception innerException)
: base(innerException)
{
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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;

namespace NuGetGallery
{
public class CloudBlobContainerNotFoundException : CloudBlobGenericNotFoundException
{
public CloudBlobContainerNotFoundException(Exception innerException)
: base("Container not found", innerException)
{
}
}
}
103 changes: 63 additions & 40 deletions src/NuGetGallery.Core/Services/CloudBlobContainerWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// 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.Threading;
using System.Threading.Tasks;
using Microsoft.WindowsAzure.Storage;
using Microsoft.WindowsAzure.Storage.Blob;

namespace NuGetGallery
Expand All @@ -20,70 +20,93 @@ public CloudBlobContainerWrapper(CloudBlobContainer blobContainer)
public async Task<ISimpleBlobResultSegment> ListBlobsSegmentedAsync(
string prefix,
bool useFlatBlobListing,
BlobListingDetails blobListingDetails,
ListingDetails blobListingDetails,
int? maxResults,
BlobContinuationToken blobContinuationToken,
BlobRequestOptions options,
OperationContext operationContext,
IBlobListContinuationToken blobContinuationToken,
TimeSpan? requestTimeout,
CloudBlobLocationMode? cloudBlobLocationMode,
CancellationToken cancellationToken)
{
var segment = await _blobContainer.ListBlobsSegmentedAsync(
prefix,
useFlatBlobListing,
blobListingDetails,
maxResults,
blobContinuationToken,
options,
operationContext,
cancellationToken);

return new BlobResultSegmentWrapper(segment);
}

public Task CreateIfNotExistAsync(BlobContainerPermissions permissions)
{
var publicAccess = permissions?.PublicAccess;
BlobContinuationToken continuationToken = null;
if (blobContinuationToken != null)
{
if (blobContinuationToken is BlobListContinuationToken token)
{
continuationToken = token.ContinuationToken;
}
else
{
throw new ArgumentException();
}
}

if (publicAccess.HasValue)
BlobRequestOptions options = null;
if (requestTimeout.HasValue || cloudBlobLocationMode.HasValue)
{
return _blobContainer.CreateIfNotExistsAsync(publicAccess.Value, options: null, operationContext: null);
options = new BlobRequestOptions();
if (requestTimeout.HasValue)
{
options.ServerTimeout = requestTimeout.Value;
}
if (cloudBlobLocationMode.HasValue)
{
options.LocationMode = CloudWrapperHelpers.GetSdkRetryPolicy(cloudBlobLocationMode.Value);
}
}

return _blobContainer.CreateIfNotExistsAsync();
}
var segment = await CloudWrapperHelpers.WrapStorageExceptionAsync(() =>
_blobContainer.ListBlobsSegmentedAsync(
prefix,
useFlatBlobListing,
CloudWrapperHelpers.GetSdkBlobListingDetails(blobListingDetails),
maxResults,
continuationToken,
options,
operationContext: null,
cancellationToken: cancellationToken));

public async Task SetPermissionsAsync(BlobContainerPermissions permissions)
return new BlobResultSegmentWrapper(segment);
}

public async Task CreateIfNotExistAsync(bool enablePublicAccess)
{
await _blobContainer.SetPermissionsAsync(permissions);
var accessType = enablePublicAccess ? BlobContainerPublicAccessType.Blob : BlobContainerPublicAccessType.Off;

await CloudWrapperHelpers.WrapStorageExceptionAsync(() =>
_blobContainer.CreateIfNotExistsAsync(accessType, options: null, operationContext: null));
}

public ISimpleCloudBlob GetBlobReference(string blobAddressUri)
{
return new CloudBlobWrapper(_blobContainer.GetBlockBlobReference(blobAddressUri));
}

public async Task<bool> ExistsAsync(BlobRequestOptions blobRequestOptions, OperationContext context)
public async Task<bool> ExistsAsync(CloudBlobLocationMode? cloudBlobLocationMode)
{
return await _blobContainer.ExistsAsync(blobRequestOptions, context);
BlobRequestOptions options = null;
if (cloudBlobLocationMode.HasValue)
{
options = new BlobRequestOptions
{
LocationMode = CloudWrapperHelpers.GetSdkRetryPolicy(cloudBlobLocationMode.Value),
};
}
return await CloudWrapperHelpers.WrapStorageExceptionAsync(() =>
_blobContainer.ExistsAsync(options, operationContext: null));
}

public async Task<bool> DeleteIfExistsAsync()
{
return await _blobContainer.DeleteIfExistsAsync();
return await CloudWrapperHelpers.WrapStorageExceptionAsync(() =>
_blobContainer.DeleteIfExistsAsync());
}

public async Task CreateAsync(BlobContainerPermissions permissions)
public async Task CreateAsync(bool enablePublicAccess)
{
var publicAccess = permissions?.PublicAccess;
var accessType = enablePublicAccess ? BlobContainerPublicAccessType.Blob : BlobContainerPublicAccessType.Off;

if (publicAccess.HasValue)
{
await _blobContainer.CreateAsync(publicAccess.Value, options: null, operationContext: null);
}
else
{
await _blobContainer.CreateAsync();
}
await CloudWrapperHelpers.WrapStorageExceptionAsync(() =>
_blobContainer.CreateAsync(accessType, options: null, operationContext: null));
}
}
}
21 changes: 21 additions & 0 deletions src/NuGetGallery.Core/Services/CloudBlobCopyState.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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 Microsoft.WindowsAzure.Storage.Blob;

namespace NuGetGallery
{
internal class CloudBlobCopyState : ICloudBlobCopyState
{
private readonly CloudBlockBlob _blob;

public CloudBlobCopyState(CloudBlockBlob blob)
{
_blob = blob ?? throw new ArgumentNullException(nameof(blob));
}
public CloudBlobCopyStatus Status => CloudWrapperHelpers.GetBlobCopyStatus(_blob.CopyState.Status);

public string StatusDescription => _blob.CopyState.StatusDescription;
}
}
Loading