Merge branch 'feat/408150' into 'master'

Add new jsonb column to store settings for default branch protection

See merge request !120139



Merged-by: Bojan Marjanovic <bmarjanovic@gitlab.com>
Approved-by: Ian Baum <ibaum@gitlab.com>
Approved-by: Bojan Marjanovic <bmarjanovic@gitlab.com>
Reviewed-by: Kamil Trzciński <ayufan@ayufan.eu>
Reviewed-by: Bojan Marjanovic <bmarjanovic@gitlab.com>
Co-authored-by: Michael Becker <11881043-wandering_person@users.noreply.gitlab.com>

app/models/application_setting.rb

@@ -55,6 +55,9 @@ class ApplicationSetting < MainClusterwide::ApplicationRecord
     archive_builds_in_seconds: 'Archive job value'
   }.freeze
 
+  # matches the size set in the database constraint
+  DEFAULT_BRANCH_PROTECTIONS_DEFAULT_MAX_SIZE = 1.kilobyte
+
   enum whats_new_variant: { all_tiers: 0, current_tier: 1, disabled: 2 }, _prefix: true
   enum email_confirmation_setting: { off: 0, soft: 1, hard: 2 }, _prefix: true
 
@@ -110,6 +113,7 @@ def self.kroki_formats_attributes
   attribute :id, default: 1
   attribute :repository_storages_weighted, default: -> { {} }
   attribute :kroki_formats, default: -> { {} }
+  attribute :default_branch_protection_defaults, default: -> { {} }
 
   chronic_duration_attr_writer :archive_builds_in_human_readable, :archive_builds_in_seconds
 
@@ -117,6 +121,9 @@ def self.kroki_formats_attributes
   chronic_duration_attr :group_runner_token_expiration_interval_human_readable, :group_runner_token_expiration_interval
   chronic_duration_attr :project_runner_token_expiration_interval_human_readable, :project_runner_token_expiration_interval
 
+  validates :default_branch_protection_defaults, json_schema: { filename: 'default_branch_protection_defaults' }
+  validates :default_branch_protection_defaults, bytesize: { maximum: -> { DEFAULT_BRANCH_PROTECTIONS_DEFAULT_MAX_SIZE } }
+
   validates :grafana_url,
     system_hook_url: ADDRESSABLE_URL_VALIDATION_OPTIONS.merge({
       blocked_message: "is blocked: %{exception_message}. #{GRAFANA_URL_ERROR_MESSAGE}"

app/models/namespace_setting.rb

@@ -12,8 +12,12 @@ class NamespaceSetting < ApplicationRecord
   enum jobs_to_be_done: { basics: 0, move_repository: 1, code_storage: 2, exploring: 3, ci: 4, other: 5 }, _suffix: true
   enum enabled_git_access_protocol: { all: 0, ssh: 1, http: 2 }, _suffix: true
 
+  attribute :default_branch_protection_defaults, default: -> { {} }
+
   validates :enabled_git_access_protocol, inclusion: { in: enabled_git_access_protocols.keys }
   validates :code_suggestions, allow_nil: false, inclusion: { in: [true, false] }
+  validates :default_branch_protection_defaults, json_schema: { filename: 'default_branch_protection_defaults' }
+  validates :default_branch_protection_defaults, bytesize: { maximum: -> { DEFAULT_BRANCH_PROTECTIONS_DEFAULT_MAX_SIZE } }
 
   validate :allow_mfa_for_group
   validate :allow_resource_access_token_creation_for_group
@@ -43,6 +47,9 @@ class NamespaceSetting < ApplicationRecord
     project_runner_token_expiration_interval
   ].freeze
 
+  # matches the size set in the database constraint
+  DEFAULT_BRANCH_PROTECTIONS_DEFAULT_MAX_SIZE = 1.kilobyte
+
   self.primary_key = :namespace_id
 
   def self.allowed_namespace_settings_params

app/validators/json_schemas/default_branch_protection_defaults.json

+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "Default settings for default branch protection",
+  "type": "object",
+  "properties": {
+    "name": {
+      "type": "string"
+    },
+    "allow_force_push": {
+      "type": "boolean"
+    },
+    "allowed_to_merge": {
+      "type": "array",
+      "items": {
+        "oneOf": [
+          {
+            "type": "object",
+            "additionalProperties": false,
+            "properties": {
+              "group_id": {
+                "type": "integer"
+              }
+            }
+          },
+          {
+            "type": "object",
+            "additionalProperties": false,
+            "properties": {
+              "access_level": {
+                "type": "integer"
+              }
+            }
+          }
+        ]
+      }
+    },
+    "allowed_to_push": {
+      "type": "array",
+      "items": {
+        "oneOf": [
+          {
+            "type": "object",
+            "additionalProperties": false,
+            "properties": {
+              "group_id": {
+                "type": "integer"
+              }
+            }
+          },
+          {
+            "type": "object",
+            "additionalProperties": false,
+            "properties": {
+              "access_level": {
+                "type": "integer"
+              }
+            }
+          }
+        ]
+      }
+    },
+    "code_owner_approval_required": {
+      "type": "boolean"
+    },
+    "merge_access_level": {
+      "type": "integer"
+    },
+    "push_access_level": {
+      "type": "integer"
+    },
+    "unprotect_access_level": {
+      "type": "integer"
+    }
+  },
+  "additionalProperties": false
+}

