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

Investigate what needs to be done to upgrade our Nuget support to use API 3 #1960

Open
4 tasks
steve-todorov opened this issue Nov 17, 2020 · 8 comments
Open
4 tasks
Assignees

Comments

@steve-todorov
Copy link
Member

steve-todorov commented Nov 17, 2020

Task Description

The Nuget team has made an announcement which states they are deprecating OData API v2 queries and will begin blocking 3rd party clients in February 2021.

Today and tomorrow (November 17th and 18th) they will be temporarily blocking all requests coming from older clients.
This is why our master is currently not building.

Example request & output:
URL: https://www.nuget.org/api/v2/Search()?%24filter=Id+eq+%27NHibernate%27&%24top=1000&includePrerelease=false

<m:error xmlns:m="http://schemas.microsoft.com/ado/2007/08/dataservices/metadata">
    <m:code>NuGet.V2.Deprecated</m:code>
    <m:message xml:lang="en-US">The combination of parameters provided to this OData endpoint is no longer supported. Please refer to the following URL for more information about this deprecation: https://aka.ms/nuget/odata-deprecation</m:message>
</m:error>

You can access NuGet integration here:

The API V2 URL is:

Tasks

The following tasks will need to be carried out:

  • Investigate what changes need to be applied.
  • Identify and fix the end-points that are affected.
  • Make sure the the strongbox-web-integration-tests are still working.
  • Add sub-tasks to Implement NuGet protocol v3 support #860 for all the work that is identified.

Task Relationships

This task will cover the preliminary exploratory work required in order to implement #860.

Useful links

Task Relationships

This task:

Help

@sbespalov
Copy link
Member

sbespalov commented Nov 17, 2020

@steve-todorov one small clarification which may be important here. This probably not means that we should fully support Nuget v3 API for strongbox. This is only about for strongbox to support proxying of Nuget v3 repositories.

@sbespalov
Copy link
Member

In other words we could just investigate would it be possible to proxy Nuget v3 thru Nuget v2.

@joelverhagen
Copy link

joelverhagen commented Nov 18, 2020

Thanks for linking to the associated announcement as this notified me of another affected customer. I am happy to provide information regarding the V2 or V3 API in NuGet. To clarify, 3rd party clients are welcome to use either our officially supported V2 endpoints or using the documented V3 API. There is no restriction on 3rd party clients, per se, but rather a restriction on certain V2 OData queries no matter the client.

There are several V2 query patterns that are still supported. These are query patterns that are used by the official NuGet client tools. They are well optimized and there is no plan to disable this category, For example, some of these queries are mentioned here: NuGet/NuGetGallery#7423 (comment). I just wanted to say that you could consider using supported V2 queries as an alternative to moving to V3, if it fits your scenario or saves you effort.

I would urge you to use the V3 protocol if possible since it is more secure, performance, and reliable. Also, it is properly documented here: https://docs.microsoft.com/en-us/nuget/api/overview.

For example, this query could be rewritten:

Unsupported: https://www.nuget.org/api/v2/Search()?%24filter=Id+eq+%27NHibernate%27&%24top=1000&includePrerelease=false
  Supported: https://www.nuget.org/api/v2/FindPackagesById()?id=%27NHibernate%27 (note: includes prerelease versions!)
         V3: https://api.nuget.org/v3-flatcontainer/nhibernate/index.json (note: includes SemVer 2.0.0 versions)

You can test different query patterns to see if they are supported using "int.nugettest.org". The error message you saw was due to a scheduled, temporary brown-out prior to the permanent disabling that we are planning for early 2021. For example:

Unsupported: https://int.nugettest.org/api/v2/Search()?%24filter=Id+eq+%27NHibernate%27&%24top=1000&includePrerelease=false
  Supported: https://int.nugettest.org/api/v2/FindPackagesById()?id=%27NHibernate%27
         V3: https://apiint.nugettest.org/v3-flatcontainer/nhibernate/index.json

@carlspring
Copy link
Member

carlspring commented Nov 18, 2020

Hi @joelverhagen ,

Thank you very much for reaching out to us and for providing these useful details!

