Commit bc83a6f6 authored by Matija Čupić's avatar Matija Čupić

Implement needs:pipeline config

Add dependecy config option to bridge jobs

Adds a "dependency" keyword to bridge job configs that denotes an
upstream dependency.

Expose bridge dependency config in model

Add SeedBridge pipeline chain step

Adds a SeedBridge Pipeline chain step that seeds bridge jobs with their
upstream dependencies.

Rename bridged_job to bridge in service

Handle bridges with nonexistent projects

Handles upstream bridges that point towards projects that do not exist.

Check user permissions before seeding bridge

Checks if the user has permissions to read pipelines on the upstream
project before seeding bridge with upstream pipeline.

Rename SeedBridge to Subscribe

Renames the Pipeline chain step from SeedBridge to Subscribe to better
reflect the intent.

Use intent revealing interfaces

Extract subscribe implementation to service

Extracts all of the bridge subscription functionality into a service for
better testing and isolation.

Pass bridge to SubscribeBridgeService

Move bridge subscription to Pipeline processing

Moves the bridge subscription mechanism from the Pipeline creation chain
to the Pipeline processing service.

Rename dependency to upstream

Renames dependency to upstream.

We already have dependencies, using dependency would be confusing.

Refactor subscription services

Refactors subscription services to better encapsulate the business logic
they enforce.

Move bridge subscription to enqueue event

Moves the bridge subscription logic from the ProcessPipelineService into
the enqueue event.

Parse upstream bridges in pipeline seed

Add optimistic lock to bridge subscription service

Add upstream mirror docs

Mirror completed statuses to bridge

Rename upstream to needs:pipeline

Renames the upstream CI config keyword to needs:pipeline.

This supports the idea of marrying DAGs and upstream dependent
pipelines.

Extract upstream_pipeline to method

Rename needs:pipeline to needs:project

