Add extra validation in corpus model
What does this MR do and why?
Adds the following validations:
- Package with package file.
- Package file in supported zip format.
- Unique package for corpuses.
How to set up and validate locally
1. package_with_package_file
project = Project.last.id
package1 = Packages::Package.create!(project_id: project, name: 'package3', package_type: 'generic', version: '1')
AppSec::Fuzzing::Coverage::Corpus.create!(package: package1, project: package1.project)
pry(main)> AppSec::Fuzzing::Coverage::Corpus.create!(package: package1, project: package1.project)
TRANSACTION (0.2ms) BEGIN /*application:console,db_config_name:main,line:/ee/app/models/app_sec/fuzzing/coverage/corpus.rb:51:in `first_package_file'*/
Packages::PackageFile Load (0.3ms) SELECT "packages_package_files".* FROM "packages_package_files" WHERE "packages_package_files"."package_id" = 39 ORDER BY "packages_package_files"."id" ASC LIMIT 1 /*application:console,db_config_name:main,line:/ee/app/models/app_sec/fuzzing/coverage/corpus.rb:51:in `first_package_file'*/
Packages::PackageFile Load (0.3ms) SELECT "packages_package_files".* FROM "packages_package_files" WHERE "packages_package_files"."package_id" = 39 ORDER BY "packages_package_files"."id" ASC LIMIT 1 /*application:console,db_config_name:main,line:/ee/app/models/app_sec/fuzzing/coverage/corpus.rb:51:in `first_package_file'*/
TRANSACTION (0.2ms) ROLLBACK /*application:console,db_config_name:main,line:/lib/gitlab/database.rb:312:in `rollback'*/
ActiveRecord::RecordInvalid: Validation failed: Package should have an associated package file
from /Users/aditya-work/.asdf/installs/ruby/2.7.4/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.1/lib/active_record/validations.rb:80:in `raise_validation_error'
[28] pry(main)>
2. validate_file_format
file = Packages::PackageFile.last
file.update!(file_name: 'test')
package1.package_files << file
pry(main)> AppSec::Fuzzing::Coverage::Corpus.create!(package: package1, project: package1.project)
ActiveRecord::RecordInvalid: Validation failed: Package format is not supported
3. unique package_id
package1.package_files.reload
file = Packages::PackageFile.last
file.update!(file_name: 'test.zip')
package1.package_files << file
[73] pry(main)> AppSec::Fuzzing::Coverage::Corpus.create!(package: package1, project: package1.project)
TRANSACTION (0.2ms) BEGIN /*application:console,db_config_name:main,line:(pry):85:in `__pry__'*/
AppSec::Fuzzing::Coverage::Corpus Create (0.5ms) INSERT INTO "coverage_fuzzing_corpuses" ("project_id", "package_id", "created_at", "updated_at") VALUES (20, 39, '2021-11-15 10:34:00.598980', '2021-11-15 10:34:00.598980') RETURNING "id" /*application:console,db_config_name:main,line:(pry):85:in `__pry__'*/
TRANSACTION (0.2ms) ROLLBACK /*application:console,db_config_name:main,line:/lib/gitlab/database.rb:312:in `rollback'*/
ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_coverage_fuzzing_corpuses_on_package_id"
DETAIL: Key (package_id)=(39) already exists.
Numbered steps to set up and validate the change are strongly suggested.
Database
╰─>$ rails db:migrate:down VERSION=20211112073413
== 20211112073413 ChangePackageIndexOnCorpus: reverting =======================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:coverage_fuzzing_corpuses, :package_id, {:name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently})
-> 0.0063s
-- execute("SET statement_timeout TO 0")
-> 0.0005s
-- remove_index(:coverage_fuzzing_corpuses, {:name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently, :column=>:package_id})
-> 0.0103s
-- execute("RESET statement_timeout")
-> 0.0007s
-- transaction_open?()
-> 0.0000s
-- index_exists?(:coverage_fuzzing_corpuses, :package_id, {:name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently})
-> 0.0016s
-- add_index(:coverage_fuzzing_corpuses, :package_id, {:name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently})
-> 0.0069s
== 20211112073413 ChangePackageIndexOnCorpus: reverted (0.0345s) ==============
╰─>$ rails db:migrate
== 20211112073413 ChangePackageIndexOnCorpus: migrating =======================
-- execute("DELETE FROM coverage_fuzzing_corpuses")
-> 0.0045s
-- index_exists?(:coverage_fuzzing_corpuses, :package_id, {:name=>"index_coverage_fuzzing_corpuses_on_package_id"})
-> 0.0041s
-- transaction_open?()
-> 0.0000s
-- index_exists?(:coverage_fuzzing_corpuses, :package_id, {:name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently})
-> 0.0019s
-- execute("SET statement_timeout TO 0")
-> 0.0006s
-- remove_index(:coverage_fuzzing_corpuses, {:name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently, :column=>:package_id})
-> 0.0045s
-- execute("RESET statement_timeout")
-> 0.0005s
-- transaction_open?()
-> 0.0000s
-- index_exists?(:coverage_fuzzing_corpuses, :package_id, {:unique=>true, :name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently})
-> 0.0013s
-- add_index(:coverage_fuzzing_corpuses, :package_id, {:unique=>true, :name=>"index_coverage_fuzzing_corpuses_on_package_id", :algorithm=>:concurrently})
-> 0.0026s
== 20211112073413 ChangePackageIndexOnCorpus: migrated (0.0230s) ==============
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #345454 (closed)
Merge request reports
Activity
changed milestone to %14.6
assigned to @atiwari71
- A deleted user
added database databasereview pending labels
6 Warnings 
525e00a8: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. dc57c24c: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. f24e7aa7: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. b328a290: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. 93576933: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. Please add a merge request type to this merge request. Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Amparo Luna ( @a_luna) (UTC-5)Doug Stull ( @dstull) (UTC-5)database Vitali Tatarintev ( @ck3g) (UTC+1)Mayra Cabrera ( @mayra-cabrera) (UTC-6)~migration No reviewer available No maintainer available To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-reviewjob that generated this comment.Generated by
Resolved by Robert Speicher@philipcunningham Can you give this a first look?
- Last reply by Philip Cunningham
requested review from @philipcunningham
removed review request for @philipcunningham
@philipcunningham, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
requested review from @rspeicher
@atiwari71 Approving backend but it looks like this is still databasereview pending.
- Resolved by Mayra Cabrera
@stomlinson Please review the db changes.
Edited by Aditya Tiwari - Last reply by Aditya Tiwari
requested review from @stomlinson
-
Database migrations
Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).
Migration Type Total runtime Result DB size change 20211112073413 - ChangePackageIndexOnCorpus Regular 2.5 s 
+0.00 B Runtime Histogram for all migrations
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 4 0.1 seconds - 1 second 1 1 second - 5 minutes 0 5 minutes + 0 Migration: 20211112073413 - ChangePackageIndexOnCorpus
- Type: Regular
- Duration: 2.5 s
- Database size change: +0.00 B
Query Calls Total Time Max Time Mean Time Rows CREATE UNIQUE INDEX CONCURRENTLY "index_coverage_fuzzing_corpuses_on_package_id" ON "coverage_fuzzing_corpuses" ("package_id") /*application:test,db_config_name:main*/1 71.1 ms 71.1 ms 71.1 ms 0 SELECT "feature_gates".*
FROM "feature_gates"
WHERE "feature_gates"."feature_key" = $1 /*application:test,db_config_name:main*/1 6.4 ms 6.4 ms 6.4 ms 1 DROP INDEX CONCURRENTLY "index_coverage_fuzzing_corpuses_on_package_id" /*application:test,db_config_name:main*/
1 5.6 ms 5.6 ms 5.6 ms 0 SELECT "postgres_async_indexes".*
FROM "postgres_async_indexes"
WHERE "postgres_async_indexes"."name" = $1
LIMIT $2 /*application:test,db_config_name:main*/2 0.1 ms 0.1 ms 0.0 ms 0 Histogram for ChangePackageIndexOnCorpus
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 4 0.1 seconds - 1 second 1 1 second - 5 minutes 0 5 minutes + 0
Other migrations pending on GitLab.com
Migration Type Total runtime Result DB size change Clone Details
Clone ID Clone Created At Clone Data Timestamp Expected Removal Time database-testing-9409322021-12-15 20:30:20 UTC 2021-12-15 20:00:52 UTC 2021-12-16 08:31:39 +0000 Artifacts
Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic
- A deleted user
added database-testing-automation label
removed review request for @stomlinson
requested review from @stomlinson
added databasereviewed label and removed databasereview pending label
requested review from @mayra-cabrera and removed review request for @stomlinson
removed review request for @mayra-cabrera
requested review from @mayra-cabrera
removed review request for @mayra-cabrera
added 1075 commits
-
8fcf2680...596fe508 - 1067 commits from branch
master - cf08535b - Add validation for zip file format
- 3956c62e - Add unique constraint on corpus package_id
- 93576933 - Fixes spec
- b328a290 - Fixes spec
- f24e7aa7 - Add validations
- cb8ff21e - Apply 1 suggestion(s) to 1 file(s)
- dc57c24c - Add validations
- 525e00a8 - Fixes validation
Toggle commit list-
8fcf2680...596fe508 - 1067 commits from branch
mentioned in issue #348401
requested review from @mayra-cabrera
added databaseapproved label and removed databasereviewed label
enabled an automatic merge when the pipeline for c4e18f0b succeeds
-
Thanks @atiwari71! LGTM, MWPS set since have all the approvals
mentioned in commit 1af74623
added workflowstaging-canary label
added workflowstaging label and removed workflowstaging-canary label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label
mentioned in issue #345454 (closed)
mentioned in issue gitlab-com/www-gitlab-com#11934 (closed)
