NuGet - Add Push Service
What does this MR do?
This MR adds the Push service endpoint in the NuGet API.
This API is meant to be used by the following tools: nuget
and Visual Studio.
See the epic: &2271 (closed)
Related issue #36499 (closed) and #20050 (closed)
This MR changes the following:
- Implements the push service for a NuGet Feed. Basically, an endpoint accepting package uploads. See https://docs.microsoft.com/en-us/nuget/api/package-publish-resource. Only the "push" part is implemented in this MR. The "delete" part will be added at a later time (see #36499 (comment 255275410))
- Sidenote: this MR being part of the NuGet MVC. We currently allow multiple uploads of the same package file to simplify the first iteration of the implementation.
- Reuse as many existing services as possible. The only one that is added in this MR is
Packages::Nuget::CreatePackageService
- Centralize code between the Conan endpoint and the Nuget endpoint. For example, all the workhorse upload process is shared. Specs will share common functions.
- Add missing specs for
API::Helpers::PackagesHelpers
Additional notes
- nuget clients impose how the API has to be authenticated. Clients will make an anonymous request. Upon receiving
unauthorized
with the proper http header, it will make a second request but the proper credentials attached. This part has already been handled in the previous MR (!20825 (merged)). - the upload request lacks of any information about the package. Here is an example of such upload:
- As you can see above, the request doesn't have the package name, the package version or any other information. As such, the upload endpoint will simply store the package archive file and that's it. That's why the package has a fixed name and version.
- In a future MR, a job will be implemented to extract and analyze the package archive (see #36502 (closed)). That's where we will be able to set properly the package name and version.
Screenshots
Here is an upload action with nuget
:
$ nuget push DummyProject.DummyPackage.1.0.0.nupkg -source local
WARNING: No API Key was provided and no API Key could be found for 'https://gitlab.local/api/v4/projects/19/packages/nuget'. To save an API Key for a source use the 'setApiKey' command.
Pushing DummyProject.DummyPackage.1.0.0.nupkg to 'https://gitlab.local/api/v4/projects/19/packages/nuget'...
PUT https://gitlab.local/api/v4/projects/19/packages/nuget/
Created https://gitlab.local/api/v4/projects/19/packages/nuget/ 37741ms
Your package was pushed.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Merge request reports
Activity
changed milestone to %12.6
1 Warning This merge request is quite big (more than 832 lines changed), please consider splitting it into multiple merge requests. 1 Message CHANGELOG missing: If this merge request doesn’t need a CHANGELOG entry, feel free to ignore this message. You can create one with:
bin/changelog -m 21493 "NuGet - Add Push Service"
If you want to create a changelog entry for GitLab EE, run the following instead:
bin/changelog --ee -m 21493 "NuGet - Add Push Service"
Note: Merge requests with ~backstage, ci-build, meta do not trigger this check.
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.
Category Reviewer Maintainer backend Mark Fletcher ( @markglenfletcher
)Dmitriy 'DZ' Zaporozhets ( @dzaporozhets
)Generated by
DangerEdited by 🤖 GitLab Bot 🤖added 2 commits
marked the checklist item Code review guidelines as completed
marked the checklist item Merge request performance guidelines as completed
marked the checklist item Style guides as completed
marked the checklist item Separation of EE specific content as completed
- Resolved by Jan Provaznik
- Resolved by David Fernandez
added workflowin review label and removed workflowin dev label
assigned to @sabrams
assigned to @ggelatti
- Resolved by David Fernandez
added 578 commits
-
c3eac580...9682501f - 576 commits from branch
master
- 0b14b4c5 - Add Push Service endpoint in the NuGet API
- 52d6bdd1 - Fixes
-
c3eac580...9682501f - 576 commits from branch
- Resolved by Jan Provaznik
assigned to @igor.drozdov
@igor.drozdov for backend review. Thanks!unassigned @ggelatti
mentioned in issue #36502 (closed)
unassigned @sabrams
- Resolved by David Fernandez
@igor.drozdov , just wanted to check to see if you've had a moment to take a look at this issue.
- Resolved by David Fernandez
- Resolved by David Fernandez
unassigned @igor.drozdov
@jprovaznik Do you mind doing the maintainer review for this MR?
You did it for the initial MR (!20825 (merged)) and as such, you know quite well the context of this API.
Edited by David Fernandezassigned to @jprovaznik
- Resolved by David Fernandez
- Resolved by Jan Provaznik
- Resolved by David Fernandez
- Resolved by David Fernandez
- Resolved by David Fernandez
Thanks @10io, overall LGTM, I left some questions inline but only minor. Let me know what you think.
unassigned @jprovaznik
added 2 commits