Improve 'needs:' docs
parent b5af7529
......@@ -176,6 +176,21 @@ Upstream pipelines take precedence over downstream ones. If there are two
variables with the same name defined in both upstream and downstream projects,
the ones defined in the upstream project will take precedence.
### Mirroring status from upstream pipeline
You can mirror the pipeline status from an upstream pipeline to a bridge job by
using the `needs:project` keyword. The latest pipeline status from master is
replicated to the bridge job.
Example:
```yaml
upstream_bridge:
stage: test
needs:
project: other/project
```
### Limitations
Because bridge jobs are a little different to regular jobs, it is not
......
......@@ -13,17 +13,24 @@ module EE
serialize :yaml_variables, ::Gitlab::Serializer::Ci::Variables
# rubocop:enable Cop/ActiveRecordSerialize
belongs_to :upstream_pipeline, class_name: "::Ci::Pipeline",
foreign_key: :upstream_pipeline_id
belongs_to :upstream_pipeline, class_name: "::Ci::Pipeline"
has_many :sourced_pipelines, class_name: "::Ci::Sources::Pipeline",
foreign_key: :source_job_id
state_machine :status do
after_transition created: :pending do |bridge|
next unless bridge.downstream_project
bridge.run_after_commit do
bridge.schedule_downstream_pipeline!
end
end
after_transition any => :pending do |bridge|
next unless bridge.upstream_project
bridge.subscribe_to_upstream!
end
end
end
......@@ -31,14 +38,26 @@ module EE
::Ci::CreateCrossProjectPipelineWorker.perform_async(self.id)
end
def subscribe_to_upstream!
::Ci::SubscribeBridgeService.new(self.project, self.user).execute(self)
end
def target_user
self.user
end
def target_project_path
downstream_project || upstream_project
end
def downstream_project
options&.dig(:trigger, :project)
end
def upstream_project
options&.dig(:needs, :project)
end
def target_ref
options&.dig(:trigger, :branch)
end
......
......@@ -12,7 +12,9 @@ module EE
super.merge(protected_environment_failure: 1_000,
insufficient_bridge_permissions: 1_001,
downstream_bridge_project_not_found: 1_002,
invalid_bridge_trigger: 1_003)
invalid_bridge_trigger: 1_003,
upstream_bridge_project_not_found: 1_004,
insufficient_upstream_permissions: 1_005)
end
end
end
......
......@@ -7,7 +7,9 @@ module EE
EE_CALLOUT_FAILURE_MESSAGES = const_get(:CALLOUT_FAILURE_MESSAGES).merge(
protected_environment_failure: 'The environment this job is deploying to is protected. Only users with permission may successfully run this job.',
insufficient_bridge_permissions: 'This job could not be executed because of insufficient permissions to create a downstream pipeline.',
insufficient_upstream_permissions: 'This job could not be executed because of insufficient permissions to track the upstream project.',
downstream_bridge_project_not_found: 'This job could not be executed because downstream bridge project could not be found.',
upstream_bridge_project_not_found: 'This job could not be executed because upstream bridge project could not be found.',
invalid_bridge_trigger: 'This job could not be executed because downstream pipeline trigger definition is invalid.'
).freeze
......
......@@ -3,21 +3,21 @@
module Ci
class PipelineBridgeStatusService < ::BaseService
def execute(pipeline)
pipeline.downstream_bridges.each do |bridged_job|
process_bridged_job(pipeline.status, bridged_job)
pipeline.downstream_bridges.each do |bridge|
process_bridge(pipeline.status, bridge)
end
end
def process_bridged_job(status, job)
def process_bridge(status, bridge)
case status
when 'success'
job.success!
bridge.success!
when 'failed'
job.drop!
bridge.drop!
when 'canceled'
job.cancel!
bridge.cancel!
when 'skipped'
job.skip!
bridge.skip!
end
end
end
......
# frozen_string_literal: true
module Ci
class SubscribeBridgeService < ::BaseService
include ::Gitlab::Utils::StrongMemoize
def execute(bridge)
return unless bridge.upstream_project
@bridge = bridge
unless upstream_project
return bridge.drop!(:upstream_bridge_project_not_found)
end
unless can?(current_user, :read_pipeline, upstream_project)
return bridge.drop!(:insufficient_upstream_permissions)
end
return unless upstream_pipeline
bridge_updates = { upstream_pipeline: upstream_pipeline }
if completed_status?(upstream_pipeline.status)
bridge_updates[:status] = upstream_pipeline.status
end
bridge.update!(bridge_updates)
end
private
def upstream_project
strong_memoize(:upstream_project) do
::Project.find_by_full_path(@bridge.target_project_path)
end
end
def upstream_pipeline
strong_memoize(:upstream_pipeline) do
upstream_project.pipeline_for(upstream_project.default_branch)
end
end
def completed_status?(status)
Ci::Pipeline::COMPLETED_STATUSES.include?(status)
end
end
end
......@@ -14,15 +14,20 @@ module EE
include ::Gitlab::Config::Entry::Attributable
ALLOWED_KEYS = %i[trigger stage allow_failure only except
when extends variables].freeze
when extends variables needs].freeze
validations do
validates :config, allowed_keys: ALLOWED_KEYS
validates :config, presence: true
validates :trigger, presence: true
validates :name, presence: true
validates :name, type: Symbol
validate do
unless trigger.present? || needs.present?
errors.add(:config, 'should contain either a trigger or a needs:project')
end
end
with_options allow_nil: true do
validates :when,
inclusion: { in: %w[on_success on_failure always],
......@@ -34,6 +39,9 @@ module EE
entry :trigger, ::EE::Gitlab::Ci::Config::Entry::Trigger,
description: 'CI/CD Bridge downstream trigger definition.'
entry :needs, ::EE::Gitlab::Ci::Config::Entry::Needs,
description: 'CI/CD Bridge needs dependency definition.'
entry :stage, ::Gitlab::Ci::Config::Entry::Stage,
description: 'Pipeline stage this job will be executed into.'
......@@ -53,7 +61,8 @@ module EE
def self.matching?(name, config)
::Feature.enabled?(:cross_project_pipeline_triggers, default_enabled: true) &&
!name.to_s.start_with?('.') &&
config.is_a?(Hash) && config.key?(:trigger)
config.is_a?(Hash) &&
(config.key?(:trigger) || config.key?(:needs))
end
def self.visible?
......@@ -66,7 +75,8 @@ module EE
def value
{ name: name,
trigger: trigger_value,
trigger: (trigger_value if trigger_defined?),
needs: (needs_value if needs_defined?),
ignore: !!allow_failure,
stage: stage_value,
when: when_value,
......
# frozen_string_literal: true
module EE
module Gitlab
module Ci
module Config
module Entry
##
# Entry that represents a cross-project needs dependency.
#
class Needs < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
include ::Gitlab::Config::Entry::Attributable
ALLOWED_KEYS = %i[project].freeze
attributes :project
validations do
validates :config, presence: true
validates :config, allowed_keys: ALLOWED_KEYS
validates :project, type: String, presence: true
end
end
end
end
end
end
end
......@@ -13,7 +13,9 @@ module EE
protected_environment_failure: 'protected environment failure',
invalid_bridge_trigger: 'downstream pipeline trigger definition is invalid',
downstream_bridge_project_not_found: 'downstream project could not be found',
insufficient_bridge_permissions: 'no permissions to trigger downstream pipeline'
upstream_bridge_project_not_found: 'upstream project could not be found',
insufficient_bridge_permissions: 'no permissions to trigger downstream pipeline',
insufficient_upstream_permissions: 'no permissions to read upstream project'
).freeze
EE::Gitlab::Ci::Status::Build::Failed.private_constant :EE_REASONS
end
......
require 'fast_spec_helper'
require_dependency 'active_model'
require 'spec_helper'
describe EE::Gitlab::Ci::Config::Entry::Bridge do
describe '.matching?' do
......@@ -41,7 +40,7 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
end
describe '.new' do
subject { described_class.new(config, name: :my_trigger) }
subject { described_class.new(config, name: :my_bridge) }
before do
subject.compose!
......@@ -56,7 +55,7 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
describe '#value' do
it 'is returns a bridge job configuration' do
expect(subject.value).to eq(name: :my_trigger,
expect(subject.value).to eq(name: :my_bridge,
trigger: { project: 'some/project' },
ignore: false,
stage: 'test',
......@@ -65,6 +64,24 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
end
end
context 'when needs project config is a non-empty string' do
let(:config) { { needs: { project: 'some/project' } } }
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'is returns a bridge job configuration' do
expect(subject.value).to eq(name: :my_bridge,
needs: { project: 'some/project' },
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] })
end
end
end
context 'when bridge trigger is a hash' do
let(:config) do
{ trigger: { project: 'some/project', branch: 'feature' } }
......@@ -76,9 +93,9 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
describe '#value' do
it 'is returns a bridge job configuration hash' do
expect(subject.value).to eq(name: :my_trigger,
expect(subject.value).to eq(name: :my_bridge,
trigger: { project: 'some/project',
branch: 'feature' },
branch: 'feature' },
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] })
......@@ -89,6 +106,7 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
context 'when bridge configuration contains all supported keys' do
let(:config) do
{ trigger: { project: 'some/project', branch: 'feature' },
needs: { project: 'other/project' },
when: 'always',
extends: '.some-key',
stage: 'deploy',
......@@ -109,7 +127,21 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
describe '#errors' do
it 'is returns an error about empty trigger config' do
expect(subject.errors.first).to match /can't be blank/
expect(subject.errors.first).to eq('bridge config should contain either a trigger or a needs:project')
end
end
end
context 'when upstream config is nil' do
let(:config) { { needs: nil } }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about empty upstream config' do
expect(subject.errors.first).to eq('bridge config should contain either a trigger or a needs:project')
end
end
end
......
# frozen_string_literal: true
require 'fast_spec_helper'
require_dependency 'active_model'
describe EE::Gitlab::Ci::Config::Entry::Needs do
subject { described_class.new(config) }
context 'when upstream config is a non-empty string' do
let(:config) { { project: 'some/project' } }
describe '#valid?' do
it { is_expected.to be_valid }
end
describe '#value' do
it 'is returns a upstream configuration hash' do
expect(subject.value).to eq(project: 'some/project')
end
end
end
context 'when upstream config an empty string' do
let(:config) { '' }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about an empty config' do
expect(subject.errors.first)
.to eq("needs config can't be blank")
end
end
end
context 'when upstream configuration is not valid' do
context 'when branch is not provided' do
let(:config) { { project: 123 } }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'returns an error message' do
expect(subject.errors.first)
.to eq('needs project should be a string')
end
end
end
end
end
......@@ -23,13 +23,37 @@ describe Ci::Bridge do
end
describe 'state machine transitions' do
context 'when it changes status from created to pending' do
context 'when bridge points towards downstream' do
it 'does not subscribe to upstream project' do
expect(::Ci::SubscribeBridgeService).not_to receive(:new)
bridge.enqueue!
end
it 'schedules downstream pipeline creation' do
expect(bridge).to receive(:schedule_downstream_pipeline!)
bridge.enqueue!
end
end
context 'when bridge points towards upstream' do
before do
bridge.options = { needs: { project: 'my/project' } }
end
it 'subscribes to the upstream project' do
expect(::Ci::SubscribeBridgeService).to receive_message_chain(:new, :execute)
bridge.enqueue!
end
it 'does not schedule downstream pipeline creation' do
expect(bridge).not_to receive(:schedule_downstream_pipeline!)
bridge.enqueue!
end
end
end
describe '#target_user' do
......
......@@ -9,6 +9,8 @@ describe Ci::ProcessPipelineService, '#execute' do
create(:ci_empty_pipeline, ref: 'master', project: project, user: user)
end
let(:service) { described_class.new(pipeline.project, user) }
before do
project.add_maintainer(user)
downstream.add_developer(user)
......
# frozen_string_literal: true
require 'spec_helper'
describe Ci::SubscribeBridgeService do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:bridge) { build(:ci_bridge, upstream: upstream_project) }
let(:service) { described_class.new(project, user) }
describe '#execute' do
subject { service.execute(bridge) }
context 'when the upstream project exists' do
let(:upstream_project) { create(:project, :repository) }
context 'when the user has permissions' do
before do
upstream_project.add_developer(user)
end
context 'when the upstream project has a pipeline' do
let!(:upstream_pipeline) do
create(
:ci_pipeline, project: upstream_project,
ref: upstream_project.default_branch,
sha: upstream_project.commit.sha
)
end
it 'populates the pipeline project source' do
subject
expect(bridge.upstream_pipeline).to eq(upstream_pipeline)
end
context 'when the pipeline already finished' do
before do
upstream_pipeline.succeed!
end
it 'mirrors the pipeline status to the bridge job instantly' do
expect { subject }.to change { bridge.status }.from('created').to(upstream_pipeline.status)
end
end
end
context 'when the upstream project does not have a pipeline' do
it 'does not populate the pipeline project source' do
subject
expect(bridge.upstream_pipeline).to eq(nil)
end
end
end
context 'when the user does not have permissions' do
it 'drops the bridge' do
subject
expect(bridge.upstream_pipeline).to eq(nil)
expect(bridge.status).to eq('failed')
expect(bridge.failure_reason).to eq('insufficient_upstream_permissions')
end
end
end
context 'when the upstream project does not exist' do
let(:upstream_project) { nil }
before do
bridge.options = { needs: { project: 'some/project' } }
end
it 'drops the bridge' do
subject
expect(bridge.upstream_pipeline).to eq(nil)
expect(bridge.status).to eq('failed')
expect(bridge.failure_reason).to eq('upstream_bridge_project_not_found')
end
end
end
end
......@@ -39,7 +39,9 @@ module Gitlab
end
def bridge?
@attributes.to_h.dig(:options, :trigger).present?
attributes_hash = @attributes.to_h
attributes_hash.dig(:options, :trigger).present? ||
attributes_hash.dig(:options, :needs, :project).present?
end
def to_resource
......
......@@ -55,7 +55,8 @@ module Gitlab
parallel: job[:parallel],
instance: job[:instance],
start_in: job[:start_in],
trigger: job[:trigger]
trigger: job[:trigger],
needs: job[:needs]
}.compact }.compact
end
......
......@@ -8,7 +8,7 @@ FactoryBot.define do
ref 'master'
tag false
created_at 'Di 29. Okt 09:50:00 CET 2013'
status :success
status :created
pipeline factory: :ci_pipeline
......@@ -17,6 +17,7 @@ FactoryBot.define do
end
transient { downstream nil }
transient { upstream nil }
after(:build) do |bridge, evaluator|
bridge.project ||= bridge.pipeline.project
......@@ -26,6 +27,12 @@ FactoryBot.define do
trigger: { project: evaluator.downstream.full_path }
)
end
if evaluator.upstream.present?
bridge.options = bridge.options.to_h.merge(
needs: { project: evaluator.upstream.full_path }
)
end
end
end
end
......@@ -19,20 +19,36 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
describe '#bridge?' do
subject { seed_build.bridge? }
context 'when job is a bridge' do
context 'when job is a downstream bridge' do
let(:attributes) do
{ name: 'rspec', ref: 'master', options: { trigger: 'my/project' } }
end
it { is_expected.to be_truthy }
context 'when trigger definition is empty' do
let(:attributes) do
{ name: 'rspec', ref: 'master', options: { trigger: '' } }
end
it { is_expected.to be_falsey }
end
end
context 'when trigger definition is empty' do
context 'when job is an upstream bridge' do
let(:attributes) do
{ name: 'rspec', ref: 'master', options: { trigger: '' } }
{ name: 'rspec', ref: 'master', options: { needs: { project: 'my/project' } } }
end
it { is_expected.to be_falsey }
it { is_expected.to be_truthy }
context 'when upstream definition is empty' do
let(:attributes) do
{ name: 'rspec', ref: 'master', options: { needs: { project: '' } } }
end
it { is_expected.to be_falsey }
end
end
context 'when job is not a bridge' do
......
......@@ -23,7 +23,7 @@ describe Ci::Bridge do
let(:status) { bridge.detailed_status(user) }
it 'returns detailed status object' do
expect(status).to be_a Gitlab::Ci::Status::Success
expect(status).to be_a Gitlab::Ci::Status::Created
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment