Update index for security CI builds (!32584) · Merge requests · GitLab.org / GitLab

Saikat Sarkar requested to merge update-index-artifacts-expire into master May 19, 2020

What does this MR do?

We are going to add stand-alone secret analyzer for release 13.1. That's why we need to add secret_detection in the ci_builds_on_commit_id_and_artifacts_expireatandidpartial index. #215082
As part of #5593 (comment 343322846), we need to remove sast:container from the same index.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

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

Performance Analysis

CREATE INDEX index_ci_builds_on_commit_id_and_artifacts_expireatandidpart2 ON public.ci_builds USING btree (commit_id, artifacts_expire_at, id) WHERE (((type)::text = 'Ci::Build'::text) AND ((retried = false) OR (retried IS NULL)) AND ((name)::text = ANY (ARRAY[('sast'::character varying)::text, ('secret_detection'::character varying)::text, ('dependency_scanning'::character varying)::text, ('container_scanning'::character varying)::text, ('dast'::character varying)::text])));

The query has been executed. Duration: 118.488 min (edited)

Query Type 1 (without secret_detection)

SELECT MIN("ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning','container_scanning', 'dast')

before: https://explain.depesz.com/s/TONd
after: https://explain.depesz.com/s/Uida

SELECT MIN(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning','container_scanning', 'dast')

before: https://explain.depesz.com/s/JcrH
after: https://explain.depesz.com/s/OV9V

SELECT MIN(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning','container_scanning', 'dast') AND "ci_builds"."created_at" BETWEEN '2020-04-16 16:27:42.074504' AND '2020-05-14 16:27:42.074867'

before: https://explain.depesz.com/s/CvA
after: https://explain.depesz.com/s/fL7b

Query Type 2 (without secret_detection)

SELECT MAX("ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast')

before: https://explain.depesz.com/s/OBUl
after: https://explain.depesz.com/s/ihhh

SELECT MAX(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast')

before: https://explain.depesz.com/s/J4tp
after: https://explain.depesz.com/s/XR3J

SELECT MAX(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast') AND "ci_builds"."created_at" BETWEEN '2020-04-16 16:27:42.074504' AND '2020-05-14 16:27:42.074867'

before: https://explain.depesz.com/s/XtY1
after: https://explain.depesz.com/s/YgI2

Query Type 3 (without secret_detection)

SELECT COUNT("ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast') AND "ci_builds"."commit_id" BETWEEN 446900000 AND 446999999

before: https://explain.depesz.com/s/Z55K
after:

SELECT COUNT(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast') AND "ci_builds"."commit_id" BETWEEN 446900000 AND 446999999

before: https://explain.depesz.com/s/emgN
after: https://explain.depesz.com/s/tHKx

SELECT COUNT(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast') AND "ci_builds"."created_at" BETWEEN '2020-04-16 16:27:42.074504' AND '2020-05-14 16:27:42.074867' AND "ci_builds"."commit_id" BETWEEN 446900000 AND 446999999

before: https://explain.depesz.com/s/Cs9y
after:

Query Type 4 (with secret_detection)

SELECT MIN("ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection')

before: https://explain.depesz.com/s/ntdV
after: https://explain.depesz.com/s/c43P

SELECT MIN(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection')

before: https://explain.depesz.com/s/7TSU
after: https://explain.depesz.com/s/tVIO

SELECT MIN(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection') AND "ci_builds"."created_at" BETWEEN '2020-04-16 16:27:42.074504' AND '2020-05-14 16:27:42.074867'

before: https://explain.depesz.com/s/XJh8
after:

Query Type 5 (with secret_detection)

SELECT MAX("ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection')

before: https://explain.depesz.com/s/Qo7k
after: https://explain.depesz.com/s/9pC7

SELECT MAX(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection')

before: https://explain.depesz.com/s/cnAim
after: https://explain.depesz.com/s/Ln9j

SELECT MAX(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection') AND "ci_builds"."created_at" BETWEEN '2020-04-16 16:27:42.074504' AND '2020-05-14 16:27:42.074867'

before: https://explain.depesz.com/s/VUjI
after: https://explain.depesz.com/s/nJ8B

Query Type 6 (with secret_detection)

SELECT COUNT("ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection') AND "ci_builds"."commit_id" BETWEEN 446900000 AND 446999999

before: https://explain.depesz.com/s/YmgQ
after:

SELECT COUNT(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection') AND "ci_builds"."commit_id" BETWEEN 446900000 AND 446999999

before: https://explain.depesz.com/s/i18O
after: https://explain.depesz.com/s/H67x

SELECT COUNT(DISTINCT "ci_builds"."commit_id") FROM "ci_builds" WHERE "ci_builds"."type" IN ('Ci::Build') AND ("ci_builds"."retried" = 'f' OR "ci_builds"."retried" IS NULL) AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2018-08-27 16:32:37.511623') AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'container_scanning', 'dast', 'secret_detection') AND "ci_builds"."created_at" BETWEEN '2020-04-16 16:27:42.074504' AND '2020-05-14 16:27:42.074867' AND "ci_builds"."commit_id" BETWEEN 446900000 AND 446999999

before: https://explain.depesz.com/s/4D4K
after: https://explain.depesz.com/s/YU2j

Query Type 7

Before

SELECT "ci_pipelines".*
FROM "ci_pipelines"
INNER JOIN "ci_builds"
ON "ci_builds"."commit_id" = "ci_pipelines"."id"
AND "ci_builds"."type" = 'Ci::Build'
AND ("ci_builds"."retried" = false OR "ci_builds"."retried" IS NULL)
AND (EXISTS (
    SELECT 1
    FROM "ci_job_artifacts"
    WHERE (ci_builds.id = ci_job_artifacts.job_id)
    AND "ci_job_artifacts"."file_type" = 1))
AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2020-05-15 13:24:34.543026')
WHERE "ci_pipelines"."project_id" = 278964
AND "ci_pipelines"."ref" = 'master'
AND "ci_pipelines"."id" IN (
  SELECT "ci_pipelines"."id"
  FROM "ci_pipelines"
  WHERE "ci_pipelines"."project_id" = 278964
  AND "ci_pipelines"."ref" = 'master'
  ORDER BY "ci_pipelines"."id"
  DESC LIMIT 100)
AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'sast:container', 'container_scanning', 'dast')
ORDER BY "ci_pipelines"."id" DESC
LIMIT 1

plan: https://explain.depesz.com/s/bb27

After

SELECT "ci_pipelines".*
FROM "ci_pipelines"
INNER JOIN "ci_builds"
ON "ci_builds"."commit_id" = "ci_pipelines"."id"
AND "ci_builds"."type" = 'Ci::Build'
AND ("ci_builds"."retried" = false OR "ci_builds"."retried" IS NULL)
AND (EXISTS (
    SELECT 1
    FROM "ci_job_artifacts"
    WHERE (ci_builds.id = ci_job_artifacts.job_id)
    AND "ci_job_artifacts"."file_type" = 1))
AND (artifacts_expire_at IS NULL OR artifacts_expire_at > '2020-05-15 13:24:34.543026')
WHERE "ci_pipelines"."project_id" = 278964
AND "ci_pipelines"."ref" = 'master'
AND "ci_pipelines"."id" IN (
  SELECT "ci_pipelines"."id"
  FROM "ci_pipelines"
  WHERE "ci_pipelines"."project_id" = 278964
  AND "ci_pipelines"."ref" = 'master'
  ORDER BY "ci_pipelines"."id"
  DESC LIMIT 100)
AND "ci_builds"."name" IN ('sast', 'dependency_scanning', 'secret_detection', 'container_scanning', 'dast')
ORDER BY "ci_pipelines"."id" DESC
LIMIT 1

plan: https://explain.depesz.com/s/Mn5h

Migration Output

➜  gitlab git:(update-index-artifacts-expire) ✗ bin/rails db:migrate:up VERSION=20200519171058  RAILS_ENV=development
WARNING: This version of GitLab depends on gitlab-shell 13.2.0, but you're running 12.2.0. Please update gitlab-shell.
== 20200519171058 UpdateIndexCiBuildsOnCommitIdAndArtifactsExpireatandidpartial: migrating
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_builds, [:commit_id, :artifacts_expire_at, :id], {:name=>"index_ci_builds_on_commit_id_and_artifacts_expireatandidpart2", :where=>"type::text = 'Ci::Build'::text AND (retried = false OR retried IS NULL) AND\n               (name::text = ANY (ARRAY['sast'::character varying,\n               'secret_detection'::character varying,\n               'dependency_scanning'::character varying,\n               'container_scanning'::character varying,\n               'dast'::character varying]::text[]))", :algorithm=>:concurrently})
   -> 0.0092s
-- add_index(:ci_builds, [:commit_id, :artifacts_expire_at, :id], {:name=>"index_ci_builds_on_commit_id_and_artifacts_expireatandidpart2", :where=>"type::text = 'Ci::Build'::text AND (retried = false OR retried IS NULL) AND\n               (name::text = ANY (ARRAY['sast'::character varying,\n               'secret_detection'::character varying,\n               'dependency_scanning'::character varying,\n               'container_scanning'::character varying,\n               'dast'::character varying]::text[]))", :algorithm=>:concurrently})
   -> 0.0040s
-- transaction_open?()
   -> 0.0000s
-- indexes(:ci_builds)
   -> 0.0068s
-- remove_index(:ci_builds, {:algorithm=>:concurrently, :name=>"index_ci_builds_on_commit_id_and_artifacts_expireatandidpartial"})
   -> 0.0019s
== 20200519171058 UpdateIndexCiBuildsOnCommitIdAndArtifactsExpireatandidpartial: migrated (0.0225s)

➜  gitlab git:(update-index-artifacts-expire) ✗ bin/rails db:migrate:down VERSION=20200519171058  RAILS_ENV=development
WARNING: This version of GitLab depends on gitlab-shell 13.2.0, but you're running 12.2.0. Please update gitlab-shell.
== 20200519171058 UpdateIndexCiBuildsOnCommitIdAndArtifactsExpireatandidpartial: reverting
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_builds, [:commit_id, :artifacts_expire_at, :id], {:name=>"index_ci_builds_on_commit_id_and_artifacts_expireatandidpartial", :where=>"type::text = 'Ci::Build'::text AND (retried = false OR retried IS NULL) AND\n               (name::text = ANY (ARRAY['sast'::character varying,\n               'dependency_scanning'::character varying,\n               'sast:container'::character varying,\n               'container_scanning'::character varying,\n               'dast'::character varying]::text[]))", :algorithm=>:concurrently})
   -> 0.0097s
-- add_index(:ci_builds, [:commit_id, :artifacts_expire_at, :id], {:name=>"index_ci_builds_on_commit_id_and_artifacts_expireatandidpartial", :where=>"type::text = 'Ci::Build'::text AND (retried = false OR retried IS NULL) AND\n               (name::text = ANY (ARRAY['sast'::character varying,\n               'dependency_scanning'::character varying,\n               'sast:container'::character varying,\n               'container_scanning'::character varying,\n               'dast'::character varying]::text[]))", :algorithm=>:concurrently})
   -> 0.0037s
-- transaction_open?()
   -> 0.0000s
-- indexes(:ci_builds)
   -> 0.0064s
-- remove_index(:ci_builds, {:algorithm=>:concurrently, :name=>"index_ci_builds_on_commit_id_and_artifacts_expireatandidpart2"})
   -> 0.0018s
== 20200519171058 UpdateIndexCiBuildsOnCommitIdAndArtifactsExpireatandidpartial: reverted (0.0223s)

Edited May 25, 2020 by Tiger Watson

Update index for security CI builds