Allow HEAD API requests for Maven packages with AWS as object storage
What does this MR do?
Packages are uploaded to GitLab using the in place uploaders (See https://gitlab.com/gitlab-org/gitlab/-/blob/32102-fix-maven-head-redirects/ee/app/uploaders/packages/package_file_uploader.rb).
Uploads can be configured in multiple ways but here we will focus when:
- AWS S3 is used as a object storage backend.
- Proxy download is disabled. (see https://docs.gitlab.com/ee/administration/uploads.html#object-storage-settings)
With those conditions, here is what happens between a Package Manager Client (PMC), GitLab (GL) and Object Storage (OS):
- PMC -> GL: "Hey, give me file X of package Y". This is a GET request.
- PMC <- GL: "The file is not here but check this url". This is a 301 redirect to a signed url pointing to OS.
- PMC -> OS: "Hey, give me file X". This is a GET request.
The signature used in (2.) can include the http verb used. That's the case for AWS S3 (but not GCP). That means that GitLab will generate a signed url for a GET and only GET requests will be accepted by AWS S3.
The issue reported in #32102 (closed) is that Gradle, as part of its caching system (See https://github.com/gradle/gradle/issues/5322#issuecomment-543580979), can decide to make a HEAD to the file instead of a GET. My guess is that, it's trying to get the etag
so that it can compare it to its local caches and see if the file is cached locally before actually downloading it.
Currently, in the Maven API, the generated url (through lib/api/helpers.rb#present_carrierwave_file!
) is for GET requests. When Gradle receives the redirect, it will HEAD the object storage ressource and be greeted by a 403 Forbidden from AWS S3.
This MR updates the Maven API so that the request http method is inspected and if a HEAD is detected, the proper signature is generated (aka a signature for HEAD and not GET).
Note that this bug doesn't happen on GitLab.com because GCP seems to be more lax on the constraints: it accepts HEAD requests on a url signed for GET.
Design choices
- This issue is not only for Gradle and the Maven API but all downloads on the API side of GitLab. (when OS is enabled + baked with AWS S3 + proxy mode disabled + the request is a HEAD).
- We could update
lib/api/helpers.rb#present_carrierwave_file!
but that seems a bit dangerous as it will be a change for all downloads for all APIs. - That's why, for now, this MR focuses a fix for the Maven API by putting a new method(
present_carrierwave_file_with_head_support!
) in front oflib/api/helpers.rb#present_carrierwave_file!
. - The way the fix works is a bit hacky. As we don't have access to signed urls for HEAD within the CarrierWave object (
CarrierWave::Storage::Fog::File
), we have to use theFog object
directly. To use such object, we have to build it out a Fog connection (see https://gitlab.com/gitlab-org/gitlab/-/blob/32102-fix-maven-head-redirects/ee/lib/api/maven_packages.rb#L63-68). - The above is not ideal. CarrierWave should have a way to generate signed urls for HEAD but that's not the case (propose a fix to CarrierWave?)
- Note that this fix patch the three levels of the API (project, group and instance). See https://docs.gitlab.com/ee/user/packages/maven_repository/index.html#configuring-your-project-to-use-the-gitlab-maven-repository-url.
Screenshots
Without this MR:
$ rm -rf ~/.gradle/caches/ && ./gradlew build
> Task :compileJava FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':compileJava'.
> Could not resolve all files for configuration ':compileClasspath'.
> Could not resolve buggy.instance:dependency:9.8.7.
Required by:
project :
> Could not resolve buggy.instance:dependency:9.8.7.
> Could not get resource 'http://gitlab.local:8000/api/v4/packages/maven/buggy/instance/dependency/9.8.7/dependency-9.8.7.pom'.
> Could not HEAD 'http://gitlab.local:9000/packages/c2/35/c2356069e9d1e79ca924378153cfbbfb4d4416b1f99d41a2940bfdb66c5319db/packages/173/files/203/dependency-9.8.7.pom?[headers truncated]'. Received status code 403 from server: Forbidden
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
* Get more help at https://help.gradle.org
Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.2.2/userguide/command_line_interface.html#sec:command_line_warnings
BUILD FAILED in 7s
1 actionable task: 1 executed
With this MR:
$ rm -rf ~/.gradle/caches/ && ./gradlew build
Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.2.2/userguide/command_line_interface.html#sec:command_line_warnings
BUILD SUCCESSFUL in 28s
7 actionable tasks: 7 up-to-date
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