Add latest changes from gitlab-org/gitlab@master

6 files changed, 379 insertions, 24 deletions

diff --git a/gems/gem-pg.gitlab-ci.yml b/gems/gem-pg.gitlab-ci.yml
index c48e18fa297..2806437e15b 100644
--- a/gems/gem-pg.gitlab-ci.yml
+++ b/gems/gem-pg.gitlab-ci.yml
@@ -63,6 +63,7 @@ rubocop:
   rules:
     - exists: ["$[[inputs.gem_path_prefix]]$[[inputs.gem_name]]/.rubocop.yml"]
   script:
+    - $CI_PROJECT_DIR/scripts/validate-monorepo-gem "$[[inputs.gem_name]]"
     - bundle exec rubocop
 
 rspec:
diff --git a/gems/gem.gitlab-ci.yml b/gems/gem.gitlab-ci.yml
index a379a887bdd..3ced4b5e364 100644
--- a/gems/gem.gitlab-ci.yml
+++ b/gems/gem.gitlab-ci.yml
@@ -50,6 +50,7 @@ rubocop:
   rules:
     - exists: ["$[[inputs.gem_path_prefix]]$[[inputs.gem_name]]/.rubocop.yml"]
   script:
+    - $CI_PROJECT_DIR/scripts/validate-monorepo-gem "$[[inputs.gem_name]]"
     - bundle exec rubocop
 
 rspec:
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/gitlab_client.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/gitlab_client.rb
index b28d44195cb..e23dbe46cae 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/gitlab_client.rb
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/gitlab_client.rb
@@ -13,13 +13,50 @@ module Gitlab
         @base_uri = 'https://gitlab.com/api/v4'
       end
 
