Commit 0fac0300 authored by nmilojevic1's avatar nmilojevic1 Committed by Stan Hu

Fix missing attributes for import_export

Squashed commits:
 - Fix rubocop offences
 - Parametrized rspec testing all relation paths
 - Use AttributeCleaner instead duplicating code
 - Add changelog for strong validation
 - Add frozen string literal for references
 - Add excluded attributes to configuration
parent ef56b5a2
---
title: Strong validate import export references
merge_request: 19682
author:
type: added
...@@ -185,6 +185,7 @@ excluded_attributes: ...@@ -185,6 +185,7 @@ excluded_attributes:
- :moved_to_id - :moved_to_id
- :state_id - :state_id
- :duplicated_to_id - :duplicated_to_id
- :promoted_to_epic_id
merge_request: merge_request:
- :milestone_id - :milestone_id
- :ref_fetched - :ref_fetched
...@@ -222,6 +223,8 @@ excluded_attributes: ...@@ -222,6 +223,8 @@ excluded_attributes:
- :upstream_pipeline_id - :upstream_pipeline_id
- :resource_group_id - :resource_group_id
- :waiting_for_resource_at - :waiting_for_resource_at
sentry_issue:
- :issue_id
push_event_payload: push_event_payload:
- :event_id - :event_id
project_badges: project_badges:
...@@ -262,6 +265,8 @@ excluded_attributes: ...@@ -262,6 +265,8 @@ excluded_attributes:
- :issue_id - :issue_id
zoom_meetings: zoom_meetings:
- :issue_id - :issue_id
design:
- :issue_id
designs: designs:
- :issue_id - :issue_id
design_versions: design_versions:
...@@ -287,7 +292,20 @@ excluded_attributes: ...@@ -287,7 +292,20 @@ excluded_attributes:
- :pipeline_id - :pipeline_id
merge_access_levels: merge_access_levels:
- :protected_branch_id - :protected_branch_id
push_access_levels:
- :protected_branch_id
unprotect_access_levels:
- :protected_branch_id
create_access_levels:
- :protected_tag_id
deploy_access_levels:
- :protected_environment_id
boards:
- :milestone_id
lists:
- :board_id
- :label_id
- :milestone_id
methods: methods:
notes: notes:
- :type - :type
......
...@@ -16,7 +16,7 @@ describe 'Import/Export attribute configuration' do ...@@ -16,7 +16,7 @@ describe 'Import/Export attribute configuration' do
let(:safe_model_attributes) { YAML.load_file(safe_attributes_file) } let(:safe_model_attributes) { YAML.load_file(safe_attributes_file) }
it 'has no new columns' do it 'has no new columns' do
relation_names.each do |relation_name| project_relation_names.each do |relation_name|
relation_class = relation_class_for_name(relation_name) relation_class = relation_class_for_name(relation_name)
relation_attributes = relation_class.new.attributes.keys - relation_class.encrypted_attributes.keys.map(&:to_s) relation_attributes = relation_class.new.attributes.keys - relation_class.encrypted_attributes.keys.map(&:to_s)
......
...@@ -11,7 +11,7 @@ describe 'Import/Export model configuration' do ...@@ -11,7 +11,7 @@ describe 'Import/Export model configuration' do
let(:all_models_yml) { 'spec/lib/gitlab/import_export/all_models.yml' } let(:all_models_yml) { 'spec/lib/gitlab/import_export/all_models.yml' }
let(:all_models_hash) { YAML.load_file(all_models_yml) } let(:all_models_hash) { YAML.load_file(all_models_yml) }
let(:current_models) { setup_models } let(:current_models) { setup_models }
let(:model_names) { relation_names } let(:model_names) { project_relation_names }
it 'has no new models' do it 'has no new models' do
model_names.each do |model_name| model_names.each do |model_name|
......
# frozen_string_literal: true
require 'spec_helper' require 'spec_helper'
# Part of the test security suite for the Import/Export feature # Part of the test security suite for the Import/Export feature
...@@ -7,20 +9,26 @@ require 'spec_helper' ...@@ -7,20 +9,26 @@ require 'spec_helper'
# or to be blacklisted by using the import_export.yml configuration file. # or to be blacklisted by using the import_export.yml configuration file.
# Likewise, new models added to import_export.yml, will need to be added with their correspondent relations # Likewise, new models added to import_export.yml, will need to be added with their correspondent relations
# to this spec. # to this spec.
describe 'Import/Export references configuration' do describe 'Import/Export Project configuration' do
include ConfigurationHelper include ConfigurationHelper
it 'has no prohibited keys' do where(:relation_path, :relation_name) do
relation_names.each do |relation_name| project_relation_paths.map {|a| [a.join("."), a.last]}
relation_class = relation_class_for_name(relation_name) end
relation_attributes = relation_class.new.attributes.keys - relation_class.encrypted_attributes.keys.map(&:to_s)
current_attributes = parsed_attributes(relation_name, relation_attributes) with_them do
next if params[:relation_name] == "author"
prohibited_keys = current_attributes.select do |attribute| context "where relation #{params[:relation_path]}" do
prohibited_key?(attribute) || !relation_class.attribute_method?(attribute) it 'does not have prohibited keys' do
relation_class = relation_class_for_name(relation_name)
relation_attributes = relation_class.new.attributes.keys - relation_class.encrypted_attributes.keys.map(&:to_s)
current_attributes = parsed_attributes(relation_name, relation_attributes)
prohibited_keys = current_attributes.select do |attribute|
prohibited_key?(attribute) || !relation_class.attribute_method?(attribute)
end
expect(prohibited_keys).to be_empty, failure_message(relation_class.to_s, prohibited_keys)
end end
expect(prohibited_keys).to be_empty, failure_message(relation_class.to_s, prohibited_keys)
end end
end end
......
# frozen_string_literal: true # frozen_string_literal: true
module ConfigurationHelper module ConfigurationHelper
ALLOWED_REFERENCES = Gitlab::ImportExport::RelationFactory::PROJECT_REFERENCES + Gitlab::ImportExport::RelationFactory::USER_REFERENCES + %w[group_id commit_id]
PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_html\Z/).freeze
# Returns a list of models from hashes/arrays contained in +project_tree+ # Returns a list of models from hashes/arrays contained in +project_tree+
def names_from_tree(project_tree) def names_from_tree(project_tree)
project_tree.map do |branch_or_model| project_tree.map do |branch_or_model|
...@@ -13,17 +10,34 @@ module ConfigurationHelper ...@@ -13,17 +10,34 @@ module ConfigurationHelper
end end
end end
# - flattens hash to list all relation paths
def flat_hash(hash, path = [])
new_hash = {}
hash.each_pair do |key, val|
new_hash[path + [key]] = val
if val.is_a?(Hash) && val.present?
new_hash.merge!(flat_hash(val, path + [key]))
end
end
new_hash
end
def config_hash def config_hash
Gitlab::ImportExport::Config.new.to_h.deep_stringify_keys Gitlab::ImportExport::Config.new.to_h.deep_stringify_keys
end end
def relation_names def project_relation_paths
# - project is not part of the tree, so it has to be added manually.
flat_hash({ "project" => config_hash.dig('tree', 'project') }).keys
end
def project_relation_names
names = names_from_tree(config_hash.dig('tree', 'project')) names = names_from_tree(config_hash.dig('tree', 'project'))
# Remove duplicated or add missing models # Remove duplicated or add missing models
# - project is not part of the tree, so it has to be added manually. # - project is not part of the tree, so it has to be added manually.
# - milestone, labels, merge_request have both singular and plural versions in the tree, so remove the duplicates. # - milestone, labels, merge_request have both singular and plural versions in the tree, so remove the duplicates.
# - User, Author... Models we do not care about for checking models # - User, Author... Models we do not care about for checking models
names.flatten.uniq - %w(milestones labels user author merge_request) + ['project'] names.flatten.uniq - %w(milestones labels user author merge_request design) + ['project']
end end
def relation_class_for_name(relation_name) def relation_class_for_name(relation_name)
...@@ -40,11 +54,11 @@ module ConfigurationHelper ...@@ -40,11 +54,11 @@ module ConfigurationHelper
end end
def prohibited_key?(key) def prohibited_key?(key)
key =~ PROHIBITED_REFERENCES && !permitted_key?(key) key =~ Gitlab::ImportExport::AttributeCleaner::PROHIBITED_REFERENCES && !permitted_key?(key)
end end
def permitted_key?(key) def permitted_key?(key)
ALLOWED_REFERENCES.include?(key) Gitlab::ImportExport::AttributeCleaner::ALLOWED_REFERENCES.include?(key)
end end
def associations_for(safe_model) def associations_for(safe_model)
......
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