The end-points that we support can be found here. In a nutshell, this is what we currently support:

  • This is used for deleting artifacts.
    @DeleteMapping(path = { "{storageId}/{repositoryId}/{packageId}/{version}" })
    @PreAuthorize("hasAuthority('ARTIFACTS_DEPLOY')")
    public ResponseEntity deletePackage(@RequestHeader(name = "X-NuGet-ApiKey", required = false) String apiKey,
                                        @RepositoryMapping Repository repository,
                                        @PathVariable("packageId") String packageId,
                                        @PathVariable("version") String version)
  • This is used for getting the search count.
    @GetMapping(path = { "{storageId}/{repositoryId}/Search()/$count" }, produces = MediaType.TEXT_PLAIN)
    public ResponseEntity<String> countPackages(@RepositoryMapping Repository repository,
                                                @RequestParam(name = "$filter", required = false) String filter,
                                                @RequestParam(name = "searchTerm", required = false) String searchTerm,
                                                @RequestParam(name = "targetFramework", required = false) String targetFramework)
  • Used for search.
    @GetMapping(path = { "{storageId}/{repositoryId}/{searchCommandName:(?:Packages(?:\\(\\))?|Search\\(\\))}" },
                produces = MediaType.APPLICATION_XML)
    public ResponseEntity<?> searchPackages(@RepositoryMapping Repository repository,
                                            @PathVariable(name = "searchCommandName") String searchCommandName,
                                            @RequestParam(name = "$filter", required = false) String filter,
                                            @RequestParam(name = "$orderby", required = false, defaultValue = "Id") String orderBy,
                                            @RequestParam(name = "$skip", required = false) Integer skip,
                                            @RequestParam(name = "$top", required = false) Integer top,
                                            @RequestParam(name = "searchTerm", required = false) String searchTerm,
                                            @RequestParam(name = "targetFramework", required = false) String targetFramework,
                                            HttpServletResponse response)
  • Used for looking up packages by their ID.
    @GetMapping(path = { "{storageId}/{repositoryId}/FindPackagesById()" }, produces = MediaType.APPLICATION_XML)
    public ResponseEntity<?> searchPackageById(@RepositoryMapping Repository repository,
                                               @RequestParam(name = "id", required = true) String packageId,
                                               HttpServletResponse response)
  • Used to get storage metadata
    @ApiOperation(value = "Used to get storage metadata")
    @ApiResponses(value = { @ApiResponse(code = HttpURLConnection.HTTP_OK, message = "The metadata was downloaded successfully."),
                            @ApiResponse(code = HttpURLConnection.HTTP_INTERNAL_ERROR, message = "An error occurred.") })
    @GetMapping(path = { "{storageId}/{repositoryId}/$metadata" }, produces = MediaType.APPLICATION_XML)
  • Used for checking the access to a repository (this is most-likely irrelevant to the case).
    @ApiOperation(value = "Used to check storage availability")
    @ApiResponses(value = { @ApiResponse(code = HttpURLConnection.HTTP_OK, message = "Storage available."),
                            @ApiResponse(code = HttpURLConnection.HTTP_UNAUTHORIZED, message = "Storage requires authorization.") })
    @GetMapping(path = { "{storageId}/{repositoryId}", "checkRepositoryAccess" })
    @PreAuthorize("hasAuthority('ARTIFACTS_RESOLVE')")
    public ResponseEntity<String> checkRepositoryAccess()
  • Used for deploying packages.
    @ApiOperation(value = "Used to deploy a package")
    @ApiResponses(value = { @ApiResponse(code = HttpURLConnection.HTTP_OK, message = "The package was deployed successfully."),
                            @ApiResponse(code = HttpURLConnection.HTTP_INTERNAL_ERROR, message = "An error occurred.") })
    @PreAuthorize("hasAuthority('ARTIFACTS_DEPLOY')")
    @RequestMapping(path = "{storageId}/{repositoryId}/", method = RequestMethod.PUT, consumes = MediaType.MULTIPART_FORM_DATA)
    public ResponseEntity putPackage(@RequestHeader(name = "X-NuGet-ApiKey", required = false) String apiKey,
                                     @RepositoryMapping Repository repository,
                                     @RequestParam(value = "package") MultipartFile file,
                                     HttpServletRequest request)
  • Used for downloading a package. (Not sure about the difference is, @sbespalov , perhaps you could clarify how it differs to the one below...?)
    @ApiOperation(value = "Used to download a package")
    @ApiResponses(value = { @ApiResponse(code = HttpURLConnection.HTTP_OK, message = "The request was successfull."),
                            @ApiResponse(code = HttpURLConnection.HTTP_INTERNAL_ERROR, message = "Server error occurred."),
                            @ApiResponse(code = HttpURLConnection.HTTP_NOT_FOUND, message = "The requested path was not found."),
                            @ApiResponse(code = HttpURLConnection.HTTP_UNAVAILABLE, message = "Repository not in service currently.")})
    @PreAuthorize("hasAuthority('ARTIFACTS_RESOLVE')")
    @RequestMapping(path = "{storageId}/{repositoryId}/{commandName:(?:download|package)}/{packageId}/{packageVersion}",
                    method = {RequestMethod.GET, RequestMethod.HEAD},
                    produces = MediaType.APPLICATION_OCTET_STREAM)
    public void downloadPackage(@RepositoryMapping Repository repository,
                                @ApiParam(value = "The packageId", required = true) @PathVariable(name = "packageId") String packageId,
                                @ApiParam(value = "The packageVersion", required = true) @PathVariable(name = "packageVersion") String packageVersion,
                                HttpServletResponse response,
                                HttpServletRequest request,
                                @RequestHeader HttpHeaders httpHeaders)
  • Used for downloading a package. (Not sure about the difference is, @sbespalov , perhaps you could clarify how it differs to the one below...?)
    @ApiOperation(value = "Used to download a package")
    @ApiResponses(value = { @ApiResponse(code = HttpURLConnection.HTTP_OK, message = "The request was successfull."),
                            @ApiResponse(code = HttpURLConnection.HTTP_INTERNAL_ERROR, message = "Server error occurred."),
                            @ApiResponse(code = HttpURLConnection.HTTP_NOT_FOUND, message = "The requested path was not found."),
                            @ApiResponse(code = HttpURLConnection.HTTP_UNAVAILABLE, message = "Repository not in service currently.")})
    @PreAuthorize("hasAuthority('ARTIFACTS_RESOLVE')")
    @RequestMapping(path = "{storageId}/{repositoryId}/{packageId}/{packageVersion}",
                    method = {RequestMethod.GET, RequestMethod.HEAD},
                    produces = MediaType.APPLICATION_OCTET_STREAM)

