diff --git a/app/models/integration.rb b/app/models/integration.rb index 3101d257d9e..1e6b4b9bade 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -353,16 +353,17 @@ class Integration < ApplicationRecord end private_class_method :instance_level_integration - def self.create_from_active_default_integrations(scope, association) - group_ids = sorted_ancestors(scope).select(:id) + # Returns the number of successfully saved integrations + # Duplicate integrations are excluded from this count by their validations. + def self.create_from_active_default_integrations(owner, association) + group_ids = sorted_ancestors(owner).select(:id) array = group_ids.to_sql.present? ? "array(#{group_ids.to_sql})" : 'ARRAY[]' + order = Arel.sql("type_new ASC, array_position(#{array}::bigint[], #{table_name}.group_id), instance DESC") - from_union([ - active.where(instance: true), - active.where(group_id: group_ids, inherit_from_id: nil) - ]).order(Arel.sql("type_new ASC, array_position(#{array}::bigint[], #{table_name}.group_id), instance DESC")).group_by(&:type).each do |type, records| - build_from_integration(records.first, association => scope.id).save - end + from_union([active.where(instance: true), active.where(group_id: group_ids, inherit_from_id: nil)]) + .order(order) + .group_by(&:type) + .count { |type, parents| build_from_integration(parents.first, association => owner.id).save } end def self.inherited_descendants_from_self_or_ancestors_from(integration) diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index 78dbe487297..3ae64643420 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -34,7 +34,7 @@ = _('To add the entry manually, provide the following details to the application on your phone.') %p.gl-mt-0.gl-mb-0 = _('Account: %{account}') % { account: @account_string } - %p.gl-mt-0.gl-mb-0{ data: { qa_selector: 'otp_secret_content' } } + %p.gl-mt-0.gl-mb-0.two-factor-secret{ data: { qa_selector: 'otp_secret_content' } } = _('Key: %{key}') %{ key: current_user.otp_secret.scan(/.{4}/).join(' ') } %p.two-factor-new-manual-content = _('Time based: Yes') diff --git a/config/feature_flags/development/ci_validate_job_length.yml b/config/feature_flags/development/ci_validate_job_length.yml deleted file mode 100644 index 5e29d3c1435..00000000000 --- a/config/feature_flags/development/ci_validate_job_length.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_validate_job_length -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73599 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344665 -milestone: '14.5' -type: development -group: group::pipeline authoring -default_enabled: true diff --git a/doc/ci/jobs/index.md b/doc/ci/jobs/index.md index 23e62da083c..9eefdd4b6e4 100644 --- a/doc/ci/jobs/index.md +++ b/doc/ci/jobs/index.md @@ -111,9 +111,10 @@ You can't use these keywords as job names: - `false` - `nil` -Job names must be 255 characters or less. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/342800) -in GitLab 14.5, [with a feature flag](../../administration/feature_flags.md) named `ci_validate_job_length`. -Enabled by default. To disable it, ask an administrator to [disable the feature flag](../../administration/feature_flags.md). +Job names must be 255 characters or less. + +> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/342800) in GitLab 14.5. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/344665) in GitLab 14.10.[Feature flag ](https://gitlab.com/gitlab-org/gitlab/-/issues/344665) removed. Use unique names for your jobs. If multiple jobs have the same name, only one is added to the pipeline, and it's difficult to predict which one is chosen. diff --git a/lib/gitlab/ci/config/entry/processable.rb b/lib/gitlab/ci/config/entry/processable.rb index 43475742214..46afedbcc3a 100644 --- a/lib/gitlab/ci/config/entry/processable.rb +++ b/lib/gitlab/ci/config/entry/processable.rb @@ -23,7 +23,7 @@ module Gitlab validates :config, presence: true validates :name, presence: true validates :name, type: Symbol - validates :name, length: { maximum: 255 }, if: -> { ::Feature.enabled?(:ci_validate_job_length, default_enabled: :yaml) } + validates :name, length: { maximum: 255 } validates :config, disallowed_keys: { in: %i[only except start_in], diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index fe8c2227659..1e447923a39 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -289,9 +289,10 @@ module Gitlab ObjectStoreSettings::SUPPORTED_TYPES.collect do |type| section_setting = config.try(type) - next unless section_setting + next unless section_setting && section_setting['enabled'] - object_store_setting = section_setting['object_store'] + # Use #to_h to avoid Settingslogic bug: https://gitlab.com/gitlab-org/gitlab/-/issues/286873 + object_store_setting = section_setting['object_store']&.to_h next unless object_store_setting && object_store_setting['enabled'] diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index e1e4d5a249a..822bf898034 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -839,7 +839,15 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do expect(page).to have_current_path(profile_two_factor_auth_path, ignore_query: true) - fill_in 'pin_code', with: user.reload.current_otp + # Use the secret shown on the page to generate the OTP that will be entered. + # This detects issues wherein a new secret gets generated after the + # page is shown. + wait_for_requests + + otp_secret = page.find('.two-factor-secret').text.gsub('Key:', '').delete(' ') + current_otp = ROTP::TOTP.new(otp_secret).now + + fill_in 'pin_code', with: current_otp fill_in 'current_password', with: user.password click_button 'Register with two-factor app' diff --git a/spec/lib/gitlab/import_export/command_line_util_spec.rb b/spec/lib/gitlab/import_export/command_line_util_spec.rb index b71f17259a9..f47f1ab58a8 100644 --- a/spec/lib/gitlab/import_export/command_line_util_spec.rb +++ b/spec/lib/gitlab/import_export/command_line_util_spec.rb @@ -167,6 +167,7 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil do context 'for object_storage uri' do let(:enabled_object_storage_setting) do { + 'enabled' => true, 'object_store' => { 'enabled' => true, diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 57b0297a0a0..8f505606e04 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -43,6 +43,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do let(:import_url) { "#{host}/external-diffs/merge_request_diffs/mr-1/diff-1" } let(:enabled_object_storage_setting) do { + 'enabled' => true, 'object_store' => { 'enabled' => true, @@ -81,6 +82,49 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do let(:expected_hostname) { nil } end end + + context 'when LFS object storage is enabled' do + let(:lfs_config) do + { + 'enabled' => lfs_enabled, + # This nesting of Settingslogic is necessary to trigger the bug + 'object_store' => Settingslogic.new({ 'enabled' => true }) + } + end + + let(:config) do + { + 'gitlab' => Gitlab.config.gitlab, + 'repositories' => { 'storages' => { 'default' => 'test' } }, + 'lfs' => Settingslogic.new(lfs_config) + } + end + + let(:host) { 'http://127.0.0.1:9000' } + let(:settings) { Settingslogic.new(config) } + + before do + allow(Gitlab).to receive(:config).and_return(settings) + # Triggers Settingslogic bug: https://gitlab.com/gitlab-org/gitlab/-/issues/286873 + settings.repositories.storages.default + end + + context 'when LFS is disabled' do + let(:lfs_enabled) { false } + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + + context 'when LFS is enabled with no connection endpoint' do + let(:lfs_enabled) { true } + + it 'raises an error' do + expect { subject }.to raise_error(described_class::BlockedUrlError) + end + end + end end context 'when allow_object_storage is false' do diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 21f6d320d71..7e9a9b43af7 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -473,6 +473,18 @@ RSpec.describe Integration do expect(project.reload.integrations.first.inherit_from_id).to eq(group_integration.id) end + context 'there are multiple inheritable integrations, and a duplicate' do + let!(:group_integration_2) { create(:jenkins_integration, :group, group: group) } + let!(:group_integration_3) { create(:datadog_integration, :instance) } + let!(:duplicate) { create(:jenkins_integration, project: project) } + + it 'returns the number of successfully created integrations' do + expect(described_class.create_from_active_default_integrations(project, :project_id)).to eq 2 + + expect(project.reload.integrations.size).to eq(3) + end + end + context 'passing a group' do let!(:subgroup) { create(:group, parent: group) }