Andy Schoenen (6685a447) at 28 Mar 20:14
Merge branch '389765-add-zoekt_settings-in-application_settings' in...
... and 1 more commit
Add a new column zoekt_settings
of the type jsonb
in the application_settings
table. This jsonb
column can have three attributes.
This column can be used to show the settings in the admin panel and eventually, few zoekt
feature flags can be deprecated.
All three attributes have the default value of false
. In the follow-up MRs the backfilling of this column will happen.
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
git fetch && git checkout 389765-add-zoekt_settings-in-application_settings && bin/rails db:migrate
bin/rails c
ApplicationSetting
record. The value of zoekt_settings
should be {}
ApplicationSetting.last.zoekt_settings
ApplicationSetting
record. The value of zoekt_settings
should be {"zoekt_indexing_enabled"=>false, "zoekt_indexing_paused"=>false, "zoekt_search_enabled"=>false}
ApplicationSetting.new.zoekt_settings
Related to #389765
It's hard to specify hard requirements for becoming a maintainer, which is why the documentation consists of flexible guidelines. Reviewers are encouraged to think of their eligibility for maintainership in the terms of "I could be ready at any time to be a maintainer as long as my work as an author and reviewer is consistent with other maintainers".
Marcos is outstanding reviewer with a long history of great reviews. From my perspective I always see that Marcos focuses on quality, executes and reproduces verification steps and asks clarifying questions when needed. I think this puts him in a good place to become a maintainer for backend MRs at GitLab.
SPECIALITY Maintainers, please review this proposal to make TRAINEE maintainer of PROJECT.
* If you have blocking feedback adhering [to the documentation](https://about.gitlab.com/handbook/engineering/workflow/code-review/#how-to-become-a-project-maintainer) please share it with me via Slack.
* If you are in agreement, and can vouch for this proposal, please approve.
After 1 week, if there is no blocking feedback and at least 2 approvals, I will merge this MR.
#backend_maintainers
Slack channel@gitlab-org/maintainers/rails-backend
with Owner
access level.Thanks! Looks great. Enabling auto-merge
Add desired sharding keys for feature category geo_replication
.
These tables have been identified as a cell local tables. All cell local tables require a sharding key or a desired sharding key
A desired sharding key has been automatically selected for these tables.
These keys were chosen as the desired sharding keys because the
table has a :belongs_to relationship to a table that itself has a NOT NULL
sharding key.
Additionally, gitlab_schema
has been set to gitlab_main_cell
for any tables didn't use this schema already.
For these tables we have also added allow_cross_joins
, allow_cross_transactions
and
allow_cross_foreign_keys
. These will silence any existing violations, allowing the pipeline to pass without
requiring further changes. In the future, we'll remove these allow_...
statements and fix any violations as
they arise. You can read more about this in the documentation for multiple databases
We have assigned a random backend engineer from groupgeo to review these changes.
Please confirm that:
When you are finished, please request a review from the database maintainer suggested by Danger. If you have any questions or concerns, reach out to @tigerwnz or @manojmj.
If you would like to go through similar merged MRs so as to gather an understanding on this topic, you can use this link.
This change was generated by gitlab-housekeeper using the Keeps::DetermineDesiredShardingKeyFeatureCategory keep.
To provide feedback on your experience with gitlab-housekeeper
please comment in
#442003.
Andy Schoenen (5b1ca84b) at 28 Mar 15:55
Merge branch 'determine-desired-sharding-key-feature-category-servi...
... and 1 more commit
Andy Schoenen (5b1ca84b) at 28 Mar 15:54
Merge branch 'determine-desired-sharding-key-feature-category-servi...
... and 1 more commit
Add desired sharding keys for feature category service_desk
.
These tables have been identified as a cell local tables. All cell local tables require a sharding key or a desired sharding key
A desired sharding key has been automatically selected for these tables.
These keys were chosen as the desired sharding keys because the
table has a :belongs_to relationship to a table that itself has a NOT NULL
sharding key.
Additionally, gitlab_schema
has been set to gitlab_main_cell
for any tables didn't use this schema already.
For these tables we have also added allow_cross_joins
, allow_cross_transactions
and
allow_cross_foreign_keys
. These will silence any existing violations, allowing the pipeline to pass without
requiring further changes. In the future, we'll remove these allow_...
statements and fix any violations as
they arise. You can read more about this in the documentation for multiple databases
Reviewer, please confirm that:
When you are finished, please request a review from the database maintainer suggested by Danger. If you have any questions or concerns, reach out to @tigerwnz or @manojmj.
If you would like to go through similar merged MRs so as to gather an understanding on this topic, you can use this link.
This change was generated by gitlab-housekeeper using the Keeps::DetermineDesiredShardingKeyFeatureCategory keep.
To provide feedback on your experience with gitlab-housekeeper
please comment in
#442003.
The new policy type we want to introduce with #452379 should have two modes of operation:
This issue is about added the inject mode. During the experiment to add custom actions to scan execution policies we identified a few challenges to this approach:
To avoid conflicts with variables defined on the project config, variables defined in policies should be scoped down to single jobs. We discussed this in a pairing session: https://youtu.be/p0JL-xrgWy0?feature=shared&t=1326
We want to give users some control over the order of stages without interfering with project CI stages. To archive this, we want to introduce three reserved
stages that will be injected into every pipeline:
.pipeline-policy-pre
stage will run at the very beginning of the pipeline, before the .pre
stage..pipeline-policy-test
stage will be injected after the test
stage. If the test
stage does not exist, it will be injected after the build
stage. If the build
stage does not exist, it will be injected at the beginning of the pipeline after the .pre
stage..pipeline-policy-post
stage will run at the very end of the pipeline, after the .post
stage.Jobs can be assigned to one of those stages as well as any stage that exists in the project CI. However, it jobs can only be guaranteed to run if they are assigned to a security-policy stage. It will not be possible to create custom stages in a pipeline execution policy.
The stages are reserved for security policy jobs only. It is not possible to assign jobs from a project CI to one of the reserved stages.
Our previous strategy was to give stages defined in a pipeline execution policy precedence over stages from a project CI. We worked on this in #432042 but decided that this strategy will cause confusion and will be hard to configure.
Instead of using pipeline rules like in scan execution policies, the new policy type should be able to be controller by pipeline workflow rules. However, we need to handle conflicts between the project and policy CI. An idea to do this is to run Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules
for the policy CI and project CI separately.
To avoid conflicting job names, we can assign an IID to every job name resulting in this pattern: job name - IID
. The IID should be available in a variable that can be used to manage job dependencies like in this example
The new policy type described in #452379 should be able to be scoped to compliance frameworks
Add the a new policy type Pipeline execution policy to enforce custom CI jobs on projects. More details in &13266
For the iteration, the new policy can override the project CI. Injecting jobs into project CI without replacing the entire config can be added in a later iteration.
Schema examples:
Secret detection:
name: "Secret detection"
description: "triggers all protected branches except main"
enabled: true
override_project_ci: true
content:
workflow:
rules:
- if: $CI_COMMIT_REF_PROTECTED == false || $CI_COMMIT_REF_NAME == 'main'
when: never
include:
- template: Jobs/Secret-Detection.gitlab-ci.yml
Using a config file:
name: "Ci config file"
description: "triggers all protected branches except main"
enabled: true
override_project_ci: true
content:
include:
project: pipeline-execution-policy/security-policy-project
file: ci.yml
See !145565 (diffs) for a schema proposal. The MR also enforces the policy using a processor in the CI config but it would be better add enforce the policy as part of the pipeline create sequence. This was discussed in a pairing session: https://www.youtube.com/watch?v=p0JL-xrgWy0
The feature should be added behing a feature flag.
Thanks @manojmj, looks great
Add desired sharding keys for feature category service_desk
.
These tables have been identified as a cell local tables. All cell local tables require a sharding key or a desired sharding key
A desired sharding key has been automatically selected for these tables.
These keys were chosen as the desired sharding keys because the
table has a :belongs_to relationship to a table that itself has a NOT NULL
sharding key.
Additionally, gitlab_schema
has been set to gitlab_main_cell
for any tables didn't use this schema already.
For these tables we have also added allow_cross_joins
, allow_cross_transactions
and
allow_cross_foreign_keys
. These will silence any existing violations, allowing the pipeline to pass without
requiring further changes. In the future, we'll remove these allow_...
statements and fix any violations as
they arise. You can read more about this in the documentation for multiple databases
Reviewer, please confirm that:
When you are finished, please request a review from the database maintainer suggested by Danger. If you have any questions or concerns, reach out to @tigerwnz or @manojmj.
If you would like to go through similar merged MRs so as to gather an understanding on this topic, you can use this link.
This change was generated by gitlab-housekeeper using the Keeps::DetermineDesiredShardingKeyFeatureCategory keep.
To provide feedback on your experience with gitlab-housekeeper
please comment in
#442003.
Thanks @rkumar555, looks great
Add a new column zoekt_settings
of the type jsonb
in the application_settings
table. This jsonb
column can have three attributes.
This column can be used to show the settings in the admin panel and eventually, few zoekt
feature flags can be deprecated.
All three attributes have the default value of false
. In the follow-up MRs the backfilling of this column will happen.
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
git fetch && git checkout 389765-add-zoekt_settings-in-application_settings && bin/rails db:migrate
bin/rails c
ApplicationSetting
record. The value of zoekt_settings
should be {}
ApplicationSetting.last.zoekt_settings
ApplicationSetting
record. The value of zoekt_settings
should be {"zoekt_indexing_enabled"=>false, "zoekt_indexing_paused"=>false, "zoekt_search_enabled"=>false}
ApplicationSetting.new.zoekt_settings
Related to #389765
@mfanGitLab, I've updated the broken link. It is linking to this section: https://docs.gitlab.com/ee/development/migration_style_guide.html#using-application-code-in-migrations-discouraged.
I don't think we have to avoid using application code at all cost but if there is a way to do it without application code that would be better.
Hi @alan can you merge this?