Commit 22cccda5 authored by Max Woolf's avatar Max Woolf

Remove ff_external_status_checks feature flag

Removes the feature flag from across the entire codebase
enabling it for all project.

Updates the documentation to show the removal.

Changelog: added
EE: true
parent 2c2acc53
...@@ -7,14 +7,8 @@ type: reference, api ...@@ -7,14 +7,8 @@ type: reference, api
# External Status Checks API **(ULTIMATE)** # External Status Checks API **(ULTIMATE)**
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0. > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0, disabled behind the `:ff_external_status_checks` feature flag.
> - It's [deployed behind a feature flag](../user/feature_flags.md), disabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/320783) in GitLab 14.1.
> - It's disabled on GitLab.com.
> - It's not recommended for production use.
> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-external-status-checks).
WARNING:
This feature might not be available to you. Check the **version history** note above for details.
## List status checks for a merge request ## List status checks for a merge request
...@@ -151,35 +145,6 @@ PUT /projects/:id/external_status_checks/:check_id ...@@ -151,35 +145,6 @@ PUT /projects/:id/external_status_checks/:check_id
| `external_url` | string | no | URL of external status check resource | | `external_url` | string | no | URL of external status check resource |
| `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by | | `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by |
## Enable or disable external status checks **(ULTIMATE SELF)**
External status checks are:
- Under development.
- Not ready for production use.
The feature is deployed behind a feature flag that is **disabled by default**.
[GitLab administrators with access to the GitLab Rails console](../user/feature_flags.md)
can enable it.
To enable it:
```ruby
# For the instance
Feature.enable(:ff_external_status_checks)
# For a single project
Feature.enable(:ff_external_status_checks, Project.find(<project id>))
```
To disable it:
```ruby
# For the instance
Feature.disable(:ff_external_status_checks)
# For a single project
Feature.disable(:ff_external_status_checks, Project.find(<project id>))
```
## Related links ## Related links
- [External status checks](../user/project/merge_requests/status_checks.md). - [External status checks](../user/project/merge_requests/status_checks.md).
...@@ -8,11 +8,8 @@ disqus_identifier: 'https://docs.gitlab.com/ee/user/project/merge_requests/statu ...@@ -8,11 +8,8 @@ disqus_identifier: 'https://docs.gitlab.com/ee/user/project/merge_requests/statu
# External Status Checks **(ULTIMATE)** # External Status Checks **(ULTIMATE)**
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0. > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0, disabled behind the `:ff_external_status_checks` feature flag.
> - It's [deployed behind a feature flag](../../feature_flags.md), disabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/320783) in GitLab 14.1.
> - It's disabled on GitLab.com.
> - It's not recommended for production use.
> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-status-checks). **(ULTIMATE SELF)**
WARNING: WARNING:
This feature might not be available to you. Check the **version history** note above for details. This feature might not be available to you. Check the **version history** note above for details.
...@@ -116,7 +113,6 @@ the status check and it **will not** be recoverable. ...@@ -116,7 +113,6 @@ the status check and it **will not** be recoverable.
## Status checks widget ## Status checks widget
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327634) in GitLab 14.1. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327634) in GitLab 14.1.
> - The [external status checks](#external-status-checks) feature must be [enabled](#enable-or-disable-status-checks) to see the status checks widget.
The status checks widget displays in merge requests and shows the status of external The status checks widget displays in merge requests and shows the status of external
status checks: status checks:
...@@ -188,31 +184,6 @@ You should: ...@@ -188,31 +184,6 @@ You should:
- Check the [GitLab status page](https://status.gitlab.com/) if the problem persists, - Check the [GitLab status page](https://status.gitlab.com/) if the problem persists,
to see if there is a wider outage. to see if there is a wider outage.
## Enable or disable status checks **(ULTIMATE SELF)**
Status checks are under development and not ready for production use. It is
deployed behind a feature flag that is **disabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
can enable it.
To enable it:
```ruby
# For the instance
Feature.enable(:ff_external_status_checks)
# For a single project
Feature.enable(:ff_external_status_checks, Project.find(<project id>))
```
To disable it:
```ruby
# For the instance
Feature.disable(:ff_external_status_checks)
# For a single project
Feature.disable(:ff_external_status_checks, Project.find(<project id>)
```
## Related links ## Related links
- [External status checks API](../../../api/status_checks.md) - [External status checks API](../../../api/status_checks.md)
...@@ -69,8 +69,7 @@ module EE ...@@ -69,8 +69,7 @@ module EE
private private
def expose_mr_status_checks? def expose_mr_status_checks?
::Feature.enabled?(:ff_external_status_checks, project, default_enabled: :yaml) && current_user.present? &&
current_user.present? &&
project.external_status_checks.applicable_to_branch(merge_request.target_branch).any? project.external_status_checks.applicable_to_branch(merge_request.target_branch).any?
end end
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
= render_ce 'projects/merge_request_merge_checks_settings', project: @project, form: form = render_ce 'projects/merge_request_merge_checks_settings', project: @project, form: form
- if ::Feature.enabled?(:ff_external_status_checks, @project, default_enabled: :yaml) && @project.licensed_feature_available?(:external_status_checks) - if @project.licensed_feature_available?(:external_status_checks)
= render_if_exists 'projects/merge_request_status_checks_settings' = render_if_exists 'projects/merge_request_status_checks_settings'
= render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form = render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form
......
---
name: ff_external_status_checks
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54002
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320783
milestone: '14.1'
type: development
group: group::compliance
default_enabled: false
...@@ -13,8 +13,7 @@ module API ...@@ -13,8 +13,7 @@ module API
helpers do helpers do
def check_feature_enabled! def check_feature_enabled!
unauthorized! unless user_project.licensed_feature_available?(:external_status_checks) && unauthorized! unless user_project.licensed_feature_available?(:external_status_checks)
Feature.enabled?(:ff_external_status_checks, user_project)
end end
end end
...@@ -22,7 +21,6 @@ module API ...@@ -22,7 +21,6 @@ module API
segment ':id/external_status_checks' do segment ':id/external_status_checks' do
desc 'Create a new external status check' do desc 'Create a new external status check' do
success ::API::Entities::ExternalStatusCheck success ::API::Entities::ExternalStatusCheck
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end end
params do params do
requires :name, type: String, desc: 'The name of the external status check' requires :name, type: String, desc: 'The name of the external status check'
...@@ -45,9 +43,7 @@ module API ...@@ -45,9 +43,7 @@ module API
render_api_error!(service.payload[:errors], service.http_status) render_api_error!(service.payload[:errors], service.http_status)
end end
end end
desc 'List project\'s external approval rules' do desc 'List project\'s external approval rules'
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end
params do params do
use :pagination use :pagination
end end
...@@ -60,7 +56,6 @@ module API ...@@ -60,7 +56,6 @@ module API
segment ':check_id' do segment ':check_id' do
desc 'Update an external approval rule' do desc 'Update an external approval rule' do
success ::API::Entities::ExternalStatusCheck success ::API::Entities::ExternalStatusCheck
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end end
params do params do
requires :check_id, type: Integer, desc: 'The ID of the external status check' requires :check_id, type: Integer, desc: 'The ID of the external status check'
...@@ -85,9 +80,7 @@ module API ...@@ -85,9 +80,7 @@ module API
end end
end end
desc 'Delete an external status check' do desc 'Delete an external status check'
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end
params do params do
requires :check_id, type: Integer, desc: 'The ID of the status check' requires :check_id, type: Integer, desc: 'The ID of the status check'
end end
...@@ -106,7 +99,6 @@ module API ...@@ -106,7 +99,6 @@ module API
segment ':id/merge_requests/:merge_request_iid' do segment ':id/merge_requests/:merge_request_iid' do
desc 'Externally approve a merge request' do desc 'Externally approve a merge request' do
detail 'This feature was introduced in 13.12 and is gated behind the :ff_external_status_checks feature flag.'
success Entities::MergeRequests::StatusCheckResponse success Entities::MergeRequests::StatusCheckResponse
end end
params do params do
...@@ -116,8 +108,6 @@ module API ...@@ -116,8 +108,6 @@ module API
requires :sha, type: String, desc: 'The current SHA at HEAD of the merge request.' requires :sha, type: String, desc: 'The current SHA at HEAD of the merge request.'
end end
post 'status_check_responses' do post 'status_check_responses' do
not_found! unless ::Feature.enabled?(:ff_external_status_checks, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request) merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request)
check_sha_param!(params, merge_request) check_sha_param!(params, merge_request)
...@@ -127,12 +117,8 @@ module API ...@@ -127,12 +117,8 @@ module API
present(approval, with: Entities::MergeRequests::StatusCheckResponse) present(approval, with: Entities::MergeRequests::StatusCheckResponse)
end end
desc 'List all status checks for a merge request and their state.' do desc 'List all status checks for a merge request and their state.'
detail 'This feature was introduced in 13.12 and is gated behind the :ff_external_status_checks feature flag.'
end
get 'status_checks' do get 'status_checks' do
not_found! unless ::Feature.enabled?(:ff_external_status_checks, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request) merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request)
present(paginate(user_project.external_status_checks.applicable_to_branch(merge_request.target_branch)), with: Entities::MergeRequests::StatusCheck, merge_request: merge_request, sha: merge_request.source_branch_sha) present(paginate(user_project.external_status_checks.applicable_to_branch(merge_request.target_branch)), with: Entities::MergeRequests::StatusCheck, merge_request: merge_request, sha: merge_request.source_branch_sha)
......
...@@ -29,37 +29,23 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do ...@@ -29,37 +29,23 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
end end
context 'feature flag is enabled' do it 'shows the widget' do
before do expect(page).to have_content('Status checks 1 pending')
stub_feature_flags(ff_external_status_checks: true) end
end
it 'shows the widget' do it 'shows the status check issues', :aggregate_failures do
expect(page).to have_content('Status checks 1 pending') within '[data-test-id="mr-status-checks"]' do
click_button 'Expand'
end end
it 'shows the status check issues', :aggregate_failures do [check1, check2].each do |rule|
within '[data-test-id="mr-status-checks"]' do within "[data-testid='mr-status-check-issue-#{rule.id}']" do
click_button 'Expand' icon_type = rule.approved?(merge_request, merge_request.source_branch_sha) ? 'success' : 'pending'
end expect(page).to have_css(".ci-status-icon-#{icon_type}")
expect(page).to have_content("#{rule.name}, #{rule.external_url}")
[check1, check2].each do |rule|
within "[data-testid='mr-status-check-issue-#{rule.id}']" do
icon_type = rule.approved?(merge_request, merge_request.source_branch_sha) ? 'success' : 'pending'
expect(page).to have_css(".ci-status-icon-#{icon_type}")
expect(page).to have_content("#{rule.name}, #{rule.external_url}")
end
end end
end end
end end
context 'feature flag is disabled' do
before do
stub_feature_flags(ff_external_status_checks: false)
end
it_behaves_like 'no status checks widget'
end
end end
context 'user is not logged in' do context 'user is not logged in' do
......
...@@ -186,15 +186,12 @@ RSpec.describe MergeRequestPresenter do ...@@ -186,15 +186,12 @@ RSpec.describe MergeRequestPresenter do
let(:exposed_path) { expose_path("/api/v4/projects/#{merge_request.project.id}/merge_requests/#{merge_request.iid}/status_checks") } let(:exposed_path) { expose_path("/api/v4/projects/#{merge_request.project.id}/merge_requests/#{merge_request.iid}/status_checks") }
where(:feature_flag_enabled?, :authenticated?, :has_status_checks?, :exposes_path?) do where(:authenticated?, :has_status_checks?, :exposes_path?) do
false | false | false | false false | false | false
false | false | true | false false | true | false
false | true | true | false true | true | true
false | true | false | false true | false | false
true | false | false | false true | true | true
true | true | false | false
true | false | true | false
true | true | true | true
end end
with_them do with_them do
...@@ -202,7 +199,6 @@ RSpec.describe MergeRequestPresenter do ...@@ -202,7 +199,6 @@ RSpec.describe MergeRequestPresenter do
let(:path) { exposes_path? ? exposed_path : nil } let(:path) { exposes_path? ? exposed_path : nil }
before do before do
stub_feature_flags(ff_external_status_checks: feature_flag_enabled?)
allow(project.external_status_checks).to receive(:applicable_to_branch).and_return([{ branch: 'foo' }]) allow(project.external_status_checks).to receive(:applicable_to_branch).and_return([{ branch: 'foo' }])
allow(project.external_status_checks.applicable_to_branch).to receive(:any?).and_return(has_status_checks?) allow(project.external_status_checks.applicable_to_branch).to receive(:any?).and_return(has_status_checks?)
end end
...@@ -210,13 +206,9 @@ RSpec.describe MergeRequestPresenter do ...@@ -210,13 +206,9 @@ RSpec.describe MergeRequestPresenter do
it { is_expected.to eq(path) } it { is_expected.to eq(path) }
end end
context 'with the feature flag enabled and user authenticated' do context 'with the user authenticated' do
let(:presenter) { described_class.new(merge_request, current_user: user) } let(:presenter) { described_class.new(merge_request, current_user: user) }
before do
stub_feature_flags(ff_external_status_checks: true)
end
context 'without applicable branches' do context 'without applicable branches' do
before do before do
create(:external_status_check, project: project, protected_branches: [create(:protected_branch, name: 'testbranch')]) create(:external_status_check, project: project, protected_branches: [create(:protected_branch, name: 'testbranch')])
......
...@@ -32,18 +32,6 @@ RSpec.describe API::StatusChecks do ...@@ -32,18 +32,6 @@ RSpec.describe API::StatusChecks do
describe 'GET :id/merge_requests/:merge_request_iid/status_checks' do describe 'GET :id/merge_requests/:merge_request_iid/status_checks' do
subject { get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_checks", user), params: { external_status_check_id: rule.id, sha: sha } } subject { get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_checks", user), params: { external_status_check_id: rule.id, sha: sha } }
context 'feature flag is disabled' do
before do
stub_feature_flags(ff_external_status_checks: false)
end
it 'returns a not found error' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when current_user has access' do context 'when current_user has access' do
before do before do
stub_licensed_features(external_status_checks: true) stub_licensed_features(external_status_checks: true)
...@@ -81,18 +69,6 @@ RSpec.describe API::StatusChecks do ...@@ -81,18 +69,6 @@ RSpec.describe API::StatusChecks do
describe 'POST :id/:merge_requests/:merge_request_iid/status_check_responses' do describe 'POST :id/:merge_requests/:merge_request_iid/status_check_responses' do
subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_check_responses", user), params: { external_status_check_id: rule.id, sha: sha } } subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_check_responses", user), params: { external_status_check_id: rule.id, sha: sha } }
context 'feature flag is disabled' do
before do
stub_feature_flags(ff_external_status_checks: false)
end
it 'returns a not found error' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when user has access' do context 'when user has access' do
before do before do
stub_licensed_features(external_status_checks: true) stub_licensed_features(external_status_checks: true)
...@@ -153,20 +129,15 @@ RSpec.describe API::StatusChecks do ...@@ -153,20 +129,15 @@ RSpec.describe API::StatusChecks do
end end
context 'when feature is disabled, unlicensed or user has permission' do context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do where(:licensed, :project_owner, :status) do
false | false | false | :not_found false | false | :not_found
false | false | true | :unauthorized false | true | :unauthorized
false | true | true | :unauthorized true | false | :not_found
false | true | false | :not_found true | true | :success
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :success
end end
with_them do with_them do
before do before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed) stub_licensed_features(external_status_checks: licensed)
end end
...@@ -182,7 +153,6 @@ RSpec.describe API::StatusChecks do ...@@ -182,7 +153,6 @@ RSpec.describe API::StatusChecks do
describe 'POST projects/:id/external_status_checks' do describe 'POST projects/:id/external_status_checks' do
context 'successfully creating new external approval rule' do context 'successfully creating new external approval rule' do
before do before do
stub_feature_flags(ff_external_status_checks: true)
stub_licensed_features(external_status_checks: true) stub_licensed_features(external_status_checks: true)
end end
...@@ -229,20 +199,15 @@ RSpec.describe API::StatusChecks do ...@@ -229,20 +199,15 @@ RSpec.describe API::StatusChecks do
end end
context 'when feature is disabled, unlicensed or user has permission' do context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do where(:licensed, :project_owner, :status) do
false | false | false | :not_found false | false | :not_found
false | false | true | :unauthorized false | true | :unauthorized
false | true | true | :unauthorized true | false | :not_found
false | true | false | :not_found true | true | :created
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :created
end end
with_them do with_them do
before do before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed) stub_licensed_features(external_status_checks: licensed)
end end
...@@ -280,20 +245,15 @@ RSpec.describe API::StatusChecks do ...@@ -280,20 +245,15 @@ RSpec.describe API::StatusChecks do
end end
context 'when feature is disabled, unlicensed or user has permission' do context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do where(:licensed, :project_owner, :status) do
false | false | false | :not_found false | false | :not_found
false | false | true | :unauthorized false | true | :unauthorized
false | true | true | :unauthorized true | false | :not_found
false | true | false | :not_found true | true | :success
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :success
end end
with_them do with_them do
before do before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed) stub_licensed_features(external_status_checks: licensed)
end end
...@@ -311,7 +271,6 @@ RSpec.describe API::StatusChecks do ...@@ -311,7 +271,6 @@ RSpec.describe API::StatusChecks do
context 'successfully updating external approval rule' do context 'successfully updating external approval rule' do
before do before do
stub_feature_flags(ff_external_status_checks: true)
stub_licensed_features(external_status_checks: true) stub_licensed_features(external_status_checks: true)
end end
...@@ -362,20 +321,15 @@ RSpec.describe API::StatusChecks do ...@@ -362,20 +321,15 @@ RSpec.describe API::StatusChecks do
end end
context 'when feature is disabled, unlicensed or user has permission' do context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do where(:licensed, :project_owner, :status) do
false | false | false | :not_found false | false | :not_found
false | false | true | :unauthorized false | true | :unauthorized
false | true | true | :unauthorized true | false | :not_found
false | true | false | :not_found true | true | :success
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :success
end end
with_them do with_them do
before do before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed) stub_licensed_features(external_status_checks: licensed)
end end
......
...@@ -16,9 +16,9 @@ RSpec.describe 'projects/edit' do ...@@ -16,9 +16,9 @@ RSpec.describe 'projects/edit' do
end end
context 'status checks' do context 'status checks' do
context 'feature flag is disabled' do context 'feature is not available' do
before do before do
stub_feature_flags(ff_external_status_checks: false) stub_licensed_features(external_status_checks: false)
render render
end end
...@@ -26,24 +26,6 @@ RSpec.describe 'projects/edit' do ...@@ -26,24 +26,6 @@ RSpec.describe 'projects/edit' do
it 'hides the status checks area' do it 'hides the status checks area' do
expect(rendered).not_to have_content('Status check') expect(rendered).not_to have_content('Status check')
end end
end
context 'feature flag is enabled' do
before do
stub_feature_flags(ff_external_status_checks: true)
end
context 'feature is not available' do
before do
stub_licensed_features(external_status_checks: false)
render
end
it 'hides the status checks area' do
expect(rendered).not_to have_content('Status check')
end
end
context 'feature is available' do context 'feature is available' do
before do before 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