Skip to content

Allow HEAD API requests for Maven packages with AWS as object storage

David Fernandez requested to merge 32102-fix-maven-head-redirects into master

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:

With those conditions, here is what happens between a Package Manager Client (PMC), GitLab (GL) and Object Storage (OS):

  1. PMC -> GL: "Hey, give me file X of package Y". This is a GET request.
  2. PMC <- GL: "The file is not here but check this url". This is a 301 redirect to a signed url pointing to OS.
  3. 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

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

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
Edited by Peter Leitzen

Merge request reports