Unable to install packages with dependencies from Artifactory PSGallery mirror (NuGet v2)

sean-r-williams commented 2 months ago

Prerequisites

[X] Write a descriptive title.
[X] Make sure you are able to repro it on the latest released version
[X] Search the existing issues.

Steps to reproduce

Configure Artifactory with a remote feed pointing at PSGallery
Connect to said feed with PSResourceGet (using NuGet v2)
Install a package with dependencies (e.g. Microsoft.Graph.Teams, which depends on Microsoft.Graph.Authentication)

Expected behavior

PS> Install-PSResource Microsoft.Graph.Teams -Repository Artifactory
PS> <# module installs without issue #>

Actual behavior

Install-PSResource: Package(s) 'microsoft.graph.teams' could not be installed from repository 'Artifactory-Remote'.

Error details

PS> Get-Error

Exception             :
    Type    : Microsoft.PowerShell.PSResourceGet.UtilClasses.ResourceNotFoundException
    Message : Package(s) 'microsoft.graph.teams' could not be installed from repository 'Artifactory-Remote'.
    HResult : -2146233088
TargetObject          : Microsoft.PowerShell.PSResourceGet.Cmdlets.InstallPSResource
CategoryInfo          : InvalidData: (Microsoft.PowerShel…s.InstallPSResource:InstallPSResource) [Install-PSResource], ResourceNotFoundException
FullyQualifiedErrorId : InstallPackageFailure,Microsoft.PowerShell.PSResourceGet.Cmdlets.InstallPSResource
InvocationInfo        :
    MyCommand        : Install-PSResource
    ScriptLineNumber : 1
    OffsetInLine     : 1
    HistoryId        : 4
    Line             : install-psresource microsoft.graph.teams -Repository Artifactory-Remote -debug -Reinstall
    Statement        : install-psresource microsoft.graph.teams -Repository Artifactory-Remote -debug -Reinstall
    PositionMessage  : At line:1 char:1
                       + install-psresource microsoft.graph.teams -Repository Artifactory-Remo …
                       + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    InvocationName   : install-psresource
    CommandOrigin    : Internal
ScriptStackTrace      : at <ScriptBlock>, <No file>: line 1
PipelineIterationInfo :

Environment data

PS> Get-Module Microsoft.PowerShell.PSResourceGet; $PSVersionTable | ft

ModuleType Version    PreRelease Name                                ExportedCommands
---------- -------    ---------- ----                                ----------------
Binary     1.1.0.1               Microsoft.PowerShell.PSResourceGet  {Find-PSResource, Get-InstalledPSResource, Get-PSResourceRepository, Get-PSScriptFileInfo…}

Name                           Value
----                           -----
PSVersion                      7.4.1
PSEdition                      Core
GitCommitId                    7.4.1
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

Running the command with $DebugPreference="Continue" yields the following:

PS> install-psresource microsoft.graph.teams -Repository Artifactory-Remote -Reinstall
DEBUG: In InstallPSResource::ProcessInstallHelper()
DEBUG: In InstallHelper::BeginInstallPackages()
DEBUG: Parameters passed in >>> Name: 'microsoft.graph.teams'; VersionRange: ''; NuGetVersion: ''; VersionType: 'NoVersion'; Version: ''; Prerelease: 'False'; Repository: 'Artifactory-Remote'; AcceptLicense: 'False'; Quiet: 'False'; Reinstall: 'True'; TrustRepository: 'False'; NoClobber: 'False'; AsNupkg: 'False'; IncludeXml 'True'; SavePackage 'False'; TemporaryPath ''; SkipDependencyCheck: 'False'; AuthenticodeCheck: 'False'; PathsToInstallPkg: 'C:\Users\sewilliams\Documents\PowerShell\Modules,C:\Users\sewilliams\Documents\PowerShell\Scripts'; Scope 'CurrentUser'
DEBUG: In InstallHelper::ProcessRepositories()
DEBUG: In InstallHelper::InstallPackages()
DEBUG: In InstallHelper::InstallPackage()
DEBUG: In V2ServerAPICalls::FindName()
DEBUG: In V2ServerAPICalls::HttpRequestCall()
DEBUG: Request url is 'https://artifactory.f.q.d.n/artifactory/api/nuget/v2/psgallery-nuget-remote/FindPackagesById()?id='microsoft.graph.teams'&$inlinecount=allpages&$filter=IsLatestVersion'
DEBUG: In V2ServerAPICalls::InstallVersion()
DEBUG: In V2ServerAPICalls::HttpRequestCallForContent()
DEBUG: Request url is 'https://artifactory.f.q.d.n/artifactory/api/nuget/v2/psgallery-nuget-remote/Download/Microsoft.Graph.Teams/2.17.0'
DEBUG: In InstallHelper::TryInstallToTempPath()
DEBUG: In InstallHelper::CallAcceptLicense()
DEBUG: In InstallHelper::DeleteExtraneousFiles()
DEBUG: Deleting 'C:\Users\sewilliams\AppData\Local\Temp\0c008ba5-d731-496e-8d8a-76a5f9242456\microsoft.graph.teams\2.17.0\Microsoft.Graph.Teams.nuspec'
DEBUG: Deleting 'C:\Users\sewilliams\AppData\Local\Temp\0c008ba5-d731-496e-8d8a-76a5f9242456\microsoft.graph.teams\2.17.0\[Content_Types].xml'
DEBUG: Deleting 'C:\Users\sewilliams\AppData\Local\Temp\0c008ba5-d731-496e-8d8a-76a5f9242456\microsoft.graph.teams\2.17.0\_rels'
DEBUG: Deleting 'C:\Users\sewilliams\AppData\Local\Temp\0c008ba5-d731-496e-8d8a-76a5f9242456\microsoft.graph.teams\2.17.0\package'
DEBUG: In InstallHelper::CreateMetadataXMLFile()
DEBUG: In V2ServerAPICalls::FindVersionGlobbing()
DEBUG: In V2ServerAPICalls::FindVersionGlobbing()
DEBUG: In V2ServerAPICalls::HttpRequestCall()
DEBUG: Request url is 'https://artifactory.f.q.d.n/artifactory/api/nuget/v2/psgallery-nuget-remote/FindPackagesById()?id='Microsoft.Graph.Authentication'&$orderby=NormalizedVersion desc&$inlinecount=allpages&$skip=0&$filter= and NormalizedVersion ge '2.17.0' and NormalizedVersion le '2.17.9''
DEBUG: Response is empty
DEBUG: Found package 'Microsoft.Graph.Teams' version '2.17.0'
Install-PSResource: Package(s) 'microsoft.graph.teams' could not be installed from repository 'Artifactory-Remote'.

