Use allowlist of allowed attributes for imported models

This change changes the behavior that is responsible for filtering out
attributes when importing new project. We were cleaning attributes based
on list of excluded patterns and attributes. For MetricsSettings,
ProjectBadges, PipelineSchedules, ErrorTrackingSettings and
ProjectAutoDevops we are changing that behavior.

Changelog: changed

config/feature_flags/development/permitted_attributes_for_import_export.yml

+---
+name: permitted_attributes_for_import_export
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70168
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340789
+milestone: '14.4'
+type: development
+group: group::import
+default_enabled: true

lib/gitlab/import_export/attributes_permitter.rb

@@ -42,6 +42,10 @@ module Gitlab
     class AttributesPermitter
       attr_reader :permitted_attributes
 
+      # We want to use AttributesCleaner for these relations instead, in the future this should be removed to make sure
+      # we are using AttributesPermitter for every imported relation.
+      DISABLED_RELATION_NAMES = %i[user author ci_cd_settings issuable_sla push_rule].freeze
+
       def initialize(config: ImportExport::Config.new.to_h)
         @config = config
         @attributes_finder = Gitlab::ImportExport::AttributesFinder.new(config: @config)
@@ -50,16 +54,20 @@ module Gitlab
         build_permitted_attributes
       end
 
-      def permit(relation_name, relation_hash)
-        permitted_attributes = permitted_attributes_for(relation_name)
+      def permit(relation_sym, relation_hash)
+        permitted_attributes = permitted_attributes_for(relation_sym)
 
         relation_hash.select do |key, _|
-          permitted_attributes.include?(key)
+          permitted_attributes.include?(key.to_sym)
+        end
       end
+
+      def permitted_attributes_for(relation_sym)
+        @permitted_attributes[relation_sym] || []
       end
 
-      def permitted_attributes_for(relation_name)
-        @permitted_attributes[relation_name] || []
+      def permitted_attributes_defined?(relation_sym)
+        !DISABLED_RELATION_NAMES.include?(relation_sym) && @attributes_finder.included_attributes.key?(relation_sym)
       end
 
       private

lib/gitlab/import_export/base/relation_factory.rb

@@ -29,7 +29,7 @@ module Gitlab
            owner_id
          ].freeze
 
-        TOKEN_RESET_MODELS = %i[Project Namespace Group Ci::Trigger Ci::Build Ci::Runner ProjectHook].freeze
+        TOKEN_RESET_MODELS = %i[Project Namespace Group Ci::Trigger Ci::Build Ci::Runner ProjectHook ErrorTracking::ProjectErrorTrackingSetting].freeze
 
         def self.create(*args, **kwargs)
           new(*args, **kwargs).create
@@ -45,6 +45,7 @@ module Gitlab
         end
 
         def initialize(relation_sym:, relation_index:, relation_hash:, members_mapper:, object_builder:, user:, importable:, excluded_keys: [])
+          @relation_sym = relation_sym
           @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym
           @relation_index = relation_index
           @relation_hash = relation_hash.except('noteable_id')
@@ -181,8 +182,17 @@ module Gitlab
         end
 
         def parsed_relation_hash
-          @parsed_relation_hash ||= Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: @relation_hash,
-                                                                                 relation_class: relation_class)
+          strong_memoize(:parsed_relation_hash) do
+            if Feature.enabled?(:permitted_attributes_for_import_export, default_enabled: :yaml) && attributes_permitter.permitted_attributes_defined?(@relation_sym)
+              attributes_permitter.permit(@relation_sym, @relation_hash)
+            else
+              Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: @relation_hash, relation_class: relation_class)
+            end
+          end
+        end
+
+        def attributes_permitter
+          @attributes_permitter ||= Gitlab::ImportExport::AttributesPermitter.new
         end
 
         def existing_or_new_object

lib/gitlab/import_export/project/import_export.yml

@@ -121,6 +121,41 @@ included_attributes:
     - :name
   ci_cd_settings:
     - :group_runners_enabled
+  metrics_setting:
+    - :dashboard_timezone
+    - :external_dashboard_url
+    - :project_id
+  project_badges:
+    - :created_at
+    - :group_id
+    - :image_url
+    - :link_url
+    - :name
+    - :project_id
+    - :type
+    - :updated_at
+  pipeline_schedules:
+    - :active
+    - :created_at
+    - :cron
+    - :cron_timezone
+    - :description
+    - :next_run_at
+    - :owner_id
+    - :project_id
+    - :ref
+    - :updated_at
+  error_tracking_setting:
+    - :api_url
+    - :organization_name
+    - :project_id
+    - :project_name
+  auto_devops:
+    - :created_at
+    - :deploy_strategy
+    - :enabled
+    - :project_id
+    - :updated_at
 
 # Do not include the following attributes for the models specified.
 excluded_attributes:

spec/lib/gitlab/import_export/attributes_permitter_spec.rb

@@ -74,4 +74,73 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
       expect(subject.permitted_attributes_for(:labels)).to contain_exactly(:title, :description, :type, :priorities)
     end
   end
+
+  describe '#permitted_attributes_defined?' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:attributes_permitter) { described_class.new }
+
+    where(:relation_name, :permitted_attributes_defined) do
+      :user                   | false
+      :author                 | false
+      :ci_cd_settings         | false
+      :issuable_sla           | false
+      :push_rule              | false
+      :metrics_setting        | true
+      :project_badges         | true
+      :pipeline_schedules     | true
+      :error_tracking_setting | true
+      :auto_devops            | true
+    end
+
+    with_them do
+      it { expect(attributes_permitter.permitted_attributes_defined?(relation_name)).to eq(permitted_attributes_defined) }
+    end
+  end
+
+  describe 'included_attributes for Project' do
+    let(:prohibited_attributes) { %i[remote_url my_attributes my_ids token my_id test] }
+
+    subject { described_class.new }
+
+    Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes|
+      context "for #{relation_sym}" do
+        let(:import_export_config) { Gitlab::ImportExport::Config.new.to_h }
+        let(:project_relation_factory) { Gitlab::ImportExport::Project::RelationFactory }
+
+        let(:relation_hash) { (permitted_attributes + prohibited_attributes).map(&:to_s).zip([]).to_h }
+        let(:relation_name) { project_relation_factory.overrides[relation_sym]&.to_sym || relation_sym }
+        let(:relation_class) { project_relation_factory.relation_class(relation_name) }
+        let(:excluded_keys) { import_export_config.dig(:excluded_keys, relation_sym) || [] }
+
+        let(:cleaned_hash) do
+          Gitlab::ImportExport::AttributeCleaner.new(
+            relation_hash: relation_hash,
+            relation_class: relation_class,
+            excluded_keys: excluded_keys
+          ).clean
+        end
+
+        let(:permitted_hash) { subject.permit(relation_sym, relation_hash) }
+
+        if described_class.new.permitted_attributes_defined?(relation_sym)
+          it 'contains only attributes that are defined as permitted in the import/export config' do
+            expect(permitted_hash.keys).to contain_exactly(*permitted_attributes.map(&:to_s))
+          end
+
+          it 'does not contain attributes that would be cleaned with AttributeCleaner' do
+            expect(cleaned_hash.keys).to include(*permitted_hash.keys)
+          end
+
+          it 'does not contain prohibited attributes that are not related to given relation' do
+            expect(permitted_hash.keys).not_to include(*prohibited_attributes.map(&:to_s))
+          end
+        else
+          it 'is disabled' do
+            expect(subject).not_to be_permitted_attributes_defined(relation_sym)
+          end
+        end
+      end
+    end
+  end
 end