Merge branch 'jc-weighted-repository-storages' into 'master'

Save repository storages in application settings with weights

See merge request gitlab-org/gitlab!31645

app/controllers/admin/application_settings_controller.rb

@@ -214,6 +214,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
     [
       *::ApplicationSettingsHelper.visible_attributes,
       *::ApplicationSettingsHelper.external_authorization_service_attributes,
+      *ApplicationSetting.repository_storages_weighted_attributes,
       :lets_encrypt_notification_email,
       :lets_encrypt_terms_of_service_accepted,
       :domain_blacklist_file,

app/models/application_setting.rb

@@ -19,6 +19,12 @@ class ApplicationSetting < ApplicationRecord
 
   belongs_to :instance_administrators_group, class_name: "Group"
 
+  def self.repository_storages_weighted_attributes
+    @repository_storages_weighted_atributes ||= Gitlab.config.repositories.storages.keys.map { |k| "repository_storages_weighted_#{k}".to_sym }.freeze
+  end
+
+  store_accessor :repository_storages_weighted, *Gitlab.config.repositories.storages.keys, prefix: true
+
   # Include here so it can override methods from
   # `add_authentication_token_field`
   # We don't prepend for now because otherwise we'll need to
@@ -39,6 +45,7 @@ class ApplicationSetting < ApplicationRecord
   cache_markdown_field :after_sign_up_text
 
   default_value_for :id, 1
+  default_value_for :repository_storages_weighted, {}
 
   chronic_duration_attr_writer :archive_builds_in_human_readable, :archive_builds_in_seconds
 
@@ -152,6 +159,7 @@ class ApplicationSetting < ApplicationRecord
 
   validates :repository_storages, presence: true
   validate :check_repository_storages
+  validate :check_repository_storages_weighted
 
   validates :auto_devops_domain,
             allow_blank: true,
@@ -271,6 +279,10 @@ class ApplicationSetting < ApplicationRecord
 
   validates :allowed_key_types, presence: true
 
+  repository_storages_weighted_attributes.each do |attribute|
+    validates attribute, allow_nil: true, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 100 }
+  end
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     value&.each do |level|
       unless Gitlab::VisibilityLevel.options.value?(level)
@@ -434,6 +446,12 @@ class ApplicationSetting < ApplicationRecord
     recaptcha_enabled || login_recaptcha_protection_enabled
   end
 
+  repository_storages_weighted_attributes.each do |attribute|
+    define_method :"#{attribute}=" do |value|
+      super(value.to_i)
+    end
+  end
+
   private
 
   def parsed_grafana_url

app/models/application_setting_implementation.rb

@@ -104,6 +104,7 @@ module ApplicationSettingImplementation
         login_recaptcha_protection_enabled: false,
         repository_checks_enabled: true,
         repository_storages: ['default'],
+        repository_storages_weighted: { default: 100 },
         require_two_factor_authentication: false,
         restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'],
         session_expire_delay: Settings.gitlab['session_expire_delay'],
@@ -262,6 +263,10 @@ module ApplicationSettingImplementation
     Array(read_attribute(:repository_storages))
   end
 
+  def repository_storages_weighted
+    read_attribute(:repository_storages_weighted)
+  end
+
   def commit_email_hostname
     super.presence || self.class.default_commit_email_hostname
   end
@@ -422,6 +427,12 @@ module ApplicationSettingImplementation
       invalid.empty?
   end
 
+  def check_repository_storages_weighted
+    invalid = repository_storages_weighted.keys - Gitlab.config.repositories.storages.keys
+    errors.add(:repository_storages_weighted, "can't include: %{invalid_storages}" % { invalid_storages: invalid.join(", ") }) unless
+      invalid.empty?
+  end
+
   def terms_exist
     return unless enforce_terms?
 

changelogs/unreleased/jc-weighted-repository-storages.yml

+---
+title: Save repository storages in application settings with weights
+merge_request: 31645
+author:
+type: added

db/migrate/20200508203901_add_repository_storages_weighted_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddRepositoryStoragesWeightedToApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    add_column :application_settings, :repository_storages_weighted, :jsonb, default: {}, null: false
+  end
+
+  def down
+    remove_column :application_settings, :repository_storages_weighted
+  end
+end

db/structure.sql

@@ -440,6 +440,7 @@ CREATE TABLE public.application_settings (
     group_owners_can_manage_default_branch_protection boolean DEFAULT true NOT NULL,
     container_registry_vendor text DEFAULT ''::text NOT NULL,
     container_registry_version text DEFAULT ''::text NOT NULL,
+    repository_storages_weighted jsonb DEFAULT '{}'::jsonb NOT NULL,
     container_registry_features text[] DEFAULT '{}'::text[] NOT NULL,
     spam_check_endpoint_url text,
     spam_check_endpoint_enabled boolean DEFAULT false NOT NULL,
@@ -13888,6 +13889,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200508050301
 20200508091106
 20200508140959
+20200508203901
 20200511080113
 20200511083541
 20200511092246

lib/api/settings.rb

@@ -113,6 +113,7 @@ module API
       end
       optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues."
       optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects'
+      optional :repository_storages_weighted, type: Hash, desc: 'Storage paths for new projects with a weighted value between 0 and 100'
       optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to set up Two-factor authentication'
       given require_two_factor_authentication: ->(val) { val } do
         requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication'

spec/controllers/admin/application_settings_controller_spec.rb

@@ -120,6 +120,13 @@ describe Admin::ApplicationSettingsController do
       expect(ApplicationSetting.current.namespace_storage_size_limit).not_to eq(-100)
     end
 
+    it 'updates repository_storages_weighted setting' do
+      put :update, params: { application_setting: { repository_storages_weighted_default: 75 } }
+
+      expect(response).to redirect_to(general_admin_application_settings_path)
+      expect(ApplicationSetting.current.repository_storages_weighted_default).to eq(75)
+    end
+
     context 'external policy classification settings' do
       let(:settings) do
         {

spec/models/application_setting_spec.rb

@@ -105,6 +105,14 @@ describe ApplicationSetting do
 
     it { is_expected.not_to allow_value(false).for(:hashed_storage_enabled) }
 
+    it { is_expected.not_to allow_value(101).for(:repository_storages_weighted_default) }
+    it { is_expected.not_to allow_value(-1).for(:repository_storages_weighted_default) }
+    it { is_expected.to allow_value(100).for(:repository_storages_weighted_default) }
+    it { is_expected.to allow_value(0).for(:repository_storages_weighted_default) }
+    it { is_expected.to allow_value(50).for(:repository_storages_weighted_default) }
+    it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) }
+    it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) }
+
     context 'grafana_url validations' do
       before do
         subject.instance_variable_set(:@parsed_grafana_url, nil)
@@ -786,4 +794,17 @@ describe ApplicationSetting do
   end
 
   it_behaves_like 'application settings examples'
+
+  describe 'repository_storages_weighted_attributes' do
+    it 'returns the keys for repository_storages_weighted' do
+      expect(subject.class.repository_storages_weighted_attributes).to eq([:repository_storages_weighted_default])
+    end
+  end
+
+  it 'does not allow to set weight for non existing storage' do
+    setting.repository_storages_weighted = { invalid_storage: 100 }
+
+    expect(setting).not_to be_valid
+    expect(setting.errors.messages[:repository_storages_weighted]).to match_array(["can't include: invalid_storage"])
+  end
 end