Use Build relations for DAST on-demand variables (!64882) · Merge requests · GitLab.org / GitLab

Philip Cunningham requested to merge philipcunningham-switch-over-to-build-associations-for-dast-on-demand-332942 into master Jun 28, 2021

What does this MR do?

this merge request switches over dast on-demand variable delivery to use the associations at the ci_build-level rather than at the ci_pipeline-level by using the new dast_configuration ci yaml keyword.

Notes

dast is a low traffic feature and there are currently 258 rows in dast_profiles_pipelines on gitlab.com.

Related Issue(s)

Why?

whilst refining Use DAST Scan profiles from database we learned that we really wanted to model variable delivery to dast on-demand to be at the ci_build level. this is because it may be the case that a user has multiple dast jobs per pipeline. this change brings these into alignment by keeping the dast_profile as the ci_pipeline trigger whilst sharing code between contexts.

Database

Migration

% rails db:migrate:up VERSION=20210629031900 && rails db:migrate:down VERSION=20210629031900                                                                                                                                                                                                                                                                                    
== 20210629031900 AssociateExistingDastBuildsWithVariables: migrating =========
WARNING: Active Record does not support composite primary key.

dast_profiles_pipelines has composite primary key. Composite primary key is ignored.
== 20210629031900 AssociateExistingDastBuildsWithVariables: migrated (0.0861s)

== 20210629031900 AssociateExistingDastBuildsWithVariables: reverting =========
WARNING: Active Record does not support composite primary key.

dast_profiles_pipelines has composite primary key. Composite primary key is ignored.
WARNING: Active Record does not support composite primary key.

dast_site_profiles_builds has composite primary key. Composite primary key is ignored.
== 20210629031900 AssociateExistingDastBuildsWithVariables: reverted (0.0323s)

SQL

Up

[5] pry(main)> require Rails.root.join('db', 'migrate', '20210629031900_associate_existing_dast_builds_with_variables.rb')
=> true
[6] pry(main)> m = AssociateExistingDastBuildsWithVariables.new
=> #<AssociateExistingDastBuildsWithVariables:0x00007fde9fdd5128 @connection=nil, @name="AssociateExistingDastBuildsWithVariables", @version=nil>
[7] pry(main)> m.up
  AssociateExistingDastBuildsWithVariables::ProfilesPipeline Load (1.6ms)  SELECT "dast_profiles_pipelines"."ci_pipeline_id" FROM "dast_profiles_pipelines" ORDER BY "dast_profiles_pipelines"."ci_pipeline_id" ASC LIMIT 1 /*application:console,line:/app/models/concerns/each_batch.rb:61:in `each_batch'*/
WARNING: Active Record does not support composite primary key.

dast_profiles_pipelines has composite primary key. Composite primary key is ignored.
  AssociateExistingDastBuildsWithVariables::ProfilesPipeline Load (0.5ms)  SELECT "dast_profiles_pipelines"."ci_pipeline_id" FROM "dast_profiles_pipelines" WHERE "dast_profiles_pipelines"."ci_pipeline_id" >= 3 ORDER BY "dast_profiles_pipelines"."ci_pipeline_id" ASC LIMIT 1 OFFSET 300 /*application:console,line:/app/models/concerns/each_batch.rb:80:in `block in each_batch'*/
   (0.3ms)  SELECT "dast_profiles_pipelines"."ci_pipeline_id" FROM "dast_profiles_pipelines" WHERE "dast_profiles_pipelines"."ci_pipeline_id" >= 3 /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:69:in `fetch_builds'*/
  AssociateExistingDastBuildsWithVariables::Build Load (0.7ms)  SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."name" = 'dast' AND "ci_builds"."stage" = 'dast' AND "ci_builds"."commit_id" IN (3, 11, 13, 14, 15, 29, 30, 31, 68, 80) /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:38:in `group_by'*/
  AssociateExistingDastBuildsWithVariables::ProfilesPipeline Load (0.3ms)  SELECT "dast_profiles_pipelines".* FROM "dast_profiles_pipelines" WHERE "dast_profiles_pipelines"."ci_pipeline_id" >= 3 /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:40:in `block in up'*/
  AssociateExistingDastBuildsWithVariables::Profile Load (1.0ms)  SELECT "dast_profiles".* FROM "dast_profiles" WHERE "dast_profiles"."id" IN (2, 1, 5, 6, 7, 8) /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:40:in `block in up'*/
  AssociateExistingDastBuildsWithVariables::SiteProfilesBuild Bulk Insert (2.2ms)  INSERT INTO "dast_site_profiles_builds" ("dast_site_profile_id","ci_build_id") VALUES (2, 3), (1, 12), (1, 14), (1, 15), (1, 16), (6, 25), (6, 26), (6, 27), (6, 28), (12, 122) ON CONFLICT ("ci_build_id") DO NOTHING RETURNING "dast_site_profile_id","ci_build_id" /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:50:in `block in up'*/
=> nil

Down

[8] pry(main)> m.down
  AssociateExistingDastBuildsWithVariables::ProfilesPipeline Load (0.5ms)  SELECT "dast_profiles_pipelines"."ci_pipeline_id" FROM "dast_profiles_pipelines" ORDER BY "dast_profiles_pipelines"."ci_pipeline_id" ASC LIMIT 1 /*application:console,line:/app/models/concerns/each_batch.rb:61:in `each_batch'*/
  AssociateExistingDastBuildsWithVariables::ProfilesPipeline Load (0.4ms)  SELECT "dast_profiles_pipelines"."ci_pipeline_id" FROM "dast_profiles_pipelines" WHERE "dast_profiles_pipelines"."ci_pipeline_id" >= 3 ORDER BY "dast_profiles_pipelines"."ci_pipeline_id" ASC LIMIT 1 OFFSET 300 /*application:console,line:/app/models/concerns/each_batch.rb:80:in `block in each_batch'*/
   (0.4ms)  SELECT "dast_profiles_pipelines"."ci_pipeline_id" FROM "dast_profiles_pipelines" WHERE "dast_profiles_pipelines"."ci_pipeline_id" >= 3 /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:69:in `fetch_builds'*/
WARNING: Active Record does not support composite primary key.

dast_site_profiles_builds has composite primary key. Composite primary key is ignored.
  AssociateExistingDastBuildsWithVariables::SiteProfilesBuild Destroy (4.8ms)  DELETE FROM "dast_site_profiles_builds" WHERE "dast_site_profiles_builds"."ci_build_id" IN (SELECT "ci_builds"."id" FROM "ci_builds" WHERE "ci_builds"."name" = 'dast' AND "ci_builds"."stage" = 'dast' AND "ci_builds"."commit_id" IN (3, 11, 13, 14, 15, 29, 30, 31, 68, 80)) /*application:console,line:/db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb:60:in `block in down'*/
