Merge branch '341267-use-allowlist-for-import-part-2' into 'master'

Use allowlist of allowed attributes for imported models (part 2)

See merge request gitlab-org/gitlab!71046

ee/spec/lib/ee/gitlab/import_export/project/tree_restorer_spec.rb

@@ -132,4 +132,23 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
       expect(project.push_rule.deny_delete_tag).to be_truthy
     end
   end
+
+  describe 'boards' do
+    let_it_be(:project) { create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') }
+
+    let(:user) { create(:user) }
+
+    before do
+      setup_import_export_config('complex')
+      restored_project_json
+    end
+
+    it 'has milestone associated with the issue board' do
+      expect(Project.find_by_path('project').boards.find_by_name('TestBoardABC').milestone.name).to eq('test milestone')
+    end
+
+    it 'has milestone associated with the issue board list' do
+      expect(Project.find_by_path('project').boards.find_by_name('TestBoardABC').lists.first.milestone.name).to eq('test milestone')
+    end
+  end
 end

ee/spec/lib/gitlab/import_export/attributes_permitter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::ImportExport::AttributesPermitter do
+  describe '#permitted_attributes_defined?' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:attributes_permitter) { described_class.new }
+
+    where(:relation_name, :permitted_attributes_defined) do
+      :push_rule    | true
+      :issuable_sla | false
+    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
+        it_behaves_like 'a permitted attribute', relation_sym, permitted_attributes
+      end
+    end
+  end
+end

lib/gitlab/import_export/attributes_permitter.rb

@@ -44,7 +44,7 @@ module Gitlab
 
       # 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
+      DISABLED_RELATION_NAMES = %i[user author issuable_sla].freeze
 
       def initialize(config: ImportExport::Config.new.to_h)
         @config = config

lib/gitlab/import_export/base/relation_factory.rb

@@ -183,7 +183,7 @@ module Gitlab
 
         def parsed_relation_hash
           strong_memoize(:parsed_relation_hash) do
-            if Feature.enabled?(:permitted_attributes_for_import_export, default_enabled: :yaml) && attributes_permitter.permitted_attributes_defined?(@relation_sym)
+            if use_attributes_permitter? && 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)
@@ -195,6 +195,10 @@ module Gitlab
           @attributes_permitter ||= Gitlab::ImportExport::AttributesPermitter.new
         end
 
+        def use_attributes_permitter?
+          Feature.enabled?(:permitted_attributes_for_import_export, default_enabled: :yaml)
+        end
+
         def existing_or_new_object
           # Only find existing records to avoid mapping tables such as milestones
           # Otherwise always create the record, skipping the extra SELECT clause.

lib/gitlab/import_export/group/import_export.yml

@@ -10,6 +10,7 @@ tree:
     - labels:
       - :priorities
     - boards:
+      - :milestone
       - lists:
         - label:
           - :priorities

lib/gitlab/import_export/group/relation_factory.rb

@@ -36,6 +36,10 @@ module Gitlab
 
           @relation_hash['group_id'] = @importable.id
         end
+
+        def use_attributes_permitter?
+          false
+        end
       end
     end
   end

lib/gitlab/import_export/project/import_export.yml

@@ -131,7 +131,6 @@ included_attributes:
     - :link_url
     - :name
     - :project_id
-    - :type
     - :updated_at
   pipeline_schedules:
     - :active
@@ -155,6 +154,77 @@ included_attributes:
     - :enabled
     - :project_id
     - :updated_at
+  boards:
+    - :project_id
+    - :created_at
+    - :updated_at
+    - :group_id
+    - :weight
+    - :name
+    - :hide_backlog_list
+    - :hide_closed_list
+  lists:
+    - :list_type
+    - :position
+    - :created_at
+    - :updated_at
+    - :user_id
+    - :max_issue_count
+    - :max_issue_weight
+    - :limit_metric
+  custom_attributes:
+    - :created_at
+    - :updated_at
+    - :project_id
+    - :key
+    - :value
+  label:
+    - :title
+    - :color
+    - :project_id
+    - :group_id
+    - :created_at
+    - :updated_at
+    - :template
+    - :description
+    - :priority
+  labels:
+    - :title
+    - :color
+    - :project_id
+    - :group_id
+    - :created_at
+    - :updated_at
+    - :template
+    - :description
+    - :priority
+  priorities:
+    - :project_id
+    - :priority
+    - :created_at
+    - :updated_at
+  milestone:
+    - :iid
+    - :title
+    - :project_id
+    - :group_id
+    - :description
+    - :due_date
+    - :created_at
+    - :updated_at
+    - :start_date
+    - :state
+  milestones:
+    - :iid
+    - :title
+    - :project_id
+    - :group_id
+    - :description
+    - :due_date
+    - :created_at
+    - :updated_at
+    - :start_date
+    - :state
 
 # Do not include the following attributes for the models specified.
 excluded_attributes:
