Fix gradle uploads and snapshot versions
What does this MR do?
Given the amount of comments and discussions on the related issue: #207920 (closed), we would like to present this MR's context precisely here.
🌲 Context
To upload maven package files to the GitLab Package Registry, two tools are supported:
mvn
gradle
We're going to take a look at what happens when a snapshot version (eg. 1.0-SNAPSHOT
) is uploaded.
Here is the upload order for mvn
:
my-company/my-app/1.0-SNAPSHOT/my-app.jar
my-company/my-app/1.0-SNAPSHOT/my-app.pom
my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml
my-company/my-app/maven-metadata.xml
And here is the upload order for gradle
:
my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml
my-company/my-app/1.0-SNAPSHOT/my-app.jar
my-company/my-app/1.0-SNAPSHOT/my-app.pom
my-company/my-app/maven-metadata.xml
Take note that the maven-metadata.xml
file with a version is the first upload for gradle
.
🔥 Issue
When the first upload hits the GitLab Package registry, it goes through https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L12-34 to create the Packages::Package
model.
The problem is given that the first upload is not same between mvn
and gradle
, the package model is not created with the same attributes.
Taking the above example, here is the package created by mvn
:
- package name:
my-company/my-app
- package version:
1.0-SNAPSHOT
- maven path:
my-company/my-app/1.0-SNAPSHOT
Here is the package created by gradle
:
- package name:
my-company/my-app/1.0-SNAPSHOT
- package version:
nil
- maven path:
my-company/my-app/1.0-SNAPSHOT
The issue lies in this line: https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L21. This line exists to properly deal with the last upload, the maven-metadata.xml
file without a version. This last upload has a maven path set to my-company/my-app
and will thus create a second package without a version.
When mvn
sends the first upload, the line is executed and versionless package is created but the maven path is my-company/my-app/1.0-SNAPSHOT
. This path will match upload (2), (3) and these files will get appended to the versionless package model =
In addition, versionless packages are not show in the UI and are not returned by the API
🚒 How this MR fixes the issue
- Note that https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L21 was designed to work only with the versionless
maven-metadata.xml
. - We will update the condition (https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/packages/maven/find_or_create_package_service.rb#L12) to avoid entering the
if
block if there is a-SNAPSHOT
version in the maven path. - This way, the proper version
1.0-SNAPSHOT
gets extracted and the proper package with the correct version is created.
🛠 Design choices
- We added a longer note to explain why this condition should only match versionless maven-metadata.xml uploads.
- We refactored the corresponding rspec so we can use a table based test.
- This allows us to test several different conditions (maven path with version or not, snapshot version or not) in a concise way
Screenshots
Before this MR
UI at the project level
UI at the group level
API
$ curl --header "PRIVATE-TOKEN: XXXXXXX" "http://gitlab.local:8000/api/v4/projects/1/packages"
[]
->
With this MR
UI at the project level
UI at the group level
API
$ curl --header "PRIVATE-TOKEN: XXXXXXX" "http://gitlab.local:8000/api/v4/projects/1/packages"
[{"id":42,"name":"gl/pru/ghost","version":"2.2-SNAPSHOT","package_type":"maven","_links":{"web_path":"/gitlab-org/gitlab-test/-/packages/42","delete_api_path":"http://gitlab.local:8000/api/v4/projects/1/packages/42"},"created_at":"2020-08-07T12:56:09.238Z","tags":[],"versions":[]}]
Package details page
->
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 -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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
🙏 Thanks
Thanks @sahbabou for her initial investigation and fix implementation