The request URL is the problem here. The string and NormalizedVersion ge '2.17.0' and NormalizedVersion le '2.17.9' is not a valid OData filter.

sean-r-williams commented 2 months ago

The problem appears to lie within V2ServerAPICalls::FindVersionGlobbing(), specifically the part that builds filter queries here.

Around line 1145, the filter query builder switches from conditionally prepending andOperator (based on whether the filter parameter appears to be null) to always adding it. In the specific case of Artifactory, we skip adding the package ID on line 1143 so adding an and isn't appropriate. Line 1153 appears to be the specific area of issue, but L1145 looks like it'd hit another edge-case.

(FWIW, troubleshooting this was significantly impeded by vagueness in the error returned from Install-PSResource. There's no .InnerException, and only the outer package name is shown.)

I could file a PR to further conditionalize prepending andOperator in L1145 and L1153, but holistically speaking V2ServerAPICalls and NuGetServerAPICalls spend a lot of effort trying to build filters (and the URLs containing them) using the repeated-concatenation approach we see currently.

HDWF about adjusting V2ServerAPICalls and NuGetServerAPICalls to offload this to String.Join() with a List<String> of criteria? Moving to something like HttpValueCollection would also reduce the effort to aggregate filter/pagination and other query parameters. (example here)

sean-r-williams commented 2 months ago

@SydneyhSmith Can some engineering resources from your team weigh in here? I'm happy to submit PRs for either option, but I want to make effective use of your team's (and my) time on this.

Continually patching fencepost-type problems in the NuGet v2 query builders (both in V2Server and NugetServer) is going to spend a lot of time. Our team can't move to NuGet v3 until PSGallery supports it (as Artifactory doesn't translate v2/v3 when making proxied calls through a remote feed) so we need NuGet v2 to work as-expected in scenarios like this.

We're encountering internal scenarios where we need proper dependency support on module/script install, so this is of particular importance to us right now.

anamnavi commented 2 months ago

@sean-r-williams thanks for creating this issue and providing such a detailed report, it was helpful for us to see what is happening. We think a few things will be helpful:

we understand that we need to add tests for Artifactory and will add this to our backlog
the 2nd option of using HttpValueCollection sounds like a better approach to creating the URL query filter. If you are still interested in creating this PR we would appreciate it. Otherwise we can add it to our backlog as well.

sean-r-williams commented 2 months ago

@anamnavi Thanks for the insight. I'll submit two PRs in this case:

One for solving the immediate issue described here, which should be easier to accept into the next patch release of 1.0.x/1.1.x.
One with a refactor of V2ServerAPICalls and NugetServerAPICalls to use a new class for constructing NuGet v2 API queries in an object-oriented manner. (I filed #1643 to track the work needed here)

sean-r-williams commented 2 months ago

@anamnavi Both PRs have been filed. If your team would like more time to review #1645, can you prioritize merging #1644 into the next patch release? #1644 is a smaller band-aid that should be easier to integrate in the short term.

Again, this completely breaks PSResourceGet on module/script packages that declare dependencies with Artifactory. We're just starting to migrate packages with nontrivial dependencies - engineers not being able to pull said packages back out of Artifactory is impacting their work.

PowerShell / PSResourceGet