Warn when http sources are used in push/delete operations #4552
Conversation
src/NuGet.Core/NuGet.Protocol/Resources/PackageUpdateResource.cs
Outdated
Show resolved
Hide resolved
| if (packageSource.IsHttp && !packageSource.IsHttps && | ||
| (packageSource.ProtocolVersion == 3 || packageSource.Source.EndsWith("json", StringComparison.OrdinalIgnoreCase))) | ||
| { | ||
| logger.LogWarning(string.Format(CultureInfo.CurrentCulture, Strings.Warning_HttpServerUsage, "push", packageSource.Source)); |
There was a problem hiding this comment.
Should we raise the warnings for HTTP symbol servers also?
There was a problem hiding this comment.
We are. The check is done inside.
The reason why we're special casing V3 here is because V3 is the only source that redirects. For the other ones you have to pass the exact target url.
PushCommand_WhenPushingToAnHttpServerWithSymbols_Warns tests that.
| @@ -67,7 +67,15 @@ public static string GetApiKey(ISettings settings, string endpoint, string sourc | |||
| return apiKey ?? defaultApiKey; | |||
| } | |||
|
|
|||
| public static async Task<PackageUpdateResource> GetPackageUpdateResource(IPackageSourceProvider sourceProvider, string source) | |||
| public static async Task<PackageUpdateResource> GetPackageUpdateResource(IPackageSourceProvider sourceProvider, PackageSource packageSource) | |||
There was a problem hiding this comment.
Why is this change made? Why need to separate it?
There was a problem hiding this comment.
If you look at the runner code, I use the effective PackageSource to check for http.
I don't want to create the PackageSource twice.
nkolev92
force-pushed
the
dev-nkolev92-pushWarnings
branch
from
9836827 to
cd070ea
Compare
|
My build was failing due to a flaky test. I created a fix for it #4569. In the meantime, can I get an approval again @NuGet/nuget-client |
Bug
Fixes: https://github.com/NuGet/Client.Engineering/issues/1520
Regression? Last working version:
Description
Enable warnings for all nuget.exe and dotnet.exe push/delete scenarios.
Note that the changes are in shared code for push and delete and not in nuget.exe/dotnet.exe specific parts.
PR Checklist
PR has a meaningful title
PR has a linked issue.
Described changes
Tests
Documentation