Adding Geo support of Project-level Secure Files
What does this MR do and why?
Following the process described in #365595 (closed), this MR adds Geo-replication support for Project-level Secure Files.
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Project-level Secure Files can be added to a project by a maintainer by going to the Settings > CI/CD. Scroll to the bottom of the page and expand the Secure Files section. Click the Upload File button, and upload any file smaller than 5 MB.
Docs: https://docs.gitlab.com/ee/ci/secure_files/#add-a-secure-file-to-a-project
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.
Merge request reports
Activity
changed milestone to %15.2
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Suggested Reviewers @mkozono
,@mayra-cabrera
,@aakriti.gupta
,@ahegyi
,@alipniagov
If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
6 Warnings This merge request is quite big (528 lines changed), please consider splitting it into multiple merge requests. ea546a28: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. 63701295: 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. bbe82fc5: 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. aeee7a41: 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. 63608bc3: 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. 2 Messages This merge request adds or changes files that require a review from the Database team. This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
ee/app/finders/geo/ci_secure_file_registry_finder.rb
Documentation review
The following files require a review from a technical writer:
doc/administration/monitoring/prometheus/gitlab_metrics.md
doc/api/graphql/reference/index.md
doc/api/geo_nodes.md
The review does not need to block merging this merge request. See the:
-
Metadata for the
*.md
files that you've changed. The first few lines of each*.md
file identify the stage and group most closely associated with your docs change. - The Technical Writer assigned for that stage and group.
- Documentation workflows for information on when to assign a merge request for review.
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 Brian Williams ( @bwill
) (UTC-5)Jan Provaznik ( @jprovaznik
) (UTC+2)database Eulyeon K. ( @euko
) (UTC+9)Tiger Watson ( @tigerwnz
) (UTC+10)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-review
job that generated this comment.Generated by
Dangermentioned in commit e0f3809c
added 1 commit
- e0f3809c - Adding Geo support of Project-level Secure Files
assigned to @darbyfrey
mentioned in commit b1413a3a
added 1 commit
- b1413a3a - Adding Geo support of Project-level Secure Files
Allure report
allure-report-publisher
generated test report!package-and-qa-ff-disabled:
test report for bf0fbfe6expand test summary
+---------------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +----------------------+--------+--------+---------+-------+-------+--------+ | Manage | 330 | 0 | 9 | 17 | 339 | ❗ | | Create | 444 | 0 | 18 | 1 | 462 | ❗ | | Verify | 120 | 0 | 9 | 0 | 129 | ✅ | | Fulfillment | 6 | 0 | 33 | 0 | 39 | ✅ | | Plan | 162 | 0 | 0 | 0 | 162 | ✅ | | Product Intelligence | 6 | 0 | 0 | 0 | 6 | ✅ | | Package | 0 | 0 | 9 | 0 | 9 | ➖ | | Protect | 6 | 0 | 0 | 0 | 6 | ✅ | | Secure | 63 | 0 | 6 | 3 | 69 | ❗ | | Configure | 0 | 0 | 9 | 0 | 9 | ➖ | | Release | 15 | 0 | 0 | 0 | 15 | ✅ | | Version sanity check | 0 | 0 | 3 | 0 | 3 | ➖ | +----------------------+--------+--------+---------+-------+-------+--------+ | Total | 1152 | 0 | 96 | 21 | 1248 | ❗ | +----------------------+--------+--------+---------+-------+-------+--------+
package-and-qa-ff-enabled:
test report for bf0fbfe6expand test summary
+---------------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +----------------------+--------+--------+---------+-------+-------+--------+ | Create | 444 | 0 | 18 | 3 | 462 | ❗ | | Verify | 120 | 0 | 9 | 0 | 129 | ✅ | | Secure | 62 | 1 | 6 | 12 | 69 | ❌ | | Plan | 162 | 0 | 0 | 0 | 162 | ✅ | | Manage | 330 | 0 | 9 | 37 | 339 | ❗ | | Fulfillment | 6 | 0 | 33 | 0 | 39 | ✅ | | Release | 15 | 0 | 0 | 3 | 15 | ❗ | | Package | 0 | 0 | 9 | 0 | 9 | ➖ | | Configure | 0 | 0 | 9 | 0 | 9 | ➖ | | Product Intelligence | 6 | 0 | 0 | 0 | 6 | ✅ | | Version sanity check | 0 | 0 | 3 | 0 | 3 | ➖ | | Protect | 6 | 0 | 0 | 0 | 6 | ✅ | +----------------------+--------+--------+---------+-------+-------+--------+ | Total | 1151 | 1 | 96 | 55 | 1248 | ❌ | +----------------------+--------+--------+---------+-------+-------+--------+
review-qa-blocking:
test report for bf0fbfe6expand test summary
+-----------------------------------------------------------------------------------------+ | suites summary | +------------------------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Protect | 2 | 0 | 0 | 2 | 2 | ❗ | | Create | 23 | 0 | 2 | 23 | 25 | ❗ | | Plan | 47 | 0 | 1 | 47 | 48 | ❗ | | Manage | 38 | 0 | 2 | 40 | 40 | ❗ | | Verify | 12 | 0 | 1 | 12 | 13 | ❗ | | Secure | 2 | 0 | 0 | 2 | 2 | ❗ | | Configure | 0 | 0 | 1 | 0 | 1 | ➖ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | 1 | ➖ | | Feature flag handler sanity checks | 9 | 0 | 0 | 9 | 9 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Total | 133 | 0 | 9 | 135 | 142 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+
- The
gitlab-qa-mirror
downstream pipeline for !91430 (b1413a3a) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (b1413a3a) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (614e9ef1) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (614e9ef1) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (0a8aa8ca) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (0a8aa8ca) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (07d377a7) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (07d377a7) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (ce236896) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (ce236896) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (2280c7e2) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (2280c7e2) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (cdcfaea1) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (cdcfaea1) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (8eb89c74) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (b8989617) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (b8989617) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (b8989617) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (8edc4dcd) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (8edc4dcd) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (8edc4dcd) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (bf0fbfe6) failed! - The
gitlab-qa-mirror
downstream pipeline for !91430 (bf0fbfe6) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (bf0fbfe6) passed. - The
gitlab-qa-mirror
downstream pipeline for !91430 (bf0fbfe6) failed!
- The
mentioned in issue #349893 (closed)
mentioned in merge request gitlab-com/www-gitlab-com!107655 (merged)
mentioned in commit 614e9ef1
added 1653 commits
-
b1413a3a...6c2de6d5 - 1652 commits from branch
master
- 614e9ef1 - Adding Geo support of Project-level Secure Files
-
b1413a3a...6c2de6d5 - 1652 commits from branch
- A deleted user
added backend documentation feature flag labels
requested review from @dbalexandre
- Resolved by Douglas Barbosa Alexandre
Hi @dbalexandre, thanks for offering to help with this. My initial questions are around:
-
replicables_for_current_secondary
- I'm not quite sure what this method needs to do. I've looked at several examples in the code but am still a bit confused. Perhaps I missed the explanation somewhere, but I think some additional details would help me understand what this should do and how to test it. - I've added the feature flags as directed in #365595 (closed), but I'm not clear how those should be wired up. Do I need to do that? Or is that already done in the supporting libraries?
Thanks!
-
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
removed review request for @dbalexandre
added 1 commit
- 0a8aa8ca - Adding code and tests for replicables_for_current_secondary
added 1 commit
- 07d377a7 - Adding code and tests for replicables_for_current_secondary
added 1 commit
- ce236896 - Renaming SecureFileState back to CiSecureFileState
requested review from @dbalexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
changed milestone to %15.3
removed review request for @dbalexandre
added 2 commits
- A deleted user
added database databasereview pending labels
requested review from @dbalexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
- Resolved by Douglas Barbosa Alexandre
removed review request for @dbalexandre
requested review from @dbalexandre
mentioned in commit 4d5f51b1
added 1638 commits
-
b8989617...4665aeb1 - 1629 commits from branch
master
- 4d5f51b1 - Adding Geo support of Project-level Secure Files
- 7c3c0cb4 - Adding code and tests for replicables_for_current_secondary
- be64f53d - Renaming SecureFileState back to CiSecureFileState
- dd247cfe - Fixing test failures
- 7b549de9 - Several clean up items
- acf8d4da - Adding metrics gathering
- 3bfe9f69 - Implementing GraphQL API
- 1646a094 - Spec improvements
- 8edc4dcd - Updated GraphQL docs
Toggle commit list-
b8989617...4665aeb1 - 1629 commits from branch
- Resolved by Douglas Barbosa Alexandre
Thanks, @darbyfrey! This LGTM! I triggered a new pipeline and set MWPS
enabled an automatic merge when the pipeline for 792bca34 succeeds
@dbalexandre
, 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:
- Resolved by Douglas Barbosa Alexandre
Failures in https://gitlab.com/gitlab-org/gitlab/-/pipelines/594592256 are due to a master:broken. See #368628 (closed).
mentioned in commit 3bd7723b
added 75 commits
-
8edc4dcd...db2b9dc3 - 66 commits from branch
master
- 3bd7723b - Adding Geo support of Project-level Secure Files
- 63608bc3 - Adding code and tests for replicables_for_current_secondary
- 4c70e6d3 - Renaming SecureFileState back to CiSecureFileState
- aeee7a41 - Fixing test failures
- 778c3f3e - Several clean up items
- bbe82fc5 - Adding metrics gathering
- 63701295 - Implementing GraphQL API
- ea546a28 - Spec improvements
- bf0fbfe6 - Updated GraphQL docs
Toggle commit list-
8edc4dcd...db2b9dc3 - 66 commits from branch
enabled an automatic merge when the pipeline for 5d3ac9d4 succeeds
mentioned in commit 39b8a2a5
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
mentioned in issue #368805 (closed)
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
added releasedpublished label and removed releasedcandidate label
mentioned in merge request kubitus-project/kubitus-installer!1303 (merged)
mentioned in issue #362561 (closed)
mentioned in issue #362564 (closed)
mentioned in issue gitlab-org/quality/triage-reports#20624 (closed)
mentioned in merge request !172163 (closed)
mentioned in issue gitlab-org/quality/triage-reports#20981 (closed)
mentioned in issue gitlab-org/quality/triage-reports#21533 (closed)
mentioned in issue gitlab-org/quality/triage-reports#22038