Skip to content

Fix Conan project-level download urls

Steve Abrams requested to merge 270129-conan-project-download-urls-bug into master

🏘 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:

  1. The Conan /upload_urls endpoints use helpers in this module that use these two methods directly, thus we have access to params[:id] from the Grape API endpoint params. These routes are returning as expected.
  2. 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 where params[: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 in package_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

Availability and Testing

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)

Edited by Steve Abrams

Merge request reports