=> nil

Screenshots

On-Demand

[9] pry(main)> Ci::Pipeline.find(87).tap { |pipeline| puts pipeline.builds.map { |build| [pipeline.dast_profile.id, build.dast_site_profile.id, build.dast_scanner_profile.id] }.inspect }
  Ci::Pipeline Load (0.9ms)  SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 87 LIMIT 1 /*application:console,line:(pry):9:in `__pry__'*/
  Ci::Build Load (0.7ms)  SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."type" = 'Ci::Build' AND "ci_builds"."commit_id" = 87 /*application:console,line:(pry):9:in `map'*/
  Dast::Profile Load (0.5ms)  SELECT "dast_profiles".* FROM "dast_profiles" INNER JOIN "dast_profiles_pipelines" ON "dast_profiles"."id" = "dast_profiles_pipelines"."dast_profile_id" WHERE "dast_profiles_pipelines"."ci_pipeline_id" = 87 LIMIT 1 /*application:console,line:(pry):9:in `block (2 levels) in __pry__'*/
  DastSiteProfile Load (0.3ms)  SELECT "dast_site_profiles".* FROM "dast_site_profiles" INNER JOIN "dast_site_profiles_builds" ON "dast_site_profiles"."id" = "dast_site_profiles_builds"."dast_site_profile_id" WHERE "dast_site_profiles_builds"."ci_build_id" = 140 LIMIT 1 /*application:console,line:(pry):9:in `block (2 levels) in __pry__'*/
  DastScannerProfile Load (0.3ms)  SELECT "dast_scanner_profiles".* FROM "dast_scanner_profiles" INNER JOIN "dast_scanner_profiles_builds" ON "dast_scanner_profiles"."id" = "dast_scanner_profiles_builds"."dast_scanner_profile_id" WHERE "dast_scanner_profiles_builds"."ci_build_id" = 140 LIMIT 1 /*application:console,line:(pry):9:in `block (2 levels) in __pry__'*/
[[1, 1, 1]]

CI Keyword

[12] pry(main)> Ci::Pipeline.find(88).tap { |pipeline| puts pipeline.builds.map { |build| [pipeline.dast_profile&.id, build.dast_site_profile.id, build.dast_scanner_profile.id] }.inspect }
  Ci::Pipeline Load (0.5ms)  SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 88 LIMIT 1 /*application:console,line:(pry):12:in `__pry__'*/
  Ci::Build Load (0.6ms)  SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."type" = 'Ci::Build' AND "ci_builds"."commit_id" = 88 /*application:console,line:(pry):12:in `map'*/
  Dast::Profile Load (0.5ms)  SELECT "dast_profiles".* FROM "dast_profiles" INNER JOIN "dast_profiles_pipelines" ON "dast_profiles"."id" = "dast_profiles_pipelines"."dast_profile_id" WHERE "dast_profiles_pipelines"."ci_pipeline_id" = 88 LIMIT 1 /*application:console,line:(pry):12:in `block (2 levels) in __pry__'*/
  DastSiteProfile Load (0.5ms)  SELECT "dast_site_profiles".* FROM "dast_site_profiles" INNER JOIN "dast_site_profiles_builds" ON "dast_site_profiles"."id" = "dast_site_profiles_builds"."dast_site_profile_id" WHERE "dast_site_profiles_builds"."ci_build_id" = 141 LIMIT 1 /*application:console,line:(pry):12:in `block (2 levels) in __pry__'*/
  DastScannerProfile Load (0.5ms)  SELECT "dast_scanner_profiles".* FROM "dast_scanner_profiles" INNER JOIN "dast_scanner_profiles_builds" ON "dast_scanner_profiles"."id" = "dast_scanner_profiles_builds"."dast_scanner_profile_id" WHERE "dast_scanner_profiles_builds"."ci_build_id" = 141 LIMIT 1 /*application:console,line:(pry):12:in `block (2 levels) in __pry__'*/
[[nil, 1, 4]]

Does this MR meet the acceptance criteria?

Conformity

I have included changelog trailers, or none are needed. (Does this MR need a changelog?)
I have added/updated documentation, or it's not needed. (Is documentation required?)
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?)
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?)
I have self-reviewed this MR per code review guidelines.
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines)
I have followed the style guides.
This change is backwards compatible across updates, or this does not apply.

Availability and Testing

I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.)
I have tested this MR in all supported browsers, or it's not needed.
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

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 Aug 04, 2021 by Philip Cunningham

Use Build relations for DAST on-demand variables