Extract common parts of the Maven dependency proxy

Context

In Maven dependency proxy (&3610 - closed), we implemented and delivered the Maven dependency proxy. Read about it here: https://docs.gitlab.com/ee/user/packages/package_registry/dependency_proxy/.

We are now starting working on npm Virtual Registry MVC: Single Upstream and A... (&3608) which is exactly the same features set (GitLab acting as a pull through cache) but for the NPM package format.

The Maven dependency proxy has some interactions with the Package Registry as it uses it as a cache location (see https://docs.gitlab.com/ee/user/packages/package_registry/dependency_proxy/#advanced-caching). This is the core behavior of the dependency proxy for packages that we will need to implement for NPM.

To avoid a massive code duplication, in this MR, we're going to extract all the common logic that will be used by both dependency proxies.

This is preparatory work for the implementation of NPM dependency proxy: implement the download tg... (#435644 - closed) and later.

If you're interested to see these changes + the first step for the NPM proxy, see NPM dependency proxy tgz download file endpoint (!144320 - merged). If we look at the ee/lib/api/dependency_proxy/packages/npm.rb, we only need 100 lines to implement the first endpoint of the NPM dependency proxy

What does this MR do and why?

• Extract the Maven dependency proxy caching logic into helper functions.
• Update the tracking events so that they adapt to the considered package format.
• Move the DependencyProxy::Packages::Maven::VerifyPackageFileEtagService to DependencyProxy::Packages::VerifyPackageFileEtagService as this service is part of the caching logic and is not really tied to a package format (and we're going to use for NPM too).
  • Move the related specs.

No changelog here as this is a pure code extract with no impact on the existing Maven dependency proxy behaviors. If there is an impact, that will be considered as a typebug.

We are quite confident in this refactoring as the maven dependency proxy is covered by a pretty large requests spec and a feature spec (which also checks all the s done with workhorse).

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

No changes in the UI

How to set up and validate locally

Follow https://docs.gitlab.com/ee/user/packages/package_registry/dependency_proxy/

changed milestone to %16.9

added Category:Package Registry Category:Dependency Proxy GitLab Premium Package:P1 backend devopspackage feature flag grouppackage registry package:active sectionci typefeature workflowin dev labels

assigned to @10io

	2 Warnings
	This merge request is quite big (560 lines changed), please consider splitting it into multiple merge requests.
	e3c5f77d: The commit body should not contain more than 72 characters per line. For more information, take a look at our Commit message guidelines.

Reviewer roulette

Category	Reviewer	Maintainer
backend	@praba.m7n (UTC+5.5, 4.5 hours ahead of author)	@egrieff (UTC+1, same timezone as author)
test for spec/features/*	@praba.m7n (UTC+5.5, 4.5 hours ahead of author)	Maintainer review is optional for test for spec/features/*

Please check reviewer's status!

• Reviewer is available!
• Reviewer is unavailable!

Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

changed the description

removed feature flag label

added maintenancerefactor typemaintenance labels and removed typefeature label

resolved all threads

changed the description

Hello!

Here is a code extraction/reorganization MR for the dependency proxy for packages. This is required for the upcoming NPM dependency proxy.

• @mkhalifa3 Can you backend review this MR?
• @subashis Can you test review this one?

Many thanks!

added workflowin review label and removed workflowin dev label

requested review from @mkhalifa3 and @subashis

E2E Test Result Summary

allure-report-publisher generated test report!

e2e-test-on-gdk: test report for 2aa646b7

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Monitor     | 4      | 0      | 0       | 0     | 4     | ✅     |
| Data Stores | 0      | 0      | 2       | 0     | 2     | ➖     |
| Govern      | 3      | 0      | 0       | 0     | 3     | ✅     |
| Create      | 8      | 0      | 3       | 0     | 11    | ✅     |
| Package     | 24     | 0      | 2       | 0     | 26    | ✅     |
| Plan        | 4      | 0      | 0       | 0     | 4     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 43     | 0      | 7       | 0     | 50    | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

e2e-package-and-test: test report for 2aa646b7

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Package     | 227    | 0      | 16      | 18    | 243   | ✅     |
| Create      | 140    | 0      | 21      | 4     | 161   | ✅     |
| Data Stores | 0      | 0      | 2       | 0     | 2     | ➖     |
| Plan        | 4      | 0      | 0       | 0     | 4     | ✅     |
| Govern      | 3      | 0      | 0       | 0     | 3     | ✅     |
| Monitor     | 4      | 0      | 0       | 0     | 4     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 378    | 0      | 39      | 22    | 417   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

mentioned in issue #435644 (closed)

approved this merge request

@subashis, thanks for approving this merge request.

This is the first time the merge request has been approved. To ensure we don't only run predictive pipelines, and we don't break master, a new pipeline will be started shortly.

Please wait for the pipeline to start before resolving this discussion and set auto-merge for the new pipeline. See merging a merge request for more details.

added pipeline:mr-approved label

added 1 commit

• 2aa646b7 - Apply backend feedback

Compare with previous version

added pipeline:skip-undercoverage label

Applying pipeline:skip-undercoverage as the two functions without tests will be actually be used by the NPM dependency proxy.

For the Maven dependency proxy, we override them to have a different behavior.

requested review from @mkhalifa3

requested review from @egrieff

approved this merge request

removed review request for @mkhalifa3

changed milestone to %16.10

added missed:16.9 label

approved this merge request

requested review from @egrieff

resolved all threads

enabled an automatic merge when the pipeline for 47c9c444 succeeds

merged

mentioned in commit 740c2759

added workflowstaging-canary label and removed workflowin review label

added workflowcanary label and removed workflowstaging-canary label

added workflowstaging label and removed workflowcanary label

added workflowproduction label and removed workflowstaging label

added releasedcandidate label

added releasedpublished label and removed releasedcandidate label

added pipelinetier-3 label