db/migrate/20230606124754_add_default_branch_protections_json_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddDefaultBranchProtectionsJsonToApplicationSettings < Gitlab::Database::Migration[2.1]
+  def change
+    add_column :application_settings, :default_branch_protection_defaults, :jsonb, null: false, default: {}
+  end
+end

db/migrate/20230606124854_add_default_branch_protections_json_to_namespace_settings.rb

+# frozen_string_literal: true
+
+class AddDefaultBranchProtectionsJsonToNamespaceSettings < Gitlab::Database::Migration[2.1]
+  def change
+    add_column :namespace_settings, :default_branch_protection_defaults, :jsonb, null: false, default: {}
+  end
+end

db/migrate/20230606124855_add_size_constraint_to_namespace_settings_json.rb

+# frozen_string_literal: true
+
+class AddSizeConstraintToNamespaceSettingsJson < Gitlab::Database::Migration[2.1]
+  CONSTRAINT_NAME = 'default_branch_protection_defaults_size_constraint'
+
+  disable_ddl_transaction!
+
+  def up
+    add_check_constraint :namespace_settings, 'octet_length(default_branch_protection_defaults::text) <= 1024',
+      CONSTRAINT_NAME, validate: false
+  end
+
+  def down
+    remove_check_constraint :namespace_settings, CONSTRAINT_NAME
+  end
+end

db/migrate/20230606124856_add_size_constraint_to_application_settings_json.rb

+# frozen_string_literal: true
+
+class AddSizeConstraintToApplicationSettingsJson < Gitlab::Database::Migration[2.1]
+  CONSTRAINT_NAME = 'default_branch_protection_defaults_size_constraint'
+
+  disable_ddl_transaction!
+
+  def up
+    add_check_constraint :application_settings, 'octet_length(default_branch_protection_defaults::text) <= 1024',
+      CONSTRAINT_NAME, validate: false
+  end
+
+  def down
+    remove_check_constraint :application_settings, CONSTRAINT_NAME
+  end
+end

db/schema_migrations/20230606124754

+490dc775459b89701c9531c7d10698734e47bd22a17aca07df12f8ac5a6d38e9
\ No newline at end of file

db/schema_migrations/20230606124854

+dd1cf7175a283e60af315372d96ea6836aff4e678830ab9b9d8e3367dc351c92
\ No newline at end of file

db/schema_migrations/20230606124855

+afe649b351bee4ec7152d465a73c35e4356d947c25fec727db2b4e38f268da41
\ No newline at end of file

db/schema_migrations/20230606124856

+df3b650081b827ab465aa0bf35b30a8fcb81a0c3609caecfa30e71d71761c809
\ No newline at end of file

db/structure.sql

@@ -11850,6 +11850,7 @@ CREATE TABLE application_settings (
     instance_level_code_suggestions_enabled boolean DEFAULT false NOT NULL,
     delete_unconfirmed_users boolean DEFAULT false NOT NULL,
     unconfirmed_users_delete_after_days integer DEFAULT 7 NOT NULL,
+    default_branch_protection_defaults jsonb DEFAULT '{}'::jsonb NOT NULL,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)),
     CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
