Commit b654203a authored by Stan Hu's avatar Stan Hu Committed by James Lopez

Fix project imports for pipelines for merge requests

When a pipeline for a merge request was exported, the merge request was
not properly mapped, which caused a validation error, "Validation
failed: Merge request can't be blank".

Since neither merge requests nor CI pipelines have an explicit
relationship in `import_export.yml`, the full merge_request hash was
never exported. To make these older exports work, we now process
`merge_request_id` by replacing it with the newly-imported ID.

Closes https://gitlab.com/gitlab-org/gitlab/issues/31414
parent 38078c85
---
title: Fix project imports for pipelines for merge requests
merge_request: 17799
author:
type: fixed
...@@ -52,6 +52,11 @@ module Gitlab ...@@ -52,6 +52,11 @@ module Gitlab
project: restored_project) project: restored_project)
end end
# A Hash of the imported merge request ID -> imported ID.
def merge_requests_mapping
@merge_requests_mapping ||= {}
end
# Loops through the tree of models defined in import_export.yml and # Loops through the tree of models defined in import_export.yml and
# finds them in the imported JSON so they can be instantiated and saved # finds them in the imported JSON so they can be instantiated and saved
# in the DB. The structure and relationships between models are guessed from # in the DB. The structure and relationships between models are guessed from
...@@ -80,10 +85,26 @@ module Gitlab ...@@ -80,10 +85,26 @@ module Gitlab
@saved = false unless restored_project.append_or_update_attribute(relation_key, relation_hash) @saved = false unless restored_project.append_or_update_attribute(relation_key, relation_hash)
save_id_mappings(relation_key, relation_hash_batch, relation_hash)
# Restore the project again, extra query that skips holding the AR objects in memory # Restore the project again, extra query that skips holding the AR objects in memory
@restored_project = Project.find(@project_id) @restored_project = Project.find(@project_id)
end end
# Older, serialized CI pipeline exports may only have a
# merge_request_id and not the full hash of the merge request. To
# import these pipelines, we need to preserve the mapping between
# the old and new the merge request ID.
def save_id_mappings(relation_key, relation_hash_batch, relation_hash)
return unless relation_key == 'merge_requests'
relation_hash = Array(relation_hash)
Array(relation_hash_batch).each_with_index do |raw_data, index|
merge_requests_mapping[raw_data['id']] = relation_hash[index]['id']
end
end
# Remove project models that became group models as we found them at group level. # Remove project models that became group models as we found them at group level.
# This no longer required saving them at the root project level. # This no longer required saving them at the root project level.
# For example, in the case of an existing group label that matched the title. # For example, in the case of an existing group label that matched the title.
...@@ -222,6 +243,7 @@ module Gitlab ...@@ -222,6 +243,7 @@ module Gitlab
relation_sym: relation_key.to_sym, relation_sym: relation_key.to_sym,
relation_hash: relation_hash, relation_hash: relation_hash,
members_mapper: members_mapper, members_mapper: members_mapper,
merge_requests_mapping: merge_requests_mapping,
user: @user, user: @user,
project: @restored_project, project: @restored_project,
excluded_keys: excluded_keys_for_relation(relation_key)) excluded_keys: excluded_keys_for_relation(relation_key))
......
...@@ -55,10 +55,11 @@ module Gitlab ...@@ -55,10 +55,11 @@ module Gitlab
relation_name.to_s.constantize relation_name.to_s.constantize
end end
def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project:, excluded_keys: []) def initialize(relation_sym:, relation_hash:, members_mapper:, merge_requests_mapping:, user:, project:, excluded_keys: [])
@relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym
@relation_hash = relation_hash.except('noteable_id') @relation_hash = relation_hash.except('noteable_id')
@members_mapper = members_mapper @members_mapper = members_mapper
@merge_requests_mapping = merge_requests_mapping
@user = user @user = user
@project = project @project = project
@imported_object_retries = 0 @imported_object_retries = 0
...@@ -109,7 +110,10 @@ module Gitlab ...@@ -109,7 +110,10 @@ module Gitlab
update_group_references update_group_references
remove_duplicate_assignees remove_duplicate_assignees
setup_pipeline if @relation_name == :'Ci::Pipeline' if @relation_name == :'Ci::Pipeline'
update_merge_request_references
setup_pipeline
end
reset_tokens! reset_tokens!
remove_encrypted_attributes! remove_encrypted_attributes!
...@@ -194,6 +198,28 @@ module Gitlab ...@@ -194,6 +198,28 @@ module Gitlab
@relation_hash['group_id'] = @project.namespace_id @relation_hash['group_id'] = @project.namespace_id
end end
# This code is a workaround for broken project exports that don't
# export merge requests with CI pipelines (i.e. exports that were
# generated from
# https://gitlab.com/gitlab-org/gitlab/merge_requests/17844).
# This method can be removed in GitLab 12.6.
def update_merge_request_references
# If a merge request was properly created, we don't need to fix
# up this export.
return if @relation_hash['merge_request']
merge_request_id = @relation_hash['merge_request_id']
return unless merge_request_id
new_merge_request_id = @merge_requests_mapping[merge_request_id]
return unless new_merge_request_id
@relation_hash['merge_request_id'] = new_merge_request_id
parsed_relation_hash['merge_request_id'] = new_merge_request_id
end
def reset_tokens! def reset_tokens!
return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name) return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name)
......
...@@ -6175,6 +6175,8 @@ ...@@ -6175,6 +6175,8 @@
"finished_at": null, "finished_at": null,
"user_id": 9999, "user_id": 9999,
"duration": null, "duration": null,
"source": "push",
"merge_request_id": null,
"notes": [ "notes": [
{ {
"id": 999, "id": 999,
......
...@@ -96,6 +96,17 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -96,6 +96,17 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
expect(Ci::Pipeline.where(ref: nil)).not_to be_empty expect(Ci::Pipeline.where(ref: nil)).not_to be_empty
end end
it 'restores pipeline for merge request' do
pipeline = Ci::Pipeline.find_by_sha('048721d90c449b244b7b4c53a9186b04330174ec')
expect(pipeline).to be_valid
expect(pipeline.tag).to be_falsey
expect(pipeline.source).to eq('merge_request_event')
expect(pipeline.merge_request.id).to be > 0
expect(pipeline.merge_request.target_branch).to eq('feature')
expect(pipeline.merge_request.source_branch).to eq('feature_conflict')
end
it 'preserves updated_at on issues' do it 'preserves updated_at on issues' do
issue = Issue.where(description: 'Aliquam enim illo et possimus.').first issue = Issue.where(description: 'Aliquam enim illo et possimus.').first
......
...@@ -3,12 +3,14 @@ require 'spec_helper' ...@@ -3,12 +3,14 @@ require 'spec_helper'
describe Gitlab::ImportExport::RelationFactory do describe Gitlab::ImportExport::RelationFactory do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:members_mapper) { double('members_mapper').as_null_object } let(:members_mapper) { double('members_mapper').as_null_object }
let(:merge_requests_mapping) { {} }
let(:user) { create(:admin) } let(:user) { create(:admin) }
let(:excluded_keys) { [] } let(:excluded_keys) { [] }
let(:created_object) do let(:created_object) do
described_class.create(relation_sym: relation_sym, described_class.create(relation_sym: relation_sym,
relation_hash: relation_hash, relation_hash: relation_hash,
members_mapper: members_mapper, members_mapper: members_mapper,
merge_requests_mapping: merge_requests_mapping,
user: user, user: user,
project: project, project: project,
excluded_keys: excluded_keys) excluded_keys: excluded_keys)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment