Add latest changes from gitlab-org/gitlab@master

app/services/pages_domains/create_acme_order_service.rb

@@ -3,6 +3,9 @@
 module PagesDomains
   class CreateAcmeOrderService
     attr_reader :pages_domain
+    # TODO: remove this hack after https://gitlab.com/gitlab-org/gitlab/issues/30146 is implemented
+    # This makes GitLab automatically retry the certificate obtaining process every 2 hours if process wasn't finished
+    SHORT_EXPIRATION_DELAY = 2.hours
     def initialize(pages_domain)
       @pages_domain = pages_domain
@@ -17,7 +20,7 @@ module PagesDomains
       private_key = OpenSSL::PKey::RSA.new(4096)
       saved_order = pages_domain.acme_orders.create!(
         url: order.url,
-        expires_at: order.expires,
+        expires_at: [order.expires, SHORT_EXPIRATION_DELAY.from_now].min,
         private_key: private_key.to_pem,
         challenge_token: challenge.token,

changelogs/unreleased/acme-order-short-expiration.yml

+---
+title: Retry obtaining Let's Encrypt certificates every 2 hours if it wasn't successful
+merge_request: 22336
+author:
+type: fixed

lib/gitlab/database_importers/instance_administrators/create_group.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DatabaseImporters
+    module InstanceAdministrators
+      class CreateGroup < ::BaseService
+        include Stepable
+
+        VISIBILITY_LEVEL = Gitlab::VisibilityLevel::INTERNAL
+
+        steps :validate_application_settings,
+          :validate_admins,
+          :create_group,
+          :save_group_id,
+          :add_group_members,
+          :track_event
+
+        def initialize
+          super(nil)
+        end
+
+        def execute
+          execute_steps
+        end
+
+        private
+
+        def validate_application_settings(result)
+          return success(result) if application_settings
+
+          log_error('No application_settings found')
+          error(_('No application_settings found'))
+        end
+
+        def validate_admins(result)
+          unless instance_admins.any?
+            log_error('No active admin user found')
+            return error(_('No active admin user found'))
+          end
+
+          success(result)
+        end
+
+        def create_group(result)
+          if group_created?
+            log_info(_('Instance administrators group already exists'))
+            result[:group] = instance_administrators_group
+            return success(result)
+          end
+
+          result[:group] = ::Groups::CreateService.new(instance_admins.first, create_group_params).execute
+
+          if result[:group].persisted?
+            success(result)
+          else
+            log_error("Could not create instance administrators group. Errors: %{errors}" % { errors: result[:group].errors.full_messages })
+            error(_('Could not create group'))
+          end
+        end
+
+        def save_group_id(result)
+          return success(result) if group_created?
+
+          response = application_settings.update(
+            instance_administrators_group_id: result[:group].id
+          )
+
+          if response
+            success(result)
+          else
+            log_error("Could not save instance administrators group ID, errors: %{errors}" % { errors: application_settings.errors.full_messages })
+            error(_('Could not save group ID'))
+          end
+        end
+
+        def add_group_members(result)
+          group = result[:group]
+          members = group.add_users(members_to_add(group), Gitlab::Access::MAINTAINER)
+          errors = members.flat_map { |member| member.errors.full_messages }
+
+          if errors.any?
+            log_error('Could not add admins as members to self-monitoring project. Errors: %{errors}' % { errors: errors })
+            error(_('Could not add admins as members'))
+          else
+            success(result)
+          end
+        end
+
+        def track_event(result)
+          ::Gitlab::Tracking.event("instance_administrators_group", "group_created")
+
+          success(result)
+        end
+
+        def group_created?
+          instance_administrators_group.present?
+        end
+
+        def application_settings
+          @application_settings ||= ApplicationSetting.current_without_cache
+        end
+
+        def instance_administrators_group
+          application_settings.instance_administrators_group
+        end
+
+        def instance_admins
+          @instance_admins ||= User.admins.active
+        end
+
+        def members_to_add(group)
+          # Exclude admins who are already members of group because
+          # `group.add_users(users)` returns an error if the users parameter contains
+          # users who are already members of the group.
+          instance_admins - group.members.collect(&:user)
+        end
+
+        def create_group_params
+          {
+            name: 'GitLab Instance Administrators',
+            visibility_level: VISIBILITY_LEVEL,
+
+            # The 8 random characters at the end are so that the path does not
+            # clash with any existing group that the user might have created.
+            path: "gitlab-instance-administrators-#{SecureRandom.hex(4)}"
+          }
+        end
+      end
+    end
+  end
+end

lib/gitlab/database_importers/self_monitoring/project/create_service.rb

@@ -12,11 +12,9 @@ module Gitlab
           PROJECT_NAME = 'GitLab Instance Administration'
           steps :validate_application_settings,
-            :validate_admins,
             :create_group,
             :create_project,
             :save_project_id,
-            :add_group_members,
             :add_prometheus_manual_configuration,
             :track_event
@@ -37,28 +35,14 @@ module Gitlab
             error(_('No application_settings found'))
           end
-          def validate_admins(result)
-            unless instance_admins.any?
-              log_error('No active admin user found')
-              return error(_('No active admin user found'))
-            end
-
-            success(result)
-          end
-
           def create_group(result)
-            if project_created?
-              log_info(_('Instance administrators group already exists'))
-              result[:group] = self_monitoring_project.owner
-              return success(result)
-            end
-
-            result[:group] = ::Groups::CreateService.new(group_owner, create_group_params).execute
-            if result[:group].persisted?
-              success(result)
+            create_group_response =
+              Gitlab::DatabaseImporters::InstanceAdministrators::CreateGroup.new.execute
+            if create_group_response[:status] == :success
+              success(result.merge(create_group_response))
             else
-              error(_('Could not create group'))
+              error(create_group_response[:message])
             end
           end
@@ -69,7 +53,9 @@ module Gitlab
               return success(result)
             end
-            result[:project] = ::Projects::CreateService.new(group_owner, create_project_params(result[:group])).execute
+            owner = result[:group].owners.first
+
+            result[:project] = ::Projects::CreateService.new(owner, create_project_params(result[:group])).execute
             if result[:project].persisted?
               success(result)
@@ -94,19 +80,6 @@ module Gitlab
             end
           end
-          def add_group_members(result)
-            group = result[:group]
-            members = group.add_users(members_to_add(group), Gitlab::Access::MAINTAINER)
-            errors = members.flat_map { |member| member.errors.full_messages }
-
-            if errors.any?
-              log_error('Could not add admins as members to self-monitoring project. Errors: %{errors}' % { errors: errors })
-              error(_('Could not add admins as members'))
-            else
-              success(result)
-            end
-          end
-
           def add_prometheus_manual_configuration(result)
             return success(result) unless prometheus_enabled?
             return success(result) unless prometheus_listen_address.present?
@@ -140,29 +113,6 @@ module Gitlab
             ::Gitlab::Prometheus::Internal.listen_address
           end
-          def instance_admins
-            @instance_admins ||= User.admins.active
-          end
-
-          def group_owner
-            instance_admins.first
-          end
-
-          def members_to_add(group)
-            # Exclude admins who are already members of group because
-            # `group.add_users(users)` returns an error if the users parameter contains
-            # users who are already members of the group.
-            instance_admins - group.members.collect(&:user)
-          end
-
-          def create_group_params
-            {
-              name: 'GitLab Instance Administrators',
-              path: "gitlab-instance-administrators-#{SecureRandom.hex(4)}",
-              visibility_level: VISIBILITY_LEVEL
-            }
-          end
-
           def docs_path
             Rails.application.routes.url_helpers.help_page_path(
               'administration/monitoring/gitlab_instance_administration_project/index'

lib/sentry/client.rb

@@ -33,6 +33,7 @@ module Sentry
     def request_params
       {
         headers: {
+          'Content-Type' => 'application/json',
           'Authorization' => "Bearer #{@token}"
         },
         follow_redirects: false
@@ -47,7 +48,7 @@ module Sentry
     def http_put(url, params = {})
       http_request do
-        Gitlab::HTTP.put(url, **request_params.merge(body: params))
+        Gitlab::HTTP.put(url, **request_params.merge(body: params.to_json))
       end
     end

locale/gitlab.pot

@@ -5202,6 +5202,9 @@ msgstr ""
 msgid "Could not revoke personal access token %{personal_access_token_name}."
 msgstr ""
+msgid "Could not save group ID"
+msgstr ""
+
 msgid "Could not save project ID"
 msgstr ""

qa/qa/page/project/settings/protected_branches.rb

@@ -58,10 +58,9 @@ module QA
             within_element(:"allowed_to_#{action}_dropdown") do
               click_on allowed[:roles]
+              allowed[:users].each { |user| click_on user.username } if allowed.key?(:users)
+              allowed[:groups].each { |group| click_on group.name } if allowed.key?(:groups)
             end
-
-            # Click the select element again to close the dropdown
-            click_element :"allowed_to_#{action}_select"
           end
         end
       end

qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb

@@ -28,7 +28,6 @@ module QA
         Flow::Project.add_member(project: project, username: user.username)
         Resource::Issue.fabricate_via_api! do |issue|
-          issue.title = 'issue title'
           issue.project = project
         end.visit!

qa/qa/specs/features/browser_ui/2_plan/issue/collapse_comments_in_discussions_spec.rb

@@ -8,9 +8,7 @@ module QA
       before do
         Flow::Login.sign_in
-        Resource::Issue.fabricate_via_api! do |issue|
-          issue.title = 'issue title'
-        end.visit!
+        Resource::Issue.fabricate_via_api!.visit!
         Page::Project::Issue::Show.perform do |show|
           show.select_all_activities_filter

qa/qa/specs/features/browser_ui/2_plan/issue/comment_issue_spec.rb

@@ -6,9 +6,7 @@ module QA
       before do
         Flow::Login.sign_in
-        Resource::Issue.fabricate_via_api! do |issue|
-          issue.title = 'issue title'
-        end.visit!
+        Resource::Issue.fabricate_via_api!.visit!
       end
       it 'user comments on an issue and edits the comment' do

qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb

@@ -3,16 +3,12 @@
 module QA
   context 'Plan', :smoke do
     describe 'Issue creation' do
-      let(:issue_title) { 'issue title' }
-
       before do
         Flow::Login.sign_in
       end
       it 'user creates an issue' do
-        issue = Resource::Issue.fabricate_via_browser_ui! do |issue|
-          issue.title = issue_title
-        end
+        issue = Resource::Issue.fabricate_via_browser_ui!
         Page::Project::Menu.perform(&:click_issues)
@@ -28,9 +24,7 @@ module QA
         end
         before do
-          Resource::Issue.fabricate_via_api! do |issue|
-            issue.title = issue_title
-          end.visit!
+          Resource::Issue.fabricate_via_api!.visit!
         end
         it 'user comments on an issue with an attachment' do

qa/qa/specs/features/browser_ui/2_plan/issue/filter_issue_comments_spec.rb

@@ -3,14 +3,10 @@
 module QA
   context 'Plan' do
     describe 'filter issue comments activities' do
-      let(:issue_title) { 'issue title' }
-
       before do
         Flow::Login.sign_in
-        Resource::Issue.fabricate_via_api! do |issue|
-          issue.title = issue_title
-        end.visit!
+        Resource::Issue.fabricate_via_api!.visit!
       end
       it 'user filters comments and activities in an issue' do

qa/qa/specs/features/browser_ui/2_plan/issue/mentions_spec.rb

@@ -16,7 +16,6 @@ module QA
         project.add_member(@user)
         Resource::Issue.fabricate_via_api! do |issue|
-          issue.title = 'issue to test mention'
           issue.project = project
         end.visit!
       end

spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::DatabaseImporters::InstanceAdministrators::CreateGroup do
+  describe '#execute' do
+    let(:result) { subject.execute }
+
+    context 'without application_settings' do
+      it 'returns error' do
+        expect(subject).to receive(:log_error).and_call_original
+        expect(result).to eq(
+          status: :error,
+          message: 'No application_settings found',
+          last_step: :validate_application_settings
+        )
+
+        expect(Group.count).to eq(0)
+      end
+    end
+
+    context 'without admin users' do
+      let(:application_setting) { Gitlab::CurrentSettings.current_application_settings }
+
+      before do
+        allow(ApplicationSetting).to receive(:current_without_cache) { application_setting }
+      end
+
+      it 'returns error' do
+        expect(subject).to receive(:log_error).and_call_original
+        expect(result).to eq(
+          status: :error,
+          message: 'No active admin user found',
+          last_step: :validate_admins
+        )
+
+        expect(Group.count).to eq(0)
+      end
+    end
+
+    context 'with application settings and admin users' do
+      let(:group) { result[:group] }
+      let(:application_setting) { Gitlab::CurrentSettings.current_application_settings }
+
+      let!(:user) { create(:user, :admin) }
+
+      before do
+        allow(ApplicationSetting).to receive(:current_without_cache) { application_setting }
+      end
+
+      it 'returns correct keys' do
+        expect(result.keys).to contain_exactly(
+          :status, :group
+        )
+      end
+
+      it "tracks successful install" do
+        expect(::Gitlab::Tracking).to receive(:event).with(
+          'instance_administrators_group', 'group_created'
+        )
+
+        result
+      end
+
+      it 'creates group' do
+        expect(result[:status]).to eq(:success)
+        expect(group).to be_persisted
+        expect(group.name).to eq('GitLab Instance Administrators')
+        expect(group.path).to start_with('gitlab-instance-administrators')
+        expect(group.path.split('-').last.length).to eq(8)
+        expect(group.visibility_level).to eq(described_class::VISIBILITY_LEVEL)
+      end
+
+      it 'adds all admins as maintainers' do
+        admin1 = create(:user, :admin)
+        admin2 = create(:user, :admin)
+        create(:user)
+
+        expect(result[:status]).to eq(:success)
+        expect(group.members.collect(&:user)).to contain_exactly(user, admin1, admin2)
+        expect(group.members.collect(&:access_level)).to contain_exactly(
+          Gitlab::Access::OWNER,
+          Gitlab::Access::MAINTAINER,
+          Gitlab::Access::MAINTAINER
+        )
+      end
+
+      it 'saves the group id' do
+        expect(result[:status]).to eq(:success)
+        expect(application_setting.instance_administrators_group_id).to eq(group.id)
+      end
+
+      it 'returns error when saving group ID fails' do
+        allow(application_setting).to receive(:save) { false }
+
+        expect(result).to eq(
+          status: :error,
+          message: 'Could not save group ID',
+          last_step: :save_group_id
+        )
+      end
+
+      context 'when group already exists' do
+        let(:existing_group) { create(:group) }
+
+        before do
+          admin1 = create(:user, :admin)
+          admin2 = create(:user, :admin)
+
+          existing_group.add_owner(user)
+          existing_group.add_users([admin1, admin2], Gitlab::Access::MAINTAINER)
+
+          application_setting.instance_administrators_group_id = existing_group.id
+        end
+
+        it 'returns success' do
+          expect(result).to eq(
+            status: :success,
+            group: existing_group
+          )
+
+          expect(Group.count).to eq(1)
+        end
+      end
+
+      context 'when group cannot be created' do
+        let(:group) { build(:group) }
+
+        before do
+          group.errors.add(:base, "Test error")
+
+          expect_next_instance_of(::Groups::CreateService) do |group_create_service|
+            expect(group_create_service).to receive(:execute)
+              .and_return(group)
+          end
+        end
+
+        it 'returns error' do
+          expect(subject).to receive(:log_error).and_call_original
+          expect(result).to eq(
+            status: :error,
+            message: 'Could not create group',
+            last_step: :create_group
+          )
+        end
+      end
+
+      context 'when user cannot be added to group' do
+        before do
+          subject.instance_variable_set(:@instance_admins, [user, build(:user, :admin)])
+        end
+
+        it 'returns error' do
+          expect(subject).to receive(:log_error).and_call_original
+          expect(result).to eq(
+            status: :error,
+            message: 'Could not add admins as members',
+            last_step: :add_group_members
+          )
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb

@@ -39,11 +39,10 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
       end
       it 'returns error' do
-        expect(subject).to receive(:log_error).and_call_original
         expect(result).to eq(
           status: :error,
           message: 'No active admin user found',
-          last_step: :validate_admins
+          last_step: :create_group
         )
         expect(Project.count).to eq(0)
@@ -78,8 +77,8 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
       it_behaves_like 'has prometheus service', 'http://localhost:9090'
       it "tracks successful install" do
-        expect(::Gitlab::Tracking).to receive(:event)
-        expect(::Gitlab::Tracking).to receive(:event).with("self_monitoring", "project_created")
+        expect(::Gitlab::Tracking).to receive(:event).twice
+        expect(::Gitlab::Tracking).to receive(:event).with('self_monitoring', 'project_created')
         result
       end
@@ -87,10 +86,6 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
       it 'creates group' do
         expect(result[:status]).to eq(:success)
         expect(group).to be_persisted
-        expect(group.name).to eq('GitLab Instance Administrators')
-        expect(group.path).to start_with('gitlab-instance-administrators')
-        expect(group.path.split('-').last.length).to eq(8)
-        expect(group.visibility_level).to eq(described_class::VISIBILITY_LEVEL)
       end
       it 'creates project with internal visibility' do
@@ -120,19 +115,9 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
         expect(File).to exist("doc/#{path}.md")
       end
-      it 'adds all admins as maintainers' do
-        admin1 = create(:user, :admin)
-        admin2 = create(:user, :admin)
-        create(:user)
-
+      it 'creates project with group as owner' do
         expect(result[:status]).to eq(:success)
         expect(project.owner).to eq(group)
-        expect(group.members.collect(&:user)).to contain_exactly(user, admin1, admin2)
-        expect(group.members.collect(&:access_level)).to contain_exactly(
-          Gitlab::Access::OWNER,
-          Gitlab::Access::MAINTAINER,
-          Gitlab::Access::MAINTAINER
-        )
       end
       it 'saves the project id' do
@@ -141,7 +126,10 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
       end
       it 'returns error when saving project ID fails' do
-        allow(application_setting).to receive(:save) { false }
+        allow(application_setting).to receive(:update).and_call_original
+        allow(application_setting).to receive(:update)
+          .with(instance_administration_project_id: anything)
+          .and_return(false)
         expect(result).to eq(
           status: :error,
@@ -155,12 +143,7 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
         let(:existing_project) { create(:project, namespace: existing_group) }
         before do
-          admin1 = create(:user, :admin)
-          admin2 = create(:user, :admin)
-
-          existing_group.add_owner(user)
-          existing_group.add_users([admin1, admin2], Gitlab::Access::MAINTAINER)
-
+          application_setting.instance_administrators_group_id = existing_group.id
           application_setting.instance_administration_project_id = existing_project.id
         end
@@ -272,21 +255,6 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do
         end
       end
-      context 'when user cannot be added to project' do
-        before do
-          subject.instance_variable_set(:@instance_admins, [user, build(:user, :admin)])
-        end
-
-        it 'returns error' do
-          expect(subject).to receive(:log_error).and_call_original
-          expect(result).to eq(
-            status: :error,
-            message: 'Could not add admins as members',
-            last_step: :add_group_members
-          )
-        end
-      end
-
       context 'when prometheus manual configuration cannot be saved' do
         let(:prometheus_settings) do
           {

spec/lib/sentry/client/issue_spec.rb

@@ -30,7 +30,7 @@ describe Sentry::Client::Issue do
     let(:default_httparty_options) do
       {
         follow_redirects: false,
-        headers: { "Authorization" => "Bearer test-token" }
+        headers: { 'Content-Type' => 'application/json', 'Authorization' => "Bearer test-token" }
       }
     end

spec/services/pages_domains/create_acme_order_service_spec.rb

@@ -45,12 +45,34 @@ describe PagesDomains::CreateAcmeOrderService do
     expect { OpenSSL::PKey::RSA.new(saved_order.private_key) }.not_to raise_error
   end
-  it 'properly saves order attributes' do
+  it 'properly saves order url' do
     service.execute
     saved_order = PagesDomainAcmeOrder.last
     expect(saved_order.url).to eq(order_double.url)
-    expect(saved_order.expires_at).to be_like_time(order_double.expires)
+  end
+
+  context 'when order expires in 2 days' do
+    it 'sets expiration time in 2 hours' do
+      Timecop.freeze do
+        service.execute
+
+        saved_order = PagesDomainAcmeOrder.last
+        expect(saved_order.expires_at).to be_like_time(2.hours.from_now)
+      end
+    end
+  end
+
+  context 'when order expires in an hour' do
+    it 'sets expiration time accordingly to order' do
+      Timecop.freeze do
+        allow(order_double).to receive(:expires).and_return(1.hour.from_now)
+        service.execute
+
+        saved_order = PagesDomainAcmeOrder.last
+        expect(saved_order.expires_at).to be_like_time(1.hour.from_now)
+      end
+    end
   end
   it 'properly saves challenge attributes' do