Commit e512b4cb authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '330266-remove-ci-artifacts-exclude-ff' into 'master'

Remove ci_artifacts_exclude feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!64227
parents 7c3cb884 703711e4
...@@ -929,8 +929,7 @@ module Ci ...@@ -929,8 +929,7 @@ module Ci
end end
def supports_artifacts_exclude? def supports_artifacts_exclude?
options&.dig(:artifacts, :exclude)&.any? && options&.dig(:artifacts, :exclude)&.any?
Gitlab::Ci::Features.artifacts_exclude_enabled?
end end
def multi_build_steps? def multi_build_steps?
......
...@@ -82,7 +82,7 @@ module Ci ...@@ -82,7 +82,7 @@ module Ci
expire_in: artifacts[:expire_in] expire_in: artifacts[:expire_in]
} }
if artifacts.dig(:exclude).present? && ::Gitlab::Ci::Features.artifacts_exclude_enabled? if artifacts.dig(:exclude).present?
archive.merge(exclude: artifacts[:exclude]) archive.merge(exclude: artifacts[:exclude])
else else
archive archive
......
---
name: ci_artifacts_exclude
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30708
rollout_issue_url:
milestone: '13.0'
type: development
group: group::pipeline execution
default_enabled: true
...@@ -36,8 +36,7 @@ module Gitlab ...@@ -36,8 +36,7 @@ module Gitlab
}, if: :expose_as_present? }, if: :expose_as_present?
validates :expose_as, type: String, length: { maximum: 100 }, if: :expose_as_present? validates :expose_as, type: String, length: { maximum: 100 }, if: :expose_as_present?
validates :expose_as, format: { with: EXPOSE_AS_REGEX, message: EXPOSE_AS_ERROR_MESSAGE }, if: :expose_as_present? validates :expose_as, format: { with: EXPOSE_AS_REGEX, message: EXPOSE_AS_ERROR_MESSAGE }, if: :expose_as_present?
validates :exclude, array_of_strings: true, if: :exclude_enabled? validates :exclude, array_of_strings: true
validates :exclude, absence: { message: 'feature is disabled' }, unless: :exclude_enabled?
validates :reports, type: Hash validates :reports, type: Hash
validates :when, validates :when,
inclusion: { in: %w[on_success on_failure always], inclusion: { in: %w[on_success on_failure always],
...@@ -60,10 +59,6 @@ module Gitlab ...@@ -60,10 +59,6 @@ module Gitlab
!@config[:expose_as].nil? !@config[:expose_as].nil?
end end
def exclude_enabled?
::Gitlab::Ci::Features.artifacts_exclude_enabled?
end
end end
end end
end end
......
...@@ -6,10 +6,6 @@ module Gitlab ...@@ -6,10 +6,6 @@ module Gitlab
# Ci::Features is a class that aggregates all CI/CD feature flags in one place. # Ci::Features is a class that aggregates all CI/CD feature flags in one place.
# #
module Features module Features
def self.artifacts_exclude_enabled?
::Feature.enabled?(:ci_artifacts_exclude, default_enabled: true)
end
# NOTE: The feature flag `disallow_to_create_merge_request_pipelines_in_target_project` # NOTE: The feature flag `disallow_to_create_merge_request_pipelines_in_target_project`
# is a safe switch to disable the feature for a particular project when something went wrong, # is a safe switch to disable the feature for a particular project when something went wrong,
# therefore it's not supposed to be enabled by default. # therefore it's not supposed to be enabled by default.
......
...@@ -143,51 +143,22 @@ RSpec.describe Gitlab::Ci::Config::Entry::Artifacts do ...@@ -143,51 +143,22 @@ RSpec.describe Gitlab::Ci::Config::Entry::Artifacts do
end end
describe 'excluded artifacts' do describe 'excluded artifacts' do
context 'when configuration is valid and the feature is enabled' do context 'when configuration is valid' do
before do let(:config) { { untracked: true, exclude: ['some/directory/'] } }
stub_feature_flags(ci_artifacts_exclude: true)
end
context 'when configuration is valid' do
let(:config) { { untracked: true, exclude: ['some/directory/'] } }
it 'correctly parses the configuration' do
expect(entry).to be_valid
expect(entry.value).to eq config
end
end
context 'when configuration is not valid' do it 'correctly parses the configuration' do
let(:config) { { untracked: true, exclude: 1234 } } expect(entry).to be_valid
expect(entry.value).to eq config
it 'returns an error' do
expect(entry).not_to be_valid
expect(entry.errors)
.to include 'artifacts exclude should be an array of strings'
end
end end
end end
context 'when artifacts/exclude feature is disabled' do context 'when configuration is not valid' do
before do let(:config) { { untracked: true, exclude: 1234 } }
stub_feature_flags(ci_artifacts_exclude: false)
end
context 'when configuration has been provided' do
let(:config) { { untracked: true, exclude: ['some/directory/'] } }
it 'returns an error' do
expect(entry).not_to be_valid
expect(entry.errors).to include 'artifacts exclude feature is disabled'
end
end
context 'when configuration is not present' do it 'returns an error' do
let(:config) { { untracked: true } } expect(entry).not_to be_valid
expect(entry.errors)
it 'is a valid configuration' do .to include 'artifacts exclude should be an array of strings'
expect(entry).to be_valid
end
end end
end end
end end
......
...@@ -1648,8 +1648,6 @@ module Gitlab ...@@ -1648,8 +1648,6 @@ module Gitlab
end end
it 'populates a build options with complete artifacts configuration' do it 'populates a build options with complete artifacts configuration' do
stub_feature_flags(ci_artifacts_exclude: true)
config = <<~YAML config = <<~YAML
test: test:
script: echo "Hello World" script: echo "Hello World"
......
...@@ -4493,26 +4493,12 @@ RSpec.describe Ci::Build do ...@@ -4493,26 +4493,12 @@ RSpec.describe Ci::Build do
it { is_expected.to include(:upload_multiple_artifacts) } it { is_expected.to include(:upload_multiple_artifacts) }
end end
context 'when artifacts exclude is defined and the is feature enabled' do context 'when artifacts exclude is defined' do
let(:options) do let(:options) do
{ artifacts: { exclude: %w[something] } } { artifacts: { exclude: %w[something] } }
end end
context 'when a feature flag is enabled' do it { is_expected.to include(:artifacts_exclude) }
before do
stub_feature_flags(ci_artifacts_exclude: true)
end
it { is_expected.to include(:artifacts_exclude) }
end
context 'when a feature flag is disabled' do
before do
stub_feature_flags(ci_artifacts_exclude: false)
end
it { is_expected.not_to include(:artifacts_exclude) }
end
end end
end end
......
...@@ -44,29 +44,13 @@ RSpec.describe Ci::BuildRunnerPresenter do ...@@ -44,29 +44,13 @@ RSpec.describe Ci::BuildRunnerPresenter do
create(:ci_build, options: { artifacts: { paths: %w[abc], exclude: %w[cde] } }) create(:ci_build, options: { artifacts: { paths: %w[abc], exclude: %w[cde] } })
end end
context 'when the feature is enabled' do it 'includes the list of excluded paths' do
before do expect(presenter.artifacts.first).to include(
stub_feature_flags(ci_artifacts_exclude: true) artifact_type: :archive,
end artifact_format: :zip,
paths: %w[abc],
it 'includes the list of excluded paths' do exclude: %w[cde]
expect(presenter.artifacts.first).to include( )
artifact_type: :archive,
artifact_format: :zip,
paths: %w[abc],
exclude: %w[cde]
)
end
end
context 'when the feature is disabled' do
before do
stub_feature_flags(ci_artifacts_exclude: false)
end
it 'does not include the list of excluded paths' do
expect(presenter.artifacts.first).not_to have_key(:exclude)
end
end end
end end
......
...@@ -803,29 +803,16 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -803,29 +803,16 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
end end
context 'when a runner supports this feature' do context 'when a runner supports this feature' do
it 'exposes excluded paths when the feature is enabled' do it 'exposes excluded paths' do
stub_feature_flags(ci_artifacts_exclude: true)
request_job info: { features: { artifacts_exclude: true } } request_job info: { features: { artifacts_exclude: true } }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
expect(json_response.dig('artifacts').first).to include('exclude' => ['cde']) expect(json_response.dig('artifacts').first).to include('exclude' => ['cde'])
end end
it 'does not expose excluded paths when the feature is disabled' do
stub_feature_flags(ci_artifacts_exclude: false)
request_job info: { features: { artifacts_exclude: true } }
expect(response).to have_gitlab_http_status(:created)
expect(json_response.dig('artifacts').first).not_to have_key('exclude')
end
end end
context 'when a runner does not support this feature' do context 'when a runner does not support this feature' do
it 'does not expose the build at all' do it 'does not expose the build at all' do
stub_feature_flags(ci_artifacts_exclude: true)
request_job request_job
expect(response).to have_gitlab_http_status(:no_content) expect(response).to have_gitlab_http_status(:no_content)
......
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