@@ -498,6 +568,10 @@ ee:
         - :deploy_access_levels
       - :security_setting
       - :push_rule
+      - boards:
+        - :milestone
+        - lists:
+          - :milestone
 
   included_attributes:
     issuable_sla:

spec/fixtures/lib/gitlab/import_export/complex/project.json

@@ -7759,7 +7759,29 @@
       "created_at": "2019-06-06T14:01:06.204Z",
       "updated_at": "2019-06-06T14:22:37.045Z",
       "name": "TestBoardABC",
-      "milestone_id": null,
+      "milestone": {
+        "id": 1,
+        "title": "test milestone",
+        "project_id": 8,
+        "description": "test milestone",
+        "due_date": null,
+        "created_at": "2016-06-14T15:02:04.415Z",
+        "updated_at": "2016-06-14T15:02:04.415Z",
+        "state": "active",
+        "iid": 1,
+        "events": [
+          {
+            "id": 487,
+            "target_type": "Milestone",
+            "target_id": 1,
+            "project_id": 46,
+            "created_at": "2016-06-14T15:02:04.418Z",
+            "updated_at": "2016-06-14T15:02:04.418Z",
+            "action": 1,
+            "author_id": 18
+          }
+        ]
+      },
       "group_id": null,
       "weight": null,
       "lists": [
@@ -7772,7 +7794,29 @@
           "created_at": "2019-06-06T14:01:06.214Z",
           "updated_at": "2019-06-06T14:01:06.214Z",
           "user_id": null,
-          "milestone_id": null
+          "milestone": {
+            "id": 1,
+            "title": "test milestone",
+            "project_id": 8,
+            "description": "test milestone",
+            "due_date": null,
+            "created_at": "2016-06-14T15:02:04.415Z",
+            "updated_at": "2016-06-14T15:02:04.415Z",
+            "state": "active",
+            "iid": 1,
+            "events": [
+              {
+                "id": 487,
+                "target_type": "Milestone",
+                "target_id": 1,
+                "project_id": 46,
+                "created_at": "2016-06-14T15:02:04.418Z",
+                "updated_at": "2016-06-14T15:02:04.418Z",
+                "action": 1,
+                "author_id": 18
+              }
+            ]
+          }
         },
         {
           "id": 61,
@@ -7783,7 +7827,29 @@
           "created_at": "2019-06-06T14:01:43.197Z",
           "updated_at": "2019-06-06T14:01:43.197Z",
           "user_id": null,
-          "milestone_id": null,
+          "milestone": {
+            "id": 1,
+            "title": "test milestone",
+            "project_id": 8,
+            "description": "test milestone",
+            "due_date": null,
+            "created_at": "2016-06-14T15:02:04.415Z",
+            "updated_at": "2016-06-14T15:02:04.415Z",
+            "state": "active",
+            "iid": 1,
+            "events": [
+              {
+                "id": 487,
+                "target_type": "Milestone",
+                "target_id": 1,
+                "project_id": 46,
+                "created_at": "2016-06-14T15:02:04.418Z",
+                "updated_at": "2016-06-14T15:02:04.418Z",
+                "action": 1,
+                "author_id": 18
+              }
+            ]
+          },
           "label": {
             "id": 20,
             "title": "testlabel",
@@ -7807,7 +7873,29 @@
           "created_at": "2019-06-06T14:01:06.221Z",
           "updated_at": "2019-06-06T14:01:06.221Z",
           "user_id": null,
-          "milestone_id": null
+          "milestone": {
+            "id": 1,
+            "title": "test milestone",
+            "project_id": 8,
+            "description": "test milestone",
+            "due_date": null,
+            "created_at": "2016-06-14T15:02:04.415Z",
+            "updated_at": "2016-06-14T15:02:04.415Z",
+            "state": "active",
+            "iid": 1,
+            "events": [
+              {
+                "id": 487,
+                "target_type": "Milestone",
+                "target_id": 1,
+                "project_id": 46,
+                "created_at": "2016-06-14T15:02:04.418Z",
+                "updated_at": "2016-06-14T15:02:04.418Z",
+                "action": 1,
+                "author_id": 18
+              }
+            ]
+          }
         }
       ]
     }

spec/fixtures/lib/gitlab/import_export/complex/tree/project/boards.ndjson