We will have to implement this for both hosted and proxy repositories and add end-points for the NuGet protocol v3 ones, while preserving the NuGet protocol v2 (for now).

In addition to this, we also have:

Ultimately, we would really like to implement full-blown support for NuGet protocol version 3, but we're a little short-handed right now for such an undertaking, as we have some higher priority tasks. If you would like to help out (even if with just pointers of what would be required as a bare minimum), it would be great, if you could add some comments to #860 .

Looking forward to hearing your thoughts!

Many thanks in advance!
Kind regards,

Martin

@joelverhagen
Copy link

@DeleteMapping(path = { "{storageId}/{repositoryId}/{packageId}/{version}" })

Looks fine. Example request:
DELETE https://www.nuget.org/api/v2/package/{packageId}/{version}

@GetMapping(path = { "{storageId}/{repositoryId}/Search()/$count" }, produces = MediaType.TEXT_PLAIN)

Looks fine as long as $filter is not provided. The $filter parameter requires DB access for us and is extremely expensive at scale. Consider changing the approach.

Without $filter, it works fine.

https://int.nugettest.org/api/v2/Search()/$count
or
https://www.nuget.org/api/v2/Search()/$count

   @GetMapping(path = { "{storageId}/{repositoryId}/{searchCommandName:(?:Packages(?:\\(\\))?|Search\\(\\))}" },
               produces = MediaType.APPLICATION_XML)

This one can be problematic with $filter as mentioned above. Nearly all usages of $filter are deprecated. Also, only a select few $orderby values are supported.

...

And the rest look okay, from a high level glance. Again, I'd highly recommend testing on "int.nugettest.org/api/v2" to evaluate your impact in various scenarios.

Clever approach, using ANTLR. Cool stuff.

@carlspring
Copy link
Member

@sbespalov ,

As the lead developer on the NuGet implementation, is there anything else you would like to add?

Clever approach, using ANTLR. Cool stuff.

All credits go to @sbespalov who spent about a year absorbing, improving and further developing the JNuGet project (which is now dead) into Strongbox! :)

@carlspring
Copy link
Member

@j4ckofalltrades , @anki2189 , @madhukarbharti ,

Would either of you fine gentlemen be insterested in working on this issue? :)

@sbespalov
Copy link
Member

as per comments above we seems just need to check how we rely on deprecated API. First of all we need to know if this affects on strogbox-web-integration-tests and if does affects then find out exact API calls that causing issues. Then we need to check whether it is possible to do without deprecated API calls if needed.
Based on the results it will be possible to make conclusions how we could continue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants