Fix GraphQL pipeline findings pagination

What does this MR do and why?

Fix GraphQL pagination on the pipeline.securityReportFindings resolver.

This requires some fairly in depth reworking explained below:

Rework of SecurityFindingsFinder#limit calculation

This finder was implemented for a REST endpoint where pagination is implemented with page and per_page parameters. It uses offset pagination currently (conversion to keyset is planned). Because of this the pagination boundaries (OFFSET) always fall on equal divisions of per_page size.

                     REST pagination

      page:1 per_page:10           page:2 per_page:10
|----------------------------|-----------------------------|----
 1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21 ..

The GraphQL cursor based pagination can use arbitrary offsets using first or last arguments to specify page size and before or after to determine the offset

                     GQL pagination

                         first:12 after:5
               |----------------------------------|
 1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21 ..

The Security::FindingsFinder has undergone extensive customization over time to make it perform acceptably with large data sets. To facilitate this the finder needs to know some information about the current pagination position. Previously it was using the page and per_page parameters from the REST endpoint to determine this, but with this change it has needed to be modified.

Getting pagination information from the GQL resolver

The default GraphQL connection implementation removes the pagination arguments before calling resolve, so the pagination params are not available by default. Using connection_extension: ::Gitlab::Graphql::Extensions::ExternallyPaginatedArrayExtension works around this.

Once these arguments are available they require some manipulation to read. The first and last arguments are plain integers, but before and after are base64 encoded strings. Usually they contain encoded JSON with the keyset pagination parameters, but when using offset_paginate before and after contain the base64 encoded first and last id of the returned record set.

Steps to reproduce

Ensure you have EE features and pipeline runners available on your GDK.

• git switch master
• Clone https://gitlab.com/gl-demo-ultimate-myacksmith/482983-test-group/graphql-vulnerability-findings-test into your GDK
• In rails console set the feature flag to use GraphQL instead of REST on the pipeline security tab Feature.enable(:pipeline_security_dashboard_graphql)
• Once the project's pipeline has completed, visit 'Pipeline -> Security'
• The 'Scan details' section should show a count of 164 vulnerabilities'
• Paginate through the vulnerability list. The full list of findings is not shown.
• git switch 441306-security-report-finding-pagination
• You should now be able to paginate through all of the findings.

Related to #441306 (closed)

changed milestone to %16.10

added Category:Software Bill of Materials customer devopsgovern groupthreat insights priority2 sectionsec severity2 typebug workflowin dev labels

assigned to @mallocke

added backend database databasereview pending labels

	1 Warning
⚠	45a5c795: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.

	1 Message
📖	CHANGELOG missing: If this merge request needs a changelog entry, add the Changelog trailer to the commit message you want to add to the changelog. If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

Reviewer roulette

