Fix Conan project-level download urls
🏘 Context
The GitLab package registry allows users to publish and consume Conan (C/C++) packages. We have the ability to work with packages at both the project and instance level. One of the requests the Conan client makes in the package installation process is a request for a set of "download urls" that it will use to request each individual package file. There are two such routes ending in /download_urls
. When the Conan client makes this request from the instance level, it will receive a payload that looks like:
{
"conanfile.py": "http://gdk.test:3001/api/v4/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanfile.py",
"conanmanifest.txt": "http://gdk.test:3001/api/v4/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanmanifest.txt"
}
When it is requested at the project level, /projects/:id
is included in the response (as it was also part of the request):
{
"conanfile.py": "http://gdk.test:3001/api/v4/projects/8/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanfile.py",
"conanmanifest.txt": "http://gdk.test:3001/api/v4/projects/8/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanmanifest.txt"
}
The problem is, right now, both project-level and instance-level requests are returning the instance-level response. So if a user is using the project-level remote to work with their packages, these requests are returning the instance level download urls, which makes Conan say
🔬 What does this MR do?
The API::Helpers::Packages::Conan::ApiHelpers
module includes the methods #recipe_file_url
and #package_file_url
. Both of these methods use a case statement on package_scope
, which is another helper method that checks for the presence of params[:id]
.
#recipe_file_url
and #package_file_url
are used in two places:
- The Conan
/upload_urls
endpoints use helpers in this module that use these two methods directly, thus we have access toparams[:id]
from the Grape API endpoint params. These routes are returning as expected. - The
/download_urls
endpoints use a presenter that then calls these two methods. This means we do not have direct access to the original Grape API endpoint params, but only have access to the params from the presenter (in other words#recipe_file_url
and#package_file_url
are being called from a different scope whereparams[:id]
is not accessible).
So, we simply pass id: params[:id]
as a param to the presenter so when it calls these two methods, they will be able to access params[:id]
and properly evaluate the value for package_scope
.
📋 Notes
-
You'll notice that although the actual code change is in the
package_presenter.rb
file, there are no changes to the tests inpackage_presenter_spec.rb
. This is because we actually did include the missing param in the specs, so they are passing as expected🤦🏼 -
Since the presenter specs were working, I decided to update the request specs. They were stubbing out the presenter and it's response, so we never realized that the presenter was receiving the incorrect params. I've removed the presenter double from the specs so we get a full integration style test for these endpoints, proving the param is getting passed through the presenter properly.
🎥 Screenshots (strongly suggested)
Before the update
→ curl -H "Authorization: Bearer $PRIVATE_TOKEN" http://gdk.test:3001/api/v4/projects/8/packages/conan/v1/conans/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/download_urls | jq % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 295 100 295 0 0 26 0 0:00:11 0:00:11 --:--:-- 75 { "conanfile.py": "http://gdk.test:3001/api/v4/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanfile.py", "conanmanifest.txt": "http://gdk.test:3001/api/v4/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanmanifest.txt" }
After the update
→ curl -H "Authorization: Bearer $PRIVATE_TOKEN" http://gdk.test:3001/api/v4/projects/8/packages/conan/v1/conans/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/download_urls | jq % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 317 100 317 0 0 20 0 0:00:15 0:00:15 --:--:-- 72 { "conanfile.py": "http://gdk.test:3001/api/v4/projects/8/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanfile.py", "conanmanifest.txt": "http://gdk.test:3001/api/v4/projects/8/packages/conan/v1/files/my-conan-pkg-0/2.0.0/h5bp+html5-boilerplate/stable/0/export/conanmanifest.txt" }
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. - [-] I have not included a changelog entry because _____.
-
-
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
Related to #270129 (closed)