Skip to content

Revert '290944-pull-by-digest' and cleanup bad records

Steve Abrams requested to merge revert-pull-by-digest into master

What does this MR do?

Reverts !52805 (merged) because it does not seem to work with GCP object storage and has broken the Dependency Proxy on GitLab.com.

The migrations from the original MR will not be reverted, so the changelog is updated to only mention the migration.

A new post-migration is added to remove the dependency proxy manifests that have been downloaded since this has been in production. We remove all manifests that have a content_type stored (the content_type was introduced with the original MR, so any records without a content_type would predate the change). The dependency proxy is a cache, so deleting these files and records is ok. The next time the user pull that image through the dependency proxy, a new valid manifest will be downloaded and cached (stored).

So with the exceptions of the files in db/, changelogs/, and spec/migrations, all other files are exact reverts from the original MR.

Incident Reference: gitlab-com/gl-infra/production#3521 (closed)

Database

Up migration

== 20210205174154 RemoveBadDependencyProxyManifests: migrating ================
== 20210205174154 RemoveBadDependencyProxyManifests: migrated (0.0204s) =======

Down migration

== 20210205174154 RemoveBadDependencyProxyManifests: reverting ================
== 20210205174154 RemoveBadDependencyProxyManifests: reverted (0.0000s) =======

Query in migration:

This is the explain plan from a production replica:

explain(analyze,buffers) SELECT "dependency_proxy_manifests".* 
FROM "dependency_proxy_manifests" 
WHERE "dependency_proxy_manifests"."content_type" IS NOT NULL;

                                                       QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------
 Seq Scan on dependency_proxy_manifests  (cost=0.00..68.90 rows=81 width=213) (actual time=0.121..1.064 rows=83 loops=1)
   Filter: (content_type IS NOT NULL)
   Rows Removed by Filter: 407
   Buffers: shared hit=2 read=14
   I/O Timings: read=0.158
 Planning Time: 0.751 ms
 Execution Time: 1.100 ms
(7 rows)

The deletion query. One will run for each record (should be ~83 based on above query):

explain DELETE FROM "dependency_proxy_manifests" 
WHERE "dependency_proxy_manifests"."id" = 506;

                                                       QUERY PLAN
------------------------------------------------------------------------------------------------------------------------
 Delete on dependency_proxy_manifests  (cost=0.27..3.29 rows=1 width=6)
   ->  Index Scan using dependency_proxy_manifests_pkey on dependency_proxy_manifests  (cost=0.27..3.29 rows=1 width=6)
         Index Cond: (id = 506)
(3 rows)

Screenshots (strongly suggested)

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 Steve Abrams

Merge request reports