@@ -18754,6 +18755,7 @@ CREATE TABLE namespace_settings (
     code_suggestions boolean DEFAULT false NOT NULL,
     experiment_features_enabled boolean DEFAULT false NOT NULL,
     third_party_ai_features_enabled boolean DEFAULT true NOT NULL,
+    default_branch_protection_defaults jsonb DEFAULT '{}'::jsonb NOT NULL,
     CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)),
     CONSTRAINT namespace_settings_unique_project_download_limit_allowlist_size CHECK ((cardinality(unique_project_download_limit_allowlist) <= 100))
@@ -27096,6 +27098,12 @@ ALTER TABLE ONLY dast_site_validations
 ALTER TABLE ONLY dast_sites
     ADD CONSTRAINT dast_sites_pkey PRIMARY KEY (id);
+ALTER TABLE namespace_settings
+    ADD CONSTRAINT default_branch_protection_defaults_size_constraint CHECK ((octet_length((default_branch_protection_defaults)::text) <= 1024)) NOT VALID;
+
+ALTER TABLE application_settings
+    ADD CONSTRAINT default_branch_protection_defaults_size_constraint CHECK ((octet_length((default_branch_protection_defaults)::text) <= 1024)) NOT VALID;
+
 ALTER TABLE ONLY dependency_list_exports
     ADD CONSTRAINT dependency_list_exports_pkey PRIMARY KEY (id);

spec/db/schema_spec.rb

@@ -247,8 +247,7 @@
     "Packages::Composer::Metadatum" => %w[composer_json],
     "RawUsageData" => %w[payload], # Usage data payload changes often, we cannot use one schema
     "Releases::Evidence" => %w[summary],
-    "Vulnerabilities::Finding::Evidence" => %w[data], # Validation work in progress
-    "EE::Gitlab::BackgroundMigration::FixSecurityScanStatuses::SecurityScan" => %w[info] # This is a migration model
+    "Vulnerabilities::Finding::Evidence" => %w[data] # Validation work in progress
   }.freeze
 
   # We are skipping GEO models for now as it adds up complexity
@@ -258,8 +257,10 @@
         next if models_by_table_name[hash["table_name"]].nil?
 
         models_by_table_name[hash["table_name"]].each do |model|
-          jsonb_columns = [hash["column_name"]] - ignored_jsonb_columns(model.name)
+          # Skip migration models
+          next if model.name.include?('Gitlab::BackgroundMigration')
 
+          jsonb_columns = [hash["column_name"]] - ignored_jsonb_columns(model.name)
           expect(model).to validate_jsonb_schema(jsonb_columns)
         end
       end

spec/models/application_setting_spec.rb

@@ -23,6 +23,7 @@
     it { expect(setting.id).to eq(1) }
     it { expect(setting.repository_storages_weighted).to eq({}) }
     it { expect(setting.kroki_formats).to eq({}) }
+    it { expect(setting.default_branch_protection_defaults).to eq({}) }
   end
 
   describe 'validations' do
@@ -1228,6 +1229,25 @@ def expect_invalid
       it { is_expected.to allow_value(*Gitlab::ColorSchemes.valid_ids).for(:default_syntax_highlighting_theme) }
       it { is_expected.not_to allow_value(nil, 0, Gitlab::ColorSchemes.available_schemes.size + 1).for(:default_syntax_highlighting_theme) }
     end
+
+    context 'default_branch_protections_defaults validations' do
+      let(:charset) { [*'a'..'z'] + [*0..9] }
+      let(:value) { Array.new(byte_size) { charset.sample }.join }
+
+      it { expect(described_class).to validate_jsonb_schema(['default_branch_protection_defaults']) }
+
+      context 'when json is more than 1kb' do
+        let(:byte_size) { 1.1.kilobytes }
+
+        it { is_expected.not_to allow_value({ name: value }).for(:default_branch_protection_defaults) }
+      end
+
+      context 'when json less than 1kb' do
+        let(:byte_size) { 0.5.kilobytes }
+
+        it { is_expected.to allow_value({ name: value }).for(:default_branch_protection_defaults) }
+      end
+    end
   end
 
   context 'restrict creating duplicates' do