Category	Reviewer	Maintainer
backend	@jwanjohi (UTC+0, 13 hours behind author)	@jagood (UTC-4, 17 hours behind author)
database	@terrichu (UTC-4, 17 hours behind author)	@DylanGriffith (UTC-5, 18 hours behind author)
test for spec/features/*	@hmuralidhar (UTC+11, 2 hours behind author)	Maintainer review is optional for test for spec/features/*
~"Verify"	Reviewer review is optional for ~"Verify"	@grzesiek (UTC+1, 12 hours behind author)

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

mentioned in issue #441306 (closed)

added 1930 commits

• 9d62e490...c3e3cb78 - 1929 commits from branch master
• 95ff7d75 - Fix GraphQL pipeline findings pagination

Compare with previous version

added 1 commit

• 602c873e - Fix vuln findings pagination tests

Compare with previous version

added 2 commits

• 99a48452 - Fix GraphQL pipeline findings pagination
• 0c232ade - Fix vuln findings pagination tests

Compare with previous version

marked this merge request as ready

@subashis would you be able to do an initial backend review on this?

@bhrai would you mind doing an initial database review on ee/app/finders/security/findings_finder.rb? The queries are unchanged by this work.

@alberts-gitlab would you mind doing a review for Verify?

requested review from @subashis, @bhrai, and @alberts-gitlab

approved this merge request

requested review from @jon_jenkins

added pipeline:mr-approved label

👋 @bhrai, 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 databasereviewed label and removed databasereview pending label

E2E Test Result Summary

allure-report-publisher generated test report!

e2e-test-on-gdk: ✅ test report for 62a313c8

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Govern      | 66     | 0      | 0       | 0     | 66    | ✅     |
| Monitor     | 4      | 0      | 0       | 0     | 4     | ✅     |
| Create      | 8      | 0      | 3       | 0     | 11    | ✅     |
| Package     | 0      | 0      | 1       | 0     | 1     | ➖     |
| Data Stores | 2      | 0      | 0       | 0     | 2     | ✅     |
| Plan        | 4      | 0      | 0       | 0     | 4     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 84     | 0      | 4       | 0     | 88    | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

e2e-package-and-test: ✅ test report for 62a313c8

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Govern      | 272    | 0      | 19      | 0     | 291   | ✅     |
| Create      | 153    | 0      | 20      | 0     | 173   | ✅     |
| Plan        | 8      | 0      | 0       | 0     | 8     | ✅     |
| Package     | 0      | 0      | 2       | 0     | 2     | ➖     |
| Monitor     | 8      | 0      | 0       | 0     | 8     | ✅     |
| Data Stores | 4      | 0      | 0       | 0     | 4     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 445    | 0      | 41      | 0     | 486   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

added 665 commits

• 0c232ade...70239352 - 663 commits from branch master
• 2ee9efd0 - Fix GraphQL pipeline findings pagination
• ab9023f9 - Fix vuln findings pagination tests

Compare with previous version

added 2 commits

• 45a5c795 - Fix GraphQL pipeline findings pagination
• 979b0c79 - Fix vuln findings pagination tests

Compare with previous version

requested review from @minac

removed review request for @subashis

changed the description

approved this merge request

removed review request for @alberts-gitlab

approved this merge request

removed review request for @minac

requested review from @eugielimpin

requested review from @mattkasa

approved this merge request

added databaseapproved label and removed databasereviewed label

mentioned in issue #328818 (closed)

approved this merge request

removed review request for @eugielimpin

Appreciate all the notes you left @mallocke. Really helped me understand the changes in this MR

added 1 commit

• 77a2459a - Apply reviewer feedback fixes

Compare with previous version

reset approvals from @mattkasa, @minac, @alberts-gitlab, and @eugielimpin by pushing to the branch

requested review from @eugielimpin

added 1 commit

• 62a313c8 - Add more robust handling of pagination args

Compare with previous version

approved this merge request

resolved all threads

@alberts-gitlab Could you please re-approve for Verify?

requested review from @alberts-gitlab

resolved all threads

approved this merge request

approved this merge request

resolved all threads

enabled an automatic merge when the pipeline for df9f4840 succeeds

merged

Hello @mallocke 👋

The database team is looking for ways to improve the database review process and we would love your help!

If you'd be open to someone on the database team reaching out to you for a chat, or if you'd like to leave some feedback asynchronously, just post a reply to this comment mentioning:

@gitlab-org/database-team

And someone will be by shortly!

Thanks for your help! ❤

This message was generated automatically. You're welcome to improve it.

mentioned in commit 07897be2

mentioned in incident gitlab-org/quality/engineering-productivity/master-broken-incidents#5568 (closed)

added workflowstaging-canary label and removed workflowin dev label

added workflowcanary label and removed workflowstaging-canary label

added workflowstaging label and removed workflowcanary label

added workflowproduction label and removed workflowstaging label

added workflowpost-deploy-db-staging label and removed workflowproduction label

added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label

added releasedcandidate label

mentioned in issue #361152 (closed)

mentioned in issue #460494 (closed)

added releasedpublished label and removed releasedcandidate label

added pipelinetier-3 label