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

[SDKMigration]Attempt to fix missing authentication. #10245

Merged
merged 16 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Catalog/Dnx/DnxCatalogCollector.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand Down Expand Up @@ -193,7 +193,7 @@ await catalogEntries.ForEachAsync(_maxConcurrentCommitItemsWithinBatch, async ca
return processedCatalogEntries;
}

private async Task<bool> AreRequiredPropertiesPresentAsync(Storage destinationStorage, Uri destinationUri)
private async Task<bool> AreRequiredPropertiesPresentAsync(Persistence.Storage destinationStorage, Uri destinationUri)
{
var azureStorage = destinationStorage as IAzureStorage;

Expand Down Expand Up @@ -561,4 +561,4 @@ internal static CatalogEntry Create(CatalogCommitItem item)
}
}
}
}
}
29 changes: 15 additions & 14 deletions src/Catalog/Dnx/DnxMaker.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand All @@ -16,6 +16,7 @@
using NuGet.Versioning;

using ILogger = Microsoft.Extensions.Logging.ILogger;
using CatalogStorage = NuGet.Services.Metadata.Catalog.Persistence.Storage;

namespace NuGet.Services.Metadata.Catalog.Dnx
{
Expand Down Expand Up @@ -154,7 +155,7 @@ public async Task DeletePackageAsync(string id, string version, CancellationToke
await DeleteNupkgAsync(storage, id, normalizedVersion, cancellationToken);
}

public async Task<bool> HasPackageInIndexAsync(Storage storage, string id, string version, CancellationToken cancellationToken)
public async Task<bool> HasPackageInIndexAsync(CatalogStorage storage, string id, string version, CancellationToken cancellationToken)
{
if (storage == null)
{
Expand All @@ -179,7 +180,7 @@ public async Task<bool> HasPackageInIndexAsync(Storage storage, string id, strin
return versionsContext.Versions.Contains(parsedVersion);
}

private async Task<Uri> SaveNuspecAsync(Storage storage, string id, string version, string nuspec, CancellationToken cancellationToken)
private async Task<Uri> SaveNuspecAsync(CatalogStorage storage, string id, string version, string nuspec, CancellationToken cancellationToken)
{
var relativeAddress = GetRelativeAddressNuspec(id, version);
var nuspecUri = new Uri(storage.BaseAddress, relativeAddress);
Expand Down Expand Up @@ -230,7 +231,7 @@ public async Task UpdatePackageVersionIndexAsync(string id, Action<HashSet<NuGet
}
}

private async Task<VersionsResult> GetVersionsAsync(Storage storage, CancellationToken cancellationToken)
private async Task<VersionsResult> GetVersionsAsync(CatalogStorage storage, CancellationToken cancellationToken)
{
var relativeAddress = "index.json";
var resourceUri = new Uri(storage.BaseAddress, relativeAddress);
Expand Down Expand Up @@ -265,7 +266,7 @@ private StorageContent CreateContent(IEnumerable<string> versions)
return new StringStorageContent(obj.ToString(), "application/json", Constants.NoStoreCacheControl);
}

private async Task<Uri> SaveNupkgAsync(Stream nupkgStream, Storage storage, string id, string version, CancellationToken cancellationToken)
private async Task<Uri> SaveNupkgAsync(Stream nupkgStream, CatalogStorage storage, string id, string version, CancellationToken cancellationToken)
{
Uri nupkgUri = new Uri(storage.BaseAddress, GetRelativeAddressNupkg(id, version));
var content = new StreamStorageContent(
Expand All @@ -280,7 +281,7 @@ private async Task<Uri> SaveNupkgAsync(Stream nupkgStream, Storage storage, stri

private async Task<Uri> CopyNupkgAsync(
IStorage sourceStorage,
Storage destinationStorage,
CatalogStorage destinationStorage,
string id, string version, CancellationToken cancellationToken)
{
var packageFileName = PackageUtility.GetPackageFileName(id, version);
Expand All @@ -300,7 +301,7 @@ await sourceStorage.CopyAsync(

private async Task CopyIconFromAzureStorageIfExistAsync(
IAzureStorage sourceStorage,
Storage destinationStorage,
CatalogStorage destinationStorage,
string packageId,
string normalizedPackageVersion,
string iconFilename,
Expand All @@ -321,7 +322,7 @@ await CopyIconAsync(
private async Task CopyIconFromNupkgStreamAsync(
Stream nupkgStream,
string iconFilename,
Storage destinationStorage,
CatalogStorage destinationStorage,
string packageId,
string normalizedPackageVersion,
CancellationToken cancellationToken)
Expand All @@ -338,7 +339,7 @@ await CopyIconAsync(
private async Task CopyIconAsync(
Stream packageStream,
string iconFilename,
Storage destinationStorage,
CatalogStorage destinationStorage,
string packageId,
string normalizedPackageVersion,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -366,7 +367,7 @@ await ExtractAndStoreIconAsync(
private async Task ExtractAndStoreIconAsync(
Stream packageStream,
string iconPath,
Storage destinationStorage,
CatalogStorage destinationStorage,
Uri destinationUri,
CancellationToken cancellationToken,
string packageId,
Expand Down Expand Up @@ -406,7 +407,7 @@ private async Task<Stream> GetPackageStreamAsync(
return await packageSourceBlob.GetStreamAsync(cancellationToken);
}

private async Task DeleteNuspecAsync(Storage storage, string id, string version, CancellationToken cancellationToken)
private async Task DeleteNuspecAsync(CatalogStorage storage, string id, string version, CancellationToken cancellationToken)
{
string relativeAddress = GetRelativeAddressNuspec(id, version);
Uri nuspecUri = new Uri(storage.BaseAddress, relativeAddress);
Expand All @@ -416,7 +417,7 @@ private async Task DeleteNuspecAsync(Storage storage, string id, string version,
}
}

private async Task DeleteNupkgAsync(Storage storage, string id, string version, CancellationToken cancellationToken)
private async Task DeleteNupkgAsync(CatalogStorage storage, string id, string version, CancellationToken cancellationToken)
{
string relativeAddress = GetRelativeAddressNupkg(id, version);
Uri nupkgUri = new Uri(storage.BaseAddress, relativeAddress);
Expand All @@ -426,7 +427,7 @@ private async Task DeleteNupkgAsync(Storage storage, string id, string version,
}
}

private async Task DeleteIconAsync(Storage storage, string id, string version, CancellationToken cancellationToken)
private async Task DeleteIconAsync(CatalogStorage storage, string id, string version, CancellationToken cancellationToken)
{
string relativeAddress = GetRelativeAddressIcon(id, version);
Uri iconUri = new Uri(storage.BaseAddress, relativeAddress);
Expand Down Expand Up @@ -479,4 +480,4 @@ public VersionsResult(string relativeAddress, Uri resourceUri, HashSet<NuGetVers
public HashSet<NuGetVersion> Versions { get; }
}
}
}
}
8 changes: 4 additions & 4 deletions src/Catalog/DurableCursor.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand All @@ -12,10 +12,10 @@ namespace NuGet.Services.Metadata.Catalog
public class DurableCursor : ReadWriteCursor
{
Uri _address;
Storage _storage;
Persistence.Storage _storage;
DateTime _defaultValue;

public DurableCursor(Uri address, Storage storage, DateTime defaultValue)
public DurableCursor(Uri address, Persistence.Storage storage, DateTime defaultValue)
{
_address = address;
_storage = storage;
Expand Down Expand Up @@ -43,4 +43,4 @@ public override async Task LoadAsync(CancellationToken cancellationToken)
Value = obj["value"].ToObject<DateTime>();
}
}
}
}
6 changes: 3 additions & 3 deletions src/Catalog/Helpers/DeletionAuditEntry.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand Down Expand Up @@ -163,7 +163,7 @@ public static Task<IEnumerable<DeletionAuditEntry>> GetAsync(
DateTime? maxTime = null,
ILogger logger = null)
{
Storage storage = auditingStorageFactory.Create(package != null ? GetAuditRecordPrefixFromPackage(package) : null);
Persistence.Storage storage = auditingStorageFactory.Create(package != null ? GetAuditRecordPrefixFromPackage(package) : null);
return GetAsync(storage, cancellationToken, minTime, maxTime, logger);
}

Expand Down Expand Up @@ -258,4 +258,4 @@ private static bool IsPackageDelete(StorageListItem auditRecord)
return FileNameSuffixes.Any(suffix => fileName.EndsWith(suffix));
}
}
}
}
6 changes: 4 additions & 2 deletions src/Catalog/Icons/CatalogLeafDataProcessor.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand All @@ -12,6 +12,8 @@
using NuGet.Services.Metadata.Catalog.Helpers;
using NuGet.Services.Metadata.Catalog.Persistence;

using CatalogStorage = NuGet.Services.Metadata.Catalog.Persistence.Storage;

namespace NuGet.Services.Metadata.Catalog.Icons
{
public class CatalogLeafDataProcessor : ICatalogLeafDataProcessor
Expand Down Expand Up @@ -42,7 +44,7 @@ public CatalogLeafDataProcessor(
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
}

public async Task ProcessPackageDeleteLeafAsync(Storage storage, CatalogCommitItem item, CancellationToken cancellationToken)
public async Task ProcessPackageDeleteLeafAsync(CatalogStorage storage, CatalogCommitItem item, CancellationToken cancellationToken)
{
var targetStoragePath = GetTargetStorageIconPath(item);
await _iconProcessor.DeleteIconAsync(storage, targetStoragePath, cancellationToken, item.PackageIdentity.Id, item.PackageIdentity.Version.ToNormalizedString());
Expand Down
6 changes: 3 additions & 3 deletions src/Catalog/Icons/ICatalogLeafDataProcessor.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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.Threading;
Expand All @@ -9,7 +9,7 @@ namespace NuGet.Services.Metadata.Catalog.Icons
{
public interface ICatalogLeafDataProcessor
{
Task ProcessPackageDeleteLeafAsync(Storage storage, CatalogCommitItem item, CancellationToken cancellationToken);
Task ProcessPackageDeleteLeafAsync(Persistence.Storage storage, CatalogCommitItem item, CancellationToken cancellationToken);
Task ProcessPackageDetailsLeafAsync(IStorage destinationStorage, IStorage iconCacheStorage, CatalogCommitItem item, string iconUrlString, string iconFile, CancellationToken cancellationToken);
}
}
}
1 change: 1 addition & 0 deletions src/Catalog/NuGet.Services.Metadata.Catalog.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
<ProjectReference Include="..\NuGet.Services.Configuration\NuGet.Services.Configuration.csproj" />
<ProjectReference Include="..\NuGet.Services.Logging\NuGet.Services.Logging.csproj" />
<ProjectReference Include="..\NuGet.Services.Sql\NuGet.Services.Sql.csproj" />
<ProjectReference Include="..\NuGet.Services.Storage\NuGet.Services.Storage.csproj" />
<ProjectReference Include="..\NuGetGallery.Core\NuGetGallery.Core.csproj" />
</ItemGroup>

Expand Down
6 changes: 4 additions & 2 deletions src/Catalog/Persistence/AzureStorage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
using System.Threading;
using System.Threading.Tasks;
using Azure;
using Azure.Identity;
using Azure.Storage.Blobs;
using Azure.Storage.Blobs.Models;
using Azure.Storage.Blobs.Specialized;
using NuGet.Protocol;
using NuGet.Services.Metadata.Catalog.Extensions;
using NuGet.Services.Storage;
using NuGetGallery;

namespace NuGet.Services.Metadata.Catalog.Persistence
Expand All @@ -33,7 +35,7 @@ public class AzureStorage : Storage, IAzureStorage
public static readonly TimeSpan DefaultMaxExecutionTime = TimeSpan.FromMinutes(10);

public AzureStorage(
BlobServiceClient blobServiceClient,
IBlobServiceClientFactory blobServiceClient,
string containerName,
string path,
Uri baseAddress,
Expand Down Expand Up @@ -92,7 +94,7 @@ private static ICloudBlobDirectory GetCloudBlobDirectoryUri(Uri storageBaseUri)

var blobEndpoint = new Uri(storageBaseUri.GetComponents(UriComponents.SchemeAndServer, UriFormat.Unescaped));
// Create BlobServiceClient with anonymous credentials
var blobServiceClient = new BlobServiceClient(blobEndpoint);
var blobServiceClient = new BlobServiceClientFactory(blobEndpoint, new DefaultAzureCredential());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a usage of this method that requires the MSI client id? or did you just add the DefaultAzureCredential for testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as we (@advay26 and I) could tell, this particular code path is only called in icons.
The original code doesn't pass any authentication information at all, so at this point, we are unsure exactly what it does/how it worked/works.
DefaultAzureCredential was put here as from what I could see, it is likely the "closest" alternative.
For whomever migrates icons later, assuming that this doesn't behave as expected, may need to enhance BlobServiceClientFactory to be able to return a no auth client.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually migrating icons, so I can verify when I start my tests for that job.

Copy link
Member

Choose a reason for hiding this comment

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

From what I understand, DefaultAzureCredential picks up the MSI that is assigned to the VM, so it should hopefully just work for Icons too. I'm not sure what happens if we have a VM with multiple MSIs assigned to it


string containerName = pathSegments[0];
string pathInContainer = string.Join("/", pathSegments.Skip(1));
Expand Down
9 changes: 5 additions & 4 deletions src/Catalog/Persistence/AzureStorageFactory.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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 Azure.Storage.Blobs;
using NuGet.Protocol;
using NuGet.Services.Storage;

namespace NuGet.Services.Metadata.Catalog.Persistence
{
public class AzureStorageFactory : StorageFactory
{
private readonly BlobServiceClient _blobServiceClient;
private readonly IBlobServiceClientFactory _blobServiceClient;
private readonly string _containerName;
private readonly string _path;
private readonly Uri _differentBaseAddress = null;
Expand All @@ -19,7 +20,7 @@ public class AzureStorageFactory : StorageFactory
private readonly bool _initializeContainer;

public AzureStorageFactory(
BlobServiceClient blobServiceClient,
IBlobServiceClientFactory blobServiceClient,
string containerName,
TimeSpan maxExecutionTime,
TimeSpan serverTimeout,
Expand Down Expand Up @@ -107,4 +108,4 @@ public override Storage Create(string name = null)
Throttle);
}
}
}
}
25 changes: 10 additions & 15 deletions src/Catalog/Persistence/CloudBlobDirectoryWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,32 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core;
using Azure.Core.Pipeline;
using Azure.Storage.Blobs;
using Azure.Storage.Blobs.Models;
using Azure.Storage.Blobs.Specialized;
using NuGet.Services.Storage;

namespace NuGet.Services.Metadata.Catalog.Persistence
{
public class CloudBlobDirectoryWrapper : ICloudBlobDirectory
{
private readonly IBlobServiceClientFactory _blobServiceClientFactory;
private readonly BlobContainerClient _containerClient;
private readonly string _directoryPrefix;
private readonly IBlobContainerClientWrapper _blobContainerClientWrapper;
private readonly BlobClientOptions _defaultClientOptions;

public BlobServiceClient ServiceClient => _containerClient.GetParentBlobServiceClient();
public IBlobServiceClientFactory ServiceClient => new SimpleBlobServiceClientFactory(_containerClient.GetParentBlobServiceClient());
public Uri Uri { get; }
public string DirectoryPrefix => _directoryPrefix;
public BlobClientOptions ContainerOptions => _defaultClientOptions;
public IBlobContainerClientWrapper ContainerClientWrapper => _blobContainerClientWrapper;

public CloudBlobDirectoryWrapper(BlobServiceClient serviceClient, string containerName, string directoryPrefix, BlobClientOptions blobClientOptions = null)
public CloudBlobDirectoryWrapper(IBlobServiceClientFactory serviceClientFactory, string containerName, string directoryPrefix, BlobClientOptions blobClientOptions = null)
{
_blobServiceClientFactory = serviceClientFactory ?? throw new ArgumentNullException(nameof(serviceClientFactory));
_directoryPrefix = directoryPrefix ?? throw new ArgumentNullException(nameof(directoryPrefix));

if (string.IsNullOrWhiteSpace(containerName))
Expand All @@ -35,19 +40,9 @@ public CloudBlobDirectoryWrapper(BlobServiceClient serviceClient, string contain

_defaultClientOptions = blobClientOptions ?? new BlobClientOptions();

// Create the container client using the provided or default options
if (blobClientOptions != null)
{
// Extract necessary information
Uri serviceUri = serviceClient.Uri;
// Create a new BlobServiceClient instance with the new options
var newServiceClient = new BlobServiceClient(serviceUri, _defaultClientOptions);
_containerClient = newServiceClient.GetBlobContainerClient(containerName);
}
else
{
_containerClient = serviceClient.GetBlobContainerClient(containerName);
}
// Request a new BlobServiceClient instance with the current BlobClientOptions
var serviceClient = _blobServiceClientFactory.GetBlobServiceClient(blobClientOptions);
erdembayar marked this conversation as resolved.
Show resolved Hide resolved
_containerClient = serviceClient.GetBlobContainerClient(containerName);

Uri = new Uri(Storage.RemoveQueryString(_containerClient.Uri).TrimEnd('/') + "/" + _directoryPrefix);

Expand Down
5 changes: 3 additions & 2 deletions src/Catalog/Persistence/ICloudBlobDirectory.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand All @@ -8,12 +8,13 @@
using Azure.Storage.Blobs;
using Azure.Storage.Blobs.Models;
using Azure.Storage.Blobs.Specialized;
using NuGet.Services.Storage;

namespace NuGet.Services.Metadata.Catalog.Persistence
{
public interface ICloudBlobDirectory
{
BlobServiceClient ServiceClient { get; }
IBlobServiceClientFactory ServiceClient { get; }
IBlobContainerClientWrapper ContainerClientWrapper { get; }
string DirectoryPrefix { get; }
Uri Uri { get; }
Expand Down
Loading