@@ -1498,6 +1518,26 @@ def expect_invalid
     end
   end
 
+  describe 'default_branch_protection_defaults' do
+    let(:defaults) { { name: 'main', push_access_level: 30, merge_access_level: 30, unprotect_access_level: 40 } }
+
+    it 'returns the value for default_branch_protection_defaults' do
+      subject.default_branch_protection_defaults = defaults
+      expect(subject.default_branch_protection_defaults['name']).to eq('main')
+      expect(subject.default_branch_protection_defaults['push_access_level']).to eq(30)
+      expect(subject.default_branch_protection_defaults['merge_access_level']).to eq(30)
+      expect(subject.default_branch_protection_defaults['unprotect_access_level']).to eq(40)
+    end
+
+    context 'when provided with content that does not match the JSON schema' do
+      # valid json
+      it { is_expected.to allow_value({ name: 'bar' }).for(:default_branch_protection_defaults) }
+
+      # invalid json
+      it { is_expected.not_to allow_value({ foo: 'bar' }).for(:default_branch_protection_defaults) }
+    end
+  end
+
   describe '#static_objects_external_storage_auth_token=', :aggregate_failures do
     subject { setting.static_objects_external_storage_auth_token = token }
 

spec/models/namespace_setting_spec.rb

@@ -14,6 +14,12 @@
   it { is_expected.to define_enum_for(:jobs_to_be_done).with_values([:basics, :move_repository, :code_storage, :exploring, :ci, :other]).with_suffix }
   it { is_expected.to define_enum_for(:enabled_git_access_protocol).with_values([:all, :ssh, :http]).with_suffix }
 
+  describe 'default values' do
+    subject(:setting) { described_class.new }
+
+    it { expect(setting.default_branch_protection_defaults).to eq({}) }
+  end
+
   describe "validations" do
     it { is_expected.to validate_inclusion_of(:code_suggestions).in_array([true, false]) }
 
@@ -138,6 +144,25 @@
         end
       end
     end
+
+    context 'default_branch_protections_defaults validations' do
+      let(:charset) { [*'a'..'z'] + [*0..9] }
+      let(:value) { Array.new(byte_size) { charset.sample }.join }
+
+      it { expect(described_class).to validate_jsonb_schema(['default_branch_protection_defaults']) }
+
+      context 'when json is more than 1kb' do
+        let(:byte_size) { 1.1.kilobytes }
+
+        it { is_expected.not_to allow_value({ name: value }).for(:default_branch_protection_defaults) }
+      end
+
+      context 'when json less than 1kb' do
+        let(:byte_size) { 0.5.kilobytes }
+
+        it { is_expected.to allow_value({ name: value }).for(:default_branch_protection_defaults) }
+      end
+    end
   end
 
   describe '#prevent_sharing_groups_outside_hierarchy' do
@@ -404,4 +429,24 @@
   describe '#delayed_project_removal' do
     it_behaves_like 'a cascading namespace setting boolean attribute', settings_attribute_name: :delayed_project_removal
   end
+
+  describe 'default_branch_protection_defaults' do
+    let(:defaults) { { name: 'main', push_access_level: 30, merge_access_level: 30, unprotect_access_level: 40 } }
+
+    it 'returns the value for default_branch_protection_defaults' do
+      subject.default_branch_protection_defaults = defaults
+      expect(subject.default_branch_protection_defaults['name']).to eq('main')
+      expect(subject.default_branch_protection_defaults['push_access_level']).to eq(30)
+      expect(subject.default_branch_protection_defaults['merge_access_level']).to eq(30)
+      expect(subject.default_branch_protection_defaults['unprotect_access_level']).to eq(40)
+    end
+
+    context 'when provided with content that does not match the JSON schema' do
+      # valid json
+      it { is_expected.to allow_value({ name: 'bar' }).for(:default_branch_protection_defaults) }
+
+      # invalid json
+      it { is_expected.not_to allow_value({ foo: 'bar' }).for(:default_branch_protection_defaults) }
+    end
+  end
 end