Automatic merge of gitlab-org/gitlab master

.gitlab/merge_request_templates/Documentation.md

@@ -23,7 +23,13 @@
     - ~"development guidelines" when changing docs under `doc/development/*`, `CONTRIBUTING.md`, or `README.md`.
     - ~"development guidelines" and ~"Documentation guidelines" when changing docs under `development/documentation/*`.
     - ~"development guidelines" and ~"Description templates (.gitlab/\*)" when creating/updating issue and MR description templates.
-  - [ ] Assign the [designated Technical Writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments).
+  - [ ] [Request a review](https://docs.gitlab.com/ee/development/code_review.html#dogfooding-the-reviewers-feature)
+        from the [designated Technical Writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments).
+
+/label ~"development guidelines"
+/label ~"Description templates (.gitlab/\*)"
+
+/assign me
 
 Do not add the ~"feature", ~"frontend", ~"backend", ~"bug", or ~"database" labels if you are only updating documentation. These labels will cause the MR to be added to code verification QA issues.
 

app/graphql/types/release_assets_type.rb

@@ -13,7 +13,7 @@ module Types
 
     field :count, GraphQL::INT_TYPE, null: true, method: :assets_count,
           description: 'Number of assets of the release.'
-    field :links, Types::ReleaseAssetLinkType.connection_type, null: true,
+    field :links, Types::ReleaseAssetLinkType.connection_type, null: true, method: :sorted_links,
           description: 'Asset links of the release.'
     field :sources, Types::ReleaseSourceType.connection_type, null: true,
           description: 'Sources of the release.'

app/models/release.rb

@@ -13,6 +13,7 @@ class Release < ApplicationRecord
   belongs_to :author, class_name: 'User'
 
   has_many :links, class_name: 'Releases::Link'
+  has_many :sorted_links, -> { sorted }, class_name: 'Releases::Link'
 
   has_many :milestone_releases
   has_many :milestones, through: :milestone_releases
@@ -27,7 +28,10 @@ class Release < ApplicationRecord
   validates :links, nested_attributes_duplicates: { scope: :release, child_attributes: %i[name url filepath] }
 
   scope :sorted, -> { order(released_at: :desc) }
-  scope :preloaded, -> { includes(:evidences, :milestones, project: [:project_feature, :route, { namespace: :route }]) }
+  scope :preloaded, -> {
+    includes(:author, :evidences, :milestones, :links, :sorted_links,
+             project: [:project_feature, :route, { namespace: :route }])
+  }
   scope :with_project_and_namespace, -> { includes(project: :namespace) }
   scope :recent, -> { sorted.limit(MAX_NUMBER_TO_DISPLAY) }
   scope :without_evidence, -> { left_joins(:evidences).where(::Releases::Evidence.arel_table[:id].eq(nil)) }
@@ -58,8 +62,8 @@ class Release < ApplicationRecord
   end
 
   def assets_count(except: [])
-    links_count = links.count
-    sources_count = except.include?(:sources) ? 0 : sources.count
+    links_count = links.size
+    sources_count = except.include?(:sources) ? 0 : sources.size
 
     links_count + sources_count
   end

changelogs/unreleased/327584-security-orchestration-policy-schedule-tables.yml

+---
+title: Add table to store Security Orchestration Policy Schedules
+merge_request: 59842
+author:
+type: added

changelogs/unreleased/cat-releases-api-perf-nplusone.yml

+---
+title: Fix three N+1s in Releases API entity generation
+merge_request: 60189
+author:
+type: performance

db/migrate/20210412111213_create_security_orchestration_policy_rule_schedule.rb

+# frozen_string_literal: true
+
+class CreateSecurityOrchestrationPolicyRuleSchedule < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_PREFIX = 'index_sop_schedules_'
+
+  disable_ddl_transaction!
+
+  def up
+    table_comment = { owner: 'group::container security', description: 'Schedules used to store relationship between project and security policy repository' }
+
+    create_table_with_constraints :security_orchestration_policy_rule_schedules, comment: table_comment.to_json do |t|
+      t.timestamps_with_timezone
+      t.datetime_with_timezone :next_run_at, null: true
+
+      t.references :security_orchestration_policy_configuration, null: false, foreign_key: { to_table: :security_orchestration_policy_configurations, on_delete: :cascade }, index: { name: INDEX_PREFIX + 'on_sop_configuration_id' }
+      t.references :user, null: false, foreign_key: { on_delete: :cascade }, index: { name: INDEX_PREFIX + 'on_user_id' }
+
+      t.integer :policy_index, null: false
+      t.text :cron, null: false
+
+      t.text_limit :cron, 255
+    end
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :security_orchestration_policy_rule_schedules
+    end
+  end
+end

db/migrate/20210412172030_add_configured_at_to_security_orchestration_policy.rb

+# frozen_string_literal: true
+
+class AddConfiguredAtToSecurityOrchestrationPolicy < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :security_orchestration_policy_configurations, :configured_at, :datetime_with_timezone, null: true
+  end
+end

db/schema_migrations/20210412111213

+c75ab8ef4d6a4ff20109e1c5d054521bd8cd79680f96f4d9e55331d69bac73d6
\ No newline at end of file

db/schema_migrations/20210412172030

+063cfa0d8a4b9d3947aaf55f0587f6a2a9521866b6e10fc307c5cc82ca3a0623
\ No newline at end of file

db/structure.sql

@@ -17417,7 +17417,8 @@ CREATE TABLE security_orchestration_policy_configurations (
     project_id bigint NOT NULL,
     security_policy_management_project_id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
-    updated_at timestamp with time zone NOT NULL
+    updated_at timestamp with time zone NOT NULL,
+    configured_at timestamp with time zone
 );
 
 COMMENT ON TABLE security_orchestration_policy_configurations IS '{"owner":"group::container security","description":"Configuration used to store relationship between project and security policy repository"}';
@@ -17431,6 +17432,29 @@ CREATE SEQUENCE security_orchestration_policy_configurations_id_seq
 
 ALTER SEQUENCE security_orchestration_policy_configurations_id_seq OWNED BY security_orchestration_policy_configurations.id;
 
+CREATE TABLE security_orchestration_policy_rule_schedules (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    next_run_at timestamp with time zone,
+    security_orchestration_policy_configuration_id bigint NOT NULL,
+    user_id bigint NOT NULL,
+    policy_index integer NOT NULL,
+    cron text NOT NULL,
+    CONSTRAINT check_915825a76e CHECK ((char_length(cron) <= 255))
+);
+
+COMMENT ON TABLE security_orchestration_policy_rule_schedules IS '{"owner":"group::container security","description":"Schedules used to store relationship between project and security policy repository"}';
+
+CREATE SEQUENCE security_orchestration_policy_rule_schedules_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE security_orchestration_policy_rule_schedules_id_seq OWNED BY security_orchestration_policy_rule_schedules.id;
+
 CREATE TABLE security_scans (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -19848,6 +19872,8 @@ ALTER TABLE ONLY security_findings ALTER COLUMN id SET DEFAULT nextval('security
 
 ALTER TABLE ONLY security_orchestration_policy_configurations ALTER COLUMN id SET DEFAULT nextval('security_orchestration_policy_configurations_id_seq'::regclass);
 
+ALTER TABLE ONLY security_orchestration_policy_rule_schedules ALTER COLUMN id SET DEFAULT nextval('security_orchestration_policy_rule_schedules_id_seq'::regclass);
+
 ALTER TABLE ONLY security_scans ALTER COLUMN id SET DEFAULT nextval('security_scans_id_seq'::regclass);
 
 ALTER TABLE ONLY self_managed_prometheus_alert_events ALTER COLUMN id SET DEFAULT nextval('self_managed_prometheus_alert_events_id_seq'::regclass);
@@ -21410,6 +21436,9 @@ ALTER TABLE ONLY security_findings
 ALTER TABLE ONLY security_orchestration_policy_configurations
     ADD CONSTRAINT security_orchestration_policy_configurations_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY security_orchestration_policy_rule_schedules
+    ADD CONSTRAINT security_orchestration_policy_rule_schedules_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY security_scans
     ADD CONSTRAINT security_scans_pkey PRIMARY KEY (id);
 
@@ -24072,6 +24101,10 @@ CREATE UNIQUE INDEX index_sop_configs_on_project_id ON security_orchestration_po
 
 CREATE INDEX index_sop_configurations_project_id_policy_project_id ON security_orchestration_policy_configurations USING btree (security_policy_management_project_id, project_id);
 
+CREATE INDEX index_sop_schedules_on_sop_configuration_id ON security_orchestration_policy_rule_schedules USING btree (security_orchestration_policy_configuration_id);
+
+CREATE INDEX index_sop_schedules_on_user_id ON security_orchestration_policy_rule_schedules USING btree (user_id);
+
 CREATE INDEX index_spam_logs_on_user_id ON spam_logs USING btree (user_id);
 
 CREATE INDEX index_sprints_iterations_cadence_id ON sprints USING btree (iterations_cadence_id);
@@ -25669,6 +25702,9 @@ ALTER TABLE ONLY analytics_cycle_analytics_project_stages
 ALTER TABLE ONLY packages_build_infos
     ADD CONSTRAINT fk_rails_17a9a0dffc FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY security_orchestration_policy_rule_schedules
+    ADD CONSTRAINT fk_rails_17ade83f17 FOREIGN KEY (security_orchestration_policy_configuration_id) REFERENCES security_orchestration_policy_configurations(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY clusters_applications_jupyter
     ADD CONSTRAINT fk_rails_17df21c98c FOREIGN KEY (cluster_id) REFERENCES clusters(id) ON DELETE CASCADE;
 
@@ -26908,6 +26944,9 @@ ALTER TABLE ONLY label_priorities
 ALTER TABLE ONLY fork_network_members
     ADD CONSTRAINT fk_rails_efccadc4ec FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY security_orchestration_policy_rule_schedules
+    ADD CONSTRAINT fk_rails_efe1d9b133 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY prometheus_alerts
     ADD CONSTRAINT fk_rails_f0e8db86aa FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/app/models/security/orchestration_policy_configuration.rb

@@ -14,6 +14,11 @@ module Security
     belongs_to :project, inverse_of: :security_orchestration_policy_configuration
     belongs_to :security_policy_management_project, class_name: 'Project', foreign_key: 'security_policy_management_project_id'
 
+    has_many :rule_schedules,
+              class_name: 'Security::OrchestrationPolicyRuleSchedule',
+              foreign_key: :security_orchestration_policy_configuration_id,
+              inverse_of: :security_orchestration_policy_configuration
+
     validates :project, presence: true, uniqueness: true
     validates :security_policy_management_project, presence: true
 

ee/app/models/security/orchestration_policy_rule_schedule.rb

+# frozen_string_literal: true
+
+module Security
+  class OrchestrationPolicyRuleSchedule < ApplicationRecord
+    self.table_name = 'security_orchestration_policy_rule_schedules'
+
+    belongs_to :owner, class_name: 'User', foreign_key: 'user_id'
+    belongs_to :security_orchestration_policy_configuration,
+               class_name: 'Security::OrchestrationPolicyConfiguration',
+               foreign_key: 'security_orchestration_policy_configuration_id'
+
+    validates :owner, presence: true
+    validates :security_orchestration_policy_configuration, presence: true
+    validates :cron, presence: true
+    validates :policy_index, presence: true
+  end
+end

ee/spec/factories/security_orchestration_policy_rule_schedules.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :security_orchestration_policy_rule_schedule, class: 'Security::OrchestrationPolicyRuleSchedule' do
+    owner { association(:user) }
+    security_orchestration_policy_configuration
+
+    policy_index { 0 }
+    cron { '*/10 * * * *' }
+  end
+end

ee/spec/models/security/orchestration_policy_configuration_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe Security::OrchestrationPolicyConfiguration do
   describe 'associations' do
     it { is_expected.to belong_to(:project).inverse_of(:security_orchestration_policy_configuration) }
     it { is_expected.to belong_to(:security_policy_management_project).class_name('Project') }
+    it { is_expected.to have_many(:rule_schedules).class_name('Security::OrchestrationPolicyRuleSchedule').inverse_of(:security_orchestration_policy_configuration) }
   end
 
   describe 'validations' do

ee/spec/models/security/orchestration_policy_rule_schedule_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::OrchestrationPolicyRuleSchedule do
+  describe 'associations' do
+    it { is_expected.to belong_to(:owner).class_name('User') }
+    it { is_expected.to belong_to(:security_orchestration_policy_configuration).class_name('Security::OrchestrationPolicyConfiguration') }
+  end
+
+  describe 'validations' do
+    subject { create(:security_orchestration_policy_rule_schedule) }
+
+    it { is_expected.to validate_presence_of(:owner) }
+    it { is_expected.to validate_presence_of(:security_orchestration_policy_configuration) }
+    it { is_expected.to validate_presence_of(:cron) }
+    it { is_expected.to validate_presence_of(:policy_index) }
+  end
+end

lib/api/entities/release.rb

@@ -28,9 +28,7 @@ module API
       expose :assets do
         expose :assets_count, as: :count
         expose :sources, using: Entities::Releases::Source, if: ->(_, _) { can_download_code? }
-        expose :links, using: Entities::Releases::Link do |release, options|
-          release.links.sorted
-        end
+        expose :sorted_links, as: :links, using: Entities::Releases::Link
       end
       expose :evidences, using: Entities::Releases::Evidence, expose_nil: false, if: ->(_, _) { can_download_code? }
       expose :_links do

spec/lib/gitlab/import_export/all_models.yml

@@ -113,6 +113,7 @@ releases:
 - author
 - project
 - links
+- sorted_links
 - milestone_releases
 - milestones
 - evidences

spec/models/release_spec.rb

@@ -54,7 +54,7 @@ RSpec.describe Release do
   end
 
   describe '#assets_count' do
-    subject { release.assets_count }
+    subject { Release.find(release.id).assets_count }
 
     it 'returns the number of sources' do
       is_expected.to eq(Gitlab::Workhorse::ARCHIVE_FORMATS.count)
@@ -68,7 +68,7 @@ RSpec.describe Release do
       end
 
       it "excludes sources count when asked" do
-        assets_count = release.assets_count(except: [:sources])
+        assets_count = Release.find(release.id).assets_count(except: [:sources])
         expect(assets_count).to eq(1)
       end
     end

spec/requests/api/releases_spec.rb

@@ -136,8 +136,8 @@ RSpec.describe API::Releases do
         get api("/projects/#{project.id}/releases", maintainer)
       end.count
 
-      create(:release, :with_evidence, project: project, tag: 'v0.1', author: maintainer)
-      create(:release, :with_evidence, project: project, tag: 'v0.1', author: maintainer)
+      create_list(:release, 2, :with_evidence, project: project, tag: 'v0.1', author: maintainer)
+      create_list(:release, 2, project: project)
 
       expect do
         get api("/projects/#{project.id}/releases", maintainer)