+      # This looks at the system notes of the merge request to detect if it has been updated by anyone other than the
+      # current housekeeper user. If it has then it assumes that they did this for a reason and we can skip updating
+      # this detail of the merge request. Otherwise we assume we should generate it again using the latest output.
+      def non_housekeeper_changes(
+        source_project_id:,
+        source_branch:,
+        target_branch:,
+        target_project_id:
+      )
+
+        iid = get_existing_merge_request(
+          source_project_id: source_project_id,
+          source_branch: source_branch,
+          target_branch: target_branch,
+          target_project_id: target_project_id
+        )
+
+        return [] if iid.nil?
+
+        merge_request_notes = get_merge_request_notes(target_project_id: target_project_id, iid: iid)
+
+        changes = Set.new
+
+        merge_request_notes.each do |note|
+          next false unless note["system"]
+          next false if note["author"]["id"] == current_user_id
+
+          changes << :title if note['body'].start_with?("changed title from")
+          changes << :description if note['body'] == "changed the description"
+          changes << :code if note['body'].match?(/added \d+ commit/)
+        end
+
+        changes.to_a
+      end
+
       def create_or_update_merge_request(
         source_project_id:,
         title:,
         description:,
         source_branch:,
         target_branch:,
-        target_project_id:
+        target_project_id:,
+        update_title:,
+        update_description:
       )
         existing_iid = get_existing_merge_request(
           source_project_id: source_project_id,
@@ -33,7 +70,9 @@ module Gitlab
             existing_iid: existing_iid,
             title: title,
             description: description,
-            target_project_id: target_project_id
+            target_project_id: target_project_id,
+            update_title:,
+            update_description:
           )
         else
           create_merge_request(
@@ -49,6 +88,39 @@ module Gitlab
 
       private
 
+      def get_merge_request_notes(target_project_id:, iid:)
+        response = HTTParty.get(
+          "#{@base_uri}/projects/#{target_project_id}/merge_requests/#{iid}/notes",
+          query: {
+            per_page: 100
+          },
+          headers: {
+            "Private-Token" => @token
+          }
+        )
+
+        unless (200..299).cover?(response.code)
+          raise Error,
+            "Failed to get merge request notes with response code: #{response.code} and body:\n#{response.body}"
+        end
+
+        JSON.parse(response.body)
+      end
+
+      def current_user_id
+        @current_user_id = begin
+          response = HTTParty.get("#{@base_uri}/user")
+
+          unless (200..299).cover?(response.code)
+            raise Error,
+              "Failed with response code: #{response.code} and body:\n#{response.body}"
+          end
+
+          data = JSON.parse(response.body)
+          data['id']
+        end
+      end
+
       def get_existing_merge_request(source_project_id:, source_branch:, target_branch:, target_project_id:)
         response = HTTParty.get("#{@base_uri}/projects/#{target_project_id}/merge_requests",
           query: {
@@ -78,8 +150,13 @@ module Gitlab
       end
 
       def create_merge_request(
-        source_project_id:, title:, description:, source_branch:, target_branch:,
-        target_project_id:)
+        source_project_id:,
+        title:,
+        description:,
+        source_branch:,
+        target_branch:,
+        target_project_id:
+      )
         response = HTTParty.post("#{@base_uri}/projects/#{source_project_id}/merge_requests", body: {
           title: title,
           description: description,
@@ -98,11 +175,23 @@ module Gitlab
           "Failed with response code: #{response.code} and body:\n#{response.body}"
       end
 
-      def update_existing_merge_request(existing_iid:, title:, description:, target_project_id:)
-        response = HTTParty.put("#{@base_uri}/projects/#{target_project_id}/merge_requests/#{existing_iid}", body: {
-          title: title,
-          description: description
-        }.to_json,
+      def update_existing_merge_request(
+        existing_iid:,
+        title:,
+        description:,
+        target_project_id:,
+        update_title:,
+        update_description:
+      )
+        body = {}
+
+        body[:title] = title if update_title
+        body[:description] = description if update_description
+
+        return if body.empty?
+
+        response = HTTParty.put("#{@base_uri}/projects/#{target_project_id}/merge_requests/#{existing_iid}",
+          body: body.to_json,
           headers: {
             'Private-Token' => @token,
             'Content-Type' => 'application/json'
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
index 76d629e29a3..ad43ec014f5 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
@@ -70,7 +70,16 @@ module Gitlab
       def create(change, branch_name)
         dry_run(change, branch_name)
 
-        Shell.execute('git', 'push', '-f', 'housekeeper', "#{branch_name}:#{branch_name}")
+        non_housekeeper_changes = gitlab_client.non_housekeeper_changes(
+          source_project_id: housekeeper_fork_project_id,
+          source_branch: branch_name,
+          target_branch: 'master',
+          target_project_id: housekeeper_target_project_id
+        )
+
+        unless non_housekeeper_changes.include?(:code)
+          Shell.execute('git', 'push', '-f', 'housekeeper', "#{branch_name}:#{branch_name}")
+        end
 
         gitlab_client.create_or_update_merge_request(
           source_project_id: housekeeper_fork_project_id,
@@ -78,7 +87,9 @@ module Gitlab
           description: change.description,
           source_branch: branch_name,
           target_branch: 'master',
-          target_project_id: housekeeper_target_project_id
+          target_project_id: housekeeper_target_project_id,
+          update_title: !non_housekeeper_changes.include?(:title),
+          update_description: !non_housekeeper_changes.include?(:description)
         )
       end
 
diff --git a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/gitlab_client_spec.rb b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/gitlab_client_spec.rb
index 36b2afdc306..c105ecf030b 100644
--- a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/gitlab_client_spec.rb
+++ b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/gitlab_client_spec.rb
@@ -3,9 +3,146 @@
 require 'spec_helper'
 require 'gitlab/housekeeper/gitlab_client'
 
+# rubocop:disable RSpec/MultipleMemoizedHelpers -- there are lots of parameters at play
 RSpec.describe ::Gitlab::Housekeeper::GitlabClient do
   let(:client) { described_class.new }
 
+  before do
+    stub_env('HOUSEKEEPER_GITLAB_API_TOKEN', 'the-api-token')
+  end
+
+  describe '#non_housekeeper_changes' do
+    let(:housekeeper_user_id) { 666 }
+
+    let(:added_commit_note) do
+      {
+        id: 1698248524,
+        body: "added 1 commit\n\n<ul><li>41b3a17f - Update stuff to test...",
+        author: { "id" => 1234 },
+        system: true
+      }
+    end
+
+    let(:irrelevant_note1) do
+      {
+        id: 1698248523,
+        body: "changed this line in ...",
+        author: { "id" => 1234 },
+        system: true
+      }
+    end
+
+    let(:not_a_system_note) do
+      {
+        id: 1698248524,
+        body: "added 1 commit\n\n<ul><li>41b3a17f - Update stuff to test...",
+        author: { "id" => 1234 },
+        system: false
+      }
+    end
+
+    let(:updated_title_note) do
+      {
+        id: 1698248527,
+        body: "changed title from **Add sharding{- -}key `namespace_id` to achievements**...",
+        author: { "id" => 1235 },
+        system: true
+      }
+    end
+
+    let(:updated_description_note) do
+      {
+        id: 1698248530,
+        body: "changed the description",
+        author: { "id" => 1236 },
+        system: true
+      }
+    end
+
+    let(:notes) do
+      [irrelevant_note1, not_a_system_note]
+    end
+
+    subject(:non_housekeeper_changes) do
+      client.non_housekeeper_changes(
+        source_project_id: 123,
+        target_project_id: 456,
+        source_branch: 'the-source-branch',
+        target_branch: 'the-target-branch'
+      )
+    end
+
+    before do
+      # Get the current housekeeper user
+      stub_request(:get, "https://gitlab.com/api/v4/user")
+        .to_return(status: 200, body: { id: housekeeper_user_id }.to_json)
+
+      # Get the id of the current merge request
+      stub_request(:get, "https://gitlab.com/api/v4/projects/456/merge_requests?state=opened&source_branch=the-source-branch&target_branch=the-target-branch&source_project_id=123")
+        .with(
+          headers: {
+            'Private-Token' => 'the-api-token'
+          }
+        )
+        .to_return(status: 200, body: [{ iid: 8765 }].to_json)
+
+      # Get the notes of the current merge request
+      stub_request(:get, "https://gitlab.com/api/v4/projects/456/merge_requests/8765/notes?per_page=100")
+        .with(
+          headers: {
+            'Private-Token' => 'the-api-token'
+          }
+        )
+          .to_return(status: 200, body: notes.to_json)
+    end
+
+    it 'does not match irrelevant notes' do
+      expect(non_housekeeper_changes).to eq([])
+    end
+
+    context 'when all important things change' do
+      let(:notes) do
+        [not_a_system_note, updated_title_note, updated_description_note, added_commit_note]
+      end
+
+      it 'returns :title, :description, :code' do
+        expect(non_housekeeper_changes).to include(:title)
+        expect(non_housekeeper_changes).to include(:description)
+        expect(non_housekeeper_changes).to include(:code)
+      end
+    end
+
+    context 'when title changes' do
+      let(:notes) do
+        [not_a_system_note, updated_title_note]
+      end
+
+      it 'returns :title, :description, :code' do
+        expect(non_housekeeper_changes).to include(:title)
+        expect(non_housekeeper_changes).not_to include(:description)
+        expect(non_housekeeper_changes).not_to include(:code)
+      end
+    end
+
+    context 'when description changes' do
+      let(:notes) do
+        [not_a_system_note, updated_description_note]
+      end
+
+      it 'returns :title, :description, :code' do
+        expect(non_housekeeper_changes).not_to include(:title)
+        expect(non_housekeeper_changes).to include(:description)
+        expect(non_housekeeper_changes).not_to include(:code)
+      end
+    end
+
+    context 'when the merge request does not exist' do
+      it 'returns empty array' do
+        expect(non_housekeeper_changes).to eq([])
+      end
+    end
+  end
+
   describe '#create_or_update_merge_request' do
     let(:params) do
       {
@@ -14,15 +151,15 @@ RSpec.describe ::Gitlab::Housekeeper::GitlabClient do
         description: 'This merge request is pretty good.',
         source_branch: 'the-source-branch',
         target_branch: 'the-target-branch',
-        target_project_id: 456
+        target_project_id: 456,
+        update_title: true,
+        update_description: true
       }
     end
 
     let(:existing_mrs) { [] }
 
     before do
-      stub_env('HOUSEKEEPER_GITLAB_API_TOKEN', 'the-api-token')
-
       # Stub the check to see if the merge request already exists
       stub_request(:get, "https://gitlab.com/api/v4/projects/456/merge_requests?state=opened&source_branch=the-source-branch&target_branch=the-target-branch&source_project_id=123")
         .with(
@@ -85,6 +222,48 @@ RSpec.describe ::Gitlab::Housekeeper::GitlabClient do
           expect { client.create_or_update_merge_request(**params) }.to raise_error(described_class::Error)
         end
       end
+
+      context 'when update_title: false' do
+        it 'does not update the title' do
+          stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
+            .with(
+              body: {
+                description: "This merge request is pretty good."
+              }.to_json,
+              headers: {
+                'Content-Type' => 'application/json',
+                'Private-Token' => 'the-api-token'
+              }
+            ).to_return(status: 200, body: "")
+
+          client.create_or_update_merge_request(**params.merge(update_title: false))
+          expect(stub).to have_been_requested
+        end
+      end
+
+      context 'when update_description: false' do
+        it 'does not update the description' do
+          stub = stub_request(:put, "https://gitlab.com/api/v4/projects/456/merge_requests/1234")
+            .with(
+              body: {
+                title: "A new merge request!"
+              }.to_json,
+              headers: {
+                'Content-Type' => 'application/json',
+                'Private-Token' => 'the-api-token'
+              }
+            ).to_return(status: 200, body: "")
+
+          client.create_or_update_merge_request(**params.merge(update_description: false))
+          expect(stub).to have_been_requested
+        end
+      end
+
+      context 'when there is nothing to update' do
+        it 'does not make a request' do
+          client.create_or_update_merge_request(**params.merge(update_description: false, update_title: false))
+        end
+      end
     end
 
     it 'raises an error when unsuccessful response' do
@@ -97,3 +276,4 @@ RSpec.describe ::Gitlab::Housekeeper::GitlabClient do
     end
   end
 end
+# rubocop:enable RSpec/MultipleMemoizedHelpers
diff --git a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/runner_spec.rb b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/runner_spec.rb
index 49b4926dbdd..c7ba4302d88 100644
--- a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/runner_spec.rb
+++ b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/runner_spec.rb
@@ -3,6 +3,7 @@
 require 'spec_helper'
 require 'gitlab/housekeeper/runner'
 
+# rubocop:disable RSpec/MultipleMemoizedHelpers -- there are lots of parameters at play
 RSpec.describe ::Gitlab::Housekeeper::Runner do
   let(:fake_keep) { instance_double(Class) }
 
@@ -44,18 +45,34 @@ RSpec.describe ::Gitlab::Housekeeper::Runner do
   end
 
   describe '#run' do
+    let(:git) { instance_double(::Gitlab::Housekeeper::Git) }
+    let(:gitlab_client) { instance_double(::Gitlab::Housekeeper::GitlabClient) }
+
     before do
       stub_env('HOUSEKEEPER_FORK_PROJECT_ID', '123')
       stub_env('HOUSEKEEPER_TARGET_PROJECT_ID', '456')
+
+      allow(::Gitlab::Housekeeper::Git).to receive(:new)
+        .and_return(git)
+
+      allow(git).to receive(:with_branch_from_branch)
+        .and_yield
+      allow(git).to receive(:commit_in_branch).with(change1)
+        .and_return('the-identifier-for-the-first-change')
+      allow(git).to receive(:commit_in_branch).with(change2)
+        .and_return('the-identifier-for-the-second-change')
+
+      allow(::Gitlab::Housekeeper::GitlabClient).to receive(:new)
+        .and_return(gitlab_client)
+
+      allow(gitlab_client).to receive(:non_housekeeper_changes)
+        .and_return([])
+
+      allow(::Gitlab::Housekeeper::Shell).to receive(:execute)
     end
 
     it 'loops over the keeps and creates MRs limited by max_mrs' do
       # Branches get created
-      git = instance_double(::Gitlab::Housekeeper::Git)
-      expect(::Gitlab::Housekeeper::Git).to receive(:new)
-        .and_return(git)
-      expect(git).to receive(:with_branch_from_branch)
-        .and_yield
       expect(git).to receive(:commit_in_branch).with(change1)
         .and_return('the-identifier-for-the-first-change')
       expect(git).to receive(:commit_in_branch).with(change2)
@@ -76,9 +93,6 @@ RSpec.describe ::Gitlab::Housekeeper::Runner do
           'the-identifier-for-the-second-change:the-identifier-for-the-second-change')
 
       # Merge requests get created
-      gitlab_client = instance_double(::Gitlab::Housekeeper::GitlabClient)
-      expect(::Gitlab::Housekeeper::GitlabClient).to receive(:new)
-        .and_return(gitlab_client)
       expect(gitlab_client).to receive(:create_or_update_merge_request)
         .with(
           source_project_id: '123',
@@ -86,7 +100,9 @@ RSpec.describe ::Gitlab::Housekeeper::Runner do
           description: 'The description of the MR',
           source_branch: 'the-identifier-for-the-first-change',
           target_branch: 'master',
-          target_project_id: '456'
+          target_project_id: '456',
+          update_title: true,
+          update_description: true
         )
       expect(gitlab_client).to receive(:create_or_update_merge_request)
         .with(
@@ -95,10 +111,67 @@ RSpec.describe ::Gitlab::Housekeeper::Runner do
           description: 'The description of the MR',
           source_branch: 'the-identifier-for-the-second-change',
           target_branch: 'master',
-          target_project_id: '456'
+          target_project_id: '456',
+          update_title: true,
+          update_description: true
         )
 
       described_class.new(max_mrs: 2, keeps: [fake_keep]).run
     end
+
+    context 'when title, description, code has changed already' do
+      it 'does not update the changed details' do
+        # First change has updated code and description so should only update title
+        expect(gitlab_client).to receive(:non_housekeeper_changes)
+          .with(
+            source_project_id: '123',
+            source_branch: 'the-identifier-for-the-first-change',
+            target_branch: 'master',
+            target_project_id: '456'
+          ).and_return([:code, :description])
+
+        # Second change has updated title and description so it should push the code
+        expect(gitlab_client).to receive(:non_housekeeper_changes)
+          .with(
+            source_project_id: '123',
+            source_branch: 'the-identifier-for-the-second-change',
+            target_branch: 'master',
+            target_project_id: '456'
+          ).and_return([:title, :description])
+
+        expect(::Gitlab::Housekeeper::Shell).not_to receive(:execute)
+          .with('git', 'push', '-f', 'housekeeper',
+            'the-identifier-for-the-first-change:the-identifier-for-the-first-change')
+        expect(::Gitlab::Housekeeper::Shell).to receive(:execute)
+          .with('git', 'push', '-f', 'housekeeper',
+            'the-identifier-for-the-second-change:the-identifier-for-the-second-change')
+
+        expect(gitlab_client).to receive(:create_or_update_merge_request)
+          .with(
+            source_project_id: '123',
+            title: 'The title of MR1',
+            description: 'The description of the MR',
+            source_branch: 'the-identifier-for-the-first-change',
+            target_branch: 'master',
+            target_project_id: '456',
+            update_title: true,
+            update_description: false
+          )
+        expect(gitlab_client).to receive(:create_or_update_merge_request)
+          .with(
+            source_project_id: '123',
+            title: 'The title of MR2',
+            description: 'The description of the MR',
+            source_branch: 'the-identifier-for-the-second-change',
+            target_branch: 'master',
+            target_project_id: '456',
+            update_title: false,
+            update_description: false
+          )
+
+        described_class.new(max_mrs: 2, keeps: [fake_keep]).run
+      end
+    end
   end
 end
+# rubocop:enable RSpec/MultipleMemoizedHelpers