From de7960ea9cce3ca1f8180e7ebd80fa605357e190 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Jun 2022 11:51:07 +0200 Subject: [PATCH 01/19] Implement `on:` events CI configuration keyword --- lib/gitlab/ci/config/entry/events.rb | 34 +++++++++++++++++++++++ lib/gitlab/ci/config/entry/job.rb | 7 ++++- lib/gitlab/ci/yaml_processor/result.rb | 3 +- spec/lib/gitlab/ci/yaml_processor_spec.rb | 21 ++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 lib/gitlab/ci/config/entry/events.rb diff --git a/lib/gitlab/ci/config/entry/events.rb b/lib/gitlab/ci/config/entry/events.rb new file mode 100644 index 00000000000000..d7e6d304ca7b41 --- /dev/null +++ b/lib/gitlab/ci/config/entry/events.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + class Events < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Validatable + + validations do + validates :config, array_of_strings_or_string: true + + # TODO + # validate :allowed_events + end + + def value + # We only support webhooks today + # + events = Array(@config).map do |event| + unless event.start_with?('webhooks/') + raise ArgumentError, 'unexpected event' + end + + event.split('/').last + end + + { webhooks: events } if events.any? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 7513936a18a55e..b9423d925b6cb0 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -14,7 +14,7 @@ class Job < ::Gitlab::Config::Entry::Node ALLOWED_KEYS = %i[tags script image services start_in artifacts cache dependencies before_script after_script environment coverage retry parallel interruptible timeout - release].freeze + release on].freeze validations do validates :config, allowed_keys: Gitlab::Ci::Config::Entry::Job.allowed_keys + PROCESSABLE_ALLOWED_KEYS @@ -100,6 +100,10 @@ class Job < ::Gitlab::Config::Entry::Node description: 'Environment configuration for this job.', inherit: false + entry :on, Entry::Events, + description: 'Events a job will be triggered by.', + inherit: false + entry :coverage, Entry::Coverage, description: 'Coverage configuration for this job.', inherit: false @@ -143,6 +147,7 @@ def value cache: cache_value, tags: tags_value, when: self.when, + on: on_value, start_in: self.start_in, dependencies: dependencies, environment: environment_defined? ? environment_value : nil, diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 576fb509d476cf..1c9bb0acf143f3 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -99,7 +99,8 @@ def build_attributes(name) start_in: job[:start_in], trigger: job[:trigger], bridge_needs: job.dig(:needs, :bridge)&.first, - release: release(job) + release: release(job), + on: job[:on] }.compact }.compact end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 15567b3667330a..637fbc525e05cf 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -326,6 +326,27 @@ module Ci end end + describe 'a build with events' do + subject { described_class.new(config).execute } + + let(:config) do + YAML.dump( + annotate: { + on: 'webhooks/something', + script: 'run annotate issue' + } + ) + end + + it 'has events attribute' do + expect(subject.errors).to be_empty + + options = subject.build_attributes('annotate').dig(:options) + + expect(options).to match(a_hash_including(on: { webhooks: ['something'] })) + end + end + describe 'bridge job' do let(:config) do YAML.dump(rspec: { -- GitLab From bcb76bbd7d8d9fcd3edaeb6959fdfff21f6c14d5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Jun 2022 12:38:26 +0200 Subject: [PATCH 02/19] Add ci_workflow_events table to store CI Workflows --- .../20220628100300_add_ci_workflow_events.rb | 17 ++++++++++ db/schema_migrations/20220628100300 | 1 + db/structure.sql | 32 +++++++++++++++++++ lib/gitlab/database/gitlab_schemas.yml | 1 + 4 files changed, 51 insertions(+) create mode 100644 db/migrate/20220628100300_add_ci_workflow_events.rb create mode 100644 db/schema_migrations/20220628100300 diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb new file mode 100644 index 00000000000000..11901b5cb69af9 --- /dev/null +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddCiWorkflowEvents < Gitlab::Database::Migration[2.0] + def change + create_table :ci_workflow_events do |t| + t.timestamps_with_timezone null: false + t.bigint :project_id, null: false, index: true + t.bigint :groups, null: false # bitmask for event groups used (eg. webhooks) + t.bigint :subgroups, null: false # bitmask for event subgroup (eg. issues events) + t.integer :events, array: true, null: false, default: [] # concrete events from a subgroup + t.text :name, limit: 255, null: false, index: { unique: true } + + # initially we only support projct-level events + t.index [:project_id, :groups, :subgroups], name: 'ci_workflow_project_events_index' + end + end +end diff --git a/db/schema_migrations/20220628100300 b/db/schema_migrations/20220628100300 new file mode 100644 index 00000000000000..35f724c056b616 --- /dev/null +++ b/db/schema_migrations/20220628100300 @@ -0,0 +1 @@ +2d4653fe2a054d5542af513f82cb54ef683037558f66a5201d4ece611e611b88 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 7a966564e06dac..b0f2e524629bf5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13314,6 +13314,27 @@ CREATE SEQUENCE ci_variables_id_seq ALTER SEQUENCE ci_variables_id_seq OWNED BY ci_variables.id; +CREATE TABLE ci_workflow_events ( + id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + project_id bigint NOT NULL, + groups bigint NOT NULL, + subgroups bigint NOT NULL, + events integer[] DEFAULT '{}'::integer[] NOT NULL, + name text NOT NULL, + CONSTRAINT check_57c0faffdd CHECK ((char_length(name) <= 255)) +); + +CREATE SEQUENCE ci_workflow_events_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE ci_workflow_events_id_seq OWNED BY ci_workflow_events.id; + CREATE TABLE cluster_agent_tokens ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -22771,6 +22792,8 @@ ALTER TABLE ONLY ci_unit_tests ALTER COLUMN id SET DEFAULT nextval('ci_unit_test ALTER TABLE ONLY ci_variables ALTER COLUMN id SET DEFAULT nextval('ci_variables_id_seq'::regclass); +ALTER TABLE ONLY ci_workflow_events ALTER COLUMN id SET DEFAULT nextval('ci_workflow_events_id_seq'::regclass); + ALTER TABLE ONLY cluster_agent_tokens ALTER COLUMN id SET DEFAULT nextval('cluster_agent_tokens_id_seq'::regclass); ALTER TABLE ONLY cluster_agents ALTER COLUMN id SET DEFAULT nextval('cluster_agents_id_seq'::regclass); @@ -24510,6 +24533,9 @@ ALTER TABLE ONLY ci_unit_tests ALTER TABLE ONLY ci_variables ADD CONSTRAINT ci_variables_pkey PRIMARY KEY (id); +ALTER TABLE ONLY ci_workflow_events + ADD CONSTRAINT ci_workflow_events_pkey PRIMARY KEY (id); + ALTER TABLE ONLY cluster_agent_tokens ADD CONSTRAINT cluster_agent_tokens_pkey PRIMARY KEY (id); @@ -26798,6 +26824,8 @@ CREATE INDEX cadence_create_iterations_automation ON iterations_cadences USING b CREATE INDEX ci_builds_gitlab_monitor_metrics ON ci_builds USING btree (status, created_at, project_id) WHERE ((type)::text = 'Ci::Build'::text); +CREATE INDEX ci_workflow_project_events_index ON ci_workflow_events USING btree (project_id, groups, subgroups); + CREATE INDEX code_owner_approval_required ON protected_branches USING btree (project_id, code_owner_approval_required) WHERE (code_owner_approval_required = true); CREATE UNIQUE INDEX commit_user_mentions_on_commit_id_and_note_id_unique_index ON commit_user_mentions USING btree (commit_id, note_id); @@ -27598,6 +27626,10 @@ CREATE INDEX index_ci_variables_on_key ON ci_variables USING btree (key); CREATE UNIQUE INDEX index_ci_variables_on_project_id_and_key_and_environment_scope ON ci_variables USING btree (project_id, key, environment_scope); +CREATE UNIQUE INDEX index_ci_workflow_events_on_name ON ci_workflow_events USING btree (name); + +CREATE INDEX index_ci_workflow_events_on_project_id ON ci_workflow_events USING btree (project_id); + CREATE INDEX index_cicd_settings_on_namespace_id_where_stale_pruning_enabled ON namespace_ci_cd_settings USING btree (namespace_id) WHERE (allow_stale_runner_pruning = true); CREATE INDEX index_cluster_agent_tokens_on_agent_id_status_last_used_at ON cluster_agent_tokens USING btree (agent_id, status, last_used_at DESC NULLS LAST); diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index 0085b578102ce4..0b993fd2ce9869 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -120,6 +120,7 @@ ci_triggers: :gitlab_ci ci_unit_test_failures: :gitlab_ci ci_unit_tests: :gitlab_ci ci_variables: :gitlab_ci +ci_workflow_events: :gitlab_ci cluster_agents: :gitlab_main cluster_agent_tokens: :gitlab_main cluster_enabled_grants: :gitlab_main -- GitLab From 0a8d540a285a60025f8323ca0416f8073cce0c82 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Jun 2022 12:58:18 +0200 Subject: [PATCH 03/19] Add a few TODOs about making CI Workflows production ready --- db/migrate/20220628100300_add_ci_workflow_events.rb | 1 + lib/gitlab/ci/config/entry/events.rb | 4 ++-- lib/gitlab/ci/yaml_processor.rb | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb index 11901b5cb69af9..9c0704e695779b 100644 --- a/db/migrate/20220628100300_add_ci_workflow_events.rb +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -9,6 +9,7 @@ def change t.bigint :subgroups, null: false # bitmask for event subgroup (eg. issues events) t.integer :events, array: true, null: false, default: [] # concrete events from a subgroup t.text :name, limit: 255, null: false, index: { unique: true } + # TODO: add sha column to link to Git sha a workflow has been defined in # initially we only support projct-level events t.index [:project_id, :groups, :subgroups], name: 'ci_workflow_project_events_index' diff --git a/lib/gitlab/ci/config/entry/events.rb b/lib/gitlab/ci/config/entry/events.rb index d7e6d304ca7b41..923491f728578f 100644 --- a/lib/gitlab/ci/config/entry/events.rb +++ b/lib/gitlab/ci/config/entry/events.rb @@ -10,14 +10,14 @@ class Events < ::Gitlab::Config::Entry::Node validations do validates :config, array_of_strings_or_string: true - # TODO - # validate :allowed_events + # TODO validate :allowed_events end def value # We only support webhooks today # events = Array(@config).map do |event| + unless event.start_with?('webhooks/') raise ArgumentError, 'unexpected event' end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 15ebd50605531b..8d58ad010563b9 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -49,6 +49,11 @@ def run_logical_validations! end def validate_job!(name, job) + ## TODO add logical validation for CI workflows + # + # We initially don't want to support on: events for builds / pipelines webhooks to avoid possible circular + # workflows. + # validate_job_stage!(name, job) validate_job_dependencies!(name, job) validate_job_needs!(name, job) -- GitLab From 9ec13dcd2e78cbf9995b328a40537cffa112f93a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Jun 2022 14:41:02 +0200 Subject: [PATCH 04/19] Implement CI workflow events composite bitmask --- app/models/concerns/triggerable_hooks.rb | 7 ++ .../20220628100300_add_ci_workflow_events.rb | 2 +- lib/gitlab/ci/config/entry/events.rb | 4 +- lib/gitlab/ci/workflows/event.rb | 75 ++++++++++++++++ spec/lib/gitlab/ci/workflows/event_spec.rb | 85 +++++++++++++++++++ 5 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 lib/gitlab/ci/workflows/event.rb create mode 100644 spec/lib/gitlab/ci/workflows/event_spec.rb diff --git a/app/models/concerns/triggerable_hooks.rb b/app/models/concerns/triggerable_hooks.rb index 8fe34632430b9d..97eae56837d8fd 100644 --- a/app/models/concerns/triggerable_hooks.rb +++ b/app/models/concerns/triggerable_hooks.rb @@ -1,6 +1,13 @@ # frozen_string_literal: true module TriggerableHooks + ## TODO + # CI Workflow depend on the order of the values defined in the hash below. Add new ones at the end. + # + # > Hashes enumerate their values in the order that the corresponding keys were inserted. + # + # https://ruby-doc.org/core-2.7.0/Hash.html + # AVAILABLE_TRIGGERS = { repository_update_hooks: :repository_update_events, push_hooks: :push_events, diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb index 9c0704e695779b..9734ae057c722b 100644 --- a/db/migrate/20220628100300_add_ci_workflow_events.rb +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -11,7 +11,7 @@ def change t.text :name, limit: 255, null: false, index: { unique: true } # TODO: add sha column to link to Git sha a workflow has been defined in - # initially we only support projct-level events + # initially we only support project-level events t.index [:project_id, :groups, :subgroups], name: 'ci_workflow_project_events_index' end end diff --git a/lib/gitlab/ci/config/entry/events.rb b/lib/gitlab/ci/config/entry/events.rb index 923491f728578f..52f19936acc7db 100644 --- a/lib/gitlab/ci/config/entry/events.rb +++ b/lib/gitlab/ci/config/entry/events.rb @@ -17,14 +17,14 @@ def value # We only support webhooks today # events = Array(@config).map do |event| - - unless event.start_with?('webhooks/') + unless event.include?('/webhooks/') raise ArgumentError, 'unexpected event' end event.split('/').last end + # TODO surface workflow name { webhooks: events } if events.any? end end diff --git a/lib/gitlab/ci/workflows/event.rb b/lib/gitlab/ci/workflows/event.rb new file mode 100644 index 00000000000000..ece9133b520a17 --- /dev/null +++ b/lib/gitlab/ci/workflows/event.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Workflows + class Event + EVENT_GROUPS = { + webhooks: 1 # use bit masks + }.freeze + + # TODO event named group contains leading slash as it is optional + # rubocop:disable Lint/MixedRegexpCaptureTypes TODO + PATH_REGEXP = %r{(?[\w-]+)/webhooks/(?[\w-]+)(/?[\w-]+)?}.freeze + # rubocop:enable Lint/MixedRegexpCaptureTypes + + def initialize(path) + @path = path + @matches = path.match(PATH_REGEXP) + end + + def valid? + @matches.present? && self.class.webhooks.include?(@matches[:subgroup]) + end + + def group_mask + EVENT_GROUPS[:webhooks] + end + + def subgroup_index + raise ArgumentError unless valid? + + @index ||= self.class.webhook_index(@matches[:subgroup]) + end + + def subgroup_mask + raise ArgumentError unless valid? + + @mask ||= self.class.webhook_mask(@matches[:subgroup]) + end + + # This will return a composite subgroup bitmask + # + # TODO once we introduce more event groups, we will need to extract this to another class / method + # + def +(other) + subgroup_mask | other.subgroup_mask + end + + ## TODO + # + # Webhooks are not granular enough to expose concrete events. We would need to extend them to support events + # like issue/create, issue/update. This information is available in a payload, but we need additional code to + # connect it with workflow events. + # + def events + raise NotImplementedError + end + + def self.webhooks + ::TriggerableHooks::AVAILABLE_TRIGGERS.values.map do |subgroup| + subgroup.to_s.sub('_events', '') + end + end + + def self.webhook_index(hook_name) + webhooks.index(hook_name) + end + + def self.webhook_mask(hook_name) + 2 << webhook_index(hook_name) + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/workflows/event_spec.rb b/spec/lib/gitlab/ci/workflows/event_spec.rb new file mode 100644 index 00000000000000..679cc0546e0310 --- /dev/null +++ b/spec/lib/gitlab/ci/workflows/event_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_dependency 'app/models/concerns/triggerable_hooks' + +RSpec.describe Gitlab::Ci::Workflows::Event do + describe '.webhooks' do + it 'returns available system hooks' do + expect(described_class.webhooks).to include('issues') + end + end + + describe 'valid?' do + context 'when event is not webhooks event' do + it 'is not valid' do + event = described_class.new('workflow/cloud/event') + + expect(event).not_to be_valid + end + end + + context 'when webhook subgroup is not known' do + it 'is not valid' do + event = described_class.new('workflow/webhooks/unknown/event') + + expect(event).not_to be_valid + end + end + + context 'when event matches defined schema' do + it 'is not valid' do + event = described_class.new('workflow/webhooks/issues/created') + + expect(event).to be_valid + end + end + + context 'when event is using simplified schema' do + it 'is not valid' do + event = described_class.new('workflow/webhooks/issues') + + expect(event).to be_valid + end + end + end + + context 'when a recognized event path is used' do + subject { described_class.new('workflow/webhooks/issues') } + + describe '#group_mask' do + it 'returns 1' do + expect(subject.group_mask).to eq 1 + end + end + + describe '#subgroup_mask' do + it 'returns a bit mask of the issues webhooks subgroup' do + expect(subject.subgroup_mask).to eq 16 + end + end + end + + describe 'all system hooks bitmasks' do + described_class.webhooks.each do |hook| + it "generates a valid bitmask for `#{hook}` system hook" do + described_class.new("workflow/webhooks/#{hook}").then do |event| + expect(event).to be_valid + + chars = event.subgroup_mask.to_s(2) + + expect(chars.count('1')).to eq 1 + end + end + end + end + + describe 'composite bitmask' do + it 'calculates a composite bitmask' do + entry_1 = described_class.new('workflow/webhooks/issues') + entry_2 = described_class.new('workflow/webhooks/merge_requests') + + expect(entry_1 + entry_2).to eq 272 # 100010000 + end + end +end -- GitLab From 32546a7387714949d0d776de220f3ea8d67b3502 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 28 Jun 2022 15:00:40 +0200 Subject: [PATCH 05/19] Add initial events validation and simplify CI `on:` --- lib/gitlab/ci/config/entry/events.rb | 32 ++++++++++++++--------- spec/lib/gitlab/ci/yaml_processor_spec.rb | 4 +-- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/gitlab/ci/config/entry/events.rb b/lib/gitlab/ci/config/entry/events.rb index 52f19936acc7db..bbb7ab5ea565df 100644 --- a/lib/gitlab/ci/config/entry/events.rb +++ b/lib/gitlab/ci/config/entry/events.rb @@ -10,22 +10,30 @@ class Events < ::Gitlab::Config::Entry::Node validations do validates :config, array_of_strings_or_string: true - # TODO validate :allowed_events - end - - def value - # We only support webhooks today + ## TODO # - events = Array(@config).map do |event| - unless event.include?('/webhooks/') - raise ArgumentError, 'unexpected event' - end + # In YAML processor we will also need to check if proper webhooks are being used. This should be done on the + # logical-validaton level in YAML processor, not here, on the entry-level. + # + # We can also check if the provided event path conforms with the regexp defined in Workflows::Event class. + # + # Initially we also do not support pipeline-related events to avoid circular workflows. + validate do + Array(config).each do |event| + unless event.include?('/webhooks/') + errors.add(:config, 'only webhooks events are supported') + end - event.split('/').last + if event.match?(%r{webhooks/job|webhooks/pipeline|webhooks/deployment}) + errors.add(:config, 'pipeline related events are not supported yet') + end + end end + end - # TODO surface workflow name - { webhooks: events } if events.any? + # TODO remove duplication, return a hierarchical hash + def value + Array(@config) end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 637fbc525e05cf..31cce9755c467e 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -332,7 +332,7 @@ module Ci let(:config) do YAML.dump( annotate: { - on: 'webhooks/something', + on: 'workflow/webhooks/issues', script: 'run annotate issue' } ) @@ -343,7 +343,7 @@ module Ci options = subject.build_attributes('annotate').dig(:options) - expect(options).to match(a_hash_including(on: { webhooks: ['something'] })) + expect(options).to match(a_hash_including(on: ['workflow/webhooks/issues'])) end end -- GitLab From ae0322f6f1bb66fca08140eae8fe3f6c5403ca5e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 12:58:58 +0200 Subject: [PATCH 06/19] Use array types and GIN index to describe CI workflows --- .../20220628100300_add_ci_workflow_events.rb | 13 +++++++------ db/structure.sql | 14 +++++++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb index 9734ae057c722b..cc0fe057e62bd4 100644 --- a/db/migrate/20220628100300_add_ci_workflow_events.rb +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -5,14 +5,15 @@ def change create_table :ci_workflow_events do |t| t.timestamps_with_timezone null: false t.bigint :project_id, null: false, index: true - t.bigint :groups, null: false # bitmask for event groups used (eg. webhooks) - t.bigint :subgroups, null: false # bitmask for event subgroup (eg. issues events) - t.integer :events, array: true, null: false, default: [] # concrete events from a subgroup + t.bigint :event_scopes, null: false, array: true, default: [] + t.integer :event_types, null: false, array: true, default: [] + t.integer :event_groups, null: false, array: true, default: [] + t.integer :event_ids, null: false, array: true, default: [] t.text :name, limit: 255, null: false, index: { unique: true } - # TODO: add sha column to link to Git sha a workflow has been defined in + t.text :sha, limit: 32, null: false + t.uuid :uuid, null: false - # initially we only support project-level events - t.index [:project_id, :groups, :subgroups], name: 'ci_workflow_project_events_index' + t.index [:event_scopes, :event_types, :event_groups, :event_ids], using: 'GIN', name: 'ci_workflow_events_gin_index' end end end diff --git a/db/structure.sql b/db/structure.sql index b0f2e524629bf5..ef824f08118a1d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13319,11 +13319,15 @@ CREATE TABLE ci_workflow_events ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, project_id bigint NOT NULL, - groups bigint NOT NULL, - subgroups bigint NOT NULL, - events integer[] DEFAULT '{}'::integer[] NOT NULL, + event_scopes bigint[] DEFAULT '{}'::bigint[] NOT NULL, + event_types integer[] DEFAULT '{}'::integer[] NOT NULL, + event_groups integer[] DEFAULT '{}'::integer[] NOT NULL, + event_ids integer[] DEFAULT '{}'::integer[] NOT NULL, name text NOT NULL, - CONSTRAINT check_57c0faffdd CHECK ((char_length(name) <= 255)) + sha text NOT NULL, + uuid uuid NOT NULL, + CONSTRAINT check_57c0faffdd CHECK ((char_length(name) <= 255)), + CONSTRAINT check_d454861a71 CHECK ((char_length(sha) <= 32)) ); CREATE SEQUENCE ci_workflow_events_id_seq @@ -26824,7 +26828,7 @@ CREATE INDEX cadence_create_iterations_automation ON iterations_cadences USING b CREATE INDEX ci_builds_gitlab_monitor_metrics ON ci_builds USING btree (status, created_at, project_id) WHERE ((type)::text = 'Ci::Build'::text); -CREATE INDEX ci_workflow_project_events_index ON ci_workflow_events USING btree (project_id, groups, subgroups); +CREATE INDEX ci_workflow_events_gin_index ON ci_workflow_events USING gin (event_scopes, event_types, event_groups, event_ids); CREATE INDEX code_owner_approval_required ON protected_branches USING btree (project_id, code_owner_approval_required) WHERE (code_owner_approval_required = true); -- GitLab From bd0c6d585fc1ee58c26ebd50035d4aafec8354c3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 13:31:23 +0200 Subject: [PATCH 07/19] Simplify workflow events class and specs --- .../20220628100300_add_ci_workflow_events.rb | 3 +- lib/gitlab/ci/workflows/event.rb | 40 +++++-------------- spec/lib/gitlab/ci/workflows/event_spec.rb | 35 +++++----------- 3 files changed, 24 insertions(+), 54 deletions(-) diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb index cc0fe057e62bd4..d35909a60c0a64 100644 --- a/db/migrate/20220628100300_add_ci_workflow_events.rb +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -13,7 +13,8 @@ def change t.text :sha, limit: 32, null: false t.uuid :uuid, null: false - t.index [:event_scopes, :event_types, :event_groups, :event_ids], using: 'GIN', name: 'ci_workflow_events_gin_index' + t.index [:event_scopes, :event_types, :event_groups, :event_ids], + using: 'GIN', name: 'ci_workflow_events_gin_index' end end end diff --git a/lib/gitlab/ci/workflows/event.rb b/lib/gitlab/ci/workflows/event.rb index ece9133b520a17..991c84e333a3d4 100644 --- a/lib/gitlab/ci/workflows/event.rb +++ b/lib/gitlab/ci/workflows/event.rb @@ -5,12 +5,12 @@ module Ci module Workflows class Event EVENT_GROUPS = { - webhooks: 1 # use bit masks + webhooks: 1 # we can probably extract this definion to a better place }.freeze # TODO event named group contains leading slash as it is optional # rubocop:disable Lint/MixedRegexpCaptureTypes TODO - PATH_REGEXP = %r{(?[\w-]+)/webhooks/(?[\w-]+)(/?[\w-]+)?}.freeze + PATH_REGEXP = %r{(?[\w-]+)/webhooks/(?[\w-]+)(?[/\w-]+)?}.freeze # rubocop:enable Lint/MixedRegexpCaptureTypes def initialize(path) @@ -19,55 +19,37 @@ def initialize(path) end def valid? - @matches.present? && self.class.webhooks.include?(@matches[:subgroup]) + @matches.present? && self.class.webhooks.include?(@matches[:group]) end - def group_mask + def type_id EVENT_GROUPS[:webhooks] end - def subgroup_index + def group_id raise ArgumentError unless valid? - @index ||= self.class.webhook_index(@matches[:subgroup]) - end - - def subgroup_mask - raise ArgumentError unless valid? - - @mask ||= self.class.webhook_mask(@matches[:subgroup]) - end - - # This will return a composite subgroup bitmask - # - # TODO once we introduce more event groups, we will need to extract this to another class / method - # - def +(other) - subgroup_mask | other.subgroup_mask + @index ||= self.class.webhook_index(@matches[:group]) end ## TODO # # Webhooks are not granular enough to expose concrete events. We would need to extend them to support events - # like issue/create, issue/update. This information is available in a payload, but we need additional code to + # like issue/created, issue/updated. This information is available in a payload, but we need additional code to # connect it with workflow events. # - def events + def event_id raise NotImplementedError end def self.webhooks - ::TriggerableHooks::AVAILABLE_TRIGGERS.values.map do |subgroup| - subgroup.to_s.sub('_events', '') + ::TriggerableHooks::AVAILABLE_TRIGGERS.values.map do |group| + group.to_s.sub('_events', '') end end def self.webhook_index(hook_name) - webhooks.index(hook_name) - end - - def self.webhook_mask(hook_name) - 2 << webhook_index(hook_name) + webhooks.index(hook_name) + 1 end end end diff --git a/spec/lib/gitlab/ci/workflows/event_spec.rb b/spec/lib/gitlab/ci/workflows/event_spec.rb index 679cc0546e0310..125e75837a1ee8 100644 --- a/spec/lib/gitlab/ci/workflows/event_spec.rb +++ b/spec/lib/gitlab/ci/workflows/event_spec.rb @@ -47,39 +47,26 @@ context 'when a recognized event path is used' do subject { described_class.new('workflow/webhooks/issues') } - describe '#group_mask' do + describe '#type_id' do it 'returns 1' do - expect(subject.group_mask).to eq 1 + expect(subject.type_id).to eq 1 end end - describe '#subgroup_mask' do - it 'returns a bit mask of the issues webhooks subgroup' do - expect(subject.subgroup_mask).to eq 16 + describe '#group_id' do + it 'returns an id of the issues webhooks group' do + expect(subject.group_id).to eq 4 end end end - describe 'all system hooks bitmasks' do - described_class.webhooks.each do |hook| - it "generates a valid bitmask for `#{hook}` system hook" do - described_class.new("workflow/webhooks/#{hook}").then do |event| - expect(event).to be_valid + describe 'last known system hook identifier' do + it "generates an expected id for the latest known system hook" do + latest = TriggerableHooks::AVAILABLE_TRIGGERS.values.last + event = described_class.new('workflow/webhooks/subgroup') - chars = event.subgroup_mask.to_s(2) - - expect(chars.count('1')).to eq 1 - end - end - end - end - - describe 'composite bitmask' do - it 'calculates a composite bitmask' do - entry_1 = described_class.new('workflow/webhooks/issues') - entry_2 = described_class.new('workflow/webhooks/merge_requests') - - expect(entry_1 + entry_2).to eq 272 # 100010000 + expect(latest).to eq :subgroup_events + expect(event.group_id).to eq 16 end end end -- GitLab From d379948e6b0374875ed6ed3ff315e659d00f7641 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 14:07:21 +0200 Subject: [PATCH 08/19] Add a scaffold of a workflow/create pipeline chain --- .../ci/pipeline/chain/workflows/create.rb | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 lib/gitlab/ci/pipeline/chain/workflows/create.rb diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb new file mode 100644 index 00000000000000..4c6337a2e1c9e3 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Chain + module Workflows + ## + # This pipeline chain class is responsible for creating workflows events defined in a pipeline. + # + class Create < Chain::Base + def perform! + events = [] + + pipeline.stages.map(&:statuses).flatten.each do |status| + status.dig(:options, :on).each do |event| + events.push(Ci::Workflow::Event.new(event) + end + end + + # TODO create a workflow! + end + + def break? + false + end + end + end + end + end + end +end -- GitLab From cd74cfa918eb899a613acd4a03b706cd077b6b01 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 14:07:36 +0200 Subject: [PATCH 09/19] Combine CI workflows together to calculate their UUIDs --- lib/gitlab/ci/workflows/event.rb | 23 ++++++++++++++++++++++ spec/lib/gitlab/ci/workflows/event_spec.rb | 21 ++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/lib/gitlab/ci/workflows/event.rb b/lib/gitlab/ci/workflows/event.rb index 991c84e333a3d4..d1757403067585 100644 --- a/lib/gitlab/ci/workflows/event.rb +++ b/lib/gitlab/ci/workflows/event.rb @@ -13,6 +13,8 @@ class Event PATH_REGEXP = %r{(?[\w-]+)/webhooks/(?[\w-]+)(?[/\w-]+)?}.freeze # rubocop:enable Lint/MixedRegexpCaptureTypes + attr_reader :path + def initialize(path) @path = path @matches = path.match(PATH_REGEXP) @@ -38,10 +40,31 @@ def group_id # like issue/created, issue/updated. This information is available in a payload, but we need additional code to # connect it with workflow events. # + # TODO: Make it possible to parse a payload before we create a pipeline. We will need to use pipeline + # expressions combined with a JSON payload representation even before we actually start a pipeline for better + # efficency. This can be done in a next iteration. + # def event_id raise NotImplementedError end + def workflow + @matches[:name] + end + + ## + # This method combines workflow events to calulate their unique id for a given workflow name. It will return a + # hash of unique identifiers for all workflows defined. + # + # TODO: we need to move this to Workflows::Composite class and expose identifiers there too! + # + def self.combine(events) + events.each_with_object({}) do |event, hash| + hash[event.workflow] = ::OpenSSL::Digest::SHA256 + .hexdigest(event.path + hash[event.workflow].to_s)[0..31] + end + end + def self.webhooks ::TriggerableHooks::AVAILABLE_TRIGGERS.values.map do |group| group.to_s.sub('_events', '') diff --git a/spec/lib/gitlab/ci/workflows/event_spec.rb b/spec/lib/gitlab/ci/workflows/event_spec.rb index 125e75837a1ee8..b8bd5c3d81cf4d 100644 --- a/spec/lib/gitlab/ci/workflows/event_spec.rb +++ b/spec/lib/gitlab/ci/workflows/event_spec.rb @@ -69,4 +69,25 @@ expect(event.group_id).to eq 16 end end + + describe '#workflow' do + it 'properly exposes a name' do + event = described_class.new('my-workflow/webhooks/issues') + + expect(event.workflow).to eq 'my-workflow' + end + end + + describe '.combine' do + it 'properly calculates workflow unique identifiers' do + event_1 = described_class.new('my-workflow/webhooks/issues') + event_2 = described_class.new('my-workflow/webhooks/subgroup') + event_3 = described_class.new('workflow2/webhooks/issues') + + expect(described_class.combine([event_1, event_2, event_3])).to eq({ + 'my-workflow' => '79fac4b09be784b330c8ef5a22d6f0a2', + 'workflow2' => '26016ef9b76a288f528f6e0a25d85755' + }) + end + end end -- GitLab From f666141ae56d6f20a409407085475b3f4a00127d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 15:48:37 +0200 Subject: [PATCH 10/19] Implement GitLab CI workflow class with identifier --- .../ci/pipeline/chain/workflows/create.rb | 2 +- lib/gitlab/ci/workflows/event.rb | 23 +++----- lib/gitlab/ci/workflows/workflow.rb | 49 +++++++++++++++++ spec/lib/gitlab/ci/workflows/event_spec.rb | 13 ----- spec/lib/gitlab/ci/workflows/workflow_spec.rb | 55 +++++++++++++++++++ 5 files changed, 113 insertions(+), 29 deletions(-) create mode 100644 lib/gitlab/ci/workflows/workflow.rb create mode 100644 spec/lib/gitlab/ci/workflows/workflow_spec.rb diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index 4c6337a2e1c9e3..77c8b391c44507 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -14,7 +14,7 @@ def perform! pipeline.stages.map(&:statuses).flatten.each do |status| status.dig(:options, :on).each do |event| - events.push(Ci::Workflow::Event.new(event) + events.push(Ci::Workflow::Event.new(event)) end end diff --git a/lib/gitlab/ci/workflows/event.rb b/lib/gitlab/ci/workflows/event.rb index d1757403067585..7d382440ed7154 100644 --- a/lib/gitlab/ci/workflows/event.rb +++ b/lib/gitlab/ci/workflows/event.rb @@ -4,6 +4,8 @@ module Gitlab module Ci module Workflows class Event + include Comparable + EVENT_GROUPS = { webhooks: 1 # we can probably extract this definion to a better place }.freeze @@ -24,6 +26,10 @@ def valid? @matches.present? && self.class.webhooks.include?(@matches[:group]) end + def workflow + @matches[:name] + end + def type_id EVENT_GROUPS[:webhooks] end @@ -48,21 +54,8 @@ def event_id raise NotImplementedError end - def workflow - @matches[:name] - end - - ## - # This method combines workflow events to calulate their unique id for a given workflow name. It will return a - # hash of unique identifiers for all workflows defined. - # - # TODO: we need to move this to Workflows::Composite class and expose identifiers there too! - # - def self.combine(events) - events.each_with_object({}) do |event, hash| - hash[event.workflow] = ::OpenSSL::Digest::SHA256 - .hexdigest(event.path + hash[event.workflow].to_s)[0..31] - end + def <=>(other) + self.path <=> other.path end def self.webhooks diff --git a/lib/gitlab/ci/workflows/workflow.rb b/lib/gitlab/ci/workflows/workflow.rb new file mode 100644 index 00000000000000..b962b1ac7c40bb --- /dev/null +++ b/lib/gitlab/ci/workflows/workflow.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Workflows + class Workflow + include Enumerable + + attr_reader :events + + def initialize(events) + @events = events + + names = events.map(&:workflow).uniq + + raise ArgumentError if names.size > 1 + end + + def valid? + events.all?(&:valid?) + end + + def each(&block) + events.each(&block) + end + + def uuid + sha256 = events.sort.uniq.reduce(0) do |sha, event| + ::OpenSSL::Digest::SHA256.hexdigest(event.path + sha.to_s) + end + + sha256[0..31] + end + + def event_types + events.map(&:type_id).uniq.sort + end + + def event_groups + events.map(&:group_id).uniq.sort + end + + def event_ids + events.map(&:event_id).uniq.sort + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/workflows/event_spec.rb b/spec/lib/gitlab/ci/workflows/event_spec.rb index b8bd5c3d81cf4d..af324b87efb8a1 100644 --- a/spec/lib/gitlab/ci/workflows/event_spec.rb +++ b/spec/lib/gitlab/ci/workflows/event_spec.rb @@ -77,17 +77,4 @@ expect(event.workflow).to eq 'my-workflow' end end - - describe '.combine' do - it 'properly calculates workflow unique identifiers' do - event_1 = described_class.new('my-workflow/webhooks/issues') - event_2 = described_class.new('my-workflow/webhooks/subgroup') - event_3 = described_class.new('workflow2/webhooks/issues') - - expect(described_class.combine([event_1, event_2, event_3])).to eq({ - 'my-workflow' => '79fac4b09be784b330c8ef5a22d6f0a2', - 'workflow2' => '26016ef9b76a288f528f6e0a25d85755' - }) - end - end end diff --git a/spec/lib/gitlab/ci/workflows/workflow_spec.rb b/spec/lib/gitlab/ci/workflows/workflow_spec.rb new file mode 100644 index 00000000000000..80ba6e61d4cdcc --- /dev/null +++ b/spec/lib/gitlab/ci/workflows/workflow_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_dependency 'app/models/concerns/triggerable_hooks' + +RSpec.describe Gitlab::Ci::Workflows::Workflow do + let(:events) do + [Gitlab::Ci::Workflows::Event.new('workflow/webhooks/merge_requests/updated'), + Gitlab::Ci::Workflows::Event.new('workflow/webhooks/issues/created')] + end + + subject { described_class.new(events) } + + describe '#valid?' do + it 'is valid' do + expect(subject).to be_valid + end + end + + describe '#uuid' do + it 'properly calculates workflow unique identifier' do + expect(subject.uuid).to eq 'a2e2051124fc2e751272b314830add49' + end + + it 'does not change uuid when an order is changed' do + workflow = described_class.new(events.reverse) + + expect(subject.uuid).to eq workflow.uuid + end + + it 'does not change uuid when an event is duplicated' do + workflow = described_class.new(events + [events.last]) + + expect(subject.uuid).to eq workflow.uuid + end + end + + describe '#event_types' do + it 'returns ids of event types' do + expect(subject.event_types).to eq [1] + end + end + + describe '#event_groups' do + it 'returns sorted ids of event groups' do + expect(subject.event_groups).to eq [4, 8] + end + + it 'returns the same set of ids when an event is duplicated' do + workflow = described_class.new(events + [events.last]) + + expect(subject.event_groups).to eq workflow.event_groups + end + end +end -- GitLab From 7583b28da51b2ead1ea2bf37847557533d5b8fde Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 16:35:47 +0200 Subject: [PATCH 11/19] Make it possible to upsert new workflows on pipeline creation --- app/models/ci/workflow_event.rb | 28 +++++++++++++++++++ app/services/ci/create_pipeline_service.rb | 1 + .../20220628100300_add_ci_workflow_events.rb | 8 ++++-- db/structure.sql | 10 +++---- .../ci/pipeline/chain/workflows/create.rb | 26 +++++++++++++---- lib/gitlab/ci/workflows/workflow.rb | 14 +++++++++- spec/lib/gitlab/ci/workflows/workflow_spec.rb | 15 ++++++++++ .../ci/create_pipeline_service_spec.rb | 19 +++++++++++++ 8 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 app/models/ci/workflow_event.rb diff --git a/app/models/ci/workflow_event.rb b/app/models/ci/workflow_event.rb new file mode 100644 index 00000000000000..97bd335c9799df --- /dev/null +++ b/app/models/ci/workflow_event.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Ci + class WorkflowEvent < Ci::ApplicationRecord + belongs_to :project + + class << self + def upsert_workflow!(workflow, sha:, project:) + entry = self.new( + sha: sha, + project: project, + name: workflow.name, + uuid: workflow.uuid, + # TODO make it more robust and resilient + event_scopes: [project.id], + event_types: workflow.event_types, + event_groups: workflow.event_groups + # TODO: not supported yet + # event_ids: workflow.event_ids + ) + + entry.validate! + + self.upsert(entry.attributes.compact, returning: %w[uuid], unique_by: [:project_id, :name]) + end + end + end +end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 02f25a82307d63..03f4935f68832c 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -33,6 +33,7 @@ class CreatePipelineService < BaseService Gitlab::Ci::Pipeline::Chain::EnsureResourceGroups, Gitlab::Ci::Pipeline::Chain::Create, Gitlab::Ci::Pipeline::Chain::CreateDeployments, + Gitlab::Ci::Pipeline::Chain::Workflows::Create, Gitlab::Ci::Pipeline::Chain::CreateCrossDatabaseAssociations, Gitlab::Ci::Pipeline::Chain::Limit::Activity, Gitlab::Ci::Pipeline::Chain::Limit::JobActivity, diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb index d35909a60c0a64..ea792514300db1 100644 --- a/db/migrate/20220628100300_add_ci_workflow_events.rb +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -3,16 +3,18 @@ class AddCiWorkflowEvents < Gitlab::Database::Migration[2.0] def change create_table :ci_workflow_events do |t| - t.timestamps_with_timezone null: false + t.timestamps_with_timezone null: false, default: -> { 'NOW()' } t.bigint :project_id, null: false, index: true t.bigint :event_scopes, null: false, array: true, default: [] t.integer :event_types, null: false, array: true, default: [] t.integer :event_groups, null: false, array: true, default: [] t.integer :event_ids, null: false, array: true, default: [] - t.text :name, limit: 255, null: false, index: { unique: true } - t.text :sha, limit: 32, null: false + t.text :name, limit: 255, null: false + t.text :sha, limit: 40, null: false t.uuid :uuid, null: false + t.index [:project_id, :name], unique: true, + name: 'ci_workflow_events_project_id_name_index' t.index [:event_scopes, :event_types, :event_groups, :event_ids], using: 'GIN', name: 'ci_workflow_events_gin_index' end diff --git a/db/structure.sql b/db/structure.sql index ef824f08118a1d..84fb5bd605f69e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13316,8 +13316,8 @@ ALTER SEQUENCE ci_variables_id_seq OWNED BY ci_variables.id; CREATE TABLE ci_workflow_events ( id bigint NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, + created_at timestamp with time zone DEFAULT now() NOT NULL, + updated_at timestamp with time zone DEFAULT now() NOT NULL, project_id bigint NOT NULL, event_scopes bigint[] DEFAULT '{}'::bigint[] NOT NULL, event_types integer[] DEFAULT '{}'::integer[] NOT NULL, @@ -13327,7 +13327,7 @@ CREATE TABLE ci_workflow_events ( sha text NOT NULL, uuid uuid NOT NULL, CONSTRAINT check_57c0faffdd CHECK ((char_length(name) <= 255)), - CONSTRAINT check_d454861a71 CHECK ((char_length(sha) <= 32)) + CONSTRAINT check_d454861a71 CHECK ((char_length(sha) <= 40)) ); CREATE SEQUENCE ci_workflow_events_id_seq @@ -26830,6 +26830,8 @@ CREATE INDEX ci_builds_gitlab_monitor_metrics ON ci_builds USING btree (status, CREATE INDEX ci_workflow_events_gin_index ON ci_workflow_events USING gin (event_scopes, event_types, event_groups, event_ids); +CREATE UNIQUE INDEX ci_workflow_events_project_id_name_index ON ci_workflow_events USING btree (project_id, name); + CREATE INDEX code_owner_approval_required ON protected_branches USING btree (project_id, code_owner_approval_required) WHERE (code_owner_approval_required = true); CREATE UNIQUE INDEX commit_user_mentions_on_commit_id_and_note_id_unique_index ON commit_user_mentions USING btree (commit_id, note_id); @@ -27630,8 +27632,6 @@ CREATE INDEX index_ci_variables_on_key ON ci_variables USING btree (key); CREATE UNIQUE INDEX index_ci_variables_on_project_id_and_key_and_environment_scope ON ci_variables USING btree (project_id, key, environment_scope); -CREATE UNIQUE INDEX index_ci_workflow_events_on_name ON ci_workflow_events USING btree (name); - CREATE INDEX index_ci_workflow_events_on_project_id ON ci_workflow_events USING btree (project_id); CREATE INDEX index_cicd_settings_on_namespace_id_where_stale_pruning_enabled ON namespace_ci_cd_settings USING btree (namespace_id) WHERE (allow_stale_runner_pruning = true); diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index 77c8b391c44507..767554e37e729d 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -10,20 +10,34 @@ module Workflows # class Create < Chain::Base def perform! - events = [] - - pipeline.stages.map(&:statuses).flatten.each do |status| - status.dig(:options, :on).each do |event| - events.push(Ci::Workflow::Event.new(event)) + events = pipeline.stages.map(&:statuses).flatten.map do |status| + status.options.dig(:on).map do |event| + ::Gitlab::Ci::Workflows::Event.new(event) end end - # TODO create a workflow! + workflows = ::Gitlab::Ci::Workflows::Workflow.fabricate(events) + + ensure_workflows!(workflows) if workflows.any? end def break? false end + + private + + def ensure_workflows!(workflows) + ## + # TODO + # + # There are a few ways to make it better, we can for example, check if there are any changes in UUIDs to + # only upsert if we know that we need to. + # + workflows.each do |workflow| + ::Ci::WorkflowEvent.upsert_workflow!(workflow, sha: pipeline.sha, project: project) + end + end end end end diff --git a/lib/gitlab/ci/workflows/workflow.rb b/lib/gitlab/ci/workflows/workflow.rb index b962b1ac7c40bb..522e63cf359fa4 100644 --- a/lib/gitlab/ci/workflows/workflow.rb +++ b/lib/gitlab/ci/workflows/workflow.rb @@ -6,7 +6,7 @@ module Workflows class Workflow include Enumerable - attr_reader :events + attr_reader :events, :name def initialize(events) @events = events @@ -14,6 +14,8 @@ def initialize(events) names = events.map(&:workflow).uniq raise ArgumentError if names.size > 1 + + @name = names.first end def valid? @@ -43,6 +45,16 @@ def event_groups def event_ids events.map(&:event_id).uniq.sort end + + def self.fabricate(events) + workflows = events.flatten.each_with_object({}) do |event, hash| + (hash[event.workflow] ||= []).push(event) + end + + workflows.keys.map do |key| + self.new(workflows.dig(key)) + end + end end end end diff --git a/spec/lib/gitlab/ci/workflows/workflow_spec.rb b/spec/lib/gitlab/ci/workflows/workflow_spec.rb index 80ba6e61d4cdcc..1534768a82d081 100644 --- a/spec/lib/gitlab/ci/workflows/workflow_spec.rb +++ b/spec/lib/gitlab/ci/workflows/workflow_spec.rb @@ -52,4 +52,19 @@ expect(subject.event_groups).to eq workflow.event_groups end end + + describe '.fabricate' do + let(:event) do + Gitlab::Ci::Workflows::Event.new('another/webhooks/issues/created') + end + + it 'creates multiple workflow objects correctly' do + workflows = described_class.fabricate(events + [event]) + + expect(workflows).to all(be_valid) + expect(workflows.count).to eq 2 + expect(workflows.first.uuid).to eq 'a2e2051124fc2e751272b314830add49' + expect(workflows.second.uuid).to eq 'fc345747a1c44dadc3937b3c1265f134' + end + end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index aac059f2104437..9f0d331617464d 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -876,6 +876,25 @@ def previous_commit_sha_from_ref(ref) end end + context 'when pipeline workflow is used' do + before do + config = YAML.dump( + annotate: { + on: 'workflow-1/webhooks/issues/created', + script: 'run my-scripts' + } + ) + + stub_ci_pipeline_yaml_file(config) + end + + it 'creates a new workflow' do + execute_service.payload + + expect(Ci::WorkflowEvent.all.count).to eq 1 + end + end + context 'with resource group' do context 'when resource group is defined' do before do -- GitLab From c4584b64a1d960fc9abed2e23dc3c8afb25db16c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 29 Jun 2022 18:12:24 +0200 Subject: [PATCH 12/19] Add comments describing how to implement CI Workflows --- app/models/project.rb | 3 +++ lib/gitlab/ci/pipeline/chain/workflows/create.rb | 3 +++ 2 files changed, 6 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index 110f400d3ece3e..af547848044f36 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1610,6 +1610,9 @@ def first_owner # rubocop: disable CodeReuse/ServiceClass def execute_hooks(data, hooks_scope = :push_hooks) run_after_commit_or_now do + # + # TODO add Ci::DispatchWorkflowService here + # triggered_hooks(hooks_scope, data).execute SystemHooksService.new.execute_hooks(data, hooks_scope) end diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index 767554e37e729d..c6d3485e160097 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -34,6 +34,9 @@ def ensure_workflows!(workflows) # There are a few ways to make it better, we can for example, check if there are any changes in UUIDs to # only upsert if we know that we need to. # + # TODO how we create and update workflows needs more tests, benchmarks and making sure that it works well. + # Here it is just PoC, we would need to improve this area to "productionize" this code. + # workflows.each do |workflow| ::Ci::WorkflowEvent.upsert_workflow!(workflow, sha: pipeline.sha, project: project) end -- GitLab From 9729a1fa645c6af7e2dc7b3c739b0448cb1d3beb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 30 Jun 2022 13:26:27 +0200 Subject: [PATCH 13/19] Assign overship to pipelines using workflow owner --- app/models/ci/workflow_event.rb | 4 +++- .../20220628100300_add_ci_workflow_events.rb | 3 ++- db/structure.sql | 3 +++ lib/gitlab/ci/pipeline/chain/workflows/create.rb | 14 +++++++++----- spec/services/ci/create_pipeline_service_spec.rb | 3 ++- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/models/ci/workflow_event.rb b/app/models/ci/workflow_event.rb index 97bd335c9799df..6a42b4fd45eead 100644 --- a/app/models/ci/workflow_event.rb +++ b/app/models/ci/workflow_event.rb @@ -3,11 +3,13 @@ module Ci class WorkflowEvent < Ci::ApplicationRecord belongs_to :project + belongs_to :user class << self - def upsert_workflow!(workflow, sha:, project:) + def upsert_workflow!(workflow, sha:, project:, user:) entry = self.new( sha: sha, + user: user, project: project, name: workflow.name, uuid: workflow.uuid, diff --git a/db/migrate/20220628100300_add_ci_workflow_events.rb b/db/migrate/20220628100300_add_ci_workflow_events.rb index ea792514300db1..2922d371b9e4e5 100644 --- a/db/migrate/20220628100300_add_ci_workflow_events.rb +++ b/db/migrate/20220628100300_add_ci_workflow_events.rb @@ -4,7 +4,8 @@ class AddCiWorkflowEvents < Gitlab::Database::Migration[2.0] def change create_table :ci_workflow_events do |t| t.timestamps_with_timezone null: false, default: -> { 'NOW()' } - t.bigint :project_id, null: false, index: true + t.references :project, null: false, index: true, foreign_key: false # TODO LFKs + t.references :user, null: false, foreign_key: false # TODO LFKs t.bigint :event_scopes, null: false, array: true, default: [] t.integer :event_types, null: false, array: true, default: [] t.integer :event_groups, null: false, array: true, default: [] diff --git a/db/structure.sql b/db/structure.sql index 84fb5bd605f69e..ae06e678f2add5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13319,6 +13319,7 @@ CREATE TABLE ci_workflow_events ( created_at timestamp with time zone DEFAULT now() NOT NULL, updated_at timestamp with time zone DEFAULT now() NOT NULL, project_id bigint NOT NULL, + user_id bigint NOT NULL, event_scopes bigint[] DEFAULT '{}'::bigint[] NOT NULL, event_types integer[] DEFAULT '{}'::integer[] NOT NULL, event_groups integer[] DEFAULT '{}'::integer[] NOT NULL, @@ -27634,6 +27635,8 @@ CREATE UNIQUE INDEX index_ci_variables_on_project_id_and_key_and_environment_sco CREATE INDEX index_ci_workflow_events_on_project_id ON ci_workflow_events USING btree (project_id); +CREATE INDEX index_ci_workflow_events_on_user_id ON ci_workflow_events USING btree (user_id); + CREATE INDEX index_cicd_settings_on_namespace_id_where_stale_pruning_enabled ON namespace_ci_cd_settings USING btree (namespace_id) WHERE (allow_stale_runner_pruning = true); CREATE INDEX index_cluster_agent_tokens_on_agent_id_status_last_used_at ON cluster_agent_tokens USING btree (agent_id, status, last_used_at DESC NULLS LAST); diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index c6d3485e160097..ef29251fb9856d 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -29,16 +29,20 @@ def break? def ensure_workflows!(workflows) ## - # TODO - # - # There are a few ways to make it better, we can for example, check if there are any changes in UUIDs to - # only upsert if we know that we need to. + # TODO There are a few ways to make it better, we can for example, check if there are any changes in UUIDs + # to only upsert if we know that we need to. # # TODO how we create and update workflows needs more tests, benchmarks and making sure that it works well. # Here it is just PoC, we would need to improve this area to "productionize" this code. # + # TODO in this PoC creating a workflow defines ownership over pipelines that will be triggered by a + # workflow. It means that if user A defines a workflow/webhooks/issues/created, and user B creates an + # issue that will trigger it, it will be the user A who will be an owner of a pipeline and its builds. + # workflows.each do |workflow| - ::Ci::WorkflowEvent.upsert_workflow!(workflow, sha: pipeline.sha, project: project) + attributes = { sha: pipeline.sha, project: pipeline.project, user: pipeline.user } + + ::Ci::WorkflowEvent.upsert_workflow!(workflow, **attributes) end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 9f0d331617464d..85848f3112e030 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -889,9 +889,10 @@ def previous_commit_sha_from_ref(ref) end it 'creates a new workflow' do - execute_service.payload + result = execute_service.payload expect(Ci::WorkflowEvent.all.count).to eq 1 + expect(Ci::WorkflowEvent.first.user).to eq result.user end end -- GitLab From eb067806c24d3c60cd9aca5e127294ccb1dfbc8f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 30 Jun 2022 13:30:24 +0200 Subject: [PATCH 14/19] Add support for dispatching CI workflows on system hooks --- app/models/project.rb | 9 ++++++--- app/services/ci/dispatch_workflow_service.rb | 8 ++++++++ spec/services/issues/create_service_spec.rb | 8 ++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 app/services/ci/dispatch_workflow_service.rb diff --git a/app/models/project.rb b/app/models/project.rb index af547848044f36..c2f681c4050586 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1610,11 +1610,14 @@ def first_owner # rubocop: disable CodeReuse/ServiceClass def execute_hooks(data, hooks_scope = :push_hooks) run_after_commit_or_now do - # - # TODO add Ci::DispatchWorkflowService here - # triggered_hooks(hooks_scope, data).execute SystemHooksService.new.execute_hooks(data, hooks_scope) + ## + # TODO + # + # Ensure proper mult-database support + LFKs + # + Ci::DispatchWorkflowService.new(self).execute(data, hooks_scope) end end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/services/ci/dispatch_workflow_service.rb b/app/services/ci/dispatch_workflow_service.rb new file mode 100644 index 00000000000000..a17ea7112a5b84 --- /dev/null +++ b/app/services/ci/dispatch_workflow_service.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Ci + class DispatchWorkflowService < ::BaseService + def execute(data, hook) + end + end +end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 5c1544d8ebcf36..45f1539a9b310e 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -69,6 +69,14 @@ expect(issue.issue_customer_relations_contacts).to be_empty end + it 'invokes a CI workflow dispatcher' do + allow(Ci::DispatchWorkflowService) + .to receive_message_chain(:new, :execute) + .with(a_hash_including(event_type: 'issue'), :issue_hooks) + + expect(issue).to be_persisted + end + context 'when a build_service is provided' do let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params, build_service: build_service).execute } -- GitLab From aa5a3933ed2c07414115318545e0f39ebd90673e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 30 Jun 2022 14:26:06 +0200 Subject: [PATCH 15/19] Create a new pipeline when workflow is dispatched --- app/models/concerns/enums/ci/pipeline.rb | 3 +- app/models/project.rb | 5 +-- app/services/ci/dispatch_workflow_service.rb | 31 ++++++++++++++ lib/gitlab/ci/pipeline/chain/command.rb | 2 +- .../ci/pipeline/chain/workflows/create.rb | 4 +- lib/gitlab/ci/workflows/event.rb | 19 ++++++++- .../ci/dispatch_workflow_service_spec.rb | 40 +++++++++++++++++++ 7 files changed, 96 insertions(+), 8 deletions(-) create mode 100644 spec/services/ci/dispatch_workflow_service_spec.rb diff --git a/app/models/concerns/enums/ci/pipeline.rb b/app/models/concerns/enums/ci/pipeline.rb index 8ed6c54441bf16..471155ad6e39f3 100644 --- a/app/models/concerns/enums/ci/pipeline.rb +++ b/app/models/concerns/enums/ci/pipeline.rb @@ -39,7 +39,8 @@ def self.sources parent_pipeline: 12, ondemand_dast_scan: 13, ondemand_dast_validation: 14, - security_orchestration_policy: 15 + security_orchestration_policy: 15, + workflow: 16 } end diff --git a/app/models/project.rb b/app/models/project.rb index c2f681c4050586..c4cc547e8d5097 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1613,9 +1613,8 @@ def execute_hooks(data, hooks_scope = :push_hooks) triggered_hooks(hooks_scope, data).execute SystemHooksService.new.execute_hooks(data, hooks_scope) ## - # TODO - # - # Ensure proper mult-database support + LFKs + # TODO Ensure proper mult-database support + LFKs. + # TODO Ensure performance with project.has_workflows? or caching. # Ci::DispatchWorkflowService.new(self).execute(data, hooks_scope) end diff --git a/app/services/ci/dispatch_workflow_service.rb b/app/services/ci/dispatch_workflow_service.rb index a17ea7112a5b84..3f882cabc292cf 100644 --- a/app/services/ci/dispatch_workflow_service.rb +++ b/app/services/ci/dispatch_workflow_service.rb @@ -3,6 +3,37 @@ module Ci class DispatchWorkflowService < ::BaseService def execute(data, hook) + # rubocop:disable CodeReuse/ActiveRecord + + return unless ::Ci::WorkflowEvent.where(project: project).any? + + ## + # TODO extract this to a separate class / service / Ci::Workflows::Dispatcher + # + + event_group_id = ::Gitlab::Ci::Workflows::Event.system_hook_to_index(hook) + + workflows = ::Ci::WorkflowEvent.where( + "event_scopes && '{?}' AND event_types && '{?}' AND event_groups && '{?}'", project.id, 1, event_group_id + ) + + ## TODO + # + # Populate pipeline with a file-type variable with `data`. + # + # rubocop:enable CodeReuse/ActiveRecord + + workflows.each do |workflow| + params = { ignore_skip_ci: true, save_on_errors: true, workflow: workflow.id } + + ::CreatePipelineWorker.perform_async( + project.id, + workflow.user.id, + project.default_branch, + :workflow, + **params + ) + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 0a6f6fd740c243..7aedb4f7f149d5 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -11,7 +11,7 @@ module Chain :trigger_request, :schedule, :merge_request, :external_pull_request, :ignore_skip_ci, :save_incompleted, :seeds_block, :variables_attributes, :push_options, - :chat_data, :allow_mirror_update, :bridge, :content, :dry_run, :logger, + :chat_data, :allow_mirror_update, :bridge, :content, :dry_run, :logger, :workflow, # These attributes are set by Chains during processing: :config_content, :yaml_processor_result, :workflow_rules_result, :pipeline_seed ) do diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index ef29251fb9856d..815ac4d5c2c74d 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -11,7 +11,7 @@ module Workflows class Create < Chain::Base def perform! events = pipeline.stages.map(&:statuses).flatten.map do |status| - status.options.dig(:on).map do |event| + status.options.dig(:on).to_a.map do |event| ::Gitlab::Ci::Workflows::Event.new(event) end end @@ -42,6 +42,8 @@ def ensure_workflows!(workflows) workflows.each do |workflow| attributes = { sha: pipeline.sha, project: pipeline.project, user: pipeline.user } + ## TODO add `ref` to workflow event! + # ::Ci::WorkflowEvent.upsert_workflow!(workflow, **attributes) end end diff --git a/lib/gitlab/ci/workflows/event.rb b/lib/gitlab/ci/workflows/event.rb index 7d382440ed7154..adcc62785c6dc5 100644 --- a/lib/gitlab/ci/workflows/event.rb +++ b/lib/gitlab/ci/workflows/event.rb @@ -6,6 +6,10 @@ module Workflows class Event include Comparable + # TODO + # + # Rename this to 'hooks' as there is no "web" component in the implementation. + # EVENT_GROUPS = { webhooks: 1 # we can probably extract this definion to a better place }.freeze @@ -59,13 +63,24 @@ def <=>(other) end def self.webhooks - ::TriggerableHooks::AVAILABLE_TRIGGERS.values.map do |group| + @hooks ||= ::TriggerableHooks::AVAILABLE_TRIGGERS.values.map do |group| group.to_s.sub('_events', '') end end + ## + # TODO memoize methods below, refactor them, extract to a better place + # def self.webhook_index(hook_name) - webhooks.index(hook_name) + 1 + webhooks.index(hook_name.to_s) + 1 + end + + def self.hook_to_event(hook_name) + ::TriggerableHooks::AVAILABLE_TRIGGERS[hook_name.to_sym].to_s.gsub('_events', '') + end + + def self.system_hook_to_index(hook_name) + webhook_index(hook_to_event(hook_name)) end end end diff --git a/spec/services/ci/dispatch_workflow_service_spec.rb b/spec/services/ci/dispatch_workflow_service_spec.rb new file mode 100644 index 00000000000000..e3b435ed7a6efc --- /dev/null +++ b/spec/services/ci/dispatch_workflow_service_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::DispatchWorkflowService, '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + + let(:event_group_id) do + ::Gitlab::Ci::Workflows::Event.webhook_index('issues') + end + + let(:data) { spy('payload') } # rubocop:disable RSpec/VerifiedDoubles + + subject { described_class.new(project) } + + before do + project.add_developer(user) + + # TODO FactoryBot + Ci::WorkflowEvent.create!( + name: 'my-workflow', + sha: '1234asdf', + uuid: '12345678123456781234567812345678', + project: project, + user: user, + event_scopes: [project.id], + event_types: [1], + event_groups: [event_group_id] + ) + + stub_ci_pipeline_to_return_yaml_file + end + + it 'creates a new pipeline', :sidekiq_inline do + subject.execute(data, 'issue_hooks') + + expect(Ci::Pipeline.all.count).to eq 1 + end +end -- GitLab From c8e4e7b96473098d9243c39f82a086d4aceb9a1b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 30 Jun 2022 15:46:23 +0200 Subject: [PATCH 16/19] Add a pipeline chain class for selecting workflow builds --- app/models/project.rb | 2 ++ app/services/ci/create_pipeline_service.rb | 1 + app/services/ci/dispatch_workflow_service.rb | 10 ++++-- lib/gitlab/ci/pipeline/chain/command.rb | 8 ++++- .../ci/pipeline/chain/workflows/select.rb | 33 +++++++++++++++++++ 5 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 lib/gitlab/ci/pipeline/chain/workflows/select.rb diff --git a/app/models/project.rb b/app/models/project.rb index c4cc547e8d5097..badb549ae802bd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1615,6 +1615,8 @@ def execute_hooks(data, hooks_scope = :push_hooks) ## # TODO Ensure proper mult-database support + LFKs. # TODO Ensure performance with project.has_workflows? or caching. + # TODO For a limited availability we can make this an opt-in feature and + # check it with project.workflow_events_feature_available? # Ci::DispatchWorkflowService.new(self).execute(data, hooks_scope) end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 03f4935f68832c..a1c1d7bcc6d5a6 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -24,6 +24,7 @@ class CreatePipelineService < BaseService Gitlab::Ci::Pipeline::Chain::SeedBlock, Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules, Gitlab::Ci::Pipeline::Chain::Seed, + Gitlab::Ci::Pipeline::Chain::Workflows::Select, Gitlab::Ci::Pipeline::Chain::Limit::Size, Gitlab::Ci::Pipeline::Chain::Limit::Deployments, Gitlab::Ci::Pipeline::Chain::Validate::External, diff --git a/app/services/ci/dispatch_workflow_service.rb b/app/services/ci/dispatch_workflow_service.rb index 3f882cabc292cf..7ea72621a54813 100644 --- a/app/services/ci/dispatch_workflow_service.rb +++ b/app/services/ci/dispatch_workflow_service.rb @@ -24,8 +24,14 @@ def execute(data, hook) # rubocop:enable CodeReuse/ActiveRecord workflows.each do |workflow| - params = { ignore_skip_ci: true, save_on_errors: true, workflow: workflow.id } - + params = { ignore_skip_ci: true, save_on_errors: true, workflow_id: workflow.id } + + ## TODO + # + # Pipeline should run for the ref that the workflow has been defined for! + # + # Use something like `workflow.ref` instead of `project.default_branch` below. + # ::CreatePipelineWorker.perform_async( project.id, workflow.user.id, diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 7aedb4f7f149d5..1af9a5a3e15bae 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -11,7 +11,7 @@ module Chain :trigger_request, :schedule, :merge_request, :external_pull_request, :ignore_skip_ci, :save_incompleted, :seeds_block, :variables_attributes, :push_options, - :chat_data, :allow_mirror_update, :bridge, :content, :dry_run, :logger, :workflow, + :chat_data, :allow_mirror_update, :bridge, :content, :dry_run, :logger, :workflow_id, # These attributes are set by Chains during processing: :config_content, :yaml_processor_result, :workflow_rules_result, :pipeline_seed ) do @@ -76,6 +76,12 @@ def ambiguous_ref? end end + def workflow + strong_memoize(:workflow) do + ::Ci::WorkflowEvent.find(workflow_id) if workflow_id.present? + end + end + def parent_pipeline bridge&.parent_pipeline end diff --git a/lib/gitlab/ci/pipeline/chain/workflows/select.rb b/lib/gitlab/ci/pipeline/chain/workflows/select.rb new file mode 100644 index 00000000000000..7494bae50cbe5b --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/workflows/select.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Chain + ## + # TODO we can have only a single workflow per pipeline, so we can safely rename this module to `Workflow`. + # + module Workflows + ## + # This pipeline chain class is responsible for selecting jobs to run based on defined workflow. + # + class Select < Chain::Base + def perform! + return unless pipeline.workflow? + + ## + # TODO + # + # Select builds with on: that matches the workflow along with their `needs:` builds in subsequent stages. + # + end + + def break? + false + end + end + end + end + end + end +end -- GitLab From 28c5e9a1ef0977d96d931dd5c27794e8c9cc583e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 1 Jul 2022 15:24:39 +0200 Subject: [PATCH 17/19] Select builds that will run depending on the workflow --- app/models/ci/workflow_event.rb | 54 ++++++++++------ app/services/ci/create_pipeline_service.rb | 2 +- app/services/ci/dispatch_workflow_service.rb | 11 +--- lib/gitlab/ci/config/yaml.rb | 14 ++++ .../ci/pipeline/chain/workflows/create.rb | 4 ++ .../ci/pipeline/chain/workflows/select.rb | 27 +++++++- .../ci/dispatch_workflow_service_spec.rb | 64 ++++++++++++++++++- 7 files changed, 142 insertions(+), 34 deletions(-) diff --git a/app/models/ci/workflow_event.rb b/app/models/ci/workflow_event.rb index 6a42b4fd45eead..0980c9ca89b778 100644 --- a/app/models/ci/workflow_event.rb +++ b/app/models/ci/workflow_event.rb @@ -5,26 +5,40 @@ class WorkflowEvent < Ci::ApplicationRecord belongs_to :project belongs_to :user - class << self - def upsert_workflow!(workflow, sha:, project:, user:) - entry = self.new( - sha: sha, - user: user, - project: project, - name: workflow.name, - uuid: workflow.uuid, - # TODO make it more robust and resilient - event_scopes: [project.id], - event_types: workflow.event_types, - event_groups: workflow.event_groups - # TODO: not supported yet - # event_ids: workflow.event_ids - ) - - entry.validate! - - self.upsert(entry.attributes.compact, returning: %w[uuid], unique_by: [:project_id, :name]) - end + def self.upsert_workflow!(workflow, sha:, project:, user:) + entry = self.new( + sha: sha, + user: user, + project: project, + name: workflow.name, + uuid: workflow.uuid, + # TODO make it more robust and resilient + event_scopes: [project.id], + event_types: workflow.event_types, + event_groups: workflow.event_groups + # TODO: not supported yet + # event_ids: workflow.event_ids + ) + + entry.validate! + + self.upsert(entry.attributes.compact, returning: %w[uuid], unique_by: [:project_id, :name]) + end + + def self.find_matching(project, hook) + event_group_id = ::Gitlab::Ci::Workflows::Event.system_hook_to_index(hook) + + ::Ci::WorkflowEvent.where( + "event_scopes && '{?}' AND event_types && '{?}' AND event_groups && '{?}'", project.id, 1, event_group_id + ) + end + + def matches_event?(event) + name == event.workflow.to_s && + event_types.include?(event.type_id) && + event_groups.include?(event.group_id) + + ## event_ids.include?(event.event_id) TODO not supported yet end end end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index a1c1d7bcc6d5a6..21d8149fcbd620 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -21,10 +21,10 @@ class CreatePipelineService < BaseService Gitlab::Ci::Pipeline::Chain::Config::Process, Gitlab::Ci::Pipeline::Chain::Validate::AfterConfig, Gitlab::Ci::Pipeline::Chain::RemoveUnwantedChatJobs, + Gitlab::Ci::Pipeline::Chain::Workflows::Select, Gitlab::Ci::Pipeline::Chain::SeedBlock, Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules, Gitlab::Ci::Pipeline::Chain::Seed, - Gitlab::Ci::Pipeline::Chain::Workflows::Select, Gitlab::Ci::Pipeline::Chain::Limit::Size, Gitlab::Ci::Pipeline::Chain::Limit::Deployments, Gitlab::Ci::Pipeline::Chain::Validate::External, diff --git a/app/services/ci/dispatch_workflow_service.rb b/app/services/ci/dispatch_workflow_service.rb index 7ea72621a54813..32a8e041f042db 100644 --- a/app/services/ci/dispatch_workflow_service.rb +++ b/app/services/ci/dispatch_workflow_service.rb @@ -7,21 +7,14 @@ def execute(data, hook) return unless ::Ci::WorkflowEvent.where(project: project).any? - ## - # TODO extract this to a separate class / service / Ci::Workflows::Dispatcher - # - - event_group_id = ::Gitlab::Ci::Workflows::Event.system_hook_to_index(hook) + # rubocop:enable CodeReuse/ActiveRecord - workflows = ::Ci::WorkflowEvent.where( - "event_scopes && '{?}' AND event_types && '{?}' AND event_groups && '{?}'", project.id, 1, event_group_id - ) + workflows = ::Ci::WorkflowEvent.find_matching(project, hook) ## TODO # # Populate pipeline with a file-type variable with `data`. # - # rubocop:enable CodeReuse/ActiveRecord workflows.each do |workflow| params = { ignore_skip_ci: true, save_on_errors: true, workflow_id: workflow.id } diff --git a/lib/gitlab/ci/config/yaml.rb b/lib/gitlab/ci/config/yaml.rb index de833619c8df24..561d724688b849 100644 --- a/lib/gitlab/ci/config/yaml.rb +++ b/lib/gitlab/ci/config/yaml.rb @@ -10,6 +10,20 @@ class << self def load!(content) ensure_custom_tags + ## + # TODO + # + # A hack to solve `on:` YAML serialization to `true`: + # + # pry > YAML.load('abc: something') + # => {"abc"=>"something"} + # pry > YAML.load('on: something') + # => {true=>"something"} + # + # This needs to be changed!!! + # + content = content.gsub(/\s\son:/, ' "on":') + Gitlab::Config::Loader::Yaml.new(content, additional_permitted_classes: AVAILABLE_TAGS).load! end diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index 815ac4d5c2c74d..8e0f3146744499 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -19,6 +19,10 @@ def perform! workflows = ::Gitlab::Ci::Workflows::Workflow.fabricate(events) ensure_workflows!(workflows) if workflows.any? + + ## + # TODO remove workflows when these get removed from `.gitlab-ci.yml`. + # end def break? diff --git a/lib/gitlab/ci/pipeline/chain/workflows/select.rb b/lib/gitlab/ci/pipeline/chain/workflows/select.rb index 7494bae50cbe5b..0e6cdcc32fb240 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/select.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/select.rb @@ -16,10 +16,35 @@ def perform! return unless pipeline.workflow? ## - # TODO # # Select builds with on: that matches the workflow along with their `needs:` builds in subsequent stages. # + # TODO + # + # We are doing it here in a similar way this is done in Chain::RemoveUnwantedChatJobs. Both ways are + # wrong, and we need to refactor both into the new mechanism that will integrate better with pipeline + # seeds. + + raise ArgumentError, 'missing YAML processor result' unless @command.yaml_processor_result + + selected = [] + + @command.yaml_processor_result.jobs.select! do |name, config| + if config.key?(:on) + config.dig(:on).any? do |on| + event = ::Gitlab::Ci::Workflows::Event.new(on) + + if @command.workflow.matches_event?(event) + selected.push(name) + end + end + else + needs = config.dig(:needs, :job).to_a + .map { |hash| hash[:name].to_sym } + + (needs & selected).present? if needs.any? + end + end end def break? diff --git a/spec/services/ci/dispatch_workflow_service_spec.rb b/spec/services/ci/dispatch_workflow_service_spec.rb index e3b435ed7a6efc..79ad89f469f350 100644 --- a/spec/services/ci/dispatch_workflow_service_spec.rb +++ b/spec/services/ci/dispatch_workflow_service_spec.rb @@ -29,12 +29,70 @@ event_groups: [event_group_id] ) - stub_ci_pipeline_to_return_yaml_file + Ci::WorkflowEvent.create!( + name: 'workflow-2', + sha: '1234asdf', + uuid: '12345678123456781234567812345678', + project: project, + user: user, + event_scopes: [project.id], + event_types: [1], + event_groups: [event_group_id] + ) + + config = <<~CONFIG + stages: + - first + - second + - third + + first: + stage: first + script: irrelevant + + annotate: + on: my-workflow/webhooks/issues/created + stage: second + script: ./run something + + summary: + stage: third + needs: + - annotate + script: append-to-summary + + annotate-2: + on: workflow-2/webhooks/issues/updated + stage: third + script: ./run something + + fourth: + stage: third + script: irrelevant + CONFIG + + stub_ci_pipeline_yaml_file(config) end - it 'creates a new pipeline', :sidekiq_inline do + it 'creates new pipelines', :sidekiq_inline do subject.execute(data, 'issue_hooks') - expect(Ci::Pipeline.all.count).to eq 1 + expect(Ci::Pipeline.all.count).to eq 2 + end + + it 'properly selects builds to run', :sidekiq_inline do + subject.execute(data, 'issue_hooks') + + first = Ci::Pipeline.first + second = Ci::Pipeline.second + + expect(first.yaml_errors).to be_blank + expect(second.yaml_errors).to be_blank + + expect(first.statuses.count).to eq 2 + expect(second.statuses.count).to eq 1 + + expect(first.statuses.map(&:name)). to match_array(%w[annotate summary]) + expect(second.statuses.map(&:name)). to match_array(%w[annotate-2]) end end -- GitLab From c11fb3bb7a90a9f73fcc523eb2c7ef9efd3ea119 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 1 Jul 2022 16:22:31 +0200 Subject: [PATCH 18/19] Remove workflow builds from non-workflow pipelines --- .../ci/pipeline/chain/workflows/select.rb | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/pipeline/chain/workflows/select.rb b/lib/gitlab/ci/pipeline/chain/workflows/select.rb index 0e6cdcc32fb240..9ac1ba25a752da 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/select.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/select.rb @@ -13,8 +13,6 @@ module Workflows # class Select < Chain::Base def perform! - return unless pipeline.workflow? - ## # # Select builds with on: that matches the workflow along with their `needs:` builds in subsequent stages. @@ -29,6 +27,21 @@ def perform! selected = [] + unless pipeline.workflow? + @command.yaml_processor_result.jobs.reject! do |name, config| + selected.push(name) if config.key?(:on) + end + + @command.yaml_processor_result.jobs.reject! do |_, config| + needs = config.dig(:needs, :job).to_a + .map { |hash| hash[:name].to_sym } + + (needs & selected).present? if needs.any? + end + + return + end + @command.yaml_processor_result.jobs.select! do |name, config| if config.key?(:on) config.dig(:on).any? do |on| -- GitLab From ea4cbd9757923363b17317dd3a5709d3ac314a48 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jul 2022 15:50:09 +0200 Subject: [PATCH 19/19] Create workflows before selecting workflow builds --- app/services/ci/create_pipeline_service.rb | 2 +- lib/gitlab/ci/pipeline/chain/workflows/create.rb | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 21d8149fcbd620..1bb6d768a7cb51 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -21,6 +21,7 @@ class CreatePipelineService < BaseService Gitlab::Ci::Pipeline::Chain::Config::Process, Gitlab::Ci::Pipeline::Chain::Validate::AfterConfig, Gitlab::Ci::Pipeline::Chain::RemoveUnwantedChatJobs, + Gitlab::Ci::Pipeline::Chain::Workflows::Create, Gitlab::Ci::Pipeline::Chain::Workflows::Select, Gitlab::Ci::Pipeline::Chain::SeedBlock, Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules, @@ -34,7 +35,6 @@ class CreatePipelineService < BaseService Gitlab::Ci::Pipeline::Chain::EnsureResourceGroups, Gitlab::Ci::Pipeline::Chain::Create, Gitlab::Ci::Pipeline::Chain::CreateDeployments, - Gitlab::Ci::Pipeline::Chain::Workflows::Create, Gitlab::Ci::Pipeline::Chain::CreateCrossDatabaseAssociations, Gitlab::Ci::Pipeline::Chain::Limit::Activity, Gitlab::Ci::Pipeline::Chain::Limit::JobActivity, diff --git a/lib/gitlab/ci/pipeline/chain/workflows/create.rb b/lib/gitlab/ci/pipeline/chain/workflows/create.rb index 8e0f3146744499..ca069c3ff5cc80 100644 --- a/lib/gitlab/ci/pipeline/chain/workflows/create.rb +++ b/lib/gitlab/ci/pipeline/chain/workflows/create.rb @@ -10,10 +10,16 @@ module Workflows # class Create < Chain::Base def perform! - events = pipeline.stages.map(&:statuses).flatten.map do |status| - status.options.dig(:on).to_a.map do |event| - ::Gitlab::Ci::Workflows::Event.new(event) - end + raise ArgumentError, 'missing YAML processor result' unless @command.yaml_processor_result + + events = [] + + @command.yaml_processor_result.jobs.each do |name, config| + events.concat(config.dig(:on)) if config.key?(:on) + end + + events.map! do |event| + ::Gitlab::Ci::Workflows::Event.new(event) end workflows = ::Gitlab::Ci::Workflows::Workflow.fabricate(events) -- GitLab