-{"id":29,"project_id":49,"created_at":"2019-06-06T14:01:06.204Z","updated_at":"2019-06-06T14:22:37.045Z","name":"TestBoardABC","milestone_id":null,"group_id":null,"weight":null,"lists":[{"id":59,"board_id":29,"label_id":null,"list_type":"backlog","position":null,"created_at":"2019-06-06T14:01:06.214Z","updated_at":"2019-06-06T14:01:06.214Z","user_id":null,"milestone_id":null},{"id":61,"board_id":29,"label_id":20,"list_type":"label","position":0,"created_at":"2019-06-06T14:01:43.197Z","updated_at":"2019-06-06T14:01:43.197Z","user_id":null,"milestone_id":null,"label":{"id":20,"title":"testlabel","color":"#0033CC","project_id":49,"created_at":"2019-06-06T14:01:19.698Z","updated_at":"2019-06-06T14:01:19.698Z","template":false,"description":null,"group_id":null,"type":"ProjectLabel","priorities":[]}},{"id":60,"board_id":29,"label_id":null,"list_type":"closed","position":null,"created_at":"2019-06-06T14:01:06.221Z","updated_at":"2019-06-06T14:01:06.221Z","user_id":null,"milestone_id":null}]}
+{"id":29,"project_id":49,"created_at":"2019-06-06T14:01:06.204Z","updated_at":"2019-06-06T14:22:37.045Z","name":"TestBoardABC","group_id":null,"weight":null,"milestone":{"id":1,"title":"test milestone","project_id":8,"description":"test milestone","due_date":null,"created_at":"2016-06-14T15:02:04.415Z","updated_at":"2016-06-14T15:02:04.415Z","state":"active","iid":1,"events":[{"id":487,"target_type":"Milestone","target_id":1,"project_id":46,"created_at":"2016-06-14T15:02:04.418Z","updated_at":"2016-06-14T15:02:04.418Z","action":1,"author_id":18}]},"lists":[{"id":59,"board_id":29,"label_id":null,"list_type":"backlog","position":null,"created_at":"2019-06-06T14:01:06.214Z","updated_at":"2019-06-06T14:01:06.214Z","user_id":null,"milestone":{"id":1,"title":"test milestone","project_id":8,"description":"test milestone","due_date":null,"created_at":"2016-06-14T15:02:04.415Z","updated_at":"2016-06-14T15:02:04.415Z","state":"active","iid":1,"events":[{"id":487,"target_type":"Milestone","target_id":1,"project_id":46,"created_at":"2016-06-14T15:02:04.418Z","updated_at":"2016-06-14T15:02:04.418Z","action":1,"author_id":18}]}},{"id":61,"board_id":29,"label_id":20,"list_type":"label","position":0,"created_at":"2019-06-06T14:01:43.197Z","updated_at":"2019-06-06T14:01:43.197Z","user_id":null,"milestone":{"id":1,"title":"test milestone","project_id":8,"description":"test milestone","due_date":null,"created_at":"2016-06-14T15:02:04.415Z","updated_at":"2016-06-14T15:02:04.415Z","state":"active","iid":1,"events":[{"id":487,"target_type":"Milestone","target_id":1,"project_id":46,"created_at":"2016-06-14T15:02:04.418Z","updated_at":"2016-06-14T15:02:04.418Z","action":1,"author_id":18}]},"label":{"id":20,"title":"testlabel","color":"#0033CC","project_id":49,"created_at":"2019-06-06T14:01:19.698Z","updated_at":"2019-06-06T14:01:19.698Z","template":false,"description":null,"group_id":null,"type":"ProjectLabel","priorities":[]}},{"id":60,"board_id":29,"label_id":null,"list_type":"closed","position":null,"created_at":"2019-06-06T14:01:06.221Z","updated_at":"2019-06-06T14:01:06.221Z","user_id":null,"milestone":{"id":1,"title":"test milestone","project_id":8,"description":"test milestone","due_date":null,"created_at":"2016-06-14T15:02:04.415Z","updated_at":"2016-06-14T15:02:04.415Z","state":"active","iid":1,"events":[{"id":487,"target_type":"Milestone","target_id":1,"project_id":46,"created_at":"2016-06-14T15:02:04.418Z","updated_at":"2016-06-14T15:02:04.418Z","action":1,"author_id":18}]}}]}

spec/lib/gitlab/import_export/attributes_permitter_spec.rb

@@ -83,14 +83,15 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
     where(:relation_name, :permitted_attributes_defined) do
       :user                   | false
       :author                 | false
-      :ci_cd_settings         | false
-      :issuable_sla           | false
-      :push_rule              | false
+      :ci_cd_settings         | true
       :metrics_setting        | true
       :project_badges         | true
       :pipeline_schedules     | true
       :error_tracking_setting | true
       :auto_devops            | true
+      :boards                 | true
+      :custom_attributes      | true
+      :labels                 | true
     end
 
     with_them do
@@ -99,47 +100,11 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
   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
+        it_behaves_like 'a permitted attribute', relation_sym, permitted_attributes
       end
     end
   end

spec/support/shared_examples/lib/gitlab/import_export/attributes_permitter_shared_examples.rb

+# frozen_string_literal: true
+RSpec.shared_examples 'a permitted attribute' do |relation_sym, permitted_attributes|
+  let(:prohibited_attributes) { %i[remote_url my_attributes my_ids token my_id test] }
+
+  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