Skip to content

Fix manifest workhorse upload

Steve Abrams requested to merge 335560-fix-manifest-wh-upload into master

🌳 Context

In !73033 (merged), we updated the Dependency Proxy to use workhorse to manage the file fetching and uploading that was previously done in rails. This process uses workhorse accelerated uploads to upload the file and communicate the file metadata to rails.

In the previous MR, two endpoints were introduce using the #authorize_upload_manifest and #upload_manifest actions in Groups::DependencyProxyForContainersController.

The #upload_manifest action is where the new file record is actually created and the uploaded file is associated with it.

💔 What went wrong

The "manifest" is one of the files that the Dependency Proxy manages. If a given manifest file is uploaded that already exists, the record needs to be updated, rather than having a duplicate created. This is something we simply overlooked in the previous MR. You can see in the code where rails was previously downloading and storing the file, we had such logic (this is still the current code being used with the dependency_proxy_manifest_workhorse feature flag disabled. When this was translated to the new controller action, this logic was missed.

When we enabled the feature flag on production, we had the case where a manifest was pulled that was meant to overwrite an existing one, but 💥, we tried to create a new one instead and there is a PostgreSQL constraint to prevent such duplicates so we received a PG::UniqueViolation error.

🔍 What does this MR do?

This MR updates the #upload_manifest action to do a sort of find_or_create logic based on the pre-existing logic for the old rails implementation.

I've opted to include some duplication from the service here because when the feature flag is removed, the code in the service will also be removed.

💾 Database

The queries introduced are not new, they are the same as the ones in the service that is part of the old implementation (feature flag disabled).

📷 Screenshots or screen recordings

This is a backend change, so beyond the successful image pulls in the test described below, there is no screenshots to share.

🖥 How to set up and validate locally

  1. Follow these docs to set up the Dependency Proxy on your GDK.

  2. Apply the workhorse updates by running make gitlab-workhorse-setup && gdk restart gitlab-workhorse in your GDK root directory.

  3. Create a group and navigate to Packages & Registries -> Dependency Proxy to find the image prefix.

  4. Log into the Dependency Proxy using a PAT:

    docker login gdk.test:3000
    username: root
    password: <personal_access_token>
  5. Enable the feature flag in the rails console:

    Feature.enable(:dependency_proxy_manifest_workhorse)
  6. Pull an image through the dependency proxy:

    # use your image prefix, it should look like
    docker pull gdk.test:3000/<group_path>/dependency_proxy/containers/alpine:latest
  7. In the rails console, make the manifest stale so it gets overwritten in the next pull:

    DependencyProxy::Manifest.last.update!(digest: 'foo')
  8. Use docker images to find the IMAGE ID of the image you pulled and then remove it from your local machine's cache:

    docker rmi -f 14119a10abf4
  9. Pull the image again. The pull should be successful and you should see the digest value be updated in the DB.

You can reproduce the error by running this same test with the feature flag disabled.

📐 MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #335560 (closed)

Edited by Steve Abrams

Merge request reports