Commit 3cb95106 authored by Balasankar "Balu" C's avatar Balasankar "Balu" C

Put pipeline filter by source feature behind a feature flag

Move feature that let users specify `source` as a query parameter to the
pipelines API behind the feature flag named `pipeline_source_filter` so
that it can be quickly disabled if found to be causing performance
issues.
Signed-off-by: default avatarBalasankar "Balu" C <balasankar@gitlab.com>
parent fc6fa9cb
...@@ -23,13 +23,15 @@ module Ci ...@@ -23,13 +23,15 @@ module Ci
items = by_iids(items) items = by_iids(items)
items = by_scope(items) items = by_scope(items)
items = by_status(items) items = by_status(items)
items = by_source(items)
items = by_ref(items) items = by_ref(items)
items = by_sha(items) items = by_sha(items)
items = by_name(items) items = by_name(items)
items = by_username(items) items = by_username(items)
items = by_yaml_errors(items) items = by_yaml_errors(items)
items = by_updated_at(items) items = by_updated_at(items)
items = by_source(items) if Feature.enabled?(:pipeline_source_filter, project, default_enabled: :yaml)
sort_items(items) sort_items(items)
end end
......
---
name: pipeline_source_filter
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67846
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338347
milestone: '14.2'
type: development
group: group::pipeline execution
default_enabled: false
...@@ -59,7 +59,7 @@ module API ...@@ -59,7 +59,7 @@ module API
authorize! :read_build, user_project authorize! :read_build, user_project
pipelines = ::Ci::PipelinesFinder.new(user_project, current_user, params).execute pipelines = ::Ci::PipelinesFinder.new(user_project, current_user, params).execute
present paginate(pipelines), with: Entities::Ci::PipelineBasic present paginate(pipelines), with: Entities::Ci::PipelineBasic, project: user_project
end end
desc 'Create a new pipeline' do desc 'Create a new pipeline' do
......
...@@ -4,9 +4,11 @@ module API ...@@ -4,9 +4,11 @@ module API
module Entities module Entities
module Ci module Ci
class PipelineBasic < Grape::Entity class PipelineBasic < Grape::Entity
expose :id, :project_id, :sha, :ref, :status, :source expose :id, :project_id, :sha, :ref, :status
expose :created_at, :updated_at expose :created_at, :updated_at
expose :source, if: ->(pipeline, options) { ::Feature.enabled?(:pipeline_source_filter, options[:project], default_enabled: :yaml) }
expose :web_url do |pipeline, _options| expose :web_url do |pipeline, _options|
Gitlab::Routing.url_helpers.project_pipeline_url(pipeline.project, pipeline) Gitlab::Routing.url_helpers.project_pipeline_url(pipeline.project, pipeline)
end end
......
...@@ -252,17 +252,26 @@ RSpec.describe Ci::PipelinesFinder do ...@@ -252,17 +252,26 @@ RSpec.describe Ci::PipelinesFinder do
end end
end end
context "when source is specified" do context 'when source is specified' do
let(:params) { { source: 'web' } } let(:params) { { source: 'web' } }
let!(:pipeline) { create(:ci_pipeline, project: project, source: 'web') } let!(:web_pipeline) { create(:ci_pipeline, project: project, source: 'web') }
let!(:push_pipeline) { create(:ci_pipeline, project: project, source: 'push') }
let!(:api_pipeline) { create(:ci_pipeline, project: project, source: 'api') }
context 'when `pipeline_source_filter` feature flag is disabled' do
before do before do
create(:ci_pipeline, project: project, source: 'push') stub_feature_flags(pipeline_source_filter: false)
create(:ci_pipeline, project: project, source: 'chat')
end end
it 'returns matched pipelines' do it 'returns all the pipelines' do
is_expected.to eq([pipeline]) is_expected.to contain_exactly(web_pipeline, push_pipeline, api_pipeline)
end
end
context 'when `pipeline_source_filter` feature flag is enabled' do
it 'returns only the matched pipeline' do
is_expected.to eq([web_pipeline])
end
end end
end end
......
...@@ -34,8 +34,29 @@ RSpec.describe API::Ci::Pipelines do ...@@ -34,8 +34,29 @@ RSpec.describe API::Ci::Pipelines do
expect(json_response.first['sha']).to match(/\A\h{40}\z/) expect(json_response.first['sha']).to match(/\A\h{40}\z/)
expect(json_response.first['id']).to eq pipeline.id expect(json_response.first['id']).to eq pipeline.id
expect(json_response.first['web_url']).to be_present expect(json_response.first['web_url']).to be_present
end
describe 'keys in the response' do
context 'when `pipeline_source_filter` feature flag is disabled' do
before do
stub_feature_flags(pipeline_source_filter: false)
end
it 'does not includes pipeline source' do
get api("/projects/#{project.id}/pipelines", user)
expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at])
end
end
context 'when `pipeline_source_filter` feature flag is disabled' do
it 'includes pipeline source' do
get api("/projects/#{project.id}/pipelines", user)
expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at source]) expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at source])
end end
end
end
context 'when parameter is passed' do context 'when parameter is passed' do
%w[running pending].each do |target| %w[running pending].each do |target|
...@@ -302,6 +323,22 @@ RSpec.describe API::Ci::Pipelines do ...@@ -302,6 +323,22 @@ RSpec.describe API::Ci::Pipelines do
create(:ci_pipeline, project: project, source: :api) create(:ci_pipeline, project: project, source: :api)
end end
context 'when `pipeline_source_filter` feature flag is disabled' do
before do
stub_feature_flags(pipeline_source_filter: false)
end
it 'returns all pipelines' do
get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' }
expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers
expect(json_response).not_to be_empty
expect(json_response.length).to be >= 3
end
end
context 'when `pipeline_source_filter` feature flag is enabled' do
it 'returns matched pipelines' do it 'returns matched pipelines' do
get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' } get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' }
...@@ -310,7 +347,6 @@ RSpec.describe API::Ci::Pipelines do ...@@ -310,7 +347,6 @@ RSpec.describe API::Ci::Pipelines do
expect(json_response).not_to be_empty expect(json_response).not_to be_empty
json_response.each { |r| expect(r['source']).to eq('web') } json_response.each { |r| expect(r['source']).to eq('web') }
end end
end
context 'when source is invalid' do context 'when source is invalid' do
it 'returns bad_request' do it 'returns bad_request' do
...@@ -321,6 +357,8 @@ RSpec.describe API::Ci::Pipelines do ...@@ -321,6 +357,8 @@ RSpec.describe API::Ci::Pipelines do
end end
end end
end end
end
end
context 'unauthorized user' do context 'unauthorized user' do
it 'does not return project pipelines' do it 'does not return project pipelines' do
......
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