Fix dnx not authenticating for private feeds#53322
Fix dnx not authenticating for private feeds#53322baronfel merged 3 commits intodotnet:release/10.0.3xxfrom
Conversation
|
@baronfel @marcpopMSFT This is ready for review now |
|
Sorry to ping, @baronfel or @marcpopMSFT are you able to take a look at this? |
|
Is there any test that would make sense to add? |
|
Looks good. @dotnet/dotnet-sdk Do we have any test private feeds we could use to add a test for this? |
|
There were never any tests for the |
@dsplaisted I think we'd need to ask dnceng. They likely have some feeds but creating a specific testcase may be trickier. My recommendation is to take this fix as is and file a separate issue to expand on the testing of tool exec including this scenario as a separate PR. We can probably have copilot generate at least a core of exec tests. |
|
Thanks all. I added a test for the nuget downloader to ensure it selects the correct source, but further testing for the tool execute command is going to require significantly more refactoring that I'm not comfortable doing as part of this change. |
| @@ -114,8 +114,9 @@ public override int Execute() | |||
| // other feeds, but this is probably OK. | |||
| var downloadPackageLocation = new PackageLocation( | |||
There was a problem hiding this comment.
This mechanism ("constructing a packageLocation from a common set of nuget-feed-related input options") feels like something we need to be able to extract and share across many of our nuget-related commands. After this merges we should probably try to unify how we do this to prevent regressions like this.
The thing I'd immediately reach for is a docker container for a NuGet server - there doesn't seem to be one out in the wild, and the one I did see just used API Key access, and I'm not sure if that would be enough to test with. Will ask the NuGet team about how they'd go about this. |
|
Thanks for approving. I'm not sure why the build is failing, if someone can perhaps re-run it for me if required? |
|
We're having some build infra problems across multiple repos at the moment - as soon as that clears up we'll get the pipelines going here again 👍 |
baronfel
force-pushed
the
bug/fix-dnx-with-private-feed
branch
from
3bb3805 to
0e2b75d
Compare
|
With respect to testing, have we done any manual testing to make sure that this fixes the issue? |
|
Yes I've run this manually in my corporate environment where we use authenticated nuget sources and verified that it works. |
|
I don't believe that these test failures are related to this change - they cover publishing which isn't impacted by changes to tool download/invocation. |
|
@dotnet/domestic-cat I think the test errors on this PR are indicative of a known build error that we need to log and figure out - can y'all handle that? |
|
/ba-g known errors around C++/VS build components not being installed on the newly-recreated base images |
|
/backport to release/10.0.1xx |
|
/backport to release/10.0.2xx |
|
Started backporting to |
|
Started backporting to |
5 participants
Fixes #51375
For
dnxexecution (dotnet tool exec), if the package source is authenticated the previous method of passing an override source meant that the credentials are lost, since this is the equivalient of passing--source https://mysourceand ignoring any config inNuGet.config. We need to pass the explicit source fordnxbecause we confirm with the user that we will be downloading from a particular